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

[Markus Armbruster]

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.

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?

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

>> [1] It's 'legal' to break the protocol before 0.13, but this has to be
>>     coordinated with libvirt so we should have a good reason to do this
>> 
>>>  <- { "return": {} }
>>>  
>>>  EQMP
>> 
>
> Jan