[Qemu-devel] [PATCH v6 0/4] Dynamic module loading for block drivers

[Qemu-devel] [PATCH v6 0/4] Dynamic module loading for block drivers

[Colin Lord]

I'm sending one more version of this series. v5 didn't seem to need any
huge makeovers, but I found a small bug in it so I'm hoping it hasn't
been committed yet. In v5, if a user explicitly specified the driver to
use, eg driver=gluster, and the driver was modularized, it was not
correctly detected and qemu would exit since it could not find the
driver. This series fixes that.

In addition, the two patches I sent to remove unneeded lines in the
Makefile and to modularize nfs are included in this series.

v6:
- Fix bug so that users can specify a modularized driver on the cli
  without qemu exiting
- Remove extra lines from Makefile
- Add patch to modularize NFS

v5:
- No format drivers are modularized, therefore the probe functions are
  all being left completely untouched.
- Remove dmg from block-obj-m since it is not a target of the
  modularization effort.
- Modify module_block.py to only include the library name and protocol
  name fields in the generated struct. The other fields are no longer
  necessary for the drivers that are being modularized.

v4:
- Fix indentation of the generated header file module_block.h
- Drivers and probe functions are now all located in the block/
  directory, rather than being split between block/ and block/probe/. In
  addition the header files for each probe/driver pair are in the block/
  directory, not the include/block/driver/ directory (which no longer
  exists).
- Since the probe files are in block/ now, they follow the naming
  pattern of format-probe.c
- Renamed crypto probe file to be crypto-probe.c, luks is no longer in
  the filename
- Fixed formatting of parallels_probe() function header
- Enforced consistent naming convention for the probe functions. They
  now follow the pattern bdrv_format_probe().

Colin Lord (3):
  blockdev: prepare iSCSI block driver for dynamic loading
  blockdev: Add dynamic module loading for block drivers
  blockdev: Modularize nfs block driver

Marc Mari (1):
  blockdev: Add dynamic generation of module_block.h

 Makefile                        |  10 ++--
 block.c                         |  62 ++++++++++++++++++++---
 block/Makefile.objs             |   4 +-
 block/iscsi.c                   |  36 --------------
 configure                       |   4 +-
 include/qemu/module.h           |   3 ++
 scripts/modules/module_block.py | 108 ++++++++++++++++++++++++++++++++++++++++
 util/module.c                   |  38 ++++----------
 vl.c                            |  38 ++++++++++++++
 9 files changed, 226 insertions(+), 77 deletions(-)
 create mode 100644 scripts/modules/module_block.py

-- 
2.5.5

[Qemu-devel] [PATCH v6 1/4] blockdev: prepare iSCSI block driver for dynamic loading

[Colin Lord]

This commit moves the initialization of the QemuOptsList qemu_iscsi_opts
struct out of block/iscsi.c in order to allow the iscsi module to be
dynamically loaded.

Signed-off-by: Colin Lord <clord@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
---
 block/iscsi.c | 36 ------------------------------------
 vl.c          | 38 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 36 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 95ce9e1..c4a0937 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2010,45 +2010,9 @@ static BlockDriver bdrv_iscsi = {
     .bdrv_attach_aio_context = iscsi_attach_aio_context,
 };
 
-static QemuOptsList qemu_iscsi_opts = {
-    .name = "iscsi",
-    .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
-    .desc = {
-        {
-            .name = "user",
-            .type = QEMU_OPT_STRING,
-            .help = "username for CHAP authentication to target",
-        },{
-            .name = "password",
-            .type = QEMU_OPT_STRING,
-            .help = "password for CHAP authentication to target",
-        },{
-            .name = "password-secret",
-            .type = QEMU_OPT_STRING,
-            .help = "ID of the secret providing password for CHAP "
-                    "authentication to target",
-        },{
-            .name = "header-digest",
-            .type = QEMU_OPT_STRING,
-            .help = "HeaderDigest setting. "
-                    "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
-        },{
-            .name = "initiator-name",
-            .type = QEMU_OPT_STRING,
-            .help = "Initiator iqn name to use when connecting",
-        },{
-            .name = "timeout",
-            .type = QEMU_OPT_NUMBER,
-            .help = "Request timeout in seconds (default 0 = no timeout)",
-        },
-        { /* end of list */ }
-    },
-};
-
 static void iscsi_block_init(void)
 {
     bdrv_register(&bdrv_iscsi);
-    qemu_add_opts(&qemu_iscsi_opts);
 }
 
 block_init(iscsi_block_init);
