On Wed, Aug 21, 2013 at 01:42:58PM +0300, Marcel Apfelbaum wrote: > 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.
What's hard? Can't we assert in init? > > > > >> 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. It might be a good idea to distinguish between a host controller and a bridge. > > > > 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. At least serial devices should have their own category, there are enough of these. > > > > >> 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. >
