[Qemu-devel] [PATCH 0/4] keep and use backing file format (v7)

[Qemu-devel] [PATCH 0/4] keep and use backing file format (v7)

[Uri Lublin]

In this patchset there are two main features:
1. Introduce Qcow2 extensions.
2. Keep backing file format such that no probing is needed.

1. Qcow2 extensions are build of magic (id) len (in bytes) and data.
They reside between the end of the header and the backing filename (if exists).
Unknown extensions are ignored (assumed to be added by a newer version)
Based on a work done by Shahar Frank.

2. By keeping the backing file format we:
a. Provide a way to know the backing file format without probing
   it (setting the format at creation time).
b. Enable using qcow2 format over host block devices.
   (only if the user specifically asks for it, by providing the format
   at creation time).

I've added bdrv_create2 and drv->bdrv_create2 (implemented only
by block-qcow2 currently) to pass the backing-format to create.

Also fixes a security flaw found by Daniel P. Berrange on [1]
which summarizes: "Autoprobing: just say no."

[1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html

Changes from v6:
   Rebased.
   Postponed highest-allocated-block patches.

[Qemu-devel] [PATCH 1/4] Introducing qcow2 extensions

[Uri Lublin]

Qcow2 extensions are build of magic (id) len (in bytes) and data.
They reside right after the qcow2 header.
If a backing filename exists it follows the qcow2 extension (if exist)

Qcow2 extensions are read upon image open.
Qcow2 extensions are identified by their magic.
Unknown qcow2 extensions (unknown magic) are skipped.
A Special magic of 0 means end-of-qcow2-extensions.

In this patchset, to be used to keep backing file format.

Based on a work done by Shahar Frank <sfrank@redhat.com>.

Signed-off-by: Uri Lublin <uril@redhat.com>
---
 block-qcow2.c |   72 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 72 insertions(+), 0 deletions(-)

diff --git a/block-qcow2.c b/block-qcow2.c
index afbf7fe..6d07f9d 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -45,6 +45,7 @@
 
 //#define DEBUG_ALLOC
 //#define DEBUG_ALLOC2
+//#define DEBUG_EXT
 
 #define QCOW_MAGIC (('Q' << 24) | ('F' << 16) | ('I' << 8) | 0xfb)
 #define QCOW_VERSION 2
@@ -77,6 +78,14 @@ typedef struct QCowHeader {
     uint64_t snapshots_offset;
 } QCowHeader;
 
+
+typedef struct {
+    uint32_t magic;
+    uint32_t len;
+} QCowExtension;
+#define  QCOW_EXT_MAGIC_END 0
+
+
 typedef struct __attribute__((packed)) QCowSnapshotHeader {
     /* header is 8 byte aligned */
     uint64_t l1_table_offset;
@@ -183,11 +192,66 @@ static int qcow_probe(const uint8_t *buf, int buf_size, const char *filename)
         return 0;
 }
 
+
+/* 
+ * read qcow2 extension and fill bs
+ * start reading from start_offset
+ * finish reading upon magic of value 0 or when end_offset reached
+ * unknown magic is skipped (future extension this version knows nothing about)
+ * return 0 upon success, non-0 otherwise
+ */
+static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
+                                uint64_t end_offset)
+{
+    BDRVQcowState *s = bs->opaque;
+    QCowExtension ext;
+    uint64_t offset;
+
+#ifdef DEBUG_EXT
+    printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
+#endif
+    offset = start_offset;
+    while (offset < end_offset) {
+
+#ifdef DEBUG_EXT
+        /* Sanity check */
+        if (offset > s->cluster_size)
+            printf("qcow_handle_extension: suspicious offset %lu\n", offset);
+
+        printf("attemting to read extended header in offset %lu\n", offset);
+#endif
+
+        if (bdrv_pread(s->hd, offset, &ext, sizeof(ext)) != sizeof(ext)) {
+            fprintf(stderr, "qcow_handle_extension: ERROR: pread fail from offset %lu\n",
+                    offset);
+            return 1;
+        }
+        be32_to_cpus(&ext.magic);
+        be32_to_cpus(&ext.len);
+        offset += sizeof(ext);
+#ifdef DEBUG_EXT
+        printf("ext.magic = 0x%x\n", ext.magic);
+#endif
+        switch (ext.magic) {
+        case QCOW_EXT_MAGIC_END:
+            return 0;
+        default:
+            /* unknown magic -- just skip it */
+            offset += ((ext.len + 7) & ~7);
+            break;
+        }
+    }
+
+    return 0;
+}
+
+
 static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVQcowState *s = bs->opaque;
     int len, i, shift, ret;
     QCowHeader header;