diff --git a/vl.c b/vl.c
index e7c2c62..1a5f807 100644
--- a/vl.c
+++ b/vl.c
@@ -506,6 +506,41 @@ static QemuOptsList qemu_fw_cfg_opts = {
     },
 };
 
+static QemuOptsList qemu_iscsi_opts = {
+    .name = "iscsi",
+    .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
+    .desc = {
+        {
+            .name = "user",
+            .type = QEMU_OPT_STRING,
+            .help = "username for CHAP authentication to target",
+        },{
+            .name = "password",
+            .type = QEMU_OPT_STRING,
+            .help = "password for CHAP authentication to target",
+        },{
+            .name = "password-secret",
+            .type = QEMU_OPT_STRING,
+            .help = "ID of the secret providing password for CHAP "
+                    "authentication to target",
+        },{
+            .name = "header-digest",
+            .type = QEMU_OPT_STRING,
+            .help = "HeaderDigest setting. "
+                    "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
+        },{
+            .name = "initiator-name",
+            .type = QEMU_OPT_STRING,
+            .help = "Initiator iqn name to use when connecting",
+        },{
+            .name = "timeout",
+            .type = QEMU_OPT_NUMBER,
+            .help = "Request timeout in seconds (default 0 = no timeout)",
+        },
+        { /* end of list */ }
+    },
+};
+
 /**
  * Get machine options
  *
@@ -3001,6 +3036,9 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_icount_opts);
     qemu_add_opts(&qemu_semihosting_config_opts);
     qemu_add_opts(&qemu_fw_cfg_opts);
+#ifdef CONFIG_LIBISCSI
+    qemu_add_opts(&qemu_iscsi_opts);
+#endif
     module_call_init(MODULE_INIT_OPTS);
 
     runstate_init();
-- 
2.5.5

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

[Colin Lord]

From: Marc Mari <markmb@redhat.com>

To simplify the addition of new block modules, add a script that generates
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>
Signed-off-by: Colin Lord <clord@redhat.com>
---
 Makefile                        |   7 +++
 scripts/modules/module_block.py | 108 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+)
 create mode 100644 scripts/modules/module_block.py

diff --git a/Makefile b/Makefile
index 0d7647f..4b9384e 100644
--- a/Makefile
+++ b/Makefile
@@ -76,6 +76,8 @@ GENERATED_HEADERS += trace/generated-ust-provider.h
 GENERATED_SOURCES += trace/generated-ust.c
 endif
 
+GENERATED_HEADERS += module_block.h
+
 # Don't try to regenerate Makefile or configure
 # We don't generate any of them
 Makefile: ;
@@ -352,6 +354,11 @@ ivshmem-client$(EXESUF): $(ivshmem-client-obj-y) libqemuutil.a libqemustub.a
 ivshmem-server$(EXESUF): $(ivshmem-server-obj-y) libqemuutil.a libqemustub.a
 	$(call LINK, $^)
 
+module_block.h: $(SRC_PATH)/scripts/modules/module_block.py config-host.mak
+	$(call quiet-command,$(PYTHON) $< $@ \
+	$(addprefix $(SRC_PATH)/,$(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 100644
index 0000000..db4fb54
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,108 @@
+#!/usr/bin/python
+#
+# Module information generator
+#
+# Copyright Red Hat, Inc. 2015 - 2016
+#
+# 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.
+
+from __future__ import print_function
+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(fheader, library, format_name, protocol_name):
+    lines = []
+    lines.append('.library_name = "' + library + '",')
+    if format_name != "":
+        lines.append('.format_name = "' + format_name + '",')
+    if protocol_name != "":
+        lines.append('.protocol_name = "' + protocol_name + '",')
+
+    text = '\n        '.join(lines)
+    fheader.write('\n    {\n        ' + text + '\n    },')
+
+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 == "};":
+                    add_module(fheader, library, format_name, protocol_name)
+                    found_start = False
+            elif line.find("static BlockDriver") != -1:
+                found_something = True
+                found_start = True
+                format_name = ""
+                protocol_name = ""
+
+        if not found_something:
+            print("No BlockDriver struct found in " + filename + ". \
+                    Is this really a module?", file=sys.stderr)
+            sys.exit(1)
+
+def print_top(fheader):
+    fheader.write('''/* AUTOMATICALLY GENERATED, DO NOT MODIFY */
+/*
+ * QEMU Block Module Infrastructure
+ *
+ * Authors:
+ *  Marc Mari       <markmb@redhat.com>
+ */
+
+''')
+
+    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;
+} block_driver_modules[] = {''')
+
+def print_bottom(fheader):
+    fheader.write('''
+};
+
+#endif
+''')
+
+# First argument: output file
+# All other arguments: modules source files (.c)
+output_file = sys.argv[1]
+with open(output_file, '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.", file=sys.stderr)
+            sys.exit(1)
+
+    print_bottom(fheader)
+
+sys.exit(0)
-- 
2.5.5

[Qemu-devel] [PATCH v6 3/4] blockdev: Add dynamic module loading for block drivers

[Colin Lord]

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.

In addition, only the protocol drivers are being modularized, as they
are the only ones which see significant performance benefits. The format
drivers do not generally link to external libraries, so modularizing
them is of no benefit from a performance perspective.

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

Signed-off-by: Marc Marí <markmb@redhat.com>
Signed-off-by: Colin Lord <clord@redhat.com>
---
 Makefile              |  3 ---
 block.c               | 62 +++++++++++++++++++++++++++++++++++++++++++++------
 block/Makefile.objs   |  3 +--
 include/qemu/module.h |  3 +++
 util/module.c         | 38 +++++++++----------------------
 5 files changed, 70 insertions(+), 39 deletions(-)

diff --git a/Makefile b/Makefile
index 4b9384e..c7aa8cd 100644
--- a/Makefile
+++ b/Makefile
@@ -247,9 +247,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
diff --git a/block.c b/block.c
index 30d64e6..6c5e249 100644
--- a/block.c
+++ b/block.c
@@ -26,6 +26,7 @@
 #include "block/block_int.h"
 #include "block/blockjob.h"
 #include "qemu/error-report.h"
+#include "module_block.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
@@ -241,17 +242,40 @@ BlockDriverState *bdrv_new(void)
     return bs;
 }
 
