On Tue, 12 Jul 2016 12:36:17 +0530
Bharata B Rao <bhar...@linux.vnet.ibm.com> wrote:
> On Mon, Jul 11, 2016 at 03:42:29PM +0200, Igor Mammedov wrote:
> > this approach i I preffer as it uses less per machine migration glue
> > and follows typical compat pattern for devices
> >
> > Signed-off-by: Igor Mammedov <imamm...@redhat.com>
> > Signed-off-by: Bharata B Rao <bhar...@linux.vnet.ibm.com>
> > ---
> > exec.c | 7 +++++--
> > hw/i386/pc.c | 10 +++++++---
> > include/hw/compat.h | 6 +++++-
> > include/qom/cpu.h | 3 +++
> > qom/cpu.c | 13 +++++++++++++
> > 5 files changed, 33 insertions(+), 6 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 0122ef7..79e1dcb 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -670,6 +670,7 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
> > {
> > CPUClass *cc = CPU_GET_CLASS(cpu);
> > Error *local_err = NULL;
> > + int migration_id;
> >
> > cpu->as = NULL;
> > cpu->num_ases = 0;
> > @@ -708,11 +709,13 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
> > (void) cc;
> > cpu_list_unlock();
> > #else
> > + migration_id = cc->use_migration_id > 0 ?
> > + cpu->migration_id : cpu->cpu_index;
>
> cc->use_migration_id is always -1 here (See explanation below)
>
> > if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> > - vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu);
> > + vmstate_register(NULL, migration_id, &vmstate_cpu_common, cpu);
> > }
> > if (cc->vmsd != NULL) {
> > - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu);
> > + vmstate_register(NULL, migration_id, cc->vmsd, cpu);
> > }
> > #endif
> > }
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index cd1745e..f041279 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -1040,9 +1040,10 @@ void pc_acpi_smi_interrupt(void *opaque, int irq,
> > int level)
> > }
> >
> > static X86CPU *pc_new_cpu(const char *cpu_model, int64_t apic_id,
> > - Error **errp)
> > + Error **errp, int migration_id)
> > {
> > X86CPU *cpu = NULL;
> > + CPUState *cs = CPU(cpu);
> > Error *local_err = NULL;
> >
> > cpu = cpu_x86_create(cpu_model, &local_err);
>
> Assignment to cs should move here after cpu_x86_create.
thanks, I'll fix it, though there won't be this hunk in at all
in the end PC version in top of cpu-hotplug series.
>
> > @@ -1050,6 +1051,7 @@ static X86CPU *pc_new_cpu(const char *cpu_model,
> > int64_t apic_id,
> > goto out;
> > }
> >
> > + cs->migration_id = migration_id;
> > object_property_set_int(OBJECT(cpu), apic_id, "apic-id", &local_err);
> > object_property_set_bool(OBJECT(cpu), true, "realized", &local_err);
> >
> > @@ -1093,7 +1095,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
> > return;
> > }
> >
> > - cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err);
> > + cpu = pc_new_cpu(machine->cpu_model, apic_id, &local_err, id);
> > if (local_err) {
> > error_propagate(errp, local_err);
> > return;
> > @@ -1137,7 +1139,7 @@ void pc_cpus_init(PCMachineState *pcms)
> > pcms->possible_cpus->len++;
> > if (i < smp_cpus) {
> > cpu = pc_new_cpu(machine->cpu_model,
> > x86_cpu_apic_id_from_index(i),
> > - &error_fatal);
> > + &error_fatal, i);
> > pcms->possible_cpus->cpus[i].cpu = CPU(cpu);
> > object_unref(OBJECT(cpu));
> > }
> > @@ -2012,7 +2014,9 @@ static void pc_machine_class_init(ObjectClass *oc,
> > void *data)
> > PCMachineClass *pcmc = PC_MACHINE_CLASS(oc);
> > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
> > NMIClass *nc = NMI_CLASS(oc);
> > + CPUClass *cc = CPU_CLASS(object_class_by_name(TYPE_CPU));
> >
> > + cc->use_migration_id = true;
>
> It is not clear as to how setting cc->use_migration_id here in machine class
> init would affect all CPUs. What I am observing is that the cc that we
> set here is different from the cc that we get via GET_CPU_CLASS(cpu) in
> cpu_exec_init(). Hence use_migration_id is never set for any CPUs that
> get created.
It look like inherited types are initialized before this is called,
I'll try find solution and post fixed patch shortly.
>
> Regards,
> Bharata.
>
