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 <address@hidden> 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 <address@hidden>
> > Reviewed-by: Kevin Wolf <address@hidden>
> > ---
> >   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