-BlockDriver *bdrv_find_format(const char *format_name)
+static BlockDriver *bdrv_do_find_format(const char *format_name)
 {
     BlockDriver *drv1;
+
     QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (!strcmp(drv1->format_name, format_name)) {
             return drv1;
         }
     }
+
     return NULL;
 }
 
+BlockDriver *bdrv_find_format(const char *format_name)
+{
+    BlockDriver *drv1;
+    size_t i;
+
+    drv1 = bdrv_do_find_format(format_name);
+    if (drv1) {
+        return drv1;
+    }
+
+    /* The driver isn't registered, maybe we need to load a module */
+    for (i = 0; i < 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);
+            break;
+        }
+    }
+
+    return bdrv_do_find_format(format_name);
+}
+
 static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)
 {
     static const char *whitelist_rw[] = {
@@ -460,6 +484,19 @@ static BlockDriver *find_hdev_driver(const char *filename)
     return drv;
 }
 
+static BlockDriver *bdrv_do_find_protocol(const char *protocol)
+{
+    BlockDriver *drv1;
+
+    QLIST_FOREACH(drv1, &bdrv_drivers, list) {
+        if (drv1->protocol_name && !strcmp(drv1->protocol_name, protocol)) {
+            return drv1;
+        }
+    }
+
+    return NULL;
+}
+
 BlockDriver *bdrv_find_protocol(const char *filename,
                                 bool allow_protocol_prefix,
                                 Error **errp)
@@ -468,6 +505,7 @@ BlockDriver *bdrv_find_protocol(const char *filename,
     char protocol[128];
     int len;
     const char *p;
+    size_t i;
 
     /* TODO Drivers without bdrv_file_open must be specified explicitly */
 
@@ -494,15 +532,25 @@ 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)) {
-            return drv1;
+
+    drv1 = bdrv_do_find_protocol(protocol);
+    if (drv1) {
+        return drv1;
+    }
+
+    for (i = 0; i < 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);
+            break;
         }
     }
 
