On Mon, 20 Jun 2016 12:53:00 -0300
Eduardo Habkost <ehabk...@redhat.com> wrote:
> qdev_prop_check_globals() tries to warn the user if a given
> -global option was not used. But it does that only if the device
> is not hotpluggable.
>
> The warning also makes it harder for management code or people
> that write their own scripts or config files: there's no way to
> know if a given -global option will trigger a warning or not,
> because we don't have any introspection mechanism to check if a
> machine-type instantiates a given device or not.
>
> The warning is also the only reason we have the 'user_provided'
> and 'used' fields in struct GlobalProperty. Removing the check
> will let us simplify the code.
>
> In other words, the warning is nearly useless and gets in our way
> and our users' way. Remove it.
>
> With this change, we don't need subprocess tricks in
> test-qdev-global-props anymore.
>
> Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> Changes v1 -> v2:
> * Fix test-qdev-global-props unit test
> ---
> hw/core/qdev-properties.c | 27 --------------------
> include/hw/qdev-properties.h | 1 -
> tests/test-qdev-global-props.c | 57
> +++---------------------------------------
> vl.c | 1 - 4 files changed, 4
> insertions(+), 82 deletions(-)
>
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 0fe7214..c14791d 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1036,33 +1036,6 @@ void
> qdev_prop_register_global_list(GlobalProperty *props) }
> }
>
> -int qdev_prop_check_globals(void)
> -{
> - GList *l;
> - int ret = 0;
> -
> - for (l = global_props; l; l = l->next) {
> - GlobalProperty *prop = l->data;
> - ObjectClass *oc;
> - DeviceClass *dc;
> - if (prop->used) {
> - continue;
> - }
> - if (!prop->user_provided) {
> - continue;
> - }
> - oc = object_class_by_name(prop->driver);
> - dc = DEVICE_CLASS(oc);
> - if (!dc->hotpluggable && !prop->used) {
> - error_report("Warning: global %s.%s=%s not used",
> - prop->driver, prop->property, prop->value);
> - ret = 1;
> - continue;
> - }
> - }
> - return ret;
> -}
> -
> static void qdev_prop_set_globals_for_type(DeviceState *dev,
> const char *typename)
> {
> diff --git a/include/hw/qdev-properties.h
> b/include/hw/qdev-properties.h index 034b75a..3bd5ea9 100644
> --- a/include/hw/qdev-properties.h
> +++ b/include/hw/qdev-properties.h
> @@ -191,7 +191,6 @@ void qdev_prop_set_ptr(DeviceState *dev, const
> char *name, void *value);
> void qdev_prop_register_global(GlobalProperty *prop);
> void qdev_prop_register_global_list(GlobalProperty *props);
> -int qdev_prop_check_globals(void);
> void qdev_prop_set_globals(DeviceState *dev);
> void error_set_from_qdev_prop_error(Error **errp, int ret,
> DeviceState *dev, Property *prop, const char *value);
> diff --git a/tests/test-qdev-global-props.c
> b/tests/test-qdev-global-props.c index db77ad9..c0fea84 100644
> --- a/tests/test-qdev-global-props.c
> +++ b/tests/test-qdev-global-props.c
> @@ -64,7 +64,7 @@ static const TypeInfo static_prop_type = {
> };
>
> /* Test simple static property setting to default value */
> -static void test_static_prop_subprocess(void)
> +static void test_static_prop(void)
> {
> MyType *mt;
>
> @@ -74,16 +74,8 @@ static void test_static_prop_subprocess(void)
> g_assert_cmpuint(mt->prop1, ==, PROP_DEFAULT);
> }
>
> -static void test_static_prop(void)
> -{
> -
> g_test_trap_subprocess("/qdev/properties/static/default/subprocess",
> 0, 0);
> - g_test_trap_assert_passed();
> - g_test_trap_assert_stderr("");
> - g_test_trap_assert_stdout("");
> -}
> -
> /* Test setting of static property using global properties */
> -static void test_static_globalprop_subprocess(void)
> +static void test_static_globalprop(void)
> {
> MyType *mt;
> static GlobalProperty props[] = {
> @@ -100,14 +92,6 @@ static void
> test_static_globalprop_subprocess(void) g_assert_cmpuint(mt->prop2,
> ==, PROP_DEFAULT); }
>
> -static void test_static_globalprop(void)
> -{
> -
> g_test_trap_subprocess("/qdev/properties/static/global/subprocess",
> 0, 0);
> - g_test_trap_assert_passed();
> - g_test_trap_assert_stderr("");
> - g_test_trap_assert_stdout("");
> -}
> -
> #define TYPE_DYNAMIC_PROPS "dynamic-prop-type"
> #define DYNAMIC_TYPE(obj) \
> OBJECT_CHECK(MyType, (obj), TYPE_DYNAMIC_PROPS)
> @@ -195,7 +179,7 @@ static const TypeInfo nondevice_type = {
> };
>
> /* Test setting of dynamic properties using global properties */
> -static void test_dynamic_globalprop_subprocess(void)
> +static void test_dynamic_globalprop(void)
> {
> MyType *mt;
> static GlobalProperty props[] = {
> @@ -205,7 +189,6 @@ static void
> test_dynamic_globalprop_subprocess(void) { TYPE_UNUSED_NOHOTPLUG,
> "prop5", "105", true }, {}
> };
> - int all_used;
>
> qdev_prop_register_global_list(props);
>
> @@ -214,27 +197,14 @@ static void
> test_dynamic_globalprop_subprocess(void)
> g_assert_cmpuint(mt->prop1, ==, 101);
> g_assert_cmpuint(mt->prop2, ==, 102);
> - all_used = qdev_prop_check_globals();
> - g_assert_cmpuint(all_used, ==, 1);
> g_assert(props[0].used);
> g_assert(props[1].used);
> g_assert(!props[2].used);
> g_assert(!props[3].used);
> }
>
> -static void test_dynamic_globalprop(void)
> -{
> -
> g_test_trap_subprocess("/qdev/properties/dynamic/global/subprocess",
> 0, 0);
> - g_test_trap_assert_passed();
> - g_test_trap_assert_stderr_unmatched("*prop1*");
> - g_test_trap_assert_stderr_unmatched("*prop2*");
> - g_test_trap_assert_stderr_unmatched("*prop4*");
> - g_test_trap_assert_stderr("*Warning: global
> nohotplug-type.prop5=105 not used\n*");
> - g_test_trap_assert_stdout("");
> -}
> -
> /* Test setting of dynamic properties using user_provided=false
> properties */ -static void
> test_dynamic_globalprop_nouser_subprocess(void) +static void
> test_dynamic_globalprop_nouser(void) {
> MyType *mt;
> static GlobalProperty props[] = {
> @@ -244,7 +214,6 @@ static void
> test_dynamic_globalprop_nouser_subprocess(void)
> { TYPE_UNUSED_NOHOTPLUG, "prop5", "105" }, {}
> };
> - int all_used;
>
> qdev_prop_register_global_list(props);
>
> @@ -253,22 +222,12 @@ static void
> test_dynamic_globalprop_nouser_subprocess(void)
> g_assert_cmpuint(mt->prop1, ==, 101);
> g_assert_cmpuint(mt->prop2, ==, 102);
> - all_used = qdev_prop_check_globals();
> - g_assert_cmpuint(all_used, ==, 0);
> g_assert(props[0].used);
> g_assert(props[1].used);
> g_assert(!props[2].used);
> g_assert(!props[3].used);
> }
>
> -static void test_dynamic_globalprop_nouser(void)
> -{
> -
> g_test_trap_subprocess("/qdev/properties/dynamic/global/nouser/subprocess",
> 0, 0);
> - g_test_trap_assert_passed();
> - g_test_trap_assert_stderr("");
> - g_test_trap_assert_stdout("");
> -}
> -
> int main(int argc, char **argv)
> {
> g_test_init(&argc, &argv, NULL);
> @@ -280,23 +239,15 @@ int main(int argc, char **argv)
> type_register_static(&nohotplug_type);
> type_register_static(&nondevice_type);
>
> - g_test_add_func("/qdev/properties/static/default/subprocess",
> - test_static_prop_subprocess);
> g_test_add_func("/qdev/properties/static/default",
> test_static_prop);
>
> - g_test_add_func("/qdev/properties/static/global/subprocess",
> - test_static_globalprop_subprocess);
> g_test_add_func("/qdev/properties/static/global",
> test_static_globalprop);
>
> - g_test_add_func("/qdev/properties/dynamic/global/subprocess",
> - test_dynamic_globalprop_subprocess);
> g_test_add_func("/qdev/properties/dynamic/global",
> test_dynamic_globalprop);
>
> -
> g_test_add_func("/qdev/properties/dynamic/global/nouser/subprocess",
> - test_dynamic_globalprop_nouser_subprocess);
> g_test_add_func("/qdev/properties/dynamic/global/nouser",
> test_dynamic_globalprop_nouser);
>
> diff --git a/vl.c b/vl.c
> index 8c40d56..9472a26 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4631,7 +4631,6 @@ int main(int argc, char **argv, char **envp)
> }
> }
>
> - qdev_prop_check_globals();
> if (vmstate_dump_file) {
> /* dump and exit */
> dump_vmstate_json_to_file(vmstate_dump_file);
