On Wed, 2013-08-21 at 11:23 +0200, Markus Armbruster wrote: > 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? 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.