[PATCH 2/4] qapi/migration: Deprecate migrate argument @detach

Markus Armbruster posted 4 patches 3 months, 3 weeks ago
There is a newer version of this series
[PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Markus Armbruster via Devel 3 months, 3 weeks ago
Argument @detach has always been ignored.  Start the clock to get rid
of it.

Cc: Peter Xu <peterx@redhat.com>
Cc: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/about/deprecated.rst |  5 +++++
 qapi/migration.json       | 18 +++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 9665bc6fcf..ef4ea84e69 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
 
 Use ``job-finalize`` instead.
 
+``migrate`` argument ``detach`` (since 10.1)
+''''''''''''''''''''''''''''''''''''''''''''
+
+This argument has always been ignored.
+
 ``query-migrationthreads`` (since 9.2)
 ''''''''''''''''''''''''''''''''''''''
 
diff --git a/qapi/migration.json b/qapi/migration.json
index 8b9c53595c..ecd266f98e 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1660,6 +1660,10 @@
 #
 # @resume: resume one paused migration, default "off".  (since 3.0)
 #
+# Features:
+#
+# @deprecated: Argument @detach is deprecated.
+#
 # Since: 0.14
 #
 # .. admonition:: Notes
@@ -1668,19 +1672,14 @@
 #        migration's progress and final result (this information is
 #        provided by the 'status' member).
 #
-#     2. All boolean arguments default to false.
-#
-#     3. The user Monitor's "detach" argument is invalid in QMP and
-#        should not be used.
-#
-#     4. The uri argument should have the Uniform Resource Identifier
+#     2. The uri argument should have the Uniform Resource Identifier
 #        of default destination VM.  This connection will be bound to
 #        default network.
 #
-#     5. For now, number of migration streams is restricted to one,
+#     3. For now, number of migration streams is restricted to one,
 #        i.e. number of items in 'channels' list is just 1.
 #
-#     6. The 'uri' and 'channels' arguments are mutually exclusive;
+#     4. The 'uri' and 'channels' arguments are mutually exclusive;
 #        exactly one of the two should be present.
 #
 # .. qmp-example::
@@ -1724,7 +1723,8 @@
 { 'command': 'migrate',
   'data': {'*uri': 'str',
            '*channels': [ 'MigrationChannel' ],
-           '*detach': 'bool', '*resume': 'bool' } }
+           '*detach': { 'type': 'bool', 'features': [ 'deprecated' ] },
+           '*resume': 'bool' } }
 
 ##
 # @migrate-incoming:
-- 
2.48.1
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Peter Xu via Devel 3 months, 3 weeks ago
On Wed, May 21, 2025 at 08:37:09AM +0200, Markus Armbruster wrote:
> Argument @detach has always been ignored.  Start the clock to get rid
> of it.
> 
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/about/deprecated.rst |  5 +++++
>  qapi/migration.json       | 18 +++++++++---------
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 9665bc6fcf..ef4ea84e69 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
>  
>  Use ``job-finalize`` instead.
>  
> +``migrate`` argument ``detach`` (since 10.1)
> +''''''''''''''''''''''''''''''''''''''''''''
> +
> +This argument has always been ignored.
> +
>  ``query-migrationthreads`` (since 9.2)
>  ''''''''''''''''''''''''''''''''''''''
>  
> diff --git a/qapi/migration.json b/qapi/migration.json
> index 8b9c53595c..ecd266f98e 100644
> --- a/qapi/migration.json
> +++ b/qapi/migration.json
> @@ -1660,6 +1660,10 @@
>  #
>  # @resume: resume one paused migration, default "off".  (since 3.0)
>  #
> +# Features:
> +#
> +# @deprecated: Argument @detach is deprecated.
> +#
>  # Since: 0.14
>  #
>  # .. admonition:: Notes
> @@ -1668,19 +1672,14 @@
>  #        migration's progress and final result (this information is
>  #        provided by the 'status' member).
>  #
> -#     2. All boolean arguments default to false.

There's one more boolean ("resume") exists, but probably not a huge
deal.. All booleans if not mentioned should have a default-false semantics
at least to me.

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

> -#
> -#     3. The user Monitor's "detach" argument is invalid in QMP and
> -#        should not be used.
> -#
> -#     4. The uri argument should have the Uniform Resource Identifier
> +#     2. The uri argument should have the Uniform Resource Identifier
>  #        of default destination VM.  This connection will be bound to
>  #        default network.
>  #
> -#     5. For now, number of migration streams is restricted to one,
> +#     3. For now, number of migration streams is restricted to one,
>  #        i.e. number of items in 'channels' list is just 1.
>  #
> -#     6. The 'uri' and 'channels' arguments are mutually exclusive;
> +#     4. The 'uri' and 'channels' arguments are mutually exclusive;
>  #        exactly one of the two should be present.
>  #
>  # .. qmp-example::
> @@ -1724,7 +1723,8 @@
>  { 'command': 'migrate',
>    'data': {'*uri': 'str',
>             '*channels': [ 'MigrationChannel' ],
> -           '*detach': 'bool', '*resume': 'bool' } }
> +           '*detach': { 'type': 'bool', 'features': [ 'deprecated' ] },
> +           '*resume': 'bool' } }
>  
>  ##
>  # @migrate-incoming:
> -- 
> 2.48.1
> 

