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

On Tue, Jun 09, 2026 at 07:54:56PM -0300, Fabiano Rosas wrote:
> 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!

Good to know at least it looks reasonable for someone.. thanks. :)

Looks like Dan still has concern that I use the ptrs in obj props but I'll
discuss it later separately.

> 
> 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:

IIUC it works, maybe indeed it's a better way than "-incoming config:" at
least in that we stick with the current APIs.

Said that, it does bypass the singleton work I did previously:

https://lore.kernel.org/r/[email protected]

Which was also unfortunately got rejected..

So, I suppose this is still fine that all the tricks resides in migration/
so far, maybe this is acceptable.

But then, we'll need to make sure the rest QEMU object code doesn't have
assumption that all objects can be created more than one.. and making sure
nothing crash elsewhere: when it crashes or works inproperly, we may face
again that singleton problem one way or another.  That's so far the only
concern I have.

> 
> 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.

This one is easy, something like this should work:

 bool type_print_class_properties(const char *type)
 {
+    g_autoptr(Object) obj = NULL;                                                                                                       
     ObjectClass *klass;
     ObjectPropertyIterator iter;
     ObjectProperty *prop;
@@ -134,7 +135,12 @@ bool type_print_class_properties(const char *type)
     }
 
     array = g_ptr_array_new();
-    object_class_property_iter_init(&iter, klass);                                                                                      
+    if (object_class_is_abstract(klass)) {                                                                                              
+        object_class_property_iter_init(&iter, klass);                                                                                  
+    } else {                                                                                                                            
+        obj = object_new_with_class(klass);                                                                                             
+        object_property_iter_init(&iter, obj);                                                                                          
+    }                                                                                                                                   
     while ((prop = object_property_iter_next(&iter))) {
         if (!prop->set) {
             continue;

> 
> 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.

Yeah that trick should work at least for now, except the concern I raised
above.

Since the tap work is unblocked (by temporarily introducing the tap flag),
I plan to put aside the "-incoming config:*" work a bit.  We'll need to
pick it up at least when there's yet another similiar use case like tap or
"mode" of CPR, but then lower priority. But let me know if you or anyone
still think we should have it land earlier, I can re-prioritize that.

> 
>  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.

I may not get the real question behind, but.. iiuc normal objects shouldn't
be sub-class of TYPE_DEVICE, and they should work fine with -object *help,
and they should be able to be created with multiple instances in most
cases.

> 
> 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)

Oh, I didn't really notice we'll have issue with it.. I did the string prop
solution for tls* by pure accident, because we already assumed it's always
strings anyway and it's trivial to use the object prop str helpers.  I
didn't expect it an issue when used with string inputs.  So I assume for
those we need to rely on JSON formats?

  -object '{"qom-type": "migration", "id": "mig1", "tls-creds": "SOMETHING"}'

> 
> [0]
> -->8--
> From 8c6c0743f1659d9df45e80334e8dc5f068ff984e Mon Sep 17 00:00:00 2001
> From: Fabiano Rosas <[email protected]>
> 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));
> +    }

A quick comment for this one; maybe better with:

     Object *mig_obj = object_resolve_path_component(object_get_objects_root(),                                                          
                                                     "mig1");                                                                            

Or even better:

static int find_migration_object(Object *obj, void *opaque)
{
    if (object_dynamic_cast(obj, TYPE_MIGRATION)) {
        *(Object **)opaque = obj;
        return 1;
    }
    return 0;
}

Then:

     object_child_foreach(object_get_objects_root(),
                          find_migration_object, &existing);

>  
>      /*
>       * 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
> 
> 

-- 
Peter Xu