summary refs log tree commit diff stats
diff options
context:
space:
mode:
authorStefan Hajnoczi <stefanha@redhat.com>2022-11-07 05:44:35 -0500
committerStefan Hajnoczi <stefanha@redhat.com>2022-11-07 05:44:35 -0500
commit20a885a87d13fe3456b21716fdeca29519bdd9b8 (patch)
treebf8799b365f84102aaaad78c98576cc09eefa11e
parent64be791d3c251a09eb2dbbf556478f976211b30c (diff)
parent5141e9a23fc9a890d66a5700920a5ffd8885121f (diff)
downloadfocaccia-qemu-20a885a87d13fe3456b21716fdeca29519bdd9b8.tar.gz
focaccia-qemu-20a885a87d13fe3456b21716fdeca29519bdd9b8.zip
Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging
* bug fixes for Win32 event loop
* bug fixes for -Wextra
* fix gdb XML for 32-bit x86
* improve error handling for module load

# -----BEGIN PGP SIGNATURE-----
#
# iQFIBAABCAAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmNndPcUHHBib256aW5p
# QHJlZGhhdC5jb20ACgkQv/vSX3jHroOVVwf+Nfvx9aoDTo6vU4kHmCbh7+BTtcUA
# qEkaccL4pfujwNfRl5gHHKtRhgT6py9OjfIX8pAck3jtc5r+5/niFn7CLcOP2G/C
# xuyVKPx3ONCMKCLjWwg63I8/t4JZDEnYEzyddAfV7Xb5600aUJlrxScW751K5eYQ
# CzHFAASdc00eSiWE2eFL4rV7dcj0NadHnJxfjUcguHX4Qlr1TmK/ihEGv6MMRo5S
# 9Ak/vLmRs+LsQJm2cXUu5QVQiDwp31xbv3AP9knB+irYxrsp4LSDxV8eKZwLehpW
# I4A3gz2xKAprfzVJKZI3Dc/hJSRpoKoCjz+QoI1NeVbWjsnYZAwD+VMSew==
# =Nroy
# -----END PGP SIGNATURE-----
# gpg: Signature made Sun 06 Nov 2022 03:48:55 EST
# gpg:                using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg:                issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full]
# gpg:                 aka "Paolo Bonzini <pbonzini@redhat.com>" [full]
# Primary key fingerprint: 46F5 9FBD 57D6 12E7 BFD4  E2F7 7E15 100C CD36 69B1
#      Subkey fingerprint: F133 3857 4B66 2389 866C  7682 BFFB D25F 78C7 AE83

* tag 'for-upstream' of https://gitlab.com/bonzini/qemu:
  accel: abort if we fail to load the accelerator plugin
  dmg: warn when opening dmg images containing blocks of unknown type
  module: add Error arguments to module_load and module_load_qom
  module: rename module_load_one to module_load
  module: removed unused function argument "mayfail"
  Add missing include statement for global xml_builtin
  meson: avoid unused arguments of main() in compiler tests
  Fix broken configure with -Wunused-parameter
  gdb-xml: Fix size of EFER register on i386 architecture when debugged by GDB
  util/aio-win32: Correct the event array size in aio_poll()
  util/main-loop: Avoid adding the same HANDLE twice
  util/main-loop: Fix maximum number of wait objects for win32

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
-rw-r--r--accel/accel-softmmu.c8
-rw-r--r--audio/audio.c16
-rw-r--r--block.c20
-rw-r--r--block/dmg.c33
-rwxr-xr-xconfigure8
-rw-r--r--gdb-xml/i386-32bit.xml2
-rw-r--r--hw/core/qdev.c17
-rw-r--r--include/qemu/main-loop.h2
-rw-r--r--include/qemu/module.h37
-rw-r--r--meson.build8
-rw-r--r--qom/object.c18
-rw-r--r--scripts/feature_to_c.sh1
-rw-r--r--softmmu/qtest.c8
-rw-r--r--ui/console.c18
-rw-r--r--util/aio-win32.c5
-rw-r--r--util/main-loop.c20
-rw-r--r--util/module.c191
17 files changed, 280 insertions, 132 deletions
diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 67276e4f52..f9cdafb148 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -66,6 +66,7 @@ void accel_init_ops_interfaces(AccelClass *ac)
 {
     const char *ac_name;
     char *ops_name;
+    ObjectClass *oc;
     AccelOpsClass *ops;
 
     ac_name = object_class_get_name(OBJECT_CLASS(ac));
@@ -73,8 +74,13 @@ void accel_init_ops_interfaces(AccelClass *ac)
 
     ops_name = g_strdup_printf("%s" ACCEL_OPS_SUFFIX, ac_name);
     ops = ACCEL_OPS_CLASS(module_object_class_by_name(ops_name));
+    oc = module_object_class_by_name(ops_name);
+    if (!oc) {
+        error_report("fatal: could not load module for type '%s'", ops_name);
+        exit(1);
+    }
     g_free(ops_name);
-
+    ops = ACCEL_OPS_CLASS(oc);
     /*
      * all accelerators need to define ops, providing at least a mandatory
      * non-NULL create_vcpu_thread operation.
diff --git a/audio/audio.c b/audio/audio.c
index cc664271eb..065602ce1b 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -73,20 +73,24 @@ void audio_driver_register(audio_driver *drv)
 audio_driver *audio_driver_lookup(const char *name)
 {
     struct audio_driver *d;
+    Error *local_err = NULL;
+    int rv;
 
     QLIST_FOREACH(d, &audio_drivers, next) {
         if (strcmp(name, d->name) == 0) {
             return d;
         }
     }
-
-    audio_module_load_one(name);
-    QLIST_FOREACH(d, &audio_drivers, next) {
-        if (strcmp(name, d->name) == 0) {
-            return d;
+    rv = audio_module_load(name, &local_err);
+    if (rv > 0) {
+        QLIST_FOREACH(d, &audio_drivers, next) {
+            if (strcmp(name, d->name) == 0) {
+                return d;
+            }
         }
+    } else if (rv < 0) {
+        error_report_err(local_err);
     }
-
     return NULL;
 }
 
diff --git a/block.c b/block.c
index 3bd594eb2a..c5e20c0bea 100644
--- a/block.c
+++ b/block.c
@@ -464,12 +464,18 @@ BlockDriver *bdrv_find_format(const char *format_name)
     /* The driver isn't registered, maybe we need to load a module */
     for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
         if (!strcmp(block_driver_modules[i].format_name, format_name)) {
-            block_module_load_one(block_driver_modules[i].library_name);
+            Error *local_err = NULL;
+            int rv = block_module_load(block_driver_modules[i].library_name,
+                                       &local_err);
+            if (rv > 0) {
+                return bdrv_do_find_format(format_name);
+            } else if (rv < 0) {
+                error_report_err(local_err);
+            }
             break;
         }
     }
-
-    return bdrv_do_find_format(format_name);
+    return NULL;
 }
 
 static int bdrv_format_is_whitelisted(const char *format_name, bool read_only)
@@ -981,12 +987,16 @@ BlockDriver *bdrv_find_protocol(const char *filename,
     for (i = 0; i < (int)ARRAY_SIZE(block_driver_modules); ++i) {
         if (block_driver_modules[i].protocol_name &&
             !strcmp(block_driver_modules[i].protocol_name, protocol)) {
-            block_module_load_one(block_driver_modules[i].library_name);
+            int rv = block_module_load(block_driver_modules[i].library_name, errp);
+            if (rv > 0) {
+                drv1 = bdrv_do_find_protocol(protocol);
+            } else if (rv < 0) {
+                return NULL;
+            }
             break;
         }
     }
 
-    drv1 = bdrv_do_find_protocol(protocol);
     if (!drv1) {
         error_setg(errp, "Unknown protocol '%s'", protocol);
     }
diff --git a/block/dmg.c b/block/dmg.c
index 422136276a..675e840ca5 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -254,6 +254,25 @@ static int dmg_read_mish_block(BDRVDMGState *s, DmgHeaderState *ds,
     for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
         s->types[i] = buff_read_uint32(buffer, offset);
         if (!dmg_is_known_block_type(s->types[i])) {
+            switch (s->types[i]) {
+            case UDBZ:
+                warn_report_once("dmg-bzip2 module is missing, accessing bzip2 "
+                                 "compressed blocks will result in I/O errors");
+                break;
+            case ULFO:
+                warn_report_once("dmg-lzfse module is missing, accessing lzfse "
+                                 "compressed blocks will result in I/O errors");
+                break;
+            case UDCM:
+            case UDLE:
+                /* Comments and last entry can be ignored without problems */
+                break;
+            default:
+                warn_report_once("Image contains chunks of unknown type %x, "
+                                 "accessing them will result in I/O errors",
+                                 s->types[i]);
+                break;
+            }
             chunk_count--;
             i--;
             offset += 40;
@@ -444,9 +463,17 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
     if (ret < 0) {
         return ret;
     }
-
-    block_module_load_one("dmg-bz2");
-    block_module_load_one("dmg-lzfse");
+    /*
+     * NB: if uncompress submodules are absent,
+     * ie block_module_load return value == 0, the function pointers
+     * dmg_uncompress_bz2 and dmg_uncompress_lzfse will be NULL.
+     */
+    if (block_module_load("dmg-bz2", errp) < 0) {
+        return -EINVAL;
+    }
+    if (block_module_load("dmg-lzfse", errp) < 0) {
+        return -EINVAL;
+    }
 
     s->n_chunks = 0;
     s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
diff --git a/configure b/configure
index 4275f5419f..66928692b0 100755
--- a/configure
+++ b/configure
@@ -1258,7 +1258,7 @@ if test "$stack_protector" != "no"; then
   cat > $TMPC << EOF
 int main(int argc, char *argv[])
 {
-    char arr[64], *p = arr, *c = argv[0];
+    char arr[64], *p = arr, *c = argv[argc - 1];
     while (*c) {
         *p++ = *c++;
     }
@@ -1607,7 +1607,7 @@ fi
 
 if test "$safe_stack" = "yes"; then
 cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
 {
 #if ! __has_feature(safe_stack)
 #error SafeStack Disabled
@@ -1629,7 +1629,7 @@ EOF
   fi
 else
 cat > $TMPC << EOF
-int main(int argc, char *argv[])
+int main(void)
 {
 #if defined(__has_feature)
 #if __has_feature(safe_stack)
@@ -1675,7 +1675,7 @@ static const int Z = 1;
 #define TAUT(X) ((X) == Z)
 #define PAREN(X, Y) (X == Y)
 #define ID(X) (X)
-int main(int argc, char *argv[])
+int main(void)
 {
     int x = 0, y = 0;
     x = ID(x);
diff --git a/gdb-xml/i386-32bit.xml b/gdb-xml/i386-32bit.xml
index 872fcea9c2..7a66a02b67 100644
--- a/gdb-xml/i386-32bit.xml
+++ b/gdb-xml/i386-32bit.xml
@@ -110,7 +110,7 @@
 	<field name="PKE" start="22" end="22"/>
   </flags>
 
-  <flags id="i386_efer" size="8">
+  <flags id="i386_efer" size="4">
 	<field name="TCE" start="15" end="15"/>
 	<field name="FFXSR" start="14" end="14"/>
 	<field name="LMSLE" start="13" end="13"/>
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 0806d8fcaa..0145501904 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -147,8 +147,21 @@ bool qdev_set_parent_bus(DeviceState *dev, BusState *bus, Error **errp)
 
 DeviceState *qdev_new(const char *name)
 {
-    if (!object_class_by_name(name)) {
-        module_load_qom_one(name);
+    ObjectClass *oc = object_class_by_name(name);
+#ifdef CONFIG_MODULES
+    if (!oc) {
+        int rv = module_load_qom(name, &error_fatal);
+        if (rv > 0) {
+            oc = object_class_by_name(name);
+        } else {
+            error_report("could not find a module for type '%s'", name);
+            exit(1);
+        }
+    }
+#endif
+    if (!oc) {
+        error_report("unknown type '%s'", name);
+        abort();
     }
     return DEVICE(object_new(name));
 }
diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
index aac707d073..3c9a9a982d 100644
--- a/include/qemu/main-loop.h
+++ b/include/qemu/main-loop.h
@@ -157,6 +157,8 @@ typedef void WaitObjectFunc(void *opaque);
  * in the main loop's calls to WaitForMultipleObjects.  When the handle
  * is in a signaled state, QEMU will call @func.
  *
+ * If the same HANDLE is added twice, this function returns -1.
+ *
  * @handle: The Windows handle to be observed.
  * @func: A function to be called when @handle is in a signaled state.
  * @opaque: A pointer-size value that is passed to @func.
diff --git a/include/qemu/module.h b/include/qemu/module.h
index bd73607104..c37ce74b16 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -61,16 +61,43 @@ typedef enum {
 #define fuzz_target_init(function) module_init(function, \
                                                MODULE_INIT_FUZZ_TARGET)
 #define migration_init(function) module_init(function, MODULE_INIT_MIGRATION)
-#define block_module_load_one(lib) module_load_one("block-", lib, false)
-#define ui_module_load_one(lib) module_load_one("ui-", lib, false)
-#define audio_module_load_one(lib) module_load_one("audio-", lib, false)
+#define block_module_load(lib, errp) module_load("block-", lib, errp)
+#define ui_module_load(lib, errp) module_load("ui-", lib, errp)
+#define audio_module_load(lib, errp) module_load("audio-", lib, errp)
 
 void register_module_init(void (*fn)(void), module_init_type type);
 void register_dso_module_init(void (*fn)(void), module_init_type type);
 
 void module_call_init(module_init_type type);
-bool module_load_one(const char *prefix, const char *lib_name, bool mayfail);
-void module_load_qom_one(const char *type);
+
+/*
+ * module_load: attempt to load a module from a set of directories
+ *
+ * directories searched are:
+ * - getenv("QEMU_MODULE_DIR")
+ * - get_relocated_path(CONFIG_QEMU_MODDIR);
+ * - /var/run/qemu/${version_dir}
+ *
+ * prefix:         a subsystem prefix, or the empty string ("audio-", ..., "")
+ * name:           name of the module
+ * errp:           error to set in case the module is found, but load failed.
+ *
+ * Return value:   -1 on error (errp set if not NULL).
+ *                 0 if module or one of its dependencies are not installed,
+ *                 1 if the module is found and loaded,
+ *                 2 if the module is already loaded, or module is built-in.
+ */
+int module_load(const char *prefix, const char *name, Error **errp);
+
+/*
+ * module_load_qom: attempt to load a module to provide a QOM type
+ *
+ * type:           the type to be provided
+ * errp:           error to set.
+ *
+ * Return value:   as per module_load.
+ */
+int module_load_qom(const char *type, Error **errp);
 void module_load_qom_all(void);
 void module_allow_arch(const char *arch);
 
diff --git a/meson.build b/meson.build
index 1d448272ab..cf3e517e56 100644
--- a/meson.build
+++ b/meson.build
@@ -2165,7 +2165,7 @@ config_host_data.set('CONFIG_SPLICE', cc.links(gnu_source_prefix + '''
 
 config_host_data.set('HAVE_MLOCKALL', cc.links(gnu_source_prefix + '''
   #include <sys/mman.h>
-  int main(int argc, char *argv[]) {
+  int main(void) {
     return mlockall(MCL_FUTURE);
   }'''))
 
@@ -2210,7 +2210,7 @@ config_host_data.set('HAVE_FSXATTR', cc.links('''
 config_host_data.set('HAVE_BROKEN_SIZE_MAX', not cc.compiles('''
     #include <stdint.h>
     #include <stdio.h>
-    int main(int argc, char *argv[]) {
+    int main(void) {
         return printf("%zu", SIZE_MAX);
     }''', args: ['-Werror']))
 
@@ -2327,7 +2327,7 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \
       __m256i x = *(__m256i *)a;
       return _mm256_testz_si256(x, x);
     }
-    int main(int argc, char *argv[]) { return bar(argv[0]); }
+    int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
   '''), error_message: 'AVX2 not available').allowed())
 
 config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
@@ -2341,7 +2341,7 @@ config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \
       __m512i x = *(__m512i *)a;
       return _mm512_test_epi64_mask(x, x);
     }
-    int main(int argc, char *argv[]) { return bar(argv[0]); }
+    int main(int argc, char *argv[]) { return bar(argv[argc - 1]); }
   '''), error_message: 'AVX512F not available').allowed())
 
 have_pvrdma = get_option('pvrdma') \
diff --git a/qom/object.c b/qom/object.c
index e5cef30f6d..e25f1e96db 100644
--- a/qom/object.c
+++ b/qom/object.c
@@ -526,8 +526,13 @@ void object_initialize(void *data, size_t size, const char *typename)
 
 #ifdef CONFIG_MODULES
     if (!type) {
-        module_load_qom_one(typename);
-        type = type_get_by_name(typename);
+        int rv = module_load_qom(typename, &error_fatal);
+        if (rv > 0) {
+            type = type_get_by_name(typename);
+        } else {
+            error_report("missing object type '%s'", typename);
+            exit(1);
+        }
     }
 #endif
     if (!type) {
@@ -1033,8 +1038,13 @@ ObjectClass *module_object_class_by_name(const char *typename)
     oc = object_class_by_name(typename);
 #ifdef CONFIG_MODULES
     if (!oc) {
-        module_load_qom_one(typename);
-        oc = object_class_by_name(typename);
+        Error *local_err = NULL;
+        int rv = module_load_qom(typename, &local_err);
+        if (rv > 0) {
+            oc = object_class_by_name(typename);
+        } else if (rv < 0) {
+            error_report_err(local_err);
+        }
     }
 #endif
     return oc;
diff --git a/scripts/feature_to_c.sh b/scripts/feature_to_c.sh
index b1169899c1..c1f67c8f6a 100644
--- a/scripts/feature_to_c.sh
+++ b/scripts/feature_to_c.sh
@@ -56,6 +56,7 @@ for input; do
 done
 
 echo
+echo '#include "exec/gdbstub.h"'
 echo "const char *const xml_builtin[][2] = {"
 
 for input; do
diff --git a/softmmu/qtest.c b/softmmu/qtest.c
index afea7693d0..d3e0ab4eda 100644
--- a/softmmu/qtest.c
+++ b/softmmu/qtest.c
@@ -753,12 +753,18 @@ static void qtest_process_command(CharBackend *chr, gchar **words)
         qtest_sendf(chr, "OK %"PRIi64"\n",
                     (int64_t)qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL));
     } else if (strcmp(words[0], "module_load") == 0) {
+        Error *local_err = NULL;
+        int rv;
         g_assert(words[1] && words[2]);
 
         qtest_send_prefix(chr);
-        if (module_load_one(words[1], words[2], false)) {
+        rv = module_load(words[1], words[2], &local_err);
+        if (rv > 0) {
             qtest_sendf(chr, "OK\n");
         } else {
+            if (rv < 0) {
+                error_report_err(local_err);
+            }
             qtest_sendf(chr, "FAIL\n");
         }
     } else if (qtest_enabled() && strcmp(words[0], "clock_set") == 0) {
diff --git a/ui/console.c b/ui/console.c
index 65c117874c..3c0d9b061a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -2632,7 +2632,11 @@ bool qemu_display_find_default(DisplayOptions *opts)
 
     for (i = 0; i < (int)ARRAY_SIZE(prio); i++) {
         if (dpys[prio[i]] == NULL) {
-            ui_module_load_one(DisplayType_str(prio[i]));
+            Error *local_err = NULL;
+            int rv = ui_module_load(DisplayType_str(prio[i]), &local_err);
+            if (rv < 0) {
+                error_report_err(local_err);
+            }
         }
         if (dpys[prio[i]] == NULL) {
             continue;
@@ -2650,7 +2654,11 @@ void qemu_display_early_init(DisplayOptions *opts)
         return;
     }
     if (dpys[opts->type] == NULL) {
-        ui_module_load_one(DisplayType_str(opts->type));
+        Error *local_err = NULL;
+        int rv = ui_module_load(DisplayType_str(opts->type), &local_err);
+        if (rv < 0) {
+            error_report_err(local_err);
+        }
     }
     if (dpys[opts->type] == NULL) {
         error_report("Display '%s' is not available.",
@@ -2680,7 +2688,11 @@ void qemu_display_help(void)
     printf("none\n");
     for (idx = DISPLAY_TYPE_NONE; idx < DISPLAY_TYPE__MAX; idx++) {
         if (!dpys[idx]) {
-            ui_module_load_one(DisplayType_str(idx));
+            Error *local_err = NULL;
+            int rv = ui_module_load(DisplayType_str(idx), &local_err);
+            if (rv < 0) {
+                error_report_err(local_err);
+            }
         }
         if (dpys[idx]) {
             printf("%s\n",  DisplayType_str(dpys[idx]->type));
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 44003d645e..80cfe012ad 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -326,9 +326,9 @@ void aio_dispatch(AioContext *ctx)
 bool aio_poll(AioContext *ctx, bool blocking)
 {
     AioHandler *node;
-    HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
+    HANDLE events[MAXIMUM_WAIT_OBJECTS];
     bool progress, have_select_revents, first;
-    int count;
+    unsigned count;
     int timeout;
 
     /*
@@ -369,6 +369,7 @@ bool aio_poll(AioContext *ctx, bool blocking)
     QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) {
         if (!node->deleted && node->io_notify
             && aio_node_check(ctx, node->is_external)) {
+            assert(count < MAXIMUM_WAIT_OBJECTS);
             events[count++] = event_notifier_get_handle(node->e);
         }
     }
diff --git a/util/main-loop.c b/util/main-loop.c
index f00a25451b..10fa74c6e3 100644
--- a/util/main-loop.c
+++ b/util/main-loop.c
@@ -363,20 +363,30 @@ void qemu_del_polling_cb(PollingFunc *func, void *opaque)
 /* Wait objects support */
 typedef struct WaitObjects {
     int num;
-    int revents[MAXIMUM_WAIT_OBJECTS + 1];
-    HANDLE events[MAXIMUM_WAIT_OBJECTS + 1];
-    WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS + 1];
-    void *opaque[MAXIMUM_WAIT_OBJECTS + 1];
+    int revents[MAXIMUM_WAIT_OBJECTS];
+    HANDLE events[MAXIMUM_WAIT_OBJECTS];
+    WaitObjectFunc *func[MAXIMUM_WAIT_OBJECTS];
+    void *opaque[MAXIMUM_WAIT_OBJECTS];
 } WaitObjects;
 
 static WaitObjects wait_objects = {0};
 
 int qemu_add_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
 {
+    int i;
     WaitObjects *w = &wait_objects;
+
     if (w->num >= MAXIMUM_WAIT_OBJECTS) {
         return -1;
     }
+
+    for (i = 0; i < w->num; i++) {
+        /* check if the same handle is added twice */
+        if (w->events[i] == handle) {
+            return -1;
+        }
+    }
+
     w->events[w->num] = handle;
     w->func[w->num] = func;
     w->opaque[w->num] = opaque;
@@ -395,7 +405,7 @@ void qemu_del_wait_object(HANDLE handle, WaitObjectFunc *func, void *opaque)
         if (w->events[i] == handle) {
             found = 1;
         }
-        if (found) {
+        if (found && i < (MAXIMUM_WAIT_OBJECTS - 1)) {
             w->events[i] = w->events[i + 1];
             w->func[i] = w->func[i + 1];
             w->opaque[i] = w->opaque[i + 1];
diff --git a/util/module.c b/util/module.c
index 8ddb0e18f5..32e263163c 100644
--- a/util/module.c
+++ b/util/module.c
@@ -21,6 +21,7 @@
 #include "qemu/module.h"
 #include "qemu/cutils.h"
 #include "qemu/config-file.h"
+#include "qapi/error.h"
 #ifdef CONFIG_MODULE_UPGRADES
 #include "qemu-version.h"
 #endif
@@ -144,25 +145,22 @@ static bool module_check_arch(const QemuModinfo *modinfo)
     return true;
 }
 
-static int module_load_file(const char *fname, bool mayfail, bool export_symbols)
+/*
+ * module_load_dso: attempt to load an existing dso file
+ *
+ * fname:          full pathname of the file to load
+ * export_symbols: if true, add the symbols to the global name space
+ * errp:           error to set.
+ *
+ * Return value:   true on success, false on error, and errp will be set.
+ */
+static bool module_load_dso(const char *fname, bool export_symbols,
+                            Error **errp)
 {
     GModule *g_module;
     void (*sym)(void);
-    const char *dsosuf = CONFIG_HOST_DSOSUF;
-    int len = strlen(fname);
-    int suf_len = strlen(dsosuf);
     ModuleEntry *e, *next;
-    int ret, flags;
-
-    if (len <= suf_len || strcmp(&fname[len - suf_len], dsosuf)) {
-        /* wrong suffix */
-        ret = -EINVAL;
-        goto out;
-    }
-    if (access(fname, F_OK)) {
-        ret = -ENOENT;
-        goto out;
-    }
+    int flags;
 
     assert(QTAILQ_EMPTY(&dso_init_list));
 
@@ -172,48 +170,38 @@ static int module_load_file(const char *fname, bool mayfail, bool export_symbols
     }
     g_module = g_module_open(fname, flags);
     if (!g_module) {
-        if (!mayfail) {
-            fprintf(stderr, "Failed to open module: %s\n",
-                    g_module_error());
-        }
-        ret = -EINVAL;
-        goto out;
+        error_setg(errp, "failed to open module: %s", g_module_error());
+        return false;
     }
     if (!g_module_symbol(g_module, DSO_STAMP_FUN_STR, (gpointer *)&sym)) {
-        fprintf(stderr, "Failed to initialize module: %s\n",
-                fname);
-        /* Print some info if this is a QEMU module (but from different build),
-         * this will make debugging user problems easier. */
+        error_setg(errp, "failed to initialize module: %s", fname);
+        /*
+         * Print some info if this is a QEMU module (but from different build),
+         * this will make debugging user problems easier.
+         */
         if (g_module_symbol(g_module, "qemu_module_dummy", (gpointer *)&sym)) {
-            fprintf(stderr,
-                    "Note: only modules from the same build can be loaded.\n");
+            error_append_hint(errp,
+                "Only modules from the same build can be loaded.\n");
         }
         g_module_close(g_module);
-        ret = -EINVAL;
-    } else {
-        QTAILQ_FOREACH(e, &dso_init_list, node) {
-            e->init();
-            register_module_init(e->init, e->type);
-        }
-        ret = 0;
+        return false;
     }
 
+    QTAILQ_FOREACH(e, &dso_init_list, node) {
+        e->init();
+        register_module_init(e->init, e->type);
+    }
     trace_module_load_module(fname);
     QTAILQ_FOREACH_SAFE(e, &dso_init_list, node, next) {
         QTAILQ_REMOVE(&dso_init_list, e, node);
         g_free(e);
     }
-out:
-    return ret;
+    return true;
 }
-#endif
 
-bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
+int module_load(const char *prefix, const char *name, Error **errp)
 {
-    bool success = false;
-
-#ifdef CONFIG_MODULES
-    char *fname = NULL;
+    int rv = -1;
 #ifdef CONFIG_MODULE_UPGRADES
     char *version_dir;
 #endif
@@ -221,34 +209,52 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
     char *dirs[5];
     char *module_name;
     int i = 0, n_dirs = 0;
-    int ret;
     bool export_symbols = false;
     static GHashTable *loaded_modules;
     const QemuModinfo *modinfo;
     const char **sl;
 
     if (!g_module_supported()) {
-        fprintf(stderr, "Module is not supported by system.\n");
-        return false;
+        error_setg(errp, "%s", "this platform does not support GLib modules");
+        return -1;
     }
 
     if (!loaded_modules) {
         loaded_modules = g_hash_table_new(g_str_hash, g_str_equal);
     }
 
-    module_name = g_strdup_printf("%s%s", prefix, lib_name);
+    /* allocate all resources managed by the out: label here */
+    module_name = g_strdup_printf("%s%s", prefix, name);
 
     if (g_hash_table_contains(loaded_modules, module_name)) {
         g_free(module_name);
-        return true;
+        return 2; /* module already loaded */
     }
     g_hash_table_add(loaded_modules, module_name);
 
+    search_dir = getenv("QEMU_MODULE_DIR");
+    if (search_dir != NULL) {
+        dirs[n_dirs++] = g_strdup_printf("%s", search_dir);
+    }
+    dirs[n_dirs++] = get_relocated_path(CONFIG_QEMU_MODDIR);
+
+#ifdef CONFIG_MODULE_UPGRADES
+    version_dir = g_strcanon(g_strdup(QEMU_PKGVERSION),
+                             G_CSET_A_2_Z G_CSET_a_2_z G_CSET_DIGITS "+-.~",
+                             '_');
+    dirs[n_dirs++] = g_strdup_printf("/var/run/qemu/%s", version_dir);
+#endif
+    assert(n_dirs <= ARRAY_SIZE(dirs));
+
+    /* end of resources managed by the out: label */
+
     for (modinfo = module_info; modinfo->name != NULL; modinfo++) {
         if (modinfo->arch) {
             if (strcmp(modinfo->name, module_name) == 0) {
                 if (!module_check_arch(modinfo)) {
-                    return false;
+                    error_setg(errp, "module arch does not match: "
+                        "expected '%s', got '%s'", module_arch, modinfo->arch);
+                    goto out;
                 }
             }
         }
@@ -256,7 +262,11 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
             if (strcmp(modinfo->name, module_name) == 0) {
                 /* we depend on other module(s) */
                 for (sl = modinfo->deps; *sl != NULL; sl++) {
-                    module_load_one("", *sl, false);
+                    int subrv = module_load("", *sl, errp);
+                    if (subrv <= 0) {
+                        rv = subrv;
+                        goto out;
+                    }
                 }
             } else {
                 for (sl = modinfo->deps; *sl != NULL; sl++) {
@@ -269,58 +279,52 @@ bool module_load_one(const char *prefix, const char *lib_name, bool mayfail)
         }
     }
 
-    search_dir = getenv("QEMU_MODULE_DIR");
-    if (search_dir != NULL) {
-        dirs[n_dirs++] = g_strdup_printf("%s", search_dir);
-    }
-    dirs[n_dirs++] = get_relocated_path(CONFIG_QEMU_MODDIR);
-
-#ifdef CONFIG_MODULE_UPGRADES
-    version_dir = g_strcanon(g_strdup(QEMU_PKGVERSION),
-                             G_CSET_A_2_Z G_CSET_a_2_z G_CSET_DIGITS "+-.~",
-                             '_');
-    dirs[n_dirs++] = g_strdup_printf("/var/run/qemu/%s", version_dir);
-#endif
-
-    assert(n_dirs <= ARRAY_SIZE(dirs));
-
     for (i = 0; i < n_dirs; i++) {
-        fname = g_strdup_printf("%s/%s%s",
-                dirs[i], module_name, CONFIG_HOST_DSOSUF);
-        ret = module_load_file(fname, mayfail, export_symbols);
-        g_free(fname);
-        fname = NULL;
-        /* Try loading until loaded a module file */
-        if (!ret) {
-            success = true;
-            break;
+        char *fname = g_strdup_printf("%s/%s%s",
+                                      dirs[i], module_name, CONFIG_HOST_DSOSUF);
+        int ret = access(fname, F_OK);
+        if (ret != 0 && (errno == ENOENT || errno == ENOTDIR)) {
+            /*
+             * if we don't find the module in this dir, try the next one.
+             * If we don't find it in any dir, that can be fine too: user
+             * did not install the module. We will return 0 in this case
+             * with no error set.
+             */
+            g_free(fname);
+            continue;
+        } else if (ret != 0) {
+            /* most common is EACCES here */
+            error_setg_errno(errp, errno, "error trying to access %s", fname);
+        } else if (module_load_dso(fname, export_symbols, errp)) {
+            rv = 1; /* module successfully loaded */
         }
+        g_free(fname);
+        goto out;
     }
+    rv = 0; /* module not found */
 
-    if (!success) {
+out:
+    if (rv <= 0) {
         g_hash_table_remove(loaded_modules, module_name);
         g_free(module_name);
     }
-
     for (i = 0; i < n_dirs; i++) {
         g_free(dirs[i]);
     }
-
-#endif
-    return success;
+    return rv;
 }
 
-#ifdef CONFIG_MODULES
-
 static bool module_loaded_qom_all;
 
-void module_load_qom_one(const char *type)
+int module_load_qom(const char *type, Error **errp)
 {
     const QemuModinfo *modinfo;
     const char **sl;
+    int rv = 0;
 
     if (!type) {
-        return;
+        error_setg(errp, "%s", "type is NULL");
+        return -1;
     }
 
     trace_module_lookup_object_type(type);
@@ -333,15 +337,24 @@ void module_load_qom_one(const char *type)
         }
         for (sl = modinfo->objs; *sl != NULL; sl++) {
             if (strcmp(type, *sl) == 0) {
-                module_load_one("", modinfo->name, false);
+                if (rv > 0) {
+                    error_setg(errp, "multiple modules providing '%s'", type);
+                    return -1;
+                }
+                rv = module_load("", modinfo->name, errp);
+                if (rv < 0) {
+                    return rv;
+                }
             }
         }
     }
+    return rv;
 }
 
 void module_load_qom_all(void)
 {
     const QemuModinfo *modinfo;
+    Error *local_err = NULL;
 
     if (module_loaded_qom_all) {
         return;
@@ -354,7 +367,9 @@ void module_load_qom_all(void)
         if (!module_check_arch(modinfo)) {
             continue;
         }
-        module_load_one("", modinfo->name, false);
+        if (module_load("", modinfo->name, &local_err) < 0) {
+            error_report_err(local_err);
+        }
     }
     module_loaded_qom_all = true;
 }
@@ -370,7 +385,10 @@ void qemu_load_module_for_opts(const char *group)
         }
         for (sl = modinfo->opts; *sl != NULL; sl++) {
             if (strcmp(group, *sl) == 0) {
-                module_load_one("", modinfo->name, false);
+                Error *local_err = NULL;
+                if (module_load("", modinfo->name, &local_err) < 0) {
+                    error_report_err(local_err);
+                }
             }
         }
     }
@@ -380,7 +398,8 @@ void qemu_load_module_for_opts(const char *group)
 
 void module_allow_arch(const char *arch) {}
 void qemu_load_module_for_opts(const char *group) {}
-void module_load_qom_one(const char *type) {}
+int module_load(const char *prefix, const char *name, Error **errp) { return 2; }
+int module_load_qom(const char *type, Error **errp) { return 2; }
 void module_load_qom_all(void) {}
 
 #endif