Patchwork [v4,2/3] qemu-help: Sort devices by logical functionality

login
register
mail settings
Submitter Marcel Apfelbaum
Date July 29, 2013, 2:17 p.m.
Message ID <1375107465-25767-3-git-send-email-marcel.a@redhat.com>
Download mbox | patch
Permalink /patch/262787/
State New
Headers show

Comments

Marcel Apfelbaum - July 29, 2013, 2:17 p.m.
Categorize devices that appear as output to "-device ?" command
by logical functionality. Sort the devices by logical categories
before showing them to user.

The sort is done by functionality rather than alphabetical.

Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
---
Changes from v3:
    - fixed a memory leak introduced by v3
Addressed Michael S. Tsirkin's review
    - removed dead code
    - qdev_print_class_devinfo can print info
      on abstract classes
Addressed Paolo Bonzini's review
    - Renamed "Assembly" category to "Controller/Bridge/Hub"

Changes from v2:
    Addressed Michael S. Tsirkin's review:
    - Explicit connection between the categories
      and their names
    - Refactoring of unsafe code
    Addressed Paolo Bonzini's review
    - Replaced Management category by USB 

Changes from v1:
    Addressed Michael S. Tsirkin's review:
    - Used bitmap operations on categories
    - Moved category names into the header file

 include/hw/qdev-core.h | 29 +++++++++++++++++++++++++++++
 qdev-monitor.c         | 48 +++++++++++++++++++++++++++++++++++++++---------
 2 files changed, 68 insertions(+), 9 deletions(-)
