Re: [RFC 3/3] qom: Improve error message in module_object_class_by_name()

[Jose R. Ziviani]

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 :|
>

signature.asc
Description: Digital signature