Re: [Qemu-devel] [PATCH v2 13/20] block/iscsi: use UNMAP to write zeroes if LBPRZ=1

[Peter Lieven <pl@kamp.de>]

On 19.11.2013 18:07, Paolo Bonzini wrote:
> The latest revision of SCSI SBC clarifies the semantics of LBPRZ
> in a way that lets the iscsi driver remove the distinction between
> bdrv_unallocated_blocks_are_zero and bdrv_can_write_zeroes_with_unmap.
> See Table 8:
>
>      "[If] the LBA became mapped as the result of an autonomous transition,
>      and no write command has specified that LBA since the LBA was mapped,
>      [a read operation returns the] logical block data that would be returned
>      if that autonomous transition had not occurred and the LBA had remained
>      unmapped."
>
> Thus, we can assume that even after an UNMAP command, unallocated blocks
> return zero.  The distinction may remain for other drivers, for example
> qcow2 or qed or vmdk.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   block/iscsi.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 93fee6d..63b451d 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -984,6 +984,9 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
>   
>       if ((flags & BDRV_REQ_MAY_UNMAP) && !iscsilun->lbp.lbpws) {
>           /* WRITE SAME with UNMAP is not supported by the target */
> +        if (iscsilun->lbp.lbpu && iscsilun->lbprz) {
> +            return iscsi_co_discard(bs, sector_num, nb_sectors);
> +        }
>           return -ENOTSUP;
>       }
>   
> @@ -1579,7 +1582,7 @@ static int iscsi_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>   {
>       IscsiLun *iscsilun = bs->opaque;
>       bdi->unallocated_blocks_are_zero = !!iscsilun->lbprz;
> -    bdi->can_write_zeroes_with_unmap = iscsilun->lbprz && iscsilun->lbp.lbpws;
> +    bdi->can_write_zeroes_with_unmap = !!iscsilun->lbprz;
>       return 0;
>   }
>   
I would definetly not do that! I have seen at least my target to execute only parts of a discard request.
Additionally in our semantic a discard request may silently fail. In general this could lead to data corruption
due to broken implementations.

Peter