[Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Marcel Apfelbaum]

Running qemu with "-device ?" option returns ~145 lines.
It is hard to manage understanding the output.

Theses patches aim to partially solve the problem by dividing the devices
into logical categories like "Network/Display/..." and sorting them by it.

Marcel Apfelbaum (2):
  qemu-help: Sort devices by logical functionality
  devices: Associate devices to their logical category

 hw/audio/ac97.c         |  1 +
 hw/display/cirrus_vga.c |  1 +
 hw/net/eepro100.c       |  1 +
 hw/scsi/scsi-disk.c     |  4 ++++
 hw/usb/dev-hid.c        |  1 +
 include/hw/qdev-core.h  |  7 +++++++
 qdev-monitor.c          | 23 ++++++++++++++++++++++-
 7 files changed, 37 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Marcel Apfelbaum]

Categorize devices that appear as output to "-device ?" command
by logical functionality. Sort the devices by logical categories
before showing them to user.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 include/hw/qdev-core.h |  7 +++++++
 qdev-monitor.c         | 23 ++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..4f7a9b8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -17,6 +17,12 @@ enum {
 #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
 #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
 
+#define DEVICE_CATEGORY_STORAGE "storage"
+#define DEVICE_CATEGORY_NETWORK "network"
+#define DEVICE_CATEGORY_INPUT "input"
+#define DEVICE_CATEGORY_DISPLAY "display"
+#define DEVICE_CATEGORY_SOUND "sound"
+
 typedef int (*qdev_initfn)(DeviceState *dev);
 typedef int (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +87,7 @@ typedef struct DeviceClass {
     /*< public >*/
 
     const char *fw_name;
+    const char *category;
     const char *desc;
     Property *props;
     int no_user;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..1446b6e 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
     if (qdev_class_has_alias(dc)) {
         error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
     }
+    if (dc->category) {
+        error_printf(", category \"%s\"", dc->category);
+    }
     if (dc->desc) {
         error_printf(", desc \"%s\"", dc->desc);
     }
@@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
     return NULL;
 }
 
+static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)
+{
+    DeviceClass *dc1, *dc2;
+
+    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
+                                                   TYPE_DEVICE);
+    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
+                                                   TYPE_DEVICE);
+
+    return g_strcmp0(dc1->category, dc2->category);
+}
+
 int qdev_device_help(QemuOpts *opts)
 {
     const char *driver;
     Property *prop;
     ObjectClass *klass;
+    GSList *list;
 
     driver = qemu_opt_get(opts, "driver");
     if (driver && is_help_option(driver)) {
         bool show_no_user = false;
-        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
+
+        list = object_class_get_list(TYPE_DEVICE, false);
+        list = g_slist_sort(list, qdev_device_compare);
+        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
+        g_slist_free(list);
+
         return 1;
     }
 
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Michael S. Tsirkin]

On Thu, Jul 18, 2013 at 11:27:42AM +0300, Marcel Apfelbaum wrote:
> Categorize devices that appear as output to "-device ?" command
> by logical functionality. Sort the devices by logical categories
> before showing them to user.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/hw/qdev-core.h |  7 +++++++
>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 7fbffcb..4f7a9b8 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -17,6 +17,12 @@ enum {
>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>  
> +#define DEVICE_CATEGORY_STORAGE "storage"
> +#define DEVICE_CATEGORY_NETWORK "network"
> +#define DEVICE_CATEGORY_INPUT "input"
> +#define DEVICE_CATEGORY_DISPLAY "display"
> +#define DEVICE_CATEGORY_SOUND "sound"
> +
>  typedef int (*qdev_initfn)(DeviceState *dev);
>  typedef int (*qdev_event)(DeviceState *dev);
>  typedef void (*qdev_resetfn)(DeviceState *dev);
> @@ -81,6 +87,7 @@ typedef struct DeviceClass {
>      /*< public >*/
>  
>      const char *fw_name;
> +    const char *category;
>      const char *desc;
>      Property *props;
>      int no_user;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index e54dbc2..1446b6e 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
>      if (qdev_class_has_alias(dc)) {
>          error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
>      }
> +    if (dc->category) {
> +        error_printf(", category \"%s\"", dc->category);
> +    }
>      if (dc->desc) {
>          error_printf(", desc \"%s\"", dc->desc);
>      }
> @@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
>      return NULL;
>  }
>  
> +static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)

A slightly better name would be qdev_category_compare

> +{
> +    DeviceClass *dc1, *dc2;
> +
> +    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
> +                                                   TYPE_DEVICE);
> +    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
> +                                                   TYPE_DEVICE);
> +
> +    return g_strcmp0(dc1->category, dc2->category);
> +}
> +
>  int qdev_device_help(QemuOpts *opts)
>  {
>      const char *driver;
>      Property *prop;
>      ObjectClass *klass;
> +    GSList *list;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (driver && is_help_option(driver)) {
>          bool show_no_user = false;
> -        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
> +
> +        list = object_class_get_list(TYPE_DEVICE, false);
> +        list = g_slist_sort(list, qdev_device_compare);
> +        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
> +        g_slist_free(list);
> +
>          return 1;
>      }
>  
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Anthony Liguori]

Marcel Apfelbaum <marcel.a@redhat.com> writes:

