[Qemu-devel] [PATCH v3 00/32] Dynamic module loading for block drivers

[Qemu-devel] [PATCH v3 00/32] Dynamic module loading for block drivers

[Colin Lord]

This is the next version of this patch series. The first three patches
in the series are mostly the same as they were last time, but with the
issues mentioned in the reviews fixed. Most notably this means much less
copy-paste happening in block.c.

The new changes, and the reason this series is so large, is a result of
isolating the block driver probe functions from the rest of the drivers.
This was mentioned in the reviews from the last version as something
that should be added. This allows the probes to be accessed without
needing to load all the modules. There are quite a few probes, which is
why there are so many patches in this series. I tried to keep the
patches where I was relocating the code separate from the patches where
the probe function signatures were changed.

The probe functions now return both a score and a format/protocol name,
which allows them to be completely separate from the drivers they
correspond to. I didn't put all the probe functions in the same file as
Fam suggested because it turns out there are some naming conflicts
between some macros commonly used in the probe functions. For instance,
QCowHeader is used in both qcow and qcow2 probes, but the meaning is
different between them. Having separate files makes resolving these
conflicts simple.

Also, just as a point of interest, I've done some basic profiling of my
own to see what performance benefits there are to be made. I recorded
the time to main of each option that you can enable/disable in the
configuration by enabling each option by itself while explicitly
disabling all other options. With each configuration I ran qemu 5 times
while recording the time to main. All the results are in milliseconds.

minimal config      2.772
vhdx                2.776
attr                2.786
coroutine-pool      2.836
bzip                2.856
linux-aio           2.867
vnc-jpeg            2.872
snappy              2.875
fdt                 2.888
brlapi              2.916
vhost-net           2.922
bluez               2.937
cap-ng              2.940
libiscsi            2.945
nettle              2.945
seccomp             2.973
numa                2.983
lzo                 2.994
usb-redir           3.005
cocoa               3.007
libnfs              3.026
vnc-sasl            3.071
vnc-png             3.081
virtfs              3.081
uuid                3.086
tpm                 3.136
curses              3.145
gcrypt              3.190
smartcar            3.242
rdma                3.570
jemalloc            3.584
glusterfs           3.597
sdl                 3.677
libusb              3.758
libssh2             3.906
tcmalloc            3.959
xen                 4.151
xen-pci-passthrough 4.167
curl                5.245
spice               5.289
gnutls              5.400
gtk                 8.782
vte                 9.764
rbd                 44.315

Colin Lord (30):
  blockdev: prepare iSCSI block driver for dynamic loading
  blockdev: Move bochs probe into separate file
  blockdev: Move cloop probe to its own file
  blockdev: Move luks probe to its own file
  blockdev: Move dmg probe to its own file
  blockdev: Move parallels probe to its own file
  blockdev: Move qcow probe to its own file
  blockdev: Move qcow2 probe to its own file
  blockdev: Move qed probe to its own file
  blockdev: Move raw probe to its own file
  blockdev: Move vdi probe to its own file
  blockdev: Move vhdx probe to its own file
  blockdev: Move vmdk probe to its own file
  blockdev: Move vpc probe to its own file
  blockdev: Separate bochs probe from its driver
  blockdev: Separate cloop probe from its driver
  blockdev: Separate luks probe from its driver
  blockdev: Separate dmg probe from its driver
  blockdev: Separate parallels probe from its driver
  blockdev: Separate qcow probe from its driver
  blockdev: Separate qcow2 probe from its driver
  blockdev: Separate qed probe from its driver
  blockdev: Separate raw probe from its driver
  blockdev: Separate vdi probe from its driver
  blockdev: Separate vhdx probe from its driver
  blockdev: Separate vmdk probe from its driver
  blockdev: Separate vpc probe from its driver
  blockdev: Remove the .bdrv_probe field from BlockDrivers
  blockdev: Separate out bdrv_probe_device functions
  blockdev: Remove bdrv_probe_device field from BlockDriver

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

 Makefile                         |   7 ++
 block.c                          | 181 +++++++++++++++++++++++++++++++--------
 block/Makefile.objs              |   4 +
 block/bochs.c                    |  56 +-----------
 block/cloop.c                    |  17 +---
 block/crypto.c                   |  22 +----
 block/dmg.c                      |  17 +---
 block/iscsi.c                    |  36 --------
 block/parallels.c                |  44 +---------
 block/probe/bochs.c              |  28 ++++++
 block/probe/cloop.c              |  22 +++++
 block/probe/dmg.c                |  22 +++++
 block/probe/host_cdrom.c         |  47 ++++++++++
 block/probe/host_device.c        |  42 +++++++++
 block/probe/luks.c               |  26 ++++++
 block/probe/parallels.c          |  26 ++++++
 block/probe/qcow.c               |  22 +++++
 block/probe/qcow2.c              |  22 +++++
 block/probe/qed.c                |  24 ++++++
 block/probe/raw.c                |  14 +++
 block/probe/vdi.c                |  29 +++++++
 block/probe/vhdx.c               |  27 ++++++
 block/probe/vmdk.c               |  67 +++++++++++++++
 block/probe/vpc.c                |  16 ++++
 block/qcow.c                     |  32 +------
 block/qcow2.c                    |  14 +--
 block/qed.c                      |  16 +---
 block/raw-posix.c                |  55 +-----------
 block/raw-win32.c                |  11 +--
 block/raw_bsd.c                  |  10 +--
 block/vdi.c                      |  70 +--------------
 block/vhdx.c                     |  21 +----
 block/vmdk.c                     |  61 +------------
 block/vpc.c                      |   9 +-
 include/block/block_int.h        |   3 -
 include/block/driver/bochs.h     |  40 +++++++++
 include/block/driver/parallels.h |  26 ++++++
 include/block/driver/qcow.h      |  21 +++++
 include/block/driver/vdi.h       |  49 +++++++++++
 include/block/driver/vmdk.h      |   7 ++
 include/block/probe.h            |  33 +++++++
 include/qemu/module.h            |   3 +
 scripts/modules/module_block.py  | 108 +++++++++++++++++++++++
 util/module.c                    |  38 +++-----
 vl.c                             |  38 ++++++++
 45 files changed, 948 insertions(+), 535 deletions(-)
 create mode 100644 block/probe/bochs.c
 create mode 100644 block/probe/cloop.c
 create mode 100644 block/probe/dmg.c
 create mode 100644 block/probe/host_cdrom.c
 create mode 100644 block/probe/host_device.c
 create mode 100644 block/probe/luks.c
 create mode 100644 block/probe/parallels.c
 create mode 100644 block/probe/qcow.c
 create mode 100644 block/probe/qcow2.c
 create mode 100644 block/probe/qed.c
 create mode 100644 block/probe/raw.c
 create mode 100644 block/probe/vdi.c
 create mode 100644 block/probe/vhdx.c
 create mode 100644 block/probe/vmdk.c
 create mode 100644 block/probe/vpc.c
 create mode 100644 include/block/driver/bochs.h
 create mode 100644 include/block/driver/parallels.h
 create mode 100644 include/block/driver/qcow.h
 create mode 100644 include/block/driver/vdi.h
 create mode 100644 include/block/driver/vmdk.h
 create mode 100644 include/block/probe.h
 create mode 100644 scripts/modules/module_block.py

-- 
2.5.5

[Qemu-devel] [PATCH v3 01/32] 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>
---
 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 9bb5ff6..8246075 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1880,45 +1880,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 9bb7f4c..cb640c2 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
  *
@@ -2982,6 +3017,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 v3 02/32] 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 | 122 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 129 insertions(+)
 create mode 100644 scripts/modules/module_block.py

diff --git a/Makefile b/Makefile
index c054bc6..29035ac 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..4075574
--- /dev/null
+++ b/scripts/modules/module_block.py
@@ -0,0 +1,122 @@
+#!/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,
+                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
+ *
+ * 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;
+    bool has_probe;
+    bool has_probe_device;
+} 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 v3 03/32] blockdev: Add dynamic module loading for block drivers

[Colin Lord]

From: Marc Mari <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.

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>
---
 block.c               | 110 ++++++++++++++++++++++++++++++++++++++++++--------
 include/qemu/module.h |   3 ++
 util/module.c         |  38 +++++------------
 3 files changed, 108 insertions(+), 43 deletions(-)

diff --git a/block.c b/block.c
index f4648e9..88a05b2 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"
@@ -239,17 +240,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[] = {
@@ -443,8 +467,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);
@@ -458,6 +489,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)
@@ -466,6 +510,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 */
 
@@ -492,15 +537,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;
 }
 
 /*
@@ -521,8 +576,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);
@@ -2738,26 +2800,42 @@ static int qsort_strcmp(const void *a, const void *b)
     return strcmp(a, b);
 }
 
+static const char **add_format(const char **formats, int *count,
+                               const char *format_name)
+{
+    int i;
+
+    for (i = 0; i < *count; i++) {
+        if (!strcmp(formats[i], format_name)) {
+            return formats;
+        }
+    }
+
+    formats = g_renew(const char *, formats, *count + 1);
+    formats[*count] = format_name;
+    *count += 1;
+    return formats;
+}
+
 void bdrv_iterate_format(void (*it)(void *opaque, const char *name),
                          void *opaque)
 {
     BlockDriver *drv;
     int count = 0;
     int i;
+    size_t n;
     const char **formats = NULL;
 
     QLIST_FOREACH(drv, &bdrv_drivers, list) {
         if (drv->format_name) {
-            bool found = false;
-            int i = count;
-            while (formats && i && !found) {
-                found = !strcmp(formats[--i], drv->format_name);
-            }
+            formats = add_format(formats, &count, drv->format_name);
+        }
+    }
 
-            if (!found) {
-                formats = g_renew(const char *, formats, count + 1);
-                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);
         }
     }
 
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 v3 04/32] blockdev: Move bochs probe into separate file

[Colin Lord]

This puts the bochs probe function into its own separate file as part of
the process of modularizing block drivers. Having the probe functions
separate from the rest of the driver allows us to probe without having
to potentially unnecessarily load the driver.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs          |  1 +
 block/bochs.c                | 55 ++------------------------------------------
 block/probe/bochs.c          | 21 +++++++++++++++++
 include/block/driver/bochs.h | 40 ++++++++++++++++++++++++++++++++
 include/block/probe.h        |  6 +++++
 5 files changed, 70 insertions(+), 53 deletions(-)
 create mode 100644 block/probe/bochs.c
 create mode 100644 include/block/driver/bochs.h
 create mode 100644 include/block/probe.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 44a5416..bc0c2aa 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -24,6 +24,7 @@ block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
+block-obj-y += probe/bochs.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/bochs.c b/block/bochs.c
index 6c8d0f3..11da0fd 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -28,45 +28,11 @@
 #include "block/block_int.h"
 #include "qemu/module.h"
 #include "qemu/bswap.h"
+#include "block/driver/bochs.h"
+#include "block/probe.h"
 
 /**************************************************************/
 
-#define HEADER_MAGIC "Bochs Virtual HD Image"
-#define HEADER_VERSION 0x00020000
-#define HEADER_V1 0x00010000
-#define HEADER_SIZE 512
-
-#define REDOLOG_TYPE "Redolog"
-#define GROWING_TYPE "Growing"
-
-// not allocated: 0xffffffff
-
-// always little-endian
-struct bochs_header {
-    char magic[32];     /* "Bochs Virtual HD Image" */
-    char type[16];      /* "Redolog" */
-    char subtype[16];   /* "Undoable" / "Volatile" / "Growing" */
-    uint32_t version;
-    uint32_t header;    /* size of header */
-
-    uint32_t catalog;   /* num of entries */
-    uint32_t bitmap;    /* bitmap size */
-    uint32_t extent;    /* extent size */
-
-    union {
-        struct {
-            uint32_t reserved;  /* for ??? */
-            uint64_t disk;      /* disk size */
-            char padding[HEADER_SIZE - 64 - 20 - 12];
-        } QEMU_PACKED redolog;
-        struct {
-            uint64_t disk;      /* disk size */
-            char padding[HEADER_SIZE - 64 - 20 - 8];
-        } QEMU_PACKED redolog_v1;
-        char padding[HEADER_SIZE - 64 - 20];
-    } extra;
-} QEMU_PACKED;
-
 typedef struct BDRVBochsState {
     CoMutex lock;
     uint32_t *catalog_bitmap;
@@ -79,23 +45,6 @@ typedef struct BDRVBochsState {
     uint32_t extent_size;
 } BDRVBochsState;
 
-static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    const struct bochs_header *bochs = (const void *)buf;
-
-    if (buf_size < HEADER_SIZE)
-	return 0;
-
-    if (!strcmp(bochs->magic, HEADER_MAGIC) &&
-	!strcmp(bochs->type, REDOLOG_TYPE) &&
-	!strcmp(bochs->subtype, GROWING_TYPE) &&
-	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
-	(le32_to_cpu(bochs->version) == HEADER_V1)))
-	return 100;
-
-    return 0;
-}
-
 static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
diff --git a/block/probe/bochs.c b/block/probe/bochs.c
new file mode 100644
index 0000000..8adc09f
--- /dev/null
+++ b/block/probe/bochs.c
@@ -0,0 +1,21 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/bochs.h"
+
+int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const struct bochs_header *bochs = (const void *)buf;
+
+    if (buf_size < HEADER_SIZE)
+	return 0;
+
+    if (!strcmp(bochs->magic, HEADER_MAGIC) &&
+	!strcmp(bochs->type, REDOLOG_TYPE) &&
+	!strcmp(bochs->subtype, GROWING_TYPE) &&
+	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
+	(le32_to_cpu(bochs->version) == HEADER_V1)))
+	return 100;
+
+    return 0;
+}
diff --git a/include/block/driver/bochs.h b/include/block/driver/bochs.h
new file mode 100644
index 0000000..cd87256
--- /dev/null
+++ b/include/block/driver/bochs.h
@@ -0,0 +1,40 @@
+#ifndef BOCHS_H
+#define BOCHS_H
+
+#define HEADER_MAGIC "Bochs Virtual HD Image"
+#define HEADER_VERSION 0x00020000
+#define HEADER_V1 0x00010000
+#define HEADER_SIZE 512
+
+#define REDOLOG_TYPE "Redolog"
+#define GROWING_TYPE "Growing"
+
+// not allocated: 0xffffffff
+
+// always little-endian
+struct bochs_header {
+    char magic[32];     /* "Bochs Virtual HD Image" */
+    char type[16];      /* "Redolog" */
+    char subtype[16];   /* "Undoable" / "Volatile" / "Growing" */
+    uint32_t version;
+    uint32_t header;    /* size of header */
+
+    uint32_t catalog;   /* num of entries */
+    uint32_t bitmap;    /* bitmap size */
+    uint32_t extent;    /* extent size */
+
+    union {
+        struct {
+            uint32_t reserved;  /* for ??? */
+            uint64_t disk;      /* disk size */
+            char padding[HEADER_SIZE - 64 - 20 - 12];
+        } QEMU_PACKED redolog;
+        struct {
+            uint64_t disk;      /* disk size */
+            char padding[HEADER_SIZE - 64 - 20 - 8];
+        } QEMU_PACKED redolog_v1;
+        char padding[HEADER_SIZE - 64 - 20];
+    } extra;
+} QEMU_PACKED;
+
+#endif
diff --git a/include/block/probe.h b/include/block/probe.h
new file mode 100644
index 0000000..6450ca1
--- /dev/null
+++ b/include/block/probe.h
@@ -0,0 +1,6 @@
+#ifndef PROBE_H
+#define PROBE_H
+
+int bochs_probe(const uint8_t *buf, int buf_size, const char *filename);
+
+#endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 05/32] blockdev: Move cloop probe to its own file

[Colin Lord]

Isolates cloop probing function as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/cloop.c         | 16 +---------------
 block/probe/cloop.c   | 17 +++++++++++++++++
 include/block/probe.h |  1 +
 4 files changed, 20 insertions(+), 16 deletions(-)
 create mode 100644 block/probe/cloop.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index bc0c2aa..3b98001 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -24,7 +24,7 @@ block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
-block-obj-y += probe/bochs.o
+block-obj-y += probe/bochs.o probe/cloop.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/cloop.c b/block/cloop.c
index ea5a92b..bf9fb75 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "qemu/module.h"
 #include "qemu/bswap.h"
 #include <zlib.h>
@@ -44,21 +45,6 @@ typedef struct BDRVCloopState {
     z_stream zstream;
 } BDRVCloopState;
 
-static int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    const char *magic_version_2_0 = "#!/bin/sh\n"
-        "#V2.0 Format\n"
-        "modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
-    int length = strlen(magic_version_2_0);
-    if (length > buf_size) {
-        length = buf_size;
-    }
-    if (!memcmp(magic_version_2_0, buf, length)) {
-        return 2;
-    }
-    return 0;
-}
-
 static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
diff --git a/block/probe/cloop.c b/block/probe/cloop.c
new file mode 100644
index 0000000..955c29c
--- /dev/null
+++ b/block/probe/cloop.c
@@ -0,0 +1,17 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const char *magic_version_2_0 = "#!/bin/sh\n"
+        "#V2.0 Format\n"
+        "modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
+    int length = strlen(magic_version_2_0);
+    if (length > buf_size) {
+        length = buf_size;
+    }
+    if (!memcmp(magic_version_2_0, buf, length)) {
+        return 2;
+    }
+    return 0;
+}
diff --git a/include/block/probe.h b/include/block/probe.h
index 6450ca1..ed1a60b 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -2,5 +2,6 @@
 #define PROBE_H
 
 int bochs_probe(const uint8_t *buf, int buf_size, const char *filename);
+int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 06/32] blockdev: Move luks probe to its own file

[Colin Lord]

Isolates the luks probe function as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/crypto.c        | 21 +--------------------
 block/probe/luks.c    | 23 +++++++++++++++++++++++
 include/block/probe.h |  2 ++
 4 files changed, 27 insertions(+), 21 deletions(-)
 create mode 100644 block/probe/luks.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 3b98001..28a7ec3 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -24,7 +24,7 @@ block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
-block-obj-y += probe/bochs.o probe/cloop.o
+block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/crypto.c b/block/crypto.c
index 758e14e..493dd69 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "sysemu/block-backend.h"
 #include "crypto/block.h"
 #include "qapi/opts-visitor.h"
@@ -41,19 +42,6 @@ struct BlockCrypto {
 };
 
 
-static int block_crypto_probe_generic(QCryptoBlockFormat format,
-                                      const uint8_t *buf,
-                                      int buf_size,
-                                      const char *filename)
-{
-    if (qcrypto_block_has_format(format, buf, buf_size)) {
-        return 100;
-    } else {
-        return 0;
-    }
-}
-
-
 static ssize_t block_crypto_read_func(QCryptoBlock *block,
                                       size_t offset,
                                       uint8_t *buf,
@@ -540,13 +528,6 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
 }
 
 
-static int block_crypto_probe_luks(const uint8_t *buf,
-                                   int buf_size,
-                                   const char *filename) {
-    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
-                                      buf, buf_size, filename);
-}
-
 static int block_crypto_open_luks(BlockDriverState *bs,
                                   QDict *options,
                                   int flags,
diff --git a/block/probe/luks.c b/block/probe/luks.c
new file mode 100644
index 0000000..5c6427a
--- /dev/null
+++ b/block/probe/luks.c
@@ -0,0 +1,23 @@
+#include "qemu/osdep.h"
+#include "qapi-types.h"
+#include "block/probe.h"
+#include "crypto/block.h"
+
+static int block_crypto_probe_generic(QCryptoBlockFormat format,
+                                      const uint8_t *buf,
+                                      int buf_size,
+                                      const char *filename)
+{
+    if (qcrypto_block_has_format(format, buf, buf_size)) {
+        return 100;
+    } else {
+        return 0;
+    }
+}
+
+int block_crypto_probe_luks(const uint8_t *buf,
+                                   int buf_size,
+                                   const char *filename) {
+    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
+                                      buf, buf_size, filename);
+}
diff --git a/include/block/probe.h b/include/block/probe.h
index ed1a60b..35a8d00 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -3,5 +3,7 @@
 
 int bochs_probe(const uint8_t *buf, int buf_size, const char *filename);
 int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
+int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
+                            const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 07/32] blockdev: Move dmg probe to its own file

[Colin Lord]

Isolate dmg probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/dmg.c           | 16 +---------------
 block/probe/dmg.c     | 17 +++++++++++++++++
 include/block/probe.h |  1 +
 4 files changed, 20 insertions(+), 16 deletions(-)
 create mode 100644 block/probe/dmg.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 28a7ec3..1d744eb 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -24,7 +24,7 @@ block-obj-y += accounting.o dirty-bitmap.o
 block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
-block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o
+block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/dmg.c b/block/dmg.c
index 06eb513..2c70687 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "qemu/bswap.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
@@ -66,21 +67,6 @@ typedef struct BDRVDMGState {
 #endif
 } BDRVDMGState;
 
-static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    int len;
-
-    if (!filename) {
-        return 0;
-    }
-
-    len = strlen(filename);
-    if (len > 4 && !strcmp(filename + len - 4, ".dmg")) {
-        return 2;
-    }
-    return 0;
-}
-
 static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result)
 {
     uint64_t buffer;
diff --git a/block/probe/dmg.c b/block/probe/dmg.c
new file mode 100644
index 0000000..a40281b
--- /dev/null
+++ b/block/probe/dmg.c
@@ -0,0 +1,17 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    int len;
+
+    if (!filename) {
+        return 0;
+    }
+
+    len = strlen(filename);
+    if (len > 4 && !strcmp(filename + len - 4, ".dmg")) {
+        return 2;
+    }
+    return 0;
+}
diff --git a/include/block/probe.h b/include/block/probe.h
index 35a8d00..267431d 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -5,5 +5,6 @@ int bochs_probe(const uint8_t *buf, int buf_size, const char *filename);
 int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
 int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                             const char *filename);
+int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 08/32] blockdev: Move parallels probe to its own file

[Colin Lord]

Isolate parallels probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs              |  1 +
 block/parallels.c                | 43 ++--------------------------------------
 block/probe/parallels.c          | 22 ++++++++++++++++++++
 include/block/driver/parallels.h | 26 ++++++++++++++++++++++++
 include/block/probe.h            |  1 +
 5 files changed, 52 insertions(+), 41 deletions(-)
 create mode 100644 block/probe/parallels.c
 create mode 100644 include/block/driver/parallels.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 1d744eb..12607db 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -25,6 +25,7 @@ block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
+block-obj-y += probe/parallels.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/parallels.c b/block/parallels.c
index d6a1a61..547373e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -31,6 +31,8 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/parallels.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/bswap.h"
@@ -39,29 +41,6 @@
 
 /**************************************************************/
 
