Re: [Qemu-devel] [PATCH 03/10] vl: Reject invalid class names on -global

[Markus Armbruster]

Eduardo Habkost <address@hidden> writes:

> Instead of just printing a warning very late, reject obviously
> invalid -global arguments by validating the class name.
>
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  hw/core/qdev-properties.c |  7 -------
>  vl.c                      | 21 ++++++++++++++++++---
>  2 files changed, 18 insertions(+), 10 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index c10edee..64e17aa 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1052,13 +1052,6 @@ int qdev_prop_check_globals(void)
>              continue;
>          }
>          oc = object_class_by_name(prop->driver);
> -        oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> -        if (!oc) {
> -            error_report("Warning: global %s.%s has invalid class name",
> -                       prop->driver, prop->property);
> -            ret = 1;
> -            continue;
> -        }
>          dc = DEVICE_CLASS(oc);
>          if (!dc->hotpluggable && !prop->used) {
>              error_report("Warning: global %s.%s=%s not used",

qdev_prop_check_globals() runs between machine creation and the main
loop.

> diff --git a/vl.c b/vl.c
> index d2d756a..d88ddba 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2935,10 +2935,21 @@ static void set_memory_options(uint64_t *ram_slots, 
> ram_addr_t *maxram_size,
>  static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
>  {
>      GlobalProperty *g;
> +    ObjectClass *oc;
> +    const char *driver = qemu_opt_get(opts, "driver");
> +    const char *prop = qemu_opt_get(opts, "property");
> +
> +    oc = object_class_by_name(driver);
> +    oc = object_class_dynamic_cast(oc, TYPE_DEVICE);
> +    if (!oc) {

I'd write

       oc = object_class_by_name(driver);
       if (!object_class_dynamic_cast(oc, TYPE_DEVICE)) {

> +        error_setg(errp, "global %s.%s has invalid class name",
> +                   driver, prop);
> +        return -1;
> +    }
>  
>      g = g_malloc0(sizeof(*g));
> -    g->driver   = qemu_opt_get(opts, "driver");
> -    g->property = qemu_opt_get(opts, "property");
> +    g->driver   = driver;
> +    g->property = prop;
>      g->value    = qemu_opt_get(opts, "value");
>      g->user_provided = true;
>      qdev_prop_register_global(g);
> @@ -4480,7 +4491,11 @@ int main(int argc, char **argv, char **envp)
>          }
>      }
>      qemu_opts_foreach(qemu_find_opts("global"),
> -                      global_init_func, NULL, NULL);
> +                      global_init_func, NULL, &err);
> +    if (err) {
> +        error_report_err(err);
> +        exit(1);
> +    }
>  
>      /* This checkpoint is required by replay to separate prior clock
>         reading from the other reads, because timer polling functions query

This runs right before machine creation.

Any types registered between here and qdev_prop_check_globals() are no
longer visible for class name check.  But nothing should register types
then.

Reviewed-by: Markus Armbruster <address@hidden>