On Tue, Sep 29, 2020 at 08:07:38AM -0500, Eric Blake wrote:
> On 9/29/20 7:55 AM, Stefan Hajnoczi wrote:
> > Make it possible to specify the iothread where the export will run. By
> > default the block node can be moved to other AioContexts later and the
> > export will follow. The fixed-iothread option forces strict behavior
> > that prevents changing AioContext while the export is active. See the
> > QAPI docs for details.
> >
> > Signed-off-by: Stefan Hajnoczi <stefa...@redhat.com>
> > ---
> > Note the x-blockdev-set-iothread QMP command can be used to do the same,
> > but not from the command-line. And it requires sending an additional
> > command.
> >
> > In the long run vhost-user-blk will support per-virtqueue iothread
> > mappings. But for now a single iothread makes sense and most other
> > transports will just use one iothread anyway.
> > ---
> > qapi/block-export.json | 11 ++++++++++
> > block/export/export.c | 31 +++++++++++++++++++++++++++-
> > block/export/vhost-user-blk-server.c | 5 ++++-
> > nbd/server.c | 2 --
> > 4 files changed, 45 insertions(+), 4 deletions(-)
> >
> > diff --git a/qapi/block-export.json b/qapi/block-export.json
> > index 87ac5117cd..e2cb21f5f1 100644
> > --- a/qapi/block-export.json
> > +++ b/qapi/block-export.json
> > @@ -219,11 +219,22 @@
> > # export before completion is signalled. (since: 5.2;
> > # default: false)
> > #
> > +# @iothread: The name of the iothread object where the export will run. The
> > +# default is to use the thread currently associated with the #
>
> Stray #
>
> > +# block node. (since: 5.2)
> > +#
> > +# @fixed-iothread: True prevents the block node from being moved to another
> > +# thread while the export is active. If true and
> > @iothread is
> > +# given, export creation fails if the block node cannot be
> > +# moved to the iothread. The default is false.
> > +#
>
> Missing a '(since 5.2)' tag. (Hmm, we're inconsistent on whether it is
> 'since 5.2' or 'since: 5.2' inside () parentheticals; Markus, is that
> something we should be cleaning up as part of the conversion to rST?)
>
> > @@ -63,10 +64,11 @@ static const BlockExportDriver
> > *blk_exp_find_driver(BlockExportType type)
> > BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp)
> > {
> > + bool fixed_iothread = export->has_fixed_iothread &&
> > export->fixed_iothread;
>
> Technically, our QAPI code guarantees that export->fixed_iothread is false
> if export->has_fixed_iothread is false. And someday I'd love to let QAPI
> express default values for bools so that we don't need a has_FOO field when
> a default has been expressed. But neither of those points affect this
> patch; what you have is correct even if it is verbose.
>
> Otherwise looks reasonable.Great, thanks for pointing this out. I'll wait for comments from Kevin. These things could be fixed when merging. Stefan
signature.asc
Description: PGP signature
