Message ID | f64d9b6b1bc90d45c9be5aae63176b34879182c2.1556732434.git.berto@igalia.com |
---|---|
State | New |
Headers | show |
Series | Remove bdrv_read() and bdrv_write() | expand |
01.05.2019 21:13, Alberto Garcia wrote: > There's only a couple of bdrv_read() and bdrv_write() calls left in > the vdi code, and they can be trivially replaced with the byte-based > bdrv_pread() and bdrv_pwrite(). > > Signed-off-by: Alberto Garcia <berto@igalia.com> > --- > block/vdi.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/block/vdi.c b/block/vdi.c > index e1c42ad732..9caeb50dd1 100644 > --- a/block/vdi.c > +++ b/block/vdi.c > @@ -171,6 +171,8 @@ typedef struct { > uint64_t unused2[7]; > } QEMU_PACKED VdiHeader; > > +QEMU_BUILD_BUG_ON(sizeof(VdiHeader) != 512); > + > typedef struct { > /* The block map entries are little endian (even in memory). */ > uint32_t *bmap; > @@ -384,7 +386,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, > > logout("\n"); > > - ret = bdrv_read(bs->file, 0, (uint8_t *)&header, 1); > + ret = bdrv_pread(bs->file, 0, (uint8_t *)&header, sizeof(header)); bdrv_pread parameter buf parameter is void, so (uint8_t *) conversion is not needed (and even confusing) > if (ret < 0) { > goto fail; > } > @@ -484,8 +486,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > > - ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, > - bmap_size); > + ret = bdrv_pread(bs->file, header.offset_bmap, (uint8_t *)s->bmap, > + bmap_size * SECTOR_SIZE); and here > if (ret < 0) { > goto fail_free_bmap; > } > @@ -704,7 +706,7 @@ nonallocating_write: > assert(VDI_IS_ALLOCATED(bmap_first)); > *header = s->header; > vdi_header_to_le(header); > - ret = bdrv_write(bs->file, 0, block, 1); > + ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); maybe, more self-descriptive: ret = bdrv_pwrite(bs->file, 0, header, sizeof(*header)); with at least extra conversion dropped: Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > g_free(block); > block = NULL; > > @@ -722,10 +724,11 @@ nonallocating_write: > base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE; > logout("will write %u block map sectors starting from entry %u\n", > n_sectors, bmap_first); > - ret = bdrv_write(bs->file, offset, base, n_sectors); > + ret = bdrv_pwrite(bs->file, offset * SECTOR_SIZE, base, > + n_sectors * SECTOR_SIZE); > } > > - return ret; > + return ret < 0 ? ret : 0; > } > > static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options, >
Am 06.05.2019 um 18:38 hat Vladimir Sementsov-Ogievskiy geschrieben: > 01.05.2019 21:13, Alberto Garcia wrote: > > There's only a couple of bdrv_read() and bdrv_write() calls left in > > the vdi code, and they can be trivially replaced with the byte-based > > bdrv_pread() and bdrv_pwrite(). > > > > Signed-off-by: Alberto Garcia <berto@igalia.com> > > --- > > block/vdi.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/block/vdi.c b/block/vdi.c > > index e1c42ad732..9caeb50dd1 100644 > > --- a/block/vdi.c > > +++ b/block/vdi.c > > @@ -171,6 +171,8 @@ typedef struct { > > uint64_t unused2[7]; > > } QEMU_PACKED VdiHeader; > > > > +QEMU_BUILD_BUG_ON(sizeof(VdiHeader) != 512); > > + > > typedef struct { > > /* The block map entries are little endian (even in memory). */ > > uint32_t *bmap; > > @@ -384,7 +386,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, > > > > logout("\n"); > > > > - ret = bdrv_read(bs->file, 0, (uint8_t *)&header, 1); > > + ret = bdrv_pread(bs->file, 0, (uint8_t *)&header, sizeof(header)); > > bdrv_pread parameter buf parameter is void, so (uint8_t *) conversion is not needed > (and even confusing) > > > if (ret < 0) { > > goto fail; > > } > > @@ -484,8 +486,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, > > goto fail; > > } > > > > - ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, > > - bmap_size); > > + ret = bdrv_pread(bs->file, header.offset_bmap, (uint8_t *)s->bmap, > > + bmap_size * SECTOR_SIZE); > > and here > > > if (ret < 0) { > > goto fail_free_bmap; > > } > > @@ -704,7 +706,7 @@ nonallocating_write: > > assert(VDI_IS_ALLOCATED(bmap_first)); > > *header = s->header; > > vdi_header_to_le(header); > > - ret = bdrv_write(bs->file, 0, block, 1); > > + ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); > > maybe, more self-descriptive: > ret = bdrv_pwrite(bs->file, 0, header, sizeof(*header)); > > > with at least extra conversion dropped: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> I'll drop the casts while applying. Kevin
diff --git a/block/vdi.c b/block/vdi.c index e1c42ad732..9caeb50dd1 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -171,6 +171,8 @@ typedef struct { uint64_t unused2[7]; } QEMU_PACKED VdiHeader; +QEMU_BUILD_BUG_ON(sizeof(VdiHeader) != 512); + typedef struct { /* The block map entries are little endian (even in memory). */ uint32_t *bmap; @@ -384,7 +386,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, logout("\n"); - ret = bdrv_read(bs->file, 0, (uint8_t *)&header, 1); + ret = bdrv_pread(bs->file, 0, (uint8_t *)&header, sizeof(header)); if (ret < 0) { goto fail; } @@ -484,8 +486,8 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags, goto fail; } - ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, - bmap_size); + ret = bdrv_pread(bs->file, header.offset_bmap, (uint8_t *)s->bmap, + bmap_size * SECTOR_SIZE); if (ret < 0) { goto fail_free_bmap; } @@ -704,7 +706,7 @@ nonallocating_write: assert(VDI_IS_ALLOCATED(bmap_first)); *header = s->header; vdi_header_to_le(header); - ret = bdrv_write(bs->file, 0, block, 1); + ret = bdrv_pwrite(bs->file, 0, block, sizeof(VdiHeader)); g_free(block); block = NULL; @@ -722,10 +724,11 @@ nonallocating_write: base = ((uint8_t *)&s->bmap[0]) + bmap_first * SECTOR_SIZE; logout("will write %u block map sectors starting from entry %u\n", n_sectors, bmap_first); - ret = bdrv_write(bs->file, offset, base, n_sectors); + ret = bdrv_pwrite(bs->file, offset * SECTOR_SIZE, base, + n_sectors * SECTOR_SIZE); } - return ret; + return ret < 0 ? ret : 0; } static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
There's only a couple of bdrv_read() and bdrv_write() calls left in the vdi code, and they can be trivially replaced with the byte-based bdrv_pread() and bdrv_pwrite(). Signed-off-by: Alberto Garcia <berto@igalia.com> --- block/vdi.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)