[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

[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

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

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

[Eric Blake]

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

On 07/29/2013 02:24 PM, Anthony Liguori wrote:
> Applied.  Thanks.

Script botch-up?  This series has a title of RFC (which means it should
not be applied as-is), and there have been later revisions that dropped
the RFC in response to comments; did the correct version get applied?

-- 
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 --]

[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
>