On Wed, Jul 21, 2021 at 10:57:37AM +0100, Daniel P. Berrangé wrote: > On Wed, Jul 21, 2021 at 11:54:45AM +0200, Gerd Hoffmann wrote: > > > ObjectClass *module_object_class_by_name(const char *typename) > > > { > > > ObjectClass *oc; > > > @@ -1031,8 +1049,20 @@ ObjectClass *module_object_class_by_name(const char *typename) > > > oc = object_class_by_name(typename); > > > #ifdef CONFIG_MODULES > > > if (!oc) { > > > + char *module_name; > > > module_load_qom_one(typename); > > > oc = object_class_by_name(typename); > > > + module_name = get_accel_module_name(typename); > > > + if (module_name) { > > > + if (!module_is_loaded(module_name)) { > > > + fprintf(stderr, "%s module is missing, install the " > > > + "package or config the library path " > > > + "correctly.\n", module_name); > > > + g_free(module_name); > > > + exit(1); > > > + } > > > + g_free(module_name); > > > + } > > > > This error logging should IMHO be moved to util/module.c. Either have a > > helper function to print the error message, or have > > module_load_qom_one() print it. > > > > There is also no need to hard-code the module names. We have the module > > database and module_load_qom_one() uses it to figure which module must > > be loaded for a specific qom object. We can likewise use the database > > for printing the error message. > > IIUC, module loading can be triggered from hotplug of backends/devices, > If so, we really shouldn't be printing to stderr, but using Error *errp > throughout, so that QMP can give back useful error messages Thank you Gerd and Daniel, I'll improve it and send a v2. Thank you very much, Jose > > > Regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| >