diff mbox series

QOM: Can devices having link properties be user-creatable?

Message ID b41a233c-988c-19bb-0d4b-b17655912839@redhat.com
State New
Headers show
Series QOM: Can devices having link properties be user-creatable? | expand

Commit Message

Philippe Mathieu-Daudé Oct. 12, 2018, 12:47 p.m. UTC 
While looking at Mao's series
(https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02519.html) I
wondered if devices having link properties can be user-creatable.

Using the following patch (I don't think this is correct to put qdev
code into qobject, but I used this as PoC):

-- >8 --
     prop->child = child;
---

I get:

$ aarch64-softmmu/qemu-system-aarch64 -M virt
qemu-system-aarch64: warning: Device type 'bus' is user_creatable
(linked property: 'parent_bus')
qemu-system-aarch64: warning: Device type 'qemu:memory-region' is
user_creatable (linked property: 'memory')
qemu-system-aarch64: warning: Device type 'bus' is user_creatable
(linked property: 'parent_bus')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'unnamed-gpio-out[0]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'unnamed-gpio-out[1]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'unnamed-gpio-out[2]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'unnamed-gpio-out[3]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'gicv3-maintenance-interrupt[0]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'pmu-interrupt[0]')
qemu-system-aarch64: warning: Device type 'qemu:memory-region' is
user_creatable (linked property: 'secure-memory')
qemu-system-aarch64: warning: Device type 'bus' is user_creatable
(linked property: 'parent_bus')
qemu-system-aarch64: warning: Device type 'bus' is user_creatable
(linked property: 'parent_bus')
qemu-system-aarch64: warning: Device type 'bus' is user_creatable
(linked property: 'parent_bus')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'sysbus-irq[0]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'sysbus-irq[1]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'sysbus-irq[2]')
qemu-system-aarch64: warning: Device type 'irq' is user_creatable
(linked property: 'sysbus-irq[3]')

Are we incorrectly using user_creatable = true in those devices/
Am I missing something?

Thanks,

Phil.

Comments