-#define HEADER_MAGIC "WithoutFreeSpace"
-#define HEADER_MAGIC2 "WithouFreSpacExt"
-#define HEADER_VERSION 2
-#define HEADER_INUSE_MAGIC  (0x746F6E59)
-
-#define DEFAULT_CLUSTER_SIZE 1048576        /* 1 MiB */
-
-
-// always little-endian
-typedef struct ParallelsHeader {
-    char magic[16]; // "WithoutFreeSpace"
-    uint32_t version;
-    uint32_t heads;
-    uint32_t cylinders;
-    uint32_t tracks;
-    uint32_t bat_entries;
-    uint64_t nb_sectors;
-    uint32_t inuse;
-    uint32_t data_off;
-    char padding[12];
-} QEMU_PACKED ParallelsHeader;
-
-
 typedef enum ParallelsPreallocMode {
     PRL_PREALLOC_MODE_FALLOCATE = 0,
     PRL_PREALLOC_MODE_TRUNCATE = 1,
@@ -536,24 +515,6 @@ exit:
 }
 
 
-static int parallels_probe(const uint8_t *buf, int buf_size,
-                           const char *filename)
-{
-    const ParallelsHeader *ph = (const void *)buf;
-
-    if (buf_size < sizeof(ParallelsHeader)) {
-        return 0;
-    }
-
-    if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
-           !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
-           (le32_to_cpu(ph->version) == HEADER_VERSION)) {
-        return 100;
-    }
-
-    return 0;
-}
-
 static int parallels_update_header(BlockDriverState *bs)
 {
     BDRVParallelsState *s = bs->opaque;
diff --git a/block/probe/parallels.c b/block/probe/parallels.c
new file mode 100644
index 0000000..66cddea
--- /dev/null
+++ b/block/probe/parallels.c
@@ -0,0 +1,22 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/parallels.h"
+
+int parallels_probe(const uint8_t *buf, int buf_size,
+                           const char *filename)
+{
+    const ParallelsHeader *ph = (const void *)buf;
+
+    if (buf_size < sizeof(ParallelsHeader)) {
+        return 0;
+    }
+
+    if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
+           !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
+           (le32_to_cpu(ph->version) == HEADER_VERSION)) {
+        return 100;
+    }
+
+    return 0;
+}
diff --git a/include/block/driver/parallels.h b/include/block/driver/parallels.h
new file mode 100644
index 0000000..512ef5f
--- /dev/null
+++ b/include/block/driver/parallels.h
@@ -0,0 +1,26 @@
+#ifndef PARALLELS_H
+#define PARALLELS_H
+
+#define HEADER_MAGIC "WithoutFreeSpace"
+#define HEADER_MAGIC2 "WithouFreSpacExt"
+#define HEADER_VERSION 2
+#define HEADER_INUSE_MAGIC  (0x746F6E59)
+
+#define DEFAULT_CLUSTER_SIZE 1048576        /* 1 MiB */
+
+
+// always little-endian
+typedef struct ParallelsHeader {
+    char magic[16]; // "WithoutFreeSpace"
+    uint32_t version;
+    uint32_t heads;
+    uint32_t cylinders;
+    uint32_t tracks;
+    uint32_t bat_entries;
+    uint64_t nb_sectors;
+    uint32_t inuse;
+    uint32_t data_off;
+    char padding[12];
+} QEMU_PACKED ParallelsHeader;
+
+#endif
diff --git a/include/block/probe.h b/include/block/probe.h
index 267431d..f8b0984 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -6,5 +6,6 @@ int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
 int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                             const char *filename);
 int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
+int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 09/32] blockdev: Move qcow probe to its own file

[Colin Lord]

Isolates qcow probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs         |  2 +-
 block/probe/qcow.c          | 16 ++++++++++++++++
 block/qcow.c                | 31 ++-----------------------------
 include/block/driver/qcow.h | 21 +++++++++++++++++++++
 include/block/probe.h       |  1 +
 5 files changed, 41 insertions(+), 30 deletions(-)
 create mode 100644 block/probe/qcow.c
 create mode 100644 include/block/driver/qcow.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 12607db..9458da8 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -25,7 +25,7 @@ block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
-block-obj-y += probe/parallels.o
+block-obj-y += probe/parallels.o probe/qcow.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/qcow.c b/block/probe/qcow.c
new file mode 100644
index 0000000..6024d11
--- /dev/null
+++ b/block/probe/qcow.c
@@ -0,0 +1,16 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/qcow.h"
+
+int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const QCowHeader *cow_header = (const void *)buf;
+
+    if (buf_size >= sizeof(QCowHeader) &&
+        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
+        be32_to_cpu(cow_header->version) == QCOW_VERSION)
+        return 100;
+    else
+        return 0;
+}
diff --git a/block/qcow.c b/block/qcow.c
index 312af52..828749c 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -26,6 +26,8 @@
 #include "qemu-common.h"
 #include "qemu/error-report.h"
 #include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/qcow.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/bswap.h"
@@ -37,28 +39,11 @@
 /**************************************************************/
 /* QEMU COW block driver with compression and encryption support */
 
-#define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
-#define QCOW_VERSION 1
-
 #define QCOW_CRYPT_NONE 0
 #define QCOW_CRYPT_AES  1
 
 #define QCOW_OFLAG_COMPRESSED (1LL << 63)
 
-typedef struct QCowHeader {
-    uint32_t magic;
-    uint32_t version;
-    uint64_t backing_file_offset;
-    uint32_t backing_file_size;
-    uint32_t mtime;
-    uint64_t size; /* in bytes */
-    uint8_t cluster_bits;
-    uint8_t l2_bits;
-    uint16_t padding;
-    uint32_t crypt_method;
-    uint64_t l1_table_offset;
-} QEMU_PACKED QCowHeader;
-
 #define L2_CACHE_SIZE 16
 
 typedef struct BDRVQcowState {
@@ -85,18 +70,6 @@ typedef struct BDRVQcowState {
 
 static int decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 
-static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    const QCowHeader *cow_header = (const void *)buf;
-
-    if (buf_size >= sizeof(QCowHeader) &&
-        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
-        be32_to_cpu(cow_header->version) == QCOW_VERSION)
-        return 100;
-    else
-        return 0;
-}
-
 static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
                      Error **errp)
 {
diff --git a/include/block/driver/qcow.h b/include/block/driver/qcow.h
new file mode 100644
index 0000000..c96ea24
--- /dev/null
+++ b/include/block/driver/qcow.h
@@ -0,0 +1,21 @@
+#ifndef QCOW_H
+#define QCOW_H
+
+#define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
+#define QCOW_VERSION 1
+
+typedef struct QCowHeader {
+    uint32_t magic;
+    uint32_t version;
+    uint64_t backing_file_offset;
+    uint32_t backing_file_size;
+    uint32_t mtime;
+    uint64_t size; /* in bytes */
+    uint8_t cluster_bits;
+    uint8_t l2_bits;
+    uint16_t padding;
+    uint32_t crypt_method;
+    uint64_t l1_table_offset;
+} QEMU_PACKED QCowHeader;
+
+#endif
diff --git a/include/block/probe.h b/include/block/probe.h
index f8b0984..5230da4 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -7,5 +7,6 @@ int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                             const char *filename);
 int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
+int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 10/32] blockdev: Move qcow2 probe to its own file

[Colin Lord]

Isolates qcow2 probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/probe/qcow2.c   | 16 ++++++++++++++++
 block/qcow2.c         | 13 +------------
 include/block/probe.h |  1 +
 4 files changed, 19 insertions(+), 13 deletions(-)
 create mode 100644 block/probe/qcow2.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 9458da8..23240d9 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -25,7 +25,7 @@ block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
-block-obj-y += probe/parallels.o probe/qcow.o
+block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/qcow2.c b/block/probe/qcow2.c
new file mode 100644
index 0000000..56f4e82
--- /dev/null
+++ b/block/probe/qcow2.c
@@ -0,0 +1,16 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/qcow2.h"
+
+int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const QCowHeader *cow_header = (const void *)buf;
+
+    if (buf_size >= sizeof(QCowHeader) &&
+        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
+        be32_to_cpu(cow_header->version) >= 2)
+        return 100;
+    else
+        return 0;
+}
diff --git a/block/qcow2.c b/block/qcow2.c
index 23f666d..55639eb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -27,6 +27,7 @@
 #include "qemu/module.h"
 #include <zlib.h>
 #include "block/qcow2.h"
+#include "block/probe.h"
 #include "qemu/error-report.h"
 #include "qapi/qmp/qerror.h"
 #include "qapi/qmp/qbool.h"
@@ -64,18 +65,6 @@ typedef struct {
 #define  QCOW2_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 #define  QCOW2_EXT_MAGIC_FEATURE_TABLE 0x6803f857
 
-static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    const QCowHeader *cow_header = (const void *)buf;
-
-    if (buf_size >= sizeof(QCowHeader) &&
-        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
-        be32_to_cpu(cow_header->version) >= 2)
-        return 100;
-    else
-        return 0;
-}
-
 
 /* 
  * read qcow2 extension and fill bs
diff --git a/include/block/probe.h b/include/block/probe.h
index 5230da4..f9dd36e 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -8,5 +8,6 @@ int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
 int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
+int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 11/32] blockdev: Move qed probe to its own file

[Colin Lord]

Isolate qed probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/probe/qed.c     | 18 ++++++++++++++++++
 block/qed.c           | 15 +--------------
 include/block/probe.h |  1 +
 4 files changed, 21 insertions(+), 15 deletions(-)
 create mode 100644 block/probe/qed.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 23240d9..6ed2689 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -25,7 +25,7 @@ block-obj-y += write-threshold.o
 
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
-block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o
+block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/qed.c b/block/probe/qed.c
new file mode 100644
index 0000000..c902489
--- /dev/null
+++ b/block/probe/qed.c
@@ -0,0 +1,18 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/qed.h"
+
+int bdrv_qed_probe(const uint8_t *buf, int buf_size,
+                          const char *filename)
+{
+    const QEDHeader *header = (const QEDHeader *)buf;
+
+    if (buf_size < sizeof(*header)) {
+        return 0;
+    }
+    if (le32_to_cpu(header->magic) != QED_MAGIC) {
+        return 0;
+    }
+    return 100;
+}
diff --git a/block/qed.c b/block/qed.c
index 1206806..21d8982 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -16,6 +16,7 @@
 #include "qapi/error.h"
 #include "qemu/timer.h"
 #include "qemu/bswap.h"
+#include "block/probe.h"
 #include "trace.h"
 #include "qed.h"
 #include "qapi/qmp/qerror.h"
@@ -26,20 +27,6 @@ static const AIOCBInfo qed_aiocb_info = {
     .aiocb_size         = sizeof(QEDAIOCB),
 };
 
-static int bdrv_qed_probe(const uint8_t *buf, int buf_size,
-                          const char *filename)
-{
-    const QEDHeader *header = (const QEDHeader *)buf;
-
-    if (buf_size < sizeof(*header)) {
-        return 0;
-    }
-    if (le32_to_cpu(header->magic) != QED_MAGIC) {
-        return 0;
-    }
-    return 100;
-}
-
 /**
  * Check whether an image format is raw
  *
diff --git a/include/block/probe.h b/include/block/probe.h
index f9dd36e..e3e9934 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -9,5 +9,6 @@ int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
+int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 12/32] blockdev: Move raw probe to its own file

[Colin Lord]

Isolate raw probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  1 +
 block/probe/raw.c     | 10 ++++++++++
 block/raw_bsd.c       |  9 +--------
 include/block/probe.h |  1 +
 4 files changed, 13 insertions(+), 8 deletions(-)
 create mode 100644 block/probe/raw.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 6ed2689..693cc97 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -26,6 +26,7 @@ block-obj-y += write-threshold.o
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
+block-obj-y += probe/raw.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/raw.c b/block/probe/raw.c
new file mode 100644
index 0000000..22c6bcb
--- /dev/null
+++ b/block/probe/raw.c
@@ -0,0 +1,10 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    /* smallest possible positive score so that raw is used if and only if no
+     * other block driver works
+     */
+    return 1;
+}
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 7f63791..8f49637 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -28,6 +28,7 @@
 
 #include "qemu/osdep.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "qapi/error.h"
 #include "qemu/option.h"
 
@@ -218,14 +219,6 @@ static void raw_close(BlockDriverState *bs)
 {
 }
 
-static int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    /* smallest possible positive score so that raw is used if and only if no
-     * other block driver works
-     */
-    return 1;
-}
-
 static int raw_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz)
 {
     return bdrv_probe_blocksizes(bs->file->bs, bsz);
diff --git a/include/block/probe.h b/include/block/probe.h
index e3e9934..053f961 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -10,5 +10,6 @@ int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
+int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 13/32] blockdev: Move vdi probe to its own file

[Colin Lord]

Isolates vdi probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs        |  2 +-
 block/probe/vdi.c          | 26 +++++++++++++++++
 block/vdi.c                | 69 ++--------------------------------------------
 include/block/driver/vdi.h | 49 ++++++++++++++++++++++++++++++++
 include/block/probe.h      |  1 +
 5 files changed, 79 insertions(+), 68 deletions(-)
 create mode 100644 block/probe/vdi.c
 create mode 100644 include/block/driver/vdi.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 693cc97..4a5bd88 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -26,7 +26,7 @@ block-obj-y += write-threshold.o
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
-block-obj-y += probe/raw.o
+block-obj-y += probe/raw.o probe/vdi.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/vdi.c b/block/probe/vdi.c
new file mode 100644
index 0000000..9adf5e5
--- /dev/null
+++ b/block/probe/vdi.c
@@ -0,0 +1,26 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/vdi.h"
+
+int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    const VdiHeader *header = (const VdiHeader *)buf;
+    int ret = 0;
+
+    logout("\n");
+
+    if (buf_size < sizeof(*header)) {
+        /* Header too small, no VDI. */
+    } else if (le32_to_cpu(header->signature) == VDI_SIGNATURE) {
+        ret = 100;
+    }
+
+    if (ret == 0) {
+        logout("no vdi image\n");
+    } else {
+        logout("%s", header->text);
+    }
+
+    return ret;
+}
diff --git a/block/vdi.c b/block/vdi.c
index 7d9ab9c..e99ae92 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -52,6 +52,8 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/vdi.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/bswap.h"
@@ -59,14 +61,6 @@
 #include "qemu/coroutine.h"
 #include "qemu/cutils.h"
 
-#if defined(CONFIG_UUID)
-#include <uuid/uuid.h>
-#else
-/* TODO: move uuid emulation to some central place in QEMU. */
-#include "sysemu/sysemu.h"     /* UUID_FMT */
-typedef unsigned char uuid_t[16];
-#endif
-
 /* Code configuration options. */
 
 /* Enable debug messages. */
@@ -92,16 +86,6 @@ typedef unsigned char uuid_t[16];
 #define SECTOR_SIZE 512
 #define DEFAULT_CLUSTER_SIZE (1 * MiB)
 
-#if defined(CONFIG_VDI_DEBUG)
-#define logout(fmt, ...) \
-                fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
-#else
-#define logout(fmt, ...) ((void)0)
-#endif
-
-/* Image signature. */
-#define VDI_SIGNATURE 0xbeda107f
-
 /* Image version. */
 #define VDI_VERSION_1_1 0x00010001
 
@@ -163,33 +147,6 @@ static inline void uuid_unparse(const uuid_t uu, char *out)
 #endif
 
 typedef struct {
-    char text[0x40];
-    uint32_t signature;
-    uint32_t version;
-    uint32_t header_size;
-    uint32_t image_type;
-    uint32_t image_flags;
-    char description[256];
-    uint32_t offset_bmap;
-    uint32_t offset_data;
-    uint32_t cylinders;         /* disk geometry, unused here */
-    uint32_t heads;             /* disk geometry, unused here */
-    uint32_t sectors;           /* disk geometry, unused here */
-    uint32_t sector_size;
-    uint32_t unused1;
-    uint64_t disk_size;
-    uint32_t block_size;
-    uint32_t block_extra;       /* unused here */
-    uint32_t blocks_in_image;
-    uint32_t blocks_allocated;
-    uuid_t uuid_image;
-    uuid_t uuid_last_snap;
-    uuid_t uuid_link;
-    uuid_t uuid_parent;
-    uint64_t unused2[7];
-} QEMU_PACKED VdiHeader;
-
-typedef struct {
     /* The block map entries are little endian (even in memory). */
     uint32_t *bmap;
     /* Size of block (bytes). */
@@ -371,28 +328,6 @@ static int vdi_make_empty(BlockDriverState *bs)
     return 0;
 }
 
-static int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    const VdiHeader *header = (const VdiHeader *)buf;
-    int ret = 0;
-
-    logout("\n");
-
-    if (buf_size < sizeof(*header)) {
-        /* Header too small, no VDI. */
-    } else if (le32_to_cpu(header->signature) == VDI_SIGNATURE) {
-        ret = 100;
-    }
-
-    if (ret == 0) {
-        logout("no vdi image\n");
-    } else {
-        logout("%s", header->text);
-    }
-
-    return ret;
-}
-
 static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
diff --git a/include/block/driver/vdi.h b/include/block/driver/vdi.h
new file mode 100644
index 0000000..4020c94
--- /dev/null
+++ b/include/block/driver/vdi.h
@@ -0,0 +1,49 @@
+#ifndef VDI_H
+#define VDI_H
+
+#if defined(CONFIG_UUID)
+#include <uuid/uuid.h>
+#else
+/* TODO: move uuid emulation to some central place in QEMU. */
+#include "sysemu/sysemu.h"     /* UUID_FMT */
+typedef unsigned char uuid_t[16];
+#endif
+
+#if defined(CONFIG_VDI_DEBUG)
+#define logout(fmt, ...) \
+                fprintf(stderr, "vdi\t%-24s" fmt, __func__, ##__VA_ARGS__)
+#else
+#define logout(fmt, ...) ((void)0)
+#endif
+
+/* Image signature. */
+#define VDI_SIGNATURE 0xbeda107f
+
+typedef struct {
+    char text[0x40];
+    uint32_t signature;
+    uint32_t version;
+    uint32_t header_size;
+    uint32_t image_type;
+    uint32_t image_flags;
+    char description[256];
+    uint32_t offset_bmap;
+    uint32_t offset_data;
+    uint32_t cylinders;         /* disk geometry, unused here */
+    uint32_t heads;             /* disk geometry, unused here */
+    uint32_t sectors;           /* disk geometry, unused here */
+    uint32_t sector_size;
+    uint32_t unused1;
+    uint64_t disk_size;
+    uint32_t block_size;
+    uint32_t block_extra;       /* unused here */
+    uint32_t blocks_in_image;
+    uint32_t blocks_allocated;
+    uuid_t uuid_image;
+    uuid_t uuid_last_snap;
+    uuid_t uuid_link;
+    uuid_t uuid_parent;
+    uint64_t unused2[7];
+} QEMU_PACKED VdiHeader;
+
+#endif
diff --git a/include/block/probe.h b/include/block/probe.h
index 053f961..f85c178 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -11,5 +11,6 @@ int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
+int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 14/32] blockdev: Move vhdx probe to its own file

[Colin Lord]

Isolates vhdx probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   |  2 +-
 block/probe/vhdx.c    | 21 +++++++++++++++++++++
 block/vhdx.c          | 20 +-------------------
 include/block/probe.h |  1 +
 4 files changed, 24 insertions(+), 20 deletions(-)
 create mode 100644 block/probe/vhdx.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 4a5bd88..89e0da4 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -26,7 +26,7 @@ block-obj-y += write-threshold.o
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
-block-obj-y += probe/raw.o probe/vdi.o
+block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/vhdx.c b/block/probe/vhdx.c
new file mode 100644
index 0000000..6c38aac
--- /dev/null
+++ b/block/probe/vhdx.c
@@ -0,0 +1,21 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+/*
+ * Per the MS VHDX Specification, for every VHDX file:
+ *      - The header section is fixed size - 1 MB
+ *      - The header section is always the first "object"
+ *      - The first 64KB of the header is the File Identifier
+ *      - The first uint64 (8 bytes) is the VHDX Signature ("vhdxfile")
+ *      - The following 512 bytes constitute a UTF-16 string identifiying the
+ *        software that created the file, and is optional and diagnostic only.
+ *
+ *  Therefore, we probe by looking for the vhdxfile signature "vhdxfile"
+ */
+int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    if (buf_size >= 8 && !memcmp(buf, "vhdxfile", 8)) {
+        return 100;
+    }
+    return 0;
+}
diff --git a/block/vhdx.c b/block/vhdx.c
index f5605a2..ba8adfe 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -19,6 +19,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "qemu/crc32c.h"
@@ -273,25 +274,6 @@ static void vhdx_set_shift_bits(BDRVVHDXState *s)
 }
 
 /*
- * Per the MS VHDX Specification, for every VHDX file:
- *      - The header section is fixed size - 1 MB
- *      - The header section is always the first "object"
- *      - The first 64KB of the header is the File Identifier
- *      - The first uint64 (8 bytes) is the VHDX Signature ("vhdxfile")
- *      - The following 512 bytes constitute a UTF-16 string identifiying the
- *        software that created the file, and is optional and diagnostic only.
- *
- *  Therefore, we probe by looking for the vhdxfile signature "vhdxfile"
- */
-static int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    if (buf_size >= 8 && !memcmp(buf, "vhdxfile", 8)) {
-        return 100;
-    }
-    return 0;
-}
-
-/*
  * Writes the header to the specified offset.
  *
  * This will optionally read in buffer data from disk (otherwise zero-fill),
diff --git a/include/block/probe.h b/include/block/probe.h
index f85c178..e901d8f 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -12,5 +12,6 @@ int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
+int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 15/32] blockdev: Move vmdk probe to its own file

[Colin Lord]

Isolates vmdk probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs         |  2 +-
 block/probe/vmdk.c          | 60 +++++++++++++++++++++++++++++++++++++++++++++
 block/vmdk.c                | 60 ++-------------------------------------------
 include/block/driver/vmdk.h |  7 ++++++
 include/block/probe.h       |  1 +
 5 files changed, 71 insertions(+), 59 deletions(-)
 create mode 100644 block/probe/vmdk.c
 create mode 100644 include/block/driver/vmdk.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 89e0da4..4cecf68 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -26,7 +26,7 @@ block-obj-y += write-threshold.o
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
-block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o
+block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o probe/vmdk.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/vmdk.c b/block/probe/vmdk.c
new file mode 100644
index 0000000..cb9bd3d
--- /dev/null
+++ b/block/probe/vmdk.c
@@ -0,0 +1,60 @@
+#include "qemu/osdep.h"
+#include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/vmdk.h"
+
+int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    uint32_t magic;
+
+    if (buf_size < 4) {
+        return 0;
+    }
+    magic = be32_to_cpu(*(uint32_t *)buf);
+    if (magic == VMDK3_MAGIC ||
+        magic == VMDK4_MAGIC) {
+        return 100;
+    } else {
+        const char *p = (const char *)buf;
+        const char *end = p + buf_size;
+        while (p < end) {
+            if (*p == '#') {
+                /* skip comment line */
+                while (p < end && *p != '\n') {
+                    p++;
+                }
+                p++;
+                continue;
+            }
+            if (*p == ' ') {
+                while (p < end && *p == ' ') {
+                    p++;
+                }
+                /* skip '\r' if windows line endings used. */
+                if (p < end && *p == '\r') {
+                    p++;
+                }
+                /* only accept blank lines before 'version=' line */
+                if (p == end || *p != '\n') {
+                    return 0;
+                }
+                p++;
+                continue;
+            }
+            if (end - p >= strlen("version=X\n")) {
+                if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
+                    strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
+                    return 100;
+                }
+            }
+            if (end - p >= strlen("version=X\r\n")) {
+                if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
+                    strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
+                    return 100;
+                }
+            }
+            return 0;
+        }
+        return 0;
+    }
+}
diff --git a/block/vmdk.c b/block/vmdk.c
index 2901692..d22db95 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -26,6 +26,8 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "block/block_int.h"
+#include "block/probe.h"
+#include "block/driver/vmdk.h"
 #include "sysemu/block-backend.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/error-report.h"
