| Message ID | 1464128732-12667-13-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Am 25.05.2016 um 00:25 hat Eric Blake geschrieben: > Another step on our continuing quest to switch to byte-based > interfaces. > > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > block/vmdk.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index 8494d63..284d7a0 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -1704,15 +1704,14 @@ static int vmdk_write_compressed(BlockDriverState *bs, > } > } > > -static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs, > - int64_t sector_num, > - int nb_sectors, > - BdrvRequestFlags flags) > +static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs, > + int64_t offset, > + int count, > + BdrvRequestFlags flags) > { > int ret; > BDRVVmdkState *s = bs->opaque; > - uint64_t offset = sector_num * BDRV_SECTOR_SIZE; > - uint64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; > + uint64_t bytes = count; That's an unnecessary variable again. Whether you decide to change it or not: Reviewed-by: Kevin Wolf <kwolf@redhat.com>
On 05/25/2016 08:23 AM, Kevin Wolf wrote: > Am 25.05.2016 um 00:25 hat Eric Blake geschrieben: >> Another step on our continuing quest to switch to byte-based >> interfaces. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> --- >> block/vmdk.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/block/vmdk.c b/block/vmdk.c >> index 8494d63..284d7a0 100644 >> --- a/block/vmdk.c >> +++ b/block/vmdk.c >> @@ -1704,15 +1704,14 @@ static int vmdk_write_compressed(BlockDriverState *bs, >> } >> } >> >> -static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs, >> - int64_t sector_num, >> - int nb_sectors, >> - BdrvRequestFlags flags) >> +static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs, >> + int64_t offset, >> + int count, >> + BdrvRequestFlags flags) >> { >> int ret; >> BDRVVmdkState *s = bs->opaque; >> - uint64_t offset = sector_num * BDRV_SECTOR_SIZE; >> - uint64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; >> + uint64_t bytes = count; > > That's an unnecessary variable again. Whether you decide to change it or > not: > > Reviewed-by: Kevin Wolf <kwolf@redhat.com> Unnecessary, except that it is 64-bit instead of the block layer interface 32-bit, and I didn't want to have to think too hard about how 'bytes' was used in the rest of the function if I used the narrower type from the get-go. I also think that 'int count' is fishy, because it forces us to think about negative values and placating code sanitizers on undefined shift values; maybe we'd be better with making all byte interfaces use 'uint32_t' (but still limiting ourselves to 0x80000000 or 2G for any power-of-two limit, and 0xffffffff size transactions would not be possible if request_alignment is larger than 1). If we made that switch, I'd still want to keep 0 as a no-op transaction, and not a special case for a 4G transaction. Still, now might be the time to do it.
diff --git a/block/vmdk.c b/block/vmdk.c index 8494d63..284d7a0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1704,15 +1704,14 @@ static int vmdk_write_compressed(BlockDriverState *bs, } } -static int coroutine_fn vmdk_co_write_zeroes(BlockDriverState *bs, - int64_t sector_num, - int nb_sectors, - BdrvRequestFlags flags) +static int coroutine_fn vmdk_co_pwrite_zeroes(BlockDriverState *bs, + int64_t offset, + int count, + BdrvRequestFlags flags) { int ret; BDRVVmdkState *s = bs->opaque; - uint64_t offset = sector_num * BDRV_SECTOR_SIZE; - uint64_t bytes = nb_sectors * BDRV_SECTOR_SIZE; + uint64_t bytes = count; qemu_co_mutex_lock(&s->lock); /* write zeroes could fail if sectors not aligned to cluster, test it with @@ -2403,7 +2402,7 @@ static BlockDriver bdrv_vmdk = { .bdrv_co_preadv = vmdk_co_preadv, .bdrv_co_pwritev = vmdk_co_pwritev, .bdrv_write_compressed = vmdk_write_compressed, - .bdrv_co_write_zeroes = vmdk_co_write_zeroes, + .bdrv_co_pwrite_zeroes = vmdk_co_pwrite_zeroes, .bdrv_close = vmdk_close, .bdrv_create = vmdk_create, .bdrv_co_flush_to_disk = vmdk_co_flush,
Another step on our continuing quest to switch to byte-based interfaces. Signed-off-by: Eric Blake <eblake@redhat.com> --- block/vmdk.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)