Message ID | 20170425101758.3944-2-quintela@redhat.com |
---|---|
State | New |
Headers | show |
On 25/04/2017 12:17, 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 <quintela@redhat.com> Reviewed-by: Laurent Vivier <lvivier@redhat.com> > --- > 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; > +} > 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; > } >
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 <quintela@redhat.com> > --- > 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 <peterx@redhat.com> wrote: > 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 <quintela@redhat.com> > > 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, I see your point. We have to teach things about migrate to qdev.c or things about objects to migration. Will change. Thanks, Juan.
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; +} 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; }
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 <quintela@redhat.com> --- 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(-)