diff mbox series

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

Message ID 20210423214033.474034-4-vsementsov@virtuozzo.com (mailing list archive)
State New, archived
Headers show
Series qemu-io-cmds: move to coroutine | expand

Commit Message

Vladimir Sementsov-Ogievskiy April 23, 2021, 9:40 p.m. UTC
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(+)

Comments

Markus Armbruster April 24, 2021, 5:23 a.m. UTC | #1
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;
>  }
Vladimir Sementsov-Ogievskiy April 26, 2021, 9:12 a.m. UTC | #2
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
Markus Armbruster April 26, 2021, 12:23 p.m. UTC | #3
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 mbox series

Patch

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;
 }