Re: [RFC PATCH 09/22] nbd: Add writethrough to block-export-add

[Eric Blake]

On 8/17/20 7:56 AM, Max Reitz wrote:
> On 13.08.20 18:29, Kevin Wolf wrote:
> > qemu-nbd allows use of writethrough cache modes, which mean that write
> > requests made through NBD will cause a flush before they complete.
> > Expose the same functionality in block-export-add.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---

> > +++ b/qapi/block-export.json
> > @@ -167,10 +167,15 @@
> >   # Describes a block export, i.e. how single node should be exported on an
> >   # external interface.
> >   #
> > +# @writethrough: If true, caches are flushed after every write request to the
> > +#                export before completion is signalled. (since: 5.2;
> > +#                default: false)
> > +#
> >   # Since: 4.2
> >   ##
> >   { 'union': 'BlockExportOptions',
> > -  'base': { 'type': 'BlockExportType' },
> > +  'base': { 'type': 'BlockExportType',
> > +            '*writethrough': 'bool' },
> >     'discriminator': 'type',
> >     'data': {
> >         'nbd': 'BlockExportOptionsNbd'
>
> Hm.  I find it weird to have @writethrough in the base but @device in
> the specialized class.
>
> I think everything that will be common to all block exports should be in
> the base, and that probably includes a node-name.  I’m aware that will
> make things more tedious in the code, but perhaps it would be a nicer
> interface in the end.  Or is the real problem that that would create
> problems in the storage daemon’s command line interface, because then
> the specialized (legacy) NBD interface would no longer be compatible
> with the new generalized block export interface?
>
> Anyway, @writable might be a similar story.  A @read-only may make sense
> in general, I think.

And maybe even auto-read-only.  As for @writable vs. @read-only, that's 
a choice of spelling, but we don't want both; there's also the question 
of which default is saner (having to opt-in to allowing writes is more 
verbose than defaulting to allowing writes when possible, but arguably 
saner as it is less risk of unintended writes when you forgot to specify 
the option; @auto-read-only can help in choosing nicer defaults).

> Basically, I think that the export code should be separate from the code
> setting up the BlockBackend that should be exported, so all options
> regarding that BB should be common; and those options are @node-name,
> @writethrough, and @read-only.  (And perhaps other things like
> @resizable, too, even though that isn’t something to consider for NBD.)

NBD isn't resizable yet, but extending the protocol to let it become so 
is on my TODO list.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org