[Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Marc Bevand]

Thanks for the review, Anthony. I made the changes you request --sorry
about the line wraps. I split the patch in 2 parts to isolate the
unrelated change I made to qemu-img.texi in its own changeset.

-marc

[Qemu-devel] [PATCH 1/2] Update documentation for qemu-img convert options

[Marc Bevand]

Update qemu-img's documentation to match the current version of the
help text. Patch based on SVN rev 4656.

Signed-off-by: Marc Bevand <m.bevand <at> gmail.com>
---
 qemu-img.texi |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -r ac83b5658aa6 qemu-img.texi
--- a/qemu-img.texi	Tue Jun 03 21:36:45 2008 -0700
+++ b/qemu-img.texi	Tue Jun 03 21:43:45 2008 -0700
@@ -10,7 +10,7 @@
 @table @option
 @item create [-e] [-6] [-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}] @var{filename} [-O @var{output_fmt}] @var{output_filename}
+@item convert [-c] [-e] [-6] [-f @var{fmt}] [-O @var{output_fmt}] @var{filename} [@var{filename2} [...]] @var{output_filename}
 @item info [-f @var{fmt}] @var{filename}
 @end table
 

[Qemu-devel] [PATCH 2/2] New qemu-img convert -B option to preserve the COW aspect of images and/or re-base them

[Marc Bevand]

If a disk image hd_a is a copy-on-write image based on the backing
file hd_base, it is currently impossible to use qemu-img to convert
hd_a to hd_b (possibly using another disk image format) while keeping
hd_b a copy-on-write image of hd_base. qemu-img also doesn't provide a
feature that would let an enduser re-base a image, for example: adjust
hd_a's backing file name from hd_base to hd_base2 if it had to change
for some reason.

This patch solves the 2 above problems by adding a new qemu-img
convert -B option which works as described below. This is a generic
feature that should work with ANY disk image format supporting backing
files:

  -B output_base_image
     forces the output image to be created as a copy on write image of
     the specified base image; 'output_base_image' should have the
     same content as the input's base image, however the path, image
     format, etc may differ

Examples:

  $ qemu-img info hd_a
  image: hd_a
  file format: qcow
  virtual size: 6.0G (6442450944 bytes)
  disk size: 28K
  cluster_size: 512
  backing file: hd_base (actual path: hd_base)

Converting hd_a (qcow) to hd_b (qcow2) while preserving the
copy-on-write aspect of the image:

  $ qemu-img convert hd_a -O qcow2 -B hd_base hd_b
  $ qemu-img info hd_b
  image: hd_b
  file format: qcow2
  virtual size: 6.0G (6442450944 bytes)
  disk size: 36K
  cluster_size: 4096
  backing file: hd_base (actual path: hd_base)

Renaming the backing file without losing hd_a:

  $ ln hd_base hd_base2
  $ qemu-img convert hd_a -O qcow -B hd_base2 hd_a2
  $ mv hd_a2 hd_a
  $ rm hd_base
  $ qemu-img info hd_a
  image: hd_a
  file format: qcow
  virtual size: 6.0G (6442450944 bytes)
  disk size: 28K
  cluster_size: 512
  backing file: hd_base2 (actual path: hd_base2)

Patch based on SVN rev 4656.

Signed-off-by: Marc Bevand <m.bevand <at> gmail.com>
---
 block.c       |   27 +++++++++++++++++++++++++++
 block.h       |    2 ++
 qemu-img.c    |   47 +++++++++++++++++++++++++++++++++++++++++------
 qemu-img.texi |    8 ++++++--
 4 files changed, 76 insertions(+), 8 deletions(-)

diff -r bdf2a64055a3 block.c
--- a/block.c	Tue Jun 03 21:47:36 2008 -0700
+++ b/block.c	Tue Jun 03 21:54:12 2008 -0700
@@ -884,6 +884,33 @@
         bdrv_flush(bs->backing_hd);
 }
 
