Re: [Qemu-devel] [PATCH] Make qemu-img convert properly consider backing file contents when used with -o backing_file

[Kevin Wolf]

Am 29.04.2011 03:42, schrieb Brad Campbell:
> G'day all,
> 
> This patch makes qemu-img properly consider the contents of the output 
> backing file when performing a convert operation. All things considered 
> it would also perform similar to rebase, where you could specify a 
> completely different backing file and it would just de-dup.
> 
> I've poked this in as an attachment as apparently my last attempt at an 
> in-line patch munged the formatting.
> 
> Comments, pokes or flames welcome.

Please include a commit comment and a Signed-off-by line. Instead of
sending a plain diff, you'll get a better result if you have a git
commit locally and use git format-patch to create the patch file.

Also please make sure to avoid trailing whitespace.

> > @@ -719,6 +720,16 @@ static int img_convert(int argc, char **argv)
>      out_baseimg_param = get_option_parameter(param, BLOCK_OPT_BACKING_FILE);
>      if (out_baseimg_param) {
>          out_baseimg = out_baseimg_param->value.s;
> +
> +        /* out_baseimg_parm != NULL even if there is no base img specified! 
> */
> +        if (out_baseimg) {
> +            out_bf = bdrv_new_open(out_baseimg, NULL, BDRV_O_FLAGS);
> +            if (!out_bf) {
> +                error_report("Could not open backing file '%s'", 
> out_baseimg);
> +                ret = -1;
> +                goto out;
> 

bdrv_new_open already prints an error message in case of failure, so we
shouldn't duplicate it here.

> @@ -889,8 +903,15 @@ static int img_convert(int argc, char **argv)
>                     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)) {
> +                    if (bdrv_read(bs[bs_i], sector_num - bs_offset, buf, n) 
> < 0) {
> +                        error_report("error while reading input file");
> +                        goto out;
> +                    }
> +                    if (bdrv_read(out_bf, sector_num - bs_offset, buf3, n) < 
> 0) {
> +                        error_report("error while reading backing file");
> +                        goto out;
> +                    }

Did you test what happens with a backing file that is smaller than the
overlay? I suspect it will fail here after this change.

Also I think you need to set ret = -1 before jumping to out:

Kevin