Define vmstate_register_named which takes the instance name as its first
parameter, instead of generating the name from VMStateIf of the Object.
This will be needed to register objects that are not Objects.  Pass the
new name parameter to vmstate_register_with_alias_id.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 hw/core/qdev.c              |  1 +
 hw/intc/apic_common.c       |  2 +-
 include/migration/vmstate.h | 19 +++++++++++++++++--
 migration/savevm.c          | 35 ++++++++++++++++++++++++-----------
 migration/trace-events      |  1 +
 stubs/vmstate.c             |  1 +
 6 files changed, 45 insertions(+), 14 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 00efaf1..b352e8a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -535,6 +535,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
                                                qdev_get_vmsd(dev), dev,
                                                dev->instance_id_alias,
                                                dev->alias_required_for_version,
+                                               NULL,
                                                &local_err) < 0) {
                 goto post_realize_fail;
             }
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index d8fc1e2..d6cd293 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -298,7 +298,7 @@ static void apic_common_realize(DeviceState *dev, Error **errp)
         instance_id = VMSTATE_INSTANCE_ID_ANY;
     }
     vmstate_register_with_alias_id(NULL, instance_id, &vmstate_apic_common,
-                                   s, -1, 0, NULL);
+                                   s, -1, 0, NULL, NULL);
 
     /* APIC LDR in x2APIC mode */
     s->extended_log_dest = ((s->initial_apic_id >> 4) << 16) |
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index bb885d9..22aa3c6 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -1234,6 +1234,7 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
                                    const VMStateDescription *vmsd,
                                    void *base, int alias_id,
                                    int required_for_version,
+                                   const char *instance_name,
                                    Error **errp);
 
 /**
@@ -1250,7 +1251,7 @@ static inline int vmstate_register(VMStateIf *obj, int instance_id,
                                    void *opaque)
 {
     return vmstate_register_with_alias_id(obj, instance_id, vmsd,
-                                          opaque, -1, 0, NULL);
+                                          opaque, -1, 0, NULL, NULL);
 }
 
 /**
@@ -1278,7 +1279,21 @@ static inline int vmstate_register_any(VMStateIf *obj,
                                        void *opaque)
 {
     return vmstate_register_with_alias_id(obj, VMSTATE_INSTANCE_ID_ANY, vmsd,
-                                          opaque, -1, 0, NULL);
+                                          opaque, -1, 0, NULL, NULL);
+}
+
+/**
+ * vmstate_register_named() - pass an instance_name explicitly instead of
+ * implicitly via VMStateIf get_id().  Needed to register a instance-specific
+ * VMSD for objects that are not Objects.
+ */
+static inline int vmstate_register_named(const char *instance_name,
+                                         int instance_id,
+                                         const VMStateDescription *vmsd,
+                                         void *opaque)
+{
+    return vmstate_register_with_alias_id(NULL, instance_id, vmsd, opaque,
+                                          -1, 0, instance_name, NULL);
 }
 
 void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
diff --git a/migration/savevm.c b/migration/savevm.c
index 9b1a335..86b4c87 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -889,10 +889,20 @@ int vmstate_replace_hack_for_ppc(VMStateIf *obj, int instance_id,
     return vmstate_register(obj, instance_id, vmsd, opaque);
 }
 
+static bool make_new_idstr(VMStateId idstr, const char *id, Error **errp)
+{
+    if (snprintf(idstr, sizeof(VMStateId), "%s/", id) >= sizeof(VMStateId)) {
+        error_setg(errp, "Path too long for VMState (%s)", id);
+        return false;
+    }
+    return true;
+}
+
 int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
                                    const VMStateDescription *vmsd,
                                    void *opaque, int alias_id,
                                    int required_for_version,
