Message ID | 20200617103018.18026-2-lma@suse.com |
---|---|
State | New |
Headers | show |
Series | Add Support for GET LBA STATUS 16 command in scsi emulation | expand |
On Wed, Jun 17, 2020 at 06:30:16PM +0800, Lin Ma wrote: > The get lba status wrapper based on the bdrv_block_status. The following > patches will add GET LBA STATUS 16 support for scsi emulation layer. > > Signed-off-by: Lin Ma <lma@suse.com> > --- > block/io.c | 43 +++++++++++++++++++++++++++++++++++++++ > include/block/block_int.h | 5 +++++ > 2 files changed, 48 insertions(+) > > diff --git a/block/io.c b/block/io.c > index df8f2a98d4..2064016b19 100644 > --- a/block/io.c > +++ b/block/io.c > @@ -2208,6 +2208,49 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, > BDRV_REQ_ZERO_WRITE | flags); > } > > +int coroutine_fn > +bdrv_co_get_lba_status(BdrvChild *child, int64_t offset, int64_t bytes, > + uint32_t *num_blocks, uint32_t *is_deallocated) Missing doc comments. > +{ > + BlockDriverState *bs = child->bs; Why does this function take a BdrvChild argument instead of BlockDriverState? Most I/O functions take BlockDriverState. > + int ret = 0; > + int64_t target_size, count = 0; > + bool first = true; > + uint8_t wanted_bit1 = 0; > + > + target_size = bdrv_getlength(bs); > + if (target_size < 0) { > + return -EIO; > + } > + > + if (offset < 0 || bytes < 0) { > + return -EIO; > + } > + > + for ( ; offset <= target_size - bytes; offset += count) { > + ret = bdrv_block_status(bs, offset, bytes, &count, NULL, NULL); > + if (ret < 0) { > + goto out; > + } > + if (first) { > + if (ret & BDRV_BLOCK_ZERO) { > + wanted_bit1 = BDRV_BLOCK_ZERO >> 1; > + *is_deallocated = 1; This is a boolean. Please use bool instead of uint32_t. Please initialize is_deallocated to false at the beginning of the function to avoid accidental uninitialized variable accesses in the caller. > + } else { > + wanted_bit1 = 0; > + } > + first = false; > + } > + if ((ret & BDRV_BLOCK_ZERO) >> 1 == wanted_bit1) { > + (*num_blocks)++; If there is a long span of allocated/deallocated blocks then this function only increments num_blocks once without counting the number of blocks. I expected something like num_blocks += pnum / block_size. What is the relationship between bytes, count, and blocks in this function? > + } else { > + break; > + } > + } > +out: > + return ret; > +} > + > /* > * Flush ALL BDSes regardless of if they are reachable via a BlkBackend or not. > */ > diff --git a/include/block/block_int.h b/include/block/block_int.h > index 791de6a59c..43f90591b9 100644 > --- a/include/block/block_int.h > +++ b/include/block/block_int.h > @@ -1296,6 +1296,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, > int64_t *pnum, > int64_t *map, > BlockDriverState **file); > +int coroutine_fn bdrv_co_get_lba_status(BdrvChild *child, > + int64_t offset, > + int64_t bytes, > + uint32_t *num_blocks, > + uint32_t *is_deallocated); Should this function be in include/block/block.h (the public API) so that any part of QEMU can call it? It's not an internal API.
diff --git a/block/io.c b/block/io.c index df8f2a98d4..2064016b19 100644 --- a/block/io.c +++ b/block/io.c @@ -2208,6 +2208,49 @@ int coroutine_fn bdrv_co_pwrite_zeroes(BdrvChild *child, int64_t offset, BDRV_REQ_ZERO_WRITE | flags); } +int coroutine_fn +bdrv_co_get_lba_status(BdrvChild *child, int64_t offset, int64_t bytes, + uint32_t *num_blocks, uint32_t *is_deallocated) +{ + BlockDriverState *bs = child->bs; + int ret = 0; + int64_t target_size, count = 0; + bool first = true; + uint8_t wanted_bit1 = 0; + + target_size = bdrv_getlength(bs); + if (target_size < 0) { + return -EIO; + } + + if (offset < 0 || bytes < 0) { + return -EIO; + } + + for ( ; offset <= target_size - bytes; offset += count) { + ret = bdrv_block_status(bs, offset, bytes, &count, NULL, NULL); + if (ret < 0) { + goto out; + } + if (first) { + if (ret & BDRV_BLOCK_ZERO) { + wanted_bit1 = BDRV_BLOCK_ZERO >> 1; + *is_deallocated = 1; + } else { + wanted_bit1 = 0; + } + first = false; + } + if ((ret & BDRV_BLOCK_ZERO) >> 1 == wanted_bit1) { + (*num_blocks)++; + } else { + break; + } + } +out: + return ret; +} + /* * Flush ALL BDSes regardless of if they are reachable via a BlkBackend or not. */ diff --git a/include/block/block_int.h b/include/block/block_int.h index 791de6a59c..43f90591b9 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1296,6 +1296,11 @@ int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, int64_t *pnum, int64_t *map, BlockDriverState **file); +int coroutine_fn bdrv_co_get_lba_status(BdrvChild *child, + int64_t offset, + int64_t bytes, + uint32_t *num_blocks, + uint32_t *is_deallocated); const char *bdrv_get_parent_name(const BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); bool blk_dev_has_removable_media(BlockBackend *blk);
The get lba status wrapper based on the bdrv_block_status. The following patches will add GET LBA STATUS 16 support for scsi emulation layer. Signed-off-by: Lin Ma <lma@suse.com> --- block/io.c | 43 +++++++++++++++++++++++++++++++++++++++ include/block/block_int.h | 5 +++++ 2 files changed, 48 insertions(+)