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 <ebl...@redhat.com> > CC: qemu-sta...@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 =="
>
signature.asc
Description: OpenPGP digital signature
