From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=48427 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1OIOIT-00033L-KA for qemu-devel@nongnu.org; Sat, 29 May 2010 11:50:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1OIHDZ-0002yq-FY for qemu-devel@nongnu.org; Sat, 29 May 2010 04:16:30 -0400 Received: from fmmailgate03.web.de ([217.72.192.234]:51795) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1OIHDZ-0002yc-3F for qemu-devel@nongnu.org; Sat, 29 May 2010 04:16:29 -0400 Message-ID: <4C00CD5B.4080607@web.de> Date: Sat, 29 May 2010 10:16:27 +0200 From: Jan Kiszka MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH v3 06/17] qdev: Allow device specification by qtree path for device_del References: <20100527163618.2fcd95a8@redhat.com> <4BFEF004.90800@web.de> <4BFFD989.9040803@siemens.com> In-Reply-To: Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig8732BDDB3D4C300FC4DF6176" Sender: jan.kiszka@web.de List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Anthony Liguori , Juan Quintela , Jan Kiszka , "qemu-devel@nongnu.org" , Luiz Capitulino , Blue Swirl , Avi Kivity This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig8732BDDB3D4C300FC4DF6176 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Markus Armbruster wrote: > Jan Kiszka writes: >=20 >> Markus Armbruster wrote: >>> Jan Kiszka writes: >>> >>>> Luiz Capitulino wrote: >>>>> On Sun, 23 May 2010 12:59:19 +0200 >>>>> Jan Kiszka wrote: >>>>> >>>>>> From: Jan Kiszka >>>>>> >>>>>> 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 sear= ching >>>>>> for device IDs if required. >>>>> [...] >>>>> >>>>>> Arguments: >>>>>> =20 >>>>>> -- "id": the device's ID (json-string) >>>>>> +- "path": the device's qtree path or unique ID (json-string) >>>>>> =20 >>>>>> Example: >>>>>> =20 >>>>>> --> { "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 t= o >>>>> 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 a= re. >>> As long as your patch doesn't change the interpretation of IDs, we ca= n >>> 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 pa= ssed. >>>> This would at least require some way to keep the proposed unified pa= th >>>> 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 nod= e >>> in the qtree: device_add bus=3DX 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 t= he >>> 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 sam= e >>> bus, they have the *same* path. For instance, here's a snippet of in= fo >>> qtree after adding two usb-mouse: >>> >>> dev: piix3-usb-uhci, id "" >>> bus-prop: addr =3D 01.2 >>> bus-prop: romfile =3D >>> bus-prop: rombar =3D 1 >>> class USB controller, addr 00:01.2, pci id 8086:7020 (sub 1af= 4: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. >=20 > You're fixing problems in the overly complex and subtle path name looku= p > 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 --------------enig8732BDDB3D4C300FC4DF6176 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkwAzVsACgkQitSsb3rl5xTbWQCgrcAFA1cM5RNbRHPVmMGmq7JW vqEAnRvY0NiebH5YVfEe7ihCeKENUdDA =ALSc -----END PGP SIGNATURE----- --------------enig8732BDDB3D4C300FC4DF6176--