From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46566) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d3JqJ-0003Fg-Az for qemu-devel@nongnu.org; Wed, 26 Apr 2017 06:06:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d3JqC-0001bh-Py for qemu-devel@nongnu.org; Wed, 26 Apr 2017 06:06:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33064) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1d3JqC-0001am-GF for qemu-devel@nongnu.org; Wed, 26 Apr 2017 06:06:32 -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 6017880046 for ; Wed, 26 Apr 2017 10:06:31 +0000 (UTC) Date: Wed, 26 Apr 2017 18:06:23 +0800 From: Peter Xu Message-ID: <20170426100623.GD16228@pxdev.xzpeter.org> References: <20170425101758.3944-1-quintela@redhat.com> <20170425101758.3944-2-quintela@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170425101758.3944-2-quintela@redhat.com> Subject: Re: [Qemu-devel] [PATCH 1/2] migration: Move check_migratable() into qdev.c List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org, dgilbert@redhat.com, lvivier@redhat.com On Tue, Apr 25, 2017 at 12:17:57PM +0200, Juan Quintela wrote: > The function is only used once, and nothing else in migration knows > about objects. Create the function vmstate_device_is_migratable() in > savem.c that really do the bit that is related with migration. > > Signed-off-by: Juan Quintela > --- > hw/core/qdev.c | 15 ++++++++++++++- > include/migration/migration.h | 3 --- > include/migration/vmstate.h | 2 ++ > migration/migration.c | 15 --------------- > migration/savevm.c | 10 ++++++++++ > stubs/vmstate.c | 5 ++--- > 6 files changed, 28 insertions(+), 22 deletions(-) > > diff --git a/hw/core/qdev.c b/hw/core/qdev.c > index 02b632f..17ff638 100644 > --- a/hw/core/qdev.c > +++ b/hw/core/qdev.c > @@ -37,7 +37,7 @@ > #include "hw/boards.h" > #include "hw/sysbus.h" > #include "qapi-event.h" > -#include "migration/migration.h" > +#include "migration/vmstate.h" > > bool qdev_hotplug = false; > static bool qdev_hot_added = false; > @@ -861,6 +861,19 @@ static bool device_get_realized(Object *obj, Error **errp) > return dev->realized; > } > > +static int check_migratable(Object *obj, Error **err) > +{ > + DeviceClass *dc = DEVICE_GET_CLASS(obj); > + if (!vmstate_device_is_migratable(dc->vmsd)) { > + error_setg(err, "Device %s is not migratable, but " > + "--only-migratable was specified", > + object_get_typename(obj)); > + return -1; > + } > + > + return 0; > +} > + > static void device_set_realized(Object *obj, bool value, Error **errp) > { > DeviceState *dev = DEVICE(obj); > diff --git a/include/migration/migration.h b/include/migration/migration.h > index ba1a16c..dfeca38 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -22,7 +22,6 @@ > #include "qapi-types.h" > #include "exec/cpu-common.h" > #include "qemu/coroutine_int.h" > -#include "qom/object.h" > > #define QEMU_VM_FILE_MAGIC 0x5145564d > #define QEMU_VM_FILE_VERSION_COMPAT 0x00000002 > @@ -292,8 +291,6 @@ int migrate_add_blocker(Error *reason, Error **errp); > */ > void migrate_del_blocker(Error *reason); > > -int check_migratable(Object *obj, Error **err); > - > bool migrate_release_ram(void); > bool migrate_postcopy_ram(void); > bool migrate_zero_blocks(void); > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index dad3984..9452dec 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -1049,4 +1049,6 @@ int64_t self_announce_delay(int round) > > void dump_vmstate_json_to_file(FILE *out_fp); > > +bool vmstate_device_is_migratable(const VMStateDescription *vmsd); > + > #endif > diff --git a/migration/migration.c b/migration/migration.c > index 353f272..5447cab 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -1158,21 +1158,6 @@ void migrate_del_blocker(Error *reason) > migration_blockers = g_slist_remove(migration_blockers, reason); > } > > -int check_migratable(Object *obj, Error **err) > -{ > - DeviceClass *dc = DEVICE_GET_CLASS(obj); > - if (only_migratable && dc->vmsd) { > - if (dc->vmsd->unmigratable) { > - error_setg(err, "Device %s is not migratable, but " > - "--only-migratable was specified", > - object_get_typename(obj)); > - return -1; > - } > - } > - > - return 0; > -} > - > void qmp_migrate_incoming(const char *uri, Error **errp) > { > Error *local_err = NULL; > diff --git a/migration/savevm.c b/migration/savevm.c > index 03ae1bd..7421a67 100644 > --- a/migration/savevm.c > +++ b/migration/savevm.c > @@ -2480,3 +2480,13 @@ void vmstate_register_ram_global(MemoryRegion *mr) > { > vmstate_register_ram(mr, NULL); > } > + > +bool vmstate_device_is_migratable(const VMStateDescription *vmsd) > +{ > + if (only_migratable && vmsd) { > + if (vmsd->unmigratable) { > + return false; > + } > + } > + return true; > +} Here the name vmstate_device_is_migratable() let me think of "return true if the device can migrate, false otherwise". However what it actually does is mixed with "--only-migratable" parameter, say, when without that parameter, all device will be getting "true" here, even unmigratable devices... Not sure whether it'll be nicer we do this: bool vmstate_device_is_migratable(const VMStateDescription *vmsd) { return !(vmsd & vmsd->unmigratable); } Then we can define check_migratable() as: static int check_migratable(Object *obj, Error **err) { DeviceClass *dc = DEVICE_GET_CLASS(obj); /* no check needed if --only-migratable not specified */ if (!only_migratable) { return true; } if (!vmstate_device_is_migratable(dc->vmsd)) { error_setg(err, "Device %s is not migratable, but " "--only-migratable was specified", object_get_typename(obj)); return -1; } return 0; } Thanks, > diff --git a/stubs/vmstate.c b/stubs/vmstate.c > index 6d52f29..5af824b 100644 > --- a/stubs/vmstate.c > +++ b/stubs/vmstate.c > @@ -1,7 +1,6 @@ > #include "qemu/osdep.h" > #include "qemu-common.h" > #include "migration/vmstate.h" > -#include "migration/migration.h" > > const VMStateDescription vmstate_dummy = {}; > > @@ -21,7 +20,7 @@ void vmstate_unregister(DeviceState *dev, > { > } > > -int check_migratable(Object *obj, Error **err) > +bool vmstate_device_is_migratable(const VMStateDescription *vmsd) > { > - return 0; > + return true; > } > -- > 2.9.3 > -- Peter Xu