Message ID | 20231024131305.87468-20-quintela@redhat.com |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | [PULL,01/39] migration/doc: Add contents | expand |
On 24/10/2023 15.12, Juan Quintela wrote: > From: Thomas Huth <thuth@redhat.com> > > Since the instance_init() function immediately tries to set the > property to "true", the s390_skeys_set_migration_enabled() tries > to register a savevm handler during instance_init(). However, > instance_init() functions can be called multiple times, e.g. for > introspection of devices. That means multiple instances of devices > can be created during runtime (which is fine as long as they all > don't get realized, too), so the "Prevent double registration of > savevm handler" check in the s390_skeys_set_migration_enabled() > function does not work at all as expected (since there could be > more than one instance). > > Thus we must not call register_savevm_live() from an instance_init() > function at all. Move this to the realize() function instead. This > way we can also get rid of the property getter and setter functions > completely, simplifying the code along the way quite a bit. > > Acked-by: David Hildenbrand <david@redhat.com> > Reviewed-by: Eric Farman <farman@linux.ibm.com> > Acked-by: Juan Quintela <quintela@redhat.com> > Signed-off-by: Thomas Huth <thuth@redhat.com> > Signed-off-by: Juan Quintela <quintela@redhat.com> > Message-ID: <20231020150554.664422-2-thuth@redhat.com> > --- > hw/s390x/s390-skeys.c | 35 ++++++++--------------------------- > 1 file changed, 8 insertions(+), 27 deletions(-) > > diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c > index 5024faf411..8e9d9e41e8 100644 > --- a/hw/s390x/s390-skeys.c > +++ b/hw/s390x/s390-skeys.c > @@ -12,6 +12,7 @@ > #include "qemu/osdep.h" > #include "qemu/units.h" > #include "hw/boards.h" > +#include "hw/qdev-properties.h" > #include "hw/s390x/storage-keys.h" > #include "qapi/error.h" > #include "qapi/qapi-commands-misc-target.h" > @@ -432,58 +433,38 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id) > return ret; > } > > -static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp) > -{ > - S390SKeysState *ss = S390_SKEYS(obj); > - > - return ss->migration_enabled; > -} > - > static SaveVMHandlers savevm_s390_storage_keys = { > .save_state = s390_storage_keys_save, > .load_state = s390_storage_keys_load, > }; > > -static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, > - Error **errp) > +static void s390_skeys_realize(DeviceState *dev, Error **errp) > { > - S390SKeysState *ss = S390_SKEYS(obj); > - > - /* Prevent double registration of savevm handler */ > - if (ss->migration_enabled == value) { > - return; > - } > - > - ss->migration_enabled = value; > + S390SKeysState *ss = S390_SKEYS(dev); > > if (ss->migration_enabled) { > register_savevm_live(TYPE_S390_SKEYS, 0, 1, > &savevm_s390_storage_keys, ss); > - } else { > - unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss); > } > } > > -static void s390_skeys_instance_init(Object *obj) > -{ > - object_property_add_bool(obj, "migration-enabled", > - s390_skeys_get_migration_enabled, > - s390_skeys_set_migration_enabled); > - object_property_set_bool(obj, "migration-enabled", true, NULL); > -} > +static Property s390_skeys_props[] = { > + DEFINE_PROP_BOOL("migration-enabled", S390SKeysState, migration_enabled, true), This needs a DEFINE_PROP_END_OF_LIST() here ... mea culpa! Thomas > +}; > > static void s390_skeys_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc = DEVICE_CLASS(oc); > > dc->hotpluggable = false; > + dc->realize = s390_skeys_realize; > + device_class_set_props(dc, s390_skeys_props); > set_bit(DEVICE_CATEGORY_MISC, dc->categories); > } > > static const TypeInfo s390_skeys_info = { > .name = TYPE_S390_SKEYS, > .parent = TYPE_DEVICE, > - .instance_init = s390_skeys_instance_init, > .instance_size = sizeof(S390SKeysState), > .class_init = s390_skeys_class_init, > .class_size = sizeof(S390SKeysClass),
diff --git a/hw/s390x/s390-skeys.c b/hw/s390x/s390-skeys.c index 5024faf411..8e9d9e41e8 100644 --- a/hw/s390x/s390-skeys.c +++ b/hw/s390x/s390-skeys.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "qemu/units.h" #include "hw/boards.h" +#include "hw/qdev-properties.h" #include "hw/s390x/storage-keys.h" #include "qapi/error.h" #include "qapi/qapi-commands-misc-target.h" @@ -432,58 +433,38 @@ static int s390_storage_keys_load(QEMUFile *f, void *opaque, int version_id) return ret; } -static inline bool s390_skeys_get_migration_enabled(Object *obj, Error **errp) -{ - S390SKeysState *ss = S390_SKEYS(obj); - - return ss->migration_enabled; -} - static SaveVMHandlers savevm_s390_storage_keys = { .save_state = s390_storage_keys_save, .load_state = s390_storage_keys_load, }; -static inline void s390_skeys_set_migration_enabled(Object *obj, bool value, - Error **errp) +static void s390_skeys_realize(DeviceState *dev, Error **errp) { - S390SKeysState *ss = S390_SKEYS(obj); - - /* Prevent double registration of savevm handler */ - if (ss->migration_enabled == value) { - return; - } - - ss->migration_enabled = value; + S390SKeysState *ss = S390_SKEYS(dev); if (ss->migration_enabled) { register_savevm_live(TYPE_S390_SKEYS, 0, 1, &savevm_s390_storage_keys, ss); - } else { - unregister_savevm(VMSTATE_IF(ss), TYPE_S390_SKEYS, ss); } } -static void s390_skeys_instance_init(Object *obj) -{ - object_property_add_bool(obj, "migration-enabled", - s390_skeys_get_migration_enabled, - s390_skeys_set_migration_enabled); - object_property_set_bool(obj, "migration-enabled", true, NULL); -} +static Property s390_skeys_props[] = { + DEFINE_PROP_BOOL("migration-enabled", S390SKeysState, migration_enabled, true), +}; static void s390_skeys_class_init(ObjectClass *oc, void *data) { DeviceClass *dc = DEVICE_CLASS(oc); dc->hotpluggable = false; + dc->realize = s390_skeys_realize; + device_class_set_props(dc, s390_skeys_props); set_bit(DEVICE_CATEGORY_MISC, dc->categories); } static const TypeInfo s390_skeys_info = { .name = TYPE_S390_SKEYS, .parent = TYPE_DEVICE, - .instance_init = s390_skeys_instance_init, .instance_size = sizeof(S390SKeysState), .class_init = s390_skeys_class_init, .class_size = sizeof(S390SKeysClass),