> Categorize devices that appear as output to "-device ?" command
> by logical functionality. Sort the devices by logical categories
> before showing them to user.
>
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  include/hw/qdev-core.h |  7 +++++++
>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
>  2 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 7fbffcb..4f7a9b8 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -17,6 +17,12 @@ enum {
>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>  
> +#define DEVICE_CATEGORY_STORAGE "storage"
> +#define DEVICE_CATEGORY_NETWORK "network"
> +#define DEVICE_CATEGORY_INPUT "input"
> +#define DEVICE_CATEGORY_DISPLAY "display"
> +#define DEVICE_CATEGORY_SOUND "sound"
> +

Looks reasonable, but please make this a bitmap.  There are cases,
particularly if we start modeling multifunction PCI cards as a single
device, where a single device can support multiple types of
functionality.

Regards,

Anthony Liguori

>  typedef int (*qdev_initfn)(DeviceState *dev);
>  typedef int (*qdev_event)(DeviceState *dev);
>  typedef void (*qdev_resetfn)(DeviceState *dev);
> @@ -81,6 +87,7 @@ typedef struct DeviceClass {
>      /*< public >*/
>  
>      const char *fw_name;
> +    const char *category;
>      const char *desc;
>      Property *props;
>      int no_user;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index e54dbc2..1446b6e 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
>      if (qdev_class_has_alias(dc)) {
>          error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
>      }
> +    if (dc->category) {
> +        error_printf(", category \"%s\"", dc->category);
> +    }
>      if (dc->desc) {
>          error_printf(", desc \"%s\"", dc->desc);
>      }
> @@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
>      return NULL;
>  }
>  
> +static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)
> +{
> +    DeviceClass *dc1, *dc2;
> +
> +    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
> +                                                   TYPE_DEVICE);
> +    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
> +                                                   TYPE_DEVICE);
> +
> +    return g_strcmp0(dc1->category, dc2->category);
> +}
> +
>  int qdev_device_help(QemuOpts *opts)
>  {
>      const char *driver;
>      Property *prop;
>      ObjectClass *klass;
> +    GSList *list;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (driver && is_help_option(driver)) {
>          bool show_no_user = false;
> -        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
> +
> +        list = object_class_get_list(TYPE_DEVICE, false);
> +        list = g_slist_sort(list, qdev_device_compare);
> +        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
> +        g_slist_free(list);
> +
>          return 1;
>      }
>  
> -- 
> 1.8.3.1

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Marcel Apfelbaum]

On Thu, 2013-07-18 at 09:28 -0500, Anthony Liguori wrote:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> 
> > Categorize devices that appear as output to "-device ?" command
> > by logical functionality. Sort the devices by logical categories
> > before showing them to user.
> >
> > Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> > Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> >  include/hw/qdev-core.h |  7 +++++++
> >  qdev-monitor.c         | 23 ++++++++++++++++++++++-
> >  2 files changed, 29 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> > index 7fbffcb..4f7a9b8 100644
> > --- a/include/hw/qdev-core.h
> > +++ b/include/hw/qdev-core.h
> > @@ -17,6 +17,12 @@ enum {
> >  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
> >  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
> >  
> > +#define DEVICE_CATEGORY_STORAGE "storage"
> > +#define DEVICE_CATEGORY_NETWORK "network"
> > +#define DEVICE_CATEGORY_INPUT "input"
> > +#define DEVICE_CATEGORY_DISPLAY "display"
> > +#define DEVICE_CATEGORY_SOUND "sound"
> > +
> 
> Looks reasonable, but please make this a bitmap.  There are cases,
> particularly if we start modeling multifunction PCI cards as a single
> device, where a single device can support multiple types of
> functionality.

Antony, thanks for your review!
The whole point was to find a way to differentiate them by
functionality so they can be sorted...
Is it possible that a multifunction pci card will be used
for more then one category mentioned above?
I agree the list may not be exhaustive, but I really hope
I'll find a way to sort them in such a way that a
device will not fall under more than one category

Marcel

> 
> Regards,
> 
> Anthony Liguori
> 
> >  typedef int (*qdev_initfn)(DeviceState *dev);
> >  typedef int (*qdev_event)(DeviceState *dev);
> >  typedef void (*qdev_resetfn)(DeviceState *dev);
> > @@ -81,6 +87,7 @@ typedef struct DeviceClass {
> >      /*< public >*/
> >  
> >      const char *fw_name;
> > +    const char *category;
> >      const char *desc;
> >      Property *props;
> >      int no_user;
> > diff --git a/qdev-monitor.c b/qdev-monitor.c
> > index e54dbc2..1446b6e 100644
> > --- a/qdev-monitor.c
> > +++ b/qdev-monitor.c
> > @@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
> >      if (qdev_class_has_alias(dc)) {
> >          error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
> >      }
> > +    if (dc->category) {
> > +        error_printf(", category \"%s\"", dc->category);
> > +    }
> >      if (dc->desc) {
> >          error_printf(", desc \"%s\"", dc->desc);
> >      }
> > @@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
> >      return NULL;
> >  }
> >  
> > +static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)
> > +{
> > +    DeviceClass *dc1, *dc2;
> > +
> > +    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
> > +                                                   TYPE_DEVICE);
> > +    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
> > +                                                   TYPE_DEVICE);
> > +
> > +    return g_strcmp0(dc1->category, dc2->category);
> > +}
> > +
> >  int qdev_device_help(QemuOpts *opts)
> >  {
> >      const char *driver;
> >      Property *prop;
> >      ObjectClass *klass;
> > +    GSList *list;
> >  
> >      driver = qemu_opt_get(opts, "driver");
> >      if (driver && is_help_option(driver)) {
> >          bool show_no_user = false;
> > -        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
> > +
> > +        list = object_class_get_list(TYPE_DEVICE, false);
> > +        list = g_slist_sort(list, qdev_device_compare);
> > +        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
> > +        g_slist_free(list);
> > +
> >          return 1;
> >      }
> >  
> > -- 
> > 1.8.3.1
> 

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Paolo Bonzini]

