On Mon, 15 Feb 2016 15:49:17 +0100 Laurent Vivier <lviv...@redhat.com> wrote:
> On 15/02/2016 13:58, Greg Kurz wrote: > > On Mon, 15 Feb 2016 12:23:39 +0100 > > Laurent Vivier <lviv...@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 <dgilb...@redhat.com> > >>> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au> > >>> Signed-off-by: Greg Kurz <gk...@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}[,...]] > >>> > >>> > >> > > >
