Re: [Qemu-devel] [PATCH 20/25] block/nbd: Comment on discard/flush silently failing

[Max Reitz]

On 2015-03-16 at 10:44, Paolo Bonzini wrote:
> On 16/03/2015 14:58, Max Reitz wrote:
> > > > diff --git a/block/nbd-client.c b/block/nbd-client.c
> > > > index be6803d..ab13607 100644
> > > > --- a/block/nbd-client.c
> > > > +++ b/block/nbd-client.c
> > > > @@ -315,6 +315,7 @@ int nbd_client_co_flush(BlockDriverState *bs)
> > > >        ssize_t ret;
> > > >          if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) {
> > > > +        /* This mirrors the behavior of bdrv_co_flush() in block.c */
> > > >            return 0;
> > > >        }
> > > >    @@ -350,6 +351,7 @@ int nbd_client_co_discard(BlockDriverState *bs,
> > > > int64_t sector_num,
> > > >        ssize_t ret;
> > > >          if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
> > > > +        /* This mirrors the behavior of bdrv_co_discard() in block.c */
> > > >            return 0;
> > > Should this return -EOPNOTSUPP instead?
> > That's what this patch is for. I asked myself the same thing, and it
> > turns out, the functions deliberately return 0 because bdrv_co_flush()
> > and bdrv_co_discard() do the same if the block driver does not support
> > these functions at all, so that's why I'm adding these comments.
> Right, but a better model than block.c should be for example
> block/raw-posix.c, which returns ENOTSUP (I checked now...).

Maybe we should catch ENOTSUP in bdrv_co_discard() and override it?

Max