+/*
+ * Returns true iff the specified sector is present in the disk image. Drivers
+ * not implementing the functionality are assumed to not support backing files,
+ * hence all their sectors are reported as allocated.
+ *
+ * 'pnum' is set to the number of sectors (including and immediately following
+ * the specified sector) that are known to be in the same
+ * allocated/unallocated state.
+ *
+ * 'nb_sectors' is the max value 'pnum' should be set to.
+ */
+int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+	int *pnum)
+{
+    int64_t n;
+    if (!bs->drv->bdrv_is_allocated) {
+        if (sector_num >= bs->total_sectors) {
+            *pnum = 0;
+            return 0;
+        }
+        n = bs->total_sectors - sector_num;
+        *pnum = (n < nb_sectors) ? (n) : (nb_sectors);
+        return 1;
+    }
+    return bs->drv->bdrv_is_allocated(bs, sector_num, nb_sectors, pnum);
+}
+
 #ifndef QEMU_IMG
 void bdrv_info(void)
 {
diff -r bdf2a64055a3 block.h
--- a/block.h	Tue Jun 03 21:47:36 2008 -0700
+++ b/block.h	Tue Jun 03 21:54:12 2008 -0700
@@ -99,6 +99,8 @@
 
 /* Ensure contents are flushed to disk.  */
 void bdrv_flush(BlockDriverState *bs);
+int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+	int *pnum);
 
 #define BDRV_TYPE_HD     0
 #define BDRV_TYPE_CDROM  1
diff -r bdf2a64055a3 qemu-img.c
--- a/qemu-img.c	Tue Jun 03 21:47:36 2008 -0700
+++ b/qemu-img.c	Tue Jun 03 21:54:12 2008 -0700
@@ -55,13 +55,17 @@
            "Command syntax:\n"
            "  create [-e] [-6] [-b base_image] [-f fmt] filename [size]\n"
            "  commit [-f fmt] filename\n"
-           "  convert [-c] [-e] [-6] [-f fmt] [-O output_fmt] filename [filename2 [...]] output_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"
            "\n"
            "Command parameters:\n"
            "  'filename' is a disk image filename\n"
            "  'base_image' is the read-only disk image which is used as base for a copy on\n"
            "    write image; the copy on write image only stores the modified data\n"
+           "  'output_base_image' forces the output image to be created as a copy on write\n"
+           "    image of the specified base image; 'output_base_image' should have the same\n"
+           "    content as the input's base image, however the path, image format, etc may\n"
+           "    differ\n"
            "  'fmt' is the disk image format. It is guessed automatically in most cases\n"
            "  'size' is the disk image size in kilobytes. Optional suffixes 'M' (megabyte)\n"
            "    and 'G' (gigabyte) are supported\n"
@@ -350,6 +354,13 @@
     return 0;
 }
 
