Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
On 02/02/2018 08:06 AM, Edgar Kaziakhmedov wrote: However, it would be nice to remove can_write_zeroes_with_unmap from BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return !!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you think? >> Actually, I may even just give a shot at writing this alternative patch, >> to make Kevin's decision easier. > But actually qcow2 performs some checks for version inside get_info > callback before setting can_write_zeroes_with_unmap flag, > so we can't take into account such checks in > bdrv_can_write_zeroes_with_unmap subroutine. Therefore, I don't think it > is possible to do it like that. Here's the patch I proposed (it looks like I forgot to CC you): https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg06471.html -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
On 02/02/2018 05:15 PM, Eric Blake wrote: On 02/02/2018 08:06 AM, Edgar Kaziakhmedov wrote: However, it would be nice to remove can_write_zeroes_with_unmap from BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return !!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you think? Actually, I may even just give a shot at writing this alternative patch, to make Kevin's decision easier. But actually qcow2 performs some checks for version inside get_info callback before setting can_write_zeroes_with_unmap flag, so we can't take into account such checks in bdrv_can_write_zeroes_with_unmap subroutine. Therefore, I don't think it is possible to do it like that. Here's the patch I proposed (it looks like I forgot to CC you): https://lists.gnu.org/archive/html/qemu-devel/2018-01/msg06471.html Yes, it was possible to move check to open, ok, get it.
Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
On 01/26/2018 05:28 PM, Eric Blake wrote:
On 01/26/2018 06:39 AM, Edgar Kaziakhmedov wrote:
PIng
So, let me know if I need to make any changes in patch
On 1/18/18 1:09 PM, Paolo Bonzini wrote:
On 18/01/2018 12:51, Edgar Kaziakhmedov wrote:
+static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+ if (bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP) {
+ bdi->can_write_zeroes_with_unmap = true;
+ }
+ return 0;
+}
+
Other drivers set the flag always, while NBD only sets it if the server
knows the flag.
Well, other drivers may be able to always implement it (NBD can only
implement it if the server supports WRITE_ZEROES - and I'm even in the
middle of working up an nbdkit patch [1] that makes it easier to write
an NBD server that specifically does not support WRITE_ZEROES to make
code paths like this easier to test)
[1]
I think NBD is more correct, so:
Reviewed-by: Paolo Bonzini
Agreed; I'm fine queueing this on my NBD queue, except I'd first like to
hear Kevin's opinion:
However, it would be nice to remove can_write_zeroes_with_unmap from
BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return
!!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you
think?
Actually, I may even just give a shot at writing this alternative patch,
to make Kevin's decision easier.
But actually qcow2 performs some checks for version inside get_info
callback before setting can_write_zeroes_with_unmap flag,
so we can't take into account such checks in
bdrv_can_write_zeroes_with_unmap subroutine. Therefore, I don't think it
is possible to do it like that.
Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
On 01/26/2018 06:39 AM, Edgar Kaziakhmedov wrote:
> PIng
>
> So, let me know if I need to make any changes in patch
>
> On 1/18/18 1:09 PM, Paolo Bonzini wrote:
>> On 18/01/2018 12:51, Edgar Kaziakhmedov wrote:
>>> +static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>>> +{
>>> + if (bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP) {
>>> + bdi->can_write_zeroes_with_unmap = true;
>>> + }
>>> + return 0;
>>> +}
>>> +
>> Other drivers set the flag always, while NBD only sets it if the server
>> knows the flag.
Well, other drivers may be able to always implement it (NBD can only
implement it if the server supports WRITE_ZEROES - and I'm even in the
middle of working up an nbdkit patch [1] that makes it easier to write
an NBD server that specifically does not support WRITE_ZEROES to make
code paths like this easier to test)
[1]
>>
>> I think NBD is more correct, so:
>>
>> Reviewed-by: Paolo Bonzini
Agreed; I'm fine queueing this on my NBD queue, except I'd first like to
hear Kevin's opinion:
>>
>> However, it would be nice to remove can_write_zeroes_with_unmap from
>> BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return
>> !!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you
>> think?
Actually, I may even just give a shot at writing this alternative patch,
to make Kevin's decision easier.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
PIng
So, let me know if I need to make any changes in patch
On 1/18/18 1:09 PM, Paolo Bonzini wrote:
On 18/01/2018 12:51, Edgar Kaziakhmedov wrote:
+static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
+{
+if (bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP) {
+bdi->can_write_zeroes_with_unmap = true;
+}
+return 0;
+}
+
Other drivers set the flag always, while NBD only sets it if the server
knows the flag.
I think NBD is more correct, so:
Reviewed-by: Paolo Bonzini
However, it would be nice to remove can_write_zeroes_with_unmap from
BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return
!!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you
think?
Paolo
.
Re: [Qemu-devel] [PATCH 1/1] nbd: implement bdrv_get_info callback
On 18/01/2018 12:51, Edgar Kaziakhmedov wrote:
>
> +static int nbd_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
> +{
> +if (bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP) {
> +bdi->can_write_zeroes_with_unmap = true;
> +}
> +return 0;
> +}
> +
Other drivers set the flag always, while NBD only sets it if the server
knows the flag.
I think NBD is more correct, so:
Reviewed-by: Paolo Bonzini
However, it would be nice to remove can_write_zeroes_with_unmap from
BlockDriverInfo, and make bdrv_can_write_zeroes_with_unmap just return
!!(bs->supported_zero_flags & BDRV_REQ_MAY_UNMAP). Kevin, what do you
think?
Paolo
