| Message ID | 1733145611-62315-10-git-send-email-steven.sistare@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Live update: cpr-transfer | expand |
Steve Sistare <steven.sistare@oracle.com> writes: > Extend the -incoming option to allow an @MigrationChannel to be specified. > This allows channels other than 'main' to be described on the command > line, which will be needed for CPR. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com> [...] > diff --git a/qemu-options.hx b/qemu-options.hx > index 02b9118..fab50ce 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -4937,10 +4937,17 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ > "-incoming exec:cmdline\n" \ > " accept incoming migration on given file descriptor\n" \ > " or from given external command\n" \ > + "-incoming @MigrationChannel\n" \ > + " accept incoming migration on the channel\n" \ > "-incoming defer\n" \ > " wait for the URI to be specified via migrate_incoming\n", > QEMU_ARCH_ALL) > SRST > +The -incoming option specifies the migration channel for an incoming > +migration. It may be used multiple times to specify multiple > +migration channel types. Really? If I understand the code below correctly, the last -incoming wins, and any previous ones are silently ignored. > The channel type is specified in @MigrationChannel, > +and is 'main' for all other forms of -incoming. > + > ``-incoming tcp:[host]:port[,to=maxport][,ipv4=on|off][,ipv6=on|off]`` > \ > ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]`` > @@ -4960,6 +4967,16 @@ SRST > Accept incoming migration as an output from specified external > command. > > +``-incoming @MigrationChannel`` > + Accept incoming migration on the channel. See the QAPI documentation > + for the syntax of the @MigrationChannel data element. For example: > + :: I get what you're trying to express, but there's no precedence for referring to QAPI types like @TypeName in option documentation. But let's ignore this until after we nailed down the actual interface, on which I have questions below. > + > + -incoming '{"channel-type": "main", > + "addr": { "transport": "socket", > + "type": "unix", > + "path": "my.sock" }}' > + > ``-incoming defer`` > Wait for the URI to be specified via migrate\_incoming. The monitor > can be used to change settings (such as migration parameters) prior > diff --git a/system/vl.c b/system/vl.c > index 4151a79..2c24c60 100644 > --- a/system/vl.c > +++ b/system/vl.c > @@ -123,6 +123,7 @@ > #include "qapi/qapi-visit-block-core.h" > #include "qapi/qapi-visit-compat.h" > #include "qapi/qapi-visit-machine.h" > +#include "qapi/qapi-visit-migration.h" > #include "qapi/qapi-visit-ui.h" > #include "qapi/qapi-commands-block-core.h" > #include "qapi/qapi-commands-migration.h" > @@ -159,6 +160,7 @@ typedef struct DeviceOption { > static const char *cpu_option; > static const char *mem_path; > static const char *incoming; > +static MigrationChannelList *incoming_channels; > static const char *loadvm; > static const char *accelerators; > static bool have_custom_ram_size; > @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) > QTAILQ_INSERT_TAIL(&object_opts, opt, next); > } > > +static void incoming_option_parse(const char *str) > +{ > + MigrationChannel *channel; > + > + if (str[0] == '{') { > + QObject *obj = qobject_from_json(str, &error_fatal); > + Visitor *v = qobject_input_visitor_new(obj); > + > + qobject_unref(obj); > + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); > + visit_free(v); > + } else if (!strcmp(str, "defer")) { > + channel = NULL; > + } else { > + migrate_uri_parse(str, &channel, &error_fatal); > + } > + > + /* New incoming spec replaces the previous */ > + > + if (incoming_channels) { > + qapi_free_MigrationChannelList(incoming_channels); > + } > + if (channel) { > + incoming_channels = g_new0(MigrationChannelList, 1); > + incoming_channels->value = channel; > + } > + incoming = str; > +} @incoming is set to @optarg. @incoming_channels is set to a MigrationChannelList of exactly one element, parsed from @incoming. Except when @incoming is "defer", then @incoming_channels is set to null. @incoming is only ever used as a flag. Turn it into a bool? Oh, wait... see my comment on the next hunk. Option -incoming resembles QMP command migrate-incoming. Differences: * migrate-incoming keeps legacy URI and modern argument separate: there are two named arguments, and exactly one of them must be passed. -incoming overloads them: if @optarg starts with '{', it's modern, else legacy URI. Because of that, -incoming *only* supports JSON syntax for modern, not dotted keys. Other JSON-capable arguments support both. How can a management application detect that -incoming supports modern? Sure overloading -incoming this way is a good idea? * migrate-incoming takes a list of channels, currently restricted to a single channel. -incoming takes a channel. If we lift the restriction, -incoming syntax will become even messier: we'll have to additionally overload list of channel. Should -incoming take a list from the start, like migrate-incoming does? > + > static void object_option_parse(const char *str) > { > QemuOpts *opts; > @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) > if (incoming) { > Error *local_err = NULL; > if (strcmp(incoming, "defer") != 0) { > - qmp_migrate_incoming(incoming, false, NULL, true, true, > + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, > &local_err); You move the parsing of legacy URI from within qmp_migrate_incoming() into incoming_option_parse(). The alternative is not to parse it in incoming_option_parse(), but pass it to qmp_migrate_incoming() like this: qmp_migrate_incoming(incoming, !incoming, incoming_channels, true, true, &local_err); > if (local_err) { > error_reportf_err(local_err, "-incoming %s: ", incoming); > @@ -3477,7 +3508,7 @@ void qemu_init(int argc, char **argv) > if (!incoming) { > runstate_set(RUN_STATE_INMIGRATE); > } > - incoming = optarg; > + incoming_option_parse(optarg); > break; > case QEMU_OPTION_only_migratable: > only_migratable = 1;
On 12/5/2024 10:23 AM, Markus Armbruster wrote: > Steve Sistare <steven.sistare@oracle.com> writes: > >> Extend the -incoming option to allow an @MigrationChannel to be specified. >> This allows channels other than 'main' to be described on the command >> line, which will be needed for CPR. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> > > [...] > >> diff --git a/qemu-options.hx b/qemu-options.hx >> index 02b9118..fab50ce 100644 >> --- a/qemu-options.hx >> +++ b/qemu-options.hx >> @@ -4937,10 +4937,17 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ >> "-incoming exec:cmdline\n" \ >> " accept incoming migration on given file descriptor\n" \ >> " or from given external command\n" \ >> + "-incoming @MigrationChannel\n" \ >> + " accept incoming migration on the channel\n" \ >> "-incoming defer\n" \ >> " wait for the URI to be specified via migrate_incoming\n", >> QEMU_ARCH_ALL) >> SRST >> +The -incoming option specifies the migration channel for an incoming >> +migration. It may be used multiple times to specify multiple >> +migration channel types. > > Really? If I understand the code below correctly, the last -incoming > wins, and any previous ones are silently ignored. See patch "cpr-channel", where the cpr channel is saved separately. Last wins, per channel type. I did this to preserve the current behavior of -incoming in which last wins. qemu_start_incoming_migration will need modification if more types are added. >> The channel type is specified in @MigrationChannel, >> +and is 'main' for all other forms of -incoming. >> + >> ``-incoming tcp:[host]:port[,to=maxport][,ipv4=on|off][,ipv6=on|off]`` >> \ >> ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]`` >> @@ -4960,6 +4967,16 @@ SRST >> Accept incoming migration as an output from specified external >> command. >> >> +``-incoming @MigrationChannel`` >> + Accept incoming migration on the channel. See the QAPI documentation >> + for the syntax of the @MigrationChannel data element. For example: >> + :: > > I get what you're trying to express, but there's no precedence for > referring to QAPI types like @TypeName in option documentation. But > let's ignore this until after we nailed down the actual interface, on > which I have questions below. Ack. >> + >> + -incoming '{"channel-type": "main", >> + "addr": { "transport": "socket", >> + "type": "unix", >> + "path": "my.sock" }}' >> + >> ``-incoming defer`` >> Wait for the URI to be specified via migrate\_incoming. The monitor >> can be used to change settings (such as migration parameters) prior >> diff --git a/system/vl.c b/system/vl.c >> index 4151a79..2c24c60 100644 >> --- a/system/vl.c >> +++ b/system/vl.c >> @@ -123,6 +123,7 @@ >> #include "qapi/qapi-visit-block-core.h" >> #include "qapi/qapi-visit-compat.h" >> #include "qapi/qapi-visit-machine.h" >> +#include "qapi/qapi-visit-migration.h" >> #include "qapi/qapi-visit-ui.h" >> #include "qapi/qapi-commands-block-core.h" >> #include "qapi/qapi-commands-migration.h" >> @@ -159,6 +160,7 @@ typedef struct DeviceOption { >> static const char *cpu_option; >> static const char *mem_path; >> static const char *incoming; >> +static MigrationChannelList *incoming_channels; >> static const char *loadvm; >> static const char *accelerators; >> static bool have_custom_ram_size; >> @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) >> QTAILQ_INSERT_TAIL(&object_opts, opt, next); >> } >> >> +static void incoming_option_parse(const char *str) >> +{ >> + MigrationChannel *channel; >> + >> + if (str[0] == '{') { >> + QObject *obj = qobject_from_json(str, &error_fatal); >> + Visitor *v = qobject_input_visitor_new(obj); >> + >> + qobject_unref(obj); >> + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); >> + visit_free(v); >> + } else if (!strcmp(str, "defer")) { >> + channel = NULL; >> + } else { >> + migrate_uri_parse(str, &channel, &error_fatal); >> + } >> + >> + /* New incoming spec replaces the previous */ >> + >> + if (incoming_channels) { >> + qapi_free_MigrationChannelList(incoming_channels); >> + } >> + if (channel) { >> + incoming_channels = g_new0(MigrationChannelList, 1); >> + incoming_channels->value = channel; >> + } >> + incoming = str; >> +} > > @incoming is set to @optarg. > > @incoming_channels is set to a MigrationChannelList of exactly one > element, parsed from @incoming. Except when @incoming is "defer", then > @incoming_channels is set to null. > > @incoming is only ever used as a flag. Turn it into a bool? The remembered incoming specifier is also used in an error message in qmp_x_exit_preconfig: error_reportf_err(local_err, "-incoming %s: ", incoming); > Oh, wait... see my comment on the next hunk. > > Option -incoming resembles QMP command migrate-incoming. Differences: > > * migrate-incoming keeps legacy URI and modern argument separate: there > are two named arguments, and exactly one of them must be passed. > -incoming overloads them: if @optarg starts with '{', it's modern, > else legacy URI. > > Because of that, -incoming *only* supports JSON syntax for modern, not > dotted keys. Other JSON-capable arguments support both. Not sure I follow. Could you give me a dotted key example for a JSON-capable argument? Do we care about dotted key for incoming, given the user can specify a simple legacy URI? > How can a management application detect that -incoming supports > modern? How does mgmt detect when other arguments support JSON? The presence of cpr-transfer mode implies -incoming JSON support, though that is indirect. We could add a feature to the migrate-incoming command, like json-cli for device_add. Seems like overkill though. 'feature' is little used, except for unstable and deprecated. > Sure overloading -incoming this way is a good idea? > > * migrate-incoming takes a list of channels, currently restricted to a > single channel. -incoming takes a channel. If we lift the > restriction, -incoming syntax will become even messier: we'll have to > additionally overload list of channel. > > Should -incoming take a list from the start, like migrate-incoming > does? That was my first try. However, to support the equivalent of '-incoming deferred', we need to add an 'defer' key to the channel, and when defer is true, the other keys that are currently mandatory must be omitted. The tweaks to the implementation and specification seemed not worth worth it. If we want -incoming to also support a channel list in the future, we can simply check for an initial '[' token. >> + >> static void object_option_parse(const char *str) >> { >> QemuOpts *opts; >> @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) >> if (incoming) { >> Error *local_err = NULL; >> if (strcmp(incoming, "defer") != 0) { >> - qmp_migrate_incoming(incoming, false, NULL, true, true, >> + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, >> &local_err); > > You move the parsing of legacy URI from within qmp_migrate_incoming() > into incoming_option_parse(). > > The alternative is not to parse it in incoming_option_parse(), but pass > it to qmp_migrate_incoming() like this: > > qmp_migrate_incoming(incoming, !incoming, incoming_channels, > true, true, &local_err); Sure, I can tweak that, but I need to define an additional incoming_uri variable: qmp_migrate_incoming(incoming_uri, !!incoming_channels, incoming_channels, ... Only one of incoming_uri and incoming_channels can be non-NULL (checked in qemu_start_incoming_migration). Would you prefer I continue down this path, or revert to the previous -cpr-uri option? I made this change to make the incoming interface look more like the V4 outgoing interface, in which the user adds a cpr channel to the migrate command channels. - Steve
Steven Sistare <steven.sistare@oracle.com> writes: > On 12/5/2024 10:23 AM, Markus Armbruster wrote: >> Steve Sistare <steven.sistare@oracle.com> writes: >> >>> Extend the -incoming option to allow an @MigrationChannel to be specified. >>> This allows channels other than 'main' to be described on the command >>> line, which will be needed for CPR. >>> >>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >> [...] >> >>> diff --git a/qemu-options.hx b/qemu-options.hx >>> index 02b9118..fab50ce 100644 >>> --- a/qemu-options.hx >>> +++ b/qemu-options.hx >>> @@ -4937,10 +4937,17 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ >>> "-incoming exec:cmdline\n" \ >>> " accept incoming migration on given file descriptor\n" \ >>> " or from given external command\n" \ >>> + "-incoming @MigrationChannel\n" \ >>> + " accept incoming migration on the channel\n" \ >>> "-incoming defer\n" \ >>> " wait for the URI to be specified via migrate_incoming\n", >>> QEMU_ARCH_ALL) >>> SRST >>> +The -incoming option specifies the migration channel for an incoming >>> +migration. It may be used multiple times to specify multiple >>> +migration channel types. >> >> Really? If I understand the code below correctly, the last -incoming >> wins, and any previous ones are silently ignored. > > See patch "cpr-channel", where the cpr channel is saved separately. > Last wins, per channel type. > I did this to preserve the current behavior of -incoming in which last wins. Documentation needs to be clarified then. > qemu_start_incoming_migration will need modification if more types are added. > >>> The channel type is specified in @MigrationChannel, >>> +and is 'main' for all other forms of -incoming. >>> + >>> ``-incoming tcp:[host]:port[,to=maxport][,ipv4=on|off][,ipv6=on|off]`` >>> \ >>> ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]`` >>> @@ -4960,6 +4967,16 @@ SRST >>> Accept incoming migration as an output from specified external >>> command. >>> +``-incoming @MigrationChannel`` >>> + Accept incoming migration on the channel. See the QAPI documentation >>> + for the syntax of the @MigrationChannel data element. For example: >>> + :: >> >> I get what you're trying to express, but there's no precedence for >> referring to QAPI types like @TypeName in option documentation. But >> let's ignore this until after we nailed down the actual interface, on >> which I have questions below. > > Ack. > >>> + >>> + -incoming '{"channel-type": "main", >>> + "addr": { "transport": "socket", >>> + "type": "unix", >>> + "path": "my.sock" }}' >>> + >>> ``-incoming defer`` >>> Wait for the URI to be specified via migrate\_incoming. The monitor >>> can be used to change settings (such as migration parameters) prior >>> diff --git a/system/vl.c b/system/vl.c >>> index 4151a79..2c24c60 100644 >>> --- a/system/vl.c >>> +++ b/system/vl.c >>> @@ -123,6 +123,7 @@ >>> #include "qapi/qapi-visit-block-core.h" >>> #include "qapi/qapi-visit-compat.h" >>> #include "qapi/qapi-visit-machine.h" >>> +#include "qapi/qapi-visit-migration.h" >>> #include "qapi/qapi-visit-ui.h" >>> #include "qapi/qapi-commands-block-core.h" >>> #include "qapi/qapi-commands-migration.h" >>> @@ -159,6 +160,7 @@ typedef struct DeviceOption { >>> static const char *cpu_option; >>> static const char *mem_path; >>> static const char *incoming; >>> +static MigrationChannelList *incoming_channels; >>> static const char *loadvm; >>> static const char *accelerators; >>> static bool have_custom_ram_size; >>> @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) >>> QTAILQ_INSERT_TAIL(&object_opts, opt, next); >>> } >>> +static void incoming_option_parse(const char *str) >>> +{ >>> + MigrationChannel *channel; >>> + >>> + if (str[0] == '{') { >>> + QObject *obj = qobject_from_json(str, &error_fatal); >>> + Visitor *v = qobject_input_visitor_new(obj); >>> + >>> + qobject_unref(obj); >>> + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); >>> + visit_free(v); >>> + } else if (!strcmp(str, "defer")) { >>> + channel = NULL; >>> + } else { >>> + migrate_uri_parse(str, &channel, &error_fatal); >>> + } >>> + >>> + /* New incoming spec replaces the previous */ >>> + >>> + if (incoming_channels) { >>> + qapi_free_MigrationChannelList(incoming_channels); >>> + } >>> + if (channel) { >>> + incoming_channels = g_new0(MigrationChannelList, 1); >>> + incoming_channels->value = channel; >>> + } >>> + incoming = str; >>> +} >> >> @incoming is set to @optarg. >> >> @incoming_channels is set to a MigrationChannelList of exactly one >> element, parsed from @incoming. Except when @incoming is "defer", then >> @incoming_channels is set to null. >> >> @incoming is only ever used as a flag. Turn it into a bool? > > The remembered incoming specifier is also used in an error message in > qmp_x_exit_preconfig: > error_reportf_err(local_err, "-incoming %s: ", incoming); > >> Oh, wait... see my comment on the next hunk. >> >> Option -incoming resembles QMP command migrate-incoming. Differences: >> >> * migrate-incoming keeps legacy URI and modern argument separate: there >> are two named arguments, and exactly one of them must be passed. >> -incoming overloads them: if @optarg starts with '{', it's modern, >> else legacy URI. >> >> Because of that, -incoming *only* supports JSON syntax for modern, not >> dotted keys. Other JSON-capable arguments support both. > > Not sure I follow. > Could you give me a dotted key example for a JSON-capable argument? > Do we care about dotted key for incoming, given the user can specify > a simple legacy URI? A quick grep for the usual parser qobject_input_visitor_new() finds -audiodev, -blockdev, -compat, -display, and -netdev. Beware, the latter two come with backward compatibility gunk. There's also -device and -object, also with backward compatibility gunk. Simple example: JSON -compat '{"deprecated-input": "reject", "deprecated-output": "hide"} dotted keys -compat deprecated-input=reject,deprecated-output=hide Slightly more interesting: JSON -audiodev '{"id": "audiodev0", "driver": "wav", "in": {"voices": 4}}' dotted keys -audiodev id=audiodev0,driver=wav,in.voices=4 >> How can a management application detect that -incoming supports >> modern? > > How does mgmt detect when other arguments support JSON? Easy when an option supports it from the start: -audiodev, -blockdev, -compat. Awkward when we extend an existing option to support it: -display, -netdev, -device, -object. As far as I can tell at a glance, libvirt * Remains unaware of -display JSON arguments * Assumes -netdev accepts JSON when QMP netdev-add supports backend type "dgram", see commit 697e26fac66 (qemu: capabilities: Detect support for JSON args for -netdev) v8.10.0 * Assumes -device accepts JSON when QMP device_add has feature json-cli-hotplug, see commit 1a691fe1c84 (qemu: capabilities: Re-enable JSON syntax for -device) v8.1.0 * Assumes -object accepts JSON when object-add supports object type "secret", see commit f763b6e4390 (qemu: capabilities: Enable detection of QEMU_CAPS_OBJECT_QAPIFIED) v7.2.0 In theory, such indirect probing can fall apart when somebody backports JSON syntax *without* the thing libvirt probes for. They then get to adjust libvirt's detection logic, too. Hasn't been an issue in practice as far as I know. > The presence of cpr-transfer mode implies -incoming JSON support, though > that is indirect. Might be good enough. > We could add a feature to the migrate-incoming command, like json-cli > for device_add. Seems like overkill though. 'feature' is little used, > except for unstable and deprecated. 'feature' is best used sparingly. But when it's needed, using it is *fine*. >> Sure overloading -incoming this way is a good idea? >> >> * migrate-incoming takes a list of channels, currently restricted to a >> single channel. -incoming takes a channel. If we lift the >> restriction, -incoming syntax will become even messier: we'll have to >> additionally overload list of channel. >> >> Should -incoming take a list from the start, like migrate-incoming >> does? > > That was my first try. However, to support the equivalent of '-incoming deferred', > we need to add an 'defer' key to the channel, and when defer is true, the other > keys that are currently mandatory must be omitted. The tweaks to the implementation > and specification seemed not worth worth it. > > If we want -incoming to also support a channel list in the future, we can simply > check for an initial '[' token. Yes, but it'll then have to support single channels both as list of one channel object, and channel object, unlike migrate-incoming. Syntactical differences between CLI and QMP for things that are semantically identical add unnecessary complexity, don't you think? >>> + >>> static void object_option_parse(const char *str) >>> { >>> QemuOpts *opts; >>> @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) >>> if (incoming) { >>> Error *local_err = NULL; >>> if (strcmp(incoming, "defer") != 0) { >>> - qmp_migrate_incoming(incoming, false, NULL, true, true, >>> + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, >>> &local_err); >> >> You move the parsing of legacy URI from within qmp_migrate_incoming() >> into incoming_option_parse(). >> >> The alternative is not to parse it in incoming_option_parse(), but pass >> it to qmp_migrate_incoming() like this: >> >> qmp_migrate_incoming(incoming, !incoming, incoming_channels, >> true, true, &local_err); > > Sure, I can tweak that, but I need to define an additional incoming_uri variable: > qmp_migrate_incoming(incoming_uri, !!incoming_channels, incoming_channels, ... > > Only one of incoming_uri and incoming_channels can be non-NULL (checked in > qemu_start_incoming_migration). > > Would you prefer I continue down this path, or revert to the previous -cpr-uri > option? I made this change to make the incoming interface look more like the > V4 outgoing interface, in which the user adds a cpr channel to the migrate command > channels. I'm not sure. Peter, what do you think?
On Mon, Dec 09, 2024 at 01:12:25PM +0100, Markus Armbruster wrote: > >>> @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) > >>> if (incoming) { > >>> Error *local_err = NULL; > >>> if (strcmp(incoming, "defer") != 0) { > >>> - qmp_migrate_incoming(incoming, false, NULL, true, true, > >>> + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, > >>> &local_err); > >> > >> You move the parsing of legacy URI from within qmp_migrate_incoming() > >> into incoming_option_parse(). > >> > >> The alternative is not to parse it in incoming_option_parse(), but pass > >> it to qmp_migrate_incoming() like this: > >> > >> qmp_migrate_incoming(incoming, !incoming, incoming_channels, > >> true, true, &local_err); > > > > Sure, I can tweak that, but I need to define an additional incoming_uri variable: > > qmp_migrate_incoming(incoming_uri, !!incoming_channels, incoming_channels, ... > > > > Only one of incoming_uri and incoming_channels can be non-NULL (checked in > > qemu_start_incoming_migration). > > > > Would you prefer I continue down this path, or revert to the previous -cpr-uri > > option? I made this change to make the incoming interface look more like the > > V4 outgoing interface, in which the user adds a cpr channel to the migrate command > > channels. > > I'm not sure. Peter, what do you think? For this specific question, I prefer reusing -incoming rather than going back to -cpr-uri. We should have discussed about this in the previous spin of a follow up discussion, using -incoming for cpr channels seems to always be preferred. https://lore.kernel.org/qemu-devel/Zz4NqcTDK73MKOaa@redhat.com/ At that time, the concern from Dan was not reusing the JSON format or design the CLI's own format. That was always based on reusing -incoming. In this patch it's already reusing the JSON for the CPR port, which looks all good from that POV. OTOH, I don't yet have any preference on the impl of how QEMU should do the internal parsing of such JSON string / legacy URIs. Thanks,
Markus Armbruster <armbru@redhat.com> writes: > Steve Sistare <steven.sistare@oracle.com> writes: > >> Extend the -incoming option to allow an @MigrationChannel to be specified. >> This allows channels other than 'main' to be described on the command >> line, which will be needed for CPR. >> >> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> [...] >> diff --git a/system/vl.c b/system/vl.c >> index 4151a79..2c24c60 100644 >> --- a/system/vl.c >> +++ b/system/vl.c >> @@ -123,6 +123,7 @@ >> #include "qapi/qapi-visit-block-core.h" >> #include "qapi/qapi-visit-compat.h" >> #include "qapi/qapi-visit-machine.h" >> +#include "qapi/qapi-visit-migration.h" >> #include "qapi/qapi-visit-ui.h" >> #include "qapi/qapi-commands-block-core.h" >> #include "qapi/qapi-commands-migration.h" >> @@ -159,6 +160,7 @@ typedef struct DeviceOption { >> static const char *cpu_option; >> static const char *mem_path; >> static const char *incoming; >> +static MigrationChannelList *incoming_channels; >> static const char *loadvm; >> static const char *accelerators; >> static bool have_custom_ram_size; >> @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) >> QTAILQ_INSERT_TAIL(&object_opts, opt, next); >> } >> >> +static void incoming_option_parse(const char *str) >> +{ >> + MigrationChannel *channel; >> + >> + if (str[0] == '{') { >> + QObject *obj = qobject_from_json(str, &error_fatal); >> + Visitor *v = qobject_input_visitor_new(obj); >> + >> + qobject_unref(obj); >> + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); >> + visit_free(v); >> + } else if (!strcmp(str, "defer")) { >> + channel = NULL; >> + } else { >> + migrate_uri_parse(str, &channel, &error_fatal); >> + } >> + >> + /* New incoming spec replaces the previous */ >> + >> + if (incoming_channels) { >> + qapi_free_MigrationChannelList(incoming_channels); >> + } >> + if (channel) { >> + incoming_channels = g_new0(MigrationChannelList, 1); >> + incoming_channels->value = channel; >> + } >> + incoming = str; >> +} > > @incoming is set to @optarg. > > @incoming_channels is set to a MigrationChannelList of exactly one > element, parsed from @incoming. Except when @incoming is "defer", then > @incoming_channels is set to null. > > @incoming is only ever used as a flag. Turn it into a bool? > > Oh, wait... see my comment on the next hunk. > > Option -incoming resembles QMP command migrate-incoming. Differences: > > * migrate-incoming keeps legacy URI and modern argument separate: there > are two named arguments, and exactly one of them must be passed. > -incoming overloads them: if @optarg starts with '{', it's modern, > else legacy URI. > > Because of that, -incoming *only* supports JSON syntax for modern, not > dotted keys. Other JSON-capable arguments support both. Here's a way to avoid restricting modern to JSON. Legacy URI is either "defer" or starts with "KEYWORD:", where KEYWORD is one of a few well-known words. As long as we don't support an implied key, a non-empty dotted keys argument starts with "KEY=", where KEY cannot contain ':'. This lets us distinguish legacy URI from dotted keys. Say, if the argument is "defer" or starts with letters followed by ':', assume URI. > How can a management application detect that -incoming supports > modern? > > Sure overloading -incoming this way is a good idea? It'll be a pain to document. > * migrate-incoming takes a list of channels, currently restricted to a > single channel. -incoming takes a channel. If we lift the > restriction, -incoming syntax will become even messier: we'll have to > additionally overload list of channel. > > Should -incoming take a list from the start, like migrate-incoming > does? > >> + >> static void object_option_parse(const char *str) >> { >> QemuOpts *opts; >> @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) >> if (incoming) { >> Error *local_err = NULL; >> if (strcmp(incoming, "defer") != 0) { >> - qmp_migrate_incoming(incoming, false, NULL, true, true, >> + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, >> &local_err); > > You move the parsing of legacy URI from within qmp_migrate_incoming() > into incoming_option_parse(). > > The alternative is not to parse it in incoming_option_parse(), but pass > it to qmp_migrate_incoming() like this: > > qmp_migrate_incoming(incoming, !incoming, incoming_channels, > true, true, &local_err); > >> if (local_err) { >> error_reportf_err(local_err, "-incoming %s: ", incoming); >> @@ -3477,7 +3508,7 @@ void qemu_init(int argc, char **argv) >> if (!incoming) { >> runstate_set(RUN_STATE_INMIGRATE); >> } >> - incoming = optarg; >> + incoming_option_parse(optarg); >> break; >> case QEMU_OPTION_only_migratable: >> only_migratable = 1;
Markus Armbruster <armbru@redhat.com> writes: > Steven Sistare <steven.sistare@oracle.com> writes: > >> On 12/5/2024 10:23 AM, Markus Armbruster wrote: >>> Steve Sistare <steven.sistare@oracle.com> writes: >>> >>>> Extend the -incoming option to allow an @MigrationChannel to be specified. >>>> This allows channels other than 'main' to be described on the command >>>> line, which will be needed for CPR. >>>> >>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> [...] >>> Option -incoming resembles QMP command migrate-incoming. Differences: >>> >>> * migrate-incoming keeps legacy URI and modern argument separate: there >>> are two named arguments, and exactly one of them must be passed. >>> -incoming overloads them: if @optarg starts with '{', it's modern, >>> else legacy URI. >>> >>> Because of that, -incoming *only* supports JSON syntax for modern, not >>> dotted keys. Other JSON-capable arguments support both. >> >> Not sure I follow. >> Could you give me a dotted key example for a JSON-capable argument? >> Do we care about dotted key for incoming, given the user can specify >> a simple legacy URI? > > A quick grep for the usual parser qobject_input_visitor_new() finds Correction: qobject_input_visitor_new_str(). > -audiodev, -blockdev, -compat, -display, and -netdev. Beware, the > latter two come with backward compatibility gunk. There's also -device > and -object, also with backward compatibility gunk. [...]
On 12/9/2024 7:12 AM, Markus Armbruster wrote: > Steven Sistare <steven.sistare@oracle.com> writes: > >> On 12/5/2024 10:23 AM, Markus Armbruster wrote: >>> Steve Sistare <steven.sistare@oracle.com> writes: >>> >>>> Extend the -incoming option to allow an @MigrationChannel to be specified. >>>> This allows channels other than 'main' to be described on the command >>>> line, which will be needed for CPR. >>>> >>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com> >>> [...] >>> >>>> diff --git a/qemu-options.hx b/qemu-options.hx >>>> index 02b9118..fab50ce 100644 >>>> --- a/qemu-options.hx >>>> +++ b/qemu-options.hx >>>> @@ -4937,10 +4937,17 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ >>>> "-incoming exec:cmdline\n" \ >>>> " accept incoming migration on given file descriptor\n" \ >>>> " or from given external command\n" \ >>>> + "-incoming @MigrationChannel\n" \ >>>> + " accept incoming migration on the channel\n" \ >>>> "-incoming defer\n" \ >>>> " wait for the URI to be specified via migrate_incoming\n", >>>> QEMU_ARCH_ALL) >>>> SRST >>>> +The -incoming option specifies the migration channel for an incoming >>>> +migration. It may be used multiple times to specify multiple >>>> +migration channel types. >>> >>> Really? If I understand the code below correctly, the last -incoming >>> wins, and any previous ones are silently ignored. >> >> See patch "cpr-channel", where the cpr channel is saved separately. >> Last wins, per channel type. >> I did this to preserve the current behavior of -incoming in which last wins. > > Documentation needs to be clarified then. Maybe. Depends whether we want/need to take a stand on whether the current behavior is an accident of the implementation, or part of the specification. The current behavior is not documented. >> qemu_start_incoming_migration will need modification if more types are added. >> >>>> The channel type is specified in @MigrationChannel, >>>> +and is 'main' for all other forms of -incoming. >>>> + >>>> ``-incoming tcp:[host]:port[,to=maxport][,ipv4=on|off][,ipv6=on|off]`` >>>> \ >>>> ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]`` >>>> @@ -4960,6 +4967,16 @@ SRST >>>> Accept incoming migration as an output from specified external >>>> command. >>>> +``-incoming @MigrationChannel`` >>>> + Accept incoming migration on the channel. See the QAPI documentation >>>> + for the syntax of the @MigrationChannel data element. For example: >>>> + :: >>> >>> I get what you're trying to express, but there's no precedence for >>> referring to QAPI types like @TypeName in option documentation. But >>> let's ignore this until after we nailed down the actual interface, on >>> which I have questions below. >> >> Ack. >> >>>> + >>>> + -incoming '{"channel-type": "main", >>>> + "addr": { "transport": "socket", >>>> + "type": "unix", >>>> + "path": "my.sock" }}' >>>> + >>>> ``-incoming defer`` >>>> Wait for the URI to be specified via migrate\_incoming. The monitor >>>> can be used to change settings (such as migration parameters) prior >>>> diff --git a/system/vl.c b/system/vl.c >>>> index 4151a79..2c24c60 100644 >>>> --- a/system/vl.c >>>> +++ b/system/vl.c >>>> @@ -123,6 +123,7 @@ >>>> #include "qapi/qapi-visit-block-core.h" >>>> #include "qapi/qapi-visit-compat.h" >>>> #include "qapi/qapi-visit-machine.h" >>>> +#include "qapi/qapi-visit-migration.h" >>>> #include "qapi/qapi-visit-ui.h" >>>> #include "qapi/qapi-commands-block-core.h" >>>> #include "qapi/qapi-commands-migration.h" >>>> @@ -159,6 +160,7 @@ typedef struct DeviceOption { >>>> static const char *cpu_option; >>>> static const char *mem_path; >>>> static const char *incoming; >>>> +static MigrationChannelList *incoming_channels; >>>> static const char *loadvm; >>>> static const char *accelerators; >>>> static bool have_custom_ram_size; >>>> @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) >>>> QTAILQ_INSERT_TAIL(&object_opts, opt, next); >>>> } >>>> +static void incoming_option_parse(const char *str) >>>> +{ >>>> + MigrationChannel *channel; >>>> + >>>> + if (str[0] == '{') { >>>> + QObject *obj = qobject_from_json(str, &error_fatal); >>>> + Visitor *v = qobject_input_visitor_new(obj); >>>> + >>>> + qobject_unref(obj); >>>> + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); >>>> + visit_free(v); >>>> + } else if (!strcmp(str, "defer")) { >>>> + channel = NULL; >>>> + } else { >>>> + migrate_uri_parse(str, &channel, &error_fatal); >>>> + } >>>> + >>>> + /* New incoming spec replaces the previous */ >>>> + >>>> + if (incoming_channels) { >>>> + qapi_free_MigrationChannelList(incoming_channels); >>>> + } >>>> + if (channel) { >>>> + incoming_channels = g_new0(MigrationChannelList, 1); >>>> + incoming_channels->value = channel; >>>> + } >>>> + incoming = str; >>>> +} >>> >>> @incoming is set to @optarg. >>> >>> @incoming_channels is set to a MigrationChannelList of exactly one >>> element, parsed from @incoming. Except when @incoming is "defer", then >>> @incoming_channels is set to null. >>> >>> @incoming is only ever used as a flag. Turn it into a bool? >> >> The remembered incoming specifier is also used in an error message in >> qmp_x_exit_preconfig: >> error_reportf_err(local_err, "-incoming %s: ", incoming); >> >>> Oh, wait... see my comment on the next hunk. >>> >>> Option -incoming resembles QMP command migrate-incoming. Differences: >>> >>> * migrate-incoming keeps legacy URI and modern argument separate: there >>> are two named arguments, and exactly one of them must be passed. >>> -incoming overloads them: if @optarg starts with '{', it's modern, >>> else legacy URI. >>> >>> Because of that, -incoming *only* supports JSON syntax for modern, not >>> dotted keys. Other JSON-capable arguments support both. >> >> Not sure I follow. >> Could you give me a dotted key example for a JSON-capable argument? >> Do we care about dotted key for incoming, given the user can specify >> a simple legacy URI? > > A quick grep for the usual parser qobject_input_visitor_new() finds > -audiodev, -blockdev, -compat, -display, and -netdev. Beware, the > latter two come with backward compatibility gunk. There's also -device > and -object, also with backward compatibility gunk. > > Simple example: > > JSON -compat '{"deprecated-input": "reject", "deprecated-output": "hide"} > dotted keys -compat deprecated-input=reject,deprecated-output=hide > > Slightly more interesting: > > JSON -audiodev '{"id": "audiodev0", "driver": "wav", "in": {"voices": 4}}' > dotted keys -audiodev id=audiodev0,driver=wav,in.voices=4 Thank you (and for the correction to qobject_input_visitor_new_str). I did not grok that this visitor handles both json and dotted keys, as so far I had only seen examples with keys but not dotted keys. I can easily support those as well as the legacy uri, by returning a new parameter from migrate_uri_parse indicating that no uri is recognized (as opposed to recognized, but with some other error): if (!strcmp(str, "defer")) { channel = NULL; } else if (!migrate_uri_parse(str, &channel, &no_uri, &err)) { if (no_uri) { qobject_input_visitor_new_str() visit_type_MigrationChannel() } else { report error } } I implemented this and tested for all formats. >>> How can a management application detect that -incoming supports >>> modern? >> >> How does mgmt detect when other arguments support JSON? > > Easy when an option supports it from the start: -audiodev, -blockdev, > -compat. Awkward when we extend an existing option to support it: > -display, -netdev, -device, -object. > > As far as I can tell at a glance, libvirt > > * Remains unaware of -display JSON arguments > > * Assumes -netdev accepts JSON when QMP netdev-add supports backend type > "dgram", see commit 697e26fac66 (qemu: capabilities: Detect support > for JSON args for -netdev) v8.10.0 > > * Assumes -device accepts JSON when QMP device_add has feature > json-cli-hotplug, see commit 1a691fe1c84 (qemu: capabilities: > Re-enable JSON syntax for -device) v8.1.0 > > * Assumes -object accepts JSON when object-add supports object type > "secret", see commit f763b6e4390 (qemu: capabilities: Enable detection > of QEMU_CAPS_OBJECT_QAPIFIED) v7.2.0 > > In theory, such indirect probing can fall apart when somebody backports > JSON syntax *without* the thing libvirt probes for. They then get to > adjust libvirt's detection logic, too. Hasn't been an issue in practice > as far as I know. > >> The presence of cpr-transfer mode implies -incoming JSON support, though >> that is indirect. > > Might be good enough. I'll keep it simple and go with that unless someone objects. >> We could add a feature to the migrate-incoming command, like json-cli >> for device_add. Seems like overkill though. 'feature' is little used, >> except for unstable and deprecated. > > 'feature' is best used sparingly. But when it's needed, using it is > *fine*. > >>> Sure overloading -incoming this way is a good idea? >>> >>> * migrate-incoming takes a list of channels, currently restricted to a >>> single channel. -incoming takes a channel. If we lift the >>> restriction, -incoming syntax will become even messier: we'll have to >>> additionally overload list of channel. >>> >>> Should -incoming take a list from the start, like migrate-incoming >>> does? >> >> That was my first try. However, to support the equivalent of '-incoming deferred', >> we need to add an 'defer' key to the channel, and when defer is true, the other >> keys that are currently mandatory must be omitted. The tweaks to the implementation >> and specification seemed not worth worth it. >> >> If we want -incoming to also support a channel list in the future, we can simply >> check for an initial '[' token. > > Yes, but it'll then have to support single channels both as list of one > channel object, and channel object, unlike migrate-incoming. > > Syntactical differences between CLI and QMP for things that are > semantically identical add unnecessary complexity, don't you think? Agreed on both. I am just pointing out that if we implement '-incoming uri|channel' now, but in the future want to add '-incoming channel-list', then we can parse it unambiguously and be backwards compatible. I don't foresee ever needing the latter, as multiple '-incoming uri|channel' arguments are logically equivalent to a list. To reiterate, I prefer '-incoming uri|channel' because it avoids the need to add a 'defer' property to the channel specification and implementation. >>>> + >>>> static void object_option_parse(const char *str) >>>> { >>>> QemuOpts *opts; >>>> @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) >>>> if (incoming) { >>>> Error *local_err = NULL; >>>> if (strcmp(incoming, "defer") != 0) { >>>> - qmp_migrate_incoming(incoming, false, NULL, true, true, >>>> + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, >>>> &local_err); >>> >>> You move the parsing of legacy URI from within qmp_migrate_incoming() >>> into incoming_option_parse(). >>> >>> The alternative is not to parse it in incoming_option_parse(), but pass >>> it to qmp_migrate_incoming() like this: >>> >>> qmp_migrate_incoming(incoming, !incoming, incoming_channels, >>> true, true, &local_err); >> >> Sure, I can tweak that, but I need to define an additional incoming_uri variable: >> qmp_migrate_incoming(incoming_uri, !!incoming_channels, incoming_channels, ... >> >> Only one of incoming_uri and incoming_channels can be non-NULL (checked in >> qemu_start_incoming_migration). >> >> Would you prefer I continue down this path, or revert to the previous -cpr-uri >> option? I made this change to make the incoming interface look more like the >> V4 outgoing interface, in which the user adds a cpr channel to the migrate command >> channels. > > I'm not sure. Peter, what do you think? Peter likes -incoming, so I will continue with it. - Steve
diff --git a/include/migration/misc.h b/include/migration/misc.h index 804eb23..259d4aa 100644 --- a/include/migration/misc.h +++ b/include/migration/misc.h @@ -106,4 +106,6 @@ bool migration_incoming_postcopy_advised(void); /* True if background snapshot is active */ bool migration_in_bg_snapshot(void); +bool migrate_uri_parse(const char *uri, MigrationChannel **channel, + Error **errp); #endif diff --git a/migration/migration.c b/migration/migration.c index 83dabc7..a5cf148 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2046,6 +2046,7 @@ void qmp_migrate(const char *uri, bool has_channels, MigrationState *s = migrate_get_current(); g_autoptr(MigrationChannel) channel = NULL; MigrationAddress *addr = NULL; + MigrationChannel *channelv[MIGRATION_CHANNEL_TYPE__MAX] = { NULL }; /* * Having preliminary checks for uri and channel @@ -2056,12 +2057,21 @@ void qmp_migrate(const char *uri, bool has_channels, } if (channels) { - /* To verify that Migrate channel list has only item */ - if (channels->next) { - error_setg(errp, "Channel list has more than one entries"); + for ( ; channels; channels = channels->next) { + MigrationChannelType type = channels->value->channel_type; + + if (channelv[type]) { + error_setg(errp, "Channel list has more than one %s entry", + MigrationChannelType_str(type)); + return; + } + channelv[type] = channels->value; + } + addr = channelv[MIGRATION_CHANNEL_TYPE_MAIN]->addr; + if (!addr) { + error_setg(errp, "Channel list has no main entry"); return; } - addr = channels->value->addr; } if (uri) { diff --git a/migration/migration.h b/migration/migration.h index 0956e92..5cd0f29 100644 --- a/migration/migration.h +++ b/migration/migration.h @@ -522,8 +522,6 @@ bool check_dirty_bitmap_mig_alias_map(const BitmapMigrationNodeAliasList *bbm, Error **errp); void migrate_add_address(SocketAddress *address); -bool migrate_uri_parse(const char *uri, MigrationChannel **channel, - Error **errp); int foreach_not_ignored_block(RAMBlockIterFunc func, void *opaque); #define qemu_ram_foreach_block \ diff --git a/qemu-options.hx b/qemu-options.hx index 02b9118..fab50ce 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4937,10 +4937,17 @@ DEF("incoming", HAS_ARG, QEMU_OPTION_incoming, \ "-incoming exec:cmdline\n" \ " accept incoming migration on given file descriptor\n" \ " or from given external command\n" \ + "-incoming @MigrationChannel\n" \ + " accept incoming migration on the channel\n" \ "-incoming defer\n" \ " wait for the URI to be specified via migrate_incoming\n", QEMU_ARCH_ALL) SRST +The -incoming option specifies the migration channel for an incoming +migration. It may be used multiple times to specify multiple +migration channel types. The channel type is specified in @MigrationChannel, +and is 'main' for all other forms of -incoming. + ``-incoming tcp:[host]:port[,to=maxport][,ipv4=on|off][,ipv6=on|off]`` \ ``-incoming rdma:host:port[,ipv4=on|off][,ipv6=on|off]`` @@ -4960,6 +4967,16 @@ SRST Accept incoming migration as an output from specified external command. +``-incoming @MigrationChannel`` + Accept incoming migration on the channel. See the QAPI documentation + for the syntax of the @MigrationChannel data element. For example: + :: + + -incoming '{"channel-type": "main", + "addr": { "transport": "socket", + "type": "unix", + "path": "my.sock" }}' + ``-incoming defer`` Wait for the URI to be specified via migrate\_incoming. The monitor can be used to change settings (such as migration parameters) prior diff --git a/system/vl.c b/system/vl.c index 4151a79..2c24c60 100644 --- a/system/vl.c +++ b/system/vl.c @@ -123,6 +123,7 @@ #include "qapi/qapi-visit-block-core.h" #include "qapi/qapi-visit-compat.h" #include "qapi/qapi-visit-machine.h" +#include "qapi/qapi-visit-migration.h" #include "qapi/qapi-visit-ui.h" #include "qapi/qapi-commands-block-core.h" #include "qapi/qapi-commands-migration.h" @@ -159,6 +160,7 @@ typedef struct DeviceOption { static const char *cpu_option; static const char *mem_path; static const char *incoming; +static MigrationChannelList *incoming_channels; static const char *loadvm; static const char *accelerators; static bool have_custom_ram_size; @@ -1821,6 +1823,35 @@ static void object_option_add_visitor(Visitor *v) QTAILQ_INSERT_TAIL(&object_opts, opt, next); } +static void incoming_option_parse(const char *str) +{ + MigrationChannel *channel; + + if (str[0] == '{') { + QObject *obj = qobject_from_json(str, &error_fatal); + Visitor *v = qobject_input_visitor_new(obj); + + qobject_unref(obj); + visit_type_MigrationChannel(v, "channel", &channel, &error_fatal); + visit_free(v); + } else if (!strcmp(str, "defer")) { + channel = NULL; + } else { + migrate_uri_parse(str, &channel, &error_fatal); + } + + /* New incoming spec replaces the previous */ + + if (incoming_channels) { + qapi_free_MigrationChannelList(incoming_channels); + } + if (channel) { + incoming_channels = g_new0(MigrationChannelList, 1); + incoming_channels->value = channel; + } + incoming = str; +} + static void object_option_parse(const char *str) { QemuOpts *opts; @@ -2730,7 +2761,7 @@ void qmp_x_exit_preconfig(Error **errp) if (incoming) { Error *local_err = NULL; if (strcmp(incoming, "defer") != 0) { - qmp_migrate_incoming(incoming, false, NULL, true, true, + qmp_migrate_incoming(NULL, true, incoming_channels, true, true, &local_err); if (local_err) { error_reportf_err(local_err, "-incoming %s: ", incoming); @@ -3477,7 +3508,7 @@ void qemu_init(int argc, char **argv) if (!incoming) { runstate_set(RUN_STATE_INMIGRATE); } - incoming = optarg; + incoming_option_parse(optarg); break; case QEMU_OPTION_only_migratable: only_migratable = 1;
Extend the -incoming option to allow an @MigrationChannel to be specified. This allows channels other than 'main' to be described on the command line, which will be needed for CPR. Signed-off-by: Steve Sistare <steven.sistare@oracle.com> --- include/migration/misc.h | 2 ++ migration/migration.c | 18 ++++++++++++++---- migration/migration.h | 2 -- qemu-options.hx | 17 +++++++++++++++++ system/vl.c | 35 +++++++++++++++++++++++++++++++++-- 5 files changed, 66 insertions(+), 8 deletions(-)