[03/10] vl: Reject invalid class names on -global
diff mbox

Message ID 1466022773-8965-4-git-send-email-ehabkost@redhat.com
State New
Headers show

Commit Message

Eduardo Habkost June 15, 2016, 8:32 p.m. UTC
Instead of just printing a warning very late, reject obviously
invalid -global arguments by validating the class name.

Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 hw/core/qdev-properties.c |  7 -------
 vl.c                      | 21 ++++++++++++++++++---
 2 files changed, 18 insertions(+), 10 deletions(-)

Comments

Markus Armbruster June 20, 2016, 7:56 a.m. UTC | #1
Eduardo Habkost <ehabkost@redhat.com> writes:

> Instead of just printing a warning very late, reject obviously
> invalid -global arguments by validating the class name.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  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 <armbru@redhat.com>
Igor Mammedov June 20, 2016, 12:44 p.m. UTC | #2
On Wed, 15 Jun 2016 17:32:46 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Instead of just printing a warning very late, reject obviously
> invalid -global arguments by validating the class name.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
You are removing check that's used by tests, I'm getting
after applying this patch

   /qdev/properties/dynamic/global: **
ERROR:tests/test-qdev-global-props.c:232:test_dynamic_globalprop: child
process (/qdev/properties/dynamic/global/subprocess [174257]) failed
unexpectedly FAIL GTester: last random seed:
R02S52e72b8bde6002f6683785d9dbe46e76
(pid=174262) /qdev/properties/dynamic/global/nouser: OK FAIL:
tests/test-qdev-global-props

> ---
>  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",
> 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) {
> +        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

Patch
diff mbox

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",
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) {
+        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