On 16/11/2020 18.00, Markus Armbruster wrote:
> 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.
Well, currently we have the user_creatable flag and the hotpluggable flag. I
guess that's simply not enough.
I think in the long run, we should maybe replace the two flags with a
"creatable" type instead that could take the following values:
CREATABLE_AS_SUBDEVICE /* Device is part of another device and
can only by added by code */
CREATABLE_BY_QOM /* Some fancy new QOM function can be
used to e.g. create this as part of
a machine */
CREATABLE_BY_COLDPLUG /* For cold-plugging via -device */
CREATABLE_BY_HOTPLUG /* For hot-plugging via device_add */
... but that's likely something for the distant future...
>> (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:
Oops, I obviously looked at the wrong device in that file
(TYPE_BCM2835_CPRMAN instead of TYPE_CPRMAN_PLL) - thanks for the clarification!
Thomas
