* Laurent Vivier (lviv...@redhat.com) wrote:
>
>
> On 16/02/2016 10:09, Greg Kurz wrote:
> > 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 ?
>
> 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 <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}[,...]]
> >>>>>
> >>>>>
> >>>>
> >>>
> >>
> >
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