-- 
Peter Xu
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Markus Armbruster via Devel 3 months, 3 weeks ago
Peter Xu <peterx@redhat.com> writes:

> On Wed, May 21, 2025 at 08:37:09AM +0200, Markus Armbruster wrote:
>> Argument @detach has always been ignored.  Start the clock to get rid
>> of it.
>> 
>> Cc: Peter Xu <peterx@redhat.com>
>> Cc: Fabiano Rosas <farosas@suse.de>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  docs/about/deprecated.rst |  5 +++++
>>  qapi/migration.json       | 18 +++++++++---------
>>  2 files changed, 14 insertions(+), 9 deletions(-)
>> 
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 9665bc6fcf..ef4ea84e69 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
>>  
>>  Use ``job-finalize`` instead.
>>  
>> +``migrate`` argument ``detach`` (since 10.1)
>> +''''''''''''''''''''''''''''''''''''''''''''
>> +
>> +This argument has always been ignored.
>> +
>>  ``query-migrationthreads`` (since 9.2)
>>  ''''''''''''''''''''''''''''''''''''''
>>  
>> diff --git a/qapi/migration.json b/qapi/migration.json
>> index 8b9c53595c..ecd266f98e 100644
>> --- a/qapi/migration.json
>> +++ b/qapi/migration.json
>> @@ -1660,6 +1660,10 @@
>>  #
>>  # @resume: resume one paused migration, default "off".  (since 3.0)
>>  #
>> +# Features:
>> +#
>> +# @deprecated: Argument @detach is deprecated.
>> +#
>>  # Since: 0.14
>>  #
>>  # .. admonition:: Notes
>> @@ -1668,19 +1672,14 @@
>>  #        migration's progress and final result (this information is
>>  #        provided by the 'status' member).
>>  #
>> -#     2. All boolean arguments default to false.
>
> There's one more boolean ("resume") exists, but probably not a huge
> deal.. All booleans if not mentioned should have a default-false semantics
> at least to me.

Its default remains documented.  It's visible above :)

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

Thanks!
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Peter Xu via Devel 3 months, 3 weeks ago
On Wed, May 21, 2025 at 04:28:33PM +0200, Markus Armbruster wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > On Wed, May 21, 2025 at 08:37:09AM +0200, Markus Armbruster wrote:
> >> Argument @detach has always been ignored.  Start the clock to get rid
> >> of it.
> >> 
> >> Cc: Peter Xu <peterx@redhat.com>
> >> Cc: Fabiano Rosas <farosas@suse.de>
> >> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >> ---
> >>  docs/about/deprecated.rst |  5 +++++
> >>  qapi/migration.json       | 18 +++++++++---------
> >>  2 files changed, 14 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> >> index 9665bc6fcf..ef4ea84e69 100644
> >> --- a/docs/about/deprecated.rst
> >> +++ b/docs/about/deprecated.rst
> >> @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
> >>  
> >>  Use ``job-finalize`` instead.
> >>  
> >> +``migrate`` argument ``detach`` (since 10.1)
> >> +''''''''''''''''''''''''''''''''''''''''''''
> >> +
> >> +This argument has always been ignored.
> >> +
> >>  ``query-migrationthreads`` (since 9.2)
> >>  ''''''''''''''''''''''''''''''''''''''
> >>  
> >> diff --git a/qapi/migration.json b/qapi/migration.json
> >> index 8b9c53595c..ecd266f98e 100644
> >> --- a/qapi/migration.json
> >> +++ b/qapi/migration.json
> >> @@ -1660,6 +1660,10 @@
> >>  #
> >>  # @resume: resume one paused migration, default "off".  (since 3.0)
> >>  #
> >> +# Features:
> >> +#
> >> +# @deprecated: Argument @detach is deprecated.
> >> +#
> >>  # Since: 0.14
> >>  #
> >>  # .. admonition:: Notes
> >> @@ -1668,19 +1672,14 @@
> >>  #        migration's progress and final result (this information is
> >>  #        provided by the 'status' member).
> >>  #
> >> -#     2. All boolean arguments default to false.
> >
> > There's one more boolean ("resume") exists, but probably not a huge
> > deal.. All booleans if not mentioned should have a default-false semantics
> > at least to me.
> 
> Its default remains documented.  It's visible above :)

