Markus Armbruster <arm...@redhat.com> writes:
> Steve Sistare <steven.sist...@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.sist...@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;
