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.

Reply via email to