Il 18/07/2013 16:42, Marcel Apfelbaum ha scritto:
> On Thu, 2013-07-18 at 09:28 -0500, Anthony Liguori wrote:
>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>>
>>> Categorize devices that appear as output to "-device ?" command
>>> by logical functionality. Sort the devices by logical categories
>>> before showing them to user.
>>>
>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>  include/hw/qdev-core.h |  7 +++++++
>>>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
>>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>> index 7fbffcb..4f7a9b8 100644
>>> --- a/include/hw/qdev-core.h
>>> +++ b/include/hw/qdev-core.h
>>> @@ -17,6 +17,12 @@ enum {
>>>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>>>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>>>  
>>> +#define DEVICE_CATEGORY_STORAGE "storage"
>>> +#define DEVICE_CATEGORY_NETWORK "network"
>>> +#define DEVICE_CATEGORY_INPUT "input"
>>> +#define DEVICE_CATEGORY_DISPLAY "display"
>>> +#define DEVICE_CATEGORY_SOUND "sound"
>>> +
>>
>> Looks reasonable, but please make this a bitmap.  There are cases,
>> particularly if we start modeling multifunction PCI cards as a single
>> device, where a single device can support multiple types of
>> functionality.
> 
> Antony, thanks for your review!
> The whole point was to find a way to differentiate them by
> functionality so they can be sorted...
> Is it possible that a multifunction pci card will be used
> for more then one category mentioned above?

Yes, for example your laptop's GPU probably has an audio function too.

> I agree the list may not be exhaustive, but I really hope
> I'll find a way to sort them in such a way that a
> device will not fall under more than one category

If you sort the bit definitions in alphabetic order (audio=bit 30,
display=bit 29, etc. for example), the resulting integer sort should
also yield alphabetic order.  But perhaps it's not a smart idea if we
take into account future localization of the help text.

Paolo

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 18/07/2013 16:42, Marcel Apfelbaum ha scritto:
>> On Thu, 2013-07-18 at 09:28 -0500, Anthony Liguori wrote:
>>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>>>
>>>> Categorize devices that appear as output to "-device ?" command
>>>> by logical functionality. Sort the devices by logical categories
>>>> before showing them to user.
>>>>
>>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>>> ---
>>>>  include/hw/qdev-core.h |  7 +++++++
>>>>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
>>>>  2 files changed, 29 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>>>> index 7fbffcb..4f7a9b8 100644
>>>> --- a/include/hw/qdev-core.h
>>>> +++ b/include/hw/qdev-core.h
>>>> @@ -17,6 +17,12 @@ enum {
>>>>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>>>>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>>>>  
>>>> +#define DEVICE_CATEGORY_STORAGE "storage"
>>>> +#define DEVICE_CATEGORY_NETWORK "network"
>>>> +#define DEVICE_CATEGORY_INPUT "input"
>>>> +#define DEVICE_CATEGORY_DISPLAY "display"
>>>> +#define DEVICE_CATEGORY_SOUND "sound"
>>>> +
>>>
>>> Looks reasonable, but please make this a bitmap.  There are cases,
>>> particularly if we start modeling multifunction PCI cards as a single
>>> device, where a single device can support multiple types of
>>> functionality.
>> 
>> Antony, thanks for your review!
>> The whole point was to find a way to differentiate them by
>> functionality so they can be sorted...
>> Is it possible that a multifunction pci card will be used
>> for more then one category mentioned above?
>
> Yes, for example your laptop's GPU probably has an audio function too.

Another example is a converged network adapter.  It's a single device
with an ethernet port but it exposes two physical functions--one NIC and
one fibre channel device via FCoE.

The PIIX chipset and ICH9 are multifunction devices but we model them as
separate devices today.   It's quite likely that in the very near future
we'll fix that.

>> I agree the list may not be exhaustive, but I really hope
>> I'll find a way to sort them in such a way that a
>> device will not fall under more than one category
>
> If you sort the bit definitions in alphabetic order (audio=bit 30,
> display=bit 29, etc. for example), the resulting integer sort should
> also yield alphabetic order.  But perhaps it's not a smart idea if we
> take into account future localization of the help text.

I was thinking the same thing.  Could also just cheat and make the sort
function a bit more complicated.

Regards,

Anthony Liguori

>
> Paolo

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Marcel Apfelbaum]

On Thu, 2013-07-18 at 09:58 -0500, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 18/07/2013 16:42, Marcel Apfelbaum ha scritto:
> >> On Thu, 2013-07-18 at 09:28 -0500, Anthony Liguori wrote:
> >>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> >>>
> >>>> Categorize devices that appear as output to "-device ?" command
> >>>> by logical functionality. Sort the devices by logical categories
> >>>> before showing them to user.
> >>>>
> >>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> >>>> ---
> >>>>  include/hw/qdev-core.h |  7 +++++++
> >>>>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
> >>>>  2 files changed, 29 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> >>>> index 7fbffcb..4f7a9b8 100644
> >>>> --- a/include/hw/qdev-core.h
> >>>> +++ b/include/hw/qdev-core.h
> >>>> @@ -17,6 +17,12 @@ enum {
> >>>>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
> >>>>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
> >>>>  
> >>>> +#define DEVICE_CATEGORY_STORAGE "storage"
> >>>> +#define DEVICE_CATEGORY_NETWORK "network"
> >>>> +#define DEVICE_CATEGORY_INPUT "input"
> >>>> +#define DEVICE_CATEGORY_DISPLAY "display"
> >>>> +#define DEVICE_CATEGORY_SOUND "sound"
> >>>> +
> >>>
> >>> Looks reasonable, but please make this a bitmap.  There are cases,
> >>> particularly if we start modeling multifunction PCI cards as a single
> >>> device, where a single device can support multiple types of
> >>> functionality.
> >> 
> >> Antony, thanks for your review!
> >> The whole point was to find a way to differentiate them by
> >> functionality so they can be sorted...
> >> Is it possible that a multifunction pci card will be used
> >> for more then one category mentioned above?
> >
> > Yes, for example your laptop's GPU probably has an audio function too.
> 
> Another example is a converged network adapter.  It's a single device
> with an ethernet port but it exposes two physical functions--one NIC and
> one fibre channel device via FCoE.
I agree that these are different device types, but they have the same category:
"Network"
> 
> The PIIX chipset and ICH9 are multifunction devices but we model them as
> separate devices today.   It's quite likely that in the very near future
> we'll fix that.
Interesting, thanks, in one hand I can call them both "Controller",
on the other hand the bitmap can be used for creating subcategories:
For example: "Storage, Controller" and "I/O, Controller"

> 
> >> I agree the list may not be exhaustive, but I really hope
> >> I'll find a way to sort them in such a way that a
> >> device will not fall under more than one category
> >
> > If you sort the bit definitions in alphabetic order (audio=bit 30,
> > display=bit 29, etc. for example), the resulting integer sort should
> > also yield alphabetic order.  But perhaps it's not a smart idea if we
> > take into account future localization of the help text.
> 
> I was thinking the same thing.  Could also just cheat and make the sort
> function a bit more complicated.
The suggested solution works fair enough, thanks!
Marcel 

> 
> Regards,
> 
> Anthony Liguori
> 
> >
> > Paolo
> 

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Paolo Bonzini]

Il 18/07/2013 17:23, Marcel Apfelbaum ha scritto:
>>>> Antony, thanks for your review!
>>>> The whole point was to find a way to differentiate them by
>>>> functionality so they can be sorted...
>>>> Is it possible that a multifunction pci card will be used
>>>> for more then one category mentioned above?
>>>
>>> Yes, for example your laptop's GPU probably has an audio function too.
>>
>> Another example is a converged network adapter.  It's a single device
>> with an ethernet port but it exposes two physical functions--one NIC and
>> one fibre channel device via FCoE.
> I agree that these are different device types, but they have the same category:
> "Network"

I think fibre channel is usually considered Storage.

Paolo

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Michael S. Tsirkin]