+    uint64_t ext_end;
 
     /* Performance is terrible right now with cache=writethrough due mainly
      * to reference count updates.  If the user does not explicitly specify
@@ -270,6 +334,14 @@ static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
     if (refcount_init(bs) < 0)
         goto fail;
 
+    /* read qcow2 extensions */
+    if (header.backing_file_offset)
+        ext_end = header.backing_file_offset;
+    else
+        ext_end = s->cluster_size;
+    if (qcow_read_extensions(bs, sizeof(header), ext_end))
+        goto fail;
+
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
-- 
1.6.0.6

[Qemu-devel] [PATCH 2/4] block: support known backing format for image create and open

[Uri Lublin]

Added a backing_format field to BlockDriverState.
Added bdrv_create2 and drv->bdrv_create2 to create an image with
a known backing file format.
Upon bdrv_open2 if backing format is known use it, instead of
probing the (backing) image.

Signed-off-by: Uri Lublin <uril@redhat.com>
---
 block.c     |   28 ++++++++++++++++++++++++----
 block.h     |    4 ++++
 block_int.h |    7 +++++++
 3 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/block.c b/block.c
index fd09dff..0078dad 100644
--- a/block.c
+++ b/block.c
@@ -181,6 +181,20 @@ BlockDriver *bdrv_find_format(const char *format_name)
     return NULL;
 }
 
+int bdrv_create2(BlockDriver *drv,
+                const char *filename, int64_t size_in_sectors,
+                const char *backing_file, const char *backing_format,
+                int flags)
+{
+    if (drv->bdrv_create2)
+        return drv->bdrv_create2(filename, size_in_sectors, backing_file,
+                                 backing_format, flags);
+    if (drv->bdrv_create)
+        return drv->bdrv_create(filename, size_in_sectors, backing_file,
+                                flags);
+    return -ENOTSUP;
+}
+
 int bdrv_create(BlockDriver *drv,
                 const char *filename, int64_t size_in_sectors,
                 const char *backing_file, int flags)
@@ -357,7 +371,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
 
         /* if there is a backing file, use it */
         bs1 = bdrv_new("");
-        ret = bdrv_open(bs1, filename, 0);
+        ret = bdrv_open2(bs1, filename, 0, drv);
         if (ret < 0) {
             bdrv_delete(bs1);
             return ret;
@@ -378,12 +392,14 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         else
             realpath(filename, backing_filename);
 
-        ret = bdrv_create(&bdrv_qcow2, tmp_filename,
-                          total_size, backing_filename, 0);
+        ret = bdrv_create2(&bdrv_qcow2, tmp_filename,
+                           total_size, backing_filename, 
+                           (drv ? drv->format_name : NULL), 0);
         if (ret < 0) {
             return ret;
         }
         filename = tmp_filename;
+        drv = &bdrv_qcow2;
         bs->is_temporary = 1;
     }
 
