On Wed, Jun 08, 2016 at 04:06:52PM -0700, Jianjun Duan wrote: > In QOM(QEMU Object Model) migrated objects are identified with instance_id > which is calculated automatically using their path in the QOM composition > tree. For some objects, this path could change from source to target in > migration. To migrate such objects, we need to make sure the instance_id does > not change from source to target. We add a hook in DeviceClass to do > customized > instance_id calculation in such cases. > > As a result, in these cases compat will not be set in the concerned > SaveStateEntry. This will prevent the inconsistent idstr to be sent over in > migration. We could have set alias_id in a similar way. But that will be > overloading the purpose of alias_id. > > The first application will be setting instance_id for DRC using its unique > index. Doing this makes the instance_id of DRC to be consistent across > migration > and supports flexible management of DRC objects in migration. > > Signed-off-by: Jianjun Duan <du...@linux.vnet.ibm.com>
Ah, ok. I misunderstood your earlier comment - I didn't realise you
needed a change to the migration core in order to set the instance
ids.
That said, I think being able to customize the instance ids could give
better ability to future-proof migration data, at least for certain
buses and devices.
Juan, Dave, what do you think?
> ---
> include/hw/qdev-core.h | 6 ++++++
> migration/savevm.c | 20 ++++++++++++++++++--
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 1ce02b2..28173a7 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -139,6 +139,12 @@ typedef struct DeviceClass {
> qdev_initfn init; /* TODO remove, once users are converted to realize */
> qdev_event exit; /* TODO remove, once users are converted to unrealize */
> const char *bus_type;
> +
> + /* When this field is set, qemu will use it to get an unique instance_id
> + * instead of calculating an auto idstr and instanc_id for the relevant
> + * SaveStateEntry
> + */
> + int (*dev_get_instance_id)(DeviceState *dev);
> } DeviceClass;
>
> typedef struct NamedGPIOList NamedGPIOList;
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 6c21231..f7e7c68 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -494,6 +494,11 @@ int register_savevm_live(DeviceState *dev,
> void *opaque)
> {
> SaveStateEntry *se;
> + /* when it is a device and it provides a way to get instance_id,
> + * we will use it and skip setting idstr and compat.
> + */
> + bool flag = (dev != NULL) &&
> + (DEVICE_GET_CLASS(dev)->dev_get_instance_id != NULL);
>
> se = g_new0(SaveStateEntry, 1);
> se->version_id = version_id;
> @@ -506,7 +511,7 @@ int register_savevm_live(DeviceState *dev,
> se->is_ram = 1;
> }
>
> - if (dev) {
> + if (dev && !flag) {
> char *id = qdev_get_dev_path(dev);
> if (id) {
> pstrcpy(se->idstr, sizeof(se->idstr), id);
> @@ -522,6 +527,9 @@ int register_savevm_live(DeviceState *dev,
> }
> pstrcat(se->idstr, sizeof(se->idstr), idstr);
>
> + if (flag) {
> + instance_id = DEVICE_GET_CLASS(dev)->dev_get_instance_id(dev);
> + }
> if (instance_id == -1) {
> se->instance_id = calculate_new_instance_id(se->idstr);
> } else {
> @@ -579,6 +587,11 @@ int vmstate_register_with_alias_id(DeviceState *dev, int
> instance_id,
> int required_for_version)
> {
> SaveStateEntry *se;
> + /* when it is a device and it provides a way to get instance_id,
> + * we will use it and skip setting idstr and compat.
> + */
> + bool flag = (dev != NULL) &&
> + (DEVICE_GET_CLASS(dev)->dev_get_instance_id != NULL);
>
> /* If this triggers, alias support can be dropped for the vmsd. */
> assert(alias_id == -1 || required_for_version >=
> vmsd->minimum_version_id);
> @@ -590,7 +603,7 @@ int vmstate_register_with_alias_id(DeviceState *dev, int
> instance_id,
> se->vmsd = vmsd;
> se->alias_id = alias_id;
>
> - if (dev) {
> + if (dev && !flag) {
> char *id = qdev_get_dev_path(dev);
> if (id) {
> pstrcpy(se->idstr, sizeof(se->idstr), id);
> @@ -606,6 +619,9 @@ int vmstate_register_with_alias_id(DeviceState *dev, int
> instance_id,
> }
> pstrcat(se->idstr, sizeof(se->idstr), vmsd->name);
>
> + if (flag) {
> + instance_id = DEVICE_GET_CLASS(dev)->dev_get_instance_id(dev);
> + }
> if (instance_id == -1) {
> se->instance_id = calculate_new_instance_id(se->idstr);
> } else {
--
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
signature.asc
Description: PGP signature