@@ -35,8 +37,6 @@
 #include "qemu/cutils.h"
 #include <zlib.h>
 
-#define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
-#define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
 #define VMDK4_COMPRESSION_DEFLATE 1
 #define VMDK4_FLAG_NL_DETECT (1 << 0)
 #define VMDK4_FLAG_RGD (1 << 1)
@@ -151,62 +151,6 @@ enum {
     MARKER_FOOTER           = 3,
 };
 
-static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    uint32_t magic;
-
-    if (buf_size < 4) {
-        return 0;
-    }
-    magic = be32_to_cpu(*(uint32_t *)buf);
-    if (magic == VMDK3_MAGIC ||
-        magic == VMDK4_MAGIC) {
-        return 100;
-    } else {
-        const char *p = (const char *)buf;
-        const char *end = p + buf_size;
-        while (p < end) {
-            if (*p == '#') {
-                /* skip comment line */
-                while (p < end && *p != '\n') {
-                    p++;
-                }
-                p++;
-                continue;
-            }
-            if (*p == ' ') {
-                while (p < end && *p == ' ') {
-                    p++;
-                }
-                /* skip '\r' if windows line endings used. */
-                if (p < end && *p == '\r') {
-                    p++;
-                }
-                /* only accept blank lines before 'version=' line */
-                if (p == end || *p != '\n') {
-                    return 0;
-                }
-                p++;
-                continue;
-            }
-            if (end - p >= strlen("version=X\n")) {
-                if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
-                    strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
-                    return 100;
-                }
-            }
-            if (end - p >= strlen("version=X\r\n")) {
-                if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
-                    strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
-                    return 100;
-                }
-            }
-            return 0;
-        }
-        return 0;
-    }
-}
-
 #define SECTOR_SIZE 512
 #define DESC_SIZE (20 * SECTOR_SIZE)    /* 20 sectors of 512 bytes each */
 #define BUF_SIZE 4096
diff --git a/include/block/driver/vmdk.h b/include/block/driver/vmdk.h
new file mode 100644
index 0000000..d631468
--- /dev/null
+++ b/include/block/driver/vmdk.h
@@ -0,0 +1,7 @@
+#ifndef VMDK_H
+#define VMDK_H
+
+#define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
+#define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
+
+#endif
diff --git a/include/block/probe.h b/include/block/probe.h
index e901d8f..392515d 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -13,5 +13,6 @@ int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
+int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 16/32] blockdev: Move vpc probe to its own file

[Colin Lord]

Isolates vpc probe as part of the modularization process.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs   | 2 +-
 block/probe/vpc.c     | 9 +++++++++
 block/vpc.c           | 8 +-------
 include/block/probe.h | 1 +
 4 files changed, 12 insertions(+), 8 deletions(-)
 create mode 100644 block/probe/vpc.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 4cecf68..2d2691e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -26,7 +26,7 @@ block-obj-y += write-threshold.o
 block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
-block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o probe/vmdk.o
+block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o probe/vmdk.o probe/vpc.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/vpc.c b/block/probe/vpc.c
new file mode 100644
index 0000000..afe8271
--- /dev/null
+++ b/block/probe/vpc.c
@@ -0,0 +1,9 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
+{
+    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
+	return 100;
+    return 0;
+}
diff --git a/block/vpc.c b/block/vpc.c
index 076a7ce..cb65022 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -26,6 +26,7 @@
 #include "qapi/error.h"
 #include "qemu-common.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "sysemu/block-backend.h"
 #include "qemu/module.h"
 #include "migration/migration.h"
@@ -179,13 +180,6 @@ static uint32_t vpc_checksum(uint8_t* buf, size_t size)
 }
 
 
-static int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
-{
-    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
-	return 100;
-    return 0;
-}
-
 static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts,
                               Error **errp)
 {
diff --git a/include/block/probe.h b/include/block/probe.h
index 392515d..6cf878b 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -14,5 +14,6 @@ int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
+int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

[Colin Lord]

Modifies the bochs probe to return the format name as well as the
score as the final step of separating the probe function from the
driver. This keeps the probe completely independent of the driver,
making future modularization easier to accomplish. Returning the format
name as well as the score allows the score to be correlated to the
driver without the probe function needing to be part of the driver.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               | 19 +++++++++++++++++++
 block/bochs.c         |  1 -
 block/probe/bochs.c   | 25 ++++++++++++++++---------
 include/block/probe.h |  3 ++-
 4 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/block.c b/block.c
index 88a05b2..eab8a6e 100644
--- a/block.c
+++ b/block.c
@@ -25,6 +25,7 @@
 #include "trace.h"
 #include "block/block_int.h"
 #include "block/blockjob.h"
+#include "block/probe.h"
 #include "qemu/error-report.h"
 #include "module_block.h"
 #include "qemu/module.h"
@@ -56,6 +57,13 @@
 
 #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
 
+typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
+                                  const char *filename, int *score);
+
+static BdrvProbeFunc *format_probes[] = {
+    bochs_probe,
+};
+
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
     QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
 
@@ -576,6 +584,8 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
                             const char *filename)
 {
     int score_max = 0, score;
+    const char *format_max = NULL;
+    const char *format;
     size_t i;
     BlockDriver *drv = NULL, *d;
 
@@ -595,6 +605,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
         }
     }
 
+    for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
+        format = format_probes[i](buf, buf_size, filename, &score);
+        if (score > score_max) {
+            score_max = score;
+            format_max = format;
+            drv = bdrv_find_format(format_max);
+        }
+    }
+
     return drv;
 }
 
diff --git a/block/bochs.c b/block/bochs.c
index 11da0fd..5c94bc6 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -230,7 +230,6 @@ static void bochs_close(BlockDriverState *bs)
 static BlockDriver bdrv_bochs = {
     .format_name	= "bochs",
     .instance_size	= sizeof(BDRVBochsState),
-    .bdrv_probe		= bochs_probe,
     .bdrv_open		= bochs_open,
     .bdrv_co_preadv = bochs_co_preadv,
     .bdrv_close		= bochs_close,
diff --git a/block/probe/bochs.c b/block/probe/bochs.c
index 8adc09f..8206930 100644
--- a/block/probe/bochs.c
+++ b/block/probe/bochs.c
@@ -3,19 +3,26 @@
 #include "block/probe.h"
 #include "block/driver/bochs.h"
 
-int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score)
 {
+    const char *format = "bochs";
     const struct bochs_header *bochs = (const void *)buf;
+    assert(score);
+    *score = 0;
 
-    if (buf_size < HEADER_SIZE)
-	return 0;
+    if (buf_size < HEADER_SIZE) {
+        return format;
+    }
 
     if (!strcmp(bochs->magic, HEADER_MAGIC) &&
-	!strcmp(bochs->type, REDOLOG_TYPE) &&
-	!strcmp(bochs->subtype, GROWING_TYPE) &&
-	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
-	(le32_to_cpu(bochs->version) == HEADER_V1)))
-	return 100;
+        !strcmp(bochs->type, REDOLOG_TYPE) &&
+        !strcmp(bochs->subtype, GROWING_TYPE) &&
+        ((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
+        (le32_to_cpu(bochs->version) == HEADER_V1))) {
+        *score = 100;
+        return format;
+    }
 
-    return 0;
+    return format;
 }
diff --git a/include/block/probe.h b/include/block/probe.h
index 6cf878b..13c08bd 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int bochs_probe(const uint8_t *buf, int buf_size, const char *filename);
 int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
 int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                             const char *filename);
@@ -15,5 +14,7 @@ int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
+const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 18/32] blockdev: Separate cloop probe from its driver

[Colin Lord]

Completes the separation of the cloop probe from the cloop driver. The
cloop probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/cloop.c         |  1 -
 block/probe/cloop.c   | 11 ++++++++---
 include/block/probe.h |  3 ++-
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index eab8a6e..baef612 100644
--- a/block.c
+++ b/block.c
@@ -62,6 +62,7 @@ typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
 
 static BdrvProbeFunc *format_probes[] = {
     bochs_probe,
+    cloop_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/cloop.c b/block/cloop.c
index bf9fb75..b5db80b 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -264,7 +264,6 @@ static void cloop_close(BlockDriverState *bs)
 static BlockDriver bdrv_cloop = {
     .format_name    = "cloop",
     .instance_size  = sizeof(BDRVCloopState),
-    .bdrv_probe     = cloop_probe,
     .bdrv_open      = cloop_open,
     .bdrv_co_preadv = cloop_co_preadv,
     .bdrv_close     = cloop_close,
diff --git a/block/probe/cloop.c b/block/probe/cloop.c
index 955c29c..b9c2605 100644
--- a/block/probe/cloop.c
+++ b/block/probe/cloop.c
@@ -1,17 +1,22 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
-int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *cloop_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score)
 {
+    const char *format = "cloop";
     const char *magic_version_2_0 = "#!/bin/sh\n"
         "#V2.0 Format\n"
         "modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
     int length = strlen(magic_version_2_0);
+    assert(score);
     if (length > buf_size) {
         length = buf_size;
     }
     if (!memcmp(magic_version_2_0, buf, length)) {
-        return 2;
+        *score = 2;
+        return format;
     }
-    return 0;
+    *score = 0;
+    return format;
 }
diff --git a/include/block/probe.h b/include/block/probe.h
index 13c08bd..804f77c 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int cloop_probe(const uint8_t *buf, int buf_size, const char *filename);
 int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                             const char *filename);
 int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -16,5 +15,7 @@ int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
 const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
+const char *cloop_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 19/32] blockdev: Separate luks probe from its driver

[Colin Lord]

