Re: [PATCH v2 00/10] migration/qom: Remove TYPE_DEVICE dependency on migration object

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> CI: https://gitlab.com/peterx/qemu/-/pipelines/2588717620
>     (two irrelevant failures due to no runner)
>
> rfc: https://lore.kernel.org/r/20251209162857.857593-1-peterx@redhat.com
> v1:  https://lore.kernel.org/r/20260604231118.1584889-1-peterx@redhat.com
>
> This v2 is a full rewrite of v1, changelog doesn't apply.
>
> This version majorly addressed the concern in v1 having object API exported
> in qdev files. Instead of trying to expose qdev properties, this patchset
> switched migration object to use the object property API to add properties.
>
> Not all QOM facilities are ready for it, there're a few prior patches to
> improve QOM API to achieve it.
>
> The major concept that this series introduces that does not exist
> previously is the default value mechanisms for object properties (rather
> than object class properties).  Here migration object will rely on that to
> set default values.
>
> Patch 1:   A cleanup to use OBJECT_DECLARE_SIMPLE_TYPE for migration
> Patch 2-4: Qdev changes needed for the transition
> Patch 5-9: Improve QOM API to prepare for the property switch
> Patch 10:  Switch migration object to use TYPE_OBJECT and object props
>
> Comments welcomed, thanks.
>
> Peter Xu (10):
>   migration: Use OBJECT_DECLARE_SIMPLE_TYPE
>   qdev: Export global_props()
>   qdev: Introduce DEFINE_PROP_*_NODEFAULT for bool/uint32
>   hw/arm: Use nodefault version of qdev props when not needed
>   qom: Create object-property-ptr.[ch]
>   qom: Add object_property_add_bool_ptr()
>   qom: Add object_property_add_size_ptr()
>   qom: Add object_property_add_*_ptr_def()
>   qom: Allow default values for instance properties
>   migration: Switch to TYPE_OBJECT with object properties
>
>  include/hw/core/qdev-properties.h |   7 +
>  include/qom/object-property-ptr.h | 162 +++++++++
>  include/qom/object.h              |  99 +-----
>  migration/migration.h             |   9 +-
>  migration/options.h               |   8 +-
>  hw/arm/bcm2836.c                  |   3 +-
>  hw/core/qdev-properties.c         |   2 +-
>  migration/migration.c             |  42 ++-
>  migration/options.c               | 526 +++++++++++++++++++-----------
>  qom/object-property-ptr.c         | 394 ++++++++++++++++++++++
>  qom/object.c                      | 266 ++-------------
>  target/arm/cpu64.c                |   3 +-
>  qom/meson.build                   |   1 +
>  rust/bindings/hwcore-sys/lib.rs   |   2 +-
>  14 files changed, 957 insertions(+), 567 deletions(-)
>  create mode 100644 include/qom/object-property-ptr.h
>  create mode 100644 qom/object-property-ptr.c

Hi,

After we discussed your v1 I started playing with this in parallel and
stumbled into pretty much all of the issues this series resolves. Nice
work!

I applied a patch [0] on top of this series to allow using the
command-line to create the migration object. It can set all (most)
options with -object migration,id=mig1,key=val,...

 $ ~/qemu-system-x86_64 -nodefaults -nographic -S \
 -object migration,id=mig1,announce-initial=99,downtime-limit=99,multifd-channels=99,multifd-compression=zlib,multifd=on
 ...
 (qemu) info migrate_parameters
 (qemu) info migrate_capabilities
 ...
 announce-initial: 99 ms
 downtime-limit: 99 ms
 multifd-channels: 99
 multifd-compression: zlib
 multifd: on

I'm not saying we should do that now. I just want to check with you to
make sure we're not closing the door for future improvements:

1) It seems the "help" option is tied to the class. Setting options
work, but the help says otherwise:

 $ ~/qemu-system-x86_64 -nographic -object migration,id=mig1,help                                                                            
 There are no options for migration.

2) user_creatable_add_qapi

 The command line parsing goes through user_creatable_add_qapi() and
 instantiates an object. Since migrate_params_init runs from inside
 .instance_init, it cannot see any parameters that are set this way.

 Moreover, the migration_object_init() call from vl.c will init a
 second migration object.

 In the patch below I have hacked the current_migration assignment to
 first check if the object has already been created and use that
 instead of creating a new one.

 How does this work for normal objects? I suppose most of them have the
 TYPE_DEVICE as a parent, so they're not hanging in the "objects"
 container.

