* Igor Mammedov (imamm...@redhat.com) wrote:
> instance_id is generated by last_used_id + 1 for a given device type
> so for QEMU with 3 CPUs instance_id for APICs is a seti of [0, 1, 2]
> When CPU in the middle is hot-removed and migration started
> APICs with instance_ids 0 and 2 are transferred in migration stream.
> However target starts with 2 CPUs and APICs' instance_ids are
> generated from scratch [0, 1] hence migration fails with error
> Unknown savevm section or instance 'apic' 2
>
> Fix issue by manually registering APIC's vmsd with apic_id as
> instance_id, in this case instance_id on target will always
> match instance_id on source as apic_id is the same for a given
> cpu instance.
>
> Reported-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> Signed-off-by: Igor Mammedov <imamm...@redhat.com>
OK, I think this works from the migration side of things.
The old version used the qdev registration but since you didn't
have a bus it was still the simple "apic" naming, so I don't think
that changes.
Since you're tieing it to machine type we never have to worry
about clashes between the old and new numbering.
So, from the migration side of things I think this is about
as simple a fix as we're going to see, although I'm
expecting you're running into similar problems in other devices.
Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
Dave
> ---
> hw/intc/apic_common.c | 12 +++++++++++-
> include/hw/i386/apic_internal.h | 1 +
> include/hw/i386/pc.h | 5 +++++
> 3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
> index 0bb9ad5..14ac43c 100644
> --- a/hw/intc/apic_common.c
> +++ b/hw/intc/apic_common.c
> @@ -294,11 +294,14 @@ static int apic_load_old(QEMUFile *f, void *opaque, int
> version_id)
> return 0;
> }
>
> +static const VMStateDescription vmstate_apic_common;
> +
> static void apic_common_realize(DeviceState *dev, Error **errp)
> {
> APICCommonState *s = APIC_COMMON(dev);
> APICCommonClass *info;
> static DeviceState *vapic;
> + int instance_id = s->id;
>
> info = APIC_COMMON_GET_CLASS(s);
> info->realize(dev, errp);
> @@ -313,6 +316,11 @@ static void apic_common_realize(DeviceState *dev, Error
> **errp)
> info->enable_tpr_reporting(s, true);
> }
>
> + if (s->legacy_instance_id) {
> + instance_id = -1;
> + }
> + vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
> + s, -1, 0);
> }
>
> static void apic_common_unrealize(DeviceState *dev, Error **errp)
> @@ -320,6 +328,7 @@ static void apic_common_unrealize(DeviceState *dev, Error
> **errp)
> APICCommonState *s = APIC_COMMON(dev);
> APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
>
> + vmstate_unregister(NULL, &vmstate_apic_common, s);
> info->unrealize(dev, errp);
>
> if (apic_report_tpr_access && info->enable_tpr_reporting) {
> @@ -422,6 +431,8 @@ static Property apic_properties_common[] = {
> DEFINE_PROP_UINT8("version", APICCommonState, version, 0x14),
> DEFINE_PROP_BIT("vapic", APICCommonState, vapic_control,
> VAPIC_ENABLE_BIT,
> true),
> + DEFINE_PROP_BOOL("legacy-instance-id", APICCommonState,
> legacy_instance_id,
> + false),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> @@ -429,7 +440,6 @@ static void apic_common_class_init(ObjectClass *klass,
> void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->vmsd = &vmstate_apic_common;
> dc->reset = apic_reset_common;
> dc->props = apic_properties_common;
> dc->realize = apic_common_realize;
> diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
> index 52ca45d..a694322 100644
> --- a/include/hw/i386/apic_internal.h
> +++ b/include/hw/i386/apic_internal.h
> @@ -182,6 +182,7 @@ struct APICCommonState {
> uint32_t vapic_control;
> DeviceState *vapic;
> hwaddr vapic_paddr; /* note: persistence via kvmvapic */
> + bool legacy_instance_id;
> };
>
> typedef struct VAPICState {
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index 7e43b20..e2f0426 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -374,6 +374,11 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t
> *);
> .driver = TYPE_X86_CPU,\
> .property = "cpuid-0xb",\
> .value = "off",\
> + },\
> + {\
> + .driver = "apic",\
> + .property = "legacy-instance-id",\
> + .value = "on",\
> },
>
> #define PC_COMPAT_2_5 \
> --
> 2.7.0
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
