[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] Re: [PATCH v3 06/17] qdev: Allow device specification b
From: |
Jan Kiszka |
Subject: |
Re: [Qemu-devel] Re: [PATCH v3 06/17] qdev: Allow device specification by qtree path for device_del |
Date: |
Sat, 29 May 2010 10:16:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686 (x86_64); de; rv:1.8.1.12) Gecko/20080226 SUSE/2.0.0.12-1.1 Thunderbird/2.0.0.12 Mnenhy/0.7.5.666 |
Markus Armbruster wrote:
> Jan Kiszka <address@hidden> writes:
>
>> 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.
>
> You're fixing problems in the overly complex and subtle path name lookup
> by making it more complex and subtle. Let's make it simple and
> straightforward instead.
I have no problem with ripping out all of those abbreviations, requiring
the user to either specify a '/'-less unique ID or a full qtree path
that must start with a '/'. If paths get long, we now have interactive
completions.
Jan
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v3 02/17] qdev: Fix scanning across single-bus devices, (continued)
[Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Jan Kiszka, 2010/05/23
- Re: [Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Markus Armbruster, 2010/05/29
- Re: [Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Avi Kivity, 2010/05/30
- Re: [Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Markus Armbruster, 2010/05/31
- Re: [Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Gerd Hoffmann, 2010/05/31
- Re: [Qemu-devel] [PATCH v3 04/17] qdev: Give qtree names precedence over user-assigned IDs, Markus Armbruster, 2010/05/31
- [Qemu-devel] [PATCH] qdev: Reject duplicate and anti-social device IDs, Markus Armbruster, 2010/05/31
- [Qemu-devel] Re: [PATCH] qdev: Reject duplicate and anti-social device IDs, Gerd Hoffmann, 2010/05/31
[Qemu-devel] [PATCH v3 05/17] qdev: Convert device and bus lists to QTAILQ, Jan Kiszka, 2010/05/23
[Qemu-devel] [PATCH v3 01/17] Add dependency of JSON unit tests on config-host.h, Jan Kiszka, 2010/05/23