Completes the separation of the luks probe from the crypto driver. The
luks probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/crypto.c        |  1 -
 block/probe/luks.c    | 13 ++++++++-----
 include/block/probe.h |  4 ++--
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index baef612..d936da1 100644
--- a/block.c
+++ b/block.c
@@ -63,6 +63,7 @@ typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
 static BdrvProbeFunc *format_probes[] = {
     bochs_probe,
     cloop_probe,
+    block_crypto_probe_luks,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/crypto.c b/block/crypto.c
index 493dd69..6f37aec 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -549,7 +549,6 @@ static int block_crypto_create_luks(const char *filename,
 BlockDriver bdrv_crypto_luks = {
     .format_name        = "luks",
     .instance_size      = sizeof(BlockCrypto),
-    .bdrv_probe         = block_crypto_probe_luks,
     .bdrv_open          = block_crypto_open_luks,
     .bdrv_close         = block_crypto_close,
     .bdrv_create        = block_crypto_create_luks,
diff --git a/block/probe/luks.c b/block/probe/luks.c
index 5c6427a..4c58586 100644
--- a/block/probe/luks.c
+++ b/block/probe/luks.c
@@ -15,9 +15,12 @@ static int block_crypto_probe_generic(QCryptoBlockFormat format,
     }
 }
 
-int block_crypto_probe_luks(const uint8_t *buf,
-                                   int buf_size,
-                                   const char *filename) {
-    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
-                                      buf, buf_size, filename);
+const char *block_crypto_probe_luks(const uint8_t *buf, int buf_size,
+                                    const char *filename, int *score)
+{
+    const char *format = "luks";
+    assert(score);
+    *score = block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
+                                        buf, buf_size, filename);
+    return format;
 }
diff --git a/include/block/probe.h b/include/block/probe.h
index 804f77c..e3bf04e 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,8 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int block_crypto_probe_luks(const uint8_t *buf, int buf_size,
-                            const char *filename);
 int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -17,5 +15,7 @@ const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
 const char *cloop_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
+const char *block_crypto_probe_luks(const uint8_t *buf, int buf_size,
+                                    const char *filename, int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 20/32] blockdev: Separate dmg probe from its driver

[Colin Lord]

Completes the separation of the dmg probe from the dmg driver. The dmg
probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/dmg.c           |  1 -
 block/probe/dmg.c     | 13 +++++++++----
 include/block/probe.h |  3 ++-
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index d936da1..7dbe201 100644
--- a/block.c
+++ b/block.c
@@ -64,6 +64,7 @@ static BdrvProbeFunc *format_probes[] = {
     bochs_probe,
     cloop_probe,
     block_crypto_probe_luks,
+    dmg_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/dmg.c b/block/dmg.c
index 2c70687..945cc00 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -704,7 +704,6 @@ static void dmg_close(BlockDriverState *bs)
 static BlockDriver bdrv_dmg = {
     .format_name    = "dmg",
     .instance_size  = sizeof(BDRVDMGState),
-    .bdrv_probe     = dmg_probe,
     .bdrv_open      = dmg_open,
     .bdrv_co_preadv = dmg_co_preadv,
     .bdrv_close     = dmg_close,
diff --git a/block/probe/dmg.c b/block/probe/dmg.c
index a40281b..5415b04 100644
--- a/block/probe/dmg.c
+++ b/block/probe/dmg.c
@@ -1,17 +1,22 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
-int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *dmg_probe(const uint8_t *buf, int buf_size, const char *filename,
+                     int *score)
 {
+    const char *format = "dmg";
     int len;
+    assert(score);
+    *score = 0;
 
     if (!filename) {
-        return 0;
+        return format;
     }
 
     len = strlen(filename);
     if (len > 4 && !strcmp(filename + len - 4, ".dmg")) {
-        return 2;
+        *score = 2;
+        return format;
     }
-    return 0;
+    return format;
 }
diff --git a/include/block/probe.h b/include/block/probe.h
index e3bf04e..ef6629f 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
 int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -17,5 +16,7 @@ const char *cloop_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
 const char *block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                                     const char *filename, int *score);
+const char *dmg_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 21/32] blockdev: Separate parallels probe from its driver

[Colin Lord]

Completes the separation of the parallels probe from the parallels
driver. The parallels probe now returns the format in addition to the
score, allowing correlation of the score and driver without the probe
function being part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c                 |  1 +
 block/parallels.c       |  1 -
 block/probe/parallels.c | 18 +++++++++++-------
 include/block/probe.h   |  3 ++-
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 7dbe201..0f4c9f6 100644
--- a/block.c
+++ b/block.c
@@ -65,6 +65,7 @@ static BdrvProbeFunc *format_probes[] = {
     cloop_probe,
     block_crypto_probe_luks,
     dmg_probe,
+    parallels_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/parallels.c b/block/parallels.c
index 547373e..6994c41 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -709,7 +709,6 @@ static QemuOptsList parallels_create_opts = {
 static BlockDriver bdrv_parallels = {
     .format_name	= "parallels",
     .instance_size	= sizeof(BDRVParallelsState),
-    .bdrv_probe		= parallels_probe,
     .bdrv_open		= parallels_open,
     .bdrv_close		= parallels_close,
     .bdrv_co_get_block_status = parallels_co_get_block_status,
diff --git a/block/probe/parallels.c b/block/probe/parallels.c
index 66cddea..3ebeddd 100644
--- a/block/probe/parallels.c
+++ b/block/probe/parallels.c
@@ -3,20 +3,24 @@
 #include "block/probe.h"
 #include "block/driver/parallels.h"
 
-int parallels_probe(const uint8_t *buf, int buf_size,
-                           const char *filename)
+const char *parallels_probe(const uint8_t *buf, int buf_size,
+                            const char *filename, int *score)
 {
+    const char *format = "parallels";
     const ParallelsHeader *ph = (const void *)buf;
+    assert(score);
+    *score = 0;
 
     if (buf_size < sizeof(ParallelsHeader)) {
-        return 0;
+        return format;
     }
 
     if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
-           !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
-           (le32_to_cpu(ph->version) == HEADER_VERSION)) {
-        return 100;
+        !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
+        (le32_to_cpu(ph->version) == HEADER_VERSION)) {
+        *score = 100;
+        return format;
     }
 
-    return 0;
+    return format;
 }
diff --git a/include/block/probe.h b/include/block/probe.h
index ef6629f..7383a64 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -18,5 +17,7 @@ const char *block_crypto_probe_luks(const uint8_t *buf, int buf_size,
                                     const char *filename, int *score);
 const char *dmg_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
+const char *parallels_probe(const uint8_t *buf, int buf_size,
+                            const char *filename, int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 22/32] blockdev: Separate qcow probe from its driver

[Colin Lord]

Completes the separation of the qcow probe from the qcow driver. The
qcow probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/qcow.c    | 16 +++++++++++-----
 block/qcow.c          |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 0f4c9f6..236e562 100644
--- a/block.c
+++ b/block.c
@@ -66,6 +66,7 @@ static BdrvProbeFunc *format_probes[] = {
     block_crypto_probe_luks,
     dmg_probe,
     parallels_probe,
+    qcow_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/qcow.c b/block/probe/qcow.c
index 6024d11..d249e02 100644
--- a/block/probe/qcow.c
+++ b/block/probe/qcow.c
@@ -3,14 +3,20 @@
 #include "block/probe.h"
 #include "block/driver/qcow.h"
 
-int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *qcow_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score)
 {
+    const char *format = "qcow";
     const QCowHeader *cow_header = (const void *)buf;
+    assert(score);
 
     if (buf_size >= sizeof(QCowHeader) &&
         be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
-        be32_to_cpu(cow_header->version) == QCOW_VERSION)
-        return 100;
-    else
-        return 0;
+        be32_to_cpu(cow_header->version) == QCOW_VERSION) {
+        *score = 100;
+        return format;
+    }
+
+    *score = 0;
+    return format;
 }
diff --git a/block/qcow.c b/block/qcow.c
index 828749c..9475ed0 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -1002,7 +1002,6 @@ static QemuOptsList qcow_create_opts = {
 static BlockDriver bdrv_qcow = {
     .format_name	= "qcow",
     .instance_size	= sizeof(BDRVQcowState),
-    .bdrv_probe		= qcow_probe,
     .bdrv_open		= qcow_open,
     .bdrv_close		= qcow_close,
     .bdrv_reopen_prepare    = qcow_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index 7383a64..3aeab2d 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
 int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -19,5 +18,7 @@ const char *dmg_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
 const char *parallels_probe(const uint8_t *buf, int buf_size,
                             const char *filename, int *score);
+const char *qcow_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 23/32] blockdev: Separate qcow2 probe from its driver

[Colin Lord]

Completes the separation of the qcow2 probe from the qcow2 driver. The
qcow2 probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/qcow2.c   | 16 +++++++++++-----
 block/qcow2.c         |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 236e562..b4e347a 100644
--- a/block.c
+++ b/block.c
@@ -67,6 +67,7 @@ static BdrvProbeFunc *format_probes[] = {
     dmg_probe,
     parallels_probe,
     qcow_probe,
+    qcow2_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/qcow2.c b/block/probe/qcow2.c
index 56f4e82..bd351e0 100644
--- a/block/probe/qcow2.c
+++ b/block/probe/qcow2.c
@@ -3,14 +3,20 @@
 #include "block/probe.h"
 #include "block/qcow2.h"
 
-int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *qcow2_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score)
 {
+    const char *format = "qcow2";
     const QCowHeader *cow_header = (const void *)buf;
+    assert(score);
 
     if (buf_size >= sizeof(QCowHeader) &&
         be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
-        be32_to_cpu(cow_header->version) >= 2)
-        return 100;
-    else
-        return 0;
+        be32_to_cpu(cow_header->version) >= 2) {
+        *score = 100;
+        return format;
+    }
+
+    *score = 0;
+    return format;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index 55639eb..bc2da36 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3336,7 +3336,6 @@ static QemuOptsList qcow2_create_opts = {
 BlockDriver bdrv_qcow2 = {
     .format_name        = "qcow2",
     .instance_size      = sizeof(BDRVQcow2State),
-    .bdrv_probe         = qcow2_probe,
     .bdrv_open          = qcow2_open,
     .bdrv_close         = qcow2_close,
     .bdrv_reopen_prepare  = qcow2_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index 3aeab2d..222185b 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
 int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -20,5 +19,7 @@ const char *parallels_probe(const uint8_t *buf, int buf_size,
                             const char *filename, int *score);
 const char *qcow_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
+const char *qcow2_probe(const uint8_t *buf, int buf_size, const char *filename,
+                        int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 24/32] blockdev: Separate qed probe from its driver

[Colin Lord]

Completes the separation of the qed probe from the qed driver. The
qed probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/qed.c     | 16 +++++++++++-----
 block/qed.c           |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index b4e347a..fee9f8c 100644
--- a/block.c
+++ b/block.c
@@ -68,6 +68,7 @@ static BdrvProbeFunc *format_probes[] = {
     parallels_probe,
     qcow_probe,
     qcow2_probe,
+    bdrv_qed_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/qed.c b/block/probe/qed.c
index c902489..3fdb5d7 100644
--- a/block/probe/qed.c
+++ b/block/probe/qed.c
@@ -3,16 +3,22 @@
 #include "block/probe.h"
 #include "block/qed.h"
 
-int bdrv_qed_probe(const uint8_t *buf, int buf_size,
-                          const char *filename)
+const char *bdrv_qed_probe(const uint8_t *buf, int buf_size,
+                           const char *filename, int *score)
 {
+    const char *format = "qed";
     const QEDHeader *header = (const QEDHeader *)buf;
+    assert(score);
+    *score = 0;
 
     if (buf_size < sizeof(*header)) {
-        return 0;
+        return format;
     }
+
     if (le32_to_cpu(header->magic) != QED_MAGIC) {
-        return 0;
+        return format;
     }
-    return 100;
+
+    *score = 100;
+    return format;
 }
diff --git a/block/qed.c b/block/qed.c
index 21d8982..cf78ff0 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1638,7 +1638,6 @@ static BlockDriver bdrv_qed = {
     .create_opts              = &qed_create_opts,
     .supports_backing         = true,
 
-    .bdrv_probe               = bdrv_qed_probe,
     .bdrv_open                = bdrv_qed_open,
     .bdrv_close               = bdrv_qed_close,
     .bdrv_reopen_prepare      = bdrv_qed_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index 222185b..63edd74 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);
 int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -21,5 +20,7 @@ const char *qcow_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
 const char *qcow2_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
+const char *bdrv_qed_probe(const uint8_t *buf, int buf_size,
+                           const char *filename, int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 25/32] blockdev: Separate raw probe from its driver

[Colin Lord]

Completes the separation of the raw probe from the raw driver. The
raw probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               | 1 +
 block/probe/raw.c     | 8 ++++++--
 block/raw_bsd.c       | 1 -
 include/block/probe.h | 3 ++-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index fee9f8c..a3d983f 100644
--- a/block.c
+++ b/block.c
@@ -69,6 +69,7 @@ static BdrvProbeFunc *format_probes[] = {
     qcow_probe,
     qcow2_probe,
     bdrv_qed_probe,
+    raw_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/raw.c b/block/probe/raw.c
index 22c6bcb..9923bb6 100644
--- a/block/probe/raw.c
+++ b/block/probe/raw.c
@@ -1,10 +1,14 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
-int raw_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *raw_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score)
 {
+    const char *format = "raw";
+    assert(score);
     /* smallest possible positive score so that raw is used if and only if no
      * other block driver works
      */
-    return 1;
+    *score = 1;
+    return format;
 }
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index 8f49637..4b24a36 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -231,7 +231,6 @@ static int raw_probe_geometry(BlockDriverState *bs, HDGeometry *geo)
 
 BlockDriver bdrv_raw = {
     .format_name          = "raw",
-    .bdrv_probe           = &raw_probe,
     .bdrv_reopen_prepare  = &raw_reopen_prepare,
     .bdrv_open            = &raw_open,
     .bdrv_close           = &raw_close,
diff --git a/include/block/probe.h b/include/block/probe.h
index 63edd74..b49663d 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int raw_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -22,5 +21,7 @@ const char *qcow2_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
 const char *bdrv_qed_probe(const uint8_t *buf, int buf_size,
                            const char *filename, int *score);
+const char *raw_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 26/32] blockdev: Separate vdi probe from its driver

[Colin Lord]

Completes the separation of the vdi probe from the vdi driver. The
vdi probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/vdi.c     | 13 ++++++++-----
 block/vdi.c           |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index a3d983f..fd97795 100644
--- a/block.c
+++ b/block.c
@@ -70,6 +70,7 @@ static BdrvProbeFunc *format_probes[] = {
     qcow2_probe,
     bdrv_qed_probe,
     raw_probe,
+    vdi_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/vdi.c b/block/probe/vdi.c
index 9adf5e5..1dcf7ab 100644
--- a/block/probe/vdi.c
+++ b/block/probe/vdi.c
@@ -3,24 +3,27 @@
 #include "block/probe.h"
 #include "block/driver/vdi.h"
 
-int vdi_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *vdi_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score)
 {
+    const char *format = "vdi";
     const VdiHeader *header = (const VdiHeader *)buf;
-    int ret = 0;
+    assert(score);
+    *score = 0;
 
     logout("\n");
 
     if (buf_size < sizeof(*header)) {
         /* Header too small, no VDI. */
     } else if (le32_to_cpu(header->signature) == VDI_SIGNATURE) {
-        ret = 100;
+        *score = 100;
     }
 
-    if (ret == 0) {
+    if (*score == 0) {
         logout("no vdi image\n");
     } else {
         logout("%s", header->text);
     }
 
-    return ret;
+    return format;
 }
diff --git a/block/vdi.c b/block/vdi.c
index e99ae92..445e2b8 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -849,7 +849,6 @@ static QemuOptsList vdi_create_opts = {
 static BlockDriver bdrv_vdi = {
     .format_name = "vdi",
     .instance_size = sizeof(BDRVVdiState),
-    .bdrv_probe = vdi_probe,
     .bdrv_open = vdi_open,
     .bdrv_close = vdi_close,
     .bdrv_reopen_prepare = vdi_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index b49663d..b19b586 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int vdi_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
@@ -23,5 +22,7 @@ const char *bdrv_qed_probe(const uint8_t *buf, int buf_size,
                            const char *filename, int *score);
 const char *raw_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
+const char *vdi_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 27/32] blockdev: Separate vhdx probe from its driver

[Colin Lord]

Completes the separation of the vhdx probe from the vhdx driver. The
vhdx probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/vhdx.c    | 12 +++++++++---
 block/vhdx.c          |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/block.c b/block.c
index fd97795..cc48279 100644
--- a/block.c
+++ b/block.c
@@ -71,6 +71,7 @@ static BdrvProbeFunc *format_probes[] = {
     bdrv_qed_probe,
     raw_probe,
     vdi_probe,
+    vhdx_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/vhdx.c b/block/probe/vhdx.c
index 6c38aac..cbb74c1 100644
--- a/block/probe/vhdx.c
+++ b/block/probe/vhdx.c
@@ -12,10 +12,16 @@
  *
  *  Therefore, we probe by looking for the vhdxfile signature "vhdxfile"
  */
-int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *vhdx_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score)
 {
+    const char *format = "vhdx";
+    assert(score);
+
     if (buf_size >= 8 && !memcmp(buf, "vhdxfile", 8)) {
-        return 100;
+        *score = 100;
+        return format;
     }
-    return 0;
+    *score = 0;
+    return format;
 }
diff --git a/block/vhdx.c b/block/vhdx.c
index ba8adfe..2353569 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1942,7 +1942,6 @@ static QemuOptsList vhdx_create_opts = {
 static BlockDriver bdrv_vhdx = {
     .format_name            = "vhdx",
     .instance_size          = sizeof(BDRVVHDXState),
-    .bdrv_probe             = vhdx_probe,
     .bdrv_open              = vhdx_open,
     .bdrv_close             = vhdx_close,
     .bdrv_reopen_prepare    = vhdx_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index b19b586..3e2e328 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int vhdx_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
 const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
@@ -24,5 +23,7 @@ const char *raw_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
 const char *vdi_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
+const char *vhdx_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 28/32] blockdev: Separate vmdk probe from its driver

[Colin Lord]

Completes the separation of the vmdk probe from the vmdk driver. The
vmdk probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/vmdk.c    | 23 +++++++++++++++--------
 block/vmdk.c          |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index cc48279..9b839df 100644
--- a/block.c
+++ b/block.c
@@ -72,6 +72,7 @@ static BdrvProbeFunc *format_probes[] = {
     raw_probe,
     vdi_probe,
     vhdx_probe,
+    vmdk_probe,
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/vmdk.c b/block/probe/vmdk.c
index cb9bd3d..305a95c 100644
--- a/block/probe/vmdk.c
+++ b/block/probe/vmdk.c
@@ -3,17 +3,22 @@
 #include "block/probe.h"
 #include "block/driver/vmdk.h"
 
-int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score)
 {
+    const char *format = "vmdk";
     uint32_t magic;
+    assert(score);
+    *score = 0;
 
     if (buf_size < 4) {
-        return 0;
+        return format;
     }
     magic = be32_to_cpu(*(uint32_t *)buf);
     if (magic == VMDK3_MAGIC ||
         magic == VMDK4_MAGIC) {
-        return 100;
+        *score = 100;
+        return format;
     } else {
         const char *p = (const char *)buf;
         const char *end = p + buf_size;
@@ -36,7 +41,7 @@ int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
                 }
                 /* only accept blank lines before 'version=' line */
                 if (p == end || *p != '\n') {
-                    return 0;
+                    return format;
                 }
                 p++;
                 continue;
@@ -44,17 +49,19 @@ int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
             if (end - p >= strlen("version=X\n")) {
                 if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
                     strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
-                    return 100;
+                    *score = 100;
+                    return format;
                 }
             }
             if (end - p >= strlen("version=X\r\n")) {
                 if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
                     strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
-                    return 100;
+                    *score = 100;
+                    return format;
                 }
             }
-            return 0;
+            return format;
         }
-        return 0;
+        return format;
     }
 }
diff --git a/block/vmdk.c b/block/vmdk.c
index d22db95..1cf63b4 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2335,7 +2335,6 @@ static QemuOptsList vmdk_create_opts = {
 static BlockDriver bdrv_vmdk = {
     .format_name                  = "vmdk",
     .instance_size                = sizeof(BDRVVmdkState),
-    .bdrv_probe                   = vmdk_probe,
     .bdrv_open                    = vmdk_open,
     .bdrv_check                   = vmdk_check,
     .bdrv_reopen_prepare          = vmdk_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index 3e2e328..13df1d3 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename);
 int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
 const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
@@ -25,5 +24,7 @@ const char *vdi_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
 const char *vhdx_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
+const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
+                       int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 29/32] blockdev: Separate vpc probe from its driver

[Colin Lord]

Completes the separation of the vpc probe from the vpc driver. The
vpc probe now returns the format in addition to the score, allowing
correlation of the score and driver without the probe function being
part of the driver itself.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c               |  1 +
 block/probe/vpc.c     | 15 +++++++++++----
 block/vpc.c           |  1 -
 include/block/probe.h |  3 ++-
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/block.c b/block.c
index 9b839df..8226124 100644
--- a/block.c
+++ b/block.c
@@ -73,6 +73,7 @@ static BdrvProbeFunc *format_probes[] = {
     vdi_probe,
     vhdx_probe,
     vmdk_probe,
+    vpc_probe
 };
 
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
diff --git a/block/probe/vpc.c b/block/probe/vpc.c
index afe8271..0fe8a65 100644
--- a/block/probe/vpc.c
+++ b/block/probe/vpc.c
@@ -1,9 +1,16 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
-int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
+const char *vpc_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score)
 {
-    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
-	return 100;
-    return 0;
+    const char *format = "vpc";
+    assert(score);
+    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8)) {
+        *score = 100;
+        return format;
+    }
+
+    *score = 0;
+    return format;
 }
diff --git a/block/vpc.c b/block/vpc.c
index cb65022..cd87d8f 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -1050,7 +1050,6 @@ static BlockDriver bdrv_vpc = {
     .format_name    = "vpc",
     .instance_size  = sizeof(BDRVVPCState),
 
-    .bdrv_probe             = vpc_probe,
     .bdrv_open              = vpc_open,
     .bdrv_close             = vpc_close,
     .bdrv_reopen_prepare    = vpc_reopen_prepare,
diff --git a/include/block/probe.h b/include/block/probe.h
index 13df1d3..7facb75 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -1,7 +1,6 @@
 #ifndef PROBE_H
 #define PROBE_H
 
-int vpc_probe(const uint8_t *buf, int buf_size, const char *filename);
 const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
                         int *score);
 const char *cloop_probe(const uint8_t *buf, int buf_size, const char *filename,
@@ -26,5 +25,7 @@ const char *vhdx_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
 const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
+const char *vpc_probe(const uint8_t *buf, int buf_size, const char *filename,
+                      int *score);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 30/32] blockdev: Remove the .bdrv_probe field from BlockDrivers

[Colin Lord]

This commit finalizes the separation of the block driver and probe
function by removing the .bdrv_probe field from all BlockDrivers.
Probing is now accomplished solely by iterating over the array of probe
function pointers in the format_probes array.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c                         | 20 +-------------------
 block/raw-posix.c               |  1 -
 include/block/block_int.h       |  1 -
 scripts/modules/module_block.py | 10 ++--------
 4 files changed, 3 insertions(+), 29 deletions(-)

diff --git a/block.c b/block.c
index 8226124..7e441fe 100644
--- a/block.c
+++ b/block.c
@@ -599,34 +599,16 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
     const char *format_max = NULL;
     const char *format;
     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);
-            if (score > score_max) {
-                score_max = score;
-                drv = d;
-            }
-        }
-    }
 
     for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
         format = format_probes[i](buf, buf_size, filename, &score);
         if (score > score_max) {
             score_max = score;
             format_max = format;
-            drv = bdrv_find_format(format_max);
         }
     }
 