@@ -429,10 +445,14 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
 #endif
     if (bs->backing_file[0] != '\0') {
         /* if there is a backing file, use it */
+        BlockDriver *back_drv = NULL;
         bs->backing_hd = bdrv_new("");
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
-        ret = bdrv_open(bs->backing_hd, backing_filename, open_flags);
+        if (bs->backing_format[0] != '\0')
+            back_drv = bdrv_find_format(bs->backing_format);
+        ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
+                         back_drv);
         if (ret < 0) {
             bdrv_close(bs);
             return ret;
diff --git a/block.h b/block.h
index b0c63ac..fab01e6 100644
--- a/block.h
+++ b/block.h
@@ -62,6 +62,10 @@ BlockDriver *bdrv_find_format(const char *format_name);
 int bdrv_create(BlockDriver *drv,
                 const char *filename, int64_t size_in_sectors,
                 const char *backing_file, int flags);
+int bdrv_create2(BlockDriver *drv,
+                 const char *filename, int64_t size_in_sectors,
+                 const char *backing_file, const char *backing_format,
+                 int flags);
 BlockDriverState *bdrv_new(const char *device_name);
 void bdrv_delete(BlockDriverState *bs);
 int bdrv_file_open(BlockDriverState **pbs, const char *filename, int flags);
diff --git a/block_int.h b/block_int.h
index 8b4c5fe..9f94c4c 100644
--- a/block_int.h
+++ b/block_int.h
@@ -98,6 +98,12 @@ struct BlockDriver {
                                            void *opaque);
 
     AIOPool aio_pool;
+
+    /* new create with backing file format */
+    int (*bdrv_create2)(const char *filename, int64_t total_sectors,
+                        const char *backing_file, const char *backing_format,
+                        int flags);
+
     struct BlockDriver *next;
 };
 
@@ -120,6 +126,7 @@ struct BlockDriverState {
     char filename[1024];
     char backing_file[1024]; /* if non zero, the image is a diff of
                                 this file image */
+    char backing_format[16]; /* if non-zero and backing_file exists */
     int is_temporary;
     int media_changed;
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 3/4] block-qcow2: keep backing file format in a qcow2 extension

[Uri Lublin]

Use a qcow2 extension to keep the backing file format.

By keeping the backing file format, we can:
1. Provide a way to know the backing file format without probing
   it (setting the format at creation time).
2. Enable using qcow2 format over host block devices.
   (only if the user specifically asks for it, by providing the format
   at creation time).

Also fixes a security flaw found by Daniel P. Berrange on [1]
which summarizes: "Autoprobing: just say no."

