[Qemu-devel] [PATCH 00/14] block (mostly qcow2) changes (v6)

[Qemu-devel] [PATCH 00/14] block (mostly qcow2) changes (v6)

[Uri Lublin]

In this patchset there are three main features:
1. Introduce Qcow2 extensions.
2. Keep backing file format such that no probing is needed.
3. Keep track of highest-allocated-offset and report it upon a user request.
4. Add an open-flag to distinguish between a leaf image and a backing file
   (base image). Also keep the open-flags so we can later use them to
   close and reopen images.

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

3. One of the main usage for highest-allocated-offset is to know and act
upon a case of end-of-storage-space. Instead of waiting for
-ENOSPACE (which now may cause the VM to stop), one can
define a threshold and extend the diskspace allocated for
the image.
This info is also good for management systems, so they
can report the user disk space status.

4. With bdrv_close and reopen capability we can possibly:
   a. Open backing files with read-only permission, and reopen with RW
      if needed.
   b. Reopen on destination side after migration before starting to run
      This solves a case where the source changes the disk (especially
      meta-data) while migration is in process.
   c. Check open-flags to keep track of highest-allocated-offset for 
      only leaf images.
      

Changes from v5:
   Patchset includes newly introduced qcow2 extensions.
   Usage of such qcow2 extensions for keeping both backing format and 
       highest-allocated-offset.
   No scanning of qcow2 images upon open.
   Not yet support for num-free.
   Added bdrv_close_all + calling it at end of main.
   Support highest_alloc only for leaf images.

Uri Lublin (14):
      Introducing qcow2 extensions
      block: support known backing format for image create and open
      block-qcow2: keep backing file format in a qcow2 extension
      qemu-img: adding a "-F base_fmt" option to "qemu-img create -b"
      block-qcow2: keep highest allocated offset
      block-qcow2: export highest-alloc through BlockDriverInfo and get_info()
      block: info blockstats: show highest_allocated if exists
      Add a bdrv_close_all() and  call it at the end of main()
      block-qcow2: keep highest alloc offset in a qcow2 extension
      qemu-img: info: show highest_alloc if exists
      qcow2: qcow_read_extensions: make "advance offset over extension" common
      block: pass BDRV_BACKING flag to open of a backing file
      block: keep flags an image was opened with
      block-qcow2: do not keep track of highest-alloc for backing files

[Qemu-devel] [PATCH 01/14] 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.

To be used to keep backing file format, highest allocated offset
and more.

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

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

diff --git a/block-qcow2.c b/block-qcow2.c
index afbf7fe..23e66b0 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,65 @@ 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 +333,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 02/14] 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 135fbe6..bdf66a4 100644
--- a/block.c
+++ b/block.c
@@ -178,6 +178,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)
@@ -354,7 +368,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;
@@ -375,12 +389,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;
     }
 
@@ -426,10 +442,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 d3c81fa..2206178 100644
--- a/block_int.h
+++ b/block_int.h
@@ -91,6 +91,12 @@ struct BlockDriver {
                                            BlockDriverCompletionFunc *cb,
                                            void *opaque);
 
+    /* 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);
+
+
     BlockDriverAIOCB *free_aiocb;
     struct BlockDriver *next;
 };
@@ -114,6 +120,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 03/14] 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 23e66b0..8a910fa 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 {
@@ -234,6 +235,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);
@@ -1525,13 +1544,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));
 
@@ -1545,6 +1569,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);
@@ -1589,6 +1619,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);
@@ -1608,6 +1651,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
@@ -2684,4 +2733,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 04/14] 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

[Qemu-devel] [PATCH 05/14] block-qcow2: keep highest allocated offset

[Uri Lublin]

We want to know the highest written offset for qcow2 images.
This gives a pretty good (and easy to calculate) estimation to how
much more allocation can be done for the block device.

It can be usefull for allocating more diskspace for that image
(if possible, e.g. lvm) before we run out-of-disk-space

It does not help images which are files in a file system.

The highest allocated offset is updated only upon cluster
allocation (with highest offset).

Currently, since we do not want to scan the refcount table (as
it takes a lot of time), highest-alloc is initialized to 0.
That means it may be inaccurate, but still is useful.

A following patch would keep highest-alloc in a qcow2-extension,
which would make it accurate.

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

diff --git a/block-qcow2.c b/block-qcow2.c
index 8a910fa..c070e05 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -153,6 +153,9 @@ typedef struct BDRVQcowState {
     uint32_t crypt_method_header;
     AES_KEY aes_encrypt_key;
     AES_KEY aes_decrypt_key;
+
+    int64_t highest_alloc; /* highest cluester allocated (in clusters) */
+
     uint64_t snapshots_offset;
     int snapshots_size;
     int nb_snapshots;
