[QEMU,RFC,v4,1/6] migration: alternative way to set instance_id in SaveStateEntry

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 <duanj@linux.vnet.ibm.com>
---
 include/hw/qdev-core.h |  6 ++++++
 migration/savevm.c     | 20 ++++++++++++++++++--
 2 files changed, 24 insertions(+), 2 deletions(-)

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 <duanj@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 {