Re: [PATCH 4/4] block/export: add BlockExportOptions->iothread member

[Stefan Hajnoczi]

On Fri, Sep 25, 2020 at 05:01:42PM +0200, Kevin Wolf wrote:
> Am 25.09.2020 um 15:42 hat Stefan Hajnoczi geschrieben:
> > Make it possible to specify the iothread where the export will run.
> > 
> > Signed-off-by: Stefan Hajnoczi <stefanha@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 |  4 ++++
> >  block/export/export.c  | 26 +++++++++++++++++++++++++-
> >  2 files changed, 29 insertions(+), 1 deletion(-)
> > 
> > diff --git a/qapi/block-export.json b/qapi/block-export.json
> > index 87ac5117cd..eba6f6eae9 100644
> > --- a/qapi/block-export.json
> > +++ b/qapi/block-export.json
> > @@ -219,11 +219,15 @@
> >  #                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 the main loop thread. (since: 5.2)
> 
> NBD exports currently switch automatically to a different AioContext if
> another user (usually a guest device using the same node) tries to
> change the AioContext. I believe this is also the most useful mode in
> the context of the system emulator.
> 
> I can see the need for an iothread option in qemu-storage-daemon where
> usually nobody else will move the node into a different AioContext.
> 
> But we need to define the semantics more precisely and specify what
> happens if another user wants to change the AioContext later. Currently,
> the NBD export will allow this and just switch the AioContext - after
> this patch, ignoring what the user set explicitly with this new option.
> 
> I see two options to handle this more consistently:
> 
> 1. If @iothread is set, move the block node into the requested
>    AioContext, and if that fails, block-export-add fails. Other users of
>    the node will be denied to change the AioContext while the export is
>    active.
> 
>    If @iothread is not given, it behaves like today: Use whatever
>    AioContext the node is currently in and switch whenever another user
>    requests it.
> 
> 2. Add a bool option @fixed-iothread that determines whether other users
>    can change the AioContext while the export is active.
> 
>    Giving an @iothread and fixed-iothread == true means that we'll
>    enforce the given AioContext during the whole lifetime of the export.
>    With fixed-iothread == false it means that we try to move the block
>    node to the requested iothread if possible (but we won't fail if it
>    isn't possible) and will follow any other user switching the
>    AioContext of the node.
> 
>    Not giving @iothread means that we start with the current AioContext
>    of the node, and @fixed-iothread then means the same as before.
> 
> Does this make sense to you?

Thanks for raising this. #2 is more flexible. I suspect most users will
be happy with fixed-iothread = false by default (i.e. things keep
working even if the iothread is changed).

I can adjust this patch to follow those semantics.

Stefan

signature.asc
Description: PGP signature