Peter Maydell Oct. 12, 2018, 1:01 p.m. UTC | #1 
On 12 October 2018 at 13:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> While looking at Mao's series
> (https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02519.html) I
> wondered if devices having link properties can be user-creatable.
>
> Using the following patch (I don't think this is correct to put qdev
> code into qobject, but I used this as PoC):
>
> -- >8 --
> diff --git a/qom/object.c b/qom/object.c
> index 547dcf97c3..2dd3a25531 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -16,4 +16,5 @@
>  #include "qom/object_interfaces.h"
>  #include "qemu/cutils.h"
> +#include "qemu/error-report.h"
>  #include "qapi/visitor.h"
>  #include "qapi/string-input-visitor.h"
> @@ -1662,4 +1663,11 @@ void object_property_add_link(Object *obj, const
> char *name,
>      gchar *full_type;
>      ObjectProperty *op;
> +    ObjectClass *klass = object_get_class(obj);
> +
> +    if (klass && object_dynamic_cast(obj, TYPE_DEVICE) &&
> +                 DEVICE_CLASS(klass)->user_creatable == false) {
> +        warn_report("Device type '%s' is user_creatable "
> +                    "(linked property: '%s')", type, name);
> +    }
>
>      prop->child = child;
> ---
>
> I get:
>
> $ aarch64-softmmu/qemu-system-aarch64 -M virt
> qemu-system-aarch64: warning: Device type 'bus' is user_creatable
> (linked property: 'parent_bus')
> qemu-system-aarch64: warning: Device type 'qemu:memory-region' is
> user_creatable (linked property: 'memory')

Is your warning printing the wrong thing? "bus", "irq",
etc don't seem like names of devices...

I think whether a device with a link property is
user creatable might depend on what the property is
for and whether the device has a useful fallback for
"link not connected".

thanks
-- PMM
Philippe Mathieu-Daudé Oct. 12, 2018, 1:06 p.m. UTC | #2 
On 12/10/2018 15:01, Peter Maydell wrote:
> On 12 October 2018 at 13:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> While looking at Mao's series
>> (https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02519.html) I
>> wondered if devices having link properties can be user-creatable.
>>
>> Using the following patch (I don't think this is correct to put qdev
>> code into qobject, but I used this as PoC):
>>
>> -- >8 --
>> diff --git a/qom/object.c b/qom/object.c
>> index 547dcf97c3..2dd3a25531 100644
>> --- a/qom/object.c
>> +++ b/qom/object.c
>> @@ -16,4 +16,5 @@
>>  #include "qom/object_interfaces.h"
>>  #include "qemu/cutils.h"
>> +#include "qemu/error-report.h"
>>  #include "qapi/visitor.h"
>>  #include "qapi/string-input-visitor.h"
>> @@ -1662,4 +1663,11 @@ void object_property_add_link(Object *obj, const
>> char *name,
>>      gchar *full_type;
>>      ObjectProperty *op;
>> +    ObjectClass *klass = object_get_class(obj);
>> +
>> +    if (klass && object_dynamic_cast(obj, TYPE_DEVICE) &&
>> +                 DEVICE_CLASS(klass)->user_creatable == false) {
>> +        warn_report("Device type '%s' is user_creatable "
>> +                    "(linked property: '%s')", type, name);
>> +    }
>>
>>      prop->child = child;
>> ---
>>
>> I get:
>>
>> $ aarch64-softmmu/qemu-system-aarch64 -M virt
>> qemu-system-aarch64: warning: Device type 'bus' is user_creatable
>> (linked property: 'parent_bus')
>> qemu-system-aarch64: warning: Device type 'qemu:memory-region' is
>> user_creatable (linked property: 'memory')
> 
> Is your warning printing the wrong thing? "bus", "irq",
> etc don't seem like names of devices...

Oh indeed I inverted (type, name) :)

> I think whether a device with a link property is
> user creatable might depend on what the property is
> for and whether the device has a useful fallback for
> "link not connected".

OK it now makes sens, reading this example:

    static void bcm2836_realize(DeviceState *dev, Error **errp)
    {
        Object *obj;
        Error *err = NULL;

        obj = object_property_get_link(OBJECT(dev), "ram", &err);
        if (obj == NULL) {
            error_setg(errp, "%s: required ram link not found: %s",
                       __func__, error_get_pretty(err));
            return;
        }
        ...

Thanks!

Phil.
Peter Maydell Oct. 12, 2018, 1:14 p.m. UTC | #3 
On 12 October 2018 at 14:06, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 12/10/2018 15:01, Peter Maydell wrote:
>> On 12 October 2018 at 13:47, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>> While looking at Mao's series
>>> (https://lists.gnu.org/archive/html/qemu-devel/2018-10/msg02519.html) I
>>> wondered if devices having link properties can be user-creatable.
>>>
>>> Using the following patch (I don't think this is correct to put qdev
>>> code into qobject, but I used this as PoC):
>>>
>>> -- >8 --
>>> diff --git a/qom/object.c b/qom/object.c
>>> index 547dcf97c3..2dd3a25531 100644
>>> --- a/qom/object.c
>>> +++ b/qom/object.c
>>> @@ -16,4 +16,5 @@
>>>  #include "qom/object_interfaces.h"
>>>  #include "qemu/cutils.h"
>>> +#include "qemu/error-report.h"
>>>  #include "qapi/visitor.h"
>>>  #include "qapi/string-input-visitor.h"
>>> @@ -1662,4 +1663,11 @@ void object_property_add_link(Object *obj, const
>>> char *name,
>>>      gchar *full_type;
>>>      ObjectProperty *op;
>>> +    ObjectClass *klass = object_get_class(obj);
>>> +
>>> +    if (klass && object_dynamic_cast(obj, TYPE_DEVICE) &&
>>> +                 DEVICE_CLASS(klass)->user_creatable == false) {
>>> +        warn_report("Device type '%s' is user_creatable "
>>> +                    "(linked property: '%s')", type, name);
>>> +    }
>>>
>>>      prop->child = child;
>>> ---
>>>
>>> I get:
>>>
>>> $ aarch64-softmmu/qemu-system-aarch64 -M virt
>>> qemu-system-aarch64: warning: Device type 'bus' is user_creatable
>>> (linked property: 'parent_bus')
>>> qemu-system-aarch64: warning: Device type 'qemu:memory-region' is
>>> user_creatable (linked property: 'memory')
>>
>> Is your warning printing the wrong thing? "bus", "irq",
>> etc don't seem like names of devices...
>
> Oh indeed I inverted (type, name) :)

Yes, but also what we want to know is the type name of
the object obj we're adding the link property to (that
is the object which is marked user-creatable). The
"type" variable here is the type of the thing being linked.
So I think printing object_get_typename(obj) would be helpful.

thanks
-- PMM
diff mbox series

Patch

diff --git a/qom/object.c b/qom/object.c
index 547dcf97c3..2dd3a25531 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -16,4 +16,5 @@ 
 #include "qom/object_interfaces.h"
 #include "qemu/cutils.h"
+#include "qemu/error-report.h"
 #include "qapi/visitor.h"
 #include "qapi/string-input-visitor.h"
@@ -1662,4 +1663,11 @@  void object_property_add_link(Object *obj, const
char *name,
     gchar *full_type;
     ObjectProperty *op;
+    ObjectClass *klass = object_get_class(obj);
+
+    if (klass && object_dynamic_cast(obj, TYPE_DEVICE) &&
+                 DEVICE_CLASS(klass)->user_creatable == false) {
+        warn_report("Device type '%s' is user_creatable "
+                    "(linked property: '%s')", type, name);
+    }