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. > > 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". > > 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". > > 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. > Why is usb-tablet DEVICE_CATEGORY_MISC, but usb-wacom-tablet > DEVICE_CATEGORY_INPUT? This is a bug. Thanks for catching it! > > 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". > > 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. > > > > [*] I hate that.