From efec3dd631d94160288392721a5f9c39e50fb2bc Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 28 Nov 2013 17:26:54 +0100 Subject: qdev: Replace no_user by cannot_instantiate_with_device_add_yet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In an ideal world, machines can be built by wiring devices together with configuration, not code. Unfortunately, that's not the world we live in right now. We still have quite a few devices that need to be wired up by code. If you try to device_add such a device, it'll fail in sometimes mysterious ways. If you're lucky, you get an unmysterious immediate crash. To protect users from such badness, DeviceClass member no_user used to make device models unavailable with -device / device_add, but that regressed in commit 18b6dad. The device model is still omitted from help, but is available anyway. Attempts to fix the regression have been rejected with the argument that the purpose of no_user isn't clear, and it's prone to misuse. This commit clarifies no_user's purpose. Anthony suggested to rename it cannot_instantiate_with_device_add_yet_due_to_internal_bugs, which I shorten somewhat to keep checkpatch happy. While there, make it bool. Every use of cannot_instantiate_with_device_add_yet gets a FIXME comment asking for rationale. The next few commits will clean them all up, either by providing a rationale, or by getting rid of the use. With that done, the regression fix is hopefully acceptable. Signed-off-by: Markus Armbruster Reviewed-by: Marcel Apfelbaum Signed-off-by: Andreas Färber --- include/hw/qdev-core.h | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index f2043a6..c8945a4 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -97,7 +97,18 @@ typedef struct DeviceClass { const char *fw_name; const char *desc; Property *props; - int no_user; + + /* + * Shall we hide this device model from -device / device_add? + * All devices should support instantiation with device_add, and + * this flag should not exist. But we're not there, yet. Some + * devices fail to instantiate with cryptic error messages. + * Others instantiate, but don't work. Exposing users to such + * behavior would be cruel; this flag serves to protect them. It + * should never be set without a comment explaining why it is set. + * TODO remove once we're there + */ + bool cannot_instantiate_with_device_add_yet; /* callbacks */ void (*reset)(DeviceState *dev); -- cgit v1.1 From c272758f93b9c88c884461a2baa37b8f4008bf02 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 29 Nov 2013 10:43:45 +0100 Subject: qdev: Document that pointer properties kill device_add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ask users of DEFINE_PROP_PTR() to set cannot_instantiate_with_device_add_yet, or explain why it's not needed. Signed-off-by: Markus Armbruster Signed-off-by: Andreas Färber --- include/hw/qdev-properties.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'include/hw') diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 692f82e..77c6f7c 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -122,8 +122,25 @@ extern PropertyInfo qdev_prop_arraylen; #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) +/* + * Please avoid pointer properties. If you must use them, you must + * cover them in their device's class init function as follows: + * + * - If the property must be set, the device cannot be used with + * device_add, so add code like this: + * |* Reason: pointer property "NAME-OF-YOUR-PROP" *| + * DeviceClass *dc = DEVICE_CLASS(class); + * dc->cannot_instantiate_with_device_add_yet = true; + * + * - If the property may safely remain null, document it like this: + * |* + * * Note: pointer property "interrupt_vector" may remain null, thus + * * no need for dc->cannot_instantiate_with_device_add_yet = true; + * *| + */ #define DEFINE_PROP_PTR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*) + #define DEFINE_PROP_CHR(_n, _s, _f) \ DEFINE_PROP(_n, _s, _f, qdev_prop_chr, CharDriverState*) #define DEFINE_PROP_STRING(_n, _s, _f) \ -- cgit v1.1 From 6780a22cc71227068925e7b70faa71d6641a9b1b Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 18 Dec 2013 17:15:51 +0100 Subject: qdev: Drop misleading qbus_free() function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same reasoning as commit 02a5c4c97422b40034f31265e0f139f7846172a8 ("qdev: Drop misleading qdev_free() function"). The qbus_free() function removes the child from the namespace and decrements the reference count. It does not, however, guarantee to free the child since the refcount may still be held. Just call object_unparent() directly. Suggested-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi Signed-off-by: Andreas Färber --- include/hw/qdev-core.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include/hw') diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index c8945a4..651c3e5 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -283,8 +283,6 @@ void qdev_reset_all(DeviceState *dev); void qbus_reset_all(BusState *bus); void qbus_reset_all_fn(void *opaque); -void qbus_free(BusState *bus); - /* This should go away once we get rid of the NULL bus hack */ BusState *sysbus_get_default(void); -- cgit v1.1 From ff6986ce618c69f988e4419efd67ea5cbf7792a5 Mon Sep 17 00:00:00 2001 From: xiaoqiang zhao Date: Tue, 5 Nov 2013 18:16:03 +0800 Subject: apic: QOM'ify APIC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert 'init' function to QOM's 'realize' for apic, kvm/apic and xen/xen_apic. Signed-off-by: xiaoqiang zhao Signed-off-by: Andreas Färber --- include/hw/i386/apic_internal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h index 1b0a7fb..70542a6 100644 --- a/include/hw/i386/apic_internal.h +++ b/include/hw/i386/apic_internal.h @@ -80,7 +80,7 @@ typedef struct APICCommonClass { ICCDeviceClass parent_class; - void (*init)(APICCommonState *s); + DeviceRealize realize; void (*set_base)(APICCommonState *s, uint64_t val); void (*set_tpr)(APICCommonState *s, uint8_t val); uint8_t (*get_tpr)(APICCommonState *s); -- cgit v1.1 From 494c271784a5e360523e874be9f67259932ea68c Mon Sep 17 00:00:00 2001 From: xiaoqiang zhao Date: Wed, 18 Dec 2013 18:21:46 +0100 Subject: icc_bus: QOM'ify ICC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For consistency, QOM'ify APIC's parent bus. Signed-off-by: xiaoqiang zhao Signed-off-by: Andreas Färber --- include/hw/cpu/icc_bus.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/cpu/icc_bus.h b/include/hw/cpu/icc_bus.h index b550070..98a979f 100644 --- a/include/hw/cpu/icc_bus.h +++ b/include/hw/cpu/icc_bus.h @@ -66,7 +66,7 @@ typedef struct ICCDeviceClass { DeviceClass parent_class; /*< public >*/ - int (*init)(ICCDevice *dev); /* TODO replace with QOM realize */ + DeviceRealize realize; } ICCDeviceClass; #define TYPE_ICC_DEVICE "icc-device" -- cgit v1.1 From db0f888848bc5cc578d005d04f4cf7a1105bb758 Mon Sep 17 00:00:00 2001 From: xiaoqiang zhao Date: Tue, 5 Nov 2013 18:16:05 +0800 Subject: ioapic: QOM'ify ioapic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert 'init' function to QOM's 'realize' for ioapic and kvm-ioapic. Change variable 'ioapic_no' from static to global. Then we can drop the 'instance_no' function argument. Signed-off-by: xiaoqiang zhao Signed-off-by: Andreas Färber --- include/hw/i386/ioapic_internal.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'include/hw') diff --git a/include/hw/i386/ioapic_internal.h b/include/hw/i386/ioapic_internal.h index 25576c8..3be3352 100644 --- a/include/hw/i386/ioapic_internal.h +++ b/include/hw/i386/ioapic_internal.h @@ -83,7 +83,8 @@ typedef struct IOAPICCommonState IOAPICCommonState; typedef struct IOAPICCommonClass { SysBusDeviceClass parent_class; - void (*init)(IOAPICCommonState *s, int instance_no); + + DeviceRealize realize; void (*pre_save)(IOAPICCommonState *s); void (*post_load)(IOAPICCommonState *s); } IOAPICCommonClass; -- cgit v1.1