qdev: Move global validation to a single function
Currently GlobalProperty.not_used=false has multiple meanings: * It may be a property for a hotpluggable device, which may or may not have been used by a device; * It may be a machine-type-provided property, which may or may not have been used by a device. * It may be a user-provided property that was actually not used by any device. Simplify the logic by having two separate fields: 'user_provided' and 'used'. This allows the entire global property validation logic to be contained in a single function, and allows more specific error messages. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This commit is contained in:
		
							parent
							
								
									d828c430eb
								
							
						
					
					
						commit
						b3ce84fea4
					
				| @ -388,28 +388,12 @@ void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd) | ||||
| static int qdev_add_one_global(QemuOpts *opts, void *opaque) | ||||
| { | ||||
|     GlobalProperty *g; | ||||
|     ObjectClass *oc; | ||||
| 
 | ||||
|     g = g_malloc0(sizeof(*g)); | ||||
|     g->driver   = qemu_opt_get(opts, "driver"); | ||||
|     g->property = qemu_opt_get(opts, "property"); | ||||
|     g->value    = qemu_opt_get(opts, "value"); | ||||
|     oc = object_class_dynamic_cast(object_class_by_name(g->driver), | ||||
|                                    TYPE_DEVICE); | ||||
|     if (oc) { | ||||
|         DeviceClass *dc = DEVICE_CLASS(oc); | ||||
| 
 | ||||
|         if (dc->hotpluggable) { | ||||
|             /* If hotpluggable then skip not_used checking. */ | ||||
|             g->not_used = false; | ||||
|         } else { | ||||
|             /* Maybe a typo. */ | ||||
|             g->not_used = true; | ||||
|         } | ||||
|     } else { | ||||
|         /* Maybe a typo. */ | ||||
|         g->not_used = true; | ||||
|     } | ||||
|     g->user_provided = true; | ||||
|     qdev_prop_register_global(g); | ||||
|     return 0; | ||||
| } | ||||
|  | ||||
| @ -961,13 +961,29 @@ int qdev_prop_check_globals(void) | ||||
|     int ret = 0; | ||||
| 
 | ||||
