Thomas Huth <th...@redhat.com> writes:
> On 16/11/2020 14.25, Philippe Mathieu-Daudé wrote:
>> Hi Gan,
>>
>> On 11/15/20 7:49 PM, Gan Qixin wrote:
>>> Some peripherals of bcm2835 cprman have no category, put them into the
>>> 'misc'
>>> category.
>>>
>>> Signed-off-by: Gan Qixin <ganqi...@huawei.com>
>>> ---
>>> Cc: Philippe Mathieu-Daudé <f4...@amsat.org>
>>> ---
>>> hw/misc/bcm2835_cprman.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/hw/misc/bcm2835_cprman.c b/hw/misc/bcm2835_cprman.c
>>> index 7e415a017c..c62958a99e 100644
>>> --- a/hw/misc/bcm2835_cprman.c
>>> +++ b/hw/misc/bcm2835_cprman.c
>>> @@ -136,6 +136,7 @@ static void pll_class_init(ObjectClass *klass, void
>>> *data)
>>>
>>> dc->reset = pll_reset;
>>> dc->vmsd = &pll_vmstate;
>>> + set_bit(DEVICE_CATEGORY_MISC, dc->categories);
>>
>> Well, this is not an usable device but a part of a bigger device,
>> so here we want the opposite: not list this device in any category.
>>
>> Maybe we could add a DEVICE_CATEGORY_COMPOSITE for all such QOM
>> types so management apps can filter them out? (And so we are sure
>> all QOM is classified).
>>
>> Thomas, you already dealt with categorizing devices in the past,
>> what do you think about this? Who else could help? Maybe add
>> someone from libvirt in the thread?
>
> My 0.02 € : Mark the device as user_creatable = false if it can not really
> be used by the user with the -device CLI parameter. Then it also does not
> need a category. I know Markus will likely have a different opinion, but in
You're hurting my feelings! ;-P
> my eyes it's just ugly if we present devices to the users that they can not
> use.
If we believe a device should only ever be used from C, then we should
keep it away from the UI.
However, I'm wary of overloading user_creatable. Even though it has
shifted shape a number of times (cannot_instantiate_with_device_add_yet,
no_user, and now user_creatable), its purpose has always been focused:
distinguishing devices that can be instantiated by generic code from the
ones that need device-specific code. See user_creatable's comment in
qdev-core.h.
I don't want to lose that distinction. That's all.
> (By the way, this device here seems to be a decendant of TYPE_SYS_BUS_DEVICE
> ... shouldn't these show up as user_creatable = false automatically?)
Yes, unless it is a dynamic sysbus device (which I consider a flawed
concept).
But TYPE_CPRMAN_PLL is *not* a descendant of TYPE_SYS_BUS_DEVICE, it's a
bus-less device:
static const TypeInfo cprman_pll_info = {
.name = TYPE_CPRMAN_PLL,
---> .parent = TYPE_DEVICE,
.instance_size = sizeof(CprmanPllState),
.class_init = pll_class_init,
.instance_init = pll_init,
};
Unless bus-less devices are somehow usable with -device, they should
have user_creatable = false.
qdev_device_add() looks like a bus-less device is usable if the machine
provides a hotplug handler for it. Commit 03fcbd9dc5 "qdev: Check for
the availability of a hotplug controller before adding a device" seems
to be pertinent.
Are there any hotplug handlers for this device? If yes, which machines
provide one?