-    return drv;
+    return bdrv_find_format(format_max);
 }
 
 static int find_image_format(BlockDriverState *bs, const char *filename,
diff --git a/block/raw-posix.c b/block/raw-posix.c
index bef7a67..a6ad689 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -1936,7 +1936,6 @@ BlockDriver bdrv_file = {
     .protocol_name = "file",
     .instance_size = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe = NULL, /* no probe for protocols */
     .bdrv_parse_filename = raw_parse_filename,
     .bdrv_file_open = raw_open,
     .bdrv_reopen_prepare = raw_reopen_prepare,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2057156..2bca115 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -99,7 +99,6 @@ struct BlockDriver {
     bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
                                              BlockDriverState *candidate);
 
-    int (*bdrv_probe)(const uint8_t *buf, int buf_size, const char *filename);
     int (*bdrv_probe_device)(const char *filename);
 
     /* Any driver implementing this callback is expected to be able to handle
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
index 4075574..18200e2 100644
--- a/scripts/modules/module_block.py
+++ b/scripts/modules/module_block.py
@@ -24,15 +24,13 @@ def get_string_struct(line):
     return data[2].replace('"', '')[:-1]
 
 def add_module(fheader, library, format_name, protocol_name,
-                probe, probe_device):
+               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,')
 
@@ -52,20 +50,17 @@ def process_file(fheader, filename):
                     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)
+                               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:
@@ -93,7 +88,6 @@ 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[] = {''')
 
-- 
2.5.5

[Qemu-devel] [PATCH v3 31/32] blockdev: Separate out bdrv_probe_device functions

[Colin Lord]

This puts the bdrv_probe_device functions into their own files to
facilitate the modularization of the block drivers.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block/Makefile.objs       |  1 +
 block/probe/host_cdrom.c  | 40 +++++++++++++++++++++++++++++++++++++
 block/probe/host_device.c | 30 ++++++++++++++++++++++++++++
 block/raw-posix.c         | 51 +----------------------------------------------
 block/raw-win32.c         | 10 +---------
 include/block/probe.h     |  2 ++
 6 files changed, 75 insertions(+), 59 deletions(-)
 create mode 100644 block/probe/host_cdrom.c
 create mode 100644 block/probe/host_device.c

diff --git a/block/Makefile.objs b/block/Makefile.objs
index 2d2691e..3d997e7 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -27,6 +27,7 @@ block-obj-y += crypto.o
 block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o probe/dmg.o
 block-obj-y += probe/parallels.o probe/qcow.o probe/qcow2.o probe/qed.o
 block-obj-y += probe/raw.o probe/vdi.o probe/vhdx.o probe/vmdk.o probe/vpc.o
+block-obj-y += probe/host_device.o probe/host_cdrom.o
 
 common-obj-y += stream.o
 common-obj-y += commit.o
diff --git a/block/probe/host_cdrom.c b/block/probe/host_cdrom.c
new file mode 100644
index 0000000..1886cad
--- /dev/null
+++ b/block/probe/host_cdrom.c
@@ -0,0 +1,40 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
+int cdrom_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/cd", NULL) ||
+            strstart(filename, "/dev/acd", NULL))
+        return 100;
+    return 0;
+}
+#elif defined(__linux__)
+#include <sys/ioctl.h>
+#include <linux/cdrom.h>
+int cdrom_probe_device(const char *filename)
+{
+    int fd, ret;
+    int prio = 0;
+    struct stat st;
+
+    fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
+    if (fd < 0) {
+        goto out;
+    }
+    ret = fstat(fd, &st);
+    if (ret == -1 || !S_ISBLK(st.st_mode)) {
+        goto outc;
+    }
+
+    /* Attempt to detect via a CDROM specific ioctl */
+    ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
+    if (ret >= 0)
+        prio = 100;
+
+outc:
+    qemu_close(fd);
+out:
+    return prio;
+}
+#endif
diff --git a/block/probe/host_device.c b/block/probe/host_device.c
new file mode 100644
index 0000000..ebd969b
--- /dev/null
+++ b/block/probe/host_device.c
@@ -0,0 +1,30 @@
+#include "qemu/osdep.h"
+#include "block/probe.h"
+#include "qemu/cutils.h"
+
+#ifdef _WIN32
+int hdev_probe_device(const char *filename)
+{
+    if (strstart(filename, "/dev/cdrom", NULL))
+        return 100;
+    if (is_windows_drive(filename))
+        return 100;
+    return 0;
+}
+#else
+int hdev_probe_device(const char *filename)
+{
+    struct stat st;
+
+    /* allow a dedicated CD-ROM driver to match with a higher priority */
+    if (strstart(filename, "/dev/cdrom", NULL))
+        return 50;
+
+    if (stat(filename, &st) >= 0 &&
+            (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
+        return 100;
+    }
+
+    return 0;
+}
+#endif
diff --git a/block/raw-posix.c b/block/raw-posix.c
index a6ad689..56b2952 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -28,6 +28,7 @@
 #include "qemu/timer.h"
 #include "qemu/log.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "qemu/module.h"
 #include "trace.h"
 #include "block/thread-pool.h"
@@ -2084,22 +2085,6 @@ static void print_unmounting_directions(const char *file_name)
 
 #endif /* defined(__APPLE__) && defined(__MACH__) */
 
-static int hdev_probe_device(const char *filename)
-{
-    struct stat st;
-
-    /* allow a dedicated CD-ROM driver to match with a higher priority */
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 50;
-
-    if (stat(filename, &st) >= 0 &&
-            (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
-        return 100;
-    }
-
-    return 0;
-}
-
 static int check_hdev_writable(BDRVRawState *s)
 {
 #if defined(BLKROGET)
@@ -2441,32 +2426,6 @@ static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
     return raw_open_common(bs, options, flags, O_NONBLOCK, errp);
 }
 
-static int cdrom_probe_device(const char *filename)
-{
-    int fd, ret;
-    int prio = 0;
-    struct stat st;
-
-    fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
-    if (fd < 0) {
-        goto out;
-    }
-    ret = fstat(fd, &st);
-    if (ret == -1 || !S_ISBLK(st.st_mode)) {
-        goto outc;
-    }
-
-    /* Attempt to detect via a CDROM specific ioctl */
-    ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
-    if (ret >= 0)
-        prio = 100;
-
-outc:
-    qemu_close(fd);
-out:
-    return prio;
-}
-
 static bool cdrom_is_inserted(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
@@ -2565,14 +2524,6 @@ static int cdrom_open(BlockDriverState *bs, QDict *options, int flags,
     return 0;
 }
 
-static int cdrom_probe_device(const char *filename)
-{
-    if (strstart(filename, "/dev/cd", NULL) ||
-            strstart(filename, "/dev/acd", NULL))
-        return 100;
-    return 0;
-}
-
 static int cdrom_reopen(BlockDriverState *bs)
 {
     BDRVRawState *s = bs->opaque;
diff --git a/block/raw-win32.c b/block/raw-win32.c
index fd23891..44cb503 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -26,6 +26,7 @@
 #include "qemu/cutils.h"
 #include "qemu/timer.h"
 #include "block/block_int.h"
+#include "block/probe.h"
 #include "qemu/module.h"
 #include "raw-aio.h"
 #include "trace.h"
@@ -615,15 +616,6 @@ static int find_device_type(BlockDriverState *bs, const char *filename)
     }
 }
 
-static int hdev_probe_device(const char *filename)
-{
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 100;
-    if (is_windows_drive(filename))
-        return 100;
-    return 0;
-}
-
 static void hdev_parse_filename(const char *filename, QDict *options,
                                 Error **errp)
 {
diff --git a/include/block/probe.h b/include/block/probe.h
index 7facb75..2732f56 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -27,5 +27,7 @@ const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
 const char *vpc_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
+int hdev_probe_device(const char *filename);
+int cdrom_probe_device(const char *filename);
 
 #endif
-- 
2.5.5

[Qemu-devel] [PATCH v3 32/32] blockdev: Remove bdrv_probe_device field from BlockDriver

[Colin Lord]

This commit finalizes the separation of the BlockDriver from its
device probing function. Now the accesses to these functions in block.c
occur through the protocol_probes array, and each function returns a
score and protocol name with which to find the corresponding driver.

Signed-off-by: Colin Lord <clord@redhat.com>
---
 block.c                         | 46 ++++++++++++++++++++++++++++++-----------
 block/probe/host_cdrom.c        | 23 ++++++++++++++-------
 block/probe/host_device.c       | 34 ++++++++++++++++++++----------
 block/raw-posix.c               |  3 ---
 block/raw-win32.c               |  1 -
 include/block/block_int.h       |  2 --
 include/block/probe.h           |  4 ++--
 scripts/modules/module_block.py | 12 ++---------
 8 files changed, 76 insertions(+), 49 deletions(-)

diff --git a/block.c b/block.c
index 7e441fe..bc1046b 100644
--- a/block.c
+++ b/block.c
@@ -59,6 +59,7 @@
 
 typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
                                   const char *filename, int *score);
+typedef const char *BdrvProbeDevFunc(const char *filename, int *score);
 
 static BdrvProbeFunc *format_probes[] = {
     bochs_probe,
@@ -76,6 +77,13 @@ static BdrvProbeFunc *format_probes[] = {
     vpc_probe
 };
 
+static BdrvProbeDevFunc *protocol_probes[] = {
+    hdev_probe_device,
+#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__linux__)
+    cdrom_probe_device
+#endif
+};
+
 static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
     QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
 
@@ -95,6 +103,8 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
 /* If non-zero, use only whitelisted block drivers */
 static int use_bdrv_whitelist;
 
+static BlockDriver *bdrv_do_find_protocol(const char *protocol);
+
 #ifdef _WIN32
 static int is_windows_drive_prefix(const char *filename)
 {
@@ -487,25 +497,37 @@ int get_tmp_filename(char *filename, int size)
 static BlockDriver *find_hdev_driver(const char *filename)
 {
     int score_max = 0, score;
+    const char *protocol_max = NULL;
+    const char *protocol;
+    BlockDriver *drv;
     size_t i;
-    BlockDriver *drv = NULL, *d;
+
+    for (i = 0; i < ARRAY_SIZE(protocol_probes); i++) {
+        protocol = protocol_probes[i](filename, &score);
+        if (score > score_max) {
+            protocol_max = protocol;
+            score_max = score;
+        }
+    }
+
+    if (!protocol_max) {
+        return NULL;
+    }
+
+    drv = bdrv_do_find_protocol(protocol_max);
+    if (drv) {
+        return drv;
+    }
 
     for (i = 0; i < ARRAY_SIZE(block_driver_modules); ++i) {
-        if (block_driver_modules[i].has_probe_device) {
+        if (block_driver_modules[i].protocol_name &&
+            !strcmp(block_driver_modules[i].protocol_name, protocol_max)) {
             block_module_load_one(block_driver_modules[i].library_name);
+            break;
         }
     }
 
-    QLIST_FOREACH(d, &bdrv_drivers, list) {
-        if (d->bdrv_probe_device) {
-            score = d->bdrv_probe_device(filename);
-            if (score > score_max) {
-                score_max = score;
-                drv = d;
-            }
-        }
-    }
-
+    drv = bdrv_do_find_protocol(protocol);
     return drv;
 }
 
diff --git a/block/probe/host_cdrom.c b/block/probe/host_cdrom.c
index 1886cad..3f7d863 100644
--- a/block/probe/host_cdrom.c
+++ b/block/probe/host_cdrom.c
@@ -1,22 +1,28 @@
 #include "qemu/osdep.h"
 #include "block/probe.h"
 
+static const char *protocol = "host_cdrom";
+
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
-int cdrom_probe_device(const char *filename)
+const char *cdrom_probe_device(const char *filename, int *score)
 {
+    assert(score);
     if (strstart(filename, "/dev/cd", NULL) ||
-            strstart(filename, "/dev/acd", NULL))
-        return 100;
+        strstart(filename, "/dev/acd", NULL)) {
+        *score = 100;
+        return protocol;
+    }
     return 0;
 }
 #elif defined(__linux__)
 #include <sys/ioctl.h>
 #include <linux/cdrom.h>
-int cdrom_probe_device(const char *filename)
+const char *cdrom_probe_device(const char *filename, int *score)
 {
     int fd, ret;
-    int prio = 0;
     struct stat st;
+    assert(score);
+    *score = 0;
 
     fd = qemu_open(filename, O_RDONLY | O_NONBLOCK);
     if (fd < 0) {
@@ -29,12 +35,13 @@ int cdrom_probe_device(const char *filename)
 
     /* Attempt to detect via a CDROM specific ioctl */
     ret = ioctl(fd, CDROM_DRIVE_STATUS, CDSL_CURRENT);
-    if (ret >= 0)
-        prio = 100;
+    if (ret >= 0) {
+        *score = 100;
+    }
 
 outc:
     qemu_close(fd);
 out:
-    return prio;
+    return protocol;
 }
 #endif
diff --git a/block/probe/host_device.c b/block/probe/host_device.c
index ebd969b..b4e4d20 100644
--- a/block/probe/host_device.c
+++ b/block/probe/host_device.c
@@ -2,29 +2,41 @@
 #include "block/probe.h"
 #include "qemu/cutils.h"
 
+static const char *protocol = "host_device";
+
 #ifdef _WIN32
-int hdev_probe_device(const char *filename)
+const char *hdev_probe_device(const char *filename, int *score)
 {
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 100;
-    if (is_windows_drive(filename))
-        return 100;
-    return 0;
+    assert(score);
+    *score = 100;
+    if (strstart(filename, "/dev/cdrom", NULL)) {
+        return protocol;
+    }
+    if (is_windows_drive(filename)) {
+        return protocol
+    }
+    *score = 0;
+    return protocol;
 }
 #else
-int hdev_probe_device(const char *filename)
+const char *hdev_probe_device(const char *filename, int *score)
 {
     struct stat st;
+    assert(score);
 
     /* allow a dedicated CD-ROM driver to match with a higher priority */
-    if (strstart(filename, "/dev/cdrom", NULL))
-        return 50;
+    if (strstart(filename, "/dev/cdrom", NULL)) {
+        *score = 50;
+        return protocol;
+    }
 
     if (stat(filename, &st) >= 0 &&
             (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
-        return 100;
+        *score = 100;
+        return protocol;
     }
 
-    return 0;
+    *score = 0;
+    return protocol;
 }
 #endif
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 56b2952..9497f5b 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -2367,7 +2367,6 @@ static BlockDriver bdrv_host_device = {
     .protocol_name        = "host_device",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device  = hdev_probe_device,
     .bdrv_parse_filename = hdev_parse_filename,
     .bdrv_file_open     = hdev_open,
     .bdrv_close         = raw_close,
@@ -2466,7 +2465,6 @@ static BlockDriver bdrv_host_cdrom = {
     .protocol_name      = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device	= cdrom_probe_device,
     .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
@@ -2592,7 +2590,6 @@ static BlockDriver bdrv_host_cdrom = {
     .protocol_name      = "host_cdrom",
     .instance_size      = sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
-    .bdrv_probe_device	= cdrom_probe_device,
     .bdrv_parse_filename = cdrom_parse_filename,
     .bdrv_file_open     = cdrom_open,
     .bdrv_close         = raw_close,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 44cb503..8c9a3f8 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -696,7 +696,6 @@ static BlockDriver bdrv_host_device = {
     .instance_size	= sizeof(BDRVRawState),
     .bdrv_needs_filename = true,
     .bdrv_parse_filename = hdev_parse_filename,
-    .bdrv_probe_device	= hdev_probe_device,
     .bdrv_file_open	= hdev_open,
     .bdrv_close		= raw_close,
 
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 2bca115..f0340c6 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -99,8 +99,6 @@ struct BlockDriver {
     bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs,
                                              BlockDriverState *candidate);
 
-    int (*bdrv_probe_device)(const char *filename);
-
     /* Any driver implementing this callback is expected to be able to handle
      * NULL file names in its .bdrv_open() implementation */
     void (*bdrv_parse_filename)(const char *filename, QDict *options, Error **errp);
diff --git a/include/block/probe.h b/include/block/probe.h
index 2732f56..c974414 100644
--- a/include/block/probe.h
+++ b/include/block/probe.h
@@ -27,7 +27,7 @@ const char *vmdk_probe(const uint8_t *buf, int buf_size, const char *filename,
                        int *score);
 const char *vpc_probe(const uint8_t *buf, int buf_size, const char *filename,
                       int *score);
-int hdev_probe_device(const char *filename);
-int cdrom_probe_device(const char *filename);
+const char *hdev_probe_device(const char *filename, int *score);
+const char *cdrom_probe_device(const char *filename, int *score);
 
 #endif
diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
index 18200e2..e0f5896 100644
--- a/scripts/modules/module_block.py
+++ b/scripts/modules/module_block.py
@@ -23,16 +23,13 @@ def get_string_struct(line):
 
     return data[2].replace('"', '')[:-1]
 
-def add_module(fheader, library, format_name, protocol_name,
-               probe_device):
+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 + '",')
-    if probe_device:
-        lines.append('.has_probe_device = true,')
 
     text = '\n\t'.join(lines)
     fheader.write('\n\t{\n\t' + text + '\n\t},')
@@ -50,18 +47,14 @@ def process_file(fheader, filename):
                     format_name = get_string_struct(line)
                 elif line.find(".protocol_name") != -1:
                     protocol_name = get_string_struct(line)
-                elif line.find(".bdrv_probe_device") != -1:
-                    probe_device = True
                 elif line == "};":
-                    add_module(fheader, library, format_name, protocol_name,
-                               probe_device)
+                    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 = ""
-                probe_device = False
 
         if not found_something:
             print("No BlockDriver struct found in " + filename + ". \
@@ -88,7 +81,6 @@ static const struct {
     const char *format_name;
     const char *protocol_name;
     const char *library_name;
-    bool has_probe_device;
 } block_driver_modules[] = {''')
 
 def print_bottom(fheader):
-- 
2.5.5

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Daniel P. Berrange]

On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
> This puts the bochs probe function into its own separate file as part of
> the process of modularizing block drivers. Having the probe functions
> separate from the rest of the driver allows us to probe without having
> to potentially unnecessarily load the driver.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs          |  1 +
>  block/bochs.c                | 55 ++------------------------------------------
>  block/probe/bochs.c          | 21 +++++++++++++++++

Do we really need a sub-dir for this ?  If we were going to
have sub-dirs under block/, I'd suggest we have one subdir
per block driver, not spread code for one block driver
across multiple dirs.

IMHO a block/bochs-probe.c would be better, unless we did
move block/bochs.c into a block/bochs/driver.c dir.

Either way, you should update MAINTAINERS file to record
this newly added filename, against the bochs entry. The
same applies to most other patches in this series adding
new files.


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 v3 06/32] blockdev: Move luks probe to its own file

[Daniel P. Berrange]

On Tue, Jul 05, 2016 at 11:24:06AM -0400, Colin Lord wrote:
> Isolates the luks probe function as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/crypto.c        | 21 +--------------------
>  block/probe/luks.c    | 23 +++++++++++++++++++++++
>  include/block/probe.h |  2 ++
>  4 files changed, 27 insertions(+), 21 deletions(-)
>  create mode 100644 block/probe/luks.c
> 
> diff --git a/block/Makefile.objs b/block/Makefile.objs
> index 3b98001..28a7ec3 100644
> --- a/block/Makefile.objs
> +++ b/block/Makefile.objs
> @@ -24,7 +24,7 @@ block-obj-y += accounting.o dirty-bitmap.o
>  block-obj-y += write-threshold.o
>  
>  block-obj-y += crypto.o
> -block-obj-y += probe/bochs.o probe/cloop.o
> +block-obj-y += probe/bochs.o probe/cloop.o probe/luks.o
>  
>  common-obj-y += stream.o
>  common-obj-y += commit.o
> diff --git a/block/crypto.c b/block/crypto.c
> index 758e14e..493dd69 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -21,6 +21,7 @@
>  #include "qemu/osdep.h"
>  
>  #include "block/block_int.h"
> +#include "block/probe.h"
>  #include "sysemu/block-backend.h"
>  #include "crypto/block.h"
>  #include "qapi/opts-visitor.h"
> @@ -41,19 +42,6 @@ struct BlockCrypto {
>  };
>  
>  
> -static int block_crypto_probe_generic(QCryptoBlockFormat format,
> -                                      const uint8_t *buf,
> -                                      int buf_size,
> -                                      const char *filename)
> -{
> -    if (qcrypto_block_has_format(format, buf, buf_size)) {
> -        return 100;
> -    } else {
> -        return 0;
> -    }
> -}
> -
> -
>  static ssize_t block_crypto_read_func(QCryptoBlock *block,
>                                        size_t offset,
>                                        uint8_t *buf,
> @@ -540,13 +528,6 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)
>  }
>  
>  
> -static int block_crypto_probe_luks(const uint8_t *buf,
> -                                   int buf_size,
> -                                   const char *filename) {
> -    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
> -                                      buf, buf_size, filename);
> -}
> -
>  static int block_crypto_open_luks(BlockDriverState *bs,
>                                    QDict *options,
>                                    int flags,
> diff --git a/block/probe/luks.c b/block/probe/luks.c
> new file mode 100644
> index 0000000..5c6427a
> --- /dev/null
> +++ b/block/probe/luks.c

This should be a crypto.c file, since the same probing logic is
intended to work for any crypto format, not just luks, hence why
the original file is block/crypto.c, not block/luks.c

> @@ -0,0 +1,23 @@
> +#include "qemu/osdep.h"
> +#include "qapi-types.h"
> +#include "block/probe.h"
> +#include "crypto/block.h"
> +
> +static int block_crypto_probe_generic(QCryptoBlockFormat format,
> +                                      const uint8_t *buf,
> +                                      int buf_size,
> +                                      const char *filename)
> +{
> +    if (qcrypto_block_has_format(format, buf, buf_size)) {
> +        return 100;
> +    } else {
> +        return 0;
> +    }
> +}
> +
> +int block_crypto_probe_luks(const uint8_t *buf,
> +                                   int buf_size,
> +                                   const char *filename) {
> +    return block_crypto_probe_generic(Q_CRYPTO_BLOCK_FORMAT_LUKS,
> +                                      buf, buf_size, filename);
> +}
> 

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] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[John Snow]

On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>> This puts the bochs probe function into its own separate file as part of
>> the process of modularizing block drivers. Having the probe functions
>> separate from the rest of the driver allows us to probe without having
>> to potentially unnecessarily load the driver.
>>
>> Signed-off-by: Colin Lord <clord@redhat.com>
>> ---
>>  block/Makefile.objs          |  1 +
>>  block/bochs.c                | 55 ++------------------------------------------
>>  block/probe/bochs.c          | 21 +++++++++++++++++
> 
> Do we really need a sub-dir for this ?  If we were going to
> have sub-dirs under block/, I'd suggest we have one subdir
> per block driver, not spread code for one block driver
> across multiple dirs.
> 

Admittedly I have been nudging Colin to shoot from the hip a bit on
filename organization because I was short of ideas.

Some ideas:

(1) A combined probe.c file. This keeps the existing organization and
localizes everything to just one new file.

Downside: many formats rely on at least some minimal amount of structure
and constant definitions, and some of those overlap with each other.
qcow and qcow2 both using "QcowHeader" would be a prominent example.

They could all be disentangled, but it's less clear on where all the
common definitions go. A common probe.h is a bad idea since the modular
portion of the driver has no business gaining access to other formats'
definitions. We could create a probe.c and matching
include/block/bdrv/fmt.h includes, but we lost our zeal for this method.

(2) Separate probe files for each driver.

What we went with. Keeps refactoring to a minimum. Adds a bunch of
little files, but it's minimal and fairly noninvasive.

Like #1 though, we still have to figure out what to do with the common
includes.

> IMHO a block/bochs-probe.c would be better, unless we did
> move block/bochs.c into a block/bochs/driver.c dir.
> 
> Either way, you should update MAINTAINERS file to record
> this newly added filename, against the bochs entry. The
> same applies to most other patches in this series adding
> new files.
> 
> 
> Regards,
> Daniel
> 

So, something like:

block/drivers/bochs/

bochs.c
probe.c (or bochs-probe.c)

and

include/block/drivers/bochs/

common.h (or internal.h)


Any objections from the gallery?

--js

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Max Reitz]

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

On 05.07.2016 22:50, John Snow wrote:
> 
> 
> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>> This puts the bochs probe function into its own separate file as part of
>>> the process of modularizing block drivers. Having the probe functions
>>> separate from the rest of the driver allows us to probe without having
>>> to potentially unnecessarily load the driver.
>>>
>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>> ---
>>>  block/Makefile.objs          |  1 +
>>>  block/bochs.c                | 55 ++------------------------------------------
>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>
>> Do we really need a sub-dir for this ?  If we were going to
>> have sub-dirs under block/, I'd suggest we have one subdir
>> per block driver, not spread code for one block driver
>> across multiple dirs.
>>
> 
> Admittedly I have been nudging Colin to shoot from the hip a bit on
> filename organization because I was short of ideas.
> 
> Some ideas:
> 
> (1) A combined probe.c file. This keeps the existing organization and
> localizes everything to just one new file.
> 
> Downside: many formats rely on at least some minimal amount of structure
> and constant definitions, and some of those overlap with each other.
> qcow and qcow2 both using "QcowHeader" would be a prominent example.
> 
> They could all be disentangled, but it's less clear on where all the
> common definitions go. A common probe.h is a bad idea since the modular
> portion of the driver has no business gaining access to other formats'
> definitions. We could create a probe.c and matching
> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
> 
> (2) Separate probe files for each driver.
> 
> What we went with. Keeps refactoring to a minimum. Adds a bunch of
> little files, but it's minimal and fairly noninvasive.
> 
> Like #1 though, we still have to figure out what to do with the common
> includes.
> 
>> IMHO a block/bochs-probe.c would be better, unless we did
>> move block/bochs.c into a block/bochs/driver.c dir.
>>
>> Either way, you should update MAINTAINERS file to record
>> this newly added filename, against the bochs entry. The
>> same applies to most other patches in this series adding
>> new files.
>>
>>
>> Regards,
>> Daniel
>>
> 
> So, something like:
> 
> block/drivers/bochs/
> 
> bochs.c
> probe.c (or bochs-probe.c)
> 
> and
> 
> include/block/drivers/bochs/
> 
> common.h (or internal.h)
> 
> 
> Any objections from the gallery?

Yea (or “Nay”?). I'd rather not have as many directories in block/ as we
have files there right now and in most of these directories just two
files, for two reasons:

(1) I don't want it, because of my personal taste. If you just did it, I
probably wouldn't complain for too long, though.

(2) Code motion shouldn't be done without a good reason. I know this is
of no concern to upstream (which we are talking about), but it's always
iffy when it comes to backports. And I am a Red Hat employee, so I am
paid to think about them.

Also, there's another argument: As far as I know we sooner or later want
to make probing some kind of a block driver anyway (i.e. if you choose
the "probe" block driver, it'll automatically replace itself by the
right one). So in that sense, one could actually argue that probing is a
block driver.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[John Snow]

On 07/05/2016 05:00 PM, Max Reitz wrote:
> On 05.07.2016 22:50, John Snow wrote:
>>
>>
>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>> This puts the bochs probe function into its own separate file as part of
>>>> the process of modularizing block drivers. Having the probe functions
>>>> separate from the rest of the driver allows us to probe without having
>>>> to potentially unnecessarily load the driver.
>>>>
>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>> ---
>>>>  block/Makefile.objs          |  1 +
>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>
>>> Do we really need a sub-dir for this ?  If we were going to
>>> have sub-dirs under block/, I'd suggest we have one subdir
>>> per block driver, not spread code for one block driver
>>> across multiple dirs.
>>>
>>
>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>> filename organization because I was short of ideas.
>>
>> Some ideas:
>>
>> (1) A combined probe.c file. This keeps the existing organization and
>> localizes everything to just one new file.
>>
>> Downside: many formats rely on at least some minimal amount of structure
>> and constant definitions, and some of those overlap with each other.
>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>
>> They could all be disentangled, but it's less clear on where all the
>> common definitions go. A common probe.h is a bad idea since the modular
>> portion of the driver has no business gaining access to other formats'
>> definitions. We could create a probe.c and matching
>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>
>> (2) Separate probe files for each driver.
>>
>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>> little files, but it's minimal and fairly noninvasive.
>>
>> Like #1 though, we still have to figure out what to do with the common
>> includes.
>>
>>> IMHO a block/bochs-probe.c would be better, unless we did
>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>
>>> Either way, you should update MAINTAINERS file to record
>>> this newly added filename, against the bochs entry. The
>>> same applies to most other patches in this series adding
>>> new files.
>>>
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> So, something like:
>>
>> block/drivers/bochs/
>>
>> bochs.c
>> probe.c (or bochs-probe.c)
>>
>> and
>>
>> include/block/drivers/bochs/
>>
>> common.h (or internal.h)
>>
>>
>> Any objections from the gallery?
> 
> Yea (or “Nay”?). I'd rather not have as many directories in block/ as we
> have files there right now and in most of these directories just two
> files, for two reasons:
> 
> (1) I don't want it, because of my personal taste. If you just did it, I
> probably wouldn't complain for too long, though.
> 
> (2) Code motion shouldn't be done without a good reason. I know this is
> of no concern to upstream (which we are talking about), but it's always
> iffy when it comes to backports. And I am a Red Hat employee, so I am
> paid to think about them.
> 

Reason: We haven't had modules before. Now we do. Shared constants and
structures need to go somewhere, probes need to get split out.

Now, existing files (that will become the modular portions) can stay put
if you'd like, but the probes and common includes need to go somewhere.

Block drivers will be more decentralized than they've ever been. 1-3
files per each driver, depending on if they have a probe or if they have
shared definitions that the probe needs to access.

This at least raises the question for organization to minimize future
confusion. The answer to that question might be "Please leave the core
modules/drivers alone," but the question gets asked.

> Also, there's another argument: As far as I know we sooner or later want
> to make probing some kind of a block driver anyway (i.e. if you choose
> the "probe" block driver, it'll automatically replace itself by the
> right one). So in that sense, one could actually argue that probing is a
> block driver.
> 

Doesn't really sound like an argument against the file layout you're
replying to.

> Max
> 

12 weeks isn't a very long time, so if you have a preferred
organizational structure, I'd prefer you present that instead of just a
NACK, or put your vote for the currently presented organization in this v3.

--js

Re: [Qemu-devel] [PATCH v3 01/32] blockdev: prepare iSCSI block driver for dynamic loading

[Fam Zheng]

On Tue, 07/05 11:24, Colin Lord wrote:
> 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>
> ---
>  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 9bb5ff6..8246075 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -1880,45 +1880,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 9bb7f4c..cb640c2 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
>   *
> @@ -2982,6 +3017,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
> 
> 

Reviewed-by: Fam Zheng <famz@redhat.com>

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Kevin Wolf]

Am 05.07.2016 um 22:50 hat John Snow geschrieben:
> 
> 
> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
> > On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
> >> This puts the bochs probe function into its own separate file as part of
> >> the process of modularizing block drivers. Having the probe functions
> >> separate from the rest of the driver allows us to probe without having
> >> to potentially unnecessarily load the driver.
> >>
> >> Signed-off-by: Colin Lord <clord@redhat.com>
> >> ---
> >>  block/Makefile.objs          |  1 +
> >>  block/bochs.c                | 55 ++------------------------------------------
> >>  block/probe/bochs.c          | 21 +++++++++++++++++
> > 
> > Do we really need a sub-dir for this ?  If we were going to
> > have sub-dirs under block/, I'd suggest we have one subdir
> > per block driver, not spread code for one block driver
> > across multiple dirs.
> > 
> 
> Admittedly I have been nudging Colin to shoot from the hip a bit on
> filename organization because I was short of ideas.
> 
> Some ideas:
> 
> (1) A combined probe.c file. This keeps the existing organization and
> localizes everything to just one new file.
> 
> Downside: many formats rely on at least some minimal amount of structure
> and constant definitions, and some of those overlap with each other.
> qcow and qcow2 both using "QcowHeader" would be a prominent example.
> 
> They could all be disentangled, but it's less clear on where all the
> common definitions go. A common probe.h is a bad idea since the modular
> portion of the driver has no business gaining access to other formats'
> definitions. We could create a probe.c and matching
> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
> 
> (2) Separate probe files for each driver.
> 
> What we went with. Keeps refactoring to a minimum. Adds a bunch of
> little files, but it's minimal and fairly noninvasive.
> 
> Like #1 though, we still have to figure out what to do with the common
> includes.
> 
> > IMHO a block/bochs-probe.c would be better, unless we did
> > move block/bochs.c into a block/bochs/driver.c dir.
> > 
> > Either way, you should update MAINTAINERS file to record
> > this newly added filename, against the bochs entry. The
> > same applies to most other patches in this series adding
> > new files.
> > 
> > 
> > Regards,
> > Daniel
> > 
> 
> So, something like:
> 
> block/drivers/bochs/

block/bochs/ if anything. We don't have to nest deeply just because we
can. I don't really like new subdirectories, but when all drivers start
to have multiple files, it might be unavoidable.

> bochs.c
> probe.c (or bochs-probe.c)
> 
> and
> 
> include/block/drivers/bochs/
> 
> common.h (or internal.h)

block/bochs/internal.h (or bochs.h)

Just like we already have some header files directly in block/ (e.g.
qcow2.h). They are internal to the block driver, so no reason to move
them to the global include directory.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Max Reitz]

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

On 05.07.2016 23:12, John Snow wrote:
> 
> 
> On 07/05/2016 05:00 PM, Max Reitz wrote:
>> On 05.07.2016 22:50, John Snow wrote:
>>>
>>>
>>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>>> This puts the bochs probe function into its own separate file as part of
>>>>> the process of modularizing block drivers. Having the probe functions
>>>>> separate from the rest of the driver allows us to probe without having
>>>>> to potentially unnecessarily load the driver.
>>>>>
>>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>>> ---
>>>>>  block/Makefile.objs          |  1 +
>>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>>
>>>> Do we really need a sub-dir for this ?  If we were going to
>>>> have sub-dirs under block/, I'd suggest we have one subdir
>>>> per block driver, not spread code for one block driver
>>>> across multiple dirs.
>>>>
>>>
>>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>>> filename organization because I was short of ideas.
>>>
>>> Some ideas:
>>>
>>> (1) A combined probe.c file. This keeps the existing organization and
>>> localizes everything to just one new file.
>>>
>>> Downside: many formats rely on at least some minimal amount of structure
>>> and constant definitions, and some of those overlap with each other.
>>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>>
>>> They could all be disentangled, but it's less clear on where all the
>>> common definitions go. A common probe.h is a bad idea since the modular
>>> portion of the driver has no business gaining access to other formats'
>>> definitions. We could create a probe.c and matching
>>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>>
>>> (2) Separate probe files for each driver.
>>>
>>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>>> little files, but it's minimal and fairly noninvasive.
>>>
>>> Like #1 though, we still have to figure out what to do with the common
>>> includes.
>>>
>>>> IMHO a block/bochs-probe.c would be better, unless we did
>>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>>
>>>> Either way, you should update MAINTAINERS file to record
>>>> this newly added filename, against the bochs entry. The
>>>> same applies to most other patches in this series adding
>>>> new files.
>>>>
>>>>
>>>> Regards,
>>>> Daniel
>>>>
>>>
>>> So, something like:
>>>
>>> block/drivers/bochs/
>>>
>>> bochs.c
>>> probe.c (or bochs-probe.c)
>>>
>>> and
>>>
>>> include/block/drivers/bochs/
>>>
>>> common.h (or internal.h)
>>>
>>>
>>> Any objections from the gallery?
>>
>> Yea (or “Nay”?). I'd rather not have as many directories in block/ as we
>> have files there right now and in most of these directories just two
>> files, for two reasons:
>>
>> (1) I don't want it, because of my personal taste. If you just did it, I
>> probably wouldn't complain for too long, though.
>>
>> (2) Code motion shouldn't be done without a good reason. I know this is
>> of no concern to upstream (which we are talking about), but it's always
>> iffy when it comes to backports. And I am a Red Hat employee, so I am
>> paid to think about them.
>>
> 
> Reason: We haven't had modules before. Now we do. Shared constants and
> structures need to go somewhere, probes need to get split out.
> 
> Now, existing files (that will become the modular portions) can stay put
> if you'd like, but the probes and common includes need to go somewhere.
> 
> Block drivers will be more decentralized than they've ever been. 1-3
> files per each driver, depending on if they have a probe or if they have
> shared definitions that the probe needs to access.
> 
> This at least raises the question for organization to minimize future
> confusion. The answer to that question might be "Please leave the core
> modules/drivers alone," but the question gets asked.
> 
>> Also, there's another argument: As far as I know we sooner or later want
>> to make probing some kind of a block driver anyway (i.e. if you choose
>> the "probe" block driver, it'll automatically replace itself by the
>> right one). So in that sense, one could actually argue that probing is a
>> block driver.
>>
> 
> Doesn't really sound like an argument against the file layout you're
> replying to.
> 
>> Max
>>
> 
> 12 weeks isn't a very long time, so if you have a preferred
> organizational structure, I'd prefer you present that instead of just a
> NACK, or put your vote for the currently presented organization in this v3.

I liked block/probe/.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 02/32] blockdev: Add dynamic generation of module_block.h

[Max Reitz]

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

On 05.07.2016 17:24, 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.

But the script itself doesn't follow it. :-P
("ERROR: code indent should never use tabs", to quote checkpatch.pl)

> 
> Signed-off-by: Marc Marí <markmb@redhat.com>
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  Makefile                        |   7 +++
>  scripts/modules/module_block.py | 122 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 129 insertions(+)
>  create mode 100644 scripts/modules/module_block.py
> 

[...]

> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
> new file mode 100644
> index 0000000..4075574
> --- /dev/null
> +++ b/scripts/modules/module_block.py
> @@ -0,0 +1,122 @@

[...]

> +def add_module(fheader, 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},')

I think our normal coding style would be met with:

text = '\n        '.join(lines)
fheader.write('\n    {\n        ' + text + '\n    },')

Rest looks good, so with that fixed (or maybe we don't need to fix it
because it's just a generated header, I don't know):

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 03/32] blockdev: Add dynamic module loading for block drivers

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> From: Marc Mari <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.
> 
> 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>
> ---
>  block.c               | 110 ++++++++++++++++++++++++++++++++++++++++++--------
>  include/qemu/module.h |   3 ++
>  util/module.c         |  38 +++++------------
>  3 files changed, 108 insertions(+), 43 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/block.c b/block.c
> index f4648e9..88a05b2 100644
> --- a/block.c
> +++ b/block.c

[...]

> +    for (n = 0; n < ARRAY_SIZE(block_driver_modules); n++) {

(yay for post-increment)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> This puts the bochs probe function into its own separate file as part of
> the process of modularizing block drivers. Having the probe functions
> separate from the rest of the driver allows us to probe without having
> to potentially unnecessarily load the driver.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs          |  1 +
>  block/bochs.c                | 55 ++------------------------------------------
>  block/probe/bochs.c          | 21 +++++++++++++++++
>  include/block/driver/bochs.h | 40 ++++++++++++++++++++++++++++++++
>  include/block/probe.h        |  6 +++++
>  5 files changed, 70 insertions(+), 53 deletions(-)
>  create mode 100644 block/probe/bochs.c
>  create mode 100644 include/block/driver/bochs.h
>  create mode 100644 include/block/probe.h
> 

[...]

> diff --git a/block/probe/bochs.c b/block/probe/bochs.c
> new file mode 100644
> index 0000000..8adc09f
> --- /dev/null
> +++ b/block/probe/bochs.c
> @@ -0,0 +1,21 @@
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "block/probe.h"
> +#include "block/driver/bochs.h"
> +
> +int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const struct bochs_header *bochs = (const void *)buf;
> +
> +    if (buf_size < HEADER_SIZE)
> +	return 0;
> +
> +    if (!strcmp(bochs->magic, HEADER_MAGIC) &&
> +	!strcmp(bochs->type, REDOLOG_TYPE) &&
> +	!strcmp(bochs->subtype, GROWING_TYPE) &&
> +	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
> +	(le32_to_cpu(bochs->version) == HEADER_V1)))
> +	return 100;
> +
> +    return 0;
> +}

Not sure what to do about the coding style here. Some people prefer code
movement to be pure, but I feel bad about doing code movement and then
leaving the code wrongly formatted. That is, in my opinion, every patch
should pass checkpatch.pl (but I won't object to a patch that doesn't
pass checkpatch.pl simply because of pre-existing code).

> diff --git a/include/block/driver/bochs.h b/include/block/driver/bochs.h
> new file mode 100644
> index 0000000..cd87256
> --- /dev/null
> +++ b/include/block/driver/bochs.h

Kevin's point that we maybe should just put this into block/ itself
(just like block/qcow2.h) is not a bad one, but I'm fine either way.

> @@ -0,0 +1,40 @@
> +#ifndef BOCHS_H
> +#define BOCHS_H

Maybe BLOCK_BOCHS_H would be better, considering that Bochs is primarily
a system emulator and its image format doesn't really have an own name.

(Compare block/qcow2.h, which uses BLOCK_QCOW2_H.)

Independently of what you decide to do in any of these three places:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 05/32] blockdev: Move cloop probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates cloop probing function as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/cloop.c         | 16 +---------------
>  block/probe/cloop.c   | 17 +++++++++++++++++
>  include/block/probe.h |  1 +
>  4 files changed, 20 insertions(+), 16 deletions(-)
>  create mode 100644 block/probe/cloop.c

Reviewed-by: Max Reitz <mreitz@redhat.com>

[...]

> diff --git a/block/probe/cloop.c b/block/probe/cloop.c
> new file mode 100644
> index 0000000..955c29c
> --- /dev/null
> +++ b/block/probe/cloop.c
> @@ -0,0 +1,17 @@
> +#include "qemu/osdep.h"
> +#include "block/probe.h"
> +
> +int cloop_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const char *magic_version_2_0 = "#!/bin/sh\n"
> +        "#V2.0 Format\n"
> +        "modprobe cloop file=$0 && mount -r -t iso9660 /dev/cloop $1\n";
> +    int length = strlen(magic_version_2_0);
> +    if (length > buf_size) {
> +        length = buf_size;

Pre-existing, but this seems a bit fishy to me. But, well, as long as
buf_size (BLOCK_PROBE_BUF_SIZE) will actually always be at least the
size of that string...

> +    }
> +    if (!memcmp(magic_version_2_0, buf, length)) {
> +        return 2;
> +    }
> +    return 0;
> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 06/32] blockdev: Move luks probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates the luks probe function as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/crypto.c        | 21 +--------------------
>  block/probe/luks.c    | 23 +++++++++++++++++++++++
>  include/block/probe.h |  2 ++
>  4 files changed, 27 insertions(+), 21 deletions(-)
>  create mode 100644 block/probe/luks.c

With the file renamed to crypto.c:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 07/32] blockdev: Move dmg probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolate dmg probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/dmg.c           | 16 +---------------
>  block/probe/dmg.c     | 17 +++++++++++++++++
>  include/block/probe.h |  1 +
>  4 files changed, 20 insertions(+), 16 deletions(-)
>  create mode 100644 block/probe/dmg.c

Reviewed-by: Max Reitz <mreitz@redhat.com>

[...]

> diff --git a/block/probe/dmg.c b/block/probe/dmg.c
> new file mode 100644
> index 0000000..a40281b
> --- /dev/null
> +++ b/block/probe/dmg.c
> @@ -0,0 +1,17 @@
> +#include "qemu/osdep.h"
> +#include "block/probe.h"
> +
> +int dmg_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    int len;
> +
> +    if (!filename) {
> +        return 0;
> +    }
> +
> +    len = strlen(filename);
> +    if (len > 4 && !strcmp(filename + len - 4, ".dmg")) {

Reading this probing code makes me feel all funny inside. :-)

> +        return 2;
> +    }
> +    return 0;
> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 08/32] blockdev: Move parallels probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolate parallels probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs              |  1 +
>  block/parallels.c                | 43 ++--------------------------------------
>  block/probe/parallels.c          | 22 ++++++++++++++++++++
>  include/block/driver/parallels.h | 26 ++++++++++++++++++++++++
>  include/block/probe.h            |  1 +
>  5 files changed, 52 insertions(+), 41 deletions(-)
>  create mode 100644 block/probe/parallels.c
>  create mode 100644 include/block/driver/parallels.h

[...]

> diff --git a/block/probe/parallels.c b/block/probe/parallels.c
> new file mode 100644
> index 0000000..66cddea
> --- /dev/null
> +++ b/block/probe/parallels.c
> @@ -0,0 +1,22 @@
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "block/probe.h"
> +#include "block/driver/parallels.h"
> +
> +int parallels_probe(const uint8_t *buf, int buf_size,
> +                           const char *filename)

This line should be aligned to the opening parenthesis; or you can just
put it in on a single line.

> +{
> +    const ParallelsHeader *ph = (const void *)buf;
> +
> +    if (buf_size < sizeof(ParallelsHeader)) {
> +        return 0;
> +    }
> +
> +    if ((!memcmp(ph->magic, HEADER_MAGIC, 16) ||
> +           !memcmp(ph->magic, HEADER_MAGIC2, 16)) &&
> +           (le32_to_cpu(ph->version) == HEADER_VERSION)) {

So should these two lines probably, but I don't care much about them
because this is a pre-existing issue.

> +        return 100;
> +    }
> +
> +    return 0;
> +}
> diff --git a/include/block/driver/parallels.h b/include/block/driver/parallels.h
> new file mode 100644
> index 0000000..512ef5f
> --- /dev/null
> +++ b/include/block/driver/parallels.h
> @@ -0,0 +1,26 @@
> +#ifndef PARALLELS_H
> +#define PARALLELS_H
> +
> +#define HEADER_MAGIC "WithoutFreeSpace"
> +#define HEADER_MAGIC2 "WithouFreSpacExt"

I'm somehow very amused by this second magic string ("We want to append
an Ext, but that means the buffer is three chars short... Thank god the
rest of the magic string consists of exactly three words, so let's just
strip a letter off of each!").

> +#define HEADER_VERSION 2
> +#define HEADER_INUSE_MAGIC  (0x746F6E59)
> +
> +#define DEFAULT_CLUSTER_SIZE 1048576        /* 1 MiB */
> +
> +
> +// always little-endian
> +typedef struct ParallelsHeader {
> +    char magic[16]; // "WithoutFreeSpace"

checkpatch complains about the C++-style comments, but this is again the
question of whether we have to fix pre-existing coding style issues here.

> +    uint32_t version;
> +    uint32_t heads;
> +    uint32_t cylinders;
> +    uint32_t tracks;
> +    uint32_t bat_entries;
> +    uint64_t nb_sectors;
> +    uint32_t inuse;
> +    uint32_t data_off;
> +    char padding[12];
> +} QEMU_PACKED ParallelsHeader;
> +
> +#endif

With the function header of parallels_probe() fixed:

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 09/32] blockdev: Move qcow probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates qcow probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs         |  2 +-
>  block/probe/qcow.c          | 16 ++++++++++++++++
>  block/qcow.c                | 31 ++-----------------------------
>  include/block/driver/qcow.h | 21 +++++++++++++++++++++
>  include/block/probe.h       |  1 +
>  5 files changed, 41 insertions(+), 30 deletions(-)
>  create mode 100644 block/probe/qcow.c
>  create mode 100644 include/block/driver/qcow.h

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/block/probe/qcow.c b/block/probe/qcow.c
> new file mode 100644
> index 0000000..6024d11
> --- /dev/null
> +++ b/block/probe/qcow.c
> @@ -0,0 +1,16 @@
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "block/probe.h"
> +#include "block/driver/qcow.h"
> +
> +int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const QCowHeader *cow_header = (const void *)buf;
> +
> +    if (buf_size >= sizeof(QCowHeader) &&
> +        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
> +        be32_to_cpu(cow_header->version) == QCOW_VERSION)
> +        return 100;
> +    else
> +        return 0;

Again, pre-existing code style issues.

> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 10/32] blockdev: Move qcow2 probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates qcow2 probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/probe/qcow2.c   | 16 ++++++++++++++++
>  block/qcow2.c         | 13 +------------
>  include/block/probe.h |  1 +
>  4 files changed, 19 insertions(+), 13 deletions(-)
>  create mode 100644 block/probe/qcow2.c

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/block/probe/qcow2.c b/block/probe/qcow2.c
> new file mode 100644
> index 0000000..56f4e82
> --- /dev/null
> +++ b/block/probe/qcow2.c
> @@ -0,0 +1,16 @@
> +#include "qemu/osdep.h"
> +#include "block/block_int.h"
> +#include "block/probe.h"
> +#include "block/qcow2.h"
> +
> +int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    const QCowHeader *cow_header = (const void *)buf;
> +
> +    if (buf_size >= sizeof(QCowHeader) &&
> +        be32_to_cpu(cow_header->magic) == QCOW_MAGIC &&
> +        be32_to_cpu(cow_header->version) >= 2)
> +        return 100;
> +    else
> +        return 0;

...aaand even more of this...

> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 11/32] blockdev: Move qed probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolate qed probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/probe/qed.c     | 18 ++++++++++++++++++
>  block/qed.c           | 15 +--------------
>  include/block/probe.h |  1 +
>  4 files changed, 21 insertions(+), 15 deletions(-)
>  create mode 100644 block/probe/qed.c

Reviewed-by: Max Reitz <mreitz@redhat.com>

> diff --git a/include/block/probe.h b/include/block/probe.h
> index f9dd36e..e3e9934 100644
> --- a/include/block/probe.h
> +++ b/include/block/probe.h
> @@ -9,5 +9,6 @@ int dmg_probe(const uint8_t *buf, int buf_size, const char *filename);
>  int parallels_probe(const uint8_t *buf, int buf_size, const char *filename);
>  int qcow_probe(const uint8_t *buf, int buf_size, const char *filename);
>  int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename);
> +int bdrv_qed_probe(const uint8_t *buf, int buf_size, const char *filename);

Hmmm, maybe it would make sense to follow a consistent naming scheme
here, i.e. either ${format}_probe() or bdrv_${format}_probe(). The
latter is probably better, but the former is what most formats do, so...

But maybe that is something that should be done in the "Separate X probe
from its driver" patches.

Max

>  
>  #endif
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 12/32] blockdev: Move raw probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolate raw probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  1 +
>  block/probe/raw.c     | 10 ++++++++++
>  block/raw_bsd.c       |  9 +--------
>  include/block/probe.h |  1 +
>  4 files changed, 13 insertions(+), 8 deletions(-)
>  create mode 100644 block/probe/raw.c

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 13/32] blockdev: Move vdi probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates vdi probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs        |  2 +-
>  block/probe/vdi.c          | 26 +++++++++++++++++
>  block/vdi.c                | 69 ++--------------------------------------------
>  include/block/driver/vdi.h | 49 ++++++++++++++++++++++++++++++++
>  include/block/probe.h      |  1 +
>  5 files changed, 79 insertions(+), 68 deletions(-)
>  create mode 100644 block/probe/vdi.c
>  create mode 100644 include/block/driver/vdi.h

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 14/32] blockdev: Move vhdx probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates vhdx probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   |  2 +-
>  block/probe/vhdx.c    | 21 +++++++++++++++++++++
>  block/vhdx.c          | 20 +-------------------
>  include/block/probe.h |  1 +
>  4 files changed, 24 insertions(+), 20 deletions(-)
>  create mode 100644 block/probe/vhdx.c

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 15/32] blockdev: Move vmdk probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates vmdk probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs         |  2 +-
>  block/probe/vmdk.c          | 60 +++++++++++++++++++++++++++++++++++++++++++++
>  block/vmdk.c                | 60 ++-------------------------------------------
>  include/block/driver/vmdk.h |  7 ++++++
>  include/block/probe.h       |  1 +
>  5 files changed, 71 insertions(+), 59 deletions(-)
>  create mode 100644 block/probe/vmdk.c
>  create mode 100644 include/block/driver/vmdk.h

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 16/32] blockdev: Move vpc probe to its own file

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Isolates vpc probe as part of the modularization process.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs   | 2 +-
>  block/probe/vpc.c     | 9 +++++++++
>  block/vpc.c           | 8 +-------
>  include/block/probe.h | 1 +
>  4 files changed, 12 insertions(+), 8 deletions(-)
>  create mode 100644 block/probe/vpc.c

Reviewed-by: Max Reitz <mreitz@redhat.com>

[...]

> diff --git a/block/probe/vpc.c b/block/probe/vpc.c
> new file mode 100644
> index 0000000..afe8271
> --- /dev/null
> +++ b/block/probe/vpc.c
> @@ -0,0 +1,9 @@
> +#include "qemu/osdep.h"
> +#include "block/probe.h"
> +
> +int vpc_probe(const uint8_t *buf, int buf_size, const char *filename)
> +{
> +    if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8))
> +	return 100;

*cough*

> +    return 0;
> +}


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Max Reitz]

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

On 06.07.2016 16:19, Max Reitz wrote:
> On 05.07.2016 17:24, Colin Lord wrote:
>> This puts the bochs probe function into its own separate file as part of
>> the process of modularizing block drivers. Having the probe functions
>> separate from the rest of the driver allows us to probe without having
>> to potentially unnecessarily load the driver.
>>
>> Signed-off-by: Colin Lord <clord@redhat.com>
>> ---
>>  block/Makefile.objs          |  1 +
>>  block/bochs.c                | 55 ++------------------------------------------
>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>  include/block/driver/bochs.h | 40 ++++++++++++++++++++++++++++++++
>>  include/block/probe.h        |  6 +++++
>>  5 files changed, 70 insertions(+), 53 deletions(-)
>>  create mode 100644 block/probe/bochs.c
>>  create mode 100644 include/block/driver/bochs.h
>>  create mode 100644 include/block/probe.h
>>
> 
> [...]
> 
>> diff --git a/block/probe/bochs.c b/block/probe/bochs.c
>> new file mode 100644
>> index 0000000..8adc09f
>> --- /dev/null
>> +++ b/block/probe/bochs.c
>> @@ -0,0 +1,21 @@
>> +#include "qemu/osdep.h"
>> +#include "block/block_int.h"
>> +#include "block/probe.h"
>> +#include "block/driver/bochs.h"
>> +
>> +int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
>> +{
>> +    const struct bochs_header *bochs = (const void *)buf;
>> +
>> +    if (buf_size < HEADER_SIZE)
>> +	return 0;
>> +
>> +    if (!strcmp(bochs->magic, HEADER_MAGIC) &&
>> +	!strcmp(bochs->type, REDOLOG_TYPE) &&
>> +	!strcmp(bochs->subtype, GROWING_TYPE) &&
>> +	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
>> +	(le32_to_cpu(bochs->version) == HEADER_V1)))
>> +	return 100;
>> +
>> +    return 0;
>> +}
> 
> Not sure what to do about the coding style here. Some people prefer code
> movement to be pure, but I feel bad about doing code movement and then
> leaving the code wrongly formatted. That is, in my opinion, every patch
> should pass checkpatch.pl (but I won't object to a patch that doesn't
> pass checkpatch.pl simply because of pre-existing code).

I've just seen that you indeed fix this later in this series. That's
good enough for me.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Modifies the bochs probe to return the format name as well as the
> score as the final step of separating the probe function from the
> driver. This keeps the probe completely independent of the driver,
> making future modularization easier to accomplish. Returning the format
> name as well as the score allows the score to be correlated to the
> driver without the probe function needing to be part of the driver.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               | 19 +++++++++++++++++++
>  block/bochs.c         |  1 -
>  block/probe/bochs.c   | 25 ++++++++++++++++---------
>  include/block/probe.h |  3 ++-
>  4 files changed, 37 insertions(+), 11 deletions(-)

As I've proposed before, maybe this would be a good place to rename the
probing function to bdrv_bochs_probe() since it is no longer a static
function.

> 
> diff --git a/block.c b/block.c
> index 88a05b2..eab8a6e 100644
> --- a/block.c
> +++ b/block.c
> @@ -25,6 +25,7 @@
>  #include "trace.h"
>  #include "block/block_int.h"
>  #include "block/blockjob.h"
> +#include "block/probe.h"
>  #include "qemu/error-report.h"
>  #include "module_block.h"
>  #include "qemu/module.h"
> @@ -56,6 +57,13 @@
>  
>  #define NOT_DONE 0x7fffffff /* used while emulated sync operation in progress */
>  
> +typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
> +                                  const char *filename, int *score);
> +
> +static BdrvProbeFunc *format_probes[] = {
> +    bochs_probe,
> +};

Works, but... Eh. Something like the following would suit my personal
tastes (I think by now everyone should have realized that I have
horrible taste) better:

typedef struct BdrvProbeFunc {
    const char *format_name;
    int (*probe)(const uint8_t *buf, int buf_size,
                 const char *filename);
} BdrvProbeFunc;

static BdrvProbeFunc *format_probes[] = {
    { "bochs", bochs_probe },
};

It just feels strange to me that the probing function always returns a
constant string.

(This is an optional suggestion, you don't need to follow it.)

> +
>  static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
>      QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
>  
> @@ -576,6 +584,8 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
>                              const char *filename)
>  {
>      int score_max = 0, score;
> +    const char *format_max = NULL;
> +    const char *format;
>      size_t i;
>      BlockDriver *drv = NULL, *d;
>  
> @@ -595,6 +605,15 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
>          }
>      }
>  
> +    for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
> +        format = format_probes[i](buf, buf_size, filename, &score);
> +        if (score > score_max) {
> +            score_max = score;
> +            format_max = format;
> +            drv = bdrv_find_format(format_max);
> +        }
> +    }

I think the bdrv_find_format() call should be done after the loop
(otherwise we may unnecessarily load some formats which we then actually
don't use).

> +
>      return drv;
>  }

[...]

> diff --git a/block/probe/bochs.c b/block/probe/bochs.c
> index 8adc09f..8206930 100644
> --- a/block/probe/bochs.c
> +++ b/block/probe/bochs.c
> @@ -3,19 +3,26 @@
>  #include "block/probe.h"
>  #include "block/driver/bochs.h"
>  
> -int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
> +const char *bochs_probe(const uint8_t *buf, int buf_size, const char *filename,
> +                        int *score)
>  {
> +    const char *format = "bochs";
>      const struct bochs_header *bochs = (const void *)buf;
> +    assert(score);
> +    *score = 0;
>  
> -    if (buf_size < HEADER_SIZE)
> -	return 0;
> +    if (buf_size < HEADER_SIZE) {
> +        return format;
> +    }
>  
>      if (!strcmp(bochs->magic, HEADER_MAGIC) &&
> -	!strcmp(bochs->type, REDOLOG_TYPE) &&
> -	!strcmp(bochs->subtype, GROWING_TYPE) &&
> -	((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
> -	(le32_to_cpu(bochs->version) == HEADER_V1)))
> -	return 100;
> +        !strcmp(bochs->type, REDOLOG_TYPE) &&
> +        !strcmp(bochs->subtype, GROWING_TYPE) &&
> +        ((le32_to_cpu(bochs->version) == HEADER_VERSION) ||
> +        (le32_to_cpu(bochs->version) == HEADER_V1))) {
> +        *score = 100;
> +        return format;
> +    }

Ah. Seems like I've complained too early. :-)

Max

>  
> -    return 0;
> +    return format;
>  }


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Modifies the bochs probe to return the format name as well as the
> score as the final step of separating the probe function from the
> driver. This keeps the probe completely independent of the driver,
> making future modularization easier to accomplish. Returning the format
> name as well as the score allows the score to be correlated to the
> driver without the probe function needing to be part of the driver.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               | 19 +++++++++++++++++++
>  block/bochs.c         |  1 -
>  block/probe/bochs.c   | 25 ++++++++++++++++---------
>  include/block/probe.h |  3 ++-
>  4 files changed, 37 insertions(+), 11 deletions(-)

Oh, and another comment: I think it would be better to split this patch
into a part that modifies block.c (introducing the format_probes array
and using it in bdrv_probe_all()) and another one that actually does the
bochs-specific stuff.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 18/32] blockdev: Separate cloop probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the cloop probe from the cloop driver. The
> cloop probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/cloop.c         |  1 -
>  block/probe/cloop.c   | 11 ++++++++---
>  include/block/probe.h |  3 ++-
>  4 files changed, 11 insertions(+), 5 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 19/32] blockdev: Separate luks probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the luks probe from the crypto driver. The
> luks probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/crypto.c        |  1 -
>  block/probe/luks.c    | 13 ++++++++-----
>  include/block/probe.h |  4 ++--
>  4 files changed, 11 insertions(+), 8 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 20/32] blockdev: Separate dmg probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the dmg probe from the dmg driver. The dmg
> probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/dmg.c           |  1 -
>  block/probe/dmg.c     | 13 +++++++++----
>  include/block/probe.h |  3 ++-
>  4 files changed, 12 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 21/32] blockdev: Separate parallels probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the parallels probe from the parallels
> driver. The parallels probe now returns the format in addition to the
> score, allowing correlation of the score and driver without the probe
> function being part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c                 |  1 +
>  block/parallels.c       |  1 -
>  block/probe/parallels.c | 18 +++++++++++-------
>  include/block/probe.h   |  3 ++-
>  4 files changed, 14 insertions(+), 9 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[John Snow]

On 07/06/2016 04:24 AM, Kevin Wolf wrote:
> Am 05.07.2016 um 22:50 hat John Snow geschrieben:
>>
>>
>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>> This puts the bochs probe function into its own separate file as part of
>>>> the process of modularizing block drivers. Having the probe functions
>>>> separate from the rest of the driver allows us to probe without having
>>>> to potentially unnecessarily load the driver.
>>>>
>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>> ---
>>>>  block/Makefile.objs          |  1 +
>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>
>>> Do we really need a sub-dir for this ?  If we were going to
>>> have sub-dirs under block/, I'd suggest we have one subdir
>>> per block driver, not spread code for one block driver
>>> across multiple dirs.
>>>
>>
>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>> filename organization because I was short of ideas.
>>
>> Some ideas:
>>
>> (1) A combined probe.c file. This keeps the existing organization and
>> localizes everything to just one new file.
>>
>> Downside: many formats rely on at least some minimal amount of structure
>> and constant definitions, and some of those overlap with each other.
>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>
>> They could all be disentangled, but it's less clear on where all the
>> common definitions go. A common probe.h is a bad idea since the modular
>> portion of the driver has no business gaining access to other formats'
>> definitions. We could create a probe.c and matching
>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>
>> (2) Separate probe files for each driver.
>>
>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>> little files, but it's minimal and fairly noninvasive.
>>
>> Like #1 though, we still have to figure out what to do with the common
>> includes.
>>
>>> IMHO a block/bochs-probe.c would be better, unless we did
>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>
>>> Either way, you should update MAINTAINERS file to record
>>> this newly added filename, against the bochs entry. The
>>> same applies to most other patches in this series adding
>>> new files.
>>>
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> So, something like:
>>
>> block/drivers/bochs/
> 
> block/bochs/ if anything. We don't have to nest deeply just because we
> can. I don't really like new subdirectories, but when all drivers start
> to have multiple files, it might be unavoidable.
> 
>> bochs.c
>> probe.c (or bochs-probe.c)
>>
>> and
>>
>> include/block/drivers/bochs/
>>
>> common.h (or internal.h)
> 
> block/bochs/internal.h (or bochs.h)
> 
> Just like we already have some header files directly in block/ (e.g.
> qcow2.h). They are internal to the block driver, so no reason to move
> them to the global include directory.
> 
> Kevin
> 

I was actually curious about this. [CCing Markus, our new #include Czar.
[or Kaiser?]]

Recently the include files in hw/ide/ were moved to include/ without
anybody mentioning it to me, including internal.h.

Why?

I don't know.

--js

Re: [Qemu-devel] [PATCH v3 22/32] blockdev: Separate qcow probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the qcow probe from the qcow driver. The
> qcow probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/qcow.c    | 16 +++++++++++-----
>  block/qcow.c          |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 14 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 23/32] blockdev: Separate qcow2 probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the qcow2 probe from the qcow2 driver. The
> qcow2 probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/qcow2.c   | 16 +++++++++++-----
>  block/qcow2.c         |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 14 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 24/32] blockdev: Separate qed probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the qed probe from the qed driver. The
> qed probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/qed.c     | 16 +++++++++++-----
>  block/qed.c           |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 14 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 25/32] blockdev: Separate raw probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the raw probe from the raw driver. The
> raw probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               | 1 +
>  block/probe/raw.c     | 8 ++++++--
>  block/raw_bsd.c       | 1 -
>  include/block/probe.h | 3 ++-
>  4 files changed, 9 insertions(+), 4 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 26/32] blockdev: Separate vdi probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the vdi probe from the vdi driver. The
> vdi probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/vdi.c     | 13 ++++++++-----
>  block/vdi.c           |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 11 insertions(+), 7 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 27/32] blockdev: Separate vhdx probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the vhdx probe from the vhdx driver. The
> vhdx probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/vhdx.c    | 12 +++++++++---
>  block/vhdx.c          |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 12 insertions(+), 5 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 28/32] blockdev: Separate vmdk probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the vmdk probe from the vmdk driver. The
> vmdk probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/vmdk.c    | 23 +++++++++++++++--------
>  block/vmdk.c          |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 18 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 29/32] blockdev: Separate vpc probe from its driver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> Completes the separation of the vpc probe from the vpc driver. The
> vpc probe now returns the format in addition to the score, allowing
> correlation of the score and driver without the probe function being
> part of the driver itself.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c               |  1 +
>  block/probe/vpc.c     | 15 +++++++++++----
>  block/vpc.c           |  1 -
>  include/block/probe.h |  3 ++-
>  4 files changed, 14 insertions(+), 6 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 30/32] blockdev: Remove the .bdrv_probe field from BlockDrivers

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> This commit finalizes the separation of the block driver and probe
> function by removing the .bdrv_probe field from all BlockDrivers.
> Probing is now accomplished solely by iterating over the array of probe
> function pointers in the format_probes array.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c                         | 20 +-------------------
>  block/raw-posix.c               |  1 -
>  include/block/block_int.h       |  1 -
>  scripts/modules/module_block.py | 10 ++--------
>  4 files changed, 3 insertions(+), 29 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 8226124..7e441fe 100644
> --- a/block.c
> +++ b/block.c
> @@ -599,34 +599,16 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
>      const char *format_max = NULL;
>      const char *format;
>      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);
> -            if (score > score_max) {
> -                score_max = score;
> -                drv = d;
> -            }
> -        }
> -    }
>  
>      for (i = 0; i < ARRAY_SIZE(format_probes); i++) {
>          format = format_probes[i](buf, buf_size, filename, &score);
>          if (score > score_max) {
>              score_max = score;
>              format_max = format;
> -            drv = bdrv_find_format(format_max);
>          }
>      }
>  
> -    return drv;
> +    return bdrv_find_format(format_max);

OK, so you move that function call here. Then, I'd at least like to see
a comment in patch 17 that you are going to do this later (like "TODO:
Move this call outside of this loop").

For this patch, however:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>  }
>  
>  static int find_image_format(BlockDriverState *bs, const char *filename,


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 31/32] blockdev: Separate out bdrv_probe_device functions

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> This puts the bdrv_probe_device functions into their own files to
> facilitate the modularization of the block drivers.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block/Makefile.objs       |  1 +
>  block/probe/host_cdrom.c  | 40 +++++++++++++++++++++++++++++++++++++
>  block/probe/host_device.c | 30 ++++++++++++++++++++++++++++
>  block/raw-posix.c         | 51 +----------------------------------------------
>  block/raw-win32.c         | 10 +---------
>  include/block/probe.h     |  2 ++
>  6 files changed, 75 insertions(+), 59 deletions(-)
>  create mode 100644 block/probe/host_cdrom.c
>  create mode 100644 block/probe/host_device.c

Reviewed-by: Max Reitz <mreitz@redhat.com>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 32/32] blockdev: Remove bdrv_probe_device field from BlockDriver

[Max Reitz]

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

On 05.07.2016 17:24, Colin Lord wrote:
> This commit finalizes the separation of the BlockDriver from its
> device probing function. Now the accesses to these functions in block.c
> occur through the protocol_probes array, and each function returns a
> score and protocol name with which to find the corresponding driver.
> 
> Signed-off-by: Colin Lord <clord@redhat.com>
> ---
>  block.c                         | 46 ++++++++++++++++++++++++++++++-----------
>  block/probe/host_cdrom.c        | 23 ++++++++++++++-------
>  block/probe/host_device.c       | 34 ++++++++++++++++++++----------
>  block/raw-posix.c               |  3 ---
>  block/raw-win32.c               |  1 -
>  include/block/block_int.h       |  2 --
>  include/block/probe.h           |  4 ++--
>  scripts/modules/module_block.py | 12 ++---------
>  8 files changed, 76 insertions(+), 49 deletions(-)

As I suggested for patch 17, I'd split the additions to block.c from the
conversion of the actual probe functions.

> diff --git a/block.c b/block.c
> index 7e441fe..bc1046b 100644
> --- a/block.c
> +++ b/block.c
> @@ -59,6 +59,7 @@
>  
>  typedef const char *BdrvProbeFunc(const uint8_t *buf, int buf_size,
>                                    const char *filename, int *score);
> +typedef const char *BdrvProbeDevFunc(const char *filename, int *score);
>  
>  static BdrvProbeFunc *format_probes[] = {
>      bochs_probe,
> @@ -76,6 +77,13 @@ static BdrvProbeFunc *format_probes[] = {
>      vpc_probe
>  };
>  
> +static BdrvProbeDevFunc *protocol_probes[] = {
> +    hdev_probe_device,
> +#if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__linux__)
> +    cdrom_probe_device
> +#endif
> +};
> +

Same as what I've said in my reply to patch 17: I'd rather have a struct
that contains the name of the protocol and the probe function separated
from each other than have the probe function return the protocol name.

In case you decide not to follow this suggestion (nor the suggestion on
splitting this patch) though:

Reviewed-by: Max Reitz <mreitz@redhat.com>

>  static QTAILQ_HEAD(, BlockDriverState) graph_bdrv_states =
>      QTAILQ_HEAD_INITIALIZER(graph_bdrv_states);
>  


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 498 bytes --]

Re: [Qemu-devel] [PATCH v3 02/32] blockdev: Add dynamic generation of module_block.h

[Colin Lord]

On 07/06/2016 09:17 AM, Max Reitz wrote:
> On 05.07.2016 17:24, 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.
> 
> But the script itself doesn't follow it. :-P
> ("ERROR: code indent should never use tabs", to quote checkpatch.pl)
> 
I guess that is the case.

>>
>> Signed-off-by: Marc Marí <markmb@redhat.com>
>> Signed-off-by: Colin Lord <clord@redhat.com>
>> ---
>>  Makefile                        |   7 +++
>>  scripts/modules/module_block.py | 122 ++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 129 insertions(+)
>>  create mode 100644 scripts/modules/module_block.py
>>
> 
> [...]
> 
>> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
>> new file mode 100644
>> index 0000000..4075574
>> --- /dev/null
>> +++ b/scripts/modules/module_block.py
>> @@ -0,0 +1,122 @@
> 
> [...]
> 
>> +def add_module(fheader, 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},')
> 
> I think our normal coding style would be met with:
> 
> text = '\n        '.join(lines)
> fheader.write('\n    {\n        ' + text + '\n    },')
> 
> Rest looks good, so with that fixed (or maybe we don't need to fix it
> because it's just a generated header, I don't know):
> 
Shouldn't be too big a deal to fix.

> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 07/06/2016 04:24 AM, Kevin Wolf wrote:
>> Am 05.07.2016 um 22:50 hat John Snow geschrieben:
>>>
>>>
>>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>>> This puts the bochs probe function into its own separate file as part of
>>>>> the process of modularizing block drivers. Having the probe functions
>>>>> separate from the rest of the driver allows us to probe without having
>>>>> to potentially unnecessarily load the driver.
>>>>>
>>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>>> ---
>>>>>  block/Makefile.objs          |  1 +
>>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>>
>>>> Do we really need a sub-dir for this ?  If we were going to
>>>> have sub-dirs under block/, I'd suggest we have one subdir
>>>> per block driver, not spread code for one block driver
>>>> across multiple dirs.
>>>>
>>>
>>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>>> filename organization because I was short of ideas.
>>>
>>> Some ideas:
>>>
>>> (1) A combined probe.c file. This keeps the existing organization and
>>> localizes everything to just one new file.
>>>
>>> Downside: many formats rely on at least some minimal amount of structure
>>> and constant definitions, and some of those overlap with each other.
>>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>>
>>> They could all be disentangled, but it's less clear on where all the
>>> common definitions go. A common probe.h is a bad idea since the modular
>>> portion of the driver has no business gaining access to other formats'
>>> definitions. We could create a probe.c and matching
>>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>>
>>> (2) Separate probe files for each driver.
>>>
>>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>>> little files, but it's minimal and fairly noninvasive.
>>>
>>> Like #1 though, we still have to figure out what to do with the common
>>> includes.
>>>
>>>> IMHO a block/bochs-probe.c would be better, unless we did
>>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>>
>>>> Either way, you should update MAINTAINERS file to record
>>>> this newly added filename, against the bochs entry. The
>>>> same applies to most other patches in this series adding
>>>> new files.
>>>>
>>>>
>>>> Regards,
>>>> Daniel
>>>>
>>>
>>> So, something like:
>>>
>>> block/drivers/bochs/
>> 
>> block/bochs/ if anything. We don't have to nest deeply just because we
>> can. I don't really like new subdirectories, but when all drivers start
>> to have multiple files, it might be unavoidable.
>> 
>>> bochs.c
>>> probe.c (or bochs-probe.c)
>>>
>>> and
>>>
>>> include/block/drivers/bochs/
>>>
>>> common.h (or internal.h)
>> 
>> block/bochs/internal.h (or bochs.h)
>> 
>> Just like we already have some header files directly in block/ (e.g.
>> qcow2.h). They are internal to the block driver, so no reason to move
>> them to the global include directory.
>> 
>> Kevin
>> 
>
> I was actually curious about this. [CCing Markus, our new #include Czar.
> [or Kaiser?]]
>
> Recently the include files in hw/ide/ were moved to include/ without
> anybody mentioning it to me, including internal.h.
>
> Why?
>
> I don't know.

You're the maintainer, move them right back :)

If a header is only included from one directory, and that directory
actually has some thematic focus, then the header is probably private,
and should probably sit in that directory.

Else, the header is probably public, and should sit somewhere under
include/.

When we deviate from this rule, it's usually ugly.  Example:
include/block/block_int.h.

Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

[Colin Lord]

On 07/06/2016 11:59 AM, Max Reitz wrote:
> On 05.07.2016 17:24, Colin Lord wrote:
>> Modifies the bochs probe to return the format name as well as the
>> score as the final step of separating the probe function from the
>> driver. This keeps the probe completely independent of the driver,
>> making future modularization easier to accomplish. Returning the format
>> name as well as the score allows the score to be correlated to the
>> driver without the probe function needing to be part of the driver.
>>
>> Signed-off-by: Colin Lord <clord@redhat.com>
>> ---
>>  block.c               | 19 +++++++++++++++++++
>>  block/bochs.c         |  1 -
>>  block/probe/bochs.c   | 25 ++++++++++++++++---------
>>  include/block/probe.h |  3 ++-
>>  4 files changed, 37 insertions(+), 11 deletions(-)
> 
> Oh, and another comment: I think it would be better to split this patch
> into a part that modifies block.c (introducing the format_probes array
> and using it in bdrv_probe_all()) and another one that actually does the
> bochs-specific stuff.
> 
> Max
> 
I could do that, but if I leave the variable i as an unsigned type, I
get compiler warnings/errors that the for loop condition of
i < ARRAY_SIZE(format_probes)
must be false since the size of format_probes will be 0 (and unsigned is
always positive). Is it okay to change the type to int? Not sure how
strictly this aspect of the coding style gets enforced.

Colin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[John Snow]

On 07/07/2016 02:36 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 07/06/2016 04:24 AM, Kevin Wolf wrote:
>>> Am 05.07.2016 um 22:50 hat John Snow geschrieben:
>>>>
>>>>
>>>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>>>> This puts the bochs probe function into its own separate file as part of
>>>>>> the process of modularizing block drivers. Having the probe functions
>>>>>> separate from the rest of the driver allows us to probe without having
>>>>>> to potentially unnecessarily load the driver.
>>>>>>
>>>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>>>> ---
>>>>>>  block/Makefile.objs          |  1 +
>>>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>>>
>>>>> Do we really need a sub-dir for this ?  If we were going to
>>>>> have sub-dirs under block/, I'd suggest we have one subdir
>>>>> per block driver, not spread code for one block driver
>>>>> across multiple dirs.
>>>>>
>>>>
>>>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>>>> filename organization because I was short of ideas.
>>>>
>>>> Some ideas:
>>>>
>>>> (1) A combined probe.c file. This keeps the existing organization and
>>>> localizes everything to just one new file.
>>>>
>>>> Downside: many formats rely on at least some minimal amount of structure
>>>> and constant definitions, and some of those overlap with each other.
>>>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>>>
>>>> They could all be disentangled, but it's less clear on where all the
>>>> common definitions go. A common probe.h is a bad idea since the modular
>>>> portion of the driver has no business gaining access to other formats'
>>>> definitions. We could create a probe.c and matching
>>>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>>>
>>>> (2) Separate probe files for each driver.
>>>>
>>>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>>>> little files, but it's minimal and fairly noninvasive.
>>>>
>>>> Like #1 though, we still have to figure out what to do with the common
>>>> includes.
>>>>
>>>>> IMHO a block/bochs-probe.c would be better, unless we did
>>>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>>>
>>>>> Either way, you should update MAINTAINERS file to record
>>>>> this newly added filename, against the bochs entry. The
>>>>> same applies to most other patches in this series adding
>>>>> new files.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Daniel
>>>>>
>>>>
>>>> So, something like:
>>>>
>>>> block/drivers/bochs/
>>>
>>> block/bochs/ if anything. We don't have to nest deeply just because we
>>> can. I don't really like new subdirectories, but when all drivers start
>>> to have multiple files, it might be unavoidable.
>>>
>>>> bochs.c
>>>> probe.c (or bochs-probe.c)
>>>>
>>>> and
>>>>
>>>> include/block/drivers/bochs/
>>>>
>>>> common.h (or internal.h)
>>>
>>> block/bochs/internal.h (or bochs.h)
>>>
>>> Just like we already have some header files directly in block/ (e.g.
>>> qcow2.h). They are internal to the block driver, so no reason to move
>>> them to the global include directory.
>>>
>>> Kevin
>>>
>>
>> I was actually curious about this. [CCing Markus, our new #include Czar.
>> [or Kaiser?]]
>>
>> Recently the include files in hw/ide/ were moved to include/ without
>> anybody mentioning it to me, including internal.h.
>>
>> Why?
>>
>> I don't know.
> 
> You're the maintainer, move them right back :)
> 
> If a header is only included from one directory, and that directory
> actually has some thematic focus, then the header is probably private,
> and should probably sit in that directory.
> 
> Else, the header is probably public, and should sit somewhere under
> include/.
> 
> When we deviate from this rule, it's usually ugly.  Example:
> include/block/block_int.h.
> 

OK, thanks. Just making sure I didn't miss a memo on some more
militaristic #include paradigm.

Kevin's suggestion for organization sounds good.

--js

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Paolo Bonzini]

On 05/07/2016 22:50, John Snow wrote:
> So, something like:
> 
> block/drivers/bochs/
> 
> bochs.c
> probe.c (or bochs-probe.c)
> 
> and
> 
> include/block/drivers/bochs/
> 
> common.h (or internal.h)
> 
> 
> Any objections from the gallery?

I guess I'm missing why it is useful to modularize drivers that don't
need any external libraries; those that do in general are protocols and
need no probe function.

Sorry if I haven't noticed this in a previous discussion.

Paolo

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Paolo Bonzini]

On 06/07/2016 18:09, John Snow wrote:
> Recently the include files in hw/ide/ were moved to include/ without
> anybody mentioning it to me, including internal.h.
> 
> Why?

Because hw/ide/internal.h is not so internal.  In particular, it is
included by hw/ide/pci.h, which is included by hw/i386/pc_q35.c.

Paolo

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[John Snow]

On 07/07/2016 12:01 PM, Paolo Bonzini wrote:
> 
> 
> On 06/07/2016 18:09, John Snow wrote:
>> Recently the include files in hw/ide/ were moved to include/ without
>> anybody mentioning it to me, including internal.h.
>>
>> Why?
> 
> Because hw/ide/internal.h is not so internal.  In particular, it is
> included by hw/ide/pci.h, which is included by hw/i386/pc_q35.c.
> 
> Paolo
> 

Joy. Something to fix.

Re: [Qemu-devel] [Qemu-block] [PATCH v3 00/32] Dynamic module loading for block drivers

[John Snow]

On 07/05/2016 11:24 AM, Colin Lord wrote:
> This is the next version of this patch series. The first three patches
> in the series are mostly the same as they were last time, but with the
> issues mentioned in the reviews fixed. Most notably this means much less
> copy-paste happening in block.c.
> 
> The new changes, and the reason this series is so large, is a result of
> isolating the block driver probe functions from the rest of the drivers.
> This was mentioned in the reviews from the last version as something
> that should be added. This allows the probes to be accessed without
> needing to load all the modules. There are quite a few probes, which is
> why there are so many patches in this series. I tried to keep the
> patches where I was relocating the code separate from the patches where
> the probe function signatures were changed.
> 
> The probe functions now return both a score and a format/protocol name,
> which allows them to be completely separate from the drivers they
> correspond to. I didn't put all the probe functions in the same file as
> Fam suggested because it turns out there are some naming conflicts
> between some macros commonly used in the probe functions. For instance,
> QCowHeader is used in both qcow and qcow2 probes, but the meaning is
> different between them. Having separate files makes resolving these
> conflicts simple.
> 
> Also, just as a point of interest, I've done some basic profiling of my
> own to see what performance benefits there are to be made. I recorded
> the time to main of each option that you can enable/disable in the
> configuration by enabling each option by itself while explicitly
> disabling all other options. With each configuration I ran qemu 5 times
> while recording the time to main. All the results are in milliseconds.
> 
> minimal config      2.772
> vhdx                2.776
> attr                2.786
> coroutine-pool      2.836
> bzip                2.856
> linux-aio           2.867
> vnc-jpeg            2.872
> snappy              2.875
> fdt                 2.888
> brlapi              2.916
> vhost-net           2.922
> bluez               2.937
> cap-ng              2.940
> libiscsi            2.945
> nettle              2.945
> seccomp             2.973
> numa                2.983
> lzo                 2.994
> usb-redir           3.005
> cocoa               3.007
> libnfs              3.026
> vnc-sasl            3.071
> vnc-png             3.081
> virtfs              3.081
> uuid                3.086
> tpm                 3.136
> curses              3.145
> gcrypt              3.190
> smartcar            3.242
> rdma                3.570
> jemalloc            3.584
> glusterfs           3.597
> sdl                 3.677
> libusb              3.758
> libssh2             3.906
> tcmalloc            3.959
> xen                 4.151
> xen-pci-passthrough 4.167
> curl                5.245
> spice               5.289
> gnutls              5.400
> gtk                 8.782
> vte                 9.764
> rbd                 44.315
> 

Right around here is probably a pretty good place to include the
patchset changelog.

For larger series like this, you *may* also want to call on a script
written by Jeff Cody[1] to produce a patchset changelog. It works pretty
well in conjunction with git-publish[2].

(Not everybody does this. In fact, mostly just Max and I do. (You still
need at least a hand-written changelog, though.))

Using it is at your discretion, it can be useful at times.
Take a look at Max's series "[Qemu-devel] [PATCH v3 for-2.7 00/14] qapi:
Allow blockdev-add for NBD" for a good example [3].

the script git-backport-diff is usually used to check differences in a
series between an upstream and downstream version of a series, but it
can also be used to compare a v3 against a v2.

> Colin Lord (30):
>   blockdev: prepare iSCSI block driver for dynamic loading
>   blockdev: Move bochs probe into separate file
>   blockdev: Move cloop probe to its own file
>   blockdev: Move luks probe to its own file
>   blockdev: Move dmg probe to its own file
>   blockdev: Move parallels probe to its own file
>   blockdev: Move qcow probe to its own file
>   blockdev: Move qcow2 probe to its own file
>   blockdev: Move qed probe to its own file
>   blockdev: Move raw probe to its own file
>   blockdev: Move vdi probe to its own file
>   blockdev: Move vhdx probe to its own file
>   blockdev: Move vmdk probe to its own file
>   blockdev: Move vpc probe to its own file
>   blockdev: Separate bochs probe from its driver
>   blockdev: Separate cloop probe from its driver
>   blockdev: Separate luks probe from its driver
>   blockdev: Separate dmg probe from its driver
>   blockdev: Separate parallels probe from its driver
>   blockdev: Separate qcow probe from its driver
>   blockdev: Separate qcow2 probe from its driver
>   blockdev: Separate qed probe from its driver
>   blockdev: Separate raw probe from its driver
>   blockdev: Separate vdi probe from its driver
>   blockdev: Separate vhdx probe from its driver
>   blockdev: Separate vmdk probe from its driver
>   blockdev: Separate vpc probe from its driver
>   blockdev: Remove the .bdrv_probe field from BlockDrivers
>   blockdev: Separate out bdrv_probe_device functions
>   blockdev: Remove bdrv_probe_device field from BlockDriver
> 
> Marc Mari (2):
>   blockdev: Add dynamic generation of module_block.h
>   blockdev: Add dynamic module loading for block drivers
> 
>  Makefile                         |   7 ++
>  block.c                          | 181 +++++++++++++++++++++++++++++++--------
>  block/Makefile.objs              |   4 +
>  block/bochs.c                    |  56 +-----------
>  block/cloop.c                    |  17 +---
>  block/crypto.c                   |  22 +----
>  block/dmg.c                      |  17 +---
>  block/iscsi.c                    |  36 --------
>  block/parallels.c                |  44 +---------
>  block/probe/bochs.c              |  28 ++++++
>  block/probe/cloop.c              |  22 +++++
>  block/probe/dmg.c                |  22 +++++
>  block/probe/host_cdrom.c         |  47 ++++++++++
>  block/probe/host_device.c        |  42 +++++++++
>  block/probe/luks.c               |  26 ++++++
>  block/probe/parallels.c          |  26 ++++++
>  block/probe/qcow.c               |  22 +++++
>  block/probe/qcow2.c              |  22 +++++
>  block/probe/qed.c                |  24 ++++++
>  block/probe/raw.c                |  14 +++
>  block/probe/vdi.c                |  29 +++++++
>  block/probe/vhdx.c               |  27 ++++++
>  block/probe/vmdk.c               |  67 +++++++++++++++
>  block/probe/vpc.c                |  16 ++++
>  block/qcow.c                     |  32 +------
>  block/qcow2.c                    |  14 +--
>  block/qed.c                      |  16 +---
>  block/raw-posix.c                |  55 +-----------
>  block/raw-win32.c                |  11 +--
>  block/raw_bsd.c                  |  10 +--
>  block/vdi.c                      |  70 +--------------
>  block/vhdx.c                     |  21 +----
>  block/vmdk.c                     |  61 +------------
>  block/vpc.c                      |   9 +-
>  include/block/block_int.h        |   3 -
>  include/block/driver/bochs.h     |  40 +++++++++
>  include/block/driver/parallels.h |  26 ++++++
>  include/block/driver/qcow.h      |  21 +++++
>  include/block/driver/vdi.h       |  49 +++++++++++
>  include/block/driver/vmdk.h      |   7 ++
>  include/block/probe.h            |  33 +++++++
>  include/qemu/module.h            |   3 +
>  scripts/modules/module_block.py  | 108 +++++++++++++++++++++++
>  util/module.c                    |  38 +++-----
>  vl.c                             |  38 ++++++++
>  45 files changed, 948 insertions(+), 535 deletions(-)
>  create mode 100644 block/probe/bochs.c
>  create mode 100644 block/probe/cloop.c
>  create mode 100644 block/probe/dmg.c
>  create mode 100644 block/probe/host_cdrom.c
>  create mode 100644 block/probe/host_device.c
>  create mode 100644 block/probe/luks.c
>  create mode 100644 block/probe/parallels.c
>  create mode 100644 block/probe/qcow.c
>  create mode 100644 block/probe/qcow2.c
>  create mode 100644 block/probe/qed.c
>  create mode 100644 block/probe/raw.c
>  create mode 100644 block/probe/vdi.c
>  create mode 100644 block/probe/vhdx.c
>  create mode 100644 block/probe/vmdk.c
>  create mode 100644 block/probe/vpc.c
>  create mode 100644 include/block/driver/bochs.h
>  create mode 100644 include/block/driver/parallels.h
>  create mode 100644 include/block/driver/qcow.h
>  create mode 100644 include/block/driver/vdi.h
>  create mode 100644 include/block/driver/vmdk.h
>  create mode 100644 include/block/probe.h
>  create mode 100644 scripts/modules/module_block.py
> 

[1] https://github.com/codyprime/git-scripts
[2] https://github.com/stefanha/git-publish
[3] https://lists.gnu.org/archive/html/qemu-devel/2016-04/msg01048.html

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Colin Lord]

On 07/06/2016 04:24 AM, Kevin Wolf wrote:
> Am 05.07.2016 um 22:50 hat John Snow geschrieben:
>>
>>
>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>>> On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>>>> This puts the bochs probe function into its own separate file as part of
>>>> the process of modularizing block drivers. Having the probe functions
>>>> separate from the rest of the driver allows us to probe without having
>>>> to potentially unnecessarily load the driver.
>>>>
>>>> Signed-off-by: Colin Lord <clord@redhat.com>
>>>> ---
>>>>  block/Makefile.objs          |  1 +
>>>>  block/bochs.c                | 55 ++------------------------------------------
>>>>  block/probe/bochs.c          | 21 +++++++++++++++++
>>>
>>> Do we really need a sub-dir for this ?  If we were going to
>>> have sub-dirs under block/, I'd suggest we have one subdir
>>> per block driver, not spread code for one block driver
>>> across multiple dirs.
>>>
>>
>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>> filename organization because I was short of ideas.
>>
>> Some ideas:
>>
>> (1) A combined probe.c file. This keeps the existing organization and
>> localizes everything to just one new file.
>>
>> Downside: many formats rely on at least some minimal amount of structure
>> and constant definitions, and some of those overlap with each other.
>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>>
>> They could all be disentangled, but it's less clear on where all the
>> common definitions go. A common probe.h is a bad idea since the modular
>> portion of the driver has no business gaining access to other formats'
>> definitions. We could create a probe.c and matching
>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>>
>> (2) Separate probe files for each driver.
>>
>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>> little files, but it's minimal and fairly noninvasive.
>>
>> Like #1 though, we still have to figure out what to do with the common
>> includes.
>>
>>> IMHO a block/bochs-probe.c would be better, unless we did
>>> move block/bochs.c into a block/bochs/driver.c dir.
>>>
>>> Either way, you should update MAINTAINERS file to record
>>> this newly added filename, against the bochs entry. The
>>> same applies to most other patches in this series adding
>>> new files.
>>>
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> So, something like:
>>
>> block/drivers/bochs/
> 
> block/bochs/ if anything. We don't have to nest deeply just because we
> can. I don't really like new subdirectories, but when all drivers start
> to have multiple files, it might be unavoidable.
> 
>> bochs.c
>> probe.c (or bochs-probe.c)
>>
>> and
>>
>> include/block/drivers/bochs/
>>
>> common.h (or internal.h)
> 
> block/bochs/internal.h (or bochs.h)
> 
> Just like we already have some header files directly in block/ (e.g.
> qcow2.h). They are internal to the block driver, so no reason to move
> them to the global include directory.
> 
> Kevin
> 
So it sounds like what I should do is that for each driver I remove a
probe from, I'll create a new directory underneath block/ and put the
driver file, the probe file, and any new headers in it. eg for bochs,
there will be a new directory block/bochs/ under which there will be
block/bochs/bochs.c, block/bochs/bochs.h, and block/bochs/probe.c. Thus
no new headers will end up going in the include/ directory.

Also if that's what I'm doing, I assume I should leave alone any headers
already in the block/ directory that are used by other files that I'm
not touching? eg block/qed.h already exists and is used by files other
than the driver and probe, so it seems like I should leave that alone.

Colin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 05.07.2016 um 22:50 hat John Snow geschrieben:
>> 
>> 
>> On 07/05/2016 11:49 AM, Daniel P. Berrange wrote:
>> > On Tue, Jul 05, 2016 at 11:24:04AM -0400, Colin Lord wrote:
>> >> This puts the bochs probe function into its own separate file as part of
>> >> the process of modularizing block drivers. Having the probe functions
>> >> separate from the rest of the driver allows us to probe without having
>> >> to potentially unnecessarily load the driver.
>> >>
>> >> Signed-off-by: Colin Lord <clord@redhat.com>
>> >> ---
>> >>  block/Makefile.objs          |  1 +
>> >>  block/bochs.c                | 55 ++------------------------------------------
>> >>  block/probe/bochs.c          | 21 +++++++++++++++++
>> > 
>> > Do we really need a sub-dir for this ?  If we were going to
>> > have sub-dirs under block/, I'd suggest we have one subdir
>> > per block driver, not spread code for one block driver
>> > across multiple dirs.
>> > 
>> 
>> Admittedly I have been nudging Colin to shoot from the hip a bit on
>> filename organization because I was short of ideas.
>> 
>> Some ideas:
>> 
>> (1) A combined probe.c file. This keeps the existing organization and
>> localizes everything to just one new file.
>> 
>> Downside: many formats rely on at least some minimal amount of structure
>> and constant definitions, and some of those overlap with each other.
>> qcow and qcow2 both using "QcowHeader" would be a prominent example.
>> 
>> They could all be disentangled, but it's less clear on where all the
>> common definitions go. A common probe.h is a bad idea since the modular
>> portion of the driver has no business gaining access to other formats'
>> definitions. We could create a probe.c and matching
>> include/block/bdrv/fmt.h includes, but we lost our zeal for this method.
>> 
>> (2) Separate probe files for each driver.
>> 
>> What we went with. Keeps refactoring to a minimum. Adds a bunch of
>> little files, but it's minimal and fairly noninvasive.
>> 
>> Like #1 though, we still have to figure out what to do with the common
>> includes.
>> 
>> > IMHO a block/bochs-probe.c would be better, unless we did
>> > move block/bochs.c into a block/bochs/driver.c dir.
>> > 
>> > Either way, you should update MAINTAINERS file to record
>> > this newly added filename, against the bochs entry. The
>> > same applies to most other patches in this series adding
>> > new files.
>> > 
>> > 
>> > Regards,
>> > Daniel
>> > 
>> 
>> So, something like:
>> 
>> block/drivers/bochs/
>
> block/bochs/ if anything. We don't have to nest deeply just because we
> can. I don't really like new subdirectories, but when all drivers start
> to have multiple files, it might be unavoidable.

A block driver consisting of a dozen files would probably be easier to
work with in its own directory.  But three files?

Our more complex block drivers already consist of multiple files.  Have
we had problems with them all sitting in block/?  If not, why move
stuff?

>> bochs.c
>> probe.c (or bochs-probe.c)
>> 
>> and
>> 
>> include/block/drivers/bochs/
>> 
>> common.h (or internal.h)
>
> block/bochs/internal.h (or bochs.h)

bochs.h, please.  I don't fancy having a dozen headers called
"internal.h".

> Just like we already have some header files directly in block/ (e.g.
> qcow2.h). They are internal to the block driver, so no reason to move
> them to the global include directory.

Genau.

Re: [Qemu-devel] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Kevin Wolf]

Am 07.07.2016 um 18:01 hat Paolo Bonzini geschrieben:
> On 06/07/2016 18:09, John Snow wrote:
> > Recently the include files in hw/ide/ were moved to include/ without
> > anybody mentioning it to me, including internal.h.
> > 
> > Why?
> 
> Because hw/ide/internal.h is not so internal.  In particular, it is
> included by hw/ide/pci.h, which is included by hw/i386/pc_q35.c.

Probably it should be split in a truly internal part that remains in
hw/ide/ and part for the public interface.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 04/32] blockdev: Move bochs probe into separate file

[Kevin Wolf]

Am 07.07.2016 um 22:32 hat Colin Lord geschrieben:
> On 07/06/2016 04:24 AM, Kevin Wolf wrote:
> > Am 05.07.2016 um 22:50 hat John Snow geschrieben:
> >> So, something like:
> >>
> >> block/drivers/bochs/
> > 
> > block/bochs/ if anything. We don't have to nest deeply just because we
> > can. I don't really like new subdirectories, but when all drivers start
> > to have multiple files, it might be unavoidable.
> > 
> >> bochs.c
> >> probe.c (or bochs-probe.c)
> >>
> >> and
> >>
> >> include/block/drivers/bochs/
> >>
> >> common.h (or internal.h)
> > 
> > block/bochs/internal.h (or bochs.h)
> > 
> > Just like we already have some header files directly in block/ (e.g.
> > qcow2.h). They are internal to the block driver, so no reason to move
> > them to the global include directory.
> > 
> > Kevin
> > 
> So it sounds like what I should do is that for each driver I remove a
> probe from, I'll create a new directory underneath block/ and put the
> driver file, the probe file, and any new headers in it. eg for bochs,
> there will be a new directory block/bochs/ under which there will be
> block/bochs/bochs.c, block/bochs/bochs.h, and block/bochs/probe.c. Thus
> no new headers will end up going in the include/ directory.

Yes, either that or like Markus says, just create a block/bochs-probe.c
and block/bochs.h and leave the existing files where they are. Maybe
give us a bit more time to come to a conclusion. I could live with
either.

> Also if that's what I'm doing, I assume I should leave alone any headers
> already in the block/ directory that are used by other files that I'm
> not touching? eg block/qed.h already exists and is used by files other
> than the driver and probe, so it seems like I should leave that alone.

block/qed.h isn't used outside the driver. It's just that the driver
consists of multiple files. If you decide to move files, of course you
shouldn't only move qed.c, but qed-*.c, too.

Kevin

Re: [Qemu-devel] [PATCH v3 17/32] blockdev: Separate bochs probe from its driver

[Kevin Wolf]

Am 07.07.2016 um 16:56 hat Colin Lord geschrieben:
> On 07/06/2016 11:59 AM, Max Reitz wrote:
> > On 05.07.2016 17:24, Colin Lord wrote:
> >> Modifies the bochs probe to return the format name as well as the
> >> score as the final step of separating the probe function from the
> >> driver. This keeps the probe completely independent of the driver,
> >> making future modularization easier to accomplish. Returning the format
> >> name as well as the score allows the score to be correlated to the
> >> driver without the probe function needing to be part of the driver.
> >>
> >> Signed-off-by: Colin Lord <clord@redhat.com>
> >> ---
> >>  block.c               | 19 +++++++++++++++++++
> >>  block/bochs.c         |  1 -
> >>  block/probe/bochs.c   | 25 ++++++++++++++++---------
> >>  include/block/probe.h |  3 ++-
> >>  4 files changed, 37 insertions(+), 11 deletions(-)
> > 
> > Oh, and another comment: I think it would be better to split this patch
> > into a part that modifies block.c (introducing the format_probes array
> > and using it in bdrv_probe_all()) and another one that actually does the
> > bochs-specific stuff.
> > 
> > Max
> > 
> I could do that, but if I leave the variable i as an unsigned type, I
> get compiler warnings/errors that the for loop condition of
> i < ARRAY_SIZE(format_probes)
> must be false since the size of format_probes will be 0 (and unsigned is
> always positive). Is it okay to change the type to int? Not sure how
> strictly this aspect of the coding style gets enforced.

Leave it unsigned. The resulting code after the series is more important
than having the nicest possible split. If this means that you need to
leave both parts in the same patch, so be it.

Kevin

Re: [Qemu-devel] [Qemu-block] [PATCH v3 00/32] Dynamic module loading for block drivers

[Stefan Hajnoczi]

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

On Tue, Jul 05, 2016 at 11:24:00AM -0400, Colin Lord wrote:
> The new changes, and the reason this series is so large, is a result of
> isolating the block driver probe functions from the rest of the drivers.

Last time I checked the list of block drivers that can be built as
modules was small.  It seems like only modular block drivers need a
split probe function, but you've already done the work so no harm there.

Stefan

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