On Thu, Jul 18, 2013 at 09:58:59AM -0500, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 18/07/2013 16:42, Marcel Apfelbaum ha scritto:
> >> On Thu, 2013-07-18 at 09:28 -0500, Anthony Liguori wrote:
> >>> Marcel Apfelbaum <marcel.a@redhat.com> writes:
> >>>
> >>>> Categorize devices that appear as output to "-device ?" command
> >>>> by logical functionality. Sort the devices by logical categories
> >>>> before showing them to user.
> >>>>
> >>>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> >>>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
> >>>> ---
> >>>>  include/hw/qdev-core.h |  7 +++++++
> >>>>  qdev-monitor.c         | 23 ++++++++++++++++++++++-
> >>>>  2 files changed, 29 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> >>>> index 7fbffcb..4f7a9b8 100644
> >>>> --- a/include/hw/qdev-core.h
> >>>> +++ b/include/hw/qdev-core.h
> >>>> @@ -17,6 +17,12 @@ enum {
> >>>>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
> >>>>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
> >>>>  
> >>>> +#define DEVICE_CATEGORY_STORAGE "storage"
> >>>> +#define DEVICE_CATEGORY_NETWORK "network"
> >>>> +#define DEVICE_CATEGORY_INPUT "input"
> >>>> +#define DEVICE_CATEGORY_DISPLAY "display"
> >>>> +#define DEVICE_CATEGORY_SOUND "sound"
> >>>> +
> >>>
> >>> Looks reasonable, but please make this a bitmap.  There are cases,
> >>> particularly if we start modeling multifunction PCI cards as a single
> >>> device, where a single device can support multiple types of
> >>> functionality.
> >> 
> >> Antony, thanks for your review!
> >> The whole point was to find a way to differentiate them by
> >> functionality so they can be sorted...
> >> Is it possible that a multifunction pci card will be used
> >> for more then one category mentioned above?
> >
> > Yes, for example your laptop's GPU probably has an audio function too.
> 
> Another example is a converged network adapter.  It's a single device
> with an ethernet port but it exposes two physical functions--one NIC and
> one fibre channel device via FCoE.
> 
> The PIIX chipset and ICH9 are multifunction devices but we model them as
> separate devices today.   It's quite likely that in the very near future
> we'll fix that.
> 
> >> I agree the list may not be exhaustive, but I really hope
> >> I'll find a way to sort them in such a way that a
> >> device will not fall under more than one category
> >
> > If you sort the bit definitions in alphabetic order (audio=bit 30,
> > display=bit 29, etc. for example), the resulting integer sort should
> > also yield alphabetic order.  But perhaps it's not a smart idea if we
> > take into account future localization of the help text.
> 
> I was thinking the same thing.  Could also just cheat and make the sort
> function a bit more complicated.
> 
> Regards,
> 
> Anthony Liguori

No, I think a multi-purpose device should appear in multiple
places, not be lumped with one randomly selected category.
E.g. storage/network controller should appear
under both storage and network. Otherwise how will users
find it?






> >
> > Paolo
> 

Re: [Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Ronen Hod]

On 07/18/2013 05:28 PM, Anthony Liguori wrote:
> Marcel Apfelbaum <marcel.a@redhat.com> writes:
>
>> Categorize devices that appear as output to "-device ?" command
>> by logical functionality. Sort the devices by logical categories
>> before showing them to user.
>>
>> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
>> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   include/hw/qdev-core.h |  7 +++++++
>>   qdev-monitor.c         | 23 ++++++++++++++++++++++-
>>   2 files changed, 29 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
>> index 7fbffcb..4f7a9b8 100644
>> --- a/include/hw/qdev-core.h
>> +++ b/include/hw/qdev-core.h
>> @@ -17,6 +17,12 @@ enum {
>>   #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>>   #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>>   
>> +#define DEVICE_CATEGORY_STORAGE "storage"
>> +#define DEVICE_CATEGORY_NETWORK "network"
>> +#define DEVICE_CATEGORY_INPUT "input"
>> +#define DEVICE_CATEGORY_DISPLAY "display"
>> +#define DEVICE_CATEGORY_SOUND "sound"
>> +
> Looks reasonable, but please make this a bitmap.  There are cases,
> particularly if we start modeling multifunction PCI cards as a single
> device, where a single device can support multiple types of
> functionality.

I agree.
On the one hand, there is no proper sorting by DEVICE_CATEGORY for devices
with multiple functions, but OTOH that's the reality.
If you add get_all_devices_by_category(dev_category), then it can be useful as is,
and also for looping over all the categories for printout. It will output those devices
in multiple categories, which is what we want.

Thanks, Ronen.

>
> Regards,
>
> Anthony Liguori
>
>>   typedef int (*qdev_initfn)(DeviceState *dev);
>>   typedef int (*qdev_event)(DeviceState *dev);
>>   typedef void (*qdev_resetfn)(DeviceState *dev);
>> @@ -81,6 +87,7 @@ typedef struct DeviceClass {
>>       /*< public >*/
>>   
>>       const char *fw_name;
>> +    const char *category;
>>       const char *desc;
>>       Property *props;
>>       int no_user;
>> diff --git a/qdev-monitor.c b/qdev-monitor.c
>> index e54dbc2..1446b6e 100644
>> --- a/qdev-monitor.c
>> +++ b/qdev-monitor.c
>> @@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
>>       if (qdev_class_has_alias(dc)) {
>>           error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
>>       }
>> +    if (dc->category) {
>> +        error_printf(", category \"%s\"", dc->category);
>> +    }
>>       if (dc->desc) {
>>           error_printf(", desc \"%s\"", dc->desc);
>>       }
>> @@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
>>       return NULL;
>>   }
>>   
>> +static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)
>> +{
>> +    DeviceClass *dc1, *dc2;
>> +
>> +    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
>> +                                                   TYPE_DEVICE);
>> +    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
>> +                                                   TYPE_DEVICE);
>> +
>> +    return g_strcmp0(dc1->category, dc2->category);
>> +}
>> +
>>   int qdev_device_help(QemuOpts *opts)
>>   {
>>       const char *driver;
>>       Property *prop;
>>       ObjectClass *klass;
>> +    GSList *list;
>>   
>>       driver = qemu_opt_get(opts, "driver");
>>       if (driver && is_help_option(driver)) {
>>           bool show_no_user = false;
>> -        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
>> +
>> +        list = object_class_get_list(TYPE_DEVICE, false);
>> +        list = g_slist_sort(list, qdev_device_compare);
>> +        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
>> +        g_slist_free(list);
>> +
>>           return 1;
>>       }
>>   
>> -- 
>> 1.8.3.1
>

[Qemu-devel] [RFC PATCH 2/2] devices: Associate devices to their logical category

[Marcel Apfelbaum]

The category will be used to sort the devices displayed in
the command line help.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
Note that these are not all the needed changes, the only purpose of
this patch is to be a proof of concept.

 hw/audio/ac97.c         | 1 +
 hw/display/cirrus_vga.c | 1 +
 hw/net/eepro100.c       | 1 +
 hw/scsi/scsi-disk.c     | 4 ++++
 hw/usb/dev-hid.c        | 1 +
 5 files changed, 8 insertions(+)

diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index 365b2f1..cbedf11 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1420,6 +1420,7 @@ static void ac97_class_init (ObjectClass *klass, void *data)
     k->device_id = PCI_DEVICE_ID_INTEL_82801AA_5;
     k->revision = 0x01;
     k->class_id = PCI_CLASS_MULTIMEDIA_AUDIO;
+    dc->category = DEVICE_CATEGORY_SOUND;
     dc->desc = "Intel 82801AA AC97 Audio";
     dc->vmsd = &vmstate_ac97;
     dc->props = ac97_properties;
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index a440575..0f4be70 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -3002,6 +3002,7 @@ static void cirrus_vga_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_CIRRUS;
     k->device_id = CIRRUS_ID_CLGD5446;
     k->class_id = PCI_CLASS_DISPLAY_VGA;
+    dc->category = DEVICE_CATEGORY_DISPLAY;
     dc->desc = "Cirrus CLGD 54xx VGA";
     dc->vmsd = &vmstate_pci_cirrus_vga;
     dc->props = pci_vga_cirrus_properties;
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index e0befb2..944a7ac 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -2084,6 +2084,7 @@ static void eepro100_class_init(ObjectClass *klass, void *data)
     info = eepro100_get_class_by_name(object_class_get_name(klass));
 
     dc->props = e100_properties;
+    dc->category = DEVICE_CATEGORY_NETWORK;
     dc->desc = info->desc;
     k->vendor_id = PCI_VENDOR_ID_INTEL;
     k->class_id = PCI_CLASS_NETWORK_ETHERNET;
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index 74e6a14..3b2cd99 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2428,6 +2428,7 @@ static void scsi_hd_class_initfn(ObjectClass *klass, void *data)
     sc->alloc_req    = scsi_new_request;
     sc->unit_attention_reported = scsi_disk_unit_attention_reported;
     dc->fw_name = "disk";
+    dc->category = DEVICE_CATEGORY_STORAGE;
     dc->desc = "virtual SCSI disk";
     dc->reset = scsi_disk_reset;
     dc->props = scsi_hd_properties;
@@ -2457,6 +2458,7 @@ static void scsi_cd_class_initfn(ObjectClass *klass, void *data)
     sc->alloc_req    = scsi_new_request;
     sc->unit_attention_reported = scsi_disk_unit_attention_reported;
     dc->fw_name = "disk";
+    dc->category = DEVICE_CATEGORY_STORAGE;
     dc->desc = "virtual SCSI CD-ROM";
     dc->reset = scsi_disk_reset;
     dc->props = scsi_cd_properties;
@@ -2486,6 +2488,7 @@ static void scsi_block_class_initfn(ObjectClass *klass, void *data)
     sc->destroy      = scsi_destroy;
     sc->alloc_req    = scsi_block_new_request;
     dc->fw_name = "disk";
+    dc->category = DEVICE_CATEGORY_STORAGE;
     dc->desc = "SCSI block device passthrough";
     dc->reset = scsi_disk_reset;
     dc->props = scsi_block_properties;
@@ -2520,6 +2523,7 @@ static void scsi_disk_class_initfn(ObjectClass *klass, void *data)
     sc->alloc_req    = scsi_new_request;
     sc->unit_attention_reported = scsi_disk_unit_attention_reported;
     dc->fw_name = "disk";
+    dc->category = DEVICE_CATEGORY_STORAGE;
     dc->desc = "virtual SCSI disk or CD-ROM (legacy)";
     dc->reset = scsi_disk_reset;
     dc->props = scsi_disk_properties;
diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c
index 31f3cde..9d006ba 100644
--- a/hw/usb/dev-hid.c
+++ b/hw/usb/dev-hid.c
@@ -677,6 +677,7 @@ static void usb_mouse_class_initfn(ObjectClass *klass, void *data)
     uc->product_desc   = "QEMU USB Mouse";
     uc->usb_desc       = &desc_mouse;
     dc->vmsd = &vmstate_usb_ptr;
+    dc->category = DEVICE_CATEGORY_INPUT;
 }
 
 static const TypeInfo usb_mouse_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Paolo Bonzini]