@@ -360,6 +363,8 @@ static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
     if (qcow_read_extensions(bs, sizeof(header), ext_end))
         goto fail;
 
+    s->highest_alloc = 0;
+
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
@@ -2316,6 +2321,10 @@ retry:
             size,
             (s->free_cluster_index - nb_clusters) << s->cluster_bits);
 #endif
+
+    if (s->highest_alloc < s->free_cluster_index)
+        s->highest_alloc = s->free_cluster_index;
+
     return (s->free_cluster_index - nb_clusters) << s->cluster_bits;
 }
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 06/14] block-qcow2: export highest-alloc through BlockDriverInfo and get_info()

[Uri Lublin]

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

diff --git a/block-qcow2.c b/block-qcow2.c
index c070e05..53364ff 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -1763,6 +1763,7 @@ static int qcow_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
     bdi->cluster_size = s->cluster_size;
     bdi->vm_state_offset = (int64_t)s->l1_vm_state_index <<
         (s->cluster_bits + s->l2_bits);
+    bdi->highest_alloc = s->highest_alloc << s->cluster_bits;
     return 0;
 }
 
diff --git a/block.h b/block.h
index fab01e6..c01b192 100644
--- a/block.h
+++ b/block.h
@@ -26,6 +26,8 @@ typedef struct BlockDriverInfo {
     int cluster_size;
     /* offset at which the VM state can be saved (0 if not possible) */
     int64_t vm_state_offset;
+    /* highest allocated block offset (in bytes) */
+    int64_t highest_alloc;
 } BlockDriverInfo;
 
 typedef struct QEMUSnapshotInfo {
-- 
1.6.0.6

[Qemu-devel] [PATCH 07/14] block: info blockstats: show highest_allocated if exists

[Uri Lublin]

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

diff --git a/block.c b/block.c
index bdf66a4..411514a 100644
--- a/block.c
+++ b/block.c
@@ -1093,6 +1093,7 @@ void bdrv_info(Monitor *mon)
 void bdrv_info_stats(Monitor *mon)
 {
     BlockDriverState *bs;
+    BlockDriverInfo bdi;
 
     for (bs = bdrv_first; bs != NULL; bs = bs->next) {
         monitor_printf(mon, "%s:"
@@ -1100,10 +1101,17 @@ void bdrv_info_stats(Monitor *mon)
                        " wr_bytes=%" PRIu64
                        " rd_operations=%" PRIu64
                        " wr_operations=%" PRIu64
-                       "\n",
+                       ,
                        bs->device_name,
                        bs->rd_bytes, bs->wr_bytes,
                        bs->rd_ops, bs->wr_ops);
+        if (bdrv_get_info(bs, &bdi) == 0) {
+            if (bdi.highest_alloc) {
+                monitor_printf(mon, " high=%" PRIu64,
+                               bdi.highest_alloc);
+            }
+        }
+        monitor_printf(mon, "\n");
     }
 }
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 08/14] Add a bdrv_close_all() and call it at the end of main()

[Uri Lublin]

Cleanly close all images.

Signed-off-by: Uri Lublin <uril@redhat.com>
---
 block.c |    8 ++++++++
 block.h |    1 +
 vl.c    |    1 +
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/block.c b/block.c
index 411514a..39ff1dd 100644
--- a/block.c
+++ b/block.c
@@ -1023,6 +1023,14 @@ void bdrv_flush_all(void)
             bdrv_flush(bs);
 }
 
