[PATCH] qapi: Misc cleanups to migrate QAPIs

Het Gala posted 1 patch 8 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20240216195659.189091-1-het.gala@nutanix.com
Maintainers: Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>
qapi/migration.json | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
[PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Het Gala 8 months, 2 weeks ago
Signed-off-by: Het Gala <het.gala@nutanix.com>
---
 qapi/migration.json | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 5a565d9b8d..5756e650b0 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1728,6 +1728,7 @@
 #
 # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
 # <- { "return": {} }
+#
 # -> { "execute": "migrate",
 #      "arguments": {
 #          "channels": [ { "channel-type": "main",
@@ -1796,19 +1797,19 @@
 #
 #     3. The uri format is the same as for -incoming
 #
-#     5. For now, number of migration streams is restricted to one,
+#     4. For now, number of migration streams is restricted to one,
 #        i.e number of items in 'channels' list is just 1.
 #
-#     4. The 'uri' and 'channels' arguments are mutually exclusive;
+#     5. The 'uri' and 'channels' arguments are mutually exclusive;
 #        exactly one of the two should be present.
 #
 # Example:
 #
 # -> { "execute": "migrate-incoming",
-#      "arguments": { "uri": "tcp::4446" } }
+#      "arguments": { "uri": "tcp:0:4446" } }
 # <- { "return": {} }
 #
-# -> { "execute": "migrate",
+# -> { "execute": "migrate-incoming",
 #      "arguments": {
 #          "channels": [ { "channel-type": "main",
 #                          "addr": { "transport": "socket",
@@ -1817,7 +1818,7 @@
 #                                    "port": "1050" } } ] } }
 # <- { "return": {} }
 #
-# -> { "execute": "migrate",
+# -> { "execute": "migrate-incoming",
 #      "arguments": {
 #          "channels": [ { "channel-type": "main",
 #                          "addr": { "transport": "exec",
@@ -1825,7 +1826,7 @@
 #                                              "/some/sock" ] } } ] } }
 # <- { "return": {} }
 #
-# -> { "execute": "migrate",
+# -> { "execute": "migrate-incoming",
 #      "arguments": {
 #          "channels": [ { "channel-type": "main",
 #                          "addr": { "transport": "rdma",
-- 
2.22.3
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Markus Armbruster 8 months, 1 week ago
You neglected to cc: migration maintainers; I'm doing that for you now.

Peter or Fabiano, please have a look.

Het Gala <het.gala@nutanix.com> writes:

> Signed-off-by: Het Gala <het.gala@nutanix.com>
> ---
>  qapi/migration.json | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 5a565d9b8d..5756e650b0 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1728,6 +1728,7 @@
>  #
>  # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
>  # <- { "return": {} }
> +#
>  # -> { "execute": "migrate",
>  #      "arguments": {
>  #          "channels": [ { "channel-type": "main",
> @@ -1796,19 +1797,19 @@
>  #
>  #     3. The uri format is the same as for -incoming
>  #
> -#     5. For now, number of migration streams is restricted to one,
> +#     4. For now, number of migration streams is restricted to one,
>  #        i.e number of items in 'channels' list is just 1.
>  #
> -#     4. The 'uri' and 'channels' arguments are mutually exclusive;
> +#     5. The 'uri' and 'channels' arguments are mutually exclusive;
>  #        exactly one of the two should be present.
>  #
>  # Example:
>  #
>  # -> { "execute": "migrate-incoming",
> -#      "arguments": { "uri": "tcp::4446" } }
> +#      "arguments": { "uri": "tcp:0:4446" } }
>  # <- { "return": {} }
>  #
> -# -> { "execute": "migrate",
> +# -> { "execute": "migrate-incoming",
>  #      "arguments": {
>  #          "channels": [ { "channel-type": "main",
>  #                          "addr": { "transport": "socket",
> @@ -1817,7 +1818,7 @@
>  #                                    "port": "1050" } } ] } }
>  # <- { "return": {} }
>  #
> -# -> { "execute": "migrate",
> +# -> { "execute": "migrate-incoming",
>  #      "arguments": {
>  #          "channels": [ { "channel-type": "main",
>  #                          "addr": { "transport": "exec",
> @@ -1825,7 +1826,7 @@
>  #                                              "/some/sock" ] } } ] } }
>  # <- { "return": {} }
>  #
> -# -> { "execute": "migrate",
> +# -> { "execute": "migrate-incoming",
>  #      "arguments": {
>  #          "channels": [ { "channel-type": "main",
>  #                          "addr": { "transport": "rdma",
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Het Gala 8 months, 1 week ago
Sorry Markus, firstly I thought its just regarding qapi documentation so 
migration maintainers might not be needed ? but then I realize the 
commit message fails to specify that

IIRC, you are one of the maintainers for qapi. So, just cc'd to you, but 
you are right, should have done to migration maintainers also.

Have we got the wrong Fabiano here ? Isn't Fabiano Rosas the migration 
maintainer ?

cc'ing to Fabiano Rosas too.

On 21/02/24 12:27 pm, Markus Armbruster wrote:
> You neglected to cc: migration maintainers; I'm doing that for you now.
>
> Peter or Fabiano, please have a look.
>
> Het Gala <het.gala@nutanix.com> writes:
>
>> Signed-off-by: Het Gala <het.gala@nutanix.com>
>> ---
>>   qapi/migration.json | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 5a565d9b8d..5756e650b0 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1728,6 +1728,7 @@
>>   #
>>   # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
>>   # <- { "return": {} }
>> +#
>>   # -> { "execute": "migrate",
>>   #      "arguments": {
>>   #          "channels": [ { "channel-type": "main",
>> @@ -1796,19 +1797,19 @@
>>   #
>>   #     3. The uri format is the same as for -incoming
>>   #
>> -#     5. For now, number of migration streams is restricted to one,
>> +#     4. For now, number of migration streams is restricted to one,
>>   #        i.e number of items in 'channels' list is just 1.
>>   #
>> -#     4. The 'uri' and 'channels' arguments are mutually exclusive;
>> +#     5. The 'uri' and 'channels' arguments are mutually exclusive;
>>   #        exactly one of the two should be present.
>>   #
>>   # Example:
>>   #
>>   # -> { "execute": "migrate-incoming",
>> -#      "arguments": { "uri": "tcp::4446" } }
>> +#      "arguments": { "uri": "tcp:0:4446" } }
>>   # <- { "return": {} }
>>   #
>> -# -> { "execute": "migrate",
>> +# -> { "execute": "migrate-incoming",
>>   #      "arguments": {
>>   #          "channels": [ { "channel-type": "main",
>>   #                          "addr": { "transport": "socket",
>> @@ -1817,7 +1818,7 @@
>>   #                                    "port": "1050" } } ] } }
>>   # <- { "return": {} }
>>   #
>> -# -> { "execute": "migrate",
>> +# -> { "execute": "migrate-incoming",
>>   #      "arguments": {
>>   #          "channels": [ { "channel-type": "main",
>>   #                          "addr": { "transport": "exec",
>> @@ -1825,7 +1826,7 @@
>>   #                                              "/some/sock" ] } } ] } }
>>   # <- { "return": {} }
>>   #
>> -# -> { "execute": "migrate",
>> +# -> { "execute": "migrate-incoming",
>>   #      "arguments": {
>>   #          "channels": [ { "channel-type": "main",
>>   #                          "addr": { "transport": "rdma",

Regards,

Het Gala
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Markus Armbruster 8 months, 1 week ago
Het Gala <het.gala@nutanix.com> writes:

> Sorry Markus, firstly I thought its just regarding qapi documentation so migration maintainers might not be needed ? but then I realize the commit message fails to specify that
>
> IIRC, you are one of the maintainers for qapi. So, just cc'd to you, but you are right, should have done to migration maintainers also.

Cc'ing me was definitely appropriate.

You can use scripts/get_maintainer.pl to help you figugure out who to
cc.  For this patch, its output is

    Eric Blake <eblake@redhat.com> (supporter:QAPI Schema)
    Markus Armbruster <armbru@redhat.com> (supporter:QAPI Schema)
    Peter Xu <peterx@redhat.com> (maintainer:Migration)
    Fabiano Rosas <farosas@suse.de> (maintainer:Migration)
    qemu-devel@nongnu.org (open list:All patches CC here)

> Have we got the wrong Fabiano here ? Isn't Fabiano Rosas the migration maintainer ?

We do!  Butterfingers...

> cc'ing to Fabiano Rosas too.

Thanks for paying attention :)
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Peter Xu 8 months, 1 week ago
Thanks, Markus.

