Re: [Qemu-devel] [PATCH 4/4] qemu-img: conditionally discard target on convert

[Peter Lieven <pl@kamp.de>]

On 18.07.2013 15:52, Paolo Bonzini wrote:
> Il 18/07/2013 15:29, Peter Lieven ha scritto:
>>>> If the driver would have a better method of writing zeroes than
>>>> discard it simply should not set bdi->write_zeroes_w_discard = 1.
>>> If the driver had a better method of writing zeroes than discard, it
>>> simply should ignore the BDRV_MAY_UNMAP (or BDRV_MAY_DISCARD) flag in
>>> its bdrv_write_zeros implementation.
>> ok, but this would require an individual patch in every driver, wouldn't
>> it.  i am ok with that.
> Yes (making the drivers return the flag in the BDI would also require
> per-driver patches).
we still might need a hint for qemu-img convert that the driver does support
writing zeroes by unmap because using write_zeroes in the main loop
might result in unaligned requests that the target is not able to unmap.
and to avoid writing several blocks twice by first writing all zeroes
to the target and then writing all data blocks again I would need to keep the loop
at the beginning of qemu-img convert to write zeroes with correct
alignment and granularity if the driver supports write_zeroes_w_discard.

>
> BDRV_MAY_UNMAP is an advisory flag, that the driver is free to ignore
> (just like bdrv_discard can be ignored altogether).
>
> block.c probably should avoid passing down BDRV_MAY_UNMAP altogether if
> the BDRV_O_UNMAP flag is not set.
>
>> the BDRV_MAY_DISCARD flag is at the end a hint if the driver can optimize
>> writing zeroes by a discard or if real zeroes should be written e.g. to
>> sanitize the device?
>>
>> talking for iscsi:
>> bdrv->discard can remain to use UNMAP and silently fail if lbpu == 0.
>> bdrv->write_zeroes will use writesame16 and set the unmap flag only if
>> BDRV_MAY_DISCARD == 1 and BDRV_O_UNMAP == 1 and lbprz == 1.
>> in case lbpws == 0 it will return -ENOSUP.
> Almost: for LBPWS == 0 it can use WRITE SAME(16) without the unmap flag.
>   If that doesn't work either it can return -ENOTSUP.
Ok, i was not aware that WRITE SAME(16) is always available.

Peter