From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47140) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5sb-0007Ha-Ov for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:43:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VC5sV-00045h-ED for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:43:09 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59049) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VC5sV-00045C-6q for qemu-devel@nongnu.org; Wed, 21 Aug 2013 06:43:03 -0400 Message-ID: <1377081778.1888.60.camel@localhost.localdomain> From: Marcel Apfelbaum Date: Wed, 21 Aug 2013 13:42:58 +0300 In-Reply-To: <87eh9ns8hh.fsf@blackfin.pond.sub.org> References: <1375107465-25767-1-git-send-email-marcel.a@redhat.com> <87mwol29rg.fsf@blackfin.pond.sub.org> <1377071228.1888.28.camel@localhost.localdomain> <87eh9ns8hh.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 0/3] qemu-help: improve -device command line help List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, pbonzini@redhat.com, afaerber@suse.de On Wed, 2013-08-21 at 11:23 +0200, Markus Armbruster wrote: > Marcel Apfelbaum writes: > > > On Tue, 2013-08-13 at 11:57 +0200, Markus Armbruster wrote: > >> This isn't patch review, just a couple of observations and questions. > >> > >> Current use of categories, please correct misunderstandings: > >> > >> * A device can have multiple categories. Most (all?) devices currently > >> have exactly one. > > All device have only one category for now. > > This is a preparation for multifunction devices. > > > >> > >> * -device help shows categories, like this: > >> > >> name "NAME", bus "BUS", categories "CAT1" "CAT2"... > >> > >> * -device help is sorted by category > >> > >> * -device help shows the device once per category. If the device has no > >> categories, it's not shown at all. > >> > >> Should we require devices to have at least one category? > > The whole idea of the patch was to help user navigating the command line help. > > A device category will give a user at least a hint. > > Understand. > > Devices without category are omitted from help. That's not good. > Should we require devices to have at least one category? Or should we > change help to show devices without a category? I prefer to require each device to have a category. The interesting part is how to enforce it. > > >> Eric, does libvirt still parse -device help? If yes, can it cope with > >> the addition of "categories ..."? > > Also the "old" parsing mechanism would still work, it splits the raw > > by "," and looks for the key like "name". > > > >> > >> A possibly better way to group help by category: instead of adding > >> categories to each line, add category headlines, like this: > >> > >> Controller/Bridge/Hub devices: > >> name "NAME", bus "BUS"... > >> ... > >> USB devices: > >> name "NAME", bus "BUS"... > >> ... > >> Storage devices: > >> ... > >> > >> This way, showing devices with multiple categories once per category > >> actually makes sense. > > You are right. This is a very good "next step". > > I'd love to see a patch from you :) On my to-do list ... > > >> DEVICE_CATEGORY_STORAGE comprises both storage controller devices > >> (providing storage buses such as IDE, SCSI) and storage devices > >> (plugging into such buses). Some of our devices (*-fdc, virtio-blk) > >> integrate both in one device model[*]. > > Yes, it does comprises both. It still helps the user that can now > > grep by this storage category and select from it rather than > > going on all the help. > > > >> > >> DEVICE_CATEGORY_USB comprises *only* host controller devices (providing > >> USB bus(es)), *not* USB devices (plugging into USB bus). These are > >> categorized by function instead: > > The "USB" is used here as a code-name rather than the BUS. > > It was never my intention to clone the bus type. It is already > > part of the description. > > > >> > >> * DEVICE_CATEGORY_BRIDGE: usb-host, usb-hub > >> > >> * DEVICE_CATEGORY_STORAGE: usb-bot, usb-uas, usb-storage > >> > >> * DEVICE_CATEGORY_NETWORK: usb-bt-dongle, usb-net > >> > >> * DEVICE_CATEGORY_INPUT: usb-kbd, usb-ccid, usb-wacom-tablet, > >> usb-braille, usb-mouse, usb-serial > >> > >> * DEVICE_CATEGORY_SOUND: usb-audio > >> > >> * DEVICE_CATEGORY_MISC: usb-tablet, usb-redir > >> > >> Should they additionally be DEVICE_CATEGORY_USB? > > As mentioned earlier, better if not (in my opinion.) > > > >> > >> Why do we have DEVICE_CATEGORY_USB, but no categories for other buses, > >> like PCI or ISA? Devices providing such buses are > >> DEVICE_CATEGORY_BRIDGE. Why is USB different? > > Again, we already have the bus information, I was looking for > > functional info. "USB" was not used here as a BUS, but like a > > standalone "function". > > Let me rephrase. Why do we have a category for devices bridging to a > USB bus (USB host controllers), but don't have categories for devices > bridging to other buses? > > Perhaps a possible answer is "because we have so many USB host > controllers, but usually only few to no user-selectable options for the > other buses". Just thinking aloud; I'm not sure it's true. It is true, it was the exact purpose for it. > > >> Why is usb-host DEVICE_CATEGORY_BRIDGE? > > The category is named "Controller/Bridge/Hub" at command line > > I didn't want the name to be too long in the code. > > I can't see how USB host device fits "Controller/Bridge/Hub"... I am open to suggestions. > > The PCI device passhtrough devices kvm-pci-assign and vfio-pci are both > DEVICE_CATEGORY_MISC. Any problem with this? I think the Misc Category is appropriate for them. > > >> Why is usb-tablet DEVICE_CATEGORY_MISC, but usb-wacom-tablet > >> DEVICE_CATEGORY_INPUT? > > This is a bug. Thanks for catching it! > > You're welcome :) > > Will you send a patch? Sure. Soon enough :) > > >> DEVICE_CATEGORY_INPUT is weird. Some devices in that category are truly > >> about input (usb-mouse, usb-kbd), others are at least as often used for > >> output (serial devices, PIOs)... > > It makes sense to rename it to "Input/Output". > > Looks like the category comprises what we call character devices. > Perhaps not the friendliest term for casual users, but we already use it > in our documentation. And here is my problem. I (maybe) can infer from "char device" that it refers to input/output devices, but to expose it to user it is not user friendly/helpful in any way. The code constant may be DEVICE_CATEGORY_CHARACTER but we need a more meaningful name for the user. > > >> The difference between DEVICE_CATEGORY_INPUT and DEVICE_CATEGORY_MISC > >> seems unclear (see usb-tablet vs. usb-wacom-tablet above). > > Putting the bug aside, MISC is the category for devices that does > > not match a specific category. > > > > > > Thanks for the review Markus! > > The bottom line is that I wanted to help users in their adventure to form > > the command line by creating "Categories" that would split the 145 help rows > > in batches of ~20. In this way the user can first select the desired > > category and then choose the device. > > Improvement, very much appreciated. Thanks > > >> [*] I hate that.