ping, can this series move on? Thanks,
Claudio On 9/29/22 11:30, Claudio Fontana wrote: > CHANGELOG: > > v8 -> v9: > > * add Signed-off-by tag for Kevin's commit > * fully reviewed, added tags. > > v7 -> v8: > > * fix a problem in module_load, where the module_name in v7 was mistakenly > freed > via g_free() also in the success code path, and instead module_name memory > is owned by g_hash_table afer g_hash_table_add. > > * add more text to the commit message to indicate areas of further > improvements, > and more details about changes. > > * in PATCH 5/5, change the commit message to align with the change in v7, > ie, we exit(), we do not abort(). > > v6 -> v7: > > * changed instances of abort() to exit(1), for the CONFIG_MODULES case > (Philippe). > > * dmg: do not use a separate local error, use the existing errp (Kevin) > > * block: do not use a separate local error, use the existing errp for > bdrv_find_protocol (Markus) > > v5 -> v6: > > * added a patch by Kevin to handle the dmg warning about missing > decompression submodules. (Kevin) > > * added more verbose comments about all the affected callers of module_load > and module_load_qom (Markus) > > (OPEN ISSUE): change abort() to exit() when type not present even after > loading module? > > (Philippe) > > v4 -> v5: > > * added a patch to rename module_load_one and friends to module_load > > * qdev_new: just reuse module_object_class_by_name, to avoid duplicating code > > * changed return value of module_load to an int: > -1 error (Error **errp set). > 0 module or dependencies not installed, > 1 loaded > 2 already loaded (or built-in) > > Adapted all callers. > > * module_load: fixed some pre-existing memory leaks, used an out: label > to do the cleanup. > > v3 -> v4: (Richard) > > * module_object_class_by_name: return NULL immediately on load error. > * audio_driver_lookup: same. > * bdrv_find_format: same. > > * dmg_open: handle optional compression submodules better: f.e., > if "dmg-bz2" is not present, continue but offer a warning. > If "dmg-bz2" load fails with error, error out and return. > > * module_load_dso: add newline to error_append_hint. > > v2 -> v3: > > * take the file existence check outside of module_load_file, > rename module_load_file to module_load_dso, will be called only on > an existing file. This will simplify the return value. (Richard) > > * move exported function documentation into header files (Richard) > > v1 -> v2: > > * do not treat the display help text any differently and do report > module load _errors_. If the module does not exist (ENOENT, ENOTDIR), > no error will be produced. > > DESCRIPTION: > > while investigating a permission issue in accel, where accel-tcg-x86_64.so > was not accessible, I noticed that no errors were produced regarding the > module load failure. > > This series attempts to improve module_load_one and module_load_qom_one > to handle the error cases better and produce some errors. > > Patch 1 is already reviewed and is about removing an unused existing > argument "mayfail" from the call stack. > > Patch 2 is the real meat, and that one I would say is RFC. > Will follow up with comments on the specific questions I have. > > Patch 3 finally adds a simple check in accel/, aborting if a module > is not found, but relying on the existing error report from > module_load_qom_one. > > Claudio Fontana (4): > module: removed unused function argument "mayfail" > module: rename module_load_one to module_load > module: add Error arguments to module_load and module_load_qom > accel: abort if we fail to load the accelerator plugin > > Kevin Wolf (1): > dmg: warn when opening dmg images containing blocks of unknown type > > accel/accel-softmmu.c | 8 +- > audio/audio.c | 16 ++-- > block.c | 20 +++- > block/dmg.c | 33 ++++++- > hw/core/qdev.c | 17 +++- > include/qemu/module.h | 37 +++++++- > qom/object.c | 18 +++- > softmmu/qtest.c | 8 +- > ui/console.c | 18 +++- > util/module.c | 211 +++++++++++++++++++++++------------------- > 10 files changed, 260 insertions(+), 126 deletions(-) >