Il 18/07/2013 10:27, Marcel Apfelbaum ha scritto:
> Running qemu with "-device ?" option returns ~145 lines.
> It is hard to manage understanding the output.
> 
> Theses patches aim to partially solve the problem by dividing the devices
> into logical categories like "Network/Display/..." and sorting them by it.

Nice! :)  An unexpected benefit of moving all the hw/ files to separate
per-type directory.

I checked libvirt and this should not break it.

Paolo

> Marcel Apfelbaum (2):
>   qemu-help: Sort devices by logical functionality
>   devices: Associate devices to their logical category
> 
>  hw/audio/ac97.c         |  1 +
>  hw/display/cirrus_vga.c |  1 +
>  hw/net/eepro100.c       |  1 +
>  hw/scsi/scsi-disk.c     |  4 ++++
>  hw/usb/dev-hid.c        |  1 +
>  include/hw/qdev-core.h  |  7 +++++++
>  qdev-monitor.c          | 23 ++++++++++++++++++++++-
>  7 files changed, 37 insertions(+), 1 deletion(-)
> 

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Marcel Apfelbaum]

On Thu, 2013-07-18 at 15:56 +0200, Paolo Bonzini wrote:
> Il 18/07/2013 10:27, Marcel Apfelbaum ha scritto:
> > Running qemu with "-device ?" option returns ~145 lines.
> > It is hard to manage understanding the output.
> > 
> > Theses patches aim to partially solve the problem by dividing the devices
> > into logical categories like "Network/Display/..." and sorting them by it.
> 
> Nice! :)  An unexpected benefit of moving all the hw/ files to separate
> per-type directory.
Thanks Paolo,
> 
> I checked libvirt and this should not break it.
Me too :)
> 
> Paolo
> 
> > Marcel Apfelbaum (2):
> >   qemu-help: Sort devices by logical functionality
> >   devices: Associate devices to their logical category
> > 
> >  hw/audio/ac97.c         |  1 +
> >  hw/display/cirrus_vga.c |  1 +
> >  hw/net/eepro100.c       |  1 +
> >  hw/scsi/scsi-disk.c     |  4 ++++
> >  hw/usb/dev-hid.c        |  1 +
> >  include/hw/qdev-core.h  |  7 +++++++
> >  qdev-monitor.c          | 23 ++++++++++++++++++++++-
> >  7 files changed, 37 insertions(+), 1 deletion(-)
> > 
> 
> 

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

