[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 15/19] apic: use apic_id as apic's migration
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3 15/19] apic: use apic_id as apic's migration instance_id |
Date: |
Mon, 11 Jul 2016 18:21:20 +0100 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
* Igor Mammedov (address@hidden) 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 <address@hidden>
> Signed-off-by: Igor Mammedov <address@hidden>
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 <address@hidden>
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 / address@hidden / Manchester, UK
- [Qemu-devel] [PATCH v3 10/19] pc: register created initial and hotpluged CPUs in one place pc_cpu_plug(), (continued)
- [Qemu-devel] [PATCH v3 11/19] pc: cpu: allow device_add to be used with x86 cpu, Igor Mammedov, 2016/07/06
- [Qemu-devel] [PATCH v3 12/19] apic: move MAX_APICS check to 'apic' class, Igor Mammedov, 2016/07/06
- [Qemu-devel] [PATCH v3 13/19] apic: drop APICCommonState.idx and use APIC ID as index in local_apics[], Igor Mammedov, 2016/07/06
- [Qemu-devel] [PATCH v3 14/19] (kvm)apic: add unrealize callbacks, Igor Mammedov, 2016/07/06
- [Qemu-devel] [PATCH v3 15/19] apic: use apic_id as apic's migration instance_id, Igor Mammedov, 2016/07/06
- Re: [Qemu-devel] [PATCH v3 15/19] apic: use apic_id as apic's migration instance_id,
Dr. David Alan Gilbert <=
- [Qemu-devel] [PATCH v3 16/19] target-i386: cpu: do not ignore error and fix apic parent, Igor Mammedov, 2016/07/06
- [Qemu-devel] [PATCH v3 17/19] target-i386: fix apic object leak when CPU is deleted, Igor Mammedov, 2016/07/06
Re: [Qemu-devel] [PATCH v3 17/19] target-i386: fix apic object leak when CPU is deleted, Bandan Das, 2016/07/13
[Qemu-devel] [PATCH v3 18/19] target-i386: add x86_cpu_unrealizefn(), Igor Mammedov, 2016/07/06