3) TLS (of course)

 With the patch below, setting TLS options from the cmdline asserts:
 visit_start_alternate: Assertion `!(v->type & VISITOR_INPUT)' failed.
 (just mentioning in case it can affect the design of this series)

[0]
-->8--
From 8c6c0743f1659d9df45e80334e8dc5f068ff984e Mon Sep 17 00:00:00 2001
From: Fabiano Rosas <farosas@suse.de>
Date: Tue, 9 Jun 2026 16:00:21 -0300
Subject: [PATCH] wip

---
 migration/migration.c | 11 ++++++++++-
 migration/options.c   |  1 +
 qapi/qom.json         | 29 +++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index ae0c373549..22c8ced766 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -297,7 +297,16 @@ void migration_object_init(void)
 {
     /* This can only be called once. */
     assert(!current_migration);
-    current_migration = MIGRATION(object_new(TYPE_MIGRATION));
+
+    Object *root = object_get_objects_root();
+    ObjectProperty *prop = g_hash_table_lookup(root->properties, "mig1");
+
+    if (prop->opaque) {
+        current_migration = prop->opaque;
+        object_ref(current_migration);
+    } else {
+        current_migration = MIGRATION(object_new(TYPE_MIGRATION));
+    }
 
     /*
      * Init the migrate incoming object as well no matter whether
diff --git a/migration/options.c b/migration/options.c
index 1cc99382d3..88f02c45a1 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -21,6 +21,7 @@
 #include "qapi/qapi-visit-migration.h"
 #include "qapi/qmp/qerror.h"
 #include "qobject/qnull.h"
+#include "qom/object_interfaces.h"
 #include "system/runstate.h"
 #include "migration/colo.h"
 #include "migration/cpr.h"
diff --git a/qapi/qom.json b/qapi/qom.json
index dd45ac1087..fe7dd4673c 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -8,6 +8,11 @@
 { 'include': 'block-core.json' }
 { 'include': 'common.json' }
 { 'include': 'crypto.json' }
+# FIXME: this requires --disable-tools due to:
+# /usr/bin/ld.bfd: libqemuutil.a.p/meson-generated_.._qapi_qapi-commands-migration.c.o:
+# in function `qmp_marshal_query_migr: qemu/build/qapi/qapi-commands-migration.c:48:(.text+0x181c):
+# undefined reference to `qmp_query_migrate'
+{ 'include': 'migration.json' }
 
 ##
 # ***********************
@@ -1187,6 +1192,28 @@
   'data': { '*cpu-affinity': ['uint16'],
             '*node-affinity': ['uint16'] } }
 
+
+##
+# @MigProperties:
+#
+# Properties for migration objects.
+#
+# @multifd: this is a capability and therefore is not part of
+#     MigrationParameters yet (WIP).  (default: 0)
+#
+# @store-global-state: this is a compat property and therefore is not
+#     part of MigrationParameters.  It's probably best to keep it like
+#     this so we don't have to deal with previously impossible
+#     scenarios if the user tries to set it via set-migrate  (default:
+#     0)
+#
+# Since: 11.1
+##
+{ 'struct': 'MigProperties',
+  'base': 'MigrationParameters',
+  'data': { '*multifd': 'bool',
+            '*store-global-state': 'bool' } }
+
 ##
 # @ObjectType:
 #
@@ -1237,6 +1264,7 @@
     'memory-backend-ram',
     { 'name': 'memory-backend-shm',
       'if': 'CONFIG_POSIX' },
+    'migration',
     'pef-guest',
     { 'name': 'pr-manager-helper',
       'if': 'CONFIG_LINUX' },
@@ -1315,6 +1343,7 @@
       'memory-backend-ram':         'MemoryBackendProperties',
       'memory-backend-shm':         { 'type': 'MemoryBackendShmProperties',
                                       'if': 'CONFIG_POSIX' },
+      'migration':                    'MigProperties',
       'pr-manager-helper':          { 'type': 'PrManagerHelperProperties',
                                       'if': 'CONFIG_LINUX' },
       'qtest':                      'QtestProperties',
-- 
2.53.0