On Wed, Feb 21, 2024 at 12:36:57PM +0530, Het Gala wrote:
> > > diff --git a/qapi/migration.json b/qapi/migration.json
> > > index 5a565d9b8d..5756e650b0 100644
> > > --- a/qapi/migration.json
> > > +++ b/qapi/migration.json
> > > @@ -1728,6 +1728,7 @@
> > >   #
> > >   # -> { "execute": "migrate", "arguments": { "uri": "tcp:0:4446" } }
> > >   # <- { "return": {} }
> > > +#
> > >   # -> { "execute": "migrate",
> > >   #      "arguments": {
> > >   #          "channels": [ { "channel-type": "main",
> > > @@ -1796,19 +1797,19 @@
> > >   #
> > >   #     3. The uri format is the same as for -incoming
> > >   #
> > > -#     5. For now, number of migration streams is restricted to one,
> > > +#     4. For now, number of migration streams is restricted to one,
> > >   #        i.e number of items in 'channels' list is just 1.
> > >   #
> > > -#     4. The 'uri' and 'channels' arguments are mutually exclusive;
> > > +#     5. The 'uri' and 'channels' arguments are mutually exclusive;
> > >   #        exactly one of the two should be present.
> > >   #
> > >   # Example:
> > >   #
> > >   # -> { "execute": "migrate-incoming",
> > > -#      "arguments": { "uri": "tcp::4446" } }
> > > +#      "arguments": { "uri": "tcp:0:4446" } }
> > >   # <- { "return": {} }
> > >   #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > >   #      "arguments": {
> > >   #          "channels": [ { "channel-type": "main",
> > >   #                          "addr": { "transport": "socket",
> > > @@ -1817,7 +1818,7 @@
> > >   #                                    "port": "1050" } } ] } }
> > >   # <- { "return": {} }
> > >   #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > >   #      "arguments": {
> > >   #          "channels": [ { "channel-type": "main",
> > >   #                          "addr": { "transport": "exec",
> > > @@ -1825,7 +1826,7 @@
> > >   #                                              "/some/sock" ] } } ] } }
> > >   # <- { "return": {} }
> > >   #
> > > -# -> { "execute": "migrate",
> > > +# -> { "execute": "migrate-incoming",
> > >   #      "arguments": {
> > >   #          "channels": [ { "channel-type": "main",
> > >   #                          "addr": { "transport": "rdma",

Reviewed-by: Peter Xu <peterx@redhat.com>

Markus, do you want us to pick it up, or let it go via qapi?

Thanks,

-- 
Peter Xu
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Markus Armbruster 8 months, 1 week ago
Peter Xu <peterx@redhat.com> writes:

> Thanks, Markus.

[...]

> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> Markus, do you want us to pick it up, or let it go via qapi?

I can stick it into my next qapi PR in a few days, if you guys don't
beat me to the punch.
Re: [PATCH] qapi: Misc cleanups to migrate QAPIs
Posted by Peter Xu 8 months, 1 week ago
On Wed, Feb 21, 2024 at 09:30:52AM +0100, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > Thanks, Markus.
> 
> [...]
> 
> > Reviewed-by: Peter Xu <peterx@redhat.com>
> >
> > Markus, do you want us to pick it up, or let it go via qapi?
> 
> I can stick it into my next qapi PR in a few days, if you guys don't
> beat me to the punch.

That works, thanks!

-- 
Peter Xu