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

On Wed, Jun 10, 2026 at 02:30:57PM -0400, Peter Xu wrote:
> 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]

You can force  a QOM object into being a singleton if you like.

In the UserCreatable interface "complete" callback, for the first
instance that's completed, stash its pointer in a global var,
and then when subsequent instances are created have "complete"
always report an error.


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

This is an example of why this needs to be using class properties,
not instance properties.

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

No, we don't want to do that - everything intended to be used
with -object needs to be using class properties.



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

If you want a singleton pattern, then it is simpler to just stash
the "Migration" object pointer in the "complete" method, and not
assume anything about the ID name the mgmt app chose to use.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|