+void bdrv_close_all(void)
+{
+    BlockDriverState *bs;
+
+    for (bs = bdrv_first; bs != NULL; bs = bs->next)
+        bdrv_delete(bs);
+}
+
 /*
  * Returns true iff the specified sector is present in the disk image. Drivers
  * not implementing the functionality are assumed to not support backing files,
diff --git a/block.h b/block.h
index c01b192..fbb445e 100644
--- a/block.h
+++ b/block.h
@@ -118,6 +118,7 @@ BlockDriverAIOCB *bdrv_sg_aio_write(BlockDriverState *bs, void *buf, int count,
 /* Ensure contents are flushed to disk.  */
 void bdrv_flush(BlockDriverState *bs);
 void bdrv_flush_all(void);
+void bdrv_close_all(void);
 
 int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
 	int *pnum);
diff --git a/vl.c b/vl.c
index b62a2d4..6c0e0cf 100644
--- a/vl.c
+++ b/vl.c
@@ -5815,6 +5815,7 @@ int main(int argc, char **argv, char **envp)
     main_loop();
     quit_timers();
     net_cleanup();
+    bdrv_close_all();
 
     return 0;
 }
-- 
1.6.0.6

[Qemu-devel] [PATCH 09/14] block-qcow2: keep highest alloc offset in a qcow2 extension

[Uri Lublin]

Write it upon qcow_create.
Read it upon qcow_open.
Update (if changed and an extension already exists) upon qcow_close.

This makes highest allocated offset statistic more accurate.

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

diff --git a/block-qcow2.c b/block-qcow2.c
index 53364ff..ed84a56 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -85,6 +85,7 @@ typedef struct {
 } QCowExtension;
 #define  QCOW_EXT_MAGIC_END 0
 #define  QCOW_EXT_MAGIC_BACKING_FORMAT 0xE2792ACA
