Re: [Qemu-block] [PATCH 1/1] nbd: implement bdrv_get_info callback

[Edgar Kaziakhmedov]

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 <address@hidden>
> 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.