All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 REPOST 0/2] Add dynamic module loading for block drivers
@ 2016-04-12 13:41 Richard W.M. Jones
  2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 1/2] " Richard W.M. Jones
  2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 2/2] Add dynamic generation of module_block.h Richard W.M. Jones
  0 siblings, 2 replies; 4+ messages in thread
From: Richard W.M. Jones @ 2016-04-12 13:41 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, qemu-block, famz, den-lists

This is a repost of the support for dynamically loaded block drivers.
It is identical to how it was posted last summer, except that I have
rebased it and checked that it still works.  It was last posted here:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/threads.html#01995

Last time this was reviewed there were two strands of comments/
objections:

(1) Fam Zheng objected to parsing C structs using the Python generator
code and wanted special macros to be used instead:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02201.html

My objection to that is that it means the same information is defined
in two places, with the usual opportunities for the information to get
out of synch, which could cause crashes or modules not to be loaded.

(2) Denis Lunev wanted module loading to work more like Linux modules:

https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg05331.html

That makes the changes much larger.

Rich.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v2 REPOST 1/2] Add dynamic module loading for block drivers
  2016-04-12 13:41 [Qemu-devel] [PATCH v2 REPOST 0/2] Add dynamic module loading for block drivers Richard W.M. Jones
@ 2016-04-12 13:41 ` Richard W.M. Jones
  2016-04-19  9:43   ` Stefan Hajnoczi
  2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 2/2] Add dynamic generation of module_block.h Richard W.M. Jones
  1 sibling, 1 reply; 4+ messages in thread
From: Richard W.M. Jones @ 2016-04-12 13:41 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, qemu-block, famz, den-lists

From: Marc Marí <markmb@redhat.com>

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                     | 70 +++++++++++++++++++++++++++++++++++
 configure                   |  2 +-
 include/qemu/module.h       |  3 ++
 include/qemu/module_block.h | 90 +++++++++++++++++++++++++++++++++++++++++++++
 util/module.c               | 38 ++++++-------------
 5 files changed, 175 insertions(+), 28 deletions(-)
 create mode 100644 include/qemu/module_block.h

diff --git a/block.c b/block.c
index d4939b4..ccd9e57 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 "qemu/module_block.h"
 #include "qemu/module.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
@@ -252,11 +253,30 @@ BlockDriverState *bdrv_new(void)
 BlockDriver *bdrv_find_format(const char *format_name)
 {
     BlockDriver *drv1;
+    size_t i;
+
     QLIST_FOREACH(drv1, &bdrv_drivers, list) {
         if (!strcmp(drv1->format_name, format_name)) {
             return drv1;
         }
     }
+
+    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);
+            /* 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;
 }
 
@@ -457,8 +477,15 @@ int get_tmp_filename(char *filename, int size)
 static BlockDriver *find_hdev_driver(const char *filename)
 {
     int score_max = 0, score;
+    size_t i;
     BlockDriver *drv = NULL, *d;
 
+    for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+        if (block_driver_modules[i].has_probe_device) {
+            block_module_load_one(block_driver_modules[i].library_name);
+        }
+    }
+
     QLIST_FOREACH(d, &bdrv_drivers, list) {
         if (d->bdrv_probe_device) {
             score = d->bdrv_probe_device(filename);
@@ -480,6 +507,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 */
 
@@ -506,6 +534,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)) {
@@ -513,6 +542,23 @@ BlockDriver *bdrv_find_protocol(const char *filename,
         }
     }
 
+    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);
+            /* 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;
 }
@@ -535,8 +581,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                             const char *filename)
 {
     int score_max = 0, score;
+    size_t i;
     BlockDriver *drv = NULL, *d;
 
+    for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
+        if (block_driver_modules[i].has_probe) {
+            block_module_load_one(block_driver_modules[i].library_name);
+        }
+    }
+
     QLIST_FOREACH(d, &bdrv_drivers, list) {
         if (d->bdrv_probe) {
             score = d->bdrv_probe(buf, buf_size, filename);
@@ -2794,6 +2847,7 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
     BlockDriver *drv;
     int count = 0;
     int i;
+    size_t n;
     const char **formats = NULL;
 
     QLIST_FOREACH(drv, &bdrv_drivers, list) {
@@ -2811,6 +2865,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
         }
     }
 
+    for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
+        if (block_driver_modules[n].format_name) {
+            bool found = false;
+            int i = count;
+            while (formats && i && !found) {
+                found = !strcmp(formats[--i],
+                                block_driver_modules[n].format_name);
+            }
+
+            if (!found) {
+                formats = g_renew(const char *, formats, count + 1);
+                formats[count++] = block_driver_modules[n].format_name;
+            }
+        }
+    }
+
     qsort(formats, count, sizeof(formats[0]), qsort_strcmp);
 
     for (i = 0; i < count; i++) {
diff --git a/configure b/configure
index 5db29f0..725d576 100755
--- a/configure
+++ b/configure
@@ -5261,7 +5261,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 2370708..4729858 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/include/qemu/module_block.h b/include/qemu/module_block.h
new file mode 100644
index 0000000..d725db8
--- /dev/null
+++ b/include/qemu/module_block.h
@@ -0,0 +1,90 @@
+/* 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.
+ *
+ */
+
+#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_modules[] = {
+	{
+	.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
+
diff --git a/util/module.c b/util/module.c
index ce058ae..8ff1340 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.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [Qemu-devel] [PATCH v2 REPOST 2/2] Add dynamic generation of module_block.h
  2016-04-12 13:41 [Qemu-devel] [PATCH v2 REPOST 0/2] Add dynamic module loading for block drivers Richard W.M. Jones
  2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 1/2] " Richard W.M. Jones
