From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48719) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dPK6w-0005Zi-Eh for qemu-devel@nongnu.org; Sun, 25 Jun 2017 22:50:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dPK6r-00074n-Ja for qemu-devel@nongnu.org; Sun, 25 Jun 2017 22:50:46 -0400 Received: from mx1.redhat.com ([209.132.183.28]:57028) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dPK6r-00074j-AF for qemu-devel@nongnu.org; Sun, 25 Jun 2017 22:50:41 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E1F6988E60 for ; Mon, 26 Jun 2017 02:50:39 +0000 (UTC) Date: Mon, 26 Jun 2017 10:50:35 +0800 From: Peter Xu Message-ID: <20170626025035.GH3936@pxdev.xzpeter.org> References: <1498193206-18007-1-git-send-email-peterx@redhat.com> <1498193206-18007-5-git-send-email-peterx@redhat.com> <20170623221819.GE10776@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170623221819.GE10776@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCH v5 04/10] migration: let MigrationState be a qdev List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Laurent Vivier , Juan Quintela , "Dr . David Alan Gilbert" , Markus Armbruster On Fri, Jun 23, 2017 at 07:18:19PM -0300, Eduardo Habkost wrote: > On Fri, Jun 23, 2017 at 12:46:40PM +0800, Peter Xu wrote: > > Let the old man "MigrationState" join the object family. Direct benefit > > is that we can start to use all the property features derived from > > current QDev, like: HW_COMPAT_* bits, command line setup for migration > > parameters (so will never need to set them up each time using HMP/QMP, > > this is really, really attractive for test writters), etc. > > > > I see no reason to disallow this happen yet. So let's start from this > > one, to see whether it would be anything good. > > > > Now we init the MigrationState struct statically in main() to make sure > > it's initialized after global properties are applied, since we'll use > > them during creation of the object. > > > > No functional change at all. > > > > Reviewed-by: Juan Quintela > > Signed-off-by: Peter Xu > > --- > > include/migration/misc.h | 1 + > > migration/migration.c | 78 ++++++++++++++++++++++++++++++++++-------------- > > migration/migration.h | 19 ++++++++++++ > > vl.c | 6 ++++ > > 4 files changed, 81 insertions(+), 23 deletions(-) > > > [...] > > diff --git a/vl.c b/vl.c > > index cdd2ec8..9b04ba7 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -4596,6 +4596,12 @@ int main(int argc, char **argv, char **envp) > > */ > > register_global_properties(current_machine); > > > > + /* > > + * Migration object can only be created after global properties > > + * are applied correctly. > > + */ > > + migration_object_init(); > > + > > Do we really need this? Can't be we just do: > > if (!current_migration) { > current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION)); > } > > inside migration_get_current()? I did this change on purpose (after AccelClass.global_props is introduced). The comment above tried to explain it but looks like it's still not clear enough... The reason is that currently the creation of migration object is depending on the global properties, so we need to create the object after register_global_properties(), while the old migrate_get_current() cannot really be sure of this ordering: it just creates the object on the first call of the function, but the first call can be even before register_global_properties(). If so, we'll have a problem (e.g. Xen compat properties will be missing). Now this restriction is strictly followed if we create the migrate object here. If anyone calls migrate_get_current() before register_global_properties(), there will be an assert, and that should be treated as a programming error. Thanks, -- Peter Xu