Michael S. Tsirkin - July 29, 2013, 6:11 p.m.
On Mon, Jul 29, 2013 at 05:17:44PM +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.
> 
> The sort is done by functionality rather than alphabetical.
> 
> Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
> ---
> Changes from v3:
>     - fixed a memory leak introduced by v3
> Addressed Michael S. Tsirkin's review
>     - removed dead code
>     - qdev_print_class_devinfo can print info
>       on abstract classes
> Addressed Paolo Bonzini's review
>     - Renamed "Assembly" category to "Controller/Bridge/Hub"
> 
> Changes from v2:
>     Addressed Michael S. Tsirkin's review:
>     - Explicit connection between the categories
>       and their names
>     - Refactoring of unsafe code
>     Addressed Paolo Bonzini's review
>     - Replaced Management category by USB 
> 
> Changes from v1:
>     Addressed Michael S. Tsirkin's review:
>     - Used bitmap operations on categories
>     - Moved category names into the header file
> 
>  include/hw/qdev-core.h | 29 +++++++++++++++++++++++++++++
>  qdev-monitor.c         | 48 +++++++++++++++++++++++++++++++++++++++---------
>  2 files changed, 68 insertions(+), 9 deletions(-)
> 
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e8b89b1..46972f4 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -18,6 +18,34 @@ enum {
>  #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
>  #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
>  
> +typedef enum DeviceCategory {
> +    DEVICE_CATEGORY_BRIDGE,
> +    DEVICE_CATEGORY_USB,
> +    DEVICE_CATEGORY_STORAGE,
> +    DEVICE_CATEGORY_NETWORK,
> +    DEVICE_CATEGORY_INPUT,
> +    DEVICE_CATEGORY_DISPLAY,
> +    DEVICE_CATEGORY_SOUND,
> +    DEVICE_CATEGORY_MISC,
> +    DEVICE_CATEGORY_MAX
> +} DeviceCategory;
> +
> +static inline const char *qdev_category_get_name(DeviceCategory category)
> +{
> +    static const char *category_names[DEVICE_CATEGORY_MAX] = {
> +        [DEVICE_CATEGORY_BRIDGE]  = "Controller/Bridge/Hub",
> +        [DEVICE_CATEGORY_USB]     = "USB",
> +        [DEVICE_CATEGORY_STORAGE] = "Storage",
> +        [DEVICE_CATEGORY_NETWORK] = "Network",
> +        [DEVICE_CATEGORY_INPUT]   = "Input",
> +        [DEVICE_CATEGORY_DISPLAY] = "Display",
> +        [DEVICE_CATEGORY_SOUND]   = "Sound",
> +        [DEVICE_CATEGORY_MISC]    = "Misc",
> +    };
> +
> +    return category_names[category];
> +};
> +
>  typedef int (*qdev_initfn)(DeviceState *dev);
>  typedef int (*qdev_event)(DeviceState *dev);
>  typedef void (*qdev_resetfn)(DeviceState *dev);
> @@ -81,6 +109,7 @@ typedef struct DeviceClass {
>      ObjectClass parent_class;
>      /*< public >*/
>  
> +    DECLARE_BITMAP(categories, DEVICE_CATEGORY_MAX);
>      const char *fw_name;
>      const char *desc;
>      Property *props;
> diff --git a/qdev-monitor.c b/qdev-monitor.c
> index e54dbc2..230a8df 100644
> --- a/qdev-monitor.c
> +++ b/qdev-monitor.c
> @@ -75,24 +75,27 @@ static bool qdev_class_has_alias(DeviceClass *dc)
>      return (qdev_class_get_alias(dc) != NULL);
>  }
>  
> -static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
> +static void qdev_print_class_devinfo(DeviceClass *dc)
>  {
> -    DeviceClass *dc;
> -    bool *show_no_user = opaque;
> -
> -    dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
> +    DeviceCategory category;
>  
> -    if (!dc || (show_no_user && !*show_no_user && dc->no_user)) {
> +    if (!dc) {
>          return;
>      }
>  
> -    error_printf("name \"%s\"", object_class_get_name(klass));
> +    error_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
>      if (dc->bus_type) {
>          error_printf(", bus %s", dc->bus_type);
>      }
>      if (qdev_class_has_alias(dc)) {
>          error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
>      }
> +    error_printf(", categories");
> +    for (category = 0; category < DEVICE_CATEGORY_MAX; ++category) {
> +        if (test_bit(category, dc->categories)) {
> +            error_printf(" \"%s\"", qdev_category_get_name(category));
> +        }
> +    }
>      if (dc->desc) {
>          error_printf(", desc \"%s\"", dc->desc);
>      }
> @@ -102,6 +105,15 @@ static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
>      error_printf("\n");
>  }
>  
> +static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
> +{
> +    DeviceClass *dc;
> +
> +    dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
> +
> +    qdev_print_class_devinfo(dc);
> +}
> +
>  static int set_property(const char *name, const char *value, void *opaque)
>  {
>      DeviceState *dev = opaque;
> @@ -139,6 +151,21 @@ static const char *find_typename_by_alias(const char *alias)
>      return NULL;
>  }
>  
> +static void qdev_print_category_devices(DeviceCategory category)
> +{
> +    DeviceClass *dc;
> +    GSList *list, *curr;
> +
> +    list = object_class_get_list(TYPE_DEVICE, false);

It's probably a bit cleaner to pass in the list, so we don't
end up building up/destroying it repeatedly for
each category.

> +    for (curr = list; curr; curr = g_slist_next(curr)) {
> +        dc = (DeviceClass *)object_class_dynamic_cast(curr->data, TYPE_DEVICE);
> +        if (!dc->no_user && test_bit(category, dc->categories)) {
> +            qdev_print_class_devinfo(dc);
> +        }
> +    }
> +    g_slist_free(list);
> +}
> +
>  int qdev_device_help(QemuOpts *opts)
>  {
>      const char *driver;
> @@ -147,8 +174,11 @@ int qdev_device_help(QemuOpts *opts)
>  
>      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);
> +        DeviceCategory category;
> +        for (category = 0; category < DEVICE_CATEGORY_MAX; ++category) {
> +            qdev_print_category_devices(category);
> +        }
> +
>          return 1;
>      }
>  
> -- 
> 1.8.3.1

Patch

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e8b89b1..46972f4 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -18,6 +18,34 @@  enum {
 #define DEVICE_CLASS(klass) OBJECT_CLASS_CHECK(DeviceClass, (klass), TYPE_DEVICE)
 #define DEVICE_GET_CLASS(obj) OBJECT_GET_CLASS(DeviceClass, (obj), TYPE_DEVICE)
 
+typedef enum DeviceCategory {
+    DEVICE_CATEGORY_BRIDGE,
+    DEVICE_CATEGORY_USB,
+    DEVICE_CATEGORY_STORAGE,
+    DEVICE_CATEGORY_NETWORK,
+    DEVICE_CATEGORY_INPUT,
+    DEVICE_CATEGORY_DISPLAY,
+    DEVICE_CATEGORY_SOUND,
+    DEVICE_CATEGORY_MISC,
+    DEVICE_CATEGORY_MAX
+} DeviceCategory;
+
+static inline const char *qdev_category_get_name(DeviceCategory category)
+{
+    static const char *category_names[DEVICE_CATEGORY_MAX] = {
+        [DEVICE_CATEGORY_BRIDGE]  = "Controller/Bridge/Hub",
+        [DEVICE_CATEGORY_USB]     = "USB",
+        [DEVICE_CATEGORY_STORAGE] = "Storage",
+        [DEVICE_CATEGORY_NETWORK] = "Network",
+        [DEVICE_CATEGORY_INPUT]   = "Input",
+        [DEVICE_CATEGORY_DISPLAY] = "Display",
+        [DEVICE_CATEGORY_SOUND]   = "Sound",
+        [DEVICE_CATEGORY_MISC]    = "Misc",
+    };
+
+    return category_names[category];
+};
+
 typedef int (*qdev_initfn)(DeviceState *dev);
 typedef int (*qdev_event)(DeviceState *dev);
 typedef void (*qdev_resetfn)(DeviceState *dev);
@@ -81,6 +109,7 @@  typedef struct DeviceClass {
     ObjectClass parent_class;
     /*< public >*/
 
+    DECLARE_BITMAP(categories, DEVICE_CATEGORY_MAX);
     const char *fw_name;
     const char *desc;
     Property *props;
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e54dbc2..230a8df 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -75,24 +75,27 @@  static bool qdev_class_has_alias(DeviceClass *dc)
     return (qdev_class_get_alias(dc) != NULL);
 }
 
-static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
+static void qdev_print_class_devinfo(DeviceClass *dc)
 {
-    DeviceClass *dc;
-    bool *show_no_user = opaque;
-
-    dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
+    DeviceCategory category;
 
-    if (!dc || (show_no_user && !*show_no_user && dc->no_user)) {
+    if (!dc) {
         return;
     }
 
-    error_printf("name \"%s\"", object_class_get_name(klass));
+    error_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc)));
     if (dc->bus_type) {
         error_printf(", bus %s", dc->bus_type);
     }
     if (qdev_class_has_alias(dc)) {
         error_printf(", alias \"%s\"", qdev_class_get_alias(dc));
     }
+    error_printf(", categories");
+    for (category = 0; category < DEVICE_CATEGORY_MAX; ++category) {
+        if (test_bit(category, dc->categories)) {
+            error_printf(" \"%s\"", qdev_category_get_name(category));
+        }
+    }
     if (dc->desc) {
         error_printf(", desc \"%s\"", dc->desc);
     }
@@ -102,6 +105,15 @@  static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
     error_printf("\n");
 }
 
