[pve-devel] [PATCH kvm] Fix #932: passing BDRV_O_PROTOCOL breaks qcow2 on gluster

Wolfgang Bumiller w.bumiller at proxmox.com
Thu Apr 28 14:00:59 CEST 2016


Passing BDRV_O_PROTOCOL causes qemu to open protocol based
paths as raw. This fails for our storage when using
glusterfs because we can use qcow2 files on there.
This also causes vma to refuse to write to them as the
expected size (the one the qcow2 was created for) does not
match the recognized file size (the size of the file
itself). (Which is good because it means the files should
not have been accessed via the wrong format backend by
accident.)

The reason for passing it was to deal with format probing
when using RBD without KRBD.

As described in the patch commit: we now provide a way to
specify the format explicitly and thereby follow qemu with
deprecating automatic guessing of raw formats.

This re-enables the raw-probing warning for non-krbd ceph
storages which now has to be addressed by passing the format
to the map fifo in PVE::QemuServer::restore_vma_archive().
---
 ...5-vma-add-format-option-to-device-mapping.patch | 108 +++++++++++++++++++++
 .../0045-vma-also-guess-raw-for-dev-paths.patch    |  36 -------
 debian/patches/series                              |   2 +-
 3 files changed, 109 insertions(+), 37 deletions(-)
 create mode 100644 debian/patches/pve/0045-vma-add-format-option-to-device-mapping.patch
 delete mode 100644 debian/patches/pve/0045-vma-also-guess-raw-for-dev-paths.patch

