Re: [Qemu-block] [Qemu-devel] [PATCH v2 3/5] vvfat: Replace bdrv_{read, write}() with bdrv_{pread, pwrite}()

[Vladimir Sementsov-Ogievskiy]

01.05.2019 21:13, Alberto Garcia wrote:
> There's only a couple of bdrv_read() and bdrv_write() calls left in
> the vvfat code, and they can be trivially replaced with the byte-based
> bdrv_pread() and bdrv_pwrite().
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>   block/vvfat.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vvfat.c b/block/vvfat.c
> index 5f66787890..253cc716dd 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -1494,8 +1494,8 @@ static int vvfat_read(BlockDriverState *bs, int64_t 
> sector_num,
>                   DLOG(fprintf(stderr, "sectors %" PRId64 "+%" PRId64
>                                " allocated\n", sector_num,
>                                n >> BDRV_SECTOR_BITS));
> -                if (bdrv_read(s->qcow, sector_num, buf + i * 0x200,
> -                              n >> BDRV_SECTOR_BITS)) {
> +                if (bdrv_pread(s->qcow, sector_num * BDRV_SECTOR_SIZE,
> +                               buf + i * 0x200, n) < 0) {

Shouldn't we use QEMU_ALIGN_DOWN(n, BDRV_SECTOR_SIZE) ?
Could bdrv_is_allocated give unaligned n?

>                       return -1;
>                   }
>                   i += (n >> BDRV_SECTOR_BITS) - 1;
> @@ -1983,8 +1983,9 @@ static uint32_t 
> get_cluster_count_for_direntry(BDRVVVFATState* s,
>                           if (res) {
>                               return -1;
>                           }
> -                        res = bdrv_write(s->qcow, offset, s->cluster_buffer, 
> 1);
> -                        if (res) {
> +                        res = bdrv_pwrite(s->qcow, offset * BDRV_SECTOR_SIZE,
> +                                          s->cluster_buffer, 
> BDRV_SECTOR_SIZE);
> +                        if (res < 0) {
>                               return -2;
>                           }
>                       }
> @@ -3050,7 +3051,8 @@ DLOG(checkpoint());
>        * Use qcow backend. Commit later.
>        */
>   DLOG(fprintf(stderr, "Write to qcow backend: %d + %d\n", (int)sector_num, 
> nb_sectors));
> -    ret = bdrv_write(s->qcow, sector_num, buf, nb_sectors);
> +    ret = bdrv_pwrite(s->qcow, sector_num * BDRV_SECTOR_SIZE, buf,
> +                      nb_sectors * BDRV_SECTOR_SIZE);
>       if (ret < 0) {
>           fprintf(stderr, "Error writing to qcow backend\n");
>           return ret;
> 


-- 
Best regards,
Vladimir