-    error_setg(errp, "Unknown protocol '%s'", protocol);
-    return NULL;
+    drv1 = bdrv_do_find_protocol(protocol);
+    if (!drv1) {
+        error_setg(errp, "Unknown protocol '%s'", protocol);
+    }
+    return drv1;
 }
 
 /*
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 2593a2f..595f366 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,4 +1,4 @@
-block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o
+block-obj-y += raw_bsd.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o dmg.o
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o qcow2-cache.o
 block-obj-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o qed-cluster.o
 block-obj-y += qed-check.o
@@ -39,7 +39,6 @@ gluster.o-libs     := $(GLUSTERFS_LIBS)
 ssh.o-cflags       := $(LIBSSH2_CFLAGS)
 ssh.o-libs         := $(LIBSSH2_LIBS)
 archipelago.o-libs := $(ARCHIPELAGO_LIBS)
-block-obj-m        += dmg.o
 dmg.o-libs         := $(BZIP2_LIBS)
 qcow.o-libs        := -lz
 linux-aio.o-libs   := -laio
diff --git a/include/qemu/module.h b/include/qemu/module.h
index 2370708..dc2c9d4 100644
--- a/include/qemu/module.h
+++ b/include/qemu/module.h
@@ -52,9 +52,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/util/module.c b/util/module.c
index 86e3f7a..a5f7fbd 100644
--- a/util/module.c
+++ b/util/module.c
@@ -87,14 +87,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) {
@@ -145,6 +142,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;
@@ -159,14 +157,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;
@@ -177,15 +171,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 ? : "");
@@ -194,16 +179,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.5.5

[Qemu-devel] [PATCH v6 4/4] blockdev: Modularize nfs block driver

[Colin Lord]

Modularizes the nfs block driver so that it gets dynamically loaded.
---
 block/Makefile.objs | 1 +
 configure           | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 595f366..fa4d8b8 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -28,6 +28,7 @@ block-obj-y += crypto.o
 common-obj-y += stream.o
 common-obj-y += backup.o
 
+nfs.o-libs         := $(LIBNFS_LIBS)
 iscsi.o-cflags     := $(LIBISCSI_CFLAGS)
 iscsi.o-libs       := $(LIBISCSI_LIBS)
 curl.o-cflags      := $(CURL_CFLAGS)
diff --git a/configure b/configure
index f57fcc6..f1e7d14 100755
--- a/configure
+++ b/configure
@@ -4561,7 +4561,6 @@ if test "$libnfs" != "no" ; then
   if $pkg_config --atleast-version=1.9.3 libnfs; then
     libnfs="yes"
     libnfs_libs=$($pkg_config --libs libnfs)
-    LIBS="$LIBS $libnfs_libs"
   else
     if test "$libnfs" = "yes" ; then
       feature_not_found "libnfs" "Install libnfs devel >= 1.9.3"
@@ -5320,7 +5319,8 @@ if test "$libiscsi" = "yes" ; then
 fi
 
 if test "$libnfs" = "yes" ; then
-  echo "CONFIG_LIBNFS=y" >> $config_host_mak
+  echo "CONFIG_LIBNFS=m" >> $config_host_mak
+  echo "LIBNFS_LIBS=$libnfs_libs" >> $config_host_mak
 fi
 
 if test "$seccomp" = "yes"; then
-- 
2.5.5

Re: [Qemu-devel] [PATCH v6 0/4] Dynamic module loading for block drivers

[Colin Lord]

On 08/02/2016 10:12 AM, Colin Lord wrote:
> I'm sending one more version of this series. v5 didn't seem to need any
> huge makeovers, but I found a small bug in it so I'm hoping it hasn't
> been committed yet. In v5, if a user explicitly specified the driver to
> use, eg driver=gluster, and the driver was modularized, it was not
> correctly detected and qemu would exit since it could not find the
> driver. This series fixes that.
> 
> In addition, the two patches I sent to remove unneeded lines in the
> Makefile and to modularize nfs are included in this series.
> 
> v6:
> - Fix bug so that users can specify a modularized driver on the cli
>   without qemu exiting
> - Remove extra lines from Makefile
> - Add patch to modularize NFS
> 
> v5:
> - No format drivers are modularized, therefore the probe functions are
>   all being left completely untouched.
> - Remove dmg from block-obj-m since it is not a target of the
>   modularization effort.
> - Modify module_block.py to only include the library name and protocol
>   name fields in the generated struct. The other fields are no longer
>   necessary for the drivers that are being modularized.
> 
> v4:
> - Fix indentation of the generated header file module_block.h
> - Drivers and probe functions are now all located in the block/
>   directory, rather than being split between block/ and block/probe/. In
>   addition the header files for each probe/driver pair are in the block/
>   directory, not the include/block/driver/ directory (which no longer
>   exists).
> - Since the probe files are in block/ now, they follow the naming
>   pattern of format-probe.c
> - Renamed crypto probe file to be crypto-probe.c, luks is no longer in
>   the filename
> - Fixed formatting of parallels_probe() function header
> - Enforced consistent naming convention for the probe functions. They
>   now follow the pattern bdrv_format_probe().
> 
> Colin Lord (3):
>   blockdev: prepare iSCSI block driver for dynamic loading
>   blockdev: Add dynamic module loading for block drivers
>   blockdev: Modularize nfs block driver
> 
> Marc Mari (1):
>   blockdev: Add dynamic generation of module_block.h
> 
>  Makefile                        |  10 ++--
>  block.c                         |  62 ++++++++++++++++++++---
>  block/Makefile.objs             |   4 +-
>  block/iscsi.c                   |  36 --------------
>  configure                       |   4 +-
>  include/qemu/module.h           |   3 ++
>  scripts/modules/module_block.py | 108 ++++++++++++++++++++++++++++++++++++++++
>  util/module.c                   |  38 ++++----------
>  vl.c                            |  38 ++++++++++++++
>  9 files changed, 226 insertions(+), 77 deletions(-)
>  create mode 100644 scripts/modules/module_block.py
> 
I did want to address one of the items Fam brought up in v5 that I never
replied to, so I'm adding Fam to CC here.

It sounds like since this series removes dmg from the list of
modularized drivers, there is a libbz2 dependency that may get pulled in
from the dmg driver. I believe Fam was suggesting moving that part of
the dmg driver into its own module (although not modularizing the entire
dmg driver). Does that sound like a correct interpretation Fam? I do
like the idea, but I'm a little hesitant to put that into this series
because I'd like to keep this series fairly simple so that hopefully
there doesn't need to be too much more discussion about it. My
internship will be ending soon so I'd prefer this series be finished
sooner rather than later. Also I don't think anything in this series
excludes this idea from being added later if we want. Of course there is
always room for discussion, so if there are other thoughts I'm happy to
listen.

Colin

Re: [Qemu-devel] [PATCH v6 0/4] Dynamic module loading for block drivers

[Fam Zheng]

On Tue, 08/02 10:17, Colin Lord wrote:
> It sounds like since this series removes dmg from the list of
> modularized drivers, there is a libbz2 dependency that may get pulled in
> from the dmg driver. I believe Fam was suggesting moving that part of
> the dmg driver into its own module (although not modularizing the entire
> dmg driver). Does that sound like a correct interpretation Fam? I do

Yes.

> like the idea, but I'm a little hesitant to put that into this series
> because I'd like to keep this series fairly simple so that hopefully
> there doesn't need to be too much more discussion about it. My
> internship will be ending soon so I'd prefer this series be finished
> sooner rather than later. Also I don't think anything in this series
> excludes this idea from being added later if we want. Of course there is
> always room for discussion, so if there are other thoughts I'm happy to
> listen.

I'll prepare a patch that can be applied before this series so that we don't
regress on dmg library.

Fam

Re: [Qemu-devel] [Qemu-block] [PATCH v6 1/4] blockdev: prepare iSCSI block driver for dynamic loading

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2064 bytes --]

On Tue, Aug 02, 2016 at 10:12:14AM -0400, Colin Lord wrote:
> diff --git a/vl.c b/vl.c
> index e7c2c62..1a5f807 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -506,6 +506,41 @@ static QemuOptsList qemu_fw_cfg_opts = {
>      },
>  };
>  
> +static QemuOptsList qemu_iscsi_opts = {
> +    .name = "iscsi",
> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
> +    .desc = {
> +        {
> +            .name = "user",
> +            .type = QEMU_OPT_STRING,
> +            .help = "username for CHAP authentication to target",
> +        },{
> +            .name = "password",
> +            .type = QEMU_OPT_STRING,
> +            .help = "password for CHAP authentication to target",
> +        },{
> +            .name = "password-secret",
> +            .type = QEMU_OPT_STRING,
> +            .help = "ID of the secret providing password for CHAP "
> +                    "authentication to target",
> +        },{
> +            .name = "header-digest",
> +            .type = QEMU_OPT_STRING,
> +            .help = "HeaderDigest setting. "
> +                    "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
> +        },{
> +            .name = "initiator-name",
> +            .type = QEMU_OPT_STRING,
> +            .help = "Initiator iqn name to use when connecting",
> +        },{
> +            .name = "timeout",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "Request timeout in seconds (default 0 = no timeout)",
> +        },
> +        { /* end of list */ }
> +    },
> +};
> +
>  /**
>   * Get machine options
>   *
> @@ -3001,6 +3036,9 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_icount_opts);
>      qemu_add_opts(&qemu_semihosting_config_opts);
>      qemu_add_opts(&qemu_fw_cfg_opts);
> +#ifdef CONFIG_LIBISCSI
> +    qemu_add_opts(&qemu_iscsi_opts);
> +#endif

This will probably produce "unused variable" warnings and cause the
build to fail when CONFIG_LIBISCSI is not defined.

In this case I'd add an ifdef around qemu_iscsi_opts.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

On Tue, Aug 02, 2016 at 10:12:15AM -0400, Colin Lord wrote:
> From: Marc Mari <markmb@redhat.com>
> 
> To simplify the addition of new block modules, add a script that generates
> 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>
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  Makefile                        |   7 +++
>  scripts/modules/module_block.py | 108 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 115 insertions(+)
>  create mode 100644 scripts/modules/module_block.py

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v6 3/4] blockdev: Add dynamic module loading for block drivers

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1170 bytes --]

On Tue, Aug 02, 2016 at 10:12:16AM -0400, Colin Lord 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.
> 
> In addition, only the protocol drivers are being modularized, as they
> are the only ones which see significant performance benefits. The format
> drivers do not generally link to external libraries, so modularizing
> them is of no benefit from a performance perspective.
> 
> All the necessary module information is located in a new structure found
> in module_block.h
> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  Makefile              |  3 ---
>  block.c               | 62 +++++++++++++++++++++++++++++++++++++++++++++------
>  block/Makefile.objs   |  3 +--
>  include/qemu/module.h |  3 +++
>  util/module.c         | 38 +++++++++----------------------
>  5 files changed, 70 insertions(+), 39 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v6 4/4] blockdev: Modularize nfs block driver

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 302 bytes --]

On Tue, Aug 02, 2016 at 10:12:17AM -0400, Colin Lord wrote:
> Modularizes the nfs block driver so that it gets dynamically loaded.
> ---
>  block/Makefile.objs | 1 +
>  configure           | 4 ++--
>  2 files changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v6 1/4] blockdev: prepare iSCSI block driver for dynamic loading

[Colin Lord]

On 08/08/2016 11:18 AM, Stefan Hajnoczi wrote:
> On Tue, Aug 02, 2016 at 10:12:14AM -0400, Colin Lord wrote:
>> diff --git a/vl.c b/vl.c
>> index e7c2c62..1a5f807 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -506,6 +506,41 @@ static QemuOptsList qemu_fw_cfg_opts = {
>>      },
>>  };
>>  
>> +static QemuOptsList qemu_iscsi_opts = {
>> +    .name = "iscsi",
>> +    .head = QTAILQ_HEAD_INITIALIZER(qemu_iscsi_opts.head),
>> +    .desc = {
>> +        {
>> +            .name = "user",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "username for CHAP authentication to target",
>> +        },{
>> +            .name = "password",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "password for CHAP authentication to target",
>> +        },{
>> +            .name = "password-secret",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "ID of the secret providing password for CHAP "
>> +                    "authentication to target",
>> +        },{
>> +            .name = "header-digest",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "HeaderDigest setting. "
>> +                    "{CRC32C|CRC32C-NONE|NONE-CRC32C|NONE}",
>> +        },{
>> +            .name = "initiator-name",
>> +            .type = QEMU_OPT_STRING,
>> +            .help = "Initiator iqn name to use when connecting",
>> +        },{
>> +            .name = "timeout",
>> +            .type = QEMU_OPT_NUMBER,
>> +            .help = "Request timeout in seconds (default 0 = no timeout)",
>> +        },
>> +        { /* end of list */ }
>> +    },
>> +};
>> +
>>  /**
>>   * Get machine options
>>   *
>> @@ -3001,6 +3036,9 @@ int main(int argc, char **argv, char **envp)
>>      qemu_add_opts(&qemu_icount_opts);
>>      qemu_add_opts(&qemu_semihosting_config_opts);
>>      qemu_add_opts(&qemu_fw_cfg_opts);
>> +#ifdef CONFIG_LIBISCSI
>> +    qemu_add_opts(&qemu_iscsi_opts);
>> +#endif
> 
> This will probably produce "unused variable" warnings and cause the
> build to fail when CONFIG_LIBISCSI is not defined.
> 
> In this case I'd add an ifdef around qemu_iscsi_opts.
> 
Oddly enough I don't actually get any warnings when iscsi is disabled,
but in any case the ifdef should probably be there anyway so I'll fix
and resubmit.