Re: [PATCH 1/4] block: Add bdrv_co_get_lba_status

[Claudio Fontana <cfontana@suse.de>]

Hi Lin,

just a couple of things, leaving the core aspects to more knowledgeable people in this area to address.

In general, you want all patches in the series to build correctly.

Applying only PATCH 1/4 breaks the build for me with:

/home/claudio/git/qemu/block/io.c:2190:1: error: no previous prototype for ‘bdrv_co_get_lba_status’ [-Werror=missing-prototypes]
 bdrv_co_get_lba_status(BdrvChild *child, int64_t offset, int64_t bytes,
 ^~~~~~~~~~~~~~~~~~~~~~
/home/claudio/git/qemu/block/io.c: In function ‘bdrv_co_get_lba_status’:
/home/claudio/git/qemu/block/io.c:2194:9: error: ‘ret’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
     int ret;
         ^~~
cc1: all warnings being treated as errors

On 6/2/20 9:41 AM, 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 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/block/io.c b/block/io.c
> index 121ce17a49..dacc3c2471 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -2186,6 +2186,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;
> +    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;;

extra ; at the end of the line

> +                *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.
>   */
> 

Ciao,

CLaudio