|     QTAILQ_FOREACH(prop, &global_props, next) { | ||||
|         if (!prop->not_used) { | ||||
|         ObjectClass *oc; | ||||
|         DeviceClass *dc; | ||||
|         if (prop->used) { | ||||
|             continue; | ||||
|         } | ||||
|         if (!prop->user_provided) { | ||||
|             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", | ||||
|                        prop->driver, prop->property, prop->value); | ||||
|             ret = 1; | ||||
|             continue; | ||||
|         } | ||||
|         ret = 1; | ||||
|         error_report("Warning: \"-global %s.%s=%s\" not used", | ||||
|                      prop->driver, prop->property, prop->value); | ||||
| 
 | ||||
|     } | ||||
|     return ret; | ||||
| } | ||||
| @ -983,7 +999,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename, | ||||
|         if (strcmp(typename, prop->driver) != 0) { | ||||
|             continue; | ||||
|         } | ||||
|         prop->not_used = false; | ||||
|         prop->used = true; | ||||
|         object_property_parse(OBJECT(dev), prop->value, prop->property, &err); | ||||
|         if (err != NULL) { | ||||
|             error_propagate(errp, err); | ||||
|  | ||||
| @ -242,16 +242,16 @@ struct PropertyInfo { | ||||
| 
 | ||||
| /**
 | ||||
|  * GlobalProperty: | ||||
|  * @not_used: Track use of a global property.  Defaults to false in all C99 | ||||
|  * struct initializations. | ||||
|  * | ||||
|  * This prevents reports of .compat_props when they are not used. | ||||
|  * @user_provided: Set to true if property comes from user-provided config | ||||
|  * (command-line or config file). | ||||
|  * @used: Set to true if property was used when initializing a device. | ||||
|  */ | ||||
| typedef struct GlobalProperty { | ||||
|     const char *driver; | ||||
|     const char *property; | ||||
|     const char *value; | ||||
|     bool not_used; | ||||
|     bool user_provided; | ||||
|     bool used; | ||||
|     QTAILQ_ENTRY(GlobalProperty) next; | ||||
| } GlobalProperty; | ||||
| 
 | ||||
|  | ||||
| @ -209,8 +209,7 @@ static void test_dynamic_globalprop_subprocess(void) | ||||
|         { TYPE_DYNAMIC_PROPS, "prop1", "101", true }, | ||||
|         { TYPE_DYNAMIC_PROPS, "prop2", "102", true }, | ||||
|         { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103", true }, | ||||
|         /* .not_used=false to emulate what qdev_add_one_global() does: */ | ||||
|         { TYPE_UNUSED_HOTPLUG, "prop4", "104", false }, | ||||
|         { TYPE_UNUSED_HOTPLUG, "prop4", "104", true }, | ||||
|         { TYPE_UNUSED_NOHOTPLUG, "prop5", "105", true }, | ||||
|         { TYPE_NONDEVICE, "prop6", "106", true }, | ||||
|         {} | ||||
| @ -226,12 +225,12 @@ static void test_dynamic_globalprop_subprocess(void) | ||||
|     g_assert_cmpuint(mt->prop2, ==, 102); | ||||
|     all_used = qdev_prop_check_globals(); | ||||
|     g_assert_cmpuint(all_used, ==, 1); | ||||
|     g_assert(!props[0].not_used); | ||||
|     g_assert(!props[1].not_used); | ||||
|     g_assert(props[2].not_used); | ||||
|     g_assert(!props[3].not_used); | ||||
|     g_assert(props[4].not_used); | ||||
|     g_assert(props[5].not_used); | ||||
|     g_assert(props[0].used); | ||||
|     g_assert(props[1].used); | ||||
|     g_assert(!props[2].used); | ||||
|     g_assert(!props[3].used); | ||||
|     g_assert(!props[4].used); | ||||
|     g_assert(!props[5].used); | ||||
| } | ||||
| 
 | ||||
| static void test_dynamic_globalprop(void) | ||||
| @ -240,10 +239,50 @@ static void test_dynamic_globalprop(void) | ||||
|     g_test_trap_assert_passed(); | ||||
|     g_test_trap_assert_stderr_unmatched("*prop1*"); | ||||
|     g_test_trap_assert_stderr_unmatched("*prop2*"); | ||||
|     g_test_trap_assert_stderr("*Warning: \"-global dynamic-prop-type-bad.prop3=103\" not used\n*"); | ||||
|     g_test_trap_assert_stderr("*Warning: global dynamic-prop-type-bad.prop3 has invalid class name\n*"); | ||||
|     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_stderr("*Warning: \"-global nondevice-type.prop6=106\" not used\n*"); | ||||
|     g_test_trap_assert_stderr("*Warning: global nohotplug-type.prop5=105 not used\n*"); | ||||
|     g_test_trap_assert_stderr("*Warning: global nondevice-type.prop6 has invalid class name\n*"); | ||||
|     g_test_trap_assert_stdout(""); | ||||
| } | ||||
| 
 | ||||
| /* Test setting of dynamic properties using user_provided=false properties */ | ||||
| static void test_dynamic_globalprop_nouser_subprocess(void) | ||||
| { | ||||
|     MyType *mt; | ||||
|     static GlobalProperty props[] = { | ||||
|         { TYPE_DYNAMIC_PROPS, "prop1", "101" }, | ||||
|         { TYPE_DYNAMIC_PROPS, "prop2", "102" }, | ||||
|         { TYPE_DYNAMIC_PROPS"-bad", "prop3", "103" }, | ||||
|         { TYPE_UNUSED_HOTPLUG, "prop4", "104" }, | ||||
|         { TYPE_UNUSED_NOHOTPLUG, "prop5", "105" }, | ||||
|         { TYPE_NONDEVICE, "prop6", "106" }, | ||||
|         {} | ||||
|     }; | ||||
|     int all_used; | ||||
| 
 | ||||
|     qdev_prop_register_global_list(props); | ||||
| 
 | ||||
|     mt = DYNAMIC_TYPE(object_new(TYPE_DYNAMIC_PROPS)); | ||||
|     qdev_init_nofail(DEVICE(mt)); | ||||
| 
 | ||||
|     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); | ||||
|     g_assert(!props[4].used); | ||||
|     g_assert(!props[5].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(""); | ||||
| } | ||||
| 
 | ||||
| @ -273,6 +312,11 @@ int main(int argc, char **argv) | ||||
|     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); | ||||
| 
 | ||||
|     g_test_run(); | ||||
| 
 | ||||
|     return 0; | ||||
|  | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user
	 Eduardo Habkost
						Eduardo Habkost