[Qemu-devel] [PATCH 1/2] qemu-img: Fix dd with skip= and count=

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2359 bytes --]

On 2018-08-15 04:56, Eric Blake wrote:
> When both skip= and count= are active, qemu-img dd was not copying
> enough data. It didn't help that the code made the same check for
> dd.flags & C_SKIP in two separate places. Compute 'size' as the
> amount of bytes to be read, and 'end' as the offset to end at,
> rather than trying to cram both meanings into a single variable
> (which only worked as long as we had at most one of those two
> limiting factors to worry about, but not both).
> 
> Enhance the test to cover more combinations, and expose the problem.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> CC: qemu-stable@nongnu.org
> ---
>  qemu-img.c                 | 39 ++++++++++++++++---------------------
>  tests/qemu-iotests/160     |  9 ++++++---
>  tests/qemu-iotests/160.out | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 26 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 1acddf693c6..d72f0f0ec94 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c

[...]

> @@ -4559,19 +4559,23 @@ static int img_dd(int argc, char **argv)
>          goto out;
>      }
> 
> +    /* Overflow means the specified offset is beyond input image's size */
> +    if (dd.flags & C_SKIP && (in.offset > INT64_MAX / in.bsz ||
> +                              size < in.bsz * in.offset)) {
> +        size = 0;
> +        error_report("%s: cannot skip to specified offset", in.filename);

in_pos should be initialized as well (to "size", I suppose), or my gcc
will continue to complain. :-)

The rest looks good to me.

> +    } else {
> +        size -= in.offset * in.bsz;
> +        in_pos = in.offset * in.bsz;
> +    }
> +

[...]

> diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
> index 5c910e5bfc1..48380a3aafc 100755
> --- a/tests/qemu-iotests/160
> +++ b/tests/qemu-iotests/160
> @@ -44,6 +44,7 @@ _supported_os Linux
>  TEST_SKIP_BLOCKS="1 2 30 30K"
> 
>  for skip in $TEST_SKIP_BLOCKS; do
> +  for count in '' 'count=1 '; do

Ah, so this is why we indent everything by four spaces!  So you can
squeeze in three more block headers without having to re-indent
everything.  I finally see. O:-)

(Not sure why you put a space after the 'count=1', though, but OK)

Max

>      echo
>      echo "== Creating image =="
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]