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 <marce...@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