On 07/18/2013 07:56 AM, Paolo Bonzini wrote:
> Il 18/07/2013 10:27, Marcel Apfelbaum ha scritto:
>> Running qemu with "-device ?" option returns ~145 lines.
>> It is hard to manage understanding the output.
>>
>> Theses patches aim to partially solve the problem by dividing the devices
>> into logical categories like "Network/Display/..." and sorting them by it.
> 
> Nice! :)  An unexpected benefit of moving all the hw/ files to separate
> per-type directory.
> 
> I checked libvirt and this should not break it.

Agreed - libvirt uses QMP queries so as to be immune to changes in
'-device help' output, since qemu 1.3 or so.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Andreas Färber]

Hi,

Am 18.07.2013 10:27, schrieb Marcel Apfelbaum:
> Running qemu with "-device ?" option returns ~145 lines.
> It is hard to manage understanding the output.
> 
> Theses patches aim to partially solve the problem by dividing the devices
> into logical categories like "Network/Display/..." and sorting them by it.
> 
> Marcel Apfelbaum (2):
>   qemu-help: Sort devices by logical functionality
>   devices: Associate devices to their logical category

Sorry to jump on this discussion. I think the idea to structure devices
better does make sense. However I feel that the implementation is not ideal:

For one, this adds a new field and uses it only for command line output,
while not benefiting libvirt or other QMP users.

