Re: [Qemu-devel] Re: [PATCH v3 06/17] qdev: Allow device specification by qtree path for device_del

[Jan Kiszka]

Markus Armbruster wrote:
> Jan Kiszka <address@hidden> writes:
> 
>> Luiz Capitulino wrote:
>>> On Sun, 23 May 2010 12:59:19 +0200
>>> Jan Kiszka <address@hidden> wrote:
>>>
>>>> From: Jan Kiszka <address@hidden>
>>>>
>>>> Allow to specify the device to be removed via device_del not only by ID
>>>> but also by its full or abbreviated qtree path. For this purpose,
>>>> qdev_find is introduced which combines walking the qtree with searching
>>>> for device IDs if required.
>>>  [...]
>>>
>>>>  Arguments:
>>>>  
>>>> -- "id": the device's ID (json-string)
>>>> +- "path": the device's qtree path or unique ID (json-string)
>>>>  
>>>>  Example:
>>>>  
>>>> --> { "execute": "device_del", "arguments": { "id": "net1" } }
>>>> +-> { "execute": "device_del", "arguments": { "path": "net1" } }
>>>  Doesn't seem like a good change to me, besides being incompatible[1] we
>>> shouldn't overload arguments this way in QMP as overloading leads to
>>> interface degradation (harder to use, understand, maintain).
>> It's not overloaded, think of an ID as a (weak) symbolic link in the
>> qtree filesystem. The advantage of basing everything on top of full or
>> abbreviated qtree paths is that IDs are not always assigned, paths are.
> 
> As long as your patch doesn't change the interpretation of IDs, we can
> keep the old name.
> 
> The recent review of QMP documentation may lead to a "clean up bad
> names" flag day.  One more wouldn't make it worse, I guess.
> 
>>>  Maybe we could have both arguments as optional, but one must be passed.
>> This would at least require some way to keep the proposed unified path
>> specification for the human monitor (having separate arguments there is
>> really unhandy).
> 
> Correct.
> 
> It would be nice to have device_del support paths in addition to IDs.
> I'd expect management tools to slap IDs on everything, so they won't
> care, but human users do.
> 
> As far as I know, we have two places where we let the user name a node
> in the qtree: device_add bus=X and device_del X.  The former names a
> bus, the latter a device.  But both are nodes in the same tree, so
> consistency is in order.
> 
> Only devices have user-specified IDs.  Buses have names assigned by the
> system.  Unique names, hopefully.

...but not necessarily. The bus name device_add accepts can also be a
full, thus unambiguous path.

> 
> If the user doesn't specify a device ID, the driver name is used
> instead.  If you put multiple instances of the same device on the same
> bus, they have the *same* path.  For instance, here's a snippet of info
> qtree after adding two usb-mouse:
> 
>       dev: piix3-usb-uhci, id ""
>         bus-prop: addr = 01.2
>         bus-prop: romfile = <null>
>         bus-prop: rombar = 1
>         class USB controller, addr 00:01.2, pci id 8086:7020 (sub 1af4:1100)
>         bar 4: i/o at 0xffffffffffffffff [0x1e]
>         bus: usb.0
>           type USB
>           dev: usb-hub, id ""
>             addr 0.0, speed 12, name QEMU USB Hub, attached
>           dev: usb-mouse, id "no2"
>             addr 0.0, speed 12, name QEMU USB Mouse, attached
>           dev: usb-mouse, id ""
>             addr 0.0, speed 12, name QEMU USB Mouse, attached
> 
> Both devices have the same full path
> /i440FX-pcihost/pci.0/piix3-usb-uhci/usb.0/usb-mouse
> Which one does your code pick?  Shouldn't it refuse to pick?

Patch 3 of this series resolves this as follows:

usb-mouse[.0] -> first listed instance
usb-mouse.1   -> second instance
...

We should probably include this numbering in the qtree dump, I guess.

> 
> By the way, you *can* put '/' in IDs.  I call that a bug.

Even if we prevent this, IDs can still collide with abbreviated device
or bus paths. Therefore I give paths precedence over IDs in patch 4.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux