[Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Marc Marí]

The current module infrastructure has been improved to enable dynamic module
loading.

This reduces the load time for very simple guests. For the following
configuration (very loaded)

./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
    --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
    --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
    --enable-kvm --enable-rdma --enable-uuid --enable-vde \
    --enable-linux-aio --enable-cap-ng --enable-attr --enable-vhost-net \
    --enable-vhost-scsi --enable-spice --enable-rbd --enable-libiscsi \
    --enable-smartcard-nss --enable-guest-agent --enable-libusb \
    --enable-usb-redir --enable-lzo --enable-snappy --enable-bzip2 \
    --enable-seccomp --enable-coroutine-pool --enable-glusterfs \
    --enable-tpm --enable-libssh2 --enable-vhdx --enable-numa \
    --enable-tcmalloc --target-list=x86_64-softmmu

With modules disabled, there are 142 libraries loaded at startup. Time is
the following:
 LD time: 0.065 seconds
 QEMU time: 0.02 seconds
 Total time: 0.085 seconds

With this patch series and modules enabled, there are 128 libraries loaded
at startup. Time is the following:
 LD time: 0.02 seconds
 QEMU time: 0.02 seconds
 Total time: 0.04 seconds

Where LD time is the time between the program startup and the jump to main,
and QEMU time is the time between the start of main and the first kvm_entry.

These results are just with a few block drivers, that were already a module.
Adding more modules (block or not block) should be easy, and will reduce
the load time even more.

Marc Marí (2):
  Add dynamic module loading for block drivers
  Add dynamic generation of module_block.h

 .gitignore                      |   1 +
 Makefile                        |  10 ++-
 block.c                         |  73 +++++++++++++++++++++-
 configure                       |   2 +-
 include/qemu/module.h           |   3 +
 include/qemu/module_block.h     |  89 +++++++++++++++++++++++++++
 scripts/modules/module_block.py | 132 ++++++++++++++++++++++++++++++++++++++++
 util/module.c                   |  38 ++++--------
 8 files changed, 314 insertions(+), 34 deletions(-)
 create mode 100644 include/qemu/module_block.h
 create mode 100755 scripts/modules/module_block.py

-- 
2.4.3

[Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Marc Marí]

Extend the current module interface to allow for block drivers to be loaded
dynamically on request.

The only block drivers that can be converted into modules are the drivers
that don't perform any init operation except for registering themselves. This
is why libiscsi has been disabled as a module.

All the necessary module information is located in a new structure found in
include/qemu/module_block.h

Signed-off-by: Marc Marí <markmb@redhat.com>
---
 block.c                     | 73 +++++++++++++++++++++++++++++++++++--
 configure                   |  2 +-
 include/qemu/module.h       |  3 ++
 include/qemu/module_block.h | 89 +++++++++++++++++++++++++++++++++++++++++++++
 util/module.c               | 38 ++++++-------------
 5 files changed, 174 insertions(+), 31 deletions(-)
 create mode 100644 include/qemu/module_block.h

diff --git a/block.c b/block.c
index d088ee0..f24a624 100644
--- a/block.c
+++ b/block.c
@@ -27,6 +27,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/error-report.h"
+#include "qemu/module_block.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qjson.h"
@@ -277,11 +278,30 @@ void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
+    int i;
+
     QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (!strcmp(drv1->format_name, format_name)) {
             return drv1;
         }
     }
+
+    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
+        if (!strcmp(block_driver_module[i].format_name, format_name)) {
+            block_module_load_one(block_driver_module[i].library_name);
+            /* Copying code is not nice, but this way the current discovery is
+             * not modified. Calling recursively could fail if the library
+             * has been deleted.
+             */
+            QLIST_FOREACH(drv1, &bdrv_drivers, list) {
+                if (!strcmp(drv1->format_name, format_name)) {
+                    return drv1;
+                }
+            }
+        }
+    }
+
+
     return NULL;
 }
 
@@ -483,9 +503,15 @@ int get_tmp_filename(char *filename, int size)
  */
 static BlockDriver *find_hdev_driver(const char *filename)
 {
-    int score_max = 0, score;
+    int score_max = 0, score, i;
     BlockDriver *drv = NULL, *d;
 
+    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
+        if (block_driver_module[i].has_probe_device) {
+            block_module_load_one(block_driver_module[i].library_name);
+        }
+    }
+
     QLIST_FOREACH(d, &bdrv_drivers, list) {
         if (d->bdrv_probe_device) {
             score = d->bdrv_probe_device(filename);
@@ -507,6 +533,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
     char protocol[128];
     int len;
     const char *p;
+    int i;
 
     /* TODO Drivers without bdrv_file_open must be specified explicitly */
 
@@ -533,6 +560,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
         len = sizeof(protocol) - 1;
     memcpy(protocol, filename, len);
     protocol[len] = '\0';
+
     QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (drv1->protocol_name &&
             !strcmp(drv1->protocol_name, protocol)) {
@@ -540,6 +568,23 @@ BlockDriver *bdrv_find_protocol(const char *filename,
         }
     }
 
+    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
+        if (block_driver_module[i].protocol_name &&
+            !strcmp(block_driver_module[i].protocol_name, protocol)) {
+            block_module_load_one(block_driver_module[i].library_name);
+            /* Copying code is not nice, but this way the current discovery is
+             * not modified. Calling recursively could fail if the library
+             * has been deleted.
+             */
+            QLIST_FOREACH(drv1, &bdrv_drivers, list) {
+                if (drv1->protocol_name &&
+                    !strcmp(drv1->protocol_name, protocol)) {
+                    return drv1;
+                }
+            }
+        }
+    }
+
     error_setg(errp, "Unknown protocol '%s'", protocol);
     return NULL;
 }