[1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html

Signed-off-by: Uri Lublin <uril@redhat.com>
---
 block-qcow2.c |   55 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/block-qcow2.c b/block-qcow2.c
index 6d07f9d..7759d34 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -84,6 +84,7 @@ typedef struct {
     uint32_t len;
 } QCowExtension;
 #define  QCOW_EXT_MAGIC_END 0
+#define  QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
 
 
 typedef struct __attribute__((packed)) QCowSnapshotHeader {
@@ -235,6 +236,24 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
         switch (ext.magic) {
         case QCOW_EXT_MAGIC_END:
             return 0;
+
+        case QCOW_EXT_MAGIC_BACKING_FORMAT:
+            if (ext.len >= sizeof(bs->backing_format)) {
+                fprintf(stderr, "ERROR: ext_backing_format: len=%u too large"
+                        " (>=%lu)\n",
+                        ext.len, sizeof(bs->backing_format));
+                return 2;
+            }
+            if (bdrv_pread(s->hd, offset , bs->backing_format,
+                           ext.len) != ext.len)
+                return 3;
+            bs->backing_format[ext.len] = '\0';
+#ifdef DEBUG_EXT
+            printf("Qcow2: Got format extension %s\n", bs->backing_format);
+#endif
+            offset += ((ext.len + 7) & ~7);
+            break;
+
         default:
             /* unknown magic -- just skip it */
             offset += ((ext.len + 7) & ~7);
@@ -1526,13 +1545,18 @@ static void create_refcount_update(QCowCreateState *s,
     }
 }
 
-static int qcow_create(const char *filename, int64_t total_size,
-                      const char *backing_file, int flags)
+static int qcow_create2(const char *filename, int64_t total_size,
+                        const char *backing_file, const char *backing_format,
+                        int flags)
 {
+
     int fd, header_size, backing_filename_len, l1_size, i, shift, l2_bits;
+    int backing_format_len = 0;
     QCowHeader header;
     uint64_t tmp, offset;
     QCowCreateState s1, *s = &s1;
+    QCowExtension ext_bf = {0, 0};
+
 
     memset(s, 0, sizeof(*s));
 
@@ -1546,6 +1570,12 @@ static int qcow_create(const char *filename, int64_t total_size,
     header_size = sizeof(header);
     backing_filename_len = 0;
     if (backing_file) {
+        if (backing_format) {
+            ext_bf.magic = QCOW_EXT_MAGIC_BACKING_FORMAT;
+            backing_format_len = strlen(backing_format);
+            ext_bf.len = (backing_format_len + 7) & ~7;
+            header_size += ((sizeof(ext_bf) + ext_bf.len + 7) & ~7);
+        }
         header.backing_file_offset = cpu_to_be64(header_size);
         backing_filename_len = strlen(backing_file);
         header.backing_file_size = cpu_to_be32(backing_filename_len);
@@ -1590,6 +1620,19 @@ static int qcow_create(const char *filename, int64_t total_size,
     /* write all the data */
     write(fd, &header, sizeof(header));
     if (backing_file) {
+        if (backing_format_len) {
+            char zero[16];
+            int d = ext_bf.len - backing_format_len;
+
+            memset(zero, 0, sizeof(zero));
+            cpu_to_be32s(&ext_bf.magic);
+            cpu_to_be32s(&ext_bf.len);
+            write(fd, &ext_bf, sizeof(ext_bf));
+            write(fd, backing_format, backing_format_len);
+            if (d>0) {
+                write(fd, zero, d);
+            }
+        }
         write(fd, backing_file, backing_filename_len);
     }
     lseek(fd, s->l1_table_offset, SEEK_SET);
@@ -1609,6 +1652,12 @@ static int qcow_create(const char *filename, int64_t total_size,
     return 0;
 }
 
+static int qcow_create(const char *filename, int64_t total_size,
+                       const char *backing_file, int flags)
+{
+    return qcow_create2(filename, total_size, backing_file, NULL, flags);
+}
+
 static int qcow_make_empty(BlockDriverState *bs)
 {
 #if 0
@@ -2685,4 +2734,6 @@ BlockDriver bdrv_qcow2 = {
     .bdrv_snapshot_delete = qcow_snapshot_delete,
     .bdrv_snapshot_list	= qcow_snapshot_list,
     .bdrv_get_info	= qcow_get_info,
+
+    .bdrv_create2 = qcow_create2,
 };
-- 
1.6.0.6

[Qemu-devel] [PATCH 4/4] qemu-img: adding a "-F base_fmt" option to "qemu-img create -b"

[Uri Lublin]

If the user specifies the backing file format,
then when opening the backing file, there is no need
to probe the (backing file) image to figure out its format.

This follows my previous patches implementing bdrv_create2
which keeps (for qcow2 only) the backing file format
as a qcow2-extension

Suggested by Daniel P. Berrange.

Signed-off-by: Uri Lublin <uril@redhat.com>
---
 qemu-img.c    |   24 ++++++++++++++++++++----
 qemu-img.texi |    4 +++-
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 66ec91c..ab380c8 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -25,6 +25,7 @@
 #include "osdep.h"
 #include "block_int.h"
 #include <assert.h>
+#include <stdio.h>
 
 #ifdef _WIN32
 #include <windows.h>
@@ -57,7 +58,7 @@ static void help(void)
            "QEMU disk image utility\n"
            "\n"
            "Command syntax:\n"
-           "  create [-e] [-6] [-b base_image] [-f fmt] filename [size]\n"
+           "  create [-e] [-6] [-F fmt] [-b base_image] [-f fmt] filename [size]\n"
            "  commit [-f fmt] filename\n"
            "  convert [-c] [-e] [-6] [-f fmt] [-O output_fmt] [-B output_base_image] filename [filename2 [...]] output_filename\n"
            "  info [-f fmt] filename\n"
@@ -217,6 +218,7 @@ static int img_create(int argc, char **argv)
 {
     int c, ret, flags;
     const char *fmt = "raw";
+    const char *base_fmt = NULL;
     const char *filename;
     const char *base_filename = NULL;
     uint64_t size;
@@ -226,13 +228,16 @@ static int img_create(int argc, char **argv)
 
     flags = 0;
     for(;;) {
-        c = getopt(argc, argv, "b:f:he6");
+        c = getopt(argc, argv, "F:b:f:he6");
         if (c == -1)
             break;
         switch(c) {
         case 'h':
             help();
             break;
+        case 'F':
+            base_fmt = optarg;
+            break;
         case 'b':
             base_filename = optarg;
             break;
@@ -253,7 +258,15 @@ static int img_create(int argc, char **argv)
     size = 0;
     if (base_filename) {
         BlockDriverState *bs;
-        bs = bdrv_new_open(base_filename, NULL);
+        BlockDriver *base_drv = NULL;
+
+        if (base_fmt) {
+            base_drv = bdrv_find_format(base_fmt);
+            if (base_drv == NULL)
+                error("Unknown basefile format '%s'", base_fmt);
+        }
+
+        bs = bdrv_new_open(base_filename, base_fmt);
         bdrv_get_geometry(bs, &size);
         size *= 512;
         bdrv_delete(bs);
@@ -284,9 +297,12 @@ static int img_create(int argc, char **argv)
     if (base_filename) {
         printf(", backing_file=%s",
                base_filename);
+         if (base_fmt)
+             printf(", backing_fmt=%s",
+                    base_fmt);
     }
     printf(", size=%" PRIu64 " kB\n", size / 1024);
-    ret = bdrv_create(drv, filename, size / 512, base_filename, flags);
+    ret = bdrv_create2(drv, filename, size / 512, base_filename, base_fmt, flags);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting or formatting option not supported for file format '%s'", fmt);
diff --git a/qemu-img.texi b/qemu-img.texi
index a40f841..deef2ab 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -8,7 +8,7 @@ usage: qemu-img command [command options]
 
 The following commands are supported:
 @table @option
-@item create [-e] [-6] [-b @var{base_image}] [-f @var{fmt}] @var{filename} [@var{size}]
+@item create [-e] [-6] [-F @var{base_fmt}] [-b @var{base_image}] [-f @var{fmt}] @var{filename} [@var{size}]
 @item commit [-f @var{fmt}] @var{filename}
 @item convert [-c] [-e] [-6] [-f @var{fmt}] [-O @var{output_fmt}] [-B @var{output_base_image}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 @item info [-f @var{fmt}] @var{filename}
@@ -27,6 +27,8 @@ forces the output image to be created as a copy on write
 image of the specified base image; @code{output_base_image} should have the same
 content as the input's base image, however the path, image format, etc may
 differ
+@item base_fmt
+is the disk image format of @var{base_image}. for more information look at @var{fmt}
 @item fmt
 is the disk image format. It is guessed automatically in most cases. The following formats are supported:
 
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH 0/4] keep and use backing file format (v7)

[Anthony Liguori]

Uri Lublin wrote:
> In this patchset there are two main features:
> 1. Introduce Qcow2 extensions.
> 2. Keep backing file format such that no probing is needed.
>
> 1. Qcow2 extensions are build of magic (id) len (in bytes) and data.
> They reside between the end of the header and the backing filename (if exists).
> Unknown extensions are ignored (assumed to be added by a newer version)
> Based on a work done by Shahar Frank.
>
> 2. By keeping the backing file format we:
> a. Provide a way to know the backing file format without probing
>    it (setting the format at creation time).
> b. Enable using qcow2 format over host block devices.
>    (only if the user specifically asks for it, by providing the format
>    at creation time).
>
> I've added bdrv_create2 and drv->bdrv_create2 (implemented only
> by block-qcow2 currently) to pass the backing-format to create.
>
> Also fixes a security flaw found by Daniel P. Berrange on [1]
> which summarizes: "Autoprobing: just say no."
>
> [1] http://lists.gnu.org/archive/html/qemu-devel/2008-12/msg01083.html
>
> Changes from v6:
>    Rebased.
>    Postponed highest-allocated-block patches.
>   

Applied all.  Thanks.

Regards,

Anthony Liguori

>
>
>