Re: [PATCH 03/11] block/block-gen.h: bind monitor

[Vladimir Sementsov-Ogievskiy]

24.04.2021 08:23, Markus Armbruster wrote:
> Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes:
>
> > If we have current monitor, let's bind it to wrapper coroutine too.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   block/block-gen.h | 10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/block/block-gen.h b/block/block-gen.h
> > index c1fd3f40de..61f055a8cc 100644
> > --- a/block/block-gen.h
> > +++ b/block/block-gen.h
> > @@ -27,6 +27,7 @@
> >   #define BLOCK_BLOCK_GEN_H
> >   
> >   #include "block/block_int.h"
> > +#include "monitor/monitor.h"
> >   
> >   /* Base structure for argument packing structures */
> >   typedef struct AioPollCo {
> > @@ -38,11 +39,20 @@ typedef struct AioPollCo {
> >   
> >   static inline int aio_poll_co(AioPollCo *s)
> >   {
> > +    Monitor *mon = monitor_cur();
>
> This gets the currently executing coroutine's monitor from the hash
> table.
>
> >       assert(!qemu_in_coroutine());
> >   
> > +    if (mon) {
> > +        monitor_set_cur(s->co, mon);
>
> This writes it back.  No-op, since the coroutine hasn't changed.  Why?

No. s->co != qemu_corotuine_current(), so it's not a write back, but creating a 
new entry in the hash map. s->co is a new coroutine which we are going to start.

> > +    }
> > +
> >       aio_co_enter(s->ctx, s->co);
> >       AIO_WAIT_WHILE(s->ctx, s->in_progress);
> >   
> > +    if (mon) {
> > +        monitor_set_cur(s->co, NULL);
>
> This removes s->co's monitor from the hash table.  Why?
>
> > +    }
> > +
> >       return s->ret;
> >   }

If I comment the new code of this patch (keeping the whole series applied), 249 
fails, as error message goes simply to stderr, not to monitor:

249   fail       [11:56:54] [11:56:54]   0.3s   (last: 0.2s)  output mismatch 
(see 249.out.bad)
--- /work/src/qemu/up/hmp-qemu-io/tests/qemu-iotests/249.out
+++ 249.out.bad
@@ -9,7 +9,8 @@

 { 'execute': 'human-monitor-command',
        'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}

 === Run block-commit on base using an invalid filter node name

@@ -24,7 +25,8 @@

 { 'execute': 'human-monitor-command',
        'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}

 === Run block-commit on base using the default filter node name

@@ -43,5 +45,6 @@

 { 'execute': 'human-monitor-command',
        'arguments': {'command-line': 'qemu-io none0 "aio_write 0 2k"'}}
-{"return": "Block node is read-onlyrn"}
+QEMU_PROG: Block node is read-only
+{"return": ""}
 *** done
Failures: 249
Failed 1 of 1 iotests



--
Best regards,
Vladimir