For another, this adds a third tree overlay over devices:
We have the QOM inheritance tree with classifications such as PCIDevice,
ISADevice, VirtioDevice, USBDevice, etc.
We have Paolo's hw/ directory restructuring, which uses scsi, intc,
timer, etc. sub-directories.
The proposed category system is different from the sub-directories
(e.g., scsi/ vs STORAGE) and is completely manual, i.e. double work.

So I wonder if it would be possible to define the category per
Makefile.objs (whether in hw/Makefile.objs or in hw/*/Makefile.objs).

Or whether we can just reuse the type hierarchy and sort per base class
for now - the result would be different from the proposed categories but
hopefully reproducible elsewhere (assuming the base type is exposed in
QMP). Anthony had in the past suggested to change our inheritance tree
so that PCIDevice becomes an interface and the base class would be
specific to the chipset, i.e. might end up similar to the proposed
categories. (Getting rid of the parent field assumptions is a large
prerequisite for changing inheritance.)

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Michael S. Tsirkin]

On Sun, Jul 21, 2013 at 02:09:20PM +0200, Andreas Färber wrote:
> Hi,
> 
> Am 18.07.2013 10:27, schrieb Marcel Apfelbaum:
> > Running qemu with "-device ?" option returns ~145 lines.
> > It is hard to manage understanding the output.
> > 
> > Theses patches aim to partially solve the problem by dividing the devices
> > into logical categories like "Network/Display/..." and sorting them by it.
> > 
> > Marcel Apfelbaum (2):
> >   qemu-help: Sort devices by logical functionality
> >   devices: Associate devices to their logical category
> 
> Sorry to jump on this discussion. I think the idea to structure devices
> better does make sense. However I feel that the implementation is not ideal:
> 
> For one, this adds a new field and uses it only for command line output,
> while not benefiting libvirt or other QMP users.

This is not a problem with implementation - that can be added on top.
I think HMP is a good start, that's where we have a real
pain point now, with basically no documentation, and it's
a less risky change with no compatibility commitments.

> For another, this adds a third tree overlay over devices:

Exactly. And that's by design since this information
is currently missing.

> We have the QOM inheritance tree with classifications such as PCIDevice,
> ISADevice, VirtioDevice, USBDevice, etc.
>
> We have Paolo's hw/ directory restructuring, which uses scsi, intc,
> timer, etc. sub-directories.
>
> The proposed category system is different from the sub-directories
> (e.g., scsi/ vs STORAGE) and is completely manual, i.e. double work.
>
> So I wonder if it would be possible to define the category per
> Makefile.objs (whether in hw/Makefile.objs or in hw/*/Makefile.objs).

