Re: [Qemu-devel] [PATCHv4] qemu-img rebase: use empty string to rebase without backing file

[Kevin Wolf]

Am 16.10.2012 14:46, schrieb Alex Bligh:
> 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-img rebase
> already supports this when '-u' is used; this adds support when -u is not
> used.
> 
> Signed-off-by: Alex Bligh <address@hidden>
> ---
>  qemu-img.c    |   29 +++++++++++++++++++----------
>  qemu-img.texi |    4 +++-
>  2 files changed, 22 insertions(+), 11 deletions(-)
> 
> Also obtainable from:
>   https://github.com/abligh/qemu.git
> Commit at:
>   
> https://github.com/abligh/qemu/commit/982ec105018f9901619663642de9d0a5de86799d
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index f17f187..2816f37 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1558,13 +1558,15 @@ 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 +1582,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 +1594,9 @@ 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 +1633,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 +1679,12 @@ 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) {

I think this needs to be out_baseimg, otherwise -u is broken. I've
updated the patch, please check if you agree with the fix.

Kevin