Ah, indeed!

-- 
Peter Xu
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Fabiano Rosas 3 months, 3 weeks ago
Markus Armbruster <armbru@redhat.com> writes:

> Argument @detach has always been ignored.  Start the clock to get rid
> of it.
>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Peter Krempa via Devel 3 months, 3 weeks ago
On Wed, May 21, 2025 at 08:37:09 +0200, Markus Armbruster via Devel wrote:
> Argument @detach has always been ignored.  Start the clock to get rid
> of it.
> 
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/about/deprecated.rst |  5 +++++
>  qapi/migration.json       | 18 +++++++++---------
>  2 files changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 9665bc6fcf..ef4ea84e69 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
>  
>  Use ``job-finalize`` instead.
>  
> +``migrate`` argument ``detach`` (since 10.1)
> +''''''''''''''''''''''''''''''''''''''''''''
> +
> +This argument has always been ignored.

Huh; libvirt is actually always specifying it for some reason.

I'll post patches shortly getting rid of it completely in libvirt
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Peter Krempa via Devel 3 months, 3 weeks ago
On Wed, May 21, 2025 at 09:46:10 +0200, Peter Krempa via Devel wrote:
> On Wed, May 21, 2025 at 08:37:09 +0200, Markus Armbruster via Devel wrote:
> > Argument @detach has always been ignored.  Start the clock to get rid
> > of it.
> > 
> > Cc: Peter Xu <peterx@redhat.com>
> > Cc: Fabiano Rosas <farosas@suse.de>
> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> > ---
> >  docs/about/deprecated.rst |  5 +++++
> >  qapi/migration.json       | 18 +++++++++---------
> >  2 files changed, 14 insertions(+), 9 deletions(-)
> > 
> > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> > index 9665bc6fcf..ef4ea84e69 100644
> > --- a/docs/about/deprecated.rst
> > +++ b/docs/about/deprecated.rst
> > @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
> >  
> >  Use ``job-finalize`` instead.
> >  
> > +``migrate`` argument ``detach`` (since 10.1)
> > +''''''''''''''''''''''''''''''''''''''''''''
> > +
> > +This argument has always been ignored.
> 
> Huh; libvirt is actually always specifying it for some reason.
> 
> I'll post patches shortly getting rid of it completely in libvirt

Patch 2/2 of

https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VPMPVPG5DSFORQODG4JGMZ2MTOJDQPPF/

addresses the above. I noticed that 'dump-guest-memory' does also have
'detach' option but that one at least has effect, even when it's not
really useful (blocking monitor if not used).

On behalf of libvirt:

Series:

ACKed-by: Peter Krempa <pkrempa@redhat.com>
Re: [PATCH 2/4] qapi/migration: Deprecate migrate argument @detach
Posted by Markus Armbruster 3 months, 3 weeks ago
Peter Krempa <pkrempa@redhat.com> writes:

> On Wed, May 21, 2025 at 09:46:10 +0200, Peter Krempa via Devel wrote:
>> On Wed, May 21, 2025 at 08:37:09 +0200, Markus Armbruster via Devel wrote:
>> > Argument @detach has always been ignored.  Start the clock to get rid
>> > of it.
>> > 
>> > Cc: Peter Xu <peterx@redhat.com>
>> > Cc: Fabiano Rosas <farosas@suse.de>
>> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> > ---
>> >  docs/about/deprecated.rst |  5 +++++
>> >  qapi/migration.json       | 18 +++++++++---------
>> >  2 files changed, 14 insertions(+), 9 deletions(-)
>> > 
>> > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> > index 9665bc6fcf..ef4ea84e69 100644
>> > --- a/docs/about/deprecated.rst
>> > +++ b/docs/about/deprecated.rst
>> > @@ -179,6 +179,11 @@ Use ``job-dismiss`` instead.
>> >  
>> >  Use ``job-finalize`` instead.
>> >  
>> > +``migrate`` argument ``detach`` (since 10.1)
>> > +''''''''''''''''''''''''''''''''''''''''''''
>> > +
>> > +This argument has always been ignored.
>> 
>> Huh; libvirt is actually always specifying it for some reason.
>> 
>> I'll post patches shortly getting rid of it completely in libvirt
>
> Patch 2/2 of
>
> https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/message/VPMPVPG5DSFORQODG4JGMZ2MTOJDQPPF/
>
> addresses the above. I noticed that 'dump-guest-memory' does also have
> 'detach' option but that one at least has effect, even when it's not
> really useful (blocking monitor if not used).

Another long-running task that should be a job...

> On behalf of libvirt:
>
> Series:
>
> ACKed-by: Peter Krempa <pkrempa@redhat.com>

Thanks!