Re: [Qemu-block] [PATCH 2/2] qemu-img: change opening method for the output in dd

[Max Reitz]

On 07.10.2016 17:36, Reda Sallahi wrote:
> The subcommand dd was creating an output image regardless of whether there
> was one already created. With this patch we try to check first if the output
> image exists and resize it if necessary.
> 
> Signed-off-by: Reda Sallahi <address@hidden>
> ---
>  qemu-img.c             | 124 
> ++++++++++++++++++++++++++++++-------------------
>  tests/qemu-iotests/160 |   1 -
>  2 files changed, 75 insertions(+), 50 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 6c088bd..9b590d4 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -4083,31 +4053,87 @@ static int img_dd(int argc, char **argv)

[...]

>      blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR,
> -                    false, false, true);
> +                    false, false, false);
>  
>      if (!blk2) {

[...]

> +    } else {
> +        int64_t blk2sz = 0;
> +
> +        if (!(dd.conv & C_NOTRUNC)) {
> +            /* We make conv=notrunc mandatory for the moment to avoid
> +               accidental destruction of the output image. Needs to be
> +               changed when a better solution is found */
> +            error_report("conv=notrunc not specified");
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        blk2sz = blk_getlength(blk2);
> +        if (blk2sz < 0) {
> +            error_report("Failed to get size for '%s'", in.filename);
> +            ret = -1;
> +            goto out;
> +        }
> +
> +        if (in.offset <= INT64_MAX / in.bsz && size >= in.offset * in.bsz) {
> +            if (blk2sz < out_size) {

Looks good to me overall, but wouldn't the second condition (blk2sz <
out_size) suffice alone? In my opinion, dropping the first "if" would
make the code easier to read, too.

Max

> +                blk_truncate(blk2, out_size);
> +            }
> +        }
>      }
>  
>      if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||

signature.asc
Description: OpenPGP digital signature