+#define  QCOW_EXT_MAGIC_HIGH_ALLOC     0xE2792ACB
 
 
 typedef struct __attribute__((packed)) QCowSnapshotHeader {
@@ -155,6 +156,8 @@ typedef struct BDRVQcowState {
     AES_KEY aes_decrypt_key;
 
     int64_t highest_alloc; /* highest cluester allocated (in clusters) */
+    int64_t highest_alloc_old; /* highest-alloc value when image opened */
+    int64_t highest_alloc_offset; /* offset on disk to highest-alloc value */
 
     uint64_t snapshots_offset;
     int snapshots_size;
@@ -209,7 +212,8 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
     BDRVQcowState *s = bs->opaque;
     QCowExtension ext;
     uint64_t offset;
-
+    uint64_t high;
+        
 #ifdef DEBUG_EXT
     printf("qcow_read_extensions: start=%ld end=%ld\n", start_offset, end_offset);
 #endif
@@ -256,6 +260,24 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             offset += ((ext.len + 7) & ~7);
             break;
 
+        case QCOW_EXT_MAGIC_HIGH_ALLOC:
+            if (ext.len != sizeof(uint64_t)) {
+                fprintf(stderr, "ERROR: ext_high_alloc: len=%u too large"
+                        " (>=%lu)\n",
+                        ext.len, sizeof(uint64_t));
+                return 2;
+            }
+            if (bdrv_pread(s->hd, offset , &high, ext.len) != ext.len)
+                return 3;
+            s->highest_alloc = be64_to_cpu(high);
+            s->highest_alloc_old = s->highest_alloc;
+            s->highest_alloc_offset = offset;
+#ifdef DEBUG_EXT
+            printf("Qcow2: Got highest_alloc 0x%lu\n", s->highest_alloc);
+#endif
+            offset += ((ext.len + 7) & ~7);
+            break;
+
         default:
             /* unknown magic -- just skip it */
             offset += ((ext.len + 7) & ~7);
@@ -267,6 +289,62 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 }
 
 
+static void update_highest_alloc(BlockDriverState *bs)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t high;
+    int ret;
+
+#ifdef DEBUG_EXT
+    QCowExtension ext;
+    int len;
+    char buff[32];
+
+    printf("IN update_highest_alloc for %s: current %lu old %lu offset=%lu\n", 
+           bs->filename, s->highest_alloc, s->highest_alloc_old, 
+           s->highest_alloc_offset);
+#endif    
+
+    if (s->highest_alloc_offset <= 0)
+        return;
+    
+    if (s->highest_alloc == s->highest_alloc_old)
+        return;
+
+#ifdef DEBUG_EXT
+    printf("Writing highest_alloc=%lu, was %lu\n",
+           s->highest_alloc, s->highest_alloc_old);
+    
+    len = sizeof(ext) + sizeof(uint64_t);
+    ret = bdrv_pread(s->hd, s->highest_alloc_offset - sizeof(uint64_t),
+                     buff, len);
+    if (ret != len)
+        fprintf(stderr, "%s: bdrv_pread FAILED (ret=%d expected %d\n",
+                __FUNCTION__, ret, len);
+    if (be32_to_cpu(*(uint32_t*)buff) != QCOW_EXT_MAGIC_HIGH_ALLOC)
+        fprintf(stderr, "%s: offset points to the wrong location (magic)",
+                __FUNCTION__);
+    if (be32_to_cpu(*((uint32_t*)buff + 1)) != sizeof(uint64_t))
+        fprintf(stderr, "%s: offset points to the wrong location (len)",
+                __FUNCTION__);
+    high = be64_to_cpu(*(uint64_t*)(buff + sizeof(ext)));
+    if (high != s->highest_alloc_old)
+        fprintf(stderr, "%s: offset points to the wrong location (value)",
+                __FUNCTION__);
+#endif
+
+    high = cpu_to_be64(s->highest_alloc);
+    ret = bdrv_pwrite(s->hd, s->highest_alloc_offset, &high, sizeof(uint64_t));
+
+#ifdef DEBUG_EXT
+    if (ret != sizeof(uint64_t))
+        fprintf(stderr, "update highest-alloc: bdrv_pwrite FAILED (%d e=%d)\n",
+                ret, (int)sizeof(uint64_t));
+#endif
+}
+
+
+
 static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
 {
     BDRVQcowState *s = bs->opaque;
@@ -363,8 +441,6 @@ static int qcow_open(BlockDriverState *bs, const char *filename, int flags)
     if (qcow_read_extensions(bs, sizeof(header), ext_end))
         goto fail;
 
-    s->highest_alloc = 0;
-
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
@@ -1509,9 +1585,12 @@ static void qcow_aio_cancel(BlockDriverAIOCB *blockacb)
     qemu_aio_release(acb);
 }
 
