Am 24.09.2022 um 01:21 hat Claudio Fontana geschrieben: > improve error handling during module load, by changing: > > bool module_load(const char *prefix, const char *lib_name); > void module_load_qom(const char *type); > > to: > > int module_load(const char *prefix, const char *name, Error **errp); > int module_load_qom(const char *type, Error **errp); > > where the return value is: > > -1 on module load error, and errp is set with the error > 0 on module or one of its dependencies are not installed > 1 on module load success > 2 on module load success (module already loaded or built-in) > > module_load_qom_one has been introduced in: > > commit 28457744c345 ("module: qom module support"), which built on top of > module_load_one, but discarded the bool return value. Restore it. > > Adapt all callers to emit errors, or ignore them, or fail hard, > as appropriate in each context. > > Some memory leaks also fixed as part of the module_load changes. > > audio: when attempting to load an audio module, report module load errors. > block: when attempting to load a block module, report module load errors. > console: when attempting to load a display module, report module load errors. > > qdev: when creating a new qdev Device object (DeviceState), report load > errors. > If a module cannot be loaded to create that device, now abort execution. > > qom/object.c: when initializing a QOM object, or looking up class_by_name, > report module load errors. > > qtest: when processing the "module_load" qtest command, report errors > in the load of the module. > > Signed-off-by: Claudio Fontana <cfont...@suse.de>
> diff --git a/block/dmg.c b/block/dmg.c > index 007b8d9996..a422cf8d5b 100644 > --- a/block/dmg.c > +++ b/block/dmg.c > @@ -434,6 +434,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, > int flags, > uint64_t plist_xml_offset, plist_xml_length; > int64_t offset; > int ret; > + Error *local_err = NULL; > > ret = bdrv_apply_auto_read_only(bs, NULL, errp); > if (ret < 0) { > @@ -446,8 +447,15 @@ static int dmg_open(BlockDriverState *bs, QDict > *options, int flags, > return -EINVAL; > } > > - block_module_load("dmg-bz2"); > - block_module_load("dmg-lzfse"); > + if (block_module_load("dmg-bz2", &local_err) < 0) { > + error_report_err(local_err); > + return -EINVAL; > + } > + local_err = NULL; > + if (block_module_load("dmg-lzfse", &local_err) < 0) { > + error_report_err(local_err); > + return -EINVAL; > + } Why don't we pass the existing errp instead of adding a new local_err and printing it locally? If we use error_report_err(local_err) and leave errp untouched, the caller will print another less specific error message, which is generally not what we want. Kevin