| Message ID | 20210423214033.474034-4-vsementsov@virtuozzo.com |
|---|---|
| State | New |
| Headers | show |
| Series | qemu-io-cmds: move to coroutine | expand |
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? > + } > + > 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; > }
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
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 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. Ah, that's what I missed. Thanks! [...]
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(); assert(!qemu_in_coroutine()); + if (mon) { + monitor_set_cur(s->co, mon); + } + aio_co_enter(s->ctx, s->co); AIO_WAIT_WHILE(s->ctx, s->in_progress); + if (mon) { + monitor_set_cur(s->co, NULL); + } + return s->ret; }
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(+)