+                                   const char *instance_name,
                                    Error **errp)
 {
     SaveStateEntry *se;
@@ -907,19 +917,17 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
     se->vmsd = vmsd;
     se->alias_id = alias_id;
 
-    if (obj) {
-        char *id = vmstate_if_get_id(obj);
+    if (instance_name) {
+        if (!make_new_idstr(se->idstr, instance_name, errp)) {
+            goto err;
+        }
+
+    } else if (obj) {
+        g_autofree char *id = vmstate_if_get_id(obj);
         if (id) {
-            if (snprintf(se->idstr, sizeof(se->idstr), "%s/", id) >=
-                sizeof(se->idstr)) {
-                error_setg(errp, "Path too long for VMState (%s)", id);
-                g_free(id);
-                g_free(se);
-
-                return -1;
+            if (!make_new_idstr(se->idstr, id, errp)) {
+                goto err;
             }
-            g_free(id);
-
             se->compat = g_new0(CompatEntry, 1);
             pstrcpy(se->compat->idstr, sizeof(se->compat->idstr), vmsd->name);
             se->compat->instance_id = instance_id == VMSTATE_INSTANCE_ID_ANY ?
@@ -941,7 +949,12 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
     }
     assert(!se->compat || se->instance_id == 0);
     savevm_state_handler_insert(se);
+    trace_vmstate_register(se->idstr, se->instance_id, (void *)vmsd, opaque);
     return 0;
+
+err:
+    g_free(se);
+    return -1;
 }
 
 void vmstate_unregister(VMStateIf *obj, const VMStateDescription *vmsd,
diff --git a/migration/trace-events b/migration/trace-events
index f0e1cb8..8647147 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -53,6 +53,7 @@ vmstate_downtime_checkpoint(const char *checkpoint) "%s"
 postcopy_pause_incoming(void) ""
 postcopy_pause_incoming_continued(void) ""
 postcopy_page_req_sync(void *host_addr) "sync page req %p"
+vmstate_register(const char *idstr, int id, void *vmsd, void *opaque) "%s, %d, vmsd %p, opaque %p"
 
 # vmstate.c
 vmstate_load_field_error(const char *field, int ret) "field \"%s\" load failed, ret = %d"
diff --git a/stubs/vmstate.c b/stubs/vmstate.c
index 8513d92..d67506e 100644
--- a/stubs/vmstate.c
+++ b/stubs/vmstate.c
@@ -6,6 +6,7 @@ int vmstate_register_with_alias_id(VMStateIf *obj,
                                    const VMStateDescription *vmsd,
                                    void *base, int alias_id,
                                    int required_for_version,
+                                   const char *instance_name,
                                    Error **errp)
 {
     return 0;
-- 
1.8.3.1

Steve Sistare <steven.sistare@oracle.com> writes:

> Define vmstate_register_named which takes the instance name as its first
> parameter, instead of generating the name from VMStateIf of the Object.
> This will be needed to register objects that are not Objects.  Pass the
> new name parameter to vmstate_register_with_alias_id.
>
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>

Reviewed-by: Fabiano Rosas <farosas@suse.de>

Fabiano Rosas <farosas@suse.de> writes:

> Steve Sistare <steven.sistare@oracle.com> writes:
>
>> Define vmstate_register_named which takes the instance name as its first
>> parameter, instead of generating the name from VMStateIf of the Object.
>> This will be needed to register objects that are not Objects.  Pass the
>> new name parameter to vmstate_register_with_alias_id.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>
> Reviewed-by: Fabiano Rosas <farosas@suse.de>

Actually, can't we define a wrapper type just for this purpose? For
example, looking at dbus-vmstate.c:

static void dbus_vmstate_class_init(ObjectClass *oc, void *data)
{
...
    VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);

    vc->get_id = dbus_vmstate_get_id;
...
}

static const TypeInfo dbus_vmstate_info = {
    .name = TYPE_DBUS_VMSTATE,
    .parent = TYPE_OBJECT,
    .instance_size = sizeof(DBusVMState),
    .instance_finalize = dbus_vmstate_finalize,
    .class_init = dbus_vmstate_class_init,
    .interfaces = (InterfaceInfo[]) {
        { TYPE_USER_CREATABLE },   // without this one
        { TYPE_VMSTATE_IF },
        { }
    }
};

static void register_types(void)
{
    type_register_static(&dbus_vmstate_info);
}
type_init(register_types);

On 5/9/2024 10:32 AM, Fabiano Rosas wrote:
> Fabiano Rosas <farosas@suse.de> writes:
> 
>> Steve Sistare <steven.sistare@oracle.com> writes:
>>
>>> Define vmstate_register_named which takes the instance name as its first
>>> parameter, instead of generating the name from VMStateIf of the Object.
>>> This will be needed to register objects that are not Objects.  Pass the
>>> new name parameter to vmstate_register_with_alias_id.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>
>> Reviewed-by: Fabiano Rosas <farosas@suse.de>
> 
> Actually, can't we define a wrapper type just for this purpose? For
> example, looking at dbus-vmstate.c:

One would need to provide a separate wrapper for each struct to be registered
as vmstate.  This patch set only has RAMBlock, but there are more coming in
my next patch sets.  vmstate_register_named avoids adding such boilerplate,
and makes it easier to add more cpr state in the future.

- Steve

> static void dbus_vmstate_class_init(ObjectClass *oc, void *data)
> {
> ...
>      VMStateIfClass *vc = VMSTATE_IF_CLASS(oc);
> 
>      vc->get_id = dbus_vmstate_get_id;
> ...
> }
> 
> static const TypeInfo dbus_vmstate_info = {
>      .name = TYPE_DBUS_VMSTATE,
>      .parent = TYPE_OBJECT,
>      .instance_size = sizeof(DBusVMState),
>      .instance_finalize = dbus_vmstate_finalize,
>      .class_init = dbus_vmstate_class_init,
>      .interfaces = (InterfaceInfo[]) {
>          { TYPE_USER_CREATABLE },   // without this one
>          { TYPE_VMSTATE_IF },
>          { }
>      }
> };
> 
> static void register_types(void)
> {
>      type_register_static(&dbus_vmstate_info);
> }
> type_init(register_types);