The ICPState objects are currently registered to vmstate as qdev objects.
Their instance ids are hence computed automatically in the migration code,
and thus depends on the order the CPU cores were plugged.
If the destination had its CPU cores plugged in a different order than the
source, then ICPState objects will have different instance_ids and load
the wrong state.
Since CPU objects have a reliable cpu_index which is already used as
instance_id in vmstate, let's use it for ICPState as well.
Please note that this doesn't break migration. Older machine types used to
allocate and realize all ICPState objects at machine init time, for the whole
lifetime of the machine. The qdev instance ids are thus 0,1,2... nr_servers
and happen to map to the vCPU indexes.
Signed-off-by: Greg Kurz <groug@kaod.org>
---
v5: - explain why it doesn't break migration in the changelog
---
hw/intc/xics.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/intc/xics.c b/hw/intc/xics.c
index 7ccfb53c55a0..faa5c631f655 100644
--- a/hw/intc/xics.c
+++ b/hw/intc/xics.c
@@ -344,10 +344,14 @@ static void icp_realize(DeviceState *dev, Error **errp)
}
qemu_register_reset(icp_reset, dev);
+ vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
}
static void icp_unrealize(DeviceState *dev, Error **errp)
{
+ ICPState *icp = ICP(dev);
+
+ vmstate_unregister(NULL, &vmstate_icp_server, icp);
qemu_unregister_reset(icp_reset, dev);
}
@@ -355,7 +359,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
- dc->vmsd = &vmstate_icp_server;
dc->realize = icp_realize;
dc->unrealize = icp_unrealize;
}
On 14/06/2017 15:29, Greg Kurz wrote:
> The ICPState objects are currently registered to vmstate as qdev objects.
> Their instance ids are hence computed automatically in the migration code,
> and thus depends on the order the CPU cores were plugged.
>
> If the destination had its CPU cores plugged in a different order than the
> source, then ICPState objects will have different instance_ids and load
> the wrong state.
>
> Since CPU objects have a reliable cpu_index which is already used as
> instance_id in vmstate, let's use it for ICPState as well.
>
> Please note that this doesn't break migration. Older machine types used to
> allocate and realize all ICPState objects at machine init time, for the whole
> lifetime of the machine. The qdev instance ids are thus 0,1,2... nr_servers
> and happen to map to the vCPU indexes.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
CC: dgilbert
> ---
> v5: - explain why it doesn't break migration in the changelog
> ---
> hw/intc/xics.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 7ccfb53c55a0..faa5c631f655 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -344,10 +344,14 @@ static void icp_realize(DeviceState *dev, Error **errp)
> }
>
> qemu_register_reset(icp_reset, dev);
> + vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
> }
>
> static void icp_unrealize(DeviceState *dev, Error **errp)
> {
> + ICPState *icp = ICP(dev);
> +
> + vmstate_unregister(NULL, &vmstate_icp_server, icp);
> qemu_unregister_reset(icp_reset, dev);
> }
>
> @@ -355,7 +359,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->vmsd = &vmstate_icp_server;
> dc->realize = icp_realize;
> dc->unrealize = icp_unrealize;
> }
>
On Wed, 14 Jun 2017 16:04:12 +0200
Laurent Vivier <lvivier@redhat.com> wrote:
> On 14/06/2017 15:29, Greg Kurz wrote:
> > The ICPState objects are currently registered to vmstate as qdev objects.
> > Their instance ids are hence computed automatically in the migration code,
> > and thus depends on the order the CPU cores were plugged.
> >
> > If the destination had its CPU cores plugged in a different order than the
> > source, then ICPState objects will have different instance_ids and load
> > the wrong state.
> >
> > Since CPU objects have a reliable cpu_index which is already used as
> > instance_id in vmstate, let's use it for ICPState as well.
> >
> > Please note that this doesn't break migration. Older machine types used to
> > allocate and realize all ICPState objects at machine init time, for the whole
> > lifetime of the machine. The qdev instance ids are thus 0,1,2... nr_servers
> > and happen to map to the vCPU indexes.
> >
> > Signed-off-by: Greg Kurz <groug@kaod.org>
>
> Reviewed-by: Laurent Vivier <lvivier@redhat.com>
>
> CC: dgilbert
>
Oops, I had planned to Cc: dgilbert and I fumbled just before the
goal... of course. :P
Thanks Laurent !
> > ---
> > v5: - explain why it doesn't break migration in the changelog
> > ---
> > hw/intc/xics.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> > index 7ccfb53c55a0..faa5c631f655 100644
> > --- a/hw/intc/xics.c
> > +++ b/hw/intc/xics.c
> > @@ -344,10 +344,14 @@ static void icp_realize(DeviceState *dev, Error **errp)
> > }
> >
> > qemu_register_reset(icp_reset, dev);
> > + vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
> > }
> >
> > static void icp_unrealize(DeviceState *dev, Error **errp)
> > {
> > + ICPState *icp = ICP(dev);
> > +
> > + vmstate_unregister(NULL, &vmstate_icp_server, icp);
> > qemu_unregister_reset(icp_reset, dev);
> > }
> >
> > @@ -355,7 +359,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *dc = DEVICE_CLASS(klass);
> >
> > - dc->vmsd = &vmstate_icp_server;
> > dc->realize = icp_realize;
> > dc->unrealize = icp_unrealize;
> > }
> >
>
On Wed, Jun 14, 2017 at 03:29:10PM +0200, Greg Kurz wrote:
> The ICPState objects are currently registered to vmstate as qdev objects.
> Their instance ids are hence computed automatically in the migration code,
> and thus depends on the order the CPU cores were plugged.
>
> If the destination had its CPU cores plugged in a different order than the
> source, then ICPState objects will have different instance_ids and load
> the wrong state.
>
> Since CPU objects have a reliable cpu_index which is already used as
> instance_id in vmstate, let's use it for ICPState as well.
>
> Please note that this doesn't break migration. Older machine types used to
> allocate and realize all ICPState objects at machine init time, for the whole
> lifetime of the machine. The qdev instance ids are thus 0,1,2... nr_servers
> and happen to map to the vCPU indexes.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
Applied to ppc-for-2.10.
> ---
> v5: - explain why it doesn't break migration in the changelog
> ---
> hw/intc/xics.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/xics.c b/hw/intc/xics.c
> index 7ccfb53c55a0..faa5c631f655 100644
> --- a/hw/intc/xics.c
> +++ b/hw/intc/xics.c
> @@ -344,10 +344,14 @@ static void icp_realize(DeviceState *dev, Error **errp)
> }
>
> qemu_register_reset(icp_reset, dev);
> + vmstate_register(NULL, icp->cs->cpu_index, &vmstate_icp_server, icp);
> }
>
> static void icp_unrealize(DeviceState *dev, Error **errp)
> {
> + ICPState *icp = ICP(dev);
> +
> + vmstate_unregister(NULL, &vmstate_icp_server, icp);
> qemu_unregister_reset(icp_reset, dev);
> }
>
> @@ -355,7 +359,6 @@ static void icp_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(klass);
>
> - dc->vmsd = &vmstate_icp_server;
> dc->realize = icp_realize;
> dc->unrealize = icp_unrealize;
> }
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
© 2016 - 2026 Red Hat, Inc.