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

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2905 bytes --]

Am 17.08.2020 um 14:56 hat Max Reitz geschrieben:
> 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>
> > ---
> >  qapi/block-export.json | 7 ++++++-
> >  blockdev-nbd.c         | 2 +-
> >  2 files changed, 7 insertions(+), 2 deletions(-)
> > 
> > diff --git a/qapi/block-export.json b/qapi/block-export.json
> > index 1fdc55c53a..4ce163411f 100644
> > --- a/qapi/block-export.json
> > +++ 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?

Indeed. I think patch 15 has what you're looking for.

> Anyway, @writable might be a similar story.  A @read-only may make sense
> in general, I think.

Pulling @writable up is easier than a @read-only, but that's a naming
detail.

In general I agree, but this part isn't addressed in this series yet.
Part of the reason why this is an RFC was to find out if I should
include things like this or if we'll do it when we add another export
type or common functionality that needs the same option.

> 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.)

Do you mean that the BlockBackend should already be created by the
generic block export layer?

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]