Re: [Qemu-devel] [PATCH 07/23] qdev: kill off DeviceInfo list

["Andreas Färber" <afaerber@suse.de>]

Am 30.01.2012 22:08, schrieb Anthony Liguori:
> Teach the various bits of code that need to walk through available devices to
> do so via QOM.
> 
> Signed-off-by: Anthony Liguori

(Email missing.)

NACK. This introduces broken code, see below.

>  bool qdev_exists(const char *name)
>  {
> -    return !!qdev_find_info(NULL, name);
> +    return !!object_class_by_name(name);
>  }

Minor nit: I'd find object_class_by_name(name) != NULL easier to read
than double negation.

> @@ -245,17 +213,28 @@ DeviceState *qdev_try_create(BusState *bus, const char *name)
>      return qdev_create_from_info(bus, name);
>  }
>  
> -static void qdev_print_devinfo(DeviceInfo *info)
> +static void qdev_print_devinfo(ObjectClass *klass, void *opaque)
>  {
> -    error_printf("name \"%s\", bus %s",
> -                 info->name, info->bus_info->name);
> -    if (info->alias) {
> -        error_printf(", alias \"%s\"", info->alias);
> +    DeviceClass *dc;
> +    bool *show_no_user = opaque;

Due to this...

> +
> +    dc = (DeviceClass *)object_class_dynamic_cast(klass, TYPE_DEVICE);

...and this, ...

> +
> +    if (!dc || (show_no_user && !*show_no_user && dc->no_user)) {
> +        return;
>      }
> -    if (info->desc) {
> -        error_printf(", desc \"%s\"", info->desc);
> +
> +    error_printf("name \"%s\"", object_class_get_name(klass));
> +    if (dc->bus_info) {
> +        error_printf(", bus %s", dc->bus_info->name);
>      }
> -    if (info->no_user) {
> +    if (dc->alias) {
> +        error_printf(", alias \"%s\"", dc->alias);
> +    }
> +    if (dc->desc) {
> +        error_printf(", desc \"%s\"", dc->desc);
> +    }
> +    if (dc->no_user) {
>          error_printf(", no-user");
>      }
>      error_printf("\n");
> @@ -279,17 +258,14 @@ static int set_property(const char *name, const char *value, void *opaque)
>  int qdev_device_help(QemuOpts *opts)
>  {
>      const char *driver;
> -    DeviceInfo *info;
>      Property *prop;
> +    ObjectClass *klass;
> +    DeviceClass *info;
>  
>      driver = qemu_opt_get(opts, "driver");
>      if (driver && !strcmp(driver, "?")) {
> -        for (info = device_info_list; info != NULL; info = info->next) {
> -            if (info->no_user) {
> -                continue;       /* not available, don't show */
> -            }
> -            qdev_print_devinfo(info);
> -        }
> +        bool show_no_user = false;
> +        object_class_foreach(qdev_print_devinfo, &show_no_user);

This...

>          return 1;
>      }
>  
> @@ -297,10 +273,11 @@ int qdev_device_help(QemuOpts *opts)
>          return 0;
>      }
>  
> -    info = qdev_find_info(NULL, driver);
> -    if (!info) {
> +    klass = object_class_by_name(driver);
> +    if (!klass) {
>          return 0;
>      }
> +    info = DEVICE_CLASS(klass);
>  
>      for (prop = info->props; prop && prop->name; prop++) {
>          /*
> @@ -312,14 +289,14 @@ int qdev_device_help(QemuOpts *opts)
>          if (!prop->info->parse) {
>              continue;           /* no way to set it, don't show */
>          }
> -        error_printf("%s.%s=%s\n", info->name, prop->name,
> +        error_printf("%s.%s=%s\n", driver, prop->name,
>                       prop->info->legacy_name ?: prop->info->name);
>      }
>      for (prop = info->bus_info->props; prop && prop->name; prop++) {
>          if (!prop->info->parse) {
>              continue;           /* no way to set it, don't show */
>          }
> -        error_printf("%s.%s=%s\n", info->name, prop->name,
> +        error_printf("%s.%s=%s\n", driver, prop->name,
>                       prop->info->legacy_name ?: prop->info->name);
>      }
>      return 1;
> @@ -1085,11 +1062,7 @@ void do_info_qtree(Monitor *mon)
>  
>  void do_info_qdm(Monitor *mon)
>  {
> -    DeviceInfo *info;
> -
> -    for (info = device_info_list; info != NULL; info = info->next) {
> -        qdev_print_devinfo(info);
> -    }
> +    object_class_foreach(qdev_print_devinfo, NULL);

...and this is broken, as pointed out before:

http://patchwork.ozlabs.org/patch/138396/

Easiest would be to reorder 11/23 before this one so that we don't
introduce a broken, bloated user of foreach in the first place.

Otherwise looking good.

Andreas

>  }
>  
>  int do_device_add(Monitor *mon, const QDict *qdict, QObject **ret_data)

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