On Tue, Oct 29, 2024 at 10:47:06AM +0000, Daniel P. Berrangé wrote: > On Thu, Oct 24, 2024 at 12:56:25PM -0400, Peter Xu wrote: > > X86 IOMMUs cannot be created more than one on a system yet. Make it a > > singleton so it guards the system from accidentally create yet another > > IOMMU object when one already presents. > > > > Now if someone tries to create more than one, e.g., via: > > > > ./qemu -M q35 -device intel-iommu -device intel-iommu > > > > The error will change from: > > > > qemu-system-x86_64: -device intel-iommu: QEMU does not support multiple > > vIOMMUs for x86 yet. > > > > To: > > > > qemu-system-x86_64: -device intel-iommu: Class 'intel-iommu' only > > supports one instance > > > > Unfortunately, yet we can't remove the singleton check in the machine > > hook (pc_machine_device_pre_plug_cb), because there can also be > > virtio-iommu involved, which doesn't share a common parent class yet. > > Presumably the 'class' reported is the one that the user requested, > but this would imply if we were to do > > qemu-system-x86_64 -device intel-iommu -device virtio-iommu > > Then QEMU would report > > "Class 'virtio-iommu' only supports one instance" > > at which point the user is wondering, huh, I only requested one virtio-iommu > instance ? > > IOW, the current error message would be better as it is not referring to a > specific subclass, but rather to the more general fact that only a single > IOMMU is permitted, no matter what it's impl is.
True.. though IIUC this is more or less a cosmetic change only. E.g., if we want (assuming after we could have object_new_allowed(Error **errp), checking both abstract + singleton classes) we could make the error points to the base class (rather than the top class to be initiated) that declared TYPE_SINGLETON when it failed due to the singleton check. One step further, we can even provide a custom Error for any singleton class to say whatever it wants if it hits a duplicate. So to me it's a separate issue from whether we would like to have a generic way to define a singleton class. I am still ok if we want to avoid introducing the singleton, but just to mention I believe it can report something similar as before if we want. Thanks, -- Peter Xu
