| Message ID | 1467795561-1007-3-git-send-email-bharata@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, 6 Jul 2016 14:29:18 +0530 Bharata B Rao <bharata@linux.vnet.ibm.com> wrote: > Add CPUState::migration_id and use that as instance_id in > vmstate_register() call. > > Introduce use-migration-id property that allows target machines to > optionally switch to using migration_id instead of cpu_index. > This will help allow successful migration in cases where holes are > introduced in cpu_index range after CPU hot removals. > > The default implementation of CPUClass:get_migration_id() continues > to return cpu_index. > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > exec.c | 5 +++-- > include/qom/cpu.h | 6 ++++++ > qom/cpu.c | 21 +++++++++++++++++++++ > 3 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index fb73910..a8afeda 100644 > --- a/exec.c > +++ b/exec.c > @@ -619,12 +619,13 @@ static void cpu_release_index(CPUState *cpu) > void cpu_vmstate_register(CPUState *cpu) > { > CPUClass *cc = CPU_GET_CLASS(cpu); > + int instance_id = cc->get_migration_id(cpu); > > if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > - vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, > cpu); > + vmstate_register(NULL, instance_id, &vmstate_cpu_common, > cpu); } > if (cc->vmsd != NULL) { > - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); > + vmstate_register(NULL, instance_id, cc->vmsd, cpu); > } > } > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 29ccf5c..6d00143 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -187,6 +187,7 @@ typedef struct CPUClass { > bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); > > void (*disas_set_info)(CPUState *cpu, disassemble_info *info); > + int (*get_migration_id)(CPUState *cpu); > } CPUClass; > > #ifdef HOST_WORDS_BIGENDIAN > @@ -273,6 +274,9 @@ struct qemu_work_item { > * @kvm_fd: vCPU file descriptor for KVM. > * @work_mutex: Lock to prevent multiple access to queued_work_*. > * @queued_work_first: First asynchronous work pending. > + * @migration_id: Use as instance_id argument in cpu > vmstate_register calls with this field get_migration_id() callback is redundant. You can drop get_migration_id() and assign value to migration_id in machine specific code /spapr/ppc/x86/.../ before realize is called. That should simplify this patch and its users as well. > + * @use_migration_id: Set to enforce the use of > CPUClass.get_migration_id() > + * over cpu_index during vmstate registration. > * > * State of one CPU core or thread. > */ > @@ -360,6 +364,8 @@ struct CPUState { > (absolute value) offset as small as possible. This reduces > code size, especially for hosts without large memory offsets. */ > uint32_t tcg_exit_req; > + int migration_id; > + bool use_migration_id; > }; > > QTAILQ_HEAD(CPUTailQ, CPUState); > diff --git a/qom/cpu.c b/qom/cpu.c > index 488ecc6..01cf136 100644 > --- a/qom/cpu.c > +++ b/qom/cpu.c > @@ -326,6 +326,17 @@ static void cpu_common_realizefn(DeviceState > *dev, Error **errp) } > } > > +static bool cpu_common_get_use_migration_id(Object *obj, Error > **errp) +{ > + return CPU(obj)->use_migration_id; > +} > + > +static void cpu_common_set_use_migration_id(Object *obj, bool value, > + Error **err) > +{ > + CPU(obj)->use_migration_id = value; > +} It's possible to create dumb property in a more compact way, see DEFINE_PROP_BOOL() > static void cpu_common_initfn(Object *obj) > { > CPUState *cpu = CPU(obj); > @@ -336,6 +347,10 @@ static void cpu_common_initfn(Object *obj) > qemu_mutex_init(&cpu->work_mutex); > QTAILQ_INIT(&cpu->breakpoints); > QTAILQ_INIT(&cpu->watchpoints); > + > + object_property_add_bool(obj, "use-migration-id", > + cpu_common_get_use_migration_id, > + cpu_common_set_use_migration_id, NULL); > } > > static void cpu_common_finalize(Object *obj) > @@ -348,6 +363,11 @@ static int64_t cpu_common_get_arch_id(CPUState > *cpu) return cpu->cpu_index; > } > > +static int cpu_common_get_migration_id(CPUState *cpu) > +{ > + return cpu->cpu_index; > +} > + > static void cpu_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc = DEVICE_CLASS(klass); > @@ -372,6 +392,7 @@ static void cpu_class_init(ObjectClass *klass, > void *data) k->cpu_exec_enter = cpu_common_noop; > k->cpu_exec_exit = cpu_common_noop; > k->cpu_exec_interrupt = cpu_common_exec_interrupt; > + k->get_migration_id = cpu_common_get_migration_id; > dc->realize = cpu_common_realizefn; > dc->unrealize = cpu_common_unrealizefn; > /*
On Wed, Jul 06, 2016 at 01:34:59PM +0200, Igor Mammedov wrote: > On Wed, 6 Jul 2016 14:29:18 +0530 > Bharata B Rao <bharata@linux.vnet.ibm.com> wrote: > > > Add CPUState::migration_id and use that as instance_id in > > vmstate_register() call. > > > > Introduce use-migration-id property that allows target machines to > > optionally switch to using migration_id instead of cpu_index. > > This will help allow successful migration in cases where holes are > > introduced in cpu_index range after CPU hot removals. > > > > The default implementation of CPUClass:get_migration_id() continues > > to return cpu_index. > > > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > --- > > exec.c | 5 +++-- > > include/qom/cpu.h | 6 ++++++ > > qom/cpu.c | 21 +++++++++++++++++++++ > > 3 files changed, 30 insertions(+), 2 deletions(-) > > > > diff --git a/exec.c b/exec.c > > index fb73910..a8afeda 100644 > > --- a/exec.c > > +++ b/exec.c > > @@ -619,12 +619,13 @@ static void cpu_release_index(CPUState *cpu) > > void cpu_vmstate_register(CPUState *cpu) > > { > > CPUClass *cc = CPU_GET_CLASS(cpu); > > + int instance_id = cc->get_migration_id(cpu); > > > > if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > > - vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, > > cpu); > > + vmstate_register(NULL, instance_id, &vmstate_cpu_common, > > cpu); } > > if (cc->vmsd != NULL) { > > - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); > > + vmstate_register(NULL, instance_id, cc->vmsd, cpu); > > } > > } > > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > index 29ccf5c..6d00143 100644 > > --- a/include/qom/cpu.h > > +++ b/include/qom/cpu.h > > @@ -187,6 +187,7 @@ typedef struct CPUClass { > > bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); > > > > void (*disas_set_info)(CPUState *cpu, disassemble_info *info); > > + int (*get_migration_id)(CPUState *cpu); > > } CPUClass; > > > > #ifdef HOST_WORDS_BIGENDIAN > > @@ -273,6 +274,9 @@ struct qemu_work_item { > > * @kvm_fd: vCPU file descriptor for KVM. > > * @work_mutex: Lock to prevent multiple access to queued_work_*. > > * @queued_work_first: First asynchronous work pending. > > + * @migration_id: Use as instance_id argument in cpu > > vmstate_register calls > with this field get_migration_id() callback is redundant. > You can drop get_migration_id() and assign value to migration_id in > machine specific code /spapr/ppc/x86/.../ before realize is called. > That should simplify this patch and its users as well. Let me give that a try and see how it looks. > > > + * @use_migration_id: Set to enforce the use of > > CPUClass.get_migration_id() > > + * over cpu_index during vmstate registration. > > * > > * State of one CPU core or thread. > > */ > > @@ -360,6 +364,8 @@ struct CPUState { > > (absolute value) offset as small as possible. This reduces > > code size, especially for hosts without large memory offsets. */ > > uint32_t tcg_exit_req; > > + int migration_id; > > + bool use_migration_id; > > }; > > > > QTAILQ_HEAD(CPUTailQ, CPUState); > > diff --git a/qom/cpu.c b/qom/cpu.c > > index 488ecc6..01cf136 100644 > > --- a/qom/cpu.c > > +++ b/qom/cpu.c > > @@ -326,6 +326,17 @@ static void cpu_common_realizefn(DeviceState > > *dev, Error **errp) } > > } > > > > +static bool cpu_common_get_use_migration_id(Object *obj, Error > > **errp) +{ > > + return CPU(obj)->use_migration_id; > > +} > > + > > +static void cpu_common_set_use_migration_id(Object *obj, bool value, > > + Error **err) > > +{ > > + CPU(obj)->use_migration_id = value; > > +} > It's possible to create dumb property in a more compact way, > see DEFINE_PROP_BOOL() I stated with it and defined dc->props, but then felt it is a bit too much to define a list just for one property. But if that preferable let me switch back to it. Regards, Bharata.
On Wed, 6 Jul 2016 19:48:03 +0530 Bharata B Rao <bharata@linux.vnet.ibm.com> wrote: > On Wed, Jul 06, 2016 at 01:34:59PM +0200, Igor Mammedov wrote: > > On Wed, 6 Jul 2016 14:29:18 +0530 > > Bharata B Rao <bharata@linux.vnet.ibm.com> wrote: > > > > > Add CPUState::migration_id and use that as instance_id in > > > vmstate_register() call. > > > > > > Introduce use-migration-id property that allows target machines to > > > optionally switch to using migration_id instead of cpu_index. > > > This will help allow successful migration in cases where holes are > > > introduced in cpu_index range after CPU hot removals. > > > > > > The default implementation of CPUClass:get_migration_id() > > > continues to return cpu_index. > > > > > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > > > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > > > --- > > > exec.c | 5 +++-- > > > include/qom/cpu.h | 6 ++++++ > > > qom/cpu.c | 21 +++++++++++++++++++++ > > > 3 files changed, 30 insertions(+), 2 deletions(-) > > > > > > diff --git a/exec.c b/exec.c > > > index fb73910..a8afeda 100644 > > > --- a/exec.c > > > +++ b/exec.c > > > @@ -619,12 +619,13 @@ static void cpu_release_index(CPUState *cpu) > > > void cpu_vmstate_register(CPUState *cpu) > > > { > > > CPUClass *cc = CPU_GET_CLASS(cpu); > > > + int instance_id = cc->get_migration_id(cpu); > > > > > > if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > > > - vmstate_register(NULL, cpu->cpu_index, > > > &vmstate_cpu_common, cpu); > > > + vmstate_register(NULL, instance_id, &vmstate_cpu_common, > > > cpu); } > > > if (cc->vmsd != NULL) { > > > - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); > > > + vmstate_register(NULL, instance_id, cc->vmsd, cpu); > > > } > > > } > > > > > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > > > index 29ccf5c..6d00143 100644 > > > --- a/include/qom/cpu.h > > > +++ b/include/qom/cpu.h > > > @@ -187,6 +187,7 @@ typedef struct CPUClass { > > > bool (*cpu_exec_interrupt)(CPUState *cpu, int > > > interrupt_request); > > > void (*disas_set_info)(CPUState *cpu, disassemble_info > > > *info); > > > + int (*get_migration_id)(CPUState *cpu); > > > } CPUClass; > > > > > > #ifdef HOST_WORDS_BIGENDIAN > > > @@ -273,6 +274,9 @@ struct qemu_work_item { > > > * @kvm_fd: vCPU file descriptor for KVM. > > > * @work_mutex: Lock to prevent multiple access to queued_work_*. > > > * @queued_work_first: First asynchronous work pending. > > > + * @migration_id: Use as instance_id argument in cpu > > > vmstate_register calls > > with this field get_migration_id() callback is redundant. > > You can drop get_migration_id() and assign value to migration_id in > > machine specific code /spapr/ppc/x86/.../ before realize is called. > > That should simplify this patch and its users as well. > > Let me give that a try and see how it looks. > > > > > > + * @use_migration_id: Set to enforce the use of > > > CPUClass.get_migration_id() > > > + * over cpu_index during vmstate registration. > > > * > > > * State of one CPU core or thread. > > > */ > > > @@ -360,6 +364,8 @@ struct CPUState { > > > (absolute value) offset as small as possible. This > > > reduces code size, especially for hosts without large memory > > > offsets. */ uint32_t tcg_exit_req; > > > + int migration_id; > > > + bool use_migration_id; > > > }; > > > > > > QTAILQ_HEAD(CPUTailQ, CPUState); > > > diff --git a/qom/cpu.c b/qom/cpu.c > > > index 488ecc6..01cf136 100644 > > > --- a/qom/cpu.c > > > +++ b/qom/cpu.c > > > @@ -326,6 +326,17 @@ static void cpu_common_realizefn(DeviceState > > > *dev, Error **errp) } > > > } > > > > > > +static bool cpu_common_get_use_migration_id(Object *obj, Error > > > **errp) +{ > > > + return CPU(obj)->use_migration_id; > > > +} > > > + > > > +static void cpu_common_set_use_migration_id(Object *obj, bool > > > value, > > > + Error **err) > > > +{ > > > + CPU(obj)->use_migration_id = value; > > > +} > > It's possible to create dumb property in a more compact way, > > see DEFINE_PROP_BOOL() > > I stated with it and defined dc->props, but then felt it is a bit too > much to define a list just for one property. But if that preferable > let me switch back to it. static prop is less code to maintain and not scattered all around file, so it's more preferred if it can do the job. > > Regards, > Bharata. > >
On Wed, Jul 06, 2016 at 02:29:18PM +0530, Bharata B Rao wrote: > Add CPUState::migration_id and use that as instance_id in > vmstate_register() call. > > Introduce use-migration-id property that allows target machines to > optionally switch to using migration_id instead of cpu_index. > This will help allow successful migration in cases where holes are > introduced in cpu_index range after CPU hot removals. > > The default implementation of CPUClass:get_migration_id() continues > to return cpu_index. > > Suggested-by: Igor Mammedov <imammedo@redhat.com> > Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> > --- > exec.c | 5 +++-- > include/qom/cpu.h | 6 ++++++ > qom/cpu.c | 21 +++++++++++++++++++++ > 3 files changed, 30 insertions(+), 2 deletions(-) > > diff --git a/exec.c b/exec.c > index fb73910..a8afeda 100644 > --- a/exec.c > +++ b/exec.c > @@ -619,12 +619,13 @@ static void cpu_release_index(CPUState *cpu) > void cpu_vmstate_register(CPUState *cpu) > { > CPUClass *cc = CPU_GET_CLASS(cpu); > + int instance_id = cc->get_migration_id(cpu); > > if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { > - vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu); > + vmstate_register(NULL, instance_id, &vmstate_cpu_common, cpu); > } > if (cc->vmsd != NULL) { > - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); > + vmstate_register(NULL, instance_id, cc->vmsd, cpu); > } > } > > diff --git a/include/qom/cpu.h b/include/qom/cpu.h > index 29ccf5c..6d00143 100644 > --- a/include/qom/cpu.h > +++ b/include/qom/cpu.h > @@ -187,6 +187,7 @@ typedef struct CPUClass { > bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); > > void (*disas_set_info)(CPUState *cpu, disassemble_info *info); > + int (*get_migration_id)(CPUState *cpu); > } CPUClass; > > #ifdef HOST_WORDS_BIGENDIAN > @@ -273,6 +274,9 @@ struct qemu_work_item { > * @kvm_fd: vCPU file descriptor for KVM. > * @work_mutex: Lock to prevent multiple access to queued_work_*. > * @queued_work_first: First asynchronous work pending. > + * @migration_id: Use as instance_id argument in cpu vmstate_register calls > + * @use_migration_id: Set to enforce the use of CPUClass.get_migration_id() > + * over cpu_index during vmstate registration. I don't really understand the purpose of the use-migration-id bool. Can't you just leave the migration-id as cpu_index by default if you don't want a different migration id. AFAICT this flag is never tested.
diff --git a/exec.c b/exec.c index fb73910..a8afeda 100644 --- a/exec.c +++ b/exec.c @@ -619,12 +619,13 @@ static void cpu_release_index(CPUState *cpu) void cpu_vmstate_register(CPUState *cpu) { CPUClass *cc = CPU_GET_CLASS(cpu); + int instance_id = cc->get_migration_id(cpu); if (qdev_get_vmsd(DEVICE(cpu)) == NULL) { - vmstate_register(NULL, cpu->cpu_index, &vmstate_cpu_common, cpu); + vmstate_register(NULL, instance_id, &vmstate_cpu_common, cpu); } if (cc->vmsd != NULL) { - vmstate_register(NULL, cpu->cpu_index, cc->vmsd, cpu); + vmstate_register(NULL, instance_id, cc->vmsd, cpu); } } diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 29ccf5c..6d00143 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -187,6 +187,7 @@ typedef struct CPUClass { bool (*cpu_exec_interrupt)(CPUState *cpu, int interrupt_request); void (*disas_set_info)(CPUState *cpu, disassemble_info *info); + int (*get_migration_id)(CPUState *cpu); } CPUClass; #ifdef HOST_WORDS_BIGENDIAN @@ -273,6 +274,9 @@ struct qemu_work_item { * @kvm_fd: vCPU file descriptor for KVM. * @work_mutex: Lock to prevent multiple access to queued_work_*. * @queued_work_first: First asynchronous work pending. + * @migration_id: Use as instance_id argument in cpu vmstate_register calls + * @use_migration_id: Set to enforce the use of CPUClass.get_migration_id() + * over cpu_index during vmstate registration. * * State of one CPU core or thread. */ @@ -360,6 +364,8 @@ struct CPUState { (absolute value) offset as small as possible. This reduces code size, especially for hosts without large memory offsets. */ uint32_t tcg_exit_req; + int migration_id; + bool use_migration_id; }; QTAILQ_HEAD(CPUTailQ, CPUState); diff --git a/qom/cpu.c b/qom/cpu.c index 488ecc6..01cf136 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -326,6 +326,17 @@ static void cpu_common_realizefn(DeviceState *dev, Error **errp) } } +static bool cpu_common_get_use_migration_id(Object *obj, Error **errp) +{ + return CPU(obj)->use_migration_id; +} + +static void cpu_common_set_use_migration_id(Object *obj, bool value, + Error **err) +{ + CPU(obj)->use_migration_id = value; +} + static void cpu_common_initfn(Object *obj) { CPUState *cpu = CPU(obj); @@ -336,6 +347,10 @@ static void cpu_common_initfn(Object *obj) qemu_mutex_init(&cpu->work_mutex); QTAILQ_INIT(&cpu->breakpoints); QTAILQ_INIT(&cpu->watchpoints); + + object_property_add_bool(obj, "use-migration-id", + cpu_common_get_use_migration_id, + cpu_common_set_use_migration_id, NULL); } static void cpu_common_finalize(Object *obj) @@ -348,6 +363,11 @@ static int64_t cpu_common_get_arch_id(CPUState *cpu) return cpu->cpu_index; } +static int cpu_common_get_migration_id(CPUState *cpu) +{ + return cpu->cpu_index; +} + static void cpu_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -372,6 +392,7 @@ static void cpu_class_init(ObjectClass *klass, void *data) k->cpu_exec_enter = cpu_common_noop; k->cpu_exec_exit = cpu_common_noop; k->cpu_exec_interrupt = cpu_common_exec_interrupt; + k->get_migration_id = cpu_common_get_migration_id; dc->realize = cpu_common_realizefn; dc->unrealize = cpu_common_unrealizefn; /*
Add CPUState::migration_id and use that as instance_id in vmstate_register() call. Introduce use-migration-id property that allows target machines to optionally switch to using migration_id instead of cpu_index. This will help allow successful migration in cases where holes are introduced in cpu_index range after CPU hot removals. The default implementation of CPUClass:get_migration_id() continues to return cpu_index. Suggested-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Bharata B Rao <bharata@linux.vnet.ibm.com> --- exec.c | 5 +++-- include/qom/cpu.h | 6 ++++++ qom/cpu.c | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-)