@ 2016-04-12 13:41 ` Richard W.M. Jones
  1 sibling, 0 replies; 4+ messages in thread
From: Richard W.M. Jones @ 2016-04-12 13:41 UTC (permalink / raw)
  To: kwolf; +Cc: qemu-devel, qemu-block, famz, den-lists

From: Marc Marí <markmb@redhat.com>

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 ++-
 include/qemu/module_block.h     |  90 ---------------------------
 scripts/modules/module_block.py | 134 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 142 insertions(+), 93 deletions(-)
 delete mode 100644 include/qemu/module_block.h
 create mode 100644 scripts/modules/module_block.py

diff --git a/.gitignore b/.gitignore
index 88a80ff..e87b09a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -109,3 +109,4 @@ cscope.*
 tags
 TAGS
 *~
+/include/qemu/module_block.h
diff --git a/Makefile b/Makefile
index 1d076a9..27bde8b 100644
--- a/Makefile
+++ b/Makefile
@@ -75,6 +75,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: ;
@@ -227,9 +229,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
@@ -334,6 +333,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, $^)
 
+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/include/qemu/module_block.h b/include/qemu/module_block.h
deleted file mode 100644
index d725db8..0000000
--- a/include/qemu/module_block.h
+++ /dev/null
@@ -1,90 +0,0 @@
-/* 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.
- *
- */
-
-#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_modules[] = {
-	{
-	.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
-
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
new file mode 100644
index 0000000..0846362
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,134 @@
+#!/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.
+
+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(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?", file=sys.stderr)
+            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_modules[] = {''')
+
+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", file=sys.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.", file=sys.stderr)
+            sys.exit(1)
+
+    print_bottom(fheader)
+
+sys.exit(0)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH v2 REPOST 1/2] Add dynamic module loading for block drivers
  2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 1/2] " Richard W.M. Jones
@ 2016-04-19  9:43   ` Stefan Hajnoczi
  0 siblings, 0 replies; 4+ messages in thread
From: Stefan Hajnoczi @ 2016-04-19  9:43 UTC (permalink / raw)
  To: Richard W.M. Jones; +Cc: kwolf, famz, qemu-devel, qemu-block, den-lists

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

On Tue, Apr 12, 2016 at 02:41:13PM +0100, Richard W.M. Jones wrote:
> From: Marc Marí <markmb@redhat.com>
> 
> Extend the current module interface to allow for block drivers to be loaded
> dynamically on request.

I like this approach to run-time loading QEMU modules because it's not a
plugin system that would inevitably be abused by license violators.

> 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.

This libiscsi issue is easy to solve.  Compile QemuOptsList
qemu_iscsi_opts into the main binary.  Please do it in a separate
commit.

> @@ -2811,6 +2865,22 @@ void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
>          }
>      }
>  
> +    for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
> +        if (block_driver_modules[n].format_name) {
> +            bool found = false;
> +            int i = count;
> +            while (formats && i && !found) {
> +                found = !strcmp(formats[--i],
> +                                block_driver_modules[n].format_name);
> +            }
> +
> +            if (!found) {
> +                formats = g_renew(const char *, formats, count + 1);
> +                formats[count++] = block_driver_modules[n].format_name;
> +            }
> +        }
> +    }

There is code duplication in other hunks but this is where I'd draw the
line.  I can be done without copy-paste:

static const char **add_format(const char **formats, int *count,
                               const char *format_name)
{
    int i;

    while (i = 0; i < *count; i++) {
        if (!strcmp(formats[i], format_name) {
	    return;
	}
    }

    *count += 1;
    formats = g_renew(const char *, formats, *count);
    formats[*count] = format_name;
    return formats;
}

...

QLIST_FOREACH(drv, &bdrv_drivers, list) {
    if (drv->format_name) {
        formats = add_format(formats, &count, drv->format_name);
    }
}

for (n = 0; n < ARRAY_SIZE(block_driver_modules); ++n) {
    if (block_driver_modules[n].format_name) {
        formats = add_format(formats, &count,
	                     block_driver_modules[n].format_name);
    }
}

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2016-04-19  9:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-12 13:41 [Qemu-devel] [PATCH v2 REPOST 0/2] Add dynamic module loading for block drivers Richard W.M. Jones
2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 1/2] " Richard W.M. Jones
2016-04-19  9:43   ` Stefan Hajnoczi
2016-04-12 13:41 ` [Qemu-devel] [PATCH v2 REPOST 2/2] Add dynamic generation of module_block.h Richard W.M. Jones

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.