Re: [Qemu-devel] [PATCH 0/3] migration: export cap/params to qdev props

[Peter Xu <peterx@redhat.com>]

On Wed, Jul 12, 2017 at 08:02:40PM +0100, Dr. David Alan Gilbert wrote:
> * Peter Xu (peterx@redhat.com) wrote:
> > We have the MigrationState as QDev now (which seems crazy). Let's
> > continue to benefit.
> > 
> > This series is exporting all migration capabilities/params as global
> > parameters. Then we can do something like this:
> > 
> >   qemu -global migration.postcopy-ram=true \
> >        -global migration.max-bandwidth=4096
> > 
> > The values will be inited just like we typed these values into HMP
> > monitor. It'll simplify lots of migration scripts.
> > 
> > The changes are fairly straightforward. One tiny loss is that we still
> > don't support:
> > 
> >   -global migration.max-bandwidth=1g
> > 
> > ...just like what we did in HMP:
> > 
> >   migrate_set_speed 1g
> > 
> > ...while we need to use:
> > 
> >   -global migration.max-bandwidth=1073741824
> > 
> > However that should only be used in scripts, and that's good enough
> > imho.
> > 
> > These properties should only be used for debugging/testing purpose,
> > and we should not guarantee any interface compatibility for them (just
> > like HMP).
> 
> I guess the sanity checks in qmp_migrate_set_parameters and
> qmp_migrate_set_capabilities aren't run?

Indeed I missed this point. Before that: Eduardo, do you think below
change would make any sense?

diff --git a/qom/object.c b/qom/object.c
index 5f6fdfa..ca077a9 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -347,13 +347,13 @@ static void object_init_with_type(Object *obj, TypeImpl *ti)
 
 static void object_post_init_with_type(Object *obj, TypeImpl *ti)
 {
-    if (ti->instance_post_init) {
-        ti->instance_post_init(obj);
-    }
-
     if (type_has_parent(ti)) {
         object_post_init_with_type(obj, type_get_parent(ti));
     }
+
+    if (ti->instance_post_init) {
+        ti->instance_post_init(obj);
+    }
 }
 
 static void object_initialize_with_type(void *data, size_t size, TypeImpl *type)

IMHO it'll make more sense if we call the parent post_init first (just
like most of the other object hooks). Or is current ordering intended?

If we can have above change, it'll be much easier for me to do the
check that Dave mentioned, after the global properties applied (I can
then just provide post_init hook for the migration object).

> 
> Also, have you done any checks on multiple migrations; e.g. it's good
> to check that the flags/state are all preserved if we do a migrate, that
> migrate fails or is cancelled and then you do a 2nd migrate.

I think so. They'll be applied only when creating the object, and
they'll be there even after a e.g. migrate_cancel.  Thanks,

-- 
Peter Xu