diff --git a/debian/patches/pve/0045-vma-add-format-option-to-device-mapping.patch b/debian/patches/pve/0045-vma-add-format-option-to-device-mapping.patch
new file mode 100644
index 0000000..14017db
--- /dev/null
+++ b/debian/patches/pve/0045-vma-add-format-option-to-device-mapping.patch
@@ -0,0 +1,108 @@
+From 65a0ff1802f7cc0b69b2c2121f45089efbb27ee4 Mon Sep 17 00:00:00 2001
+From: Wolfgang Bumiller <w.bumiller at proxmox.com>
+Date: Tue, 12 Apr 2016 13:49:44 +0200
+Subject: [PATCH 2/3] vma: add format option to device mapping
+
+The BDRV_O_PROTOCOL option breaks non-raw protocol devices,
+so we instead now allow the format to be explicitly
+specified from the outside.
+
+In other words we now too deprecate the automatic guessing
+of raw formats, just like qemu already does, and have to
+silence the warnings by passing the drive mapping.
+---
+ vma.c | 34 +++++++++++++++++++++++++++-------
+ 1 file changed, 27 insertions(+), 7 deletions(-)
+
+diff --git a/vma.c b/vma.c
+index 1c4103f..46ae14b 100644
+--- a/vma.c
++++ b/vma.c
+@@ -137,6 +137,7 @@ static int list_content(int argc, char **argv)
+ typedef struct RestoreMap {
+     char *devname;
+     char *path;
++    char *format;
+     bool write_zero;
+ } RestoreMap;
+ 
+@@ -224,13 +225,24 @@ static int extract_content(int argc, char **argv)
+                 }
+             }
+ 
++            char *format = NULL;
++            if (strncmp(line, "format=", sizeof("format=")-1) == 0) {
++                format = line + sizeof("format=")-1;
++                char *colon = strchr(format, ':');
++                if (!colon) {
++                    g_error("read map failed - found only a format ('%s')", inbuf);
++                }
++                format = g_strndup(format, colon - format);
++                line = colon+1;
++            }
++
+             const char *path;
+             bool write_zero;
+             if (line[0] == '0' && line[1] == ':') {
+-                path = inbuf + 2;
++                path = line + 2;
+                 write_zero = false;
+             } else if (line[0] == '1' && line[1] == ':') {
+-                path = inbuf + 2;
++                path = line + 2;
+                 write_zero = true;
+             } else {
+                 g_error("read map failed - parse error ('%s')", inbuf);
+@@ -246,6 +258,7 @@ static int extract_content(int argc, char **argv)
+             RestoreMap *map = g_new0(RestoreMap, 1);
+             map->devname = g_strdup(devname);
+             map->path = g_strdup(path);
++            map->format = format;
+             map->write_zero = write_zero;
+ 
+             g_hash_table_insert(devmap, map->devname, map);
+@@ -270,6 +283,7 @@ static int extract_content(int argc, char **argv)
+             g_free(statefn);
+         } else if (di) {
+             char *devfn = NULL;
++            const char *format = NULL;
+             int flags = BDRV_O_RDWR|BDRV_O_CACHE_WB;
+             bool write_zero = true;
+ 
+@@ -280,6 +294,7 @@ static int extract_content(int argc, char **argv)
+                     g_error("no device name mapping for %s", di->devname);
+                 }
+                 devfn = map->path;
++                format = map->format;
+                 write_zero = map->write_zero;
+             } else {
+                 devfn = g_strdup_printf("%s/tmp-disk-%s.raw",
+@@ -302,15 +317,20 @@ static int extract_content(int argc, char **argv)
+             BlockDriverState *bs = bdrv_new();
+ 
+ 	    size_t devlen = strlen(devfn);
+-	    bool protocol = path_has_protocol(devfn);
+ 	    QDict *options = NULL;
+-	    if (devlen > 4 && strcmp(devfn+devlen-4, ".raw") == 0 && !protocol) {
++            if (format) {
++                /* explicit format from commandline */
++                options = qdict_new();
++                qdict_put(options, "driver", qstring_from_str(format));
++            } else if ((devlen > 4 && strcmp(devfn+devlen-4, ".raw") == 0) ||
++	               strncmp(devfn, "/dev/", 5) == 0)
++	    {
++                /* This part is now deprecated for PVE as well (just as qemu
++                 * deprecated not specifying an explicit raw format, too.
++                 */
+ 		/* explicit raw format */
+ 		options = qdict_new();
+ 		qdict_put(options, "driver", qstring_from_str("raw"));
+-	    } else if (protocol) {
+-		/* tell bdrv_open to honor the protocol */
+-		flags |= BDRV_O_PROTOCOL;
+ 	    }
+ 
+ 	    if (errp || bdrv_open(&bs, devfn, NULL, options, flags, &errp)) {
+-- 
+2.1.4
+
diff --git a/debian/patches/pve/0045-vma-also-guess-raw-for-dev-paths.patch b/debian/patches/pve/0045-vma-also-guess-raw-for-dev-paths.patch
deleted file mode 100644
index b48b5e8..0000000
--- a/debian/patches/pve/0045-vma-also-guess-raw-for-dev-paths.patch
+++ /dev/null
@@ -1,36 +0,0 @@
-From 7878879d6dc2330e6d2d34c1e83101ad796ce939 Mon Sep 17 00:00:00 2001
-From: Wolfgang Bumiller <w.bumiller at proxmox.com>
-Date: Tue, 12 Apr 2016 13:49:44 +0200
-Subject: [PATCH] vma: also "guess" raw for /dev/ paths
-
----
- vma.c | 10 ++++++----
- 1 file changed, 6 insertions(+), 4 deletions(-)
-
-diff --git a/vma.c b/vma.c
-index 1c4103f..2f604bb 100644
---- a/vma.c
-+++ b/vma.c
-@@ -304,13 +304,15 @@ static int extract_content(int argc, char **argv)
- 	    size_t devlen = strlen(devfn);
- 	    bool protocol = path_has_protocol(devfn);
- 	    QDict *options = NULL;
--	    if (devlen > 4 && strcmp(devfn+devlen-4, ".raw") == 0 && !protocol) {
-+	    if (protocol) {
-+		/* tell bdrv_open to honor the protocol */
-+		flags |= BDRV_O_PROTOCOL;
-+	    } else if ((devlen > 4 && strcmp(devfn+devlen-4, ".raw") == 0) ||
-+	               strncmp(devfn, "/dev/", 5) == 0)
-+	    {
- 		/* explicit raw format */
- 		options = qdict_new();
- 		qdict_put(options, "driver", qstring_from_str("raw"));
--	    } else if (protocol) {
--		/* tell bdrv_open to honor the protocol */
--		flags |= BDRV_O_PROTOCOL;
- 	    }
- 
- 	    if (errp || bdrv_open(&bs, devfn, NULL, options, flags, &errp)) {
--- 
-2.1.4
-
diff --git a/debian/patches/series b/debian/patches/series
index f8de9d4..60743b8 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -42,7 +42,7 @@ pve/0041-PVE-VNC-authentication.patch
 pve/0042-vma-writer-don-t-bail-out-on-zero-length-files.patch
 pve/0043-vma-better-driver-guessing-for-bdrv_open.patch
 pve/0044-block-add-zeroinit.patch
-pve/0045-vma-also-guess-raw-for-dev-paths.patch
+pve/0045-vma-add-format-option-to-device-mapping.patch
 extra/0001-vnc-clear-vs-tlscreds-after-unparenting-it.patch
 extra/CVE-2016-2198-ehci-null-pointer.patch
 extra/CVE-2016-2391-usb-ohci-avoid-multiple-eof-timers.patch
-- 
2.1.4




More information about the pve-devel mailing list