diff mbox

qemu-img rebase: allow empty file name as new backing file to mean rebase with no backing file [attempt 2]

Message ID 1350332607-31177-1-git-send-email-alex@alex.org.uk
State New
Headers show

Commit Message

Alex Bligh Oct. 15, 2012, 8:23 p.m. UTC 
This patch allows an empty filename to be passed as the new base image name
for qemu-img rebase to mean base the image on no backing file (i.e.
independent of any backing file). According to Eric Blake, qemu-imag rebase
already supports this when '-u' is used; this adds support when -u is not
used.

Signed-off-by: Alex Bligh <alex@alex.org.uk>
---
 qemu-img.c    |   26 ++++++++++++++++----------
 qemu-img.texi |    4 +++-
 2 files changed, 19 insertions(+), 11 deletions(-)

Also obtainable from:
  https://github.com/abligh/qemu.git
Commit at:
  https://github.com/abligh/qemu/commit/49cd454aa09062b151710cc7afd4bb7fcf1070d0

Comments

Eric Blake Oct. 15, 2012, 8:44 p.m. UTC | #1 
On 10/15/2012 02:23 PM, Alex Bligh wrote:

Rather long on the subject line; 'git shortlog -30' will give you a
taste for typical commit summary length.  Also, you will usually see
[PATCHv2] as the prefix, rather than [PATCH]...[attempt 2] as a suffix,
since 'git am' only strips a [] prefix.  It might be better as:

qemu-img rebase: use empty string to rebase without backing file

> This patch allows an empty filename to be passed as the new base image name
> for qemu-img rebase to mean base the image on no backing file (i.e.
> independent of any backing file). According to Eric Blake, qemu-imag rebase

s/qemu-imag/qemu-img/

> already supports this when '-u' is used; this adds support when -u is not
> used.
> 
> Signed-off-by: Alex Bligh <alex@alex.org.uk>
> ---

> @@ -1675,7 +1677,11 @@ static int img_rebase(int argc, char **argv)
>       * backing file are overwritten in the COW file now, so the visible content
>       * doesn't change when we switch the backing file.
>       */
> -    ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
> +    if (bs_new_backing)
> +        ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
> +    else
> +        ret = bdrv_change_backing_file(bs, NULL, NULL);

Should we error out if the user specifies basefmt but gives the empty
string, instead of silently ignoring the bogus basefmt?  But that's
bikeshedding, I can live with what you wrote.

Reviewed-by: Eric Blake <eblake@redhat.com>
Alex Bligh Oct. 15, 2012, 8:51 p.m. UTC | #2 
--On 15 October 2012 14:44:59 -0600 Eric Blake <eblake@redhat.com> wrote:

> Rather long on the subject line; 'git shortlog -30' will give you a
> taste for typical commit summary length.  Also, you will usually see
> [PATCHv2] as the prefix, rather than [PATCH]...[attempt 2] as a suffix,
> since 'git am' only strips a [] prefix.  It might be better as:
>
> qemu-img rebase: use empty string to rebase without backing file

Resent under separate cover with this fixed

>> This patch allows an empty filename to be passed as the new base image name
>> for qemu-img rebase to mean base the image on no backing file (i.e.
>> independent of any backing file). According to Eric Blake, qemu-imag rebase
>
> s/qemu-imag/qemu-img/

and that fixed
diff mbox

Patch

diff --git a/qemu-img.c b/qemu-img.c
index f17f187..e817498 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1558,13 +1558,14 @@  static int img_rebase(int argc, char **argv)
             error_report("Could not open old backing file '%s'", backing_name);
             goto out;
         }
-
-        bs_new_backing = bdrv_new("new_backing");
-        ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
+        if (out_baseimg[0]) {
+            bs_new_backing = bdrv_new("new_backing");
+            ret = bdrv_open(bs_new_backing, out_baseimg, BDRV_O_FLAGS,
                         new_backing_drv);
-        if (ret) {
-            error_report("Could not open new backing file '%s'", out_baseimg);
-            goto out;
+            if (ret) {
+                error_report("Could not open new backing file '%s'", out_baseimg);
+                goto out;
+            }
         }
     }
 
@@ -1580,7 +1581,7 @@  static int img_rebase(int argc, char **argv)
     if (!unsafe) {
         uint64_t num_sectors;
         uint64_t old_backing_num_sectors;
-        uint64_t new_backing_num_sectors;
+        uint64_t new_backing_num_sectors = 0;
         uint64_t sector;
         int n;
         uint8_t * buf_old;
@@ -1592,7 +1593,8 @@  static int img_rebase(int argc, char **argv)
 
         bdrv_get_geometry(bs, &num_sectors);
         bdrv_get_geometry(bs_old_backing, &old_backing_num_sectors);
-        bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors);
+        if (bs_new_backing)
+            bdrv_get_geometry(bs_new_backing, &new_backing_num_sectors);
 
         local_progress = (float)100 /
             (num_sectors / MIN(num_sectors, IO_BUF_SIZE / 512));
@@ -1629,7 +1631,7 @@  static int img_rebase(int argc, char **argv)
                 }
             }
 
-            if (sector >= new_backing_num_sectors) {
+            if (sector >= new_backing_num_sectors || !bs_new_backing) {
                 memset(buf_new, 0, n * BDRV_SECTOR_SIZE);
             } else {
                 if (sector + n > new_backing_num_sectors) {
@@ -1675,7 +1677,11 @@  static int img_rebase(int argc, char **argv)
      * backing file are overwritten in the COW file now, so the visible content
      * doesn't change when we switch the backing file.
      */
-    ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
+    if (bs_new_backing)
+        ret = bdrv_change_backing_file(bs, out_baseimg, out_basefmt);
+    else
+        ret = bdrv_change_backing_file(bs, NULL, NULL);
+
     if (ret == -ENOSPC) {
         error_report("Could not change the backing file to '%s': No "
                      "space left in the file header", out_baseimg);
diff --git a/qemu-img.texi b/qemu-img.texi
index 8b05f2c..42ec392 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -148,7 +148,9 @@  Changes the backing file of an image. Only the formats @code{qcow2} and
 
 The backing file is changed to @var{backing_file} and (if the image format of
 @var{filename} supports this) the backing file format is changed to
-@var{backing_fmt}.
+@var{backing_fmt}. If @var{backing_file} is specified as ``'' (the empty
+string), then the image is rebased onto no backing file (i.e. it will exist
+independently of any backing file).
 
 There are two different modes in which @code{rebase} can operate:
 @table @option