Re: [PATCH v3 2/9] qapi: make blockdev-add a coroutine command

[Vladimir Sementsov-Ogievskiy]

06.09.2021 22:28, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
> > We are going to support nbd reconnect on open in a next commit. This
> > means that we want to do several connection attempts during some time.
> > And this should be done in a coroutine, otherwise we'll stuck.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   qapi/block-core.json | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 06674c25c9..6e4042530a 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -4219,7 +4219,8 @@
> >   # <- { "return": {} }
> >   #
> >   ##
> > -{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true }
> > +{ 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true,
> > +  'coroutine': true }
> >   
> >   ##
> >   # @blockdev-reopen:
>
> Why is this safe?
>
> Prior discusson:
> Message-ID: <87lfq0yp9v.fsf@dusky.pond.sub.org>
> https://lists.gnu.org/archive/html/qemu-devel/2020-01/msg04921.html

Hmm.. I'm afraid, that I can't prove that it's safe. At least it will mean to 
audit .bdrv_open() of all block drivers.. And nothing prevents creating new 
incompatible drivers in future..

On the other hand, looking at qmp_blockdev_add, bdrv_open() is the only thing 
of interest.

And theoretically, bdrv_open() should work in coroutine context. We do call 
this function from coroutine_fn functions sometimes. So, maybe, if in some 
circumstances, bdrv_open() is not compatible with coroutine context, we can 
consider it as a bug? And fix it later, if it happen?

--
Best regards,
Vladimir