v1: https://lore.kernel.org/r/20241024165627.1372621-1-pet...@redhat.com
This patchset introduces the singleton interface for QOM. I didn't add a changelog because there're quite a few changes here and there, plus new patches. So it might just be easier to re-read, considering the patchset isn't large. I switched v2 into RFC, because we have reviewer concerns (Phil and Dan so far) that it could be error prone to try to trap every attempts to create an object. My argument is, if we already have abstract class, meanwhile we do not allow instantiation of abstract class, so the complexity is already there. I prepared patch 1 this time to collect and track all similar random object creations; it might be helpful as a cleanup on its own to deduplicate some similar error messages. Said that, I'm still always open to rejections to this proposal. I hope v2 looks slightly cleaner by having not only object_new_allowed() but also object_new_or_fetch(). Patch layout: Patch 1-2: The patches to introduce QOM singleton interface Patch 3-5: Add support for vIOMMU singleton, some qdev change needed Patch 6-7: Add support for migration singleton, fix dangle pointer Below are copy-paste of the commit message of patch 2, that I could have put into the cover letter too. ====8<==== The singleton interface declares an object class which can only create one instance globally. Backgrounds / Use Cases ======================= There can be some existing classes that can start to benefit from it. One example is vIOMMU implementations. QEMU emulated vIOMMUs are normally implemented on top of a generic device, however it's special enough to normally only allow one instance of it for the whole system, attached to the root complex. These vIOMMU classes can be singletons in this case, so that QEMU can fail or detect yet another attempt of creating such devices for more than once, which can be fatal errors to a system. We used to have some special code guarding it from happening. In x86, pc_machine_device_pre_plug_cb() has code to detect when vIOMMU is created more than once, for instance. With singleton class, logically we could consider dropping the special code, but start to rely on QOM to make sure there's only one vIOMMU for the whole system emulation. There is a similar demand raising recently (even if the problem existed over years) in migration. Firstly, the migration object can currently be created randomly, even though not wanted, e.g. during qom-list-properties QMP commands. Ideally, there can be cases where we want to have an object walking over the properties, we could use the existing migration object instead of dynamically create one. Meanwhile, migration has a long standing issue on current_migration pointer, where it can point to freed data after the migration object is finalized. It is debatable that the pointer can be cleared after the main thread (1) join() the migration thread first, then (2) release the last refcount for the migration object and clear the pointer. However there's still major challenges [1]. With singleton, we could have a slightly but hopefully working workaround to clear the pointer during finalize(). Design ====== The idea behind is pretty simple: any object that can only be created once can now declare the TYPE_SINGLETON interface. Then, QOM facilities will make sure it won't be created more than once for the whole QEMU lifecycle. Whenever possible (e.g., on object_new_allowed() checks), pretty error message will be generated to report an error. QOM also guards at the core of object_new() so that any further violation of trying to create a singleton more than once will crash QEMU as a programming error. For example, qom-list-properties, device-list-properties, etc., will be smart enough to not try to create temporary singleton objects if the class is a singleton class and if there's existing instance created. Such usages should be rare, and this patch introduced object_new_or_fetch() just for it, which either create a new temp object when available, or fetch the instance if we found an existing singleton instance. There're only two such use cases. Meanwhile, we also guard device-add or similar paths using the singleton check in object_new_allowed(), so that it'll fail properly if a singleton class instantiate more than one object. Glib Singleton implementation ============================= One note here to mention the Glib implementation of singletons [1]. QEMU chose not to follow Glib's implementation because Glib's version is not thread safe on the constructor, so that two concurrent g_object_new() on a single can race. It's not ideal to QEMU, as QEMU has to not only support the event-driven context which is normally lock-free, but also the case where threads are heavily used. It could be QEMU's desire to properly support multi-threading by default on such new interface. The "bad" side effect of that is, QEMU's object_new() on singletons can assert failures if the singleton existed, but that's also part of the design so as to forbid such from happening, taking which as a programming error. Meanwhile since we have pretty ways to fail elsewhere on qdev creations, it should already guard us in a clean way, from anywhere that the user could try to create the singleton more than once. The current QEMU impl also guarantees object_new() always return a newly allocated object as long as properly returned, rather than silently return an existing object as what Glib's impl would do. I see it a benefit, so as to avoid unknown caller manipulate a global object, wrongly assuming it was temporarily created. [1] https://lore.kernel.org/qemu-devel/20190228122822.GD4970@work-vm/ [2] https://lore.kernel.org/r/zxtqgqbd4hq4a...@redhat.com Thanks, Peter Xu (7): qom: Track dynamic initiations of random object class qom: TYPE_SINGLETON interface qdev: Make device_set_realized() be fully prepared with !machine qdev: Make qdev_get_machine() safe before machine creates x86/iommu: Make x86-iommu a singleton object migration: Make migration object a singleton object migration: Reset current_migration properly qapi/qdev.json | 2 +- qapi/qom.json | 2 +- include/qom/object.h | 25 ++++++++++++++++ include/qom/object_interfaces.h | 51 +++++++++++++++++++++++++++++++++ chardev/char.c | 4 +-- hw/core/cpu-common.c | 13 ++++++--- hw/core/qdev.c | 20 +++++++++++-- hw/i386/x86-iommu.c | 26 +++++++++++++++-- migration/migration.c | 36 +++++++++++++++++++++-- qom/object.c | 50 ++++++++++++++++++++++++++++++-- qom/object_interfaces.c | 33 +++++++++++++++++++-- qom/qom-qmp-cmds.c | 4 +-- system/qdev-monitor.c | 4 +-- 13 files changed, 243 insertions(+), 27 deletions(-) -- 2.45.0