+
 static void qcow_close(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
+
+    update_highest_alloc(bs);
     qemu_free(s->l1_table);
     qemu_free(s->l2_cache);
     qemu_free(s->cluster_cache);
@@ -1557,10 +1636,10 @@ static int qcow_create2(const char *filename, int64_t total_size,
     int fd, header_size, backing_filename_len, l1_size, i, shift, l2_bits;
     int backing_format_len = 0;
     QCowHeader header;
-    uint64_t tmp, offset;
+    uint64_t tmp, offset, high_alloc;
     QCowCreateState s1, *s = &s1;
     QCowExtension ext_bf = {0, 0};
-
+    QCowExtension ext_ha = {0, 0};
 
     memset(s, 0, sizeof(*s));
 
@@ -1572,6 +1651,9 @@ static int qcow_create2(const char *filename, int64_t total_size,
     header.version = cpu_to_be32(QCOW_VERSION);
     header.size = cpu_to_be64(total_size * 512);
     header_size = sizeof(header);
+    ext_ha.magic = QCOW_EXT_MAGIC_HIGH_ALLOC;
+    ext_ha.len   = sizeof(uint64_t);
+    header_size += ((sizeof(ext_ha) + ext_ha.len + 7) & ~7);
     backing_filename_len = 0;
     if (backing_file) {
         if (backing_format) {
@@ -1623,6 +1705,15 @@ static int qcow_create2(const char *filename, int64_t total_size,
 
     /* write all the data */
     write(fd, &header, sizeof(header));
+
+    
+    cpu_to_be32s(&ext_ha.magic);
+    cpu_to_be32s(&ext_ha.len);
+    high_alloc = cpu_to_be64(offset >> s->cluster_bits);
+
+    write(fd, &ext_ha, sizeof(ext_ha));
+    write(fd, &high_alloc, sizeof(uint64_t));
+
     if (backing_file) {
         if (backing_format_len) {
             char zero[16];
-- 
1.6.0.6

[Qemu-devel] [PATCH 10/14] qemu-img: info: show highest_alloc if exists

[Uri Lublin]

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

diff --git a/qemu-img.c b/qemu-img.c
index ab380c8..9b51e85 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -746,6 +746,8 @@ static int img_info(int argc, char **argv)
     if (bdrv_get_info(bs, &bdi) >= 0) {
         if (bdi.cluster_size != 0)
             printf("cluster_size: %d\n", bdi.cluster_size);
+        if (bdi.highest_alloc)
+            printf("highest_alloc: %ld\n", bdi.highest_alloc);
     }
     bdrv_get_backing_filename(bs, backing_filename, sizeof(backing_filename));
     if (backing_filename[0] != '\0') {
-- 
1.6.0.6

[Qemu-devel] [PATCH 11/14] qcow2: qcow_read_extensions: make "advance offset over extension" common

[Uri Lublin]

Take it out of the switch statement (and each "case").

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

diff --git a/block-qcow2.c b/block-qcow2.c
index ed84a56..c98024e 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -257,7 +257,6 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 #ifdef DEBUG_EXT
             printf("Qcow2: Got format extension %s\n", bs->backing_format);
 #endif
-            offset += ((ext.len + 7) & ~7);
             break;
 
         case QCOW_EXT_MAGIC_HIGH_ALLOC:
@@ -275,14 +274,14 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
 #ifdef DEBUG_EXT
             printf("Qcow2: Got highest_alloc 0x%lu\n", s->highest_alloc);
 #endif
-            offset += ((ext.len + 7) & ~7);
             break;
 
         default:
             /* unknown magic -- just skip it */
-            offset += ((ext.len + 7) & ~7);
             break;
         }
+        offset += ((ext.len + 7) & ~7);
+
     }
 
     return 0;
-- 
1.6.0.6

[Qemu-devel] [PATCH 12/14] block: pass BDRV_BACKING flag to open of a backing file

[Uri Lublin]

There may exist some actions needed to be taken only by
the leaf image (and not backing images), such as keeping track
of highest-allocation-offset.

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

diff --git a/block.c b/block.c
index 39ff1dd..8d92d9e 100644
--- a/block.c
+++ b/block.c
@@ -415,7 +415,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     /* Note: for compatibility, we open disk image files as RDWR, and
        RDONLY as fallback */
     if (!(flags & BDRV_O_FILE))
-        open_flags = BDRV_O_RDWR | (flags & BDRV_O_CACHE_MASK);
+        open_flags = BDRV_O_RDWR | (flags & (BDRV_O_CACHE_MASK | BDRV_O_BACKING));
     else
         open_flags = flags & ~(BDRV_O_FILE | BDRV_O_SNAPSHOT);
     ret = drv->bdrv_open(bs, filename, open_flags);
@@ -446,6 +446,7 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
         bs->backing_hd = bdrv_new("");
         path_combine(backing_filename, sizeof(backing_filename),
                      filename, bs->backing_file);
+        open_flags |= BDRV_O_BACKING;
         if (bs->backing_format[0] != '\0')
             back_drv = bdrv_find_format(bs->backing_format);
         ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
diff --git a/block.h b/block.h
index fbb445e..18147cf 100644
--- a/block.h
+++ b/block.h
@@ -56,6 +56,8 @@ typedef struct QEMUSnapshotInfo {
 
 #define BDRV_O_CACHE_MASK  (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_CACHE_DEF)
 
+#define BDRV_O_BACKING     0x0100 /* a backing file */
+
 void bdrv_info(Monitor *mon);
 void bdrv_info_stats(Monitor *mon);
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 13/14] block: keep flags an image was opened with

[Uri Lublin]

Can be used to close and reopen images.

Can be used to check if BDRV_O_BACKING is on (a backing file).
Keep it in a single place instead of keeping it in
two places (also in block-qcow2.c)

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

diff --git a/block.c b/block.c
index 8d92d9e..a938ed7 100644
--- a/block.c
+++ b/block.c
@@ -358,6 +358,8 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags,
     bs->encrypted = 0;
     bs->valid_key = 0;
 
+    bs->open_flags = flags;
+
     if (flags & BDRV_O_SNAPSHOT) {
         BlockDriverState *bs1;
         int64_t total_size;
diff --git a/block_int.h b/block_int.h
index 2206178..715bbad 100644
--- a/block_int.h
+++ b/block_int.h
@@ -124,6 +124,8 @@ struct BlockDriverState {
     int is_temporary;
     int media_changed;
 
+    int open_flags; /* flags the image was opened with */
+
     BlockDriverState *backing_hd;
     /* async read/write emulation */
 
-- 
1.6.0.6

[Qemu-devel] [PATCH 14/14] block-qcow2: do not keep track of highest-alloc for backing files

[Uri Lublin]

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

diff --git a/block-qcow2.c b/block-qcow2.c
index c98024e..63460e0 100644
--- a/block-qcow2.c
+++ b/block-qcow2.c
@@ -260,6 +260,8 @@ static int qcow_read_extensions(BlockDriverState *bs, uint64_t start_offset,
             break;
 
         case QCOW_EXT_MAGIC_HIGH_ALLOC:
+            if ((bs->open_flags & BDRV_O_BACKING) != 0)
+                break;
             if (ext.len != sizeof(uint64_t)) {
                 fprintf(stderr, "ERROR: ext_high_alloc: len=%u too large"
                         " (>=%lu)\n",
@@ -294,6 +296,9 @@ static void update_highest_alloc(BlockDriverState *bs)
     uint64_t high;
     int ret;
 
+    if ((bs->open_flags & BDRV_O_BACKING) != 0)
+        return;
+
 #ifdef DEBUG_EXT
     QCowExtension ext;
     int len;
-- 
1.6.0.6

Re: [Qemu-devel] [PATCH 00/14] block (mostly qcow2) changes (v6)

[Gleb Natapov]

On Tue, Mar 17, 2009 at 10:40:38PM +0200, Uri Lublin wrote:
> Changes from v5:
>    Patchset includes newly introduced qcow2 extensions.
>    Usage of such qcow2 extensions for keeping both backing format and 
>        highest-allocated-offset.
>    No scanning of qcow2 images upon open.
highest-allocated-offset is written to a disk only if block device was closed
properly, so this value can't actually be trusted to be accurate. How
important is to maintain it accurate? If it is important it should be
saved on disk as part of a metadata update and if it is not it can be
updated on a first guest write that requires new block allocation.

--
			Gleb.

Re: [Qemu-devel] [PATCH 00/14] block (mostly qcow2) changes (v6)

[Avi Kivity]

Gleb Natapov wrote:
> On Tue, Mar 17, 2009 at 10:40:38PM +0200, Uri Lublin wrote:
>   
>> Changes from v5:
>>    Patchset includes newly introduced qcow2 extensions.
>>    Usage of such qcow2 extensions for keeping both backing format and 
>>        highest-allocated-offset.
>>    No scanning of qcow2 images upon open.
>>     
> highest-allocated-offset is written to a disk only if block device was closed
> properly, so this value can't actually be trusted to be accurate. How
> important is to maintain it accurate? If it is important it should be
> saved on disk as part of a metadata update and if it is not it can be
> updated on a first guest write that requires new block allocation.
>   

Good catch.

I suggest a different approach.  Have a notification that triggers when 
a write past a certain offset occurs:

    (qemu) block_watermark_notify ide0-0 123564435343
    ... time passes ...
    (qemu) #block watermark exceeded: ide0-0 127454566544

This is in addition to pausing the VM on write error.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.