+static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
+{
+    DeviceClass *dc;
+
+    dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);
+
+    qdev_print_class_devinfo(dc);
+}
+
 static int set_property(const char *name, const char *value, void *opaque)
 {
     DeviceState *dev = opaque;
@@ -139,6 +151,21 @@  static const char *find_typename_by_alias(const char *alias)
     return NULL;
 }
 
+static void qdev_print_category_devices(DeviceCategory category)
+{
+    DeviceClass *dc;
+    GSList *list, *curr;
+
+    list = object_class_get_list(TYPE_DEVICE, false);
+    for (curr = list; curr; curr = g_slist_next(curr)) {
+        dc = (DeviceClass *)object_class_dynamic_cast(curr->data, TYPE_DEVICE);
+        if (!dc->no_user && test_bit(category, dc->categories)) {
+            qdev_print_class_devinfo(dc);
+        }
+    }
+    g_slist_free(list);
+}
+
 int qdev_device_help(QemuOpts *opts)
 {
     const char *driver;
@@ -147,8 +174,11 @@  int qdev_device_help(QemuOpts *opts)
 
     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);
+        DeviceCategory category;
+        for (category = 0; category < DEVICE_CATEGORY_MAX; ++category) {
+            qdev_print_category_devices(category);
+        }
+
         return 1;
     }