Marcel Apfelbaum <marce...@redhat.com> 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? >> 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 :) >> 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. >> 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"... The PCI device passhtrough devices kvm-pci-assign and vfio-pci are both DEVICE_CATEGORY_MISC. >> 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? >> 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. >> 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. >> [*] I hate that.