| Message ID | 20160215101506.32327.51662.stgit@bahia.huguette.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15/02/2016 11:15, Greg Kurz wrote: > Since QEMU 2.4, the migration stream begins with a configuration section. > It is known to break migration of pseries machine from older QEMU. It is > possible to fix this in the pseries compat code but it will then break > migration of old pseries from latest QEMU. > > As an alternative, this patch introduces a new machine property which > allows to ignore the abscence of configuration section during incoming > migration. It boils to adding: > > -machine config-section=off > > Using this property only makes sense when migrating from an older > QEMU. It has no effect on outgoing migration. I think this is not true: migrating a pseries-2.3 machine from a qemu-2.4 to a qemu-2.3 must not send the configuration section because the qemu-2.3 will not be able to decode it. 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: Invalid argument [This is the result without your patch] Laurent > > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > --- > hw/core/machine.c | 21 +++++++++++++++++++++ > include/hw/boards.h | 1 + > migration/savevm.c | 21 +++++++++++++++------ > qemu-options.hx | 3 ++- > 4 files changed, 39 insertions(+), 7 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 6d1a0d8eebc4..4a7322988fb5 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > return ms->suppress_vmdesc; > } > > +static void machine_set_config_section(Object *obj, bool value, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->config_section = value; > +} > + > +static bool machine_get_config_section(Object *obj, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + return ms->config_section; > +} > + > static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > { > error_report("Option '-device %s' cannot be handled by this machine", > @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > ms->kvm_shadow_mem = -1; > ms->dump_guest_core = true; > ms->mem_merge = true; > + ms->config_section = true; > > object_property_add_str(obj, "accel", > machine_get_accel, machine_set_accel, NULL); > @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > object_property_set_description(obj, "suppress-vmdesc", > "Set on to disable self-describing migration", > NULL); > + object_property_add_bool(obj, "config-section", > + machine_get_config_section, > + machine_set_config_section, NULL); > + object_property_set_description(obj, "config-section", > + "Set on/off to accept migration with/without configuration section", > + NULL); > > /* Register notifier when init is done for sysbus sanity checks */ > ms->sysbus_notifier.notify = machine_init_notify; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 0f30959e2e3b..853bb5905ec1 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -128,6 +128,7 @@ struct MachineState { > char *firmware; > bool iommu; > bool suppress_vmdesc; > + bool config_section; > > ram_addr_t ram_size; > ram_addr_t maxram_size; > diff --git a/migration/savevm.c b/migration/savevm.c > index 94f2894243ce..3795489aeaec 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > return 0; > } > > +static bool must_receive_configuration(void) > +{ > + MachineState *machine = MACHINE(qdev_get_machine()); > + return machine->config_section; > +} > + > int qemu_loadvm_state(QEMUFile *f) > { > MigrationIncomingState *mis = migration_incoming_get_current(); > @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > } > > if (!savevm_state.skip_configuration) { > - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > + qemu_file_skip(f, 1); > + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > + 0); > + > + if (ret) { > + return ret; > + } > + } else if (must_receive_configuration()) { > error_report("Configuration section missing"); > return -EINVAL; > } > - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > - > - if (ret) { > - return ret; > - } > } > > ret = qemu_loadvm_state_main(f, mis); > diff --git a/qemu-options.hx b/qemu-options.hx > index 2f0465eeb1d1..10cd64dc266b 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > - " nvdimm=on|off controls NVDIMM support (default=off)\n", > + " nvdimm=on|off controls NVDIMM support (default=off)\n" > + " config-section=on|off migration requires configuration section (default=on)\n", > QEMU_ARCH_ALL) > STEXI > @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > >
On Mon, 15 Feb 2016 12:23:39 +0100 Laurent Vivier <lvivier@redhat.com> wrote: > On 15/02/2016 11:15, Greg Kurz wrote: > > Since QEMU 2.4, the migration stream begins with a configuration section. > > It is known to break migration of pseries machine from older QEMU. It is > > possible to fix this in the pseries compat code but it will then break > > migration of old pseries from latest QEMU. > > > > As an alternative, this patch introduces a new machine property which > > allows to ignore the abscence of configuration section during incoming > > migration. It boils to adding: > > > > -machine config-section=off > > > > Using this property only makes sense when migrating from an older > > QEMU. It has no effect on outgoing migration. > > I think this is not true: migrating a pseries-2.3 machine from a > qemu-2.4 to a qemu-2.3 must not send the configuration section because > the qemu-2.3 will not be able to decode it. > I did not mind to also fix backward compatibility... is it mandatory ? > 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 > 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: > Invalid argument > > [This is the result without your patch] > > Laurent > > > > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > --- > > hw/core/machine.c | 21 +++++++++++++++++++++ > > include/hw/boards.h | 1 + > > migration/savevm.c | 21 +++++++++++++++------ > > qemu-options.hx | 3 ++- > > 4 files changed, 39 insertions(+), 7 deletions(-) > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index 6d1a0d8eebc4..4a7322988fb5 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > > return ms->suppress_vmdesc; > > } > > > > +static void machine_set_config_section(Object *obj, bool value, Error **errp) > > +{ > > + MachineState *ms = MACHINE(obj); > > + > > + ms->config_section = value; > > +} > > + > > +static bool machine_get_config_section(Object *obj, Error **errp) > > +{ > > + MachineState *ms = MACHINE(obj); > > + > > + return ms->config_section; > > +} > > + > > static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > > { > > error_report("Option '-device %s' cannot be handled by this machine", > > @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > > ms->kvm_shadow_mem = -1; > > ms->dump_guest_core = true; > > ms->mem_merge = true; > > + ms->config_section = true; > > > > object_property_add_str(obj, "accel", > > machine_get_accel, machine_set_accel, NULL); > > @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > > object_property_set_description(obj, "suppress-vmdesc", > > "Set on to disable self-describing migration", > > NULL); > > + object_property_add_bool(obj, "config-section", > > + machine_get_config_section, > > + machine_set_config_section, NULL); > > + object_property_set_description(obj, "config-section", > > + "Set on/off to accept migration with/without configuration section", > > + NULL); > > > > /* Register notifier when init is done for sysbus sanity checks */ > > ms->sysbus_notifier.notify = machine_init_notify; > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 0f30959e2e3b..853bb5905ec1 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -128,6 +128,7 @@ struct MachineState { > > char *firmware; > > bool iommu; > > bool suppress_vmdesc; > > + bool config_section; > > > > ram_addr_t ram_size; > > ram_addr_t maxram_size; > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 94f2894243ce..3795489aeaec 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > > return 0; > > } > > > > +static bool must_receive_configuration(void) > > +{ > > + MachineState *machine = MACHINE(qdev_get_machine()); > > + return machine->config_section; > > +} > > + > > int qemu_loadvm_state(QEMUFile *f) > > { > > MigrationIncomingState *mis = migration_incoming_get_current(); > > @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > > } > > > > if (!savevm_state.skip_configuration) { > > - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > > + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > > + qemu_file_skip(f, 1); > > + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > > + 0); > > + > > + if (ret) { > > + return ret; > > + } > > + } else if (must_receive_configuration()) { > > error_report("Configuration section missing"); > > return -EINVAL; > > } > > - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > > - > > - if (ret) { > > - return ret; > > - } > > } > > > > ret = qemu_loadvm_state_main(f, mis); > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 2f0465eeb1d1..10cd64dc266b 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > > " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > > " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > > " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > > - " nvdimm=on|off controls NVDIMM support (default=off)\n", > > + " nvdimm=on|off controls NVDIMM support (default=off)\n" > > + " config-section=on|off migration requires configuration section (default=on)\n", > > QEMU_ARCH_ALL) > > STEXI > > @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > > > > >
On 15/02/2016 13:58, Greg Kurz wrote: > On Mon, 15 Feb 2016 12:23:39 +0100 > Laurent Vivier <lvivier@redhat.com> wrote: > >> On 15/02/2016 11:15, Greg Kurz wrote: >>> Since QEMU 2.4, the migration stream begins with a configuration section. >>> It is known to break migration of pseries machine from older QEMU. It is >>> possible to fix this in the pseries compat code but it will then break >>> migration of old pseries from latest QEMU. >>> >>> As an alternative, this patch introduces a new machine property which >>> allows to ignore the abscence of configuration section during incoming >>> migration. It boils to adding: >>> >>> -machine config-section=off >>> >>> Using this property only makes sense when migrating from an older >>> QEMU. It has no effect on outgoing migration. >> >> I think this is not true: migrating a pseries-2.3 machine from a >> qemu-2.4 to a qemu-2.3 must not send the configuration section because >> the qemu-2.3 will not be able to decode it. >> > > I did not mind to also fix backward compatibility... is it mandatory ? I don't know, but as a user I would like to be able to do that (and it is possible in the other cases). And I think the fix could be smaller: something like just setting "savevm_state.skip_configuration" to the value of "config-section". [I don't know if it is as simple as it looks...] Laurent > >> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 >> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: >> Invalid argument >> >> [This is the result without your patch] >> >> Laurent >>> >>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> >>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>> --- >>> hw/core/machine.c | 21 +++++++++++++++++++++ >>> include/hw/boards.h | 1 + >>> migration/savevm.c | 21 +++++++++++++++------ >>> qemu-options.hx | 3 ++- >>> 4 files changed, 39 insertions(+), 7 deletions(-) >>> >>> diff --git a/hw/core/machine.c b/hw/core/machine.c >>> index 6d1a0d8eebc4..4a7322988fb5 100644 >>> --- a/hw/core/machine.c >>> +++ b/hw/core/machine.c >>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) >>> return ms->suppress_vmdesc; >>> } >>> >>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) >>> +{ >>> + MachineState *ms = MACHINE(obj); >>> + >>> + ms->config_section = value; >>> +} >>> + >>> +static bool machine_get_config_section(Object *obj, Error **errp) >>> +{ >>> + MachineState *ms = MACHINE(obj); >>> + >>> + return ms->config_section; >>> +} >>> + >>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) >>> { >>> error_report("Option '-device %s' cannot be handled by this machine", >>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) >>> ms->kvm_shadow_mem = -1; >>> ms->dump_guest_core = true; >>> ms->mem_merge = true; >>> + ms->config_section = true; >>> >>> object_property_add_str(obj, "accel", >>> machine_get_accel, machine_set_accel, NULL); >>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) >>> object_property_set_description(obj, "suppress-vmdesc", >>> "Set on to disable self-describing migration", >>> NULL); >>> + object_property_add_bool(obj, "config-section", >>> + machine_get_config_section, >>> + machine_set_config_section, NULL); >>> + object_property_set_description(obj, "config-section", >>> + "Set on/off to accept migration with/without configuration section", >>> + NULL); >>> >>> /* Register notifier when init is done for sysbus sanity checks */ >>> ms->sysbus_notifier.notify = machine_init_notify; >>> diff --git a/include/hw/boards.h b/include/hw/boards.h >>> index 0f30959e2e3b..853bb5905ec1 100644 >>> --- a/include/hw/boards.h >>> +++ b/include/hw/boards.h >>> @@ -128,6 +128,7 @@ struct MachineState { >>> char *firmware; >>> bool iommu; >>> bool suppress_vmdesc; >>> + bool config_section; >>> >>> ram_addr_t ram_size; >>> ram_addr_t maxram_size; >>> diff --git a/migration/savevm.c b/migration/savevm.c >>> index 94f2894243ce..3795489aeaec 100644 >>> --- a/migration/savevm.c >>> +++ b/migration/savevm.c >>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) >>> return 0; >>> } >>> >>> +static bool must_receive_configuration(void) >>> +{ >>> + MachineState *machine = MACHINE(qdev_get_machine()); >>> + return machine->config_section; >>> +} >>> + >>> int qemu_loadvm_state(QEMUFile *f) >>> { >>> MigrationIncomingState *mis = migration_incoming_get_current(); >>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) >>> } >>> >>> if (!savevm_state.skip_configuration) { >>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { >>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { >>> + qemu_file_skip(f, 1); >>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, >>> + 0); >>> + >>> + if (ret) { >>> + return ret; >>> + } >>> + } else if (must_receive_configuration()) { >>> error_report("Configuration section missing"); >>> return -EINVAL; >>> } >>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); >>> - >>> - if (ret) { >>> - return ret; >>> - } >>> } >>> >>> ret = qemu_loadvm_state_main(f, mis); >>> diff --git a/qemu-options.hx b/qemu-options.hx >>> index 2f0465eeb1d1..10cd64dc266b 100644 >>> --- a/qemu-options.hx >>> +++ b/qemu-options.hx >>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ >>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" >>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" >>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" >>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", >>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" >>> + " config-section=on|off migration requires configuration section (default=on)\n", >>> QEMU_ARCH_ALL) >>> STEXI >>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] >>> >>> >> >
On Mon, 15 Feb 2016 15:49:17 +0100 Laurent Vivier <lvivier@redhat.com> wrote: > On 15/02/2016 13:58, Greg Kurz wrote: > > On Mon, 15 Feb 2016 12:23:39 +0100 > > Laurent Vivier <lvivier@redhat.com> wrote: > > > >> On 15/02/2016 11:15, Greg Kurz wrote: > >>> Since QEMU 2.4, the migration stream begins with a configuration section. > >>> It is known to break migration of pseries machine from older QEMU. It is > >>> possible to fix this in the pseries compat code but it will then break > >>> migration of old pseries from latest QEMU. > >>> > >>> As an alternative, this patch introduces a new machine property which > >>> allows to ignore the abscence of configuration section during incoming > >>> migration. It boils to adding: > >>> > >>> -machine config-section=off > >>> > >>> Using this property only makes sense when migrating from an older > >>> QEMU. It has no effect on outgoing migration. > >> > >> I think this is not true: migrating a pseries-2.3 machine from a > >> qemu-2.4 to a qemu-2.3 must not send the configuration section because > >> the qemu-2.3 will not be able to decode it. > >> > > > > I did not mind to also fix backward compatibility... is it mandatory ? > > I don't know, but as a user I would like to be able to do that (and it > is possible in the other cases). > I fully agree backward migration is cool and we should not break it if possible. The situation is bit different here: QEMU 2.4 broke migration both ways for pseries-2.3 => all QEMU versions that were released since then (2.4/2.4.1/2.5) are ultimately incompatible with QEMU 2.3 and this cannot be fixed. What we may try to achieve is that QEMU 2.6 can accept incoming migration of pseries-2.3 from QEMU 2.3 (without configuration section) and from QEMU 2.4/2.5 (with configuration section). This is what this series does, without the need for the user to actually pass config-section=off thanks to patch 2. > And I think the fix could be smaller: something like just setting > "savevm_state.skip_configuration" to the value of "config-section". > [I don't know if it is as simple as it looks...] > Not so simple is the word indeed... if we do this, a pseries-2.3 machine started with config-section=off can only be migrated back to QEMU 2.3 since QEMU 2.4/2.4.1/2.5 want the config section... is this what you want ? If yes, then an even simpler fix is: https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg01738.html We can either fix forward migration for all QEMU versions with this series, or fix backward migration to QEMU-2.3. Not both. What is the most important ? > Laurent > > > >> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 > >> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: > >> Invalid argument > >> > >> [This is the result without your patch] > >> > >> Laurent > >>> > >>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > >>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>> --- > >>> hw/core/machine.c | 21 +++++++++++++++++++++ > >>> include/hw/boards.h | 1 + > >>> migration/savevm.c | 21 +++++++++++++++------ > >>> qemu-options.hx | 3 ++- > >>> 4 files changed, 39 insertions(+), 7 deletions(-) > >>> > >>> diff --git a/hw/core/machine.c b/hw/core/machine.c > >>> index 6d1a0d8eebc4..4a7322988fb5 100644 > >>> --- a/hw/core/machine.c > >>> +++ b/hw/core/machine.c > >>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > >>> return ms->suppress_vmdesc; > >>> } > >>> > >>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) > >>> +{ > >>> + MachineState *ms = MACHINE(obj); > >>> + > >>> + ms->config_section = value; > >>> +} > >>> + > >>> +static bool machine_get_config_section(Object *obj, Error **errp) > >>> +{ > >>> + MachineState *ms = MACHINE(obj); > >>> + > >>> + return ms->config_section; > >>> +} > >>> + > >>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > >>> { > >>> error_report("Option '-device %s' cannot be handled by this machine", > >>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > >>> ms->kvm_shadow_mem = -1; > >>> ms->dump_guest_core = true; > >>> ms->mem_merge = true; > >>> + ms->config_section = true; > >>> > >>> object_property_add_str(obj, "accel", > >>> machine_get_accel, machine_set_accel, NULL); > >>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > >>> object_property_set_description(obj, "suppress-vmdesc", > >>> "Set on to disable self-describing migration", > >>> NULL); > >>> + object_property_add_bool(obj, "config-section", > >>> + machine_get_config_section, > >>> + machine_set_config_section, NULL); > >>> + object_property_set_description(obj, "config-section", > >>> + "Set on/off to accept migration with/without configuration section", > >>> + NULL); > >>> > >>> /* Register notifier when init is done for sysbus sanity checks */ > >>> ms->sysbus_notifier.notify = machine_init_notify; > >>> diff --git a/include/hw/boards.h b/include/hw/boards.h > >>> index 0f30959e2e3b..853bb5905ec1 100644 > >>> --- a/include/hw/boards.h > >>> +++ b/include/hw/boards.h > >>> @@ -128,6 +128,7 @@ struct MachineState { > >>> char *firmware; > >>> bool iommu; > >>> bool suppress_vmdesc; > >>> + bool config_section; > >>> > >>> ram_addr_t ram_size; > >>> ram_addr_t maxram_size; > >>> diff --git a/migration/savevm.c b/migration/savevm.c > >>> index 94f2894243ce..3795489aeaec 100644 > >>> --- a/migration/savevm.c > >>> +++ b/migration/savevm.c > >>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > >>> return 0; > >>> } > >>> > >>> +static bool must_receive_configuration(void) > >>> +{ > >>> + MachineState *machine = MACHINE(qdev_get_machine()); > >>> + return machine->config_section; > >>> +} > >>> + > >>> int qemu_loadvm_state(QEMUFile *f) > >>> { > >>> MigrationIncomingState *mis = migration_incoming_get_current(); > >>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > >>> } > >>> > >>> if (!savevm_state.skip_configuration) { > >>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > >>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > >>> + qemu_file_skip(f, 1); > >>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > >>> + 0); > >>> + > >>> + if (ret) { > >>> + return ret; > >>> + } > >>> + } else if (must_receive_configuration()) { > >>> error_report("Configuration section missing"); > >>> return -EINVAL; > >>> } > >>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > >>> - > >>> - if (ret) { > >>> - return ret; > >>> - } > >>> } > >>> > >>> ret = qemu_loadvm_state_main(f, mis); > >>> diff --git a/qemu-options.hx b/qemu-options.hx > >>> index 2f0465eeb1d1..10cd64dc266b 100644 > >>> --- a/qemu-options.hx > >>> +++ b/qemu-options.hx > >>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > >>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > >>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > >>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > >>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", > >>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" > >>> + " config-section=on|off migration requires configuration section (default=on)\n", > >>> QEMU_ARCH_ALL) > >>> STEXI > >>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > >>> > >>> > >> > > >
On 16/02/2016 10:09, Greg Kurz wrote: > On Mon, 15 Feb 2016 15:49:17 +0100 > Laurent Vivier <lvivier@redhat.com> wrote: > >> On 15/02/2016 13:58, Greg Kurz wrote: >>> On Mon, 15 Feb 2016 12:23:39 +0100 >>> Laurent Vivier <lvivier@redhat.com> wrote: >>> >>>> On 15/02/2016 11:15, Greg Kurz wrote: >>>>> Since QEMU 2.4, the migration stream begins with a configuration section. >>>>> It is known to break migration of pseries machine from older QEMU. It is >>>>> possible to fix this in the pseries compat code but it will then break >>>>> migration of old pseries from latest QEMU. >>>>> >>>>> As an alternative, this patch introduces a new machine property which >>>>> allows to ignore the abscence of configuration section during incoming >>>>> migration. It boils to adding: >>>>> >>>>> -machine config-section=off >>>>> >>>>> Using this property only makes sense when migrating from an older >>>>> QEMU. It has no effect on outgoing migration. >>>> >>>> I think this is not true: migrating a pseries-2.3 machine from a >>>> qemu-2.4 to a qemu-2.3 must not send the configuration section because >>>> the qemu-2.3 will not be able to decode it. >>>> >>> >>> I did not mind to also fix backward compatibility... is it mandatory ? >> >> I don't know, but as a user I would like to be able to do that (and it >> is possible in the other cases). >> > > I fully agree backward migration is cool and we should not break it if > possible. > > The situation is bit different here: QEMU 2.4 broke migration both ways for > pseries-2.3 => all QEMU versions that were released since then (2.4/2.4.1/2.5) > are ultimately incompatible with QEMU 2.3 and this cannot be fixed. > > What we may try to achieve is that QEMU 2.6 can accept incoming migration > of pseries-2.3 from QEMU 2.3 (without configuration section) and from > QEMU 2.4/2.5 (with configuration section). > > This is what this series does, without the need for the user to actually pass > config-section=off thanks to patch 2. > >> And I think the fix could be smaller: something like just setting >> "savevm_state.skip_configuration" to the value of "config-section". >> [I don't know if it is as simple as it looks...] >> > > Not so simple is the word indeed... if we do this, a pseries-2.3 machine > started with config-section=off can only be migrated back to QEMU 2.3 since > QEMU 2.4/2.4.1/2.5 want the config section... is this what you want ? If yes, > then an even simpler fix is: > > https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg01738.html > > We can either fix forward migration for all QEMU versions with this series, > or fix backward migration to QEMU-2.3. Not both. > > What is the most important ? Yes, I agree with you, but if you pass the parameter from the QEMU monitor you can pass it when you want to migrate the machine (if needed). In fact, my opinion is not really important here, but we need the one from David Gilbert or Juan. >> Laurent >>> >>>> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 >>>> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: >>>> Invalid argument >>>> >>>> [This is the result without your patch] >>>> >>>> Laurent >>>>> >>>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> >>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> >>>>> --- >>>>> hw/core/machine.c | 21 +++++++++++++++++++++ >>>>> include/hw/boards.h | 1 + >>>>> migration/savevm.c | 21 +++++++++++++++------ >>>>> qemu-options.hx | 3 ++- >>>>> 4 files changed, 39 insertions(+), 7 deletions(-) >>>>> >>>>> diff --git a/hw/core/machine.c b/hw/core/machine.c >>>>> index 6d1a0d8eebc4..4a7322988fb5 100644 >>>>> --- a/hw/core/machine.c >>>>> +++ b/hw/core/machine.c >>>>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) >>>>> return ms->suppress_vmdesc; >>>>> } >>>>> >>>>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) >>>>> +{ >>>>> + MachineState *ms = MACHINE(obj); >>>>> + >>>>> + ms->config_section = value; >>>>> +} >>>>> + >>>>> +static bool machine_get_config_section(Object *obj, Error **errp) >>>>> +{ >>>>> + MachineState *ms = MACHINE(obj); >>>>> + >>>>> + return ms->config_section; >>>>> +} >>>>> + >>>>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) >>>>> { >>>>> error_report("Option '-device %s' cannot be handled by this machine", >>>>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) >>>>> ms->kvm_shadow_mem = -1; >>>>> ms->dump_guest_core = true; >>>>> ms->mem_merge = true; >>>>> + ms->config_section = true; >>>>> >>>>> object_property_add_str(obj, "accel", >>>>> machine_get_accel, machine_set_accel, NULL); >>>>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) >>>>> object_property_set_description(obj, "suppress-vmdesc", >>>>> "Set on to disable self-describing migration", >>>>> NULL); >>>>> + object_property_add_bool(obj, "config-section", >>>>> + machine_get_config_section, >>>>> + machine_set_config_section, NULL); >>>>> + object_property_set_description(obj, "config-section", >>>>> + "Set on/off to accept migration with/without configuration section", >>>>> + NULL); >>>>> >>>>> /* Register notifier when init is done for sysbus sanity checks */ >>>>> ms->sysbus_notifier.notify = machine_init_notify; >>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h >>>>> index 0f30959e2e3b..853bb5905ec1 100644 >>>>> --- a/include/hw/boards.h >>>>> +++ b/include/hw/boards.h >>>>> @@ -128,6 +128,7 @@ struct MachineState { >>>>> char *firmware; >>>>> bool iommu; >>>>> bool suppress_vmdesc; >>>>> + bool config_section; >>>>> >>>>> ram_addr_t ram_size; >>>>> ram_addr_t maxram_size; >>>>> diff --git a/migration/savevm.c b/migration/savevm.c >>>>> index 94f2894243ce..3795489aeaec 100644 >>>>> --- a/migration/savevm.c >>>>> +++ b/migration/savevm.c >>>>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) >>>>> return 0; >>>>> } >>>>> >>>>> +static bool must_receive_configuration(void) >>>>> +{ >>>>> + MachineState *machine = MACHINE(qdev_get_machine()); >>>>> + return machine->config_section; >>>>> +} >>>>> + >>>>> int qemu_loadvm_state(QEMUFile *f) >>>>> { >>>>> MigrationIncomingState *mis = migration_incoming_get_current(); >>>>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) >>>>> } >>>>> >>>>> if (!savevm_state.skip_configuration) { >>>>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { >>>>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { >>>>> + qemu_file_skip(f, 1); >>>>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, >>>>> + 0); >>>>> + >>>>> + if (ret) { >>>>> + return ret; >>>>> + } >>>>> + } else if (must_receive_configuration()) { >>>>> error_report("Configuration section missing"); >>>>> return -EINVAL; >>>>> } >>>>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); >>>>> - >>>>> - if (ret) { >>>>> - return ret; >>>>> - } >>>>> } >>>>> >>>>> ret = qemu_loadvm_state_main(f, mis); >>>>> diff --git a/qemu-options.hx b/qemu-options.hx >>>>> index 2f0465eeb1d1..10cd64dc266b 100644 >>>>> --- a/qemu-options.hx >>>>> +++ b/qemu-options.hx >>>>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ >>>>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" >>>>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" >>>>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" >>>>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", >>>>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" >>>>> + " config-section=on|off migration requires configuration section (default=on)\n", >>>>> QEMU_ARCH_ALL) >>>>> STEXI >>>>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] >>>>> >>>>> >>>> >>> >> >
On Tue, 16 Feb 2016 10:31:35 +0100 Laurent Vivier <lvivier@redhat.com> wrote: > On 16/02/2016 10:09, Greg Kurz wrote: > > On Mon, 15 Feb 2016 15:49:17 +0100 > > Laurent Vivier <lvivier@redhat.com> wrote: > > > >> On 15/02/2016 13:58, Greg Kurz wrote: > >>> On Mon, 15 Feb 2016 12:23:39 +0100 > >>> Laurent Vivier <lvivier@redhat.com> wrote: > >>> > >>>> On 15/02/2016 11:15, Greg Kurz wrote: > >>>>> Since QEMU 2.4, the migration stream begins with a configuration section. > >>>>> It is known to break migration of pseries machine from older QEMU. It is > >>>>> possible to fix this in the pseries compat code but it will then break > >>>>> migration of old pseries from latest QEMU. > >>>>> > >>>>> As an alternative, this patch introduces a new machine property which > >>>>> allows to ignore the abscence of configuration section during incoming > >>>>> migration. It boils to adding: > >>>>> > >>>>> -machine config-section=off > >>>>> > >>>>> Using this property only makes sense when migrating from an older > >>>>> QEMU. It has no effect on outgoing migration. > >>>> > >>>> I think this is not true: migrating a pseries-2.3 machine from a > >>>> qemu-2.4 to a qemu-2.3 must not send the configuration section because > >>>> the qemu-2.3 will not be able to decode it. > >>>> > >>> > >>> I did not mind to also fix backward compatibility... is it mandatory ? > >> > >> I don't know, but as a user I would like to be able to do that (and it > >> is possible in the other cases). > >> > > > > I fully agree backward migration is cool and we should not break it if > > possible. > > > > The situation is bit different here: QEMU 2.4 broke migration both ways for > > pseries-2.3 => all QEMU versions that were released since then (2.4/2.4.1/2.5) > > are ultimately incompatible with QEMU 2.3 and this cannot be fixed. > > > > What we may try to achieve is that QEMU 2.6 can accept incoming migration > > of pseries-2.3 from QEMU 2.3 (without configuration section) and from > > QEMU 2.4/2.5 (with configuration section). > > > > This is what this series does, without the need for the user to actually pass > > config-section=off thanks to patch 2. > > > >> And I think the fix could be smaller: something like just setting > >> "savevm_state.skip_configuration" to the value of "config-section". > >> [I don't know if it is as simple as it looks...] > >> > > > > Not so simple is the word indeed... if we do this, a pseries-2.3 machine > > started with config-section=off can only be migrated back to QEMU 2.3 since > > QEMU 2.4/2.4.1/2.5 want the config section... is this what you want ? If yes, > > then an even simpler fix is: > > > > https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg01738.html > > > > We can either fix forward migration for all QEMU versions with this series, > > or fix backward migration to QEMU-2.3. Not both. > > > > What is the most important ? > > Yes, I agree with you, but if you pass the parameter from the QEMU > monitor you can pass it when you want to migrate the machine (if needed). > Indeed... maybe it is acceptable when the user knows about the QEMU version on the target host. Is it something that could be handled by libvirt ? > In fact, my opinion is not really important here, but we need the one > from David Gilbert or Juan. > Yeah definitely. And FWIW, this patch was suggested by Dave Gilbert: https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg02639.html On the other hand, Dave Gibson agrees to break compatibility with QEMU 2.4/2.5 if we assume pseries-2.3 are mostly QEMU 2.3 based: https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg02963.html And your suggestion which should allow migration to/from all previous releases, at the cost of some extra complexity for users. I cannot decide which is less bad... > >> Laurent > >>> > >>>> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 > >>>> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: > >>>> Invalid argument > >>>> > >>>> [This is the result without your patch] > >>>> > >>>> Laurent > >>>>> > >>>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>>>> --- > >>>>> hw/core/machine.c | 21 +++++++++++++++++++++ > >>>>> include/hw/boards.h | 1 + > >>>>> migration/savevm.c | 21 +++++++++++++++------ > >>>>> qemu-options.hx | 3 ++- > >>>>> 4 files changed, 39 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/hw/core/machine.c b/hw/core/machine.c > >>>>> index 6d1a0d8eebc4..4a7322988fb5 100644 > >>>>> --- a/hw/core/machine.c > >>>>> +++ b/hw/core/machine.c > >>>>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > >>>>> return ms->suppress_vmdesc; > >>>>> } > >>>>> > >>>>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) > >>>>> +{ > >>>>> + MachineState *ms = MACHINE(obj); > >>>>> + > >>>>> + ms->config_section = value; > >>>>> +} > >>>>> + > >>>>> +static bool machine_get_config_section(Object *obj, Error **errp) > >>>>> +{ > >>>>> + MachineState *ms = MACHINE(obj); > >>>>> + > >>>>> + return ms->config_section; > >>>>> +} > >>>>> + > >>>>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > >>>>> { > >>>>> error_report("Option '-device %s' cannot be handled by this machine", > >>>>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > >>>>> ms->kvm_shadow_mem = -1; > >>>>> ms->dump_guest_core = true; > >>>>> ms->mem_merge = true; > >>>>> + ms->config_section = true; > >>>>> > >>>>> object_property_add_str(obj, "accel", > >>>>> machine_get_accel, machine_set_accel, NULL); > >>>>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > >>>>> object_property_set_description(obj, "suppress-vmdesc", > >>>>> "Set on to disable self-describing migration", > >>>>> NULL); > >>>>> + object_property_add_bool(obj, "config-section", > >>>>> + machine_get_config_section, > >>>>> + machine_set_config_section, NULL); > >>>>> + object_property_set_description(obj, "config-section", > >>>>> + "Set on/off to accept migration with/without configuration section", > >>>>> + NULL); > >>>>> > >>>>> /* Register notifier when init is done for sysbus sanity checks */ > >>>>> ms->sysbus_notifier.notify = machine_init_notify; > >>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h > >>>>> index 0f30959e2e3b..853bb5905ec1 100644 > >>>>> --- a/include/hw/boards.h > >>>>> +++ b/include/hw/boards.h > >>>>> @@ -128,6 +128,7 @@ struct MachineState { > >>>>> char *firmware; > >>>>> bool iommu; > >>>>> bool suppress_vmdesc; > >>>>> + bool config_section; > >>>>> > >>>>> ram_addr_t ram_size; > >>>>> ram_addr_t maxram_size; > >>>>> diff --git a/migration/savevm.c b/migration/savevm.c > >>>>> index 94f2894243ce..3795489aeaec 100644 > >>>>> --- a/migration/savevm.c > >>>>> +++ b/migration/savevm.c > >>>>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > >>>>> return 0; > >>>>> } > >>>>> > >>>>> +static bool must_receive_configuration(void) > >>>>> +{ > >>>>> + MachineState *machine = MACHINE(qdev_get_machine()); > >>>>> + return machine->config_section; > >>>>> +} > >>>>> + > >>>>> int qemu_loadvm_state(QEMUFile *f) > >>>>> { > >>>>> MigrationIncomingState *mis = migration_incoming_get_current(); > >>>>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > >>>>> } > >>>>> > >>>>> if (!savevm_state.skip_configuration) { > >>>>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > >>>>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > >>>>> + qemu_file_skip(f, 1); > >>>>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > >>>>> + 0); > >>>>> + > >>>>> + if (ret) { > >>>>> + return ret; > >>>>> + } > >>>>> + } else if (must_receive_configuration()) { > >>>>> error_report("Configuration section missing"); > >>>>> return -EINVAL; > >>>>> } > >>>>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > >>>>> - > >>>>> - if (ret) { > >>>>> - return ret; > >>>>> - } > >>>>> } > >>>>> > >>>>> ret = qemu_loadvm_state_main(f, mis); > >>>>> diff --git a/qemu-options.hx b/qemu-options.hx > >>>>> index 2f0465eeb1d1..10cd64dc266b 100644 > >>>>> --- a/qemu-options.hx > >>>>> +++ b/qemu-options.hx > >>>>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > >>>>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > >>>>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > >>>>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > >>>>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", > >>>>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" > >>>>> + " config-section=on|off migration requires configuration section (default=on)\n", > >>>>> QEMU_ARCH_ALL) > >>>>> STEXI > >>>>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > >>>>> > >>>>> > >>>> > >>> > >> > > >
* Greg Kurz (gkurz@linux.vnet.ibm.com) wrote: > Since QEMU 2.4, the migration stream begins with a configuration section. > It is known to break migration of pseries machine from older QEMU. It is > possible to fix this in the pseries compat code but it will then break > migration of old pseries from latest QEMU. > > As an alternative, this patch introduces a new machine property which > allows to ignore the abscence of configuration section during incoming > migration. It boils to adding: > > -machine config-section=off > > Using this property only makes sense when migrating from an older > QEMU. It has no effect on outgoing migration. Hi Greg, So I'm mostly OK with this, but please change it to: 'require-config-section' and the bool similarly. We've effectively got 2 separate things: a) Whether a config section is sent b) Whether a config section is required upon reception I want to make sure the naming makes it obvious which is being changed, and you're only affecting (b). Dave > > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > --- > hw/core/machine.c | 21 +++++++++++++++++++++ > include/hw/boards.h | 1 + > migration/savevm.c | 21 +++++++++++++++------ > qemu-options.hx | 3 ++- > 4 files changed, 39 insertions(+), 7 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 6d1a0d8eebc4..4a7322988fb5 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > return ms->suppress_vmdesc; > } > > +static void machine_set_config_section(Object *obj, bool value, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + ms->config_section = value; > +} > + > +static bool machine_get_config_section(Object *obj, Error **errp) > +{ > + MachineState *ms = MACHINE(obj); > + > + return ms->config_section; > +} > + > static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > { > error_report("Option '-device %s' cannot be handled by this machine", > @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > ms->kvm_shadow_mem = -1; > ms->dump_guest_core = true; > ms->mem_merge = true; > + ms->config_section = true; > > object_property_add_str(obj, "accel", > machine_get_accel, machine_set_accel, NULL); > @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > object_property_set_description(obj, "suppress-vmdesc", > "Set on to disable self-describing migration", > NULL); > + object_property_add_bool(obj, "config-section", > + machine_get_config_section, > + machine_set_config_section, NULL); > + object_property_set_description(obj, "config-section", > + "Set on/off to accept migration with/without configuration section", > + NULL); > > /* Register notifier when init is done for sysbus sanity checks */ > ms->sysbus_notifier.notify = machine_init_notify; > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 0f30959e2e3b..853bb5905ec1 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -128,6 +128,7 @@ struct MachineState { > char *firmware; > bool iommu; > bool suppress_vmdesc; > + bool config_section; > > ram_addr_t ram_size; > ram_addr_t maxram_size; > diff --git a/migration/savevm.c b/migration/savevm.c > index 94f2894243ce..3795489aeaec 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > return 0; > } > > +static bool must_receive_configuration(void) > +{ > + MachineState *machine = MACHINE(qdev_get_machine()); > + return machine->config_section; > +} > + > int qemu_loadvm_state(QEMUFile *f) > { > MigrationIncomingState *mis = migration_incoming_get_current(); > @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > } > > if (!savevm_state.skip_configuration) { > - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > + qemu_file_skip(f, 1); > + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > + 0); > + > + if (ret) { > + return ret; > + } > + } else if (must_receive_configuration()) { > error_report("Configuration section missing"); > return -EINVAL; > } > - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > - > - if (ret) { > - return ret; > - } > } > > ret = qemu_loadvm_state_main(f, mis); > diff --git a/qemu-options.hx b/qemu-options.hx > index 2f0465eeb1d1..10cd64dc266b 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > - " nvdimm=on|off controls NVDIMM support (default=off)\n", > + " nvdimm=on|off controls NVDIMM support (default=off)\n" > + " config-section=on|off migration requires configuration section (default=on)\n", > QEMU_ARCH_ALL) > STEXI > @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Laurent Vivier (lvivier@redhat.com) wrote: > > > On 16/02/2016 10:09, Greg Kurz wrote: > > On Mon, 15 Feb 2016 15:49:17 +0100 > > Laurent Vivier <lvivier@redhat.com> wrote: > > > >> On 15/02/2016 13:58, Greg Kurz wrote: > >>> On Mon, 15 Feb 2016 12:23:39 +0100 > >>> Laurent Vivier <lvivier@redhat.com> wrote: > >>> > >>>> On 15/02/2016 11:15, Greg Kurz wrote: > >>>>> Since QEMU 2.4, the migration stream begins with a configuration section. > >>>>> It is known to break migration of pseries machine from older QEMU. It is > >>>>> possible to fix this in the pseries compat code but it will then break > >>>>> migration of old pseries from latest QEMU. > >>>>> > >>>>> As an alternative, this patch introduces a new machine property which > >>>>> allows to ignore the abscence of configuration section during incoming > >>>>> migration. It boils to adding: > >>>>> > >>>>> -machine config-section=off > >>>>> > >>>>> Using this property only makes sense when migrating from an older > >>>>> QEMU. It has no effect on outgoing migration. > >>>> > >>>> I think this is not true: migrating a pseries-2.3 machine from a > >>>> qemu-2.4 to a qemu-2.3 must not send the configuration section because > >>>> the qemu-2.3 will not be able to decode it. > >>>> > >>> > >>> I did not mind to also fix backward compatibility... is it mandatory ? > >> > >> I don't know, but as a user I would like to be able to do that (and it > >> is possible in the other cases). > >> > > > > I fully agree backward migration is cool and we should not break it if > > possible. > > > > The situation is bit different here: QEMU 2.4 broke migration both ways for > > pseries-2.3 => all QEMU versions that were released since then (2.4/2.4.1/2.5) > > are ultimately incompatible with QEMU 2.3 and this cannot be fixed. > > > > What we may try to achieve is that QEMU 2.6 can accept incoming migration > > of pseries-2.3 from QEMU 2.3 (without configuration section) and from > > QEMU 2.4/2.5 (with configuration section). > > > > This is what this series does, without the need for the user to actually pass > > config-section=off thanks to patch 2. > > > >> And I think the fix could be smaller: something like just setting > >> "savevm_state.skip_configuration" to the value of "config-section". > >> [I don't know if it is as simple as it looks...] > >> > > > > Not so simple is the word indeed... if we do this, a pseries-2.3 machine > > started with config-section=off can only be migrated back to QEMU 2.3 since > > QEMU 2.4/2.4.1/2.5 want the config section... is this what you want ? If yes, > > then an even simpler fix is: > > > > https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg01738.html > > > > We can either fix forward migration for all QEMU versions with this series, > > or fix backward migration to QEMU-2.3. Not both. > > > > What is the most important ? > > Yes, I agree with you, but if you pass the parameter from the QEMU > monitor you can pass it when you want to migrate the machine (if needed). > > In fact, my opinion is not really important here, but we need the one > from David Gilbert or Juan. I dont think we do anything like this at the moment, however if you actually did make this machine property influence whether you sent a configuration (opposite to my previous reply!), then I think you could do: qom-set /machine config-section off before migration. However, you would have to teach all the tooling to do that, and I don't think we've got anything that would do it at the moment. Dave > > >> Laurent > >>> > >>>> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 > >>>> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: > >>>> Invalid argument > >>>> > >>>> [This is the result without your patch] > >>>> > >>>> Laurent > >>>>> > >>>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > >>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > >>>>> --- > >>>>> hw/core/machine.c | 21 +++++++++++++++++++++ > >>>>> include/hw/boards.h | 1 + > >>>>> migration/savevm.c | 21 +++++++++++++++------ > >>>>> qemu-options.hx | 3 ++- > >>>>> 4 files changed, 39 insertions(+), 7 deletions(-) > >>>>> > >>>>> diff --git a/hw/core/machine.c b/hw/core/machine.c > >>>>> index 6d1a0d8eebc4..4a7322988fb5 100644 > >>>>> --- a/hw/core/machine.c > >>>>> +++ b/hw/core/machine.c > >>>>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > >>>>> return ms->suppress_vmdesc; > >>>>> } > >>>>> > >>>>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) > >>>>> +{ > >>>>> + MachineState *ms = MACHINE(obj); > >>>>> + > >>>>> + ms->config_section = value; > >>>>> +} > >>>>> + > >>>>> +static bool machine_get_config_section(Object *obj, Error **errp) > >>>>> +{ > >>>>> + MachineState *ms = MACHINE(obj); > >>>>> + > >>>>> + return ms->config_section; > >>>>> +} > >>>>> + > >>>>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > >>>>> { > >>>>> error_report("Option '-device %s' cannot be handled by this machine", > >>>>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > >>>>> ms->kvm_shadow_mem = -1; > >>>>> ms->dump_guest_core = true; > >>>>> ms->mem_merge = true; > >>>>> + ms->config_section = true; > >>>>> > >>>>> object_property_add_str(obj, "accel", > >>>>> machine_get_accel, machine_set_accel, NULL); > >>>>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > >>>>> object_property_set_description(obj, "suppress-vmdesc", > >>>>> "Set on to disable self-describing migration", > >>>>> NULL); > >>>>> + object_property_add_bool(obj, "config-section", > >>>>> + machine_get_config_section, > >>>>> + machine_set_config_section, NULL); > >>>>> + object_property_set_description(obj, "config-section", > >>>>> + "Set on/off to accept migration with/without configuration section", > >>>>> + NULL); > >>>>> > >>>>> /* Register notifier when init is done for sysbus sanity checks */ > >>>>> ms->sysbus_notifier.notify = machine_init_notify; > >>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h > >>>>> index 0f30959e2e3b..853bb5905ec1 100644 > >>>>> --- a/include/hw/boards.h > >>>>> +++ b/include/hw/boards.h > >>>>> @@ -128,6 +128,7 @@ struct MachineState { > >>>>> char *firmware; > >>>>> bool iommu; > >>>>> bool suppress_vmdesc; > >>>>> + bool config_section; > >>>>> > >>>>> ram_addr_t ram_size; > >>>>> ram_addr_t maxram_size; > >>>>> diff --git a/migration/savevm.c b/migration/savevm.c > >>>>> index 94f2894243ce..3795489aeaec 100644 > >>>>> --- a/migration/savevm.c > >>>>> +++ b/migration/savevm.c > >>>>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > >>>>> return 0; > >>>>> } > >>>>> > >>>>> +static bool must_receive_configuration(void) > >>>>> +{ > >>>>> + MachineState *machine = MACHINE(qdev_get_machine()); > >>>>> + return machine->config_section; > >>>>> +} > >>>>> + > >>>>> int qemu_loadvm_state(QEMUFile *f) > >>>>> { > >>>>> MigrationIncomingState *mis = migration_incoming_get_current(); > >>>>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > >>>>> } > >>>>> > >>>>> if (!savevm_state.skip_configuration) { > >>>>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > >>>>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > >>>>> + qemu_file_skip(f, 1); > >>>>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > >>>>> + 0); > >>>>> + > >>>>> + if (ret) { > >>>>> + return ret; > >>>>> + } > >>>>> + } else if (must_receive_configuration()) { > >>>>> error_report("Configuration section missing"); > >>>>> return -EINVAL; > >>>>> } > >>>>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > >>>>> - > >>>>> - if (ret) { > >>>>> - return ret; > >>>>> - } > >>>>> } > >>>>> > >>>>> ret = qemu_loadvm_state_main(f, mis); > >>>>> diff --git a/qemu-options.hx b/qemu-options.hx > >>>>> index 2f0465eeb1d1..10cd64dc266b 100644 > >>>>> --- a/qemu-options.hx > >>>>> +++ b/qemu-options.hx > >>>>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > >>>>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > >>>>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > >>>>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > >>>>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", > >>>>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" > >>>>> + " config-section=on|off migration requires configuration section (default=on)\n", > >>>>> QEMU_ARCH_ALL) > >>>>> STEXI > >>>>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > >>>>> > >>>>> > >>>> > >>> > >> > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Tue, 16 Feb 2016 17:01:40 +0000 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Greg Kurz (gkurz@linux.vnet.ibm.com) wrote: > > Since QEMU 2.4, the migration stream begins with a configuration section. > > It is known to break migration of pseries machine from older QEMU. It is > > possible to fix this in the pseries compat code but it will then break > > migration of old pseries from latest QEMU. > > > > As an alternative, this patch introduces a new machine property which > > allows to ignore the abscence of configuration section during incoming > > migration. It boils to adding: > > > > -machine config-section=off > > > > Using this property only makes sense when migrating from an older > > QEMU. It has no effect on outgoing migration. > > Hi Greg, > So I'm mostly OK with this, but please change it to: > 'require-config-section' and the bool similarly. > > We've effectively got 2 separate things: > a) Whether a config section is sent > b) Whether a config section is required upon reception > > I want to make sure the naming makes it obvious which is being changed, > and you're only affecting (b). > Sure ! I'll do that. > Dave > > > > > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > --- > > hw/core/machine.c | 21 +++++++++++++++++++++ > > include/hw/boards.h | 1 + > > migration/savevm.c | 21 +++++++++++++++------ > > qemu-options.hx | 3 ++- > > 4 files changed, 39 insertions(+), 7 deletions(-) > > > > diff --git a/hw/core/machine.c b/hw/core/machine.c > > index 6d1a0d8eebc4..4a7322988fb5 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > > return ms->suppress_vmdesc; > > } > > > > +static void machine_set_config_section(Object *obj, bool value, Error **errp) > > +{ > > + MachineState *ms = MACHINE(obj); > > + > > + ms->config_section = value; > > +} > > + > > +static bool machine_get_config_section(Object *obj, Error **errp) > > +{ > > + MachineState *ms = MACHINE(obj); > > + > > + return ms->config_section; > > +} > > + > > static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > > { > > error_report("Option '-device %s' cannot be handled by this machine", > > @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > > ms->kvm_shadow_mem = -1; > > ms->dump_guest_core = true; > > ms->mem_merge = true; > > + ms->config_section = true; > > > > object_property_add_str(obj, "accel", > > machine_get_accel, machine_set_accel, NULL); > > @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > > object_property_set_description(obj, "suppress-vmdesc", > > "Set on to disable self-describing migration", > > NULL); > > + object_property_add_bool(obj, "config-section", > > + machine_get_config_section, > > + machine_set_config_section, NULL); > > + object_property_set_description(obj, "config-section", > > + "Set on/off to accept migration with/without configuration section", > > + NULL); > > > > /* Register notifier when init is done for sysbus sanity checks */ > > ms->sysbus_notifier.notify = machine_init_notify; > > diff --git a/include/hw/boards.h b/include/hw/boards.h > > index 0f30959e2e3b..853bb5905ec1 100644 > > --- a/include/hw/boards.h > > +++ b/include/hw/boards.h > > @@ -128,6 +128,7 @@ struct MachineState { > > char *firmware; > > bool iommu; > > bool suppress_vmdesc; > > + bool config_section; > > > > ram_addr_t ram_size; > > ram_addr_t maxram_size; > > diff --git a/migration/savevm.c b/migration/savevm.c > > index 94f2894243ce..3795489aeaec 100644 > > --- a/migration/savevm.c > > +++ b/migration/savevm.c > > @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > > return 0; > > } > > > > +static bool must_receive_configuration(void) > > +{ > > + MachineState *machine = MACHINE(qdev_get_machine()); > > + return machine->config_section; > > +} > > + > > int qemu_loadvm_state(QEMUFile *f) > > { > > MigrationIncomingState *mis = migration_incoming_get_current(); > > @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > > } > > > > if (!savevm_state.skip_configuration) { > > - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > > + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > > + qemu_file_skip(f, 1); > > + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > > + 0); > > + > > + if (ret) { > > + return ret; > > + } > > + } else if (must_receive_configuration()) { > > error_report("Configuration section missing"); > > return -EINVAL; > > } > > - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > > - > > - if (ret) { > > - return ret; > > - } > > } > > > > ret = qemu_loadvm_state_main(f, mis); > > diff --git a/qemu-options.hx b/qemu-options.hx > > index 2f0465eeb1d1..10cd64dc266b 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > > " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > > " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > > " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > > - " nvdimm=on|off controls NVDIMM support (default=off)\n", > > + " nvdimm=on|off controls NVDIMM support (default=off)\n" > > + " config-section=on|off migration requires configuration section (default=on)\n", > > QEMU_ARCH_ALL) > > STEXI > > @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
On Tue, 16 Feb 2016 17:09:52 +0000 "Dr. David Alan Gilbert" <dgilbert@redhat.com> wrote: > * Laurent Vivier (lvivier@redhat.com) wrote: > > > > > > On 16/02/2016 10:09, Greg Kurz wrote: > > > On Mon, 15 Feb 2016 15:49:17 +0100 > > > Laurent Vivier <lvivier@redhat.com> wrote: > > > > > >> On 15/02/2016 13:58, Greg Kurz wrote: > > >>> On Mon, 15 Feb 2016 12:23:39 +0100 > > >>> Laurent Vivier <lvivier@redhat.com> wrote: > > >>> > > >>>> On 15/02/2016 11:15, Greg Kurz wrote: > > >>>>> Since QEMU 2.4, the migration stream begins with a configuration section. > > >>>>> It is known to break migration of pseries machine from older QEMU. It is > > >>>>> possible to fix this in the pseries compat code but it will then break > > >>>>> migration of old pseries from latest QEMU. > > >>>>> > > >>>>> As an alternative, this patch introduces a new machine property which > > >>>>> allows to ignore the abscence of configuration section during incoming > > >>>>> migration. It boils to adding: > > >>>>> > > >>>>> -machine config-section=off > > >>>>> > > >>>>> Using this property only makes sense when migrating from an older > > >>>>> QEMU. It has no effect on outgoing migration. > > >>>> > > >>>> I think this is not true: migrating a pseries-2.3 machine from a > > >>>> qemu-2.4 to a qemu-2.3 must not send the configuration section because > > >>>> the qemu-2.3 will not be able to decode it. > > >>>> > > >>> > > >>> I did not mind to also fix backward compatibility... is it mandatory ? > > >> > > >> I don't know, but as a user I would like to be able to do that (and it > > >> is possible in the other cases). > > >> > > > > > > I fully agree backward migration is cool and we should not break it if > > > possible. > > > > > > The situation is bit different here: QEMU 2.4 broke migration both ways for > > > pseries-2.3 => all QEMU versions that were released since then (2.4/2.4.1/2.5) > > > are ultimately incompatible with QEMU 2.3 and this cannot be fixed. > > > > > > What we may try to achieve is that QEMU 2.6 can accept incoming migration > > > of pseries-2.3 from QEMU 2.3 (without configuration section) and from > > > QEMU 2.4/2.5 (with configuration section). > > > > > > This is what this series does, without the need for the user to actually pass > > > config-section=off thanks to patch 2. > > > > > >> And I think the fix could be smaller: something like just setting > > >> "savevm_state.skip_configuration" to the value of "config-section". > > >> [I don't know if it is as simple as it looks...] > > >> > > > > > > Not so simple is the word indeed... if we do this, a pseries-2.3 machine > > > started with config-section=off can only be migrated back to QEMU 2.3 since > > > QEMU 2.4/2.4.1/2.5 want the config section... is this what you want ? If yes, > > > then an even simpler fix is: > > > > > > https://lists.nongnu.org/archive/html/qemu-devel/2016-02/msg01738.html > > > > > > We can either fix forward migration for all QEMU versions with this series, > > > or fix backward migration to QEMU-2.3. Not both. > > > > > > What is the most important ? > > > > Yes, I agree with you, but if you pass the parameter from the QEMU > > monitor you can pass it when you want to migrate the machine (if needed). > > > > In fact, my opinion is not really important here, but we need the one > > from David Gilbert or Juan. > > I dont think we do anything like this at the moment, however if you actually > did make this machine property influence whether you sent a configuration > (opposite to my previous reply!), then I think you could do: > > qom-set /machine config-section off > > before migration. > Yes, I guess that's what Laurent was suggesting in his previous mail. Does this call for another machine option to be able to not send the configuration section (the (a) case in your other mail) ? > However, you would have to teach all the tooling to do that, and I don't > think we've got anything that would do it at the moment. > Especially the tooling should know about the target QEMU to decide if a configuration section must be sent or not... > Dave > Thanks. -- Greg > > > > >> Laurent > > >>> > > >>>> 2016-02-15T11:22:08.645978Z qemu-system-ppc64: Unknown savevm section type 7 > > >>>> 2016-02-15T11:22:08.646041Z qemu-system-ppc64: load of migration failed: > > >>>> Invalid argument > > >>>> > > >>>> [This is the result without your patch] > > >>>> > > >>>> Laurent > > >>>>> > > >>>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > > >>>>> Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > >>>>> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > > >>>>> --- > > >>>>> hw/core/machine.c | 21 +++++++++++++++++++++ > > >>>>> include/hw/boards.h | 1 + > > >>>>> migration/savevm.c | 21 +++++++++++++++------ > > >>>>> qemu-options.hx | 3 ++- > > >>>>> 4 files changed, 39 insertions(+), 7 deletions(-) > > >>>>> > > >>>>> diff --git a/hw/core/machine.c b/hw/core/machine.c > > >>>>> index 6d1a0d8eebc4..4a7322988fb5 100644 > > >>>>> --- a/hw/core/machine.c > > >>>>> +++ b/hw/core/machine.c > > >>>>> @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) > > >>>>> return ms->suppress_vmdesc; > > >>>>> } > > >>>>> > > >>>>> +static void machine_set_config_section(Object *obj, bool value, Error **errp) > > >>>>> +{ > > >>>>> + MachineState *ms = MACHINE(obj); > > >>>>> + > > >>>>> + ms->config_section = value; > > >>>>> +} > > >>>>> + > > >>>>> +static bool machine_get_config_section(Object *obj, Error **errp) > > >>>>> +{ > > >>>>> + MachineState *ms = MACHINE(obj); > > >>>>> + > > >>>>> + return ms->config_section; > > >>>>> +} > > >>>>> + > > >>>>> static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) > > >>>>> { > > >>>>> error_report("Option '-device %s' cannot be handled by this machine", > > >>>>> @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) > > >>>>> ms->kvm_shadow_mem = -1; > > >>>>> ms->dump_guest_core = true; > > >>>>> ms->mem_merge = true; > > >>>>> + ms->config_section = true; > > >>>>> > > >>>>> object_property_add_str(obj, "accel", > > >>>>> machine_get_accel, machine_set_accel, NULL); > > >>>>> @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) > > >>>>> object_property_set_description(obj, "suppress-vmdesc", > > >>>>> "Set on to disable self-describing migration", > > >>>>> NULL); > > >>>>> + object_property_add_bool(obj, "config-section", > > >>>>> + machine_get_config_section, > > >>>>> + machine_set_config_section, NULL); > > >>>>> + object_property_set_description(obj, "config-section", > > >>>>> + "Set on/off to accept migration with/without configuration section", > > >>>>> + NULL); > > >>>>> > > >>>>> /* Register notifier when init is done for sysbus sanity checks */ > > >>>>> ms->sysbus_notifier.notify = machine_init_notify; > > >>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h > > >>>>> index 0f30959e2e3b..853bb5905ec1 100644 > > >>>>> --- a/include/hw/boards.h > > >>>>> +++ b/include/hw/boards.h > > >>>>> @@ -128,6 +128,7 @@ struct MachineState { > > >>>>> char *firmware; > > >>>>> bool iommu; > > >>>>> bool suppress_vmdesc; > > >>>>> + bool config_section; > > >>>>> > > >>>>> ram_addr_t ram_size; > > >>>>> ram_addr_t maxram_size; > > >>>>> diff --git a/migration/savevm.c b/migration/savevm.c > > >>>>> index 94f2894243ce..3795489aeaec 100644 > > >>>>> --- a/migration/savevm.c > > >>>>> +++ b/migration/savevm.c > > >>>>> @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) > > >>>>> return 0; > > >>>>> } > > >>>>> > > >>>>> +static bool must_receive_configuration(void) > > >>>>> +{ > > >>>>> + MachineState *machine = MACHINE(qdev_get_machine()); > > >>>>> + return machine->config_section; > > >>>>> +} > > >>>>> + > > >>>>> int qemu_loadvm_state(QEMUFile *f) > > >>>>> { > > >>>>> MigrationIncomingState *mis = migration_incoming_get_current(); > > >>>>> @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) > > >>>>> } > > >>>>> > > >>>>> if (!savevm_state.skip_configuration) { > > >>>>> - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { > > >>>>> + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { > > >>>>> + qemu_file_skip(f, 1); > > >>>>> + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, > > >>>>> + 0); > > >>>>> + > > >>>>> + if (ret) { > > >>>>> + return ret; > > >>>>> + } > > >>>>> + } else if (must_receive_configuration()) { > > >>>>> error_report("Configuration section missing"); > > >>>>> return -EINVAL; > > >>>>> } > > >>>>> - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); > > >>>>> - > > >>>>> - if (ret) { > > >>>>> - return ret; > > >>>>> - } > > >>>>> } > > >>>>> > > >>>>> ret = qemu_loadvm_state_main(f, mis); > > >>>>> diff --git a/qemu-options.hx b/qemu-options.hx > > >>>>> index 2f0465eeb1d1..10cd64dc266b 100644 > > >>>>> --- a/qemu-options.hx > > >>>>> +++ b/qemu-options.hx > > >>>>> @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ > > >>>>> " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" > > >>>>> " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" > > >>>>> " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" > > >>>>> - " nvdimm=on|off controls NVDIMM support (default=off)\n", > > >>>>> + " nvdimm=on|off controls NVDIMM support (default=off)\n" > > >>>>> + " config-section=on|off migration requires configuration section (default=on)\n", > > >>>>> QEMU_ARCH_ALL) > > >>>>> STEXI > > >>>>> @item -machine [type=]@var{name}[,prop=@var{value}[,...]] > > >>>>> > > >>>>> > > >>>> > > >>> > > >> > > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
diff --git a/hw/core/machine.c b/hw/core/machine.c index 6d1a0d8eebc4..4a7322988fb5 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -312,6 +312,20 @@ static bool machine_get_suppress_vmdesc(Object *obj, Error **errp) return ms->suppress_vmdesc; } +static void machine_set_config_section(Object *obj, bool value, Error **errp) +{ + MachineState *ms = MACHINE(obj); + + ms->config_section = value; +} + +static bool machine_get_config_section(Object *obj, Error **errp) +{ + MachineState *ms = MACHINE(obj); + + return ms->config_section; +} + static int error_on_sysbus_device(SysBusDevice *sbdev, void *opaque) { error_report("Option '-device %s' cannot be handled by this machine", @@ -365,6 +379,7 @@ static void machine_initfn(Object *obj) ms->kvm_shadow_mem = -1; ms->dump_guest_core = true; ms->mem_merge = true; + ms->config_section = true; object_property_add_str(obj, "accel", machine_get_accel, machine_set_accel, NULL); @@ -467,6 +482,12 @@ static void machine_initfn(Object *obj) object_property_set_description(obj, "suppress-vmdesc", "Set on to disable self-describing migration", NULL); + object_property_add_bool(obj, "config-section", + machine_get_config_section, + machine_set_config_section, NULL); + object_property_set_description(obj, "config-section", + "Set on/off to accept migration with/without configuration section", + NULL); /* Register notifier when init is done for sysbus sanity checks */ ms->sysbus_notifier.notify = machine_init_notify; diff --git a/include/hw/boards.h b/include/hw/boards.h index 0f30959e2e3b..853bb5905ec1 100644 --- a/include/hw/boards.h +++ b/include/hw/boards.h @@ -128,6 +128,7 @@ struct MachineState { char *firmware; bool iommu; bool suppress_vmdesc; + bool config_section; ram_addr_t ram_size; ram_addr_t maxram_size; diff --git a/migration/savevm.c b/migration/savevm.c index 94f2894243ce..3795489aeaec 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -1847,6 +1847,12 @@ static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis) return 0; } +static bool must_receive_configuration(void) +{ + MachineState *machine = MACHINE(qdev_get_machine()); + return machine->config_section; +} + int qemu_loadvm_state(QEMUFile *f) { MigrationIncomingState *mis = migration_incoming_get_current(); @@ -1876,15 +1882,18 @@ int qemu_loadvm_state(QEMUFile *f) } if (!savevm_state.skip_configuration) { - if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) { + if (qemu_peek_byte(f, 0) == QEMU_VM_CONFIGURATION) { + qemu_file_skip(f, 1); + ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, + 0); + + if (ret) { + return ret; + } + } else if (must_receive_configuration()) { error_report("Configuration section missing"); return -EINVAL; } - ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0); - - if (ret) { - return ret; - } } ret = qemu_loadvm_state_main(f, mis); diff --git a/qemu-options.hx b/qemu-options.hx index 2f0465eeb1d1..10cd64dc266b 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -43,7 +43,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \ " aes-key-wrap=on|off controls support for AES key wrapping (default=on)\n" " dea-key-wrap=on|off controls support for DEA key wrapping (default=on)\n" " suppress-vmdesc=on|off disables self-describing migration (default=off)\n" - " nvdimm=on|off controls NVDIMM support (default=off)\n", + " nvdimm=on|off controls NVDIMM support (default=off)\n" + " config-section=on|off migration requires configuration section (default=on)\n", QEMU_ARCH_ALL) STEXI @item -machine [type=]@var{name}[,prop=@var{value}[,...]]