+/*
+ * Returns true iff the first sector pointed to by 'buf' contains at least
+ * a non-NUL byte.
+ *
+ * 'pnum' is set to the number of sectors (including and immediately following
+ * the first one) that are known to be in the same allocated/unallocated state.
+ */
 static int is_allocated_sectors(const uint8_t *buf, int n, int *pnum)
 {
     int v, i;
@@ -373,7 +384,7 @@
 static int img_convert(int argc, char **argv)
 {
     int c, ret, n, n1, bs_n, bs_i, flags, cluster_size, cluster_sectors;
-    const char *fmt, *out_fmt, *out_filename;
+    const char *fmt, *out_fmt, *out_baseimg, *out_filename;
     BlockDriver *drv;
     BlockDriverState **bs, *out_bs;
     int64_t total_sectors, nb_sectors, sector_num, bs_offset;
@@ -384,9 +395,10 @@
 
     fmt = NULL;
     out_fmt = "raw";
+    out_baseimg = NULL;
     flags = 0;
     for(;;) {
-        c = getopt(argc, argv, "f:O:hce6");
+        c = getopt(argc, argv, "f:O:B:hce6");
         if (c == -1)
             break;
         switch(c) {
@@ -398,6 +410,9 @@
             break;
         case 'O':
             out_fmt = optarg;
+            break;
+        case 'B':
+            out_baseimg = optarg;
             break;
         case 'c':
             flags |= BLOCK_FLAG_COMPRESS;
@@ -415,6 +430,9 @@
     if (bs_n < 1) help();
 
     out_filename = argv[argc - 1];
+
+    if (bs_n > 1 && out_baseimg)
+        error("-B makes no sense when concatenating multiple input images");
         
     bs = calloc(bs_n, sizeof(BlockDriverState *));
     if (!bs)
@@ -441,7 +459,7 @@
     if (flags & BLOCK_FLAG_ENCRYPT && flags & BLOCK_FLAG_COMPRESS)
         error("Compression and encryption not supported at the same time");
 
-    ret = bdrv_create(drv, out_filename, total_sectors, NULL, flags);
+    ret = bdrv_create(drv, out_filename, total_sectors, out_baseimg, flags);
     if (ret < 0) {
         if (ret == -ENOTSUP) {
             error("Formatting not supported for file format '%s'", fmt);
@@ -520,7 +538,7 @@
         /* signal EOF to align */
         bdrv_write_compressed(out_bs, 0, NULL, 0);
     } else {
-        sector_num = 0;
+        sector_num = 0; // total number of sectors converted so far
         for(;;) {
             nb_sectors = total_sectors - sector_num;
             if (nb_sectors <= 0)
@@ -543,6 +561,20 @@
             if (n > bs_offset + bs_sectors - sector_num)
                 n = bs_offset + bs_sectors - sector_num;
 
+            /* If the output image is being created as a copy on write image,
+               assume that sectors which are unallocated in the input image
+               are present in both the output's and input's base images (no
+               need to copy them). */
+            if (out_baseimg) {
+               if (!bdrv_is_allocated(bs[bs_i], sector_num - bs_offset, n, &n1)) {
+                  sector_num += n1;
+                  continue;
+               }
+               /* The next 'n1' sectors are allocated in the input image. Copy
+                  only those as they may be followed by unallocated sectors. */
+               n = n1;
+            }
+
             if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) < 0) 
                 error("error while reading");
             /* NOTE: at the same time we convert, we do not write zero
@@ -550,7 +582,10 @@
                should add a specific call to have the info to go faster */
             buf1 = buf;
             while (n > 0) {
-                if (is_allocated_sectors(buf1, n, &n1)) {
+                /* If the output image is being created as a copy on write image,
+                   copy all sectors even the ones containing only NUL bytes,
+                   because they may differ from the sectors in the base image. */
+                if (out_baseimg || is_allocated_sectors(buf1, n, &n1)) {
                     if (bdrv_write(out_bs, sector_num, buf1, n1) < 0)
                         error("error while writing");
                 }
diff -r bdf2a64055a3 qemu-img.texi
--- a/qemu-img.texi	Tue Jun 03 21:47:36 2008 -0700
+++ b/qemu-img.texi	Tue Jun 03 21:54:12 2008 -0700
@@ -10,7 +10,7 @@
 @table @option
 @item create [-e] [-6] [-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}] @var{filename} [@var{filename2} [...]] @var{output_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}
 @end table
 
@@ -21,7 +21,11 @@
 @item base_image
 is the read-only disk image which is used as base for a copy on
     write image; the copy on write image only stores the modified data
-
+@item output_base_image
+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 fmt
 is the disk image format. It is guessed automatically in most cases. The following formats are supported:
 

Re: [Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Jamie Lokier]

Marc Bevand wrote:
> Thanks for the review, Anthony. I made the changes you request --sorry
> about the line wraps. I split the patch in 2 parts to isolate the
> unrelated change I made to qemu-img.texi in its own changeset.

I wonder if a useful option would be to be able to create an image
relative to a _different_ base image.  I.e.

   - convert old flat image  to  new relative image, given new base

   - convert old relative + old base  to  new relative image, given new base

Another thing, which might be easier and more useful, would be
instantiating zeros in the source image into holes in the relative
image.  To save space, after lots of changes on the virtual hard disk
and some zeroing.  (Microsoft has a tool to do this with Virtual PC).

-- Jamie

Re: [Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Marc Bevand]

On Wed, Jun 4, 2008 at 12:37 AM, Jamie Lokier <jamie@shareable.org> wrote:
>
> I wonder if a useful option would be to be able to create an image
> relative to a _different_ base image.  I.e.

I see what you mean. qemu-img could compare the content of each sector
in images A and B (no matter what A is based on) and infer and image
A' (based on B) equivalent to A.

I wonder too if it would be _that_ useful in real-world scenarios.

> Another thing, which might be easier and more useful, would be
> instantiating zeros in the source image into holes in the relative
> image.  To save space, after lots of changes on the virtual hard disk
> and some zeroing.  (Microsoft has a tool to do this with Virtual PC).

Actually "qemu-img convert A B" already does this: zeros in A are
converted to holes in B (more precisely: real holes if B is a raw
image, or undefined clusters if the qcow/qcow2 case). B in this
example would be a flat image.

If you wanted to do the same thing with B a relative image, the
qcow/qcow2 format would need to be extended because there is currently
no defined way to efficiently represent zero'd clusters in relative
images. (Or actually now that I think about it maybe we could
represent zero'd clusters as if they were compressed even when
compression is disabled.)

I am curious, does Virtual PC support relative aka copy-on-write images ?

-- 
Marc Bevand

[Qemu-devel] Re: [PATCH 1/2] Update documentation for qemu-img convert options

[Anthony Liguori]

Marc Bevand wrote:
> Update qemu-img's documentation to match the current version of the
> help text. Patch based on SVN rev 4656.
>
> Signed-off-by: Marc Bevand <m.bevand <at> gmail.com>
> ---
>  qemu-img.texi |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff -r ac83b5658aa6 qemu-img.texi
> --- a/qemu-img.texi	Tue Jun 03 21:36:45 2008 -0700
> +++ b/qemu-img.texi	Tue Jun 03 21:43:45 2008 -0700
> @@ -10,7 +10,7 @@
>  @table @option
>  @item create [-e] [-6] [-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}] @var{filename} [-O @var{output_fmt}] @var{output_filename}
> +@item convert [-c] [-e] [-6] [-f @var{fmt}] [-O @var{output_fmt}] @var{filename} [@var{filename2} [...]] @var{output_filename}
>  @item info [-f @var{fmt}] @var{filename}
>  @end table
>   

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

[Qemu-devel] Re: [PATCH 2/2] New qemu-img convert -B option to preserve the COW aspect of images and/or re-base them

[Anthony Liguori]

Marc Bevand wrote:
> If a disk image hd_a is a copy-on-write image based on the backing
> file hd_base, it is currently impossible to use qemu-img to convert
> hd_a to hd_b (possibly using another disk image format) while keeping
> hd_b a copy-on-write image of hd_base. qemu-img also doesn't provide a
> feature that would let an enduser re-base a image, for example: adjust
> hd_a's backing file name from hd_base to hd_base2 if it had to change
> for some reason.
>
> This patch solves the 2 above problems by adding a new qemu-img
> convert -B option which works as described below. This is a generic
> feature that should work with ANY disk image format supporting backing
> files:
>   

Looks good to me.

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Jamie Lokier]

Marc Bevand wrote:
> > Another thing, which might be easier and more useful, would be
> > instantiating zeros in the source image into holes in the relative
> > image.  To save space, after lots of changes on the virtual hard disk
> > and some zeroing.  (Microsoft has a tool to do this with Virtual PC).
> 
> Actually "qemu-img convert A B" already does this: zeros in A are
> converted to holes in B (more precisely: real holes if B is a raw
> image, or undefined clusters if the qcow/qcow2 case). B in this
> example would be a flat image.
> 
> If you wanted to do the same thing with B a relative image, the
> qcow/qcow2 format would need to be extended because there is currently
> no defined way to efficiently represent zero'd clusters in relative
> images. (Or actually now that I think about it maybe we could
> represent zero'd clusters as if they were compressed even when
> compression is disabled.)

I was actually thinking the base image already has zeros in those
sectors, typically.  This is when you created a relative image, did
lots of work in it, finished by deleting lots of temporary files, ran
a cleanup tool (in the guest) which zeros sectors which aren't
referenced in the filesystem, then those sectors match what's in the
base image again.

But your idea is even better.

> I am curious, does Virtual PC support relative aka copy-on-write images ?

Yes, quite well.  It calls it "Differencing".

You can have images which are relative to other relative images too,
to any depth, to easily branch different test cases and configurations.

Btw, a couple of things I noticed, not sure where else to mention
them:

  - When using a relative image, you should be able to set the
    permissions on the base image to read-only.  If you have a lot of
    relative images, it's really important not to modify the base
    image, and worrying that one of your QEMU processes might write to
    the base image.  Virtual PC allows the base image to be read-only,
    and quite a lot of notes on Differencing stress how important it
    is to set permissions that way, if you are sharing the base.

  - I'm never sure from the documentation what happens when using
    '-snapshot' with a relative image, and you issue the 'commit'
    monitor command.  Does it commit the snapshot to the relative
    image, or the snapshot all the way to the base image (as help suggests).

  - Same for images which are relative to other relative images,
    i.e. nested.  How far up the tree does 'commit' write too?

  - qemu's 'vpc' format support isn't mentioned in the man page.
    That would explain why someone submitted another implementation a
    while back.

  - 'vpc' format support is broken: using Microsoft's freely
    downloadable "Windows Server 2003 R2" image, which is in 'vpc'
    format, will show lots of corruption and chkdsk errors.  "qemu-img
    convert" to convert to raw format, then using that, results in the
    same.  Using Virtual PC's convert-to-raw program results in an
    image which runs in QEMU without these errors, so it's clearly a
    bug in the 'vpc' format handling.

Cheers,
-- Jamie

Re: [Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Marc Bevand]

On Wed, Jun 4, 2008 at 8:39 AM, Jamie Lokier <jamie@shareable.org> wrote:
>
>  - When using a relative image, you should be able to set the
>    permissions on the base image to read-only.  If you have a lot of
>    relative images, it's really important not to modify the base
>    image, and worrying that one of your QEMU processes might write to
>    the base image.  Virtual PC allows the base image to be read-only,
>    and quite a lot of notes on Differencing stress how important it
>    is to set permissions that way, if you are sharing the base.

I have been using qemu this way for a while: hundreds of cow images
based on a small set of read-only base images (that were also
chattr +i to prevent accidents).

>  - I'm never sure from the documentation what happens when using
>    '-snapshot' with a relative image, and you issue the 'commit'
>    monitor command.  Does it commit the snapshot to the relative
>    image, or the snapshot all the way to the base image (as help suggests).

I am pretty sure it commits to the relative image though I have
never used -snapshot and have never looked at the code.

>  - Same for images which are relative to other relative images,
>    i.e. nested.  How far up the tree does 'commit' write too?

To the first base image. The commit command always commit
to the next base image only in the chain. If you have A, based
on B, based on C, the only way to commit to C is to open B and
issue "commit".

>  - qemu's 'vpc' format support isn't mentioned in the man page.
>    That would explain why someone submitted another implementation a
>    while back.

Yeah the doc is generally vastly outdated... There are tons of
undocumented qemu options too.

-- 
Marc Bevand

Re: [Qemu-devel] [PATCH 0/2] Update documentation for qemu-img + add new -B option

[Jamie Lokier]

Marc Bevand wrote:
> On Wed, Jun 4, 2008 at 8:39 AM, Jamie Lokier <jamie@shareable.org> wrote:
> >
> >  - When using a relative image, you should be able to set the
> >    permissions on the base image to read-only.  If you have a lot of
> >    relative images, it's really important not to modify the base
> >    image, and worrying that one of your QEMU processes might write to
> >    the base image.  Virtual PC allows the base image to be read-only,
> >    and quite a lot of notes on Differencing stress how important it
> >    is to set permissions that way, if you are sharing the base.
> 
> I have been using qemu this way for a while: hundreds of cow images
> based on a small set of read-only base images (that were also
> chattr +i to prevent accidents).

I could have sworn qemu complained when I tried to use a read-only
base image - several times.  That's the basis of the above comment.

But not now.  Strange.  Cool! :-)

-- Jamie