On 09.08.2016 23:20, Reda Sallahi wrote: > This patch adds a basic dd subcommand analogous to dd(1) to qemu-img. > > For the start, this implements the bs, if, of and count options and requires > both if and of to be specified (no stdin/stdout if not specified) and doesn't > support tty, pipes, etc. > > The image format must be specified with -O for the output if the raw format > is not the intended one. > > Two tests are added to test qemu-img dd. > > Signed-off-by: Reda Sallahi <fullma...@gmail.com> > --- > Changes from v7: > * Remove a C99-style for loop. > Changes from v6: > * Remove get_size() to use qemu_strtosz_suffix() instead. > * Type changes for some fields in DdIo and DdInfo. > Changes from v5: > * Add dd sections on qemu-img.texi. > Changes from v4: > * Fix the exit status. > Changes from v3: > * Delete an unused (so far) field in DdIo. > Changes from v2: > * Add copyright headers to new files. > Changes from v1: > * Removal of dead code. > * Fix a memory leak. > * Complete the cleanup function in the test cases. > > qemu-img-cmds.hx | 6 + > qemu-img.c | 302 > ++++++++++++++++++++++++++++++++++++++- > qemu-img.texi | 25 ++++ > tests/qemu-iotests/158 | 68 +++++++++ > tests/qemu-iotests/158.out | 15 ++ > tests/qemu-iotests/159 | 70 +++++++++ > tests/qemu-iotests/159.out | 87 +++++++++++ > tests/qemu-iotests/common.filter | 9 ++ > tests/qemu-iotests/group | 2 + > 9 files changed, 583 insertions(+), 1 deletion(-) > create mode 100755 tests/qemu-iotests/158 > create mode 100644 tests/qemu-iotests/158.out > create mode 100755 tests/qemu-iotests/159 > create mode 100644 tests/qemu-iotests/159.out
Looks good overall, just some minor comments below (and maybe you still
want to replace {in,out}count by {in,out}_position anyway).
[...]
> diff --git a/qemu-img.c b/qemu-img.c
> index d2865a5..10aaf0e 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
[...]
> @@ -3794,6 +3801,299 @@ out:
[...]
> + size = blk_getlength(blk1);
> + if (size < 0) {
> + error_report("Failed to get size for '%s'", in.filename);
> + ret = -1;
> + goto out;
> + }
> +
> + if (dd.flags & C_COUNT && dd.count * in.bsz < size) {
dd.count * in.bsz can overflow, there should probably be a check for
that before this point.
> + size = dd.count * in.bsz;
> + }
[...]
> diff --git a/tests/qemu-iotests/158 b/tests/qemu-iotests/158
> new file mode 100755
> index 0000000..48972c8
> --- /dev/null
> +++ b/tests/qemu-iotests/158
> @@ -0,0 +1,68 @@
[...]
> +echo
> +echo "== Converting the image with dd =="
> +
> +$QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" -O "$IMGFMT"
> +$QEMU_IMG check "$TEST_IMG.out" -f "$IMGFMT" 2>&1 | _filter_testdir | \
> + _filter_qemu_img_check
A simpler way of doing this would be:
TEST_IMG="$TEST_IMG.out" _check_test_img
[...]
> diff --git a/tests/qemu-iotests/159 b/tests/qemu-iotests/159
> new file mode 100755
> index 0000000..e55d942
> --- /dev/null
> +++ b/tests/qemu-iotests/159
> @@ -0,0 +1,70 @@
[...]
> +for bs in $TEST_SIZES; do
> + echo
> + echo "== Creating image =="
> +
> + size=1M
> + _make_test_img $size
> + _check_test_img
> + $QEMU_IO -c "write -P 0xa 0 $size" "$TEST_IMG" | _filter_qemu_io
> +
> + echo
> + echo "== Converting the image with dd with a block size of $bs =="
> +
> + $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" bs=$bs -O "$IMGFMT"
> + $QEMU_IMG check "$TEST_IMG.out" -f "$IMGFMT" 2>&1 | _filter_testdir | \
> + _filter_qemu_img_check
Again, TEST_IMG="$TEST_IMG.out" _check_test_img would be simpler.
> + echo
> + echo "== Compare the images with qemu-img compare =="
> +
> + $QEMU_IMG compare "$TEST_IMG" "$TEST_IMG.out"
> +done
> +
> +echo
> +echo "*** done"
> +rm -f "$seq.full"
> +status=0
[...]
> diff --git a/tests/qemu-iotests/common.filter
> b/tests/qemu-iotests/common.filter
> index 3ab6e4d..cef5222 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -44,6 +44,15 @@ _filter_imgfmt()
> sed -e "s#$IMGFMT#IMGFMT#g"
> }
>
> +# replace error message when the format is not supported and delete
> +# the output lines after the first one
> +_filter_qemu_img_check()
> +{
> + sed -e '/allocated.*fragmented.*compressed clusters/d' \
> + -e 's/qemu-img: This image format does not support checks/No errors
> were found on the image./' \
> + -e '/Image end offset: [0-9]\+/d'
> +}
> +
If you use the TEST_IMG="$TEST_IMG.out" _check_test_img shorthand I've
proposed above, then this function will no longer be necessary.
If you don't, then please make use of this function in _check_test_img.
That function contains exactly the same code (probably not by chance),
so you should replace that code by a call to this function.
Max
signature.asc
Description: OpenPGP digital signature