This idea can't support multi-category devices.
To me, duplication seems minimal.

On the other hand plain C is much more readable than makefile based tricks.

> Or whether we can just reuse the type hierarchy and sort per base class
> for now - the result would be different from the proposed categories but
> hopefully reproducible elsewhere (assuming the base type is exposed in
> QMP).

What you suggest is mostly already available in the form of
"bus" field in -device help output.

But this misses the whole point which is to help users find
devices implementing a specific functionality.

> Anthony had in the past suggested to change our inheritance tree
> so that PCIDevice becomes an interface and the base class would be
> specific to the chipset, i.e. might end up similar to the proposed
> categories. (Getting rid of the parent field assumptions is a large
> prerequisite for changing inheritance.)
> 
> Regards,
> Andreas

As long as there are no plans to support multiple inheritance in QOM,
this does not seem feasible.

If category information does become available in QOM, it will be easy
to rip out an extra field, so I don't think we need to keep out
help text in the current messed-up state indefinitely until it's
available.

> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

[Qemu-devel] [RFC PATCH 0/2] qemu-help: improve -device command line help

[Marcel Apfelbaum]

Running qemu with "-device ?" option returns ~145 lines.
It is hard to manage understanding the output.

Theses patches aim to partially solve the problem by dividing the devices
into logical categories like "Network/Display/..." and sorting them by it.

Marcel Apfelbaum (2):
  qemu-help: Sort devices by logical functionality
  devices: Associate devices to their logical category

 hw/audio/ac97.c         |  1 +
 hw/display/cirrus_vga.c |  1 +
 hw/net/eepro100.c       |  1 +
 hw/scsi/scsi-disk.c     |  4 ++++
 hw/usb/dev-hid.c        |  1 +
 include/hw/qdev-core.h  |  7 +++++++
 qdev-monitor.c          | 23 ++++++++++++++++++++++-
 7 files changed, 37 insertions(+), 1 deletion(-)

-- 
1.8.3.1

[Qemu-devel] [RFC PATCH 1/2] qemu-help: Sort devices by logical functionality

[Marcel Apfelbaum]

Categorize devices that appear as output to "-device ?" command
by logical functionality. Sort the devices by logical categories
before showing them to user.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
---
 include/hw/qdev-core.h |  7 +++++++
 qdev-monitor.c         | 23 ++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7fbffcb..4f7a9b8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -17,6 +17,12 @@ enum {
 #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
 #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
 
+#define DEVICE_CATEGORY_STORAGE "storage"
+#define DEVICE_CATEGORY_NETWORK "network"
+#define DEVICE_CATEGORY_INPUT "input"
+#define DEVICE_CATEGORY_DISPLAY "display"
+#define DEVICE_CATEGORY_SOUND "sound"
+
 typedef int (*qdev_initfn)(DeviceState *dev);
 typedef int (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +87,7 @@ typedef struct DeviceClass {
     /*< public >*/
 
     const char *fw_name;
+    const char *category;
     const char *desc;
     Property *props;
     int no_user;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..1446b6e 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -93,6 +93,9 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
     if (qdev_class_has_alias(dc)) {
         error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
     }
+    if (dc->category) {
+        error_printf(", category \"%s\"", dc->category);
+    }
     if (dc->desc) {
         error_printf(", desc \"%s\"", dc->desc);
     }
@@ -139,16 +142,34 @@ static const char *find_typename_by_alias(const char *alias)
     return NULL;
 }
 
+static gint qdev_device_compare(gconstpointer item1, gconstpointer item2)
+{
+    DeviceClass *dc1, *dc2;
+
+    dc1 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item1,
+                                                   TYPE_DEVICE);
+    dc2 = (DeviceClass *)object_class_dynamic_cast((ObjectClass *)item2,
+                                                   TYPE_DEVICE);
+
+    return g_strcmp0(dc1->category, dc2->category);
+}
+
 int qdev_device_help(QemuOpts *opts)
 {
     const char *driver;
     Property *prop;
     ObjectClass *klass;
+    GSList *list;
 
     driver = qemu_opt_get(opts, "driver");
     if (driver && is_help_option(driver)) {
         bool show_no_user = false;
-        object_class_foreach(qdev_print_devinfo, TYPE_DEVICE, false, &show_no_user);
+
+        list = object_class_get_list(TYPE_DEVICE, false);
+        list = g_slist_sort(list, qdev_device_compare);
+        g_slist_foreach(list, (GFunc)qdev_print_devinfo, &show_no_user);
+        g_slist_free(list);
+
         return 1;
     }
 
-- 
1.8.3.1