@@ -561,9 +606,15 @@ BlockDriver *bdrv_find_protocol(const char *filename,
 BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                             const char *filename)
 {
-    int score_max = 0, score;
+    int score_max = 0, score, i;
     BlockDriver *drv = NULL, *d;
 
+    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
+        if (block_driver_module[i].has_probe) {
+            block_module_load_one(block_driver_module[i].library_name);
+        }
+    }
+
     QLIST_FOREACH(d, &bdrv_drivers, list) {
         if (d->bdrv_probe) {
             score = d->bdrv_probe(buf, buf_size, filename);
@@ -2783,7 +2834,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
 {
     BlockDriver *drv;
     int count = 0;
-    int i;
+    int i, n;
     const char **formats = NULL;
 
     QLIST_FOREACH(drv, &bdrv_drivers, list) {
@@ -2801,6 +2852,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
         }
     }
 
+    for (n = 0; n < ARRAY_SIZE(block_driver_module); ++n) {
+        if (block_driver_module[n].format_name) {
+            bool found = false;
+            int i = count;
+            while (formats && i && !found) {
+                found = !strcmp(formats[--i],
+                                block_driver_module[n].format_name);
+            }
+
+            if (!found) {
+                formats = g_renew(const char *, formats, count + 1);
+                formats[count++] = block_driver_module[n].format_name;
+            }
+        }
+    }
+
     qsort(formats, count, sizeof(formats[0]), qsort_strcmp);
 
     for (i = 0; i < count; i++) {
diff --git a/configure b/configure
index cd219d8..9fca9ee 100755
--- a/configure
+++ b/configure
@@ -4971,7 +4971,7 @@ if test "$bzip2" = "yes" ; then
 fi
 
 if test "$libiscsi" = "yes" ; then
-  echo "CONFIG_LIBISCSI=m" >> $config_host_mak
+  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
   echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
   echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak
 fi
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 72d9498..0ad4bb7 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -53,9 +53,12 @@ typedef enum {
 #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
 #define type_init(function) module_init(function, MODULE_INIT_QOM)
 
+#define block_module_load_one(lib) module_load_one("block-", lib);
+
 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);
+void module_load_one(const char *prefix, const char *lib_name);
 
 #endif
diff --git a/include/qemu/module_block.h b/include/qemu/module_block.h
new file mode 100644
index 0000000..f1d389c
--- /dev/null
+++ b/include/qemu/module_block.h
@@ -0,0 +1,88 @@
+/*
+ * QEMU Block Module Infrastructure
+ *
+ * Copyright Red Hat, Inc. 2015
+ *
+ * Authors:
+ *  Marc Mari       <markmb@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef QEMU_MODULE_BLOCK_H
+#define QEMU_MODULE_BLOCK_H
+
+#include "qemu-common.h"
+
+static const struct {
+    const char *format_name;
+    const char *protocol_name;
+    const char *library_name;
+    bool has_probe;
+    bool has_probe_device;
+} block_driver_module[] = {
+	{
+	.library_name = "curl",
+	.format_name = "http",
+	.protocol_name = "http",
+	},
+	{
+	.library_name = "curl",
+	.format_name = "https",
+	.protocol_name = "https",
+	},
+	{
+	.library_name = "curl",
+	.format_name = "ftp",
+	.protocol_name = "ftp",
+	},
+	{
+	.library_name = "curl",
+	.format_name = "ftps",
+	.protocol_name = "ftps",
+	},
+	{
+	.library_name = "curl",
+	.format_name = "tftp",
+	.protocol_name = "tftp",
+	},
+	{
+	.library_name = "rbd",
+	.format_name = "rbd",
+	.protocol_name = "rbd",
+	},
+	{
+	.library_name = "gluster",
+	.format_name = "gluster",
+	.protocol_name = "gluster",
+	},
+	{
+	.library_name = "gluster",
+	.format_name = "gluster",
+	.protocol_name = "gluster+tcp",
+	},
+	{
+	.library_name = "gluster",
+	.format_name = "gluster",
+	.protocol_name = "gluster+unix",
+	},
+	{
+	.library_name = "gluster",
+	.format_name = "gluster",
+	.protocol_name = "gluster+rdma",
+	},
+	{
+	.library_name = "ssh",
+	.format_name = "ssh",
+	.protocol_name = "ssh",
+	},
+	{
+	.library_name = "dmg",
+	.format_name = "dmg",
+	.has_probe = true,
+	},
+};
+
+#endif
\ No newline at end of file
diff --git a/util/module.c b/util/module.c
index 4bd4a94..992d317 100644
--- a/util/module.c
+++ b/util/module.c
@@ -91,14 +91,11 @@ void register_dso_module_init(void (*fn)(void), module_init_type type)
     QTAILQ_INSERT_TAIL(&dso_init_list, e, node);
 }
 
-static void module_load(module_init_type type);
-
 void module_call_init(module_init_type type)
 {
     ModuleTypeList *l;
     ModuleEntry *e;
 
-    module_load(type);
     l = find_type(type);
 
     QTAILQ_FOREACH(e, l, node) {
@@ -149,6 +146,7 @@ static int module_load_file(const char *fname)
         ret = -EINVAL;
     } else {
         QTAILQ_FOREACH(e, &dso_init_list, node) {
+            e->init();
             register_module_init(e->init, e->type);
         }
         ret = 0;
@@ -163,14 +161,10 @@ out:
 }
 #endif
 
-static void module_load(module_init_type type)
+void module_load_one(const char *prefix, const char *lib_name)
 {
 #ifdef CONFIG_MODULES
     char *fname = NULL;
-    const char **mp;
-    static const char *block_modules[] = {
-        CONFIG_BLOCK_MODULES
-    };
     char *exec_dir;
     char *dirs[3];
     int i = 0;
@@ -181,15 +175,6 @@ static void module_load(module_init_type type)
         return;
     }
 
-    switch (type) {
-    case MODULE_INIT_BLOCK:
-        mp = block_modules;
-        break;
-    default:
-        /* no other types have dynamic modules for now*/
-        return;
-    }
-
     exec_dir = qemu_get_exec_dir();
     dirs[i++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR);
     dirs[i++] = g_strdup_printf("%s/..", exec_dir ? : "");
@@ -198,16 +183,15 @@ static void module_load(module_init_type type)
     g_free(exec_dir);
     exec_dir = NULL;
 
-    for ( ; *mp; mp++) {
-        for (i = 0; i < ARRAY_SIZE(dirs); i++) {
-            fname = g_strdup_printf("%s/%s%s", dirs[i], *mp, HOST_DSOSUF);
-            ret = module_load_file(fname);
-            g_free(fname);
-            fname = NULL;
-            /* Try loading until loaded a module file */
-            if (!ret) {
-                break;
-            }
+    for (i = 0; i < ARRAY_SIZE(dirs); i++) {
+        fname = g_strdup_printf("%s/%s%s%s",
+                dirs[i], prefix, lib_name, HOST_DSOSUF);
+        ret = module_load_file(fname);
+        g_free(fname);
+        fname = NULL;
+        /* Try loading until loaded a module file */
+        if (!ret) {
+            break;
         }
     }
 
-- 
2.4.3

[Qemu-devel] [PATCH 2/2] Add dynamic generation of module_block.h

[Marc Marí]

To simplify the addition of new block modules, add a script that generates
include/qemu/module_block.h automatically from the modules' source code.

This script assumes that the QEMU coding style rules are followed.

Signed-off-by: Marc Marí <markmb@redhat.com>
---
 .gitignore                      |   1 +
 Makefile                        |  10 ++-
 scripts/modules/module_block.py | 132 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 140 insertions(+), 3 deletions(-)
 create mode 100755 scripts/modules/module_block.py

diff --git a/.gitignore b/.gitignore
index 61bc492..8a37067 100644
--- a/.gitignore
+++ b/.gitignore
@@ -105,3 +105,4 @@ cscope.*
 tags
 TAGS
 *~
+/include/qemu/module_block.h
diff --git a/Makefile b/Makefile
index 340d9c8..47d593e 100644
--- a/Makefile
+++ b/Makefile
@@ -73,6 +73,8 @@ GENERATED_HEADERS += trace/generated-ust-provider.h
 GENERATED_SOURCES += trace/generated-ust.c
 endif
 
+GENERATED_HEADERS += include/qemu/module_block.h
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -219,9 +221,6 @@ Makefile: $(version-obj-y) $(version-lobj-y)
 libqemustub.a: $(stub-obj-y)
 libqemuutil.a: $(util-obj-y)
 
-block-modules = $(foreach o,$(block-obj-m),"$(basename $(subst /,-,$o))",) NULL
-util/module.o-cflags = -D'CONFIG_BLOCK_MODULES=$(block-modules)'
-
 ######################################################################
 
 qemu-img.o: qemu-img-cmds.h
@@ -313,6 +312,11 @@ msi:
 	@echo MSI build not configured or dependency resolution failed (reconfigure with --enable-guest-agent-msi option)
 endif
 
+include/qemu/module_block.h: $(SRC_PATH)/scripts/modules/module_block.py
+	$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/modules/module_block.py \
+		"./include/qemu/" $(patsubst %.mo,%.c,$(block-obj-m)), \
+		"  GEN   $@")
+
 clean:
 # avoid old build problems by removing potentially incorrect old files
 	rm -f config.mak op-i386.h opc-i386.h gen-op-i386.h op-arm.h opc-arm.h gen-op-arm.h
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
new file mode 100755
index 0000000..a9a9412
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,132 @@
+#!/usr/bin/python
+#
+# Module information generator
+#
+# Copyright Red Hat, Inc. 2015
+#
+# Authors:
+#  Marc Mari <markmb@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.
+# See the COPYING file in the top-level directory.
+
+import sys
+import os
+
+def get_string_struct(line):
+    data = line.split()
+
+    # data[0] -> struct element name
+    # data[1] -> =
+    # data[2] -> value
+
+    return data[2].replace('"', '')[:-1]
+
+def add_module(fhader, library, format_name, protocol_name,
+                probe, probe_device):
+    lines = []
+    lines.append('.library_name = "' + library + '",')
+    if format_name != "":
+        lines.append('.format_name = "' + format_name + '",')
+    if protocol_name != "":
+        lines.append('.protocol_name = "' + protocol_name + '",')
+    if probe:
+        lines.append('.has_probe = true,')
+    if probe_device:
+        lines.append('.has_probe_device = true,')
+
+    text = '\n\t'.join(lines)
+    fheader.write('\n\t{\n\t' + text + '\n\t},')
+
+def process_file(fheader, filename):
+    # This parser assumes the coding style rules are being followed
+    with open(filename, "r") as cfile:
+        found_something = False
+        found_start = False
+        library, _ = os.path.splitext(os.path.basename(filename))
+        for line in cfile:
+            if found_start:
+                line = line.replace('\n', '')
+                if line.find(".format_name") != -1:
+                    format_name = get_string_struct(line)
+                elif line.find(".protocol_name") != -1:
+                    protocol_name = get_string_struct(line)
+                elif line.find(".bdrv_probe") != -1:
+                    probe = True
+                elif line.find(".bdrv_probe_device") != -1:
+                    probe_device = True
+                elif line == "};":
+                    add_module(fheader, library, format_name, protocol_name,
+                                probe, probe_device)
+                    found_start = False
+            elif line.find("static BlockDriver") != -1:
+                found_something = True
+                found_start = True
+                format_name = ""
+                protocol_name = ""
+                probe = False
+                probe_device = False
+
+        if not found_something:
+            print("No BlockDriver struct found in " + filename + ". \
+                    Is this really a module?")
+            sys.exit(1)
+
+def print_top(fheader):
+    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+/*
+ * QEMU Block Module Infrastructure
+ *
+ * Copyright Red Hat, Inc. 2015
+ *
+ * Authors:
+ *  Marc Mari       <markmb@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+''')
+
+    fheader.write('''#ifndef QEMU_MODULE_BLOCK_H
+#define QEMU_MODULE_BLOCK_H
+
+#include "qemu-common.h"
+
+static const struct {
+    const char *format_name;
+    const char *protocol_name;
+    const char *library_name;
+    bool has_probe;
+    bool has_probe_device;
+} block_driver_module[] = {''')
+
+def print_bottom(fheader):
+    fheader.write('''
+};
+
+#endif''')
+
+# First argument: output folder
+# All other arguments: modules source files (.c)
+output_dir = sys.argv[1]
+if not os.path.isdir(output_dir):
+    print("Folder " + output_dir + " does not exist")
+    sys.exit(1)
+
+path = output_dir + 'module_block.h'
+
+with open(path, 'w') as fheader:
+    print_top(fheader)
+
+    for filename in sys.argv[2:]:
+        if os.path.isfile(filename):
+            process_file(fheader, filename)
+        else:
+            print("File " + filename + " does not exist.")
+            sys.exit(1)
+
+    print_bottom(fheader)
+
+sys.exit(0)
-- 
2.4.3

Re: [Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Marc Marí]

Ping

>On Mon, 17 Aug 2015 10:09:33 +0200
>Marc Marí <markmb@redhat.com> wrote:
>
> The current module infrastructure has been improved to enable dynamic
> module loading.
> 
> This reduces the load time for very simple guests. For the following
> configuration (very loaded)
> 
> ./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
>     --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
>     --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
>     --enable-kvm --enable-rdma --enable-uuid --enable-vde \
>     --enable-linux-aio --enable-cap-ng --enable-attr
> --enable-vhost-net \ --enable-vhost-scsi --enable-spice --enable-rbd
> --enable-libiscsi \ --enable-smartcard-nss --enable-guest-agent
> --enable-libusb \ --enable-usb-redir --enable-lzo --enable-snappy
> --enable-bzip2 \ --enable-seccomp --enable-coroutine-pool
> --enable-glusterfs \ --enable-tpm --enable-libssh2 --enable-vhdx
> --enable-numa \ --enable-tcmalloc --target-list=x86_64-softmmu
> 
> With modules disabled, there are 142 libraries loaded at startup.
> Time is the following:
>  LD time: 0.065 seconds
>  QEMU time: 0.02 seconds
>  Total time: 0.085 seconds
> 
> With this patch series and modules enabled, there are 128 libraries
> loaded at startup. Time is the following:
>  LD time: 0.02 seconds
>  QEMU time: 0.02 seconds
>  Total time: 0.04 seconds
> 
> Where LD time is the time between the program startup and the jump to
> main, and QEMU time is the time between the start of main and the
> first kvm_entry.
> 
> These results are just with a few block drivers, that were already a
> module. Adding more modules (block or not block) should be easy, and
> will reduce the load time even more.
> 
> Marc Marí (2):
>   Add dynamic module loading for block drivers
>   Add dynamic generation of module_block.h
> 
>  .gitignore                      |   1 +
>  Makefile                        |  10 ++-
>  block.c                         |  73 +++++++++++++++++++++-
>  configure                       |   2 +-
>  include/qemu/module.h           |   3 +
>  include/qemu/module_block.h     |  89 +++++++++++++++++++++++++++
>  scripts/modules/module_block.py | 132
> ++++++++++++++++++++++++++++++++++++++++
> util/module.c                   |  38 ++++-------- 8 files changed,
> 314 insertions(+), 34 deletions(-) create mode 100644
> include/qemu/module_block.h create mode 100755
> scripts/modules/module_block.py
> 

Re: [Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Daniel P. Berrange]

On Mon, Aug 17, 2015 at 10:09:34AM +0200, Marc Marí wrote:
> Extend the current module interface to allow for block drivers to be loaded
> dynamically on request.
> 
> The only block drivers that can be converted into modules are the drivers
> that don't perform any init operation except for registering themselves. This
> is why libiscsi has been disabled as a module.

Seems like we would just need to split the iscsi opts registration out
into a separate file that is permanently linked.

> All the necessary module information is located in a new structure found in
> include/qemu/module_block.h
> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> ---
>  block.c                     | 73 +++++++++++++++++++++++++++++++++++--
>  configure                   |  2 +-
>  include/qemu/module.h       |  3 ++
>  include/qemu/module_block.h | 89 +++++++++++++++++++++++++++++++++++++++++++++
>  util/module.c               | 38 ++++++-------------
>  5 files changed, 174 insertions(+), 31 deletions(-)
>  create mode 100644 include/qemu/module_block.h
> 
> diff --git a/block.c b/block.c
> index d088ee0..f24a624 100644
> --- a/block.c
> +++ b/block.c
> @@ -27,6 +27,7 @@
>  #include "block/block_int.h"
>  #include "block/blockjob.h"
>  #include "qemu/error-report.h"
> +#include "qemu/module_block.h"
>  #include "qemu/module.h"
>  #include "qapi/qmp/qerror.h"
>  #include "qapi/qmp/qjson.h"
> @@ -277,11 +278,30 @@ void bdrv_add_close_notifier(BlockDriverState *bs, Notifier *notify)
>  BlockDriver *bdrv_find_format(const char *format_name)
>  {
>      BlockDriver *drv1;
> +    int i;

Nit-pick  'size_t' is a better type for loop iterators, especially
when combined with a sizeof() comparison. Some comment in later
functions too.

> +
>      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
>          if (!strcmp(drv1->format_name, format_name)) {
>              return drv1;
>          }
>      }
> +
> +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> +        if (!strcmp(block_driver_module[i].format_name, format_name)) {
> +            block_module_load_one(block_driver_module[i].library_name);
> +            /* Copying code is not nice, but this way the current discovery is
> +             * not modified. Calling recursively could fail if the library
> +             * has been deleted.
> +             */

Can you explaiin what you mean here about "if the library has been deleted" ?

Are you referring to possibilty of dlclose()ing the previously loaded
library, or about possibility of the module on disk having been deleted
or something else ?

> @@ -483,9 +503,15 @@ int get_tmp_filename(char *filename, int size)
>   */
>  static BlockDriver *find_hdev_driver(const char *filename)
>  {
> -    int score_max = 0, score;
> +    int score_max = 0, score, i;
>      BlockDriver *drv = NULL, *d;
>  
> +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> +        if (block_driver_module[i].has_probe_device) {
> +            block_module_load_one(block_driver_module[i].library_name);
> +        }
> +    }

If we have multiuple disks of the same type given to QEMU, it
seems like we might end up calling block_module_load_one()
multiple times for the same module & end up loading the same
.so multiple times as a result. Should module_load() keep a
record of everything it has loaded and short-circuit itself
to a no-op, so that callers of module_load() don't have to
worry about avoiding multiple calls.

> +
>      QLIST_FOREACH(d, &bdrv_drivers, list) {
>          if (d->bdrv_probe_device) {
>              score = d->bdrv_probe_device(filename);
> @@ -507,6 +533,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
>      char protocol[128];
>      int len;
>      const char *p;
> +    int i;
>  
>      /* TODO Drivers without bdrv_file_open must be specified explicitly */
>  
> @@ -533,6 +560,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
>          len = sizeof(protocol) - 1;
>      memcpy(protocol, filename, len);
>      protocol[len] = '\0';
> +
>      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
>          if (drv1->protocol_name &&
>              !strcmp(drv1->protocol_name, protocol)) {
> @@ -540,6 +568,23 @@ BlockDriver *bdrv_find_protocol(const char *filename,
>          }
>      }
>  
> +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> +        if (block_driver_module[i].protocol_name &&
> +            !strcmp(block_driver_module[i].protocol_name, protocol)) {
> +            block_module_load_one(block_driver_module[i].library_name);
> +            /* Copying code is not nice, but this way the current discovery is
> +             * not modified. Calling recursively could fail if the library
> +             * has been deleted.
> +             */
> +            QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> +                if (drv1->protocol_name &&
> +                    !strcmp(drv1->protocol_name, protocol)) {
> +                    return drv1;
> +                }
> +            }
> +        }
> +    }
> +
>      error_setg(errp, "Unknown protocol '%s'", protocol);
>      return NULL;
>  }
> @@ -561,9 +606,15 @@ BlockDriver *bdrv_find_protocol(const char *filename,
>  BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
>                              const char *filename)
>  {
> -    int score_max = 0, score;
> +    int score_max = 0, score, i;
>      BlockDriver *drv = NULL, *d;
>  
> +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> +        if (block_driver_module[i].has_probe) {
> +            block_module_load_one(block_driver_module[i].library_name);
> +        }
> +    }
> +
>      QLIST_FOREACH(d, &bdrv_drivers, list) {
>          if (d->bdrv_probe) {
>              score = d->bdrv_probe(buf, buf_size, filename);
> @@ -2783,7 +2834,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
>  {
>      BlockDriver *drv;
>      int count = 0;
> -    int i;
> +    int i, n;
>      const char **formats = NULL;
>  
>      QLIST_FOREACH(drv, &bdrv_drivers, list) {
> @@ -2801,6 +2852,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
>          }
>      }
>  
> +    for (n = 0; n < ARRAY_SIZE(block_driver_module); ++n) {
> +        if (block_driver_module[n].format_name) {
> +            bool found = false;
> +            int i = count;
> +            while (formats && i && !found) {
> +                found = !strcmp(formats[--i],
> +                                block_driver_module[n].format_name);
> +            }
> +
> +            if (!found) {
> +                formats = g_renew(const char *, formats, count + 1);
> +                formats[count++] = block_driver_module[n].format_name;
> +            }
> +        }
> +    }
> +
>      qsort(formats, count, sizeof(formats[0]), qsort_strcmp);
>  
>      for (i = 0; i < count; i++) {
> diff --git a/configure b/configure
> index cd219d8..9fca9ee 100755
> --- a/configure
> +++ b/configure
> @@ -4971,7 +4971,7 @@ if test "$bzip2" = "yes" ; then
>  fi
>  
>  if test "$libiscsi" = "yes" ; then
> -  echo "CONFIG_LIBISCSI=m" >> $config_host_mak
> +  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
>    echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
>    echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak
>  fi
> diff --git a/include/qemu/module.h b/include/qemu/module.h
> index 72d9498..0ad4bb7 100644
> --- a/include/qemu/module.h
> +++ b/include/qemu/module.h
> @@ -53,9 +53,12 @@ typedef enum {
>  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
>  #define type_init(function) module_init(function, MODULE_INIT_QOM)
>  
> +#define block_module_load_one(lib) module_load_one("block-", lib);
> +
>  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);
> +void module_load_one(const char *prefix, const char *lib_name);
>  
>  #endif
> diff --git a/include/qemu/module_block.h b/include/qemu/module_block.h
> new file mode 100644
> index 0000000..f1d389c
> --- /dev/null
> +++ b/include/qemu/module_block.h
> @@ -0,0 +1,88 @@
> +/*
> + * QEMU Block Module Infrastructure
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  Marc Mari       <markmb@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */

[snip]

> +
> +#endif
> \ No newline at end of file

Missing newline at end of file :-)

> diff --git a/util/module.c b/util/module.c
> index 4bd4a94..992d317 100644
> --- a/util/module.c
> +++ b/util/module.c
> @@ -91,14 +91,11 @@ void register_dso_module_init(void (*fn)(void), module_init_type type)
>      QTAILQ_INSERT_TAIL(&dso_init_list, e, node);
>  }
>  
> -static void module_load(module_init_type type);
> -
>  void module_call_init(module_init_type type)
>  {
>      ModuleTypeList *l;
>      ModuleEntry *e;
>  
> -    module_load(type);
>      l = find_type(type);
>  
>      QTAILQ_FOREACH(e, l, node) {

Isn't the entire of module_call_init() a no-op now that you removed
the module_load() call. IIUC, the find_type() method should return
an empty list at that point, so the rest of the method doesn't
nothing. IOW, I would think module_call_init() and its supporting
functions can be deleted now.

> @@ -149,6 +146,7 @@ static int module_load_file(const char *fname)
>          ret = -EINVAL;
>      } else {
>          QTAILQ_FOREACH(e, &dso_init_list, node) {
> +            e->init();
>              register_module_init(e->init, e->type);
>          }
>          ret = 0;
> @@ -163,14 +161,10 @@ out:
>  }
>  #endif
>  
> -static void module_load(module_init_type type)
> +void module_load_one(const char *prefix, const char *lib_name)
>  {
>  #ifdef CONFIG_MODULES
>      char *fname = NULL;
> -    const char **mp;
> -    static const char *block_modules[] = {
> -        CONFIG_BLOCK_MODULES
> -    };
>      char *exec_dir;
>      char *dirs[3];
>      int i = 0;
> @@ -181,15 +175,6 @@ static void module_load(module_init_type type)
>          return;
>      }
>  
> -    switch (type) {
> -    case MODULE_INIT_BLOCK:
> -        mp = block_modules;
> -        break;
> -    default:
> -        /* no other types have dynamic modules for now*/
> -        return;
> -    }
> -
>      exec_dir = qemu_get_exec_dir();
>      dirs[i++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR);
>      dirs[i++] = g_strdup_printf("%s/..", exec_dir ? : "");
> @@ -198,16 +183,15 @@ static void module_load(module_init_type type)
>      g_free(exec_dir);
>      exec_dir = NULL;
>  
> -    for ( ; *mp; mp++) {
> -        for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> -            fname = g_strdup_printf("%s/%s%s", dirs[i], *mp, HOST_DSOSUF);
> -            ret = module_load_file(fname);
> -            g_free(fname);
> -            fname = NULL;
> -            /* Try loading until loaded a module file */
> -            if (!ret) {
> -                break;
> -            }
> +    for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> +        fname = g_strdup_printf("%s/%s%s%s",
> +                dirs[i], prefix, lib_name, HOST_DSOSUF);
> +        ret = module_load_file(fname);
> +        g_free(fname);
> +        fname = NULL;
> +        /* Try loading until loaded a module file */
> +        if (!ret) {
> +            break;
>          }
>      }
>  
> -- 
> 2.4.3
> 
> 

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 2/2] Add dynamic generation of module_block.h

[Daniel P. Berrange]

On Mon, Aug 17, 2015 at 10:09:35AM +0200, Marc Marí wrote:
> To simplify the addition of new block modules, add a script that generates
> include/qemu/module_block.h automatically from the modules' source code.
> 
> This script assumes that the QEMU coding style rules are followed.
> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> ---
>  .gitignore                      |   1 +
>  Makefile                        |  10 ++-
>  scripts/modules/module_block.py | 132 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 140 insertions(+), 3 deletions(-)
>  create mode 100755 scripts/modules/module_block.py

I'd expect to see module_block.h deleted in this commit, otherwise
you're re-generating a file stored in git each time someone runs
make.

> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
> new file mode 100755
> index 0000000..a9a9412
> --- /dev/null
> +++ b/scripts/modules/module_block.py
> @@ -0,0 +1,132 @@
> +#!/usr/bin/python
> +#
> +# Module information generator
> +#
> +# Copyright Red Hat, Inc. 2015
> +#
> +# Authors:
> +#  Marc Mari <markmb@redhat.com>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.
> +# See the COPYING file in the top-level directory.
> +
> +import sys
> +import os
> +
> +def get_string_struct(line):
> +    data = line.split()
> +
> +    # data[0] -> struct element name
> +    # data[1] -> =
> +    # data[2] -> value
> +
> +    return data[2].replace('"', '')[:-1]
> +
> +def add_module(fhader, library, format_name, protocol_name,
> +                probe, probe_device):
> +    lines = []
> +    lines.append('.library_name = "' + library + '",')
> +    if format_name != "":
> +        lines.append('.format_name = "' + format_name + '",')
> +    if protocol_name != "":
> +        lines.append('.protocol_name = "' + protocol_name + '",')
> +    if probe:
> +        lines.append('.has_probe = true,')
> +    if probe_device:
> +        lines.append('.has_probe_device = true,')
> +
> +    text = '\n\t'.join(lines)
> +    fheader.write('\n\t{\n\t' + text + '\n\t},')
> +
> +def process_file(fheader, filename):
> +    # This parser assumes the coding style rules are being followed
> +    with open(filename, "r") as cfile:
> +        found_something = False
> +        found_start = False
> +        library, _ = os.path.splitext(os.path.basename(filename))
> +        for line in cfile:
> +            if found_start:
> +                line = line.replace('\n', '')
> +                if line.find(".format_name") != -1:
> +                    format_name = get_string_struct(line)
> +                elif line.find(".protocol_name") != -1:
> +                    protocol_name = get_string_struct(line)
> +                elif line.find(".bdrv_probe") != -1:
> +                    probe = True
> +                elif line.find(".bdrv_probe_device") != -1:
> +                    probe_device = True
> +                elif line == "};":
> +                    add_module(fheader, library, format_name, protocol_name,
> +                                probe, probe_device)
> +                    found_start = False
> +            elif line.find("static BlockDriver") != -1:
> +                found_something = True
> +                found_start = True
> +                format_name = ""
> +                protocol_name = ""
> +                probe = False
> +                probe_device = False
> +
> +        if not found_something:
> +            print("No BlockDriver struct found in " + filename + ". \
> +                    Is this really a module?")
> +            sys.exit(1)


Errors ought to go to sys.stderr rather than stdout.



> +
> +def print_top(fheader):
> +    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> +/*
> + * QEMU Block Module Infrastructure
> + *
> + * Copyright Red Hat, Inc. 2015
> + *
> + * Authors:
> + *  Marc Mari       <markmb@redhat.com>

When the file is auto-generated, I'm not sure it is right to claim
copyright / authorship on it - if anything the copyright comes from
the files that you're using as the source for auto-generation.

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + */

> +# First argument: output folder
> +# All other arguments: modules source files (.c)
> +output_dir = sys.argv[1]
> +if not os.path.isdir(output_dir):
> +    print("Folder " + output_dir + " does not exist")

Again about stderr

> +    sys.exit(1)
> +
> +path = output_dir + 'module_block.h'
> +
> +with open(path, 'w') as fheader:
> +    print_top(fheader)
> +
> +    for filename in sys.argv[2:]:
> +        if os.path.isfile(filename):
> +            process_file(fheader, filename)
> +        else:
> +            print("File " + filename + " does not exist.")

Again here.

> +            sys.exit(1)
> +
> +    print_bottom(fheader)
> +
> +sys.exit(0)

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Marc Marí]

On Thu, 27 Aug 2015 10:19:35 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Aug 17, 2015 at 10:09:34AM +0200, Marc Marí wrote:
> > Extend the current module interface to allow for block drivers to
> > be loaded dynamically on request.
> > 
> > The only block drivers that can be converted into modules are the
> > drivers that don't perform any init operation except for
> > registering themselves. This is why libiscsi has been disabled as a
> > module.
> 
> Seems like we would just need to split the iscsi opts registration out
> into a separate file that is permanently linked.
> 
> > All the necessary module information is located in a new structure
> > found in include/qemu/module_block.h
> > 
> > Signed-off-by: Marc Marí <markmb@redhat.com>
> > ---
> >  block.c                     | 73
> > +++++++++++++++++++++++++++++++++++-- configure
> > |  2 +- include/qemu/module.h       |  3 ++
> >  include/qemu/module_block.h | 89
> > +++++++++++++++++++++++++++++++++++++++++++++
> > util/module.c               | 38 ++++++------------- 5 files
> > changed, 174 insertions(+), 31 deletions(-) create mode 100644
> > include/qemu/module_block.h
> > 
> > diff --git a/block.c b/block.c
> > index d088ee0..f24a624 100644
> > --- a/block.c
> > +++ b/block.c
> > @@ -27,6 +27,7 @@
> >  #include "block/block_int.h"
> >  #include "block/blockjob.h"
> >  #include "qemu/error-report.h"
> > +#include "qemu/module_block.h"
> >  #include "qemu/module.h"
> >  #include "qapi/qmp/qerror.h"
> >  #include "qapi/qmp/qjson.h"
> > @@ -277,11 +278,30 @@ void bdrv_add_close_notifier(BlockDriverState
> > *bs, Notifier *notify) BlockDriver *bdrv_find_format(const char
> > *format_name) {
> >      BlockDriver *drv1;
> > +    int i;
> 
> Nit-pick  'size_t' is a better type for loop iterators, especially
> when combined with a sizeof() comparison. Some comment in later
> functions too.
> 
> > +
> >      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> >          if (!strcmp(drv1->format_name, format_name)) {
> >              return drv1;
> >          }
> >      }
> > +
> > +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> > +        if (!strcmp(block_driver_module[i].format_name,
> > format_name)) {
> > +
> > block_module_load_one(block_driver_module[i].library_name);
> > +            /* Copying code is not nice, but this way the current
> > discovery is
> > +             * not modified. Calling recursively could fail if the
> > library
> > +             * has been deleted.
> > +             */
> 
> Can you explaiin what you mean here about "if the library has been
> deleted" ?
> 
> Are you referring to possibilty of dlclose()ing the previously loaded
> library, or about possibility of the module on disk having been
> deleted or something else ?

I was thinking of relaunching the search by calling recursively the
function. But this would loop infinitely if somebody, manually, deleted
the library file.

> > @@ -483,9 +503,15 @@ int get_tmp_filename(char *filename, int size)
> >   */
> >  static BlockDriver *find_hdev_driver(const char *filename)
> >  {
> > -    int score_max = 0, score;
> > +    int score_max = 0, score, i;
> >      BlockDriver *drv = NULL, *d;
> >  
> > +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> > +        if (block_driver_module[i].has_probe_device) {
> > +
> > block_module_load_one(block_driver_module[i].library_name);
> > +        }
> > +    }
> 
> If we have multiuple disks of the same type given to QEMU, it
> seems like we might end up calling block_module_load_one()
> multiple times for the same module & end up loading the same
> .so multiple times as a result. Should module_load() keep a
> record of everything it has loaded and short-circuit itself
> to a no-op, so that callers of module_load() don't have to
> worry about avoiding multiple calls.
 
I avoided that because glib already has it. It just increments the
reference count. Which is not important unless we want to dlclose it.

https://developer.gnome.org/glib/stable/glib-Dynamic-Loading-of-Modules.html#g-module-open

> > +
> >      QLIST_FOREACH(d, &bdrv_drivers, list) {
> >          if (d->bdrv_probe_device) {
> >              score = d->bdrv_probe_device(filename);
> > @@ -507,6 +533,7 @@ BlockDriver *bdrv_find_protocol(const char
> > *filename, char protocol[128];
> >      int len;
> >      const char *p;
> > +    int i;
> >  
> >      /* TODO Drivers without bdrv_file_open must be specified
> > explicitly */ 
> > @@ -533,6 +560,7 @@ BlockDriver *bdrv_find_protocol(const char
> > *filename, len = sizeof(protocol) - 1;
> >      memcpy(protocol, filename, len);
> >      protocol[len] = '\0';
> > +
> >      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> >          if (drv1->protocol_name &&
> >              !strcmp(drv1->protocol_name, protocol)) {
> > @@ -540,6 +568,23 @@ BlockDriver *bdrv_find_protocol(const char
> > *filename, }
> >      }
> >  
> > +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> > +        if (block_driver_module[i].protocol_name &&
> > +            !strcmp(block_driver_module[i].protocol_name,
> > protocol)) {
> > +
> > block_module_load_one(block_driver_module[i].library_name);
> > +            /* Copying code is not nice, but this way the current
> > discovery is
> > +             * not modified. Calling recursively could fail if the
> > library
> > +             * has been deleted.
> > +             */
> > +            QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> > +                if (drv1->protocol_name &&
> > +                    !strcmp(drv1->protocol_name, protocol)) {
> > +                    return drv1;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> >      error_setg(errp, "Unknown protocol '%s'", protocol);
> >      return NULL;
> >  }
> > @@ -561,9 +606,15 @@ BlockDriver *bdrv_find_protocol(const char
> > *filename, BlockDriver *bdrv_probe_all(const uint8_t *buf, int
> > buf_size, const char *filename)
> >  {
> > -    int score_max = 0, score;
> > +    int score_max = 0, score, i;
> >      BlockDriver *drv = NULL, *d;
> >  
> > +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> > +        if (block_driver_module[i].has_probe) {
> > +
> > block_module_load_one(block_driver_module[i].library_name);
> > +        }
> > +    }
> > +
> >      QLIST_FOREACH(d, &bdrv_drivers, list) {
> >          if (d->bdrv_probe) {
> >              score = d->bdrv_probe(buf, buf_size, filename);
> > @@ -2783,7 +2834,7 @@ void bdrv_iterate_format(void (*it)(void
> > *opaque, const char *name), {
> >      BlockDriver *drv;
> >      int count = 0;
> > -    int i;
> > +    int i, n;
> >      const char **formats = NULL;
> >  
> >      QLIST_FOREACH(drv, &bdrv_drivers, list) {
> > @@ -2801,6 +2852,22 @@ void bdrv_iterate_format(void (*it)(void
> > *opaque, const char *name), }
> >      }
> >  
> > +    for (n = 0; n < ARRAY_SIZE(block_driver_module); ++n) {
> > +        if (block_driver_module[n].format_name) {
> > +            bool found = false;
> > +            int i = count;
> > +            while (formats && i && !found) {
> > +                found = !strcmp(formats[--i],
> > +
> > block_driver_module[n].format_name);
> > +            }
> > +
> > +            if (!found) {
> > +                formats = g_renew(const char *, formats, count +
> > 1);
> > +                formats[count++] =
> > block_driver_module[n].format_name;
> > +            }
> > +        }
> > +    }
> > +
> >      qsort(formats, count, sizeof(formats[0]), qsort_strcmp);
> >  
> >      for (i = 0; i < count; i++) {
> > diff --git a/configure b/configure
> > index cd219d8..9fca9ee 100755
> > --- a/configure
> > +++ b/configure
> > @@ -4971,7 +4971,7 @@ if test "$bzip2" = "yes" ; then
> >  fi
> >  
> >  if test "$libiscsi" = "yes" ; then
> > -  echo "CONFIG_LIBISCSI=m" >> $config_host_mak
> > +  echo "CONFIG_LIBISCSI=y" >> $config_host_mak
> >    echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak
> >    echo "LIBISCSI_LIBS=$libiscsi_libs" >> $config_host_mak
> >  fi
> > diff --git a/include/qemu/module.h b/include/qemu/module.h
> > index 72d9498..0ad4bb7 100644
> > --- a/include/qemu/module.h
> > +++ b/include/qemu/module.h
> > @@ -53,9 +53,12 @@ typedef enum {
> >  #define qapi_init(function) module_init(function, MODULE_INIT_QAPI)
> >  #define type_init(function) module_init(function, MODULE_INIT_QOM)
> >  
> > +#define block_module_load_one(lib) module_load_one("block-", lib);
> > +
> >  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);
> > +void module_load_one(const char *prefix, const char *lib_name);
> >  
> >  #endif
> > diff --git a/include/qemu/module_block.h
> > b/include/qemu/module_block.h new file mode 100644
> > index 0000000..f1d389c
> > --- /dev/null
> > +++ b/include/qemu/module_block.h
> > @@ -0,0 +1,88 @@
> > +/*
> > + * QEMU Block Module Infrastructure
> > + *
> > + * Copyright Red Hat, Inc. 2015
> > + *
> > + * Authors:
> > + *  Marc Mari       <markmb@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version
> > 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> 
> [snip]
> 
> > +
> > +#endif
> > \ No newline at end of file
> 
> Missing newline at end of file :-)

Oops :). Anyway, this is deleted in the other patch :)

> > diff --git a/util/module.c b/util/module.c
> > index 4bd4a94..992d317 100644
> > --- a/util/module.c
> > +++ b/util/module.c
> > @@ -91,14 +91,11 @@ void register_dso_module_init(void (*fn)(void),
> > module_init_type type) QTAILQ_INSERT_TAIL(&dso_init_list, e, node);
> >  }
> >  
> > -static void module_load(module_init_type type);
> > -
> >  void module_call_init(module_init_type type)
> >  {
> >      ModuleTypeList *l;
> >      ModuleEntry *e;
> >  
> > -    module_load(type);
> >      l = find_type(type);
> >  
> >      QTAILQ_FOREACH(e, l, node) {
> 
> Isn't the entire of module_call_init() a no-op now that you removed
> the module_load() call. IIUC, the find_type() method should return
> an empty list at that point, so the rest of the method doesn't
> nothing. IOW, I would think module_call_init() and its supporting
> functions can be deleted now. 

The module interface is no just for dynamic library loading but for
loading all the devices into the system. Devices that are inside the
monolithic QEMU still use this interface. So, it cannot be removed.

Thanks
Marc

> > @@ -149,6 +146,7 @@ static int module_load_file(const char *fname)
> >          ret = -EINVAL;
> >      } else {
> >          QTAILQ_FOREACH(e, &dso_init_list, node) {
> > +            e->init();
> >              register_module_init(e->init, e->type);
> >          }
> >          ret = 0;
> > @@ -163,14 +161,10 @@ out:
> >  }
> >  #endif
> >  
> > -static void module_load(module_init_type type)
> > +void module_load_one(const char *prefix, const char *lib_name)
> >  {
> >  #ifdef CONFIG_MODULES
> >      char *fname = NULL;
> > -    const char **mp;
> > -    static const char *block_modules[] = {
> > -        CONFIG_BLOCK_MODULES
> > -    };
> >      char *exec_dir;
> >      char *dirs[3];
> >      int i = 0;
> > @@ -181,15 +175,6 @@ static void module_load(module_init_type type)
> >          return;
> >      }
> >  
> > -    switch (type) {
> > -    case MODULE_INIT_BLOCK:
> > -        mp = block_modules;
> > -        break;
> > -    default:
> > -        /* no other types have dynamic modules for now*/
> > -        return;
> > -    }
> > -
> >      exec_dir = qemu_get_exec_dir();
> >      dirs[i++] = g_strdup_printf("%s", CONFIG_QEMU_MODDIR);
> >      dirs[i++] = g_strdup_printf("%s/..", exec_dir ? : "");
> > @@ -198,16 +183,15 @@ static void module_load(module_init_type type)
> >      g_free(exec_dir);
> >      exec_dir = NULL;
> >  
> > -    for ( ; *mp; mp++) {
> > -        for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> > -            fname = g_strdup_printf("%s/%s%s", dirs[i], *mp,
> > HOST_DSOSUF);
> > -            ret = module_load_file(fname);
> > -            g_free(fname);
> > -            fname = NULL;
> > -            /* Try loading until loaded a module file */
> > -            if (!ret) {
> > -                break;
> > -            }
> > +    for (i = 0; i < ARRAY_SIZE(dirs); i++) {
> > +        fname = g_strdup_printf("%s/%s%s%s",
> > +                dirs[i], prefix, lib_name, HOST_DSOSUF);
> > +        ret = module_load_file(fname);
> > +        g_free(fname);
> > +        fname = NULL;
> > +        /* Try loading until loaded a module file */
> > +        if (!ret) {
> > +            break;
> >          }
> >      }
> >  
> > -- 
> > 2.4.3
> > 
> > 
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 2/2] Add dynamic generation of module_block.h

[Marc Marí]

On Thu, 27 Aug 2015 10:23:32 +0100
"Daniel P. Berrange" <berrange@redhat.com> wrote:

> On Mon, Aug 17, 2015 at 10:09:35AM +0200, Marc Marí wrote:
> > To simplify the addition of new block modules, add a script that
> > generates include/qemu/module_block.h automatically from the
> > modules' source code.
> > 
> > This script assumes that the QEMU coding style rules are followed.
> > 
> > Signed-off-by: Marc Marí <markmb@redhat.com>
> > ---
> >  .gitignore                      |   1 +
> >  Makefile                        |  10 ++-
> >  scripts/modules/module_block.py | 132
> > ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140
> > insertions(+), 3 deletions(-) create mode 100755
> > scripts/modules/module_block.py
> 
> I'd expect to see module_block.h deleted in this commit, otherwise
> you're re-generating a file stored in git each time someone runs
> make.
> 
> > diff --git a/scripts/modules/module_block.py
> > b/scripts/modules/module_block.py new file mode 100755
> > index 0000000..a9a9412
> > --- /dev/null
> > +++ b/scripts/modules/module_block.py
> > @@ -0,0 +1,132 @@
> > +#!/usr/bin/python
> > +#
> > +# Module information generator
> > +#
> > +# Copyright Red Hat, Inc. 2015
> > +#
> > +# Authors:
> > +#  Marc Mari <markmb@redhat.com>
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2.
> > +# See the COPYING file in the top-level directory.
> > +
> > +import sys
> > +import os
> > +
> > +def get_string_struct(line):
> > +    data = line.split()
> > +
> > +    # data[0] -> struct element name
> > +    # data[1] -> =
> > +    # data[2] -> value
> > +
> > +    return data[2].replace('"', '')[:-1]
> > +
> > +def add_module(fhader, library, format_name, protocol_name,
> > +                probe, probe_device):
> > +    lines = []
> > +    lines.append('.library_name = "' + library + '",')
> > +    if format_name != "":
> > +        lines.append('.format_name = "' + format_name + '",')
> > +    if protocol_name != "":
> > +        lines.append('.protocol_name = "' + protocol_name + '",')
> > +    if probe:
> > +        lines.append('.has_probe = true,')
> > +    if probe_device:
> > +        lines.append('.has_probe_device = true,')
> > +
> > +    text = '\n\t'.join(lines)
> > +    fheader.write('\n\t{\n\t' + text + '\n\t},')
> > +
> > +def process_file(fheader, filename):
> > +    # This parser assumes the coding style rules are being followed
> > +    with open(filename, "r") as cfile:
> > +        found_something = False
> > +        found_start = False
> > +        library, _ = os.path.splitext(os.path.basename(filename))
> > +        for line in cfile:
> > +            if found_start:
> > +                line = line.replace('\n', '')
> > +                if line.find(".format_name") != -1:
> > +                    format_name = get_string_struct(line)
> > +                elif line.find(".protocol_name") != -1:
> > +                    protocol_name = get_string_struct(line)
> > +                elif line.find(".bdrv_probe") != -1:
> > +                    probe = True
> > +                elif line.find(".bdrv_probe_device") != -1:
> > +                    probe_device = True
> > +                elif line == "};":
> > +                    add_module(fheader, library, format_name,
> > protocol_name,
> > +                                probe, probe_device)
> > +                    found_start = False
> > +            elif line.find("static BlockDriver") != -1:
> > +                found_something = True
> > +                found_start = True
> > +                format_name = ""
> > +                protocol_name = ""
> > +                probe = False
> > +                probe_device = False
> > +
> > +        if not found_something:
> > +            print("No BlockDriver struct found in " + filename +
> > ". \
> > +                    Is this really a module?")
> > +            sys.exit(1)
> 
> 
> Errors ought to go to sys.stderr rather than stdout.
> 
> 
> 
> > +
> > +def print_top(fheader):
> > +    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
> > +/*
> > + * QEMU Block Module Infrastructure
> > + *
> > + * Copyright Red Hat, Inc. 2015
> > + *
> > + * Authors:
> > + *  Marc Mari       <markmb@redhat.com>
> 
> When the file is auto-generated, I'm not sure it is right to claim
> copyright / authorship on it - if anything the copyright comes from
> the files that you're using as the source for auto-generation.

I just looked at other autogenerated files, and copied the template.

Thanks
Marc

> > + *
> > + * This work is licensed under the terms of the GNU GPL, version
> > 2.  See
> > + * the COPYING file in the top-level directory.
> > + *
> > + */
> 
> > +# First argument: output folder
> > +# All other arguments: modules source files (.c)
> > +output_dir = sys.argv[1]
> > +if not os.path.isdir(output_dir):
> > +    print("Folder " + output_dir + " does not exist")
> 
> Again about stderr
> 
> > +    sys.exit(1)
> > +
> > +path = output_dir + 'module_block.h'
> > +
> > +with open(path, 'w') as fheader:
> > +    print_top(fheader)
> > +
> > +    for filename in sys.argv[2:]:
> > +        if os.path.isfile(filename):
> > +            process_file(fheader, filename)
> > +        else:
> > +            print("File " + filename + " does not exist.")
> 
> Again here.
> 
> > +            sys.exit(1)
> > +
> > +    print_bottom(fheader)
> > +
> > +sys.exit(0)
> 
> Regards,
> Daniel

Re: [Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Daniel P. Berrange]

On Thu, Aug 27, 2015 at 11:35:41AM +0200, Marc Marí wrote:
> On Thu, 27 Aug 2015 10:19:35 +0100
> "Daniel P. Berrange" <berrange@redhat.com> wrote:
> 
> > On Mon, Aug 17, 2015 at 10:09:34AM +0200, Marc Marí wrote:
> > > Extend the current module interface to allow for block drivers to
> > > be loaded dynamically on request.
> > > 
> > > The only block drivers that can be converted into modules are the
> > > drivers that don't perform any init operation except for
> > > registering themselves. This is why libiscsi has been disabled as a
> > > module.
> > 
> > Seems like we would just need to split the iscsi opts registration out
> > into a separate file that is permanently linked.
> > 
> > > All the necessary module information is located in a new structure
> > > found in include/qemu/module_block.h
> > > 
> > > Signed-off-by: Marc Marí <markmb@redhat.com>
> > > ---
> > >  block.c                     | 73
> > > +++++++++++++++++++++++++++++++++++-- configure
> > > |  2 +- include/qemu/module.h       |  3 ++
> > >  include/qemu/module_block.h | 89
> > > +++++++++++++++++++++++++++++++++++++++++++++
> > > util/module.c               | 38 ++++++------------- 5 files
> > > changed, 174 insertions(+), 31 deletions(-) create mode 100644
> > > include/qemu/module_block.h
> > > 
> > > diff --git a/block.c b/block.c
> > > index d088ee0..f24a624 100644
> > > --- a/block.c
> > > +++ b/block.c
> > > @@ -27,6 +27,7 @@
> > >  #include "block/block_int.h"
> > >  #include "block/blockjob.h"
> > >  #include "qemu/error-report.h"
> > > +#include "qemu/module_block.h"
> > >  #include "qemu/module.h"
> > >  #include "qapi/qmp/qerror.h"
> > >  #include "qapi/qmp/qjson.h"
> > > @@ -277,11 +278,30 @@ void bdrv_add_close_notifier(BlockDriverState
> > > *bs, Notifier *notify) BlockDriver *bdrv_find_format(const char
> > > *format_name) {
> > >      BlockDriver *drv1;
> > > +    int i;
> > 
> > Nit-pick  'size_t' is a better type for loop iterators, especially
> > when combined with a sizeof() comparison. Some comment in later
> > functions too.
> > 
> > > +
> > >      QLIST_FOREACH(drv1, &bdrv_drivers, list) {
> > >          if (!strcmp(drv1->format_name, format_name)) {
> > >              return drv1;
> > >          }
> > >      }
> > > +
> > > +    for (i = 0; i < ARRAY_SIZE(block_driver_module); ++i) {
> > > +        if (!strcmp(block_driver_module[i].format_name,
> > > format_name)) {
> > > +
> > > block_module_load_one(block_driver_module[i].library_name);
> > > +            /* Copying code is not nice, but this way the current
> > > discovery is
> > > +             * not modified. Calling recursively could fail if the
> > > library
> > > +             * has been deleted.
> > > +             */
> > 
> > Can you explaiin what you mean here about "if the library has been
> > deleted" ?
> > 
> > Are you referring to possibilty of dlclose()ing the previously loaded
> > library, or about possibility of the module on disk having been
> > deleted or something else ?
> 
> I was thinking of relaunching the search by calling recursively the
> function. But this would loop infinitely if somebody, manually, deleted
> the library file.

Ok, yea, that makes sense.

 > If we have multiuple disks of the same type given to QEMU, it
> > seems like we might end up calling block_module_load_one()
> > multiple times for the same module & end up loading the same
> > .so multiple times as a result. Should module_load() keep a
> > record of everything it has loaded and short-circuit itself
> > to a no-op, so that callers of module_load() don't have to
> > worry about avoiding multiple calls.
>  
> I avoided that because glib already has it. It just increments the
> reference count. Which is not important unless we want to dlclose it.
> 
> https://developer.gnome.org/glib/stable/glib-Dynamic-Loading-of-Modules.html#g-module-open

Ah good.

NB, it is almost never safe to use dlclose() in a multi-threaded
application. A module may have created a thread-local variable
with a destructor function registered. There's no way to clean
these up prior to dlclose(), so the app would crash if you
dlclose() and a thread then exits. Since it is impractical to
audit all linked library code for use of thread locals, it is
safest to just avoid any use of dlclose().

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Stefan Hajnoczi]

On Mon, Aug 17, 2015 at 10:09:33AM +0200, Marc Marí wrote:
> The current module infrastructure has been improved to enable dynamic module
> loading.
> 
> This reduces the load time for very simple guests. For the following
> configuration (very loaded)
> 
> ./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
>     --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
>     --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
>     --enable-kvm --enable-rdma --enable-uuid --enable-vde \
>     --enable-linux-aio --enable-cap-ng --enable-attr --enable-vhost-net \
>     --enable-vhost-scsi --enable-spice --enable-rbd --enable-libiscsi \
>     --enable-smartcard-nss --enable-guest-agent --enable-libusb \
>     --enable-usb-redir --enable-lzo --enable-snappy --enable-bzip2 \
>     --enable-seccomp --enable-coroutine-pool --enable-glusterfs \
>     --enable-tpm --enable-libssh2 --enable-vhdx --enable-numa \
>     --enable-tcmalloc --target-list=x86_64-softmmu
> 
> With modules disabled, there are 142 libraries loaded at startup. Time is
> the following:
>  LD time: 0.065 seconds
>  QEMU time: 0.02 seconds
>  Total time: 0.085 seconds
> 
> With this patch series and modules enabled, there are 128 libraries loaded
> at startup. Time is the following:
>  LD time: 0.02 seconds
>  QEMU time: 0.02 seconds
>  Total time: 0.04 seconds
> 
> Where LD time is the time between the program startup and the jump to main,
> and QEMU time is the time between the start of main and the first kvm_entry.
> 
> These results are just with a few block drivers, that were already a module.
> Adding more modules (block or not block) should be easy, and will reduce
> the load time even more.

Which QEMU command-line did you benchmark?

Did you have a UI enabled (SDL/GTK/VNC)?

Re: [Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Stefan Hajnoczi]

On Mon, Aug 17, 2015 at 10:09:34AM +0200, Marc Marí wrote:
> +static const struct {
> +    const char *format_name;
> +    const char *protocol_name;
> +    const char *library_name;
> +    bool has_probe;
> +    bool has_probe_device;
> +} block_driver_module[] = {

Why is this list incomplete?  It doesn't cover all block drivers.
Perhaps these are the only modular block drivers.

Also, it ignores CONFIG_CURL and friends.  Perhaps it doesn't matter
because the module loading code will just see that there is no file
there, but maybe conditional compilation should be used?

A plural name would more consistent (i.e. you deleted the
plural block_modules[] variable and introduced a singular
block_driver_module[] variable).

Re: [Qemu-devel] [PATCH 1/2] Add dynamic module loading for block drivers

[Marc Marí]

On Thu, 3 Sep 2015 17:33:16 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Mon, Aug 17, 2015 at 10:09:34AM +0200, Marc Marí wrote:
> > +static const struct {
> > +    const char *format_name;
> > +    const char *protocol_name;
> > +    const char *library_name;
> > +    bool has_probe;
> > +    bool has_probe_device;
> > +} block_driver_module[] = {
> 
> Why is this list incomplete?  It doesn't cover all block drivers.
> Perhaps these are the only modular block drivers.

I think we can decide on a protocol first (these patches), and then
apply the changes to all (possible) drivers. At least, that was what I
had in mind.

> Also, it ignores CONFIG_CURL and friends.  Perhaps it doesn't matter
> because the module loading code will just see that there is no file
> there, but maybe conditional compilation should be used?

It's true that this patch doesn't look at the CONFIG options. But the
next one does (it takes block-obj-m from the Makefile), and also
replaces this file.

Thanks
Marc

> A plural name would more consistent (i.e. you deleted the
> plural block_modules[] variable and introduced a singular
> block_driver_module[] variable).

Re: [Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Marc Marí]

On Thu, 3 Sep 2015 17:12:29 +0100
Stefan Hajnoczi <stefanha@gmail.com> wrote:

> On Mon, Aug 17, 2015 at 10:09:33AM +0200, Marc Marí wrote:
> > The current module infrastructure has been improved to enable
> > dynamic module loading.
> > 
> > This reduces the load time for very simple guests. For the following
> > configuration (very loaded)
> > 
> > ./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
> >     --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
> >     --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
> >     --enable-kvm --enable-rdma --enable-uuid --enable-vde \
> >     --enable-linux-aio --enable-cap-ng --enable-attr
> > --enable-vhost-net \ --enable-vhost-scsi --enable-spice
> > --enable-rbd --enable-libiscsi \ --enable-smartcard-nss
> > --enable-guest-agent --enable-libusb \ --enable-usb-redir
> > --enable-lzo --enable-snappy --enable-bzip2 \ --enable-seccomp
> > --enable-coroutine-pool --enable-glusterfs \ --enable-tpm
> > --enable-libssh2 --enable-vhdx --enable-numa \ --enable-tcmalloc
> > --target-list=x86_64-softmmu
> > 
> > With modules disabled, there are 142 libraries loaded at startup.
> > Time is the following:
> >  LD time: 0.065 seconds
> >  QEMU time: 0.02 seconds
> >  Total time: 0.085 seconds
> > 
> > With this patch series and modules enabled, there are 128 libraries
> > loaded at startup. Time is the following:
> >  LD time: 0.02 seconds
> >  QEMU time: 0.02 seconds
> >  Total time: 0.04 seconds
> > 
> > Where LD time is the time between the program startup and the jump
> > to main, and QEMU time is the time between the start of main and
> > the first kvm_entry.
> > 
> > These results are just with a few block drivers, that were already
> > a module. Adding more modules (block or not block) should be easy,
> > and will reduce the load time even more.
> 
> Which QEMU command-line did you benchmark?
> 
> Did you have a UI enabled (SDL/GTK/VNC)?

x86_64-softmmu/qemu-system-x86_64 --enable-kvm \
    -kernel /boot/vmlinuz-4.1.4-200.fc22.x86_64 -nographic

Thanks
Marc

Re: [Qemu-devel] [PATCH 0/2] Dynamic module support for block drivers

[Stefan Hajnoczi]

On Thu, Sep 03, 2015 at 08:07:40PM +0200, Marc Marí wrote:
> On Thu, 3 Sep 2015 17:12:29 +0100
> Stefan Hajnoczi <stefanha@gmail.com> wrote:
> 
> > On Mon, Aug 17, 2015 at 10:09:33AM +0200, Marc Marí wrote:
> > > The current module infrastructure has been improved to enable
> > > dynamic module loading.
> > > 
> > > This reduces the load time for very simple guests. For the following
> > > configuration (very loaded)
> > > 
> > > ./configure --enable-sdl --enable-gtk --enable-vte --enable-curses \
> > >     --enable-vnc --enable-vnc-{jpeg,tls,sasl,png} --enable-virtfs \
> > >     --enable-brlapi --enable-curl --enable-fdt --enable-bluez \
> > >     --enable-kvm --enable-rdma --enable-uuid --enable-vde \
> > >     --enable-linux-aio --enable-cap-ng --enable-attr
> > > --enable-vhost-net \ --enable-vhost-scsi --enable-spice
> > > --enable-rbd --enable-libiscsi \ --enable-smartcard-nss
> > > --enable-guest-agent --enable-libusb \ --enable-usb-redir
> > > --enable-lzo --enable-snappy --enable-bzip2 \ --enable-seccomp
> > > --enable-coroutine-pool --enable-glusterfs \ --enable-tpm
> > > --enable-libssh2 --enable-vhdx --enable-numa \ --enable-tcmalloc
> > > --target-list=x86_64-softmmu
> > > 
> > > With modules disabled, there are 142 libraries loaded at startup.
> > > Time is the following:
> > >  LD time: 0.065 seconds
> > >  QEMU time: 0.02 seconds
> > >  Total time: 0.085 seconds
> > > 
> > > With this patch series and modules enabled, there are 128 libraries
> > > loaded at startup. Time is the following:
> > >  LD time: 0.02 seconds
> > >  QEMU time: 0.02 seconds
> > >  Total time: 0.04 seconds
> > > 
> > > Where LD time is the time between the program startup and the jump
> > > to main, and QEMU time is the time between the start of main and
> > > the first kvm_entry.
> > > 
> > > These results are just with a few block drivers, that were already
> > > a module. Adding more modules (block or not block) should be easy,
> > > and will reduce the load time even more.
> > 
> > Which QEMU command-line did you benchmark?
> > 
> > Did you have a UI enabled (SDL/GTK/VNC)?
> 
> x86_64-softmmu/qemu-system-x86_64 --enable-kvm \
>     -kernel /boot/vmlinuz-4.1.4-200.fc22.x86_64 -nographic

Nice, these times are very good.

Stefan