[RFC 4/6] migration: Deprecate -incoming
Only "defer" is recommended. After setting all migation parameters,
start incoming migration with "migrate-incoming uri" command.
Signed-off-by: Juan Quintela
---
docs/about/deprecated.rst | 7 +++
softmmu/vl.c | 2 ++
2 files changed, 9 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 47e98dc95e..518672722d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -447,3 +447,10 @@ The new way to modify migration is using migration
parameters.
``blk`` functionality can be acchieved using
``migrate_set_parameter block-incremental true``.
+``-incoming uri`` (since 8.1)
+'
+
+Everything except ``-incoming defer`` are deprecated. This allows to
+setup parameters before launching the proper migration with
+``migrate-incoming uri``.
+
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..7fe865ab59 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
+warn_report("-incoming %s is deprecated, use -incoming defer and "
+" set the uri with migrate-incoming.", incoming);
qmp_migrate_incoming(incoming, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
--
2.40.1
Re: [RFC 4/6] migration: Deprecate -incoming
On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
> Only "defer" is recommended. After setting all migation parameters,
> start incoming migration with "migrate-incoming uri" command.
>
> Signed-off-by: Juan Quintela
> ---
> docs/about/deprecated.rst | 7 +++
> softmmu/vl.c | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 47e98dc95e..518672722d 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
> parameters.
> ``blk`` functionality can be acchieved using
> ``migrate_set_parameter block-incremental true``.
>
> +``-incoming uri`` (since 8.1)
> +'
> +
> +Everything except ``-incoming defer`` are deprecated. This allows to
> +setup parameters before launching the proper migration with
> +``migrate-incoming uri``.
> +
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index b0b96f67fa..7fe865ab59 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
> if (incoming) {
> Error *local_err = NULL;
> if (strcmp(incoming, "defer") != 0) {
> +warn_report("-incoming %s is deprecated, use -incoming defer and
> "
> +" set the uri with migrate-incoming.", incoming);
I still use uri for all my scripts, alongside with "-global migration.xxx"
and it works.
Shall we just leave it there? Or is deprecating it helps us in any form?
--
Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
Peter Xu wrote:
> On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
>> Only "defer" is recommended. After setting all migation parameters,
>> start incoming migration with "migrate-incoming uri" command.
>>
>> Signed-off-by: Juan Quintela
>> ---
>> docs/about/deprecated.rst | 7 +++
>> softmmu/vl.c | 2 ++
>> 2 files changed, 9 insertions(+)
>>
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 47e98dc95e..518672722d 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
>> parameters.
>> ``blk`` functionality can be acchieved using
>> ``migrate_set_parameter block-incremental true``.
>>
>> +``-incoming uri`` (since 8.1)
>> +'
>> +
>> +Everything except ``-incoming defer`` are deprecated. This allows to
>> +setup parameters before launching the proper migration with
>> +``migrate-incoming uri``.
>> +
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index b0b96f67fa..7fe865ab59 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
>> if (incoming) {
>> Error *local_err = NULL;
>> if (strcmp(incoming, "defer") != 0) {
>> +warn_report("-incoming %s is deprecated, use -incoming defer
>> and "
>> +" set the uri with migrate-incoming.", incoming);
>
> I still use uri for all my scripts, alongside with "-global migration.xxx"
> and it works.
You know what you are doing (TM).
And remember that we don't support -gobal migration.x-foo.
Yes, I know, we should drop the "x-" prefixes.
> Shall we just leave it there? Or is deprecating it helps us in any form?
See the patches two weeks ago when people complained that lisen(.., num)
was too low. And there are other parameters that work the same way
(that I convenientely had forgotten). So the easiest way to get things
right is to use "defer" always. Using -incoming "uri" should only be
for people that "know what they are doing", so we had to ways to do it:
- review all migration options and see which ones work without defer
and document it
- deprecate everything that is not defer.
Anything else is not going to be very user unfriendly.
What do you think.
Later, Juan.
PD. This series are RFC for multiple reasons O:-)
Re: [RFC 4/6] migration: Deprecate -incoming
On Mon, Jun 12, 2023 at 10:51:08PM +0200, Juan Quintela wrote:
> Peter Xu wrote:
> > On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
> >> Only "defer" is recommended. After setting all migation parameters,
> >> start incoming migration with "migrate-incoming uri" command.
> >>
> >> Signed-off-by: Juan Quintela
> >> ---
> >> docs/about/deprecated.rst | 7 +++
> >> softmmu/vl.c | 2 ++
> >> 2 files changed, 9 insertions(+)
> >>
> >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> >> index 47e98dc95e..518672722d 100644
> >> --- a/docs/about/deprecated.rst
> >> +++ b/docs/about/deprecated.rst
> >> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
> >> parameters.
> >> ``blk`` functionality can be acchieved using
> >> ``migrate_set_parameter block-incremental true``.
> >>
> >> +``-incoming uri`` (since 8.1)
> >> +'
> >> +
> >> +Everything except ``-incoming defer`` are deprecated. This allows to
> >> +setup parameters before launching the proper migration with
> >> +``migrate-incoming uri``.
> >> +
> >> diff --git a/softmmu/vl.c b/softmmu/vl.c
> >> index b0b96f67fa..7fe865ab59 100644
> >> --- a/softmmu/vl.c
> >> +++ b/softmmu/vl.c
> >> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
> >> if (incoming) {
> >> Error *local_err = NULL;
> >> if (strcmp(incoming, "defer") != 0) {
> >> +warn_report("-incoming %s is deprecated, use -incoming defer
> >> and "
> >> +" set the uri with migrate-incoming.", incoming);
> >
> > I still use uri for all my scripts, alongside with "-global migration.xxx"
> > and it works.
>
> You know what you are doing (TM).
> And remember that we don't support -gobal migration.x-foo.
> Yes, I know, we should drop the "x-" prefixes.
I hope they'll always be there. :) They're pretty handy for tests, when we
want to boot a VM without the need to script the sequences of qmp cmds.
Yes, we probably should just always drop the x-. We can always declare
debugging purpose for all -global migration.* fields.
>
> > Shall we just leave it there? Or is deprecating it helps us in any form?
>
> See the patches two weeks ago when people complained that lisen(.., num)
> was too low. And there are other parameters that work the same way
> (that I convenientely had forgotten). So the easiest way to get things
> right is to use "defer" always. Using -incoming "uri" should only be
> for people that "know what they are doing", so we had to ways to do it:
> - review all migration options and see which ones work without defer
> and document it
> - deprecate everything that is not defer.
>
> Anything else is not going to be very user unfriendly.
> What do you think.
IIRC Wei Wang had a series just for that, so after that patchset applied we
should have fixed all issues cleanly? Is there one more thing that's not
working right there?
>
> Later, Juan.
>
> PD. This series are RFC for multiple reasons O:-)
Happy to know the rest (besides which I know will break my script :).
--
Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Mon, Jun 12, 2023 at 10:51:08PM +0200, Juan Quintela wrote:
> Peter Xu wrote:
> > On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
> >> Only "defer" is recommended. After setting all migation parameters,
> >> start incoming migration with "migrate-incoming uri" command.
> >>
> >> Signed-off-by: Juan Quintela
> >> ---
> >> docs/about/deprecated.rst | 7 +++
> >> softmmu/vl.c | 2 ++
> >> 2 files changed, 9 insertions(+)
> >>
> >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> >> index 47e98dc95e..518672722d 100644
> >> --- a/docs/about/deprecated.rst
> >> +++ b/docs/about/deprecated.rst
> >> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
> >> parameters.
> >> ``blk`` functionality can be acchieved using
> >> ``migrate_set_parameter block-incremental true``.
> >>
> >> +``-incoming uri`` (since 8.1)
> >> +'
> >> +
> >> +Everything except ``-incoming defer`` are deprecated. This allows to
> >> +setup parameters before launching the proper migration with
> >> +``migrate-incoming uri``.
> >> +
> >> diff --git a/softmmu/vl.c b/softmmu/vl.c
> >> index b0b96f67fa..7fe865ab59 100644
> >> --- a/softmmu/vl.c
> >> +++ b/softmmu/vl.c
> >> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
> >> if (incoming) {
> >> Error *local_err = NULL;
> >> if (strcmp(incoming, "defer") != 0) {
> >> +warn_report("-incoming %s is deprecated, use -incoming defer
> >> and "
> >> +" set the uri with migrate-incoming.", incoming);
> >
> > I still use uri for all my scripts, alongside with "-global migration.xxx"
> > and it works.
>
> You know what you are doing (TM).
> And remember that we don't support -gobal migration.x-foo.
> Yes, I know, we should drop the "x-" prefixes.
>
> > Shall we just leave it there? Or is deprecating it helps us in any form?
>
> See the patches two weeks ago when people complained that lisen(.., num)
> was too low. And there are other parameters that work the same way
> (that I convenientely had forgotten). So the easiest way to get things
> right is to use "defer" always. Using -incoming "uri" should only be
> for people that "know what they are doing", so we had to ways to do it:
> - review all migration options and see which ones work without defer
> and document it
> - deprecate everything that is not defer.
>
> Anything else is not going to be very user unfriendly.
> What do you think.
In some cases it is worth having a convenience option for user friendliness.
In this case, however, users are already needing to use QMP/HMP on the
source side to set migration parameters. I think it is reasonable to say
that doing *exactly* the same with QMP/HMP on the destination side is
actually more friendly than requiring use of -global on the dest side
which is different syntax.
We don't document the way to use -global with migration parameters so
when people see '-incoming' I think we are steering them into a trap,
making it look like -incoming is sufficient on its own. Hence that user's
mistake recently about not setting migration parameters.
Overall I agree with deprecating '-incoming' != 'defer', as I think it
will better guide users to following the correct steps, as is not a
usability burden as they're already using QMP/HMP on the source side.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
On Mon, Jun 12, 2023 at 05:19:40PM -0400, Peter Xu wrote:
> On Mon, Jun 12, 2023 at 10:51:08PM +0200, Juan Quintela wrote:
> > Peter Xu wrote:
> > > On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
> > >> Only "defer" is recommended. After setting all migation parameters,
> > >> start incoming migration with "migrate-incoming uri" command.
> > >>
> > >> Signed-off-by: Juan Quintela
> > >> ---
> > >> docs/about/deprecated.rst | 7 +++
> > >> softmmu/vl.c | 2 ++
> > >> 2 files changed, 9 insertions(+)
> > >>
> > >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> > >> index 47e98dc95e..518672722d 100644
> > >> --- a/docs/about/deprecated.rst
> > >> +++ b/docs/about/deprecated.rst
> > >> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
> > >> parameters.
> > >> ``blk`` functionality can be acchieved using
> > >> ``migrate_set_parameter block-incremental true``.
> > >>
> > >> +``-incoming uri`` (since 8.1)
> > >> +'
> > >> +
> > >> +Everything except ``-incoming defer`` are deprecated. This allows to
> > >> +setup parameters before launching the proper migration with
> > >> +``migrate-incoming uri``.
> > >> +
> > >> diff --git a/softmmu/vl.c b/softmmu/vl.c
> > >> index b0b96f67fa..7fe865ab59 100644
> > >> --- a/softmmu/vl.c
> > >> +++ b/softmmu/vl.c
> > >> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
> > >> if (incoming) {
> > >> Error *local_err = NULL;
> > >> if (strcmp(incoming, "defer") != 0) {
> > >> +warn_report("-incoming %s is deprecated, use -incoming
> > >> defer and "
> > >> +" set the uri with migrate-incoming.",
> > >> incoming);
> > >
> > > I still use uri for all my scripts, alongside with "-global migration.xxx"
> > > and it works.
> >
> > You know what you are doing (TM).
> > And remember that we don't support -gobal migration.x-foo.
> > Yes, I know, we should drop the "x-" prefixes.
>
> I hope they'll always be there. :) They're pretty handy for tests, when we
> want to boot a VM without the need to script the sequences of qmp cmds.
The long term vision for QEMU configuration is that everything will
be done via QMP. This doesn't mean we'll need interactive scripts
though. We ought to be able to just point QEMU as a config file
containing the QMP bits to construct the VM.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
On Tue, Jun 20, 2023 at 01:10:55PM +0100, Daniel P. Berrangé wrote: > In some cases it is worth having a convenience option for user friendliness. > > In this case, however, users are already needing to use QMP/HMP on the > source side to set migration parameters. I think it is reasonable to say > that doing *exactly* the same with QMP/HMP on the destination side is > actually more friendly than requiring use of -global on the dest side > which is different syntax. The -global parameters also work for the src side for my scripts, so I only need to attach "-incoming tcp:XXX" for dst cmdline here. > > We don't document the way to use -global with migration parameters so > when people see '-incoming' I think we are steering them into a trap, > making it look like -incoming is sufficient on its own. Hence that user's > mistake recently about not setting migration parameters. > > Overall I agree with deprecating '-incoming' != 'defer', as I think it > will better guide users to following the correct steps, as is not a > usability burden as they're already using QMP/HMP on the source side. I'd say -global is definitely not for users but for developers. Just like we keep around HMP - I'm not sure whether it's used in productions, I thought it was only for developers and we don't deprecate it eagerly. No strong opinions here if everyone wants to get rid of that, but before that I hope we can have some kind of cmdline tool that can easily tunnel qmp commands so whoever developers using pure cmdlines can switch to the new way easily. Thanks, -- Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On 12/06/2023 21.33, Juan Quintela wrote:
Only "defer" is recommended. After setting all migation parameters,
start incoming migration with "migrate-incoming uri" command.
Signed-off-by: Juan Quintela
---
docs/about/deprecated.rst | 7 +++
softmmu/vl.c | 2 ++
2 files changed, 9 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 47e98dc95e..518672722d 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -447,3 +447,10 @@ The new way to modify migration is using migration
parameters.
``blk`` functionality can be acchieved using
``migrate_set_parameter block-incremental true``.
+``-incoming uri`` (since 8.1)
+'
+
+Everything except ``-incoming defer`` are deprecated. This allows to
+setup parameters before launching the proper migration with
+``migrate-incoming uri``.
+
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..7fe865ab59 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
+warn_report("-incoming %s is deprecated, use -incoming defer and "
+" set the uri with migrate-incoming.", incoming);
qmp_migrate_incoming(incoming, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
Could we maybe keep at least the smallest set of necessary parameters
around? I'm often doing a "-incoming tcp:0:1234" for doing quick sanity
checks with migration, not caring about other migration parameters, so if
that could continue to work, that would be very appreciated.
Thomas
Re: [RFC 4/6] migration: Deprecate -incoming
Thomas Huth wrote:
> On 12/06/2023 21.33, Juan Quintela wrote:
>> Only "defer" is recommended. After setting all migation parameters,
>> start incoming migration with "migrate-incoming uri" command.
>> Signed-off-by: Juan Quintela
>> ---
>> docs/about/deprecated.rst | 7 +++
>> softmmu/vl.c | 2 ++
>> 2 files changed, 9 insertions(+)
>> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> index 47e98dc95e..518672722d 100644
>> --- a/docs/about/deprecated.rst
>> +++ b/docs/about/deprecated.rst
>> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
>> parameters.
>> ``blk`` functionality can be acchieved using
>> ``migrate_set_parameter block-incremental true``.
>> +``-incoming uri`` (since 8.1)
>> +'
>> +
>> +Everything except ``-incoming defer`` are deprecated. This allows to
>> +setup parameters before launching the proper migration with
>> +``migrate-incoming uri``.
>> +
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index b0b96f67fa..7fe865ab59 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
>> if (incoming) {
>> Error *local_err = NULL;
>> if (strcmp(incoming, "defer") != 0) {
>> +warn_report("-incoming %s is deprecated, use -incoming defer
>> and "
>> +" set the uri with migrate-incoming.", incoming);
>> qmp_migrate_incoming(incoming, &local_err);
>> if (local_err) {
>> error_reportf_err(local_err, "-incoming %s: ", incoming);
>
> Could we maybe keep at least the smallest set of necessary parameters
> around? I'm often doing a "-incoming tcp:0:1234" for doing quick
> sanity checks with migration, not caring about other migration
> parameters, so if that could continue to work, that would be very
> appreciated.
I will try to explain myself here.
I think that everything except tcp works.
But when we have tcp, we have two cases where this is a trap:
- multifd channels:
* if we default to a big number, we underuse resources in a normal
case
* if we default to a small number, we have the problem that if the
user set "later" multifd-channels to a bigger number, things can
break.
- postcopy+preempt:
this case is also problematic, but easily fixable. Put a default
of 2 instead of 1.
The only other solution that I can think of is just fail if we set
multifd without incoming defer. But more sooner than later we are going
to have to default to multifd, so ...
Later, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/12/23 22:51, Juan Quintela wrote:
Shall we just leave it there? Or is deprecating it helps us in any form?
See the patches two weeks ago when people complained that lisen(.., num)
was too low. And there are other parameters that work the same way
(that I convenientely had forgotten). So the easiest way to get things
right is to use "defer" always. Using -incoming "uri" should only be
for people that "know what they are doing", so we had to ways to do it:
- review all migration options and see which ones work without defer
and document it
- deprecate everything that is not defer.
"-incoming " is literally the same as running "migrate-incoming"
as the first thing on the monitor:
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
qmp_migrate_incoming(incoming, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
exit(1);
}
}
} else if (autostart) {
qmp_cont(NULL);
}
It's the only piece of code which distinguishes "-incoming defer" from
"-incoming ".
So I'm not sure what the problem would be with keeping it? The issue is
not how many features the command line has, but how they're implemented.
If they're just QMP wrappers and as such they're self-contained in
softmmu/vl.c, that's fine.
In fact, even for parameters, we could use keyval to parse "-incoming" and
set the parameters in the same place as above. That would remove the need
for "-global migration".
Paolo
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/21/23 09:08, Thomas Huth wrote:
if (strcmp(incoming, "defer") != 0) {
+ warn_report("-incoming %s is deprecated, use -incoming
defer and "
+ " set the uri with migrate-incoming.",
incoming);
qmp_migrate_incoming(incoming, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ",
incoming);
Could we maybe keep at least the smallest set of necessary parameters
around? I'm often doing a "-incoming tcp:0:1234" for doing quick sanity
checks with migration, not caring about other migration parameters, so
if that could continue to work, that would be very appreciated.
Yeah, this is throwing the baby out with the bathwater.
Paolo
Re: [RFC 4/6] migration: Deprecate -incoming
Paolo Bonzini wrote:
> On 6/12/23 22:51, Juan Quintela wrote:
>>> Shall we just leave it there? Or is deprecating it helps us in any form?
>> See the patches two weeks ago when people complained that lisen(.., num)
>> was too low. And there are other parameters that work the same way
>> (that I convenientely had forgotten). So the easiest way to get things
>> right is to use "defer" always. Using -incoming "uri" should only be
>> for people that "know what they are doing", so we had to ways to do it:
>> - review all migration options and see which ones work without defer
>>and document it
>> - deprecate everything that is not defer.
>
> "-incoming " is literally the same as running "migrate-incoming"
> as the first thing on the monitor:
>
> if (incoming) {
> Error *local_err = NULL;
> if (strcmp(incoming, "defer") != 0) {
> qmp_migrate_incoming(incoming, &local_err);
> if (local_err) {
> error_reportf_err(local_err, "-incoming %s: ", incoming);
> exit(1);
> }
> }
> } else if (autostart) {
> qmp_cont(NULL);
> }
>
> It's the only piece of code which distinguishes "-incoming defer" from
> "-incoming ".
>
> So I'm not sure what the problem would be with keeping it?
User friendliness.
First of all, I use it all the time. And I know that it is useful for
developers. I was the one asking peter to implement -global
migration.foo to be able to test multifd with it.
The problem is that if you use more than two channels with multifd, on
the incoming side, you need to do:
- migrate_set_parameter multifd-channels 16
- migrate_incoming
And people continue to do:
- qemu -incoming
- migrate_set_parameter multifd-channels 16 (on the command line)
And they complain that it fails, because we are calling listen with the
wrong value.
> The issue is
> not how many features the command line has, but how they're implemented.
Or if they are confusing for the user?
> If they're just QMP wrappers and as such they're self-contained in
> softmmu/vl.c, that's fine.
>
> In fact, even for parameters, we could use keyval to parse "-incoming"
What is keyval?
> and
> set the parameters in the same place as above. That would remove the need
> for "-global migration".
Could you elaborate?
The other option that I can think of is changing the error messages for
migrate_check_parameters() and give instructions that you can't set
multifd channels once that you have started incoming migration.
Explaining there to use migrate_incoming command?
Later, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
On 22/06/2023 10.52, Juan Quintela wrote:
Paolo Bonzini wrote:
On 6/12/23 22:51, Juan Quintela wrote:
Shall we just leave it there? Or is deprecating it helps us in any form?
See the patches two weeks ago when people complained that lisen(.., num)
was too low. And there are other parameters that work the same way
(that I convenientely had forgotten). So the easiest way to get things
right is to use "defer" always. Using -incoming "uri" should only be
for people that "know what they are doing", so we had to ways to do it:
- review all migration options and see which ones work without defer
and document it
- deprecate everything that is not defer.
"-incoming " is literally the same as running "migrate-incoming"
as the first thing on the monitor:
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
qmp_migrate_incoming(incoming, &local_err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
exit(1);
}
}
} else if (autostart) {
qmp_cont(NULL);
}
It's the only piece of code which distinguishes "-incoming defer" from
"-incoming ".
So I'm not sure what the problem would be with keeping it?
User friendliness.
First of all, I use it all the time. And I know that it is useful for
developers. I was the one asking peter to implement -global
migration.foo to be able to test multifd with it.
The problem is that if you use more than two channels with multifd, on
the incoming side, you need to do:
- migrate_set_parameter multifd-channels 16
- migrate_incoming
And people continue to do:
- qemu -incoming
- migrate_set_parameter multifd-channels 16 (on the command line)
And they complain that it fails, because we are calling listen with the
wrong value.
Then simply forbid "migrate_set_parameter multifd-channels ..." if the uri
has been specified on the command line?
Thomas
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/22/23 10:52, Juan Quintela wrote:
User friendliness.
The problem is that if you use more than two channels with multifd, on
the incoming side, you need to do:
You're sacrificing user-friendliness for the 99.99% that don't use
multifd, for an error (i.e. it's not even fixing the issue) for the
0.01% that use multifd. That's not user-friendly.
- migrate_set_parameter multifd-channels 16
- migrate_incoming
The issue is not how many features the command line has, but how
they're implemented.
Or if they are confusing for the user?
Anyone using multifd is not a typical user anyway.
If they're just QMP wrappers and as such they're self-contained in
softmmu/vl.c, that's fine.
In fact, even for parameters, we could use keyval to parse "-incoming"
What is keyval?
util/keyval.c and include/qemu/keyval.h. It parses a list of key=value
pairs into a QDict. Once you have removed the "source" key from the
QDict you can use a visitor to parse the rest into a
MigrateSetParameters. See the handling of QEMU_OPTION_audio, it could
be something like
case QEMU_OPTION_incoing: {
Visitor *v;
MigrateSetParameters *incoming_params = NULL;
QDict *dict = keyval_parse(optarg, "source", NULL,
&error_fatal);
if (incoming) {
if (qdict_haskey(dict, "source")) {
error_setg(&error_fatal, "Parameter 'source' is
duplicate");
}
} else {
if (!qdict_haskey(dict, "source")) {
error_setg(&error_fatal, "Parameter 'source' is
missing");
}
runstate_set(RUN_STATE_INMIGRATE);
incoming = g_strdup(qdict_get_str(dict, "source"));
qdict_del(dict, "source");
}
v = qobject_input_visitor_new_keyval(QOBJECT(dict));
qobject_unref(dict);
visit_type_MigrateSetParameters(v, NULL,
&incoming_params, &error_fatal);
visit_free(v);
qmp_migration_set_parameters(incoming_params,
&error_fatal);
qapi_free_MigrateSetParameters(incoming_params);
}
For example "-incoming [source=]tcp:foo,multifd-channels=16" would
desugar to
migrate_set_parameter multifd-channels 16
migrate_incoming tcp:foo
The only incompatibility is for people who are using "," in an URI,
which is rare and only an issue for the "exec" protocol.
Paolo
and
set the parameters in the same place as above. That would remove the need
for "-global migration".
Could you elaborate?
The other option that I can think of is changing the error messages for
migrate_check_parameters() and give instructions that you can't set
multifd channels once that you have started incoming migration.
Explaining there to use migrate_incoming command?
Later, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 10:52:12AM +0200, Juan Quintela wrote:
> Paolo Bonzini wrote:
> > On 6/12/23 22:51, Juan Quintela wrote:
> >>> Shall we just leave it there? Or is deprecating it helps us in any form?
> >> See the patches two weeks ago when people complained that lisen(.., num)
> >> was too low. And there are other parameters that work the same way
> >> (that I convenientely had forgotten). So the easiest way to get things
> >> right is to use "defer" always. Using -incoming "uri" should only be
> >> for people that "know what they are doing", so we had to ways to do it:
> >> - review all migration options and see which ones work without defer
> >>and document it
> >> - deprecate everything that is not defer.
> >
> > "-incoming " is literally the same as running "migrate-incoming"
> > as the first thing on the monitor:
> >
> > if (incoming) {
> > Error *local_err = NULL;
> > if (strcmp(incoming, "defer") != 0) {
> > qmp_migrate_incoming(incoming, &local_err);
> > if (local_err) {
> > error_reportf_err(local_err, "-incoming %s: ", incoming);
> > exit(1);
> > }
> > }
> > } else if (autostart) {
> > qmp_cont(NULL);
> > }
> >
> > It's the only piece of code which distinguishes "-incoming defer" from
> > "-incoming ".
> >
> > So I'm not sure what the problem would be with keeping it?
>
> User friendliness.
>
> First of all, I use it all the time. And I know that it is useful for
> developers. I was the one asking peter to implement -global
> migration.foo to be able to test multifd with it.
>
> The problem is that if you use more than two channels with multifd, on
> the incoming side, you need to do:
>
> - migrate_set_parameter multifd-channels 16
> - migrate_incoming
>
> And people continue to do:
>
> - qemu -incoming
> - migrate_set_parameter multifd-channels 16 (on the command line)
>
> And they complain that it fails, because we are calling listen with the
> wrong value.
IMHO if we want to improve user friendliness then arguing about use
of the CLI vs QMP for migration is completely missing the bigger
picture IMHO.
I've mentioned several times before that the user should never need to
set this multifd-channels parameter (nor many other parameters) on the
destination in the first place.
The QEMU migration stream should be changed to add a full
bi-directional handshake, with negotiation of most parameters.
IOW, the src QEMU should be configured with 16 channels, and
it should connect the primary control channel, and then directly
tell the dest that it wants to use 16 multifd channels.
If we're expecting the user to pass this info across to the dest
manually we've already spectacularly failed wrt user friendliness.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
Paolo Bonzini wrote:
> On 6/22/23 10:52, Juan Quintela wrote:
>> User friendliness.
>> The problem is that if you use more than two channels with multifd, on
>> the incoming side, you need to do:
>
> You're sacrificing user-friendliness for the 99.99% that don't use
> multifd, for an error (i.e. it's not even fixing the issue) for the
> 0.01% that use multifd. That's not user-friendly.
You are forgeting of the 0.01% that uses postocopy preempt (that is easy
just changing the default value to 2), and the 0.001% that uses
compression (they have exactly the same problem with compress_threads,
compress_zlib, etc).
>> - migrate_set_parameter multifd-channels 16
>> - migrate_incoming
>>
>>> The issue is not how many features the command line has, but how
>>> they're implemented.
>> Or if they are confusing for the user?
>
> Anyone using multifd is not a typical user anyway.
>>> If they're just QMP wrappers and as such they're self-contained in
>>> softmmu/vl.c, that's fine.
>>>
>>> In fact, even for parameters, we could use keyval to parse "-incoming"
>> What is keyval?
>
> util/keyval.c and include/qemu/keyval.h. It parses a list of
> key=value pairs into a QDict. Once you have removed the "source" key
> from the QDict you can use a visitor to parse the rest into a
> MigrateSetParameters. See the handling of QEMU_OPTION_audio, it could
> be something like
>
>
> case QEMU_OPTION_incoing: {
> Visitor *v;
> MigrateSetParameters *incoming_params = NULL;
> QDict *dict = keyval_parse(optarg, "source", NULL,
> &error_fatal);
>
> if (incoming) {
> if (qdict_haskey(dict, "source")) {
> error_setg(&error_fatal, "Parameter 'source'
> is duplicate");
> }
> } else {
> if (!qdict_haskey(dict, "source")) {
> error_setg(&error_fatal, "Parameter 'source'
> is missing");
> }
> runstate_set(RUN_STATE_INMIGRATE);
> incoming = g_strdup(qdict_get_str(dict, "source"));
> qdict_del(dict, "source");
> }
>
> v = qobject_input_visitor_new_keyval(QOBJECT(dict));
> qobject_unref(dict);
> visit_type_MigrateSetParameters(v, NULL,
> &incoming_params, &error_fatal);
> visit_free(v);
> qmp_migration_set_parameters(incoming_params,
> &error_fatal);
> qapi_free_MigrateSetParameters(incoming_params);
> }
>
>
> For example "-incoming [source=]tcp:foo,multifd-channels=16" would
> desugar to
>
> migrate_set_parameter multifd-channels 16
> migrate_incoming tcp:foo
>
> The only incompatibility is for people who are using "," in an URI,
> which is rare and only an issue for the "exec" protocol.
Aha, that makes sense. And will allow us to deprecate/remove the
--global migration.* stuff.
Thanks very much.
See why this was an RFC O:-)
Later, Juan.
>
> Paolo
>
>>> and
>>> set the parameters in the same place as above. That would remove the need
>>> for "-global migration".
>> Could you elaborate?
>
>
>
>> The other option that I can think of is changing the error messages for
>> migrate_check_parameters() and give instructions that you can't set
>> multifd channels once that you have started incoming migration.
>> Explaining there to use migrate_incoming command?
>> Later, Juan.
>>
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 12:01:55PM +0200, Juan Quintela wrote:
> Paolo Bonzini wrote:
> > On 6/22/23 10:52, Juan Quintela wrote:
> >> User friendliness.
> >> The problem is that if you use more than two channels with multifd, on
> >> the incoming side, you need to do:
> >
> > You're sacrificing user-friendliness for the 99.99% that don't use
> > multifd, for an error (i.e. it's not even fixing the issue) for the
> > 0.01% that use multifd. That's not user-friendly.
>
> You are forgeting of the 0.01% that uses postocopy preempt (that is easy
> just changing the default value to 2), and the 0.001% that uses
> compression (they have exactly the same problem with compress_threads,
> compress_zlib, etc).
>
> >> - migrate_set_parameter multifd-channels 16
> >> - migrate_incoming
> >>
> >>> The issue is not how many features the command line has, but how
> >>> they're implemented.
> >> Or if they are confusing for the user?
> >
> > Anyone using multifd is not a typical user anyway.
>
> >>> If they're just QMP wrappers and as such they're self-contained in
> >>> softmmu/vl.c, that's fine.
> >>>
> >>> In fact, even for parameters, we could use keyval to parse "-incoming"
> >> What is keyval?
> >
> > util/keyval.c and include/qemu/keyval.h. It parses a list of
> > key=value pairs into a QDict. Once you have removed the "source" key
> > from the QDict you can use a visitor to parse the rest into a
> > MigrateSetParameters. See the handling of QEMU_OPTION_audio, it could
> > be something like
> >
> >
> > case QEMU_OPTION_incoing: {
> > Visitor *v;
> > MigrateSetParameters *incoming_params = NULL;
> > QDict *dict = keyval_parse(optarg, "source", NULL,
> > &error_fatal);
> >
> > if (incoming) {
> > if (qdict_haskey(dict, "source")) {
> > error_setg(&error_fatal, "Parameter 'source'
> > is duplicate");
> > }
> > } else {
> > if (!qdict_haskey(dict, "source")) {
> > error_setg(&error_fatal, "Parameter 'source'
> > is missing");
> > }
> > runstate_set(RUN_STATE_INMIGRATE);
> > incoming = g_strdup(qdict_get_str(dict, "source"));
> > qdict_del(dict, "source");
> > }
> >
> > v = qobject_input_visitor_new_keyval(QOBJECT(dict));
> > qobject_unref(dict);
> > visit_type_MigrateSetParameters(v, NULL,
> > &incoming_params, &error_fatal);
> > visit_free(v);
> > qmp_migration_set_parameters(incoming_params,
PS: we may want to postpone this to be later than migration_object_init(),
when/if there's a real patch.
> > &error_fatal);
> > qapi_free_MigrateSetParameters(incoming_params);
> > }
> >
> >
> > For example "-incoming [source=]tcp:foo,multifd-channels=16" would
> > desugar to
> >
> > migrate_set_parameter multifd-channels 16
> > migrate_incoming tcp:foo
> >
> > The only incompatibility is for people who are using "," in an URI,
> > which is rare and only an issue for the "exec" protocol.
If we worry on breaking anyone, we can apply the keyval parsing only when
!exec. Not sure whether it matters a huge lot..
>
> Aha, that makes sense. And will allow us to deprecate/remove the
> --global migration.* stuff.
We may still need a way to set the caps/params for src qemu?..
Thanks,
--
Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 11:22:56AM +0200, Thomas Huth wrote: > Then simply forbid "migrate_set_parameter multifd-channels ..." if the uri > has been specified on the command line? Yeah, actually already in a pull (even though the pr may need a new one..): https://lore.kernel.org/r/20230622021320.66124-23-quint...@redhat.com -- Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 22, 2023 at 10:52:12AM +0200, Juan Quintela wrote:
> > Paolo Bonzini wrote:
> > > On 6/12/23 22:51, Juan Quintela wrote:
> > >>> Shall we just leave it there? Or is deprecating it helps us in any
> > >>> form?
> > >> See the patches two weeks ago when people complained that lisen(.., num)
> > >> was too low. And there are other parameters that work the same way
> > >> (that I convenientely had forgotten). So the easiest way to get things
> > >> right is to use "defer" always. Using -incoming "uri" should only be
> > >> for people that "know what they are doing", so we had to ways to do it:
> > >> - review all migration options and see which ones work without defer
> > >>and document it
> > >> - deprecate everything that is not defer.
> > >
> > > "-incoming " is literally the same as running "migrate-incoming"
> > > as the first thing on the monitor:
> > >
> > > if (incoming) {
> > > Error *local_err = NULL;
> > > if (strcmp(incoming, "defer") != 0) {
> > > qmp_migrate_incoming(incoming, &local_err);
> > > if (local_err) {
> > > error_reportf_err(local_err, "-incoming %s: ", incoming);
> > > exit(1);
> > > }
> > > }
> > > } else if (autostart) {
> > > qmp_cont(NULL);
> > > }
> > >
> > > It's the only piece of code which distinguishes "-incoming defer" from
> > > "-incoming ".
> > >
> > > So I'm not sure what the problem would be with keeping it?
> >
> > User friendliness.
> >
> > First of all, I use it all the time. And I know that it is useful for
> > developers. I was the one asking peter to implement -global
> > migration.foo to be able to test multifd with it.
> >
> > The problem is that if you use more than two channels with multifd, on
> > the incoming side, you need to do:
> >
> > - migrate_set_parameter multifd-channels 16
> > - migrate_incoming
> >
> > And people continue to do:
> >
> > - qemu -incoming
> > - migrate_set_parameter multifd-channels 16 (on the command line)
> >
> > And they complain that it fails, because we are calling listen with the
> > wrong value.
>
> IMHO if we want to improve user friendliness then arguing about use
> of the CLI vs QMP for migration is completely missing the bigger
> picture IMHO.
>
> I've mentioned several times before that the user should never need to
> set this multifd-channels parameter (nor many other parameters) on the
> destination in the first place.
>
> The QEMU migration stream should be changed to add a full
> bi-directional handshake, with negotiation of most parameters.
> IOW, the src QEMU should be configured with 16 channels, and
> it should connect the primary control channel, and then directly
> tell the dest that it wants to use 16 multifd channels.
>
> If we're expecting the user to pass this info across to the dest
> manually we've already spectacularly failed wrt user friendliness.
I can try to move the todo even higher. Trying to list the initial goals
here:
- One extra phase of handshake between src/dst (maybe the time to boost
QEMU_VM_FILE_VERSION) before anything else happens.
- Dest shouldn't need to apply any cap/param, it should get all from src.
Dest still need to be setup with an URI and that should be all it needs.
- Src shouldn't need to worry on the binary version of dst anymore as long
as dest qemu supports handshake, because src can fetch it from dest.
- Handshake can always fail gracefully if anything wrong happened, it
normally should mean dest qemu is not compatible with src's setup (either
machine, device, or migration configs) for whatever reason. Src should
be able to get a solid error from dest if so.
- Handshake protocol should always be self-bootstrap-able, it means when we
change the handshake protocol it should always works with old binaries.
- When src is newer it should be able to know what's missing on dest and
skip the new bits.
- When dst is newer it should all rely on src (which is older) and it
should always understand src's language.
- All !main channels need to be established later than the handshake - if
we're going to do this anyway we probably should do it altogether to make
channels named, so each channel used in migration needs to have a common
header. Prepare to deprecate the old tricks of channel orderings.
Does it look reasonable? Anything to add/remove?
Thanks,
--
Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 5:26 PM Peter Xu wrote: > PS: we may want to postpone this to be later than migration_object_init(), > when/if there's a real patch. Yes, that's true. > > > The only incompatibility is for people who are using "," in an URI, > > > which is rare and only an issue for the "exec" protocol. > > If we worry on breaking anyone, we can apply the keyval parsing only when > !exec. Not sure whether it matters a huge lot.. No, I don't think it does. > > Aha, that makes sense. And will allow us to deprecate/remove the > > --global migration.* stuff. > > We may still need a way to set the caps/params for src qemu?.. The source can use migrate_set_parameters normally, since migration has to be started from the monitor anyway. Paolo
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> I can try to move the todo even higher. Trying to list the initial goals
> here:
>
> - One extra phase of handshake between src/dst (maybe the time to boost
> QEMU_VM_FILE_VERSION) before anything else happens.
>
> - Dest shouldn't need to apply any cap/param, it should get all from src.
> Dest still need to be setup with an URI and that should be all it needs.
>
> - Src shouldn't need to worry on the binary version of dst anymore as long
> as dest qemu supports handshake, because src can fetch it from dest.
I'm not sure that works in general. Even if we have a handshake and
bi-directional comms for live migration, we still haave the save/restore
to file codepath to deal with. The dst QEMU doesn't exist at the time
the save process is done, so we can't add logic to VMSate handling that
assumes knowledge of the dst version at time of serialization.
> - Handshake can always fail gracefully if anything wrong happened, it
> normally should mean dest qemu is not compatible with src's setup (either
> machine, device, or migration configs) for whatever reason. Src should
> be able to get a solid error from dest if so.
>
> - Handshake protocol should always be self-bootstrap-able, it means when we
> change the handshake protocol it should always works with old binaries.
>
> - When src is newer it should be able to know what's missing on dest and
> skip the new bits.
>
> - When dst is newer it should all rely on src (which is older) and it
> should always understand src's language.
I'm not convinced it can reliably self-bootstrap in a backwards
compatible manner, precisely because the current migration stream
has no handshake and only requires a unidirectional channel. I
don't think its possible for QEMU to validate that it has a fully
bi-directional channel, without adding timeouts to its detection
which I think we should strive to avoid.
I don't think we actually need self-bootstrapping anyway.
I think the mgmt app can just indicate the new v2 bi-directional
protocol when issuing the 'migrate' and 'migrate-incoming'
commands. This becomes trivial when Het's refactoring of the
migrate address QAPI is accepted:
https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
eg:
{ "execute": "migrate",
"arguments": {
"channels": [ { "channeltype": "main",
"addr": { "transport": "socket", "type": "inet",
"host": "10.12.34.9",
"port": "1050" } } ] } }
note the 'channeltype' parameter here. If we declare the 'main'
refers to the existing migration protocol, then we merely need
to define a new 'channeltype' to use as an indicator for the
v2 migration handshake protocol.
> - All !main channels need to be established later than the handshake - if
> we're going to do this anyway we probably should do it altogether to make
> channels named, so each channel used in migration needs to have a common
> header. Prepare to deprecate the old tricks of channel orderings.
Once the primary channel involves a bi-directional handshake,
we'll trivially ensure ordering - similar to how the existing
code worked fnie in TLS mode which had a bi-directional TLS
handshake.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
Juan Quintela wrote: > Only "defer" is recommended. After setting all migation parameters, > start incoming migration with "migrate-incoming uri" command. > > Signed-off-by: Juan Quintela Nack myself. Dropped on next submissiong. keyfile properties suggested by paolo is a much better suggestion. Thanks to everybody involved.
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > I can try to move the todo even higher. Trying to list the initial goals
> > here:
> >
> > - One extra phase of handshake between src/dst (maybe the time to boost
> > QEMU_VM_FILE_VERSION) before anything else happens.
> >
> > - Dest shouldn't need to apply any cap/param, it should get all from src.
> > Dest still need to be setup with an URI and that should be all it needs.
> >
> > - Src shouldn't need to worry on the binary version of dst anymore as long
> > as dest qemu supports handshake, because src can fetch it from dest.
>
> I'm not sure that works in general. Even if we have a handshake and
> bi-directional comms for live migration, we still haave the save/restore
> to file codepath to deal with. The dst QEMU doesn't exist at the time
> the save process is done, so we can't add logic to VMSate handling that
> assumes knowledge of the dst version at time of serialization.
My current thought was still based on a new cap or anything the user would
need to specify first on both sides (but hopefully the last cap to set on
dest).
E.g. if with a new handshake cap we shouldn't set it on a exec: or file:
protocol migration, and it should just fail on qmp_migrate() telling that
the URI is not supported if the cap is set. Return path is definitely
required here.
>
> > - Handshake can always fail gracefully if anything wrong happened, it
> > normally should mean dest qemu is not compatible with src's setup (either
> > machine, device, or migration configs) for whatever reason. Src should
> > be able to get a solid error from dest if so.
> >
> > - Handshake protocol should always be self-bootstrap-able, it means when we
> > change the handshake protocol it should always works with old binaries.
> >
> > - When src is newer it should be able to know what's missing on dest and
> > skip the new bits.
> >
> > - When dst is newer it should all rely on src (which is older) and it
> > should always understand src's language.
>
> I'm not convinced it can reliably self-bootstrap in a backwards
> compatible manner, precisely because the current migration stream
> has no handshake and only requires a unidirectional channel.
Yes, please see above. I meant when we grow the handshake protocol we
should make sure we don't need anything new to be setup either on src/dst
of qemu. It won't apply to before-handshake binaries.
> I don't think its possible for QEMU to validate that it has a fully
> bi-directional channel, without adding timeouts to its detection which I
> think we should strive to avoid.
>
> I don't think we actually need self-bootstrapping anyway.
>
> I think the mgmt app can just indicate the new v2 bi-directional
> protocol when issuing the 'migrate' and 'migrate-incoming'
> commands. This becomes trivial when Het's refactoring of the
> migrate address QAPI is accepted:
>
> https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
>
> eg:
>
> { "execute": "migrate",
> "arguments": {
> "channels": [ { "channeltype": "main",
> "addr": { "transport": "socket", "type": "inet",
>"host": "10.12.34.9",
> "port": "1050" } } ] } }
>
> note the 'channeltype' parameter here. If we declare the 'main'
> refers to the existing migration protocol, then we merely need
> to define a new 'channeltype' to use as an indicator for the
> v2 migration handshake protocol.
Using a new channeltype would also work at least on src qemu, but I'm not
sure on how dest qemu would know that it needs a handshake in that case,
because it knows nothing until the connection is established.
Maybe we still need QEMU_VM_FILE_VERSION to be boosted at least in this
case, so dest can read this at the very beginning, old binaries will fail
immediately, new binaries will start to talk with v2 language.
>
> > - All !main channels need to be established later than the handshake - if
> > we're going to do this anyway we probably should do it altogether to make
> > channels named, so each channel used in migration needs to have a common
> > header. Prepare to deprecate the old tricks of channel orderings.
>
> Once the primary channel involves a bi-directional handshake,
> we'll trivially ensure ordering - similar to how the existing
> code worked fnie in TLS mode which had a bi-directional TLS
> handshake.
I'm not sure I fully get it here.
IIUC tls handshake was mostly transparent to QEMU in this case while we're
relying on gnutls_handshake(). Here IIUC we need to design the roundtrip
messages to sync up two qemus well.
The round trip messages can contain a lot of things that can be useful to
us, besides knowing what features dest supports, what caps src use, we can
e.g. also provide a device tree dump from dest and try to match it on sr
Re: [RFC 4/6] migration: Deprecate -incoming
Peter Xu wrote:
> On Mon, Jun 12, 2023 at 10:51:08PM +0200, Juan Quintela wrote:
>> Peter Xu wrote:
>> > On Mon, Jun 12, 2023 at 09:33:42PM +0200, Juan Quintela wrote:
>> >> Only "defer" is recommended. After setting all migation parameters,
>> >> start incoming migration with "migrate-incoming uri" command.
>> >>
>> >> Signed-off-by: Juan Quintela
>> >> ---
>> >> docs/about/deprecated.rst | 7 +++
>> >> softmmu/vl.c | 2 ++
>> >> 2 files changed, 9 insertions(+)
>> >>
>> >> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
>> >> index 47e98dc95e..518672722d 100644
>> >> --- a/docs/about/deprecated.rst
>> >> +++ b/docs/about/deprecated.rst
>> >> @@ -447,3 +447,10 @@ The new way to modify migration is using migration
>> >> parameters.
>> >> ``blk`` functionality can be acchieved using
>> >> ``migrate_set_parameter block-incremental true``.
>> >>
>> >> +``-incoming uri`` (since 8.1)
>> >> +'
>> >> +
>> >> +Everything except ``-incoming defer`` are deprecated. This allows to
>> >> +setup parameters before launching the proper migration with
>> >> +``migrate-incoming uri``.
>> >> +
>> >> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> >> index b0b96f67fa..7fe865ab59 100644
>> >> --- a/softmmu/vl.c
>> >> +++ b/softmmu/vl.c
>> >> @@ -2651,6 +2651,8 @@ void qmp_x_exit_preconfig(Error **errp)
>> >> if (incoming) {
>> >> Error *local_err = NULL;
>> >> if (strcmp(incoming, "defer") != 0) {
>> >> +warn_report("-incoming %s is deprecated, use -incoming defer
>> >> and "
>> >> +" set the uri with migrate-incoming.", incoming);
>> >
>> > I still use uri for all my scripts, alongside with "-global migration.xxx"
>> > and it works.
>>
>> You know what you are doing (TM).
>> And remember that we don't support -gobal migration.x-foo.
>> Yes, I know, we should drop the "x-" prefixes.
>
> I hope they'll always be there. :) They're pretty handy for tests, when we
> want to boot a VM without the need to script the sequences of qmp cmds.
>
> Yes, we probably should just always drop the x-. We can always declare
> debugging purpose for all -global migration.* fields.
>
>>
>> > Shall we just leave it there? Or is deprecating it helps us in any form?
>>
>> See the patches two weeks ago when people complained that lisen(.., num)
>> was too low. And there are other parameters that work the same way
>> (that I convenientely had forgotten). So the easiest way to get things
>> right is to use "defer" always. Using -incoming "uri" should only be
>> for people that "know what they are doing", so we had to ways to do it:
>> - review all migration options and see which ones work without defer
>> and document it
>> - deprecate everything that is not defer.
>>
>> Anything else is not going to be very user unfriendly.
>> What do you think.
>
> IIRC Wei Wang had a series just for that, so after that patchset applied we
> should have fixed all issues cleanly?
No, what he does is using always a very big value for listen. But that
is it. Anyways, I don't know how to change the backlog listen value
without restarting the listen call.
> Is there one more thing that's not
> working right there?
Compression has other problems. But independentely of that, they have
the problem that we need to set the parameters before we call incoming.
>> PD. This series are RFC for multiple reasons O:-)
>
> Happy to know the rest (besides which I know will break my script :).
Thanks, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
Peter Xu wrote: > On Thu, Jun 22, 2023 at 11:22:56AM +0200, Thomas Huth wrote: >> Then simply forbid "migrate_set_parameter multifd-channels ..." if the uri >> has been specified on the command line? > > Yeah, actually already in a pull (even though the pr may need a new one..): > > https://lore.kernel.org/r/20230622021320.66124-23-quint...@redhat.com That is a different problem, and different solution. It you try to set multifd_channels after migration has started, it just fails telling that you can't change it so late. Later, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 03:20:01PM -0400, Peter Xu wrote:
> On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote:
> > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > > I can try to move the todo even higher. Trying to list the initial goals
> > > here:
> > >
> > > - One extra phase of handshake between src/dst (maybe the time to boost
> > > QEMU_VM_FILE_VERSION) before anything else happens.
> > >
> > > - Dest shouldn't need to apply any cap/param, it should get all from src.
> > > Dest still need to be setup with an URI and that should be all it needs.
> > >
> > > - Src shouldn't need to worry on the binary version of dst anymore as long
> > > as dest qemu supports handshake, because src can fetch it from dest.
> >
> > I'm not sure that works in general. Even if we have a handshake and
> > bi-directional comms for live migration, we still haave the save/restore
> > to file codepath to deal with. The dst QEMU doesn't exist at the time
> > the save process is done, so we can't add logic to VMSate handling that
> > assumes knowledge of the dst version at time of serialization.
>
> My current thought was still based on a new cap or anything the user would
> need to specify first on both sides (but hopefully the last cap to set on
> dest).
>
> E.g. if with a new handshake cap we shouldn't set it on a exec: or file:
> protocol migration, and it should just fail on qmp_migrate() telling that
> the URI is not supported if the cap is set. Return path is definitely
> required here.
exec can support bi-directional migration - we have both stdin + stdout
for the command. For exec it is mostly a documentation problem - you
can't merely use 'cat' for example, but if you used 'socat' that could
be made to work bi-directionally.
> > I don't think its possible for QEMU to validate that it has a fully
> > bi-directional channel, without adding timeouts to its detection which I
> > think we should strive to avoid.
> >
> > I don't think we actually need self-bootstrapping anyway.
> >
> > I think the mgmt app can just indicate the new v2 bi-directional
> > protocol when issuing the 'migrate' and 'migrate-incoming'
> > commands. This becomes trivial when Het's refactoring of the
> > migrate address QAPI is accepted:
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
> >
> > eg:
> >
> > { "execute": "migrate",
> > "arguments": {
> > "channels": [ { "channeltype": "main",
> > "addr": { "transport": "socket", "type": "inet",
> >"host": "10.12.34.9",
> > "port": "1050" } } ] } }
> >
> > note the 'channeltype' parameter here. If we declare the 'main'
> > refers to the existing migration protocol, then we merely need
> > to define a new 'channeltype' to use as an indicator for the
> > v2 migration handshake protocol.
>
> Using a new channeltype would also work at least on src qemu, but I'm not
> sure on how dest qemu would know that it needs a handshake in that case,
> because it knows nothing until the connection is established.
In Het's series the 'migrate_incoming' command similarly has a chaneltype
parameter.
With regards,
Daniel
--
|: https://berrange.com -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote: > > I've mentioned several times before that the user should never need to > > set this multifd-channels parameter (nor many other parameters) on the > > destination in the first place. > > > > The QEMU migration stream should be changed to add a full > > bi-directional handshake, with negotiation of most parameters. > > IOW, the src QEMU should be configured with 16 channels, and > > it should connect the primary control channel, and then directly > > tell the dest that it wants to use 16 multifd channels. > > > > If we're expecting the user to pass this info across to the dest > > manually we've already spectacularly failed wrt user friendliness. > > I can try to move the todo even higher. Trying to list the initial goals > here: > > - One extra phase of handshake between src/dst (maybe the time to boost > QEMU_VM_FILE_VERSION) before anything else happens. > > - Dest shouldn't need to apply any cap/param, it should get all from src. > Dest still need to be setup with an URI and that should be all it needs. There are a few that the dest will still need set explicitly. Specifically the TLS parameters - tls-authz and tls-creds, because those are both related to --object parameters configured on the dst QEMU. Potentially there's an argument to be made for the TLS parameters to be part fo the initial 'migrate' and 'migrate-incoming' command data, as they're specifically related to the connection establishment, while (most) of the other params are related to the migration protocol running inside the connection. A few other parameters are also related to the connection establishment, most notably the enablement multifd, postcopy and postcopy-pre-empt. I think with those ones we don't need to set them on the src either. With the new migration handshake we should probably use multifd codepaths unconditionally, with a single channel. By matching with the introduction of new protocol, we have a nice point against which to deprecate the old non-multifd codepaths. We'll need to keep the non-multifd code around *alot* longer than the normal deprecation cycle though, as we need mig to/from very old QEMUs. The enablement of postcopy could be automatic too - src & dst can both detect if their host OS supports it. That would make all migrations post-copy capable. The mgmt app just needs to trigger the switch to post-copy mode *if* they want to use it. Likewise we can just always assume postcopy-pre-empt is available. I think 'return-path' becomes another one we can just assume too. With regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [RFC 4/6] migration: Deprecate -incoming
On Fri, Jun 23, 2023 at 08:17:46AM +0100, Daniel P. Berrangé wrote:
> On Thu, Jun 22, 2023 at 03:20:01PM -0400, Peter Xu wrote:
> > On Thu, Jun 22, 2023 at 05:33:29PM +0100, Daniel P. Berrangé wrote:
> > > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote:
> > > > I can try to move the todo even higher. Trying to list the initial
> > > > goals
> > > > here:
> > > >
> > > > - One extra phase of handshake between src/dst (maybe the time to boost
> > > > QEMU_VM_FILE_VERSION) before anything else happens.
> > > >
> > > > - Dest shouldn't need to apply any cap/param, it should get all from
> > > > src.
> > > > Dest still need to be setup with an URI and that should be all it
> > > > needs.
> > > >
> > > > - Src shouldn't need to worry on the binary version of dst anymore as
> > > > long
> > > > as dest qemu supports handshake, because src can fetch it from dest.
> > >
> > > I'm not sure that works in general. Even if we have a handshake and
> > > bi-directional comms for live migration, we still haave the save/restore
> > > to file codepath to deal with. The dst QEMU doesn't exist at the time
> > > the save process is done, so we can't add logic to VMSate handling that
> > > assumes knowledge of the dst version at time of serialization.
> >
> > My current thought was still based on a new cap or anything the user would
> > need to specify first on both sides (but hopefully the last cap to set on
> > dest).
> >
> > E.g. if with a new handshake cap we shouldn't set it on a exec: or file:
> > protocol migration, and it should just fail on qmp_migrate() telling that
> > the URI is not supported if the cap is set. Return path is definitely
> > required here.
>
> exec can support bi-directional migration - we have both stdin + stdout
> for the command. For exec it is mostly a documentation problem - you
> can't merely use 'cat' for example, but if you used 'socat' that could
> be made to work bi-directionally.
Okay. Just an example that the handshake just cannot work for all the
cases, and it should always be able to fail.
So when exec doesn't properly provide return path, I think with
handshake=on we should get a timeout of not getting response properly and
fail the migration after the timeout, then.
There're a bunch of implications and details that need to be investigated
around such a handshake if it'll be proposed for real, so I'm not yet sure
whether there's something that may be surprising. For channeltypes it
seems all fine for now. Hopefully nothing obvious overlooked.
>
> > > I don't think its possible for QEMU to validate that it has a fully
> > > bi-directional channel, without adding timeouts to its detection which I
> > > think we should strive to avoid.
> > >
> > > I don't think we actually need self-bootstrapping anyway.
> > >
> > > I think the mgmt app can just indicate the new v2 bi-directional
> > > protocol when issuing the 'migrate' and 'migrate-incoming'
> > > commands. This becomes trivial when Het's refactoring of the
> > > migrate address QAPI is accepted:
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2023-05/msg04851.html
> > >
> > > eg:
> > >
> > > { "execute": "migrate",
> > > "arguments": {
> > > "channels": [ { "channeltype": "main",
> > > "addr": { "transport": "socket", "type": "inet",
> > >"host": "10.12.34.9",
> > > "port": "1050" } } ] } }
> > >
> > > note the 'channeltype' parameter here. If we declare the 'main'
> > > refers to the existing migration protocol, then we merely need
> > > to define a new 'channeltype' to use as an indicator for the
> > > v2 migration handshake protocol.
> >
> > Using a new channeltype would also work at least on src qemu, but I'm not
> > sure on how dest qemu would know that it needs a handshake in that case,
> > because it knows nothing until the connection is established.
>
> In Het's series the 'migrate_incoming' command similarly has a chaneltype
> parameter.
Oh, yeah then that'll just work. Thanks.
--
Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Fri, Jun 23, 2023 at 09:23:18AM +0100, Daniel P. Berrangé wrote: > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote: > > > I've mentioned several times before that the user should never need to > > > set this multifd-channels parameter (nor many other parameters) on the > > > destination in the first place. > > > > > > The QEMU migration stream should be changed to add a full > > > bi-directional handshake, with negotiation of most parameters. > > > IOW, the src QEMU should be configured with 16 channels, and > > > it should connect the primary control channel, and then directly > > > tell the dest that it wants to use 16 multifd channels. > > > > > > If we're expecting the user to pass this info across to the dest > > > manually we've already spectacularly failed wrt user friendliness. > > > > I can try to move the todo even higher. Trying to list the initial goals > > here: > > > > - One extra phase of handshake between src/dst (maybe the time to boost > > QEMU_VM_FILE_VERSION) before anything else happens. > > > > - Dest shouldn't need to apply any cap/param, it should get all from src. > > Dest still need to be setup with an URI and that should be all it needs. > > There are a few that the dest will still need set explicitly. Specifically > the TLS parameters - tls-authz and tls-creds, because those are both > related to --object parameters configured on the dst QEMU. Potentially > there's an argument to be made for the TLS parameters to be part fo the > initial 'migrate' and 'migrate-incoming' command data, as they're > specifically related to the connection establishment, while (most) of > the other params are related to the migration protocol running inside > the connection. Ideally we can even make tls options to be after the main connection is established, IOW the gnutls handshake can be part of the generic handshake. But yeah I agree that may contain much more work, so we may start with assuming the v2 handshake just happen on the tls channel built for now. I think the new protocol should allow extension so when we want to move the tls handshake into it v2 protocol should be able to first detect src/dst binary support of that, and switch to that if we want - then we can even got a src qemu migration failure which tells "dest qemu forget to setup tls credentials in cmdlines", or anything wrong on dest during tls setup. > > A few other parameters are also related to the connection establishment, > most notably the enablement multifd, postcopy and postcopy-pre-empt. As I mentioned in the list, I plan to make this part of the default v2 where v2 handshake will take care of managing the connections rather than relying on the old code. I'm not sure how complicated it'll be, but the v2 protocol just sounds a good fit for having such a major change on how we setup the channels, and chance we get all things alright from the start. > > I think with those ones we don't need to set them on the src either. > With the new migration handshake we should probably use multifd > codepaths unconditionally, with a single channel. The v2 handshake will be beneficial to !multifd as well. Right now I tend to make it also work for !multifd, e.g., it always makes sense to do a device tree comparision before migration, even if someone used special tunneling so multifd may not be able to be enabled for whatever reason, but as long as a return path is available so they can talk. > By matching with the introduction of new protocol, we have a nice point > against which to deprecate the old non-multifd codepaths. We'll need to > keep the non-multifd code around *alot* longer than the normal > deprecation cycle though, as we need mig to/from very old QEMUs. I actually had a feeling that we should always keep it.. I'm not sure whether we must combine a new handshake to "making multifd the default". I do think we can make the !multifd path very simple though, e.g., I'd think we should start considering deprecate things like !multifd+compressions etc earlier than that. > > The enablement of postcopy could be automatic too - src & dst can > both detect if their host OS supports it. That would make all > migrations post-copy capable. The mgmt app just needs to trigger > the switch to post-copy mode *if* they want to use it. Sounds doable. > > Likewise we can just always assume postcopy-pre-empt is available. > > I think 'return-path' becomes another one we can just assume too. Right, handshake cap (or with the new QAPI of URI replacement) should imply return path already. Thanks, -- Peter Xu
Re: [RFC 4/6] migration: Deprecate -incoming
On Fri, Jun 23, 2023 at 10:51:53AM -0400, Peter Xu wrote: > On Fri, Jun 23, 2023 at 09:23:18AM +0100, Daniel P. Berrangé wrote: > > On Thu, Jun 22, 2023 at 11:54:43AM -0400, Peter Xu wrote: > > > On Thu, Jun 22, 2023 at 10:59:58AM +0100, Daniel P. Berrangé wrote: > > > > I've mentioned several times before that the user should never need to > > > > set this multifd-channels parameter (nor many other parameters) on the > > > > destination in the first place. > > > > > > > > The QEMU migration stream should be changed to add a full > > > > bi-directional handshake, with negotiation of most parameters. > > > > IOW, the src QEMU should be configured with 16 channels, and > > > > it should connect the primary control channel, and then directly > > > > tell the dest that it wants to use 16 multifd channels. > > > > > > > > If we're expecting the user to pass this info across to the dest > > > > manually we've already spectacularly failed wrt user friendliness. > > > > > > I can try to move the todo even higher. Trying to list the initial goals > > > here: > > > > > > - One extra phase of handshake between src/dst (maybe the time to boost > > > QEMU_VM_FILE_VERSION) before anything else happens. > > > > > > - Dest shouldn't need to apply any cap/param, it should get all from src. > > > Dest still need to be setup with an URI and that should be all it needs. > > > > There are a few that the dest will still need set explicitly. Specifically > > the TLS parameters - tls-authz and tls-creds, because those are both > > related to --object parameters configured on the dst QEMU. Potentially > > there's an argument to be made for the TLS parameters to be part fo the > > initial 'migrate' and 'migrate-incoming' command data, as they're > > specifically related to the connection establishment, while (most) of > > the other params are related to the migration protocol running inside > > the connection. > > Ideally we can even make tls options to be after the main connection is > established, IOW the gnutls handshake can be part of the generic handshake. > But yeah I agree that may contain much more work, so we may start with > assuming the v2 handshake just happen on the tls channel built for now. > > I think the new protocol should allow extension so when we want to move the > tls handshake into it v2 protocol should be able to first detect src/dst > binary support of that, and switch to that if we want - then we can even > got a src qemu migration failure which tells "dest qemu forget to setup tls > credentials in cmdlines", or anything wrong on dest during tls setup. Doing negotiated "upgrades" from plain to TLS mode is generally frowned upon, as it opens up potentially dangerous attack routes which can prevent the upgrade from happening. If the user/app controlling the client and server side of a connection both know they want TLS, the best practice is for a connection to start in TLS mode *immediately*, never sending any data in the clear. We have this ability in QEMU right now, with libvirt explicitly enabling TLS mode on src + dst, and we should keep that in any v2 migration protocol. > > A few other parameters are also related to the connection establishment, > > most notably the enablement multifd, postcopy and postcopy-pre-empt. > > As I mentioned in the list, I plan to make this part of the default v2 > where v2 handshake will take care of managing the connections rather than > relying on the old code. I'm not sure how complicated it'll be, but the v2 > protocol just sounds a good fit for having such a major change on how we > setup the channels, and chance we get all things alright from the start. > > > > > I think with those ones we don't need to set them on the src either. > > With the new migration handshake we should probably use multifd > > codepaths unconditionally, with a single channel. > > The v2 handshake will be beneficial to !multifd as well. Right now I tend > to make it also work for !multifd, e.g., it always makes sense to do a > device tree comparision before migration, even if someone used special > tunneling so multifd may not be able to be enabled for whatever reason, but > as long as a return path is available so they can talk. > > > By matching with the introduction of new protocol, we have a nice point > > against which to deprecate the old non-multifd codepaths. We'll need to > > keep the non-multifd code around *alot* longer than the normal > > deprecation cycle though, as we need mig to/from very old QEMUs. > > I actually had a feeling that we should always keep it.. I'm not sure > whether we must combine a new handshake to "making multifd the default". I > do think we can make the !multifd path very simple though, e.g., I'd think > we should start considering deprecate things like !multifd+compressions etc > earlier than that. My thought is that right now QEMU has effectively has two completely distinct migration protocols (even though they share the initial
