[Qemu-devel] [PATCH v3] qemu-img: add skip option to dd

[Qemu-devel] [PATCH v3] qemu-img: add skip option to dd

[Reda Sallahi]

This adds the skip option which allows qemu-img dd to skip a number of blocks
before copying the input.

A test case was added to test the skip option.

Signed-off-by: Reda Sallahi <fullmanet@gmail.com>
---
Depends on:
[PATCH v6] qemu-img: add the 'dd' subcommand

Changes from v2:
* Delete a tab that sneaked in (test case 160).

Changes from v1:
* Change test case output.

 qemu-img-cmds.hx           |  4 +--
 qemu-img.c                 | 37 +++++++++++++++++++++---
 qemu-img.texi              |  4 ++-
 tests/qemu-iotests/160     | 72 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/160.out | 51 ++++++++++++++++++++++++++++++++
 tests/qemu-iotests/group   |  1 +
 6 files changed, 162 insertions(+), 7 deletions(-)
 create mode 100755 tests/qemu-iotests/160
 create mode 100644 tests/qemu-iotests/160.out

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 03bdd7a..f054599 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -46,9 +46,9 @@ STEXI
 ETEXI
 
 DEF("dd", img_dd,
-    "dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] if=input of=output")
+    "dd [--image-opts] [-f fmt] [-O output_fmt] [bs=block_size] [count=blocks] [skip=blocks] if=input of=output")
 STEXI
-@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [--image-opts] [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
 ETEXI
 
 DEF("info", img_info,
diff --git a/qemu-img.c b/qemu-img.c
index 498626b..e83f838 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -173,7 +173,8 @@ static void QEMU_NORETURN help(void)
            "(default: 512)\n"
            "  'count=N' copy only N input blocks\n"
            "  'if=FILE' read from FILE\n"
-           "  'of=FILE' write to FILE\n";
+           "  'of=FILE' write to FILE\n"
+           "  'skip=N' skip N bs-sized blocks at the start of input\n";
 
     printf("%s\nSupported formats:", help_msg);
     bdrv_iterate_format(format_print, NULL);
@@ -3805,6 +3806,7 @@ out:
 #define C_COUNT   02
 #define C_IF      04
 #define C_OF      010
+#define C_SKIP    020
 
 struct DdInfo {
     unsigned int flags;
@@ -3815,6 +3817,7 @@ struct DdIo {
     size_t bsz;    /* Block size */
     char *filename;
     uint8_t *buf;
+    int64_t offset;
 };
 
 struct DdOpts {
@@ -3940,6 +3943,19 @@ static int img_dd_of(const char *arg,
     return 0;
 }
 
+static int img_dd_skip(const char *arg,
+                       struct DdIo *in, struct DdIo *out,
+                       struct DdInfo *dd)
+{
+    in->offset = get_size(arg);
+
+    if (in->offset == 0 && (errno == EINVAL || errno == ERANGE)) {
+        return 1;
+    }
+
+    return 0;
+}
+
 static int img_dd(int argc, char **argv)
 {
     int ret = 0;
@@ -3963,12 +3979,14 @@ static int img_dd(int argc, char **argv)
     struct DdIo in = {
         .bsz = 512, /* Block size is by default 512 bytes */
         .filename = NULL,
-        .buf = NULL
+        .buf = NULL,
+        .offset = 0
     };
     struct DdIo out = {
         .bsz = 512,
         .filename = NULL,
-        .buf = NULL
+        .buf = NULL,
+        .offset = 0
     };
 
     const struct DdOpts options[] = {
@@ -3976,6 +3994,7 @@ static int img_dd(int argc, char **argv)
         { "count", img_dd_count, C_COUNT },
         { "if", img_dd_if, C_IF },
         { "of", img_dd_of, C_OF },
+        { "skip", img_dd_skip, C_SKIP },
         { NULL, NULL, 0 }
     };
     const struct option long_options[] = {
@@ -4090,7 +4109,13 @@ static int img_dd(int argc, char **argv)
         size = dd.count * in.bsz;
     }
 
-    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
+    if (dd.flags & C_SKIP && size < in.bsz * in.offset) {
+        error_report("%s: cannot skip to specified offset", in.filename);
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
+    } else {
+        qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
+                            size - in.bsz * in.offset, &error_abort);
+    }
 
     ret = bdrv_create(drv, out.filename, opts, &local_err);
     if (ret < 0) {
@@ -4111,6 +4136,10 @@ static int img_dd(int argc, char **argv)
 
     in.buf = g_new(uint8_t, in.bsz);
 
+    if (dd.flags & C_SKIP) {
+        incount = in.offset * in.bsz;
+    }
+
     for (; incount < size; block_count++) {
         int in_ret, out_ret;
 
diff --git a/qemu-img.texi b/qemu-img.texi
index 880293a..174aae3 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -151,6 +151,8 @@ sets the number of input blocks to copy
 sets the input file
 @item of=@var{output}
 sets the output file
+@item skip=@var{blocks}
+sets the number of input blocks to skip
 @end table
 
 Command description:
@@ -324,7 +326,7 @@ skipped. This is useful for formats such as @code{rbd} if the target
 volume has already been created with site specific options that cannot
 be supplied through qemu-img.
 
-@item dd [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] if=@var{input} of=@var{output}
+@item dd [-f @var{fmt}] [-O @var{output_fmt}] [bs=@var{block_size}] [count=@var{blocks}] [skip=@var{blocks}] if=@var{input} of=@var{output}
 
 Dd copies from @var{input} file to @var{output} file converting it from
 @var{fmt} format to @var{output_fmt} format.
diff --git a/tests/qemu-iotests/160 b/tests/qemu-iotests/160
new file mode 100755
index 0000000..16e2957
--- /dev/null
+++ b/tests/qemu-iotests/160
@@ -0,0 +1,72 @@
+#! /bin/bash
+#
+# qemu-img dd test for the skip option
+#
+# Copyright (C) 2016 Reda Sallahi
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+owner=fullmanet@gmail.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1
+
+_cleanup()
+{
+    _cleanup_test_img
+    rm -f "$TEST_IMG.out" "$TEST_IMG.out.dd"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+TEST_SKIP_BLOCKS="1 2 30 30K"
+
+for skip in $TEST_SKIP_BLOCKS; 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 skip=$skip =="
+
+    $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" skip="$skip" -O "$IMGFMT" \
+        2> /dev/null
+    $QEMU_IMG check "$TEST_IMG.out" -f "$IMGFMT" 2>&1  | _filter_testdir | \
+        _filter_qemu_img_check
+    dd if="$TEST_IMG" of="$TEST_IMG.out.dd" skip="$skip" status=none
+    echo
+    echo "== Compare the images with qemu-img compare =="
+
+    $QEMU_IMG compare "$TEST_IMG.out.dd" "$TEST_IMG.out"
+done
+
+echo
+echo "*** done"
+rm -f "$seq.full"
+status=0
diff --git a/tests/qemu-iotests/160.out b/tests/qemu-iotests/160.out
new file mode 100644
index 0000000..565b03f
--- /dev/null
+++ b/tests/qemu-iotests/160.out
@@ -0,0 +1,51 @@
+QA output created by 160
+
+== Creating image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+No errors were found on the image.
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Converting the image with dd with skip=1 ==
+No errors were found on the image.
+
+== Compare the images with qemu-img compare ==
+Images are identical.
+
+== Creating image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+No errors were found on the image.
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Converting the image with dd with skip=2 ==
+No errors were found on the image.
+
+== Compare the images with qemu-img compare ==
+Images are identical.
+
+== Creating image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+No errors were found on the image.
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Converting the image with dd with skip=30 ==
+No errors were found on the image.
+
+== Compare the images with qemu-img compare ==
+Images are identical.
+
+== Creating image ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+No errors were found on the image.
+wrote 1048576/1048576 bytes at offset 0
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Converting the image with dd with skip=30K ==
+No errors were found on the image.
+
+== Compare the images with qemu-img compare ==
+Images are identical.
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ec712bc..a042f48 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -159,3 +159,4 @@
 157 auto
 158 rw auto quick
 159 rw auto quick
+160 rw auto quick
-- 
2.9.0

Re: [Qemu-devel] [PATCH v3] qemu-img: add skip option to dd

[Fam Zheng]

On Wed, 07/27 16:51, Reda Sallahi wrote:
> +for skip in $TEST_SKIP_BLOCKS; 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

I think the data pattern could to be made less plain (i.e. add some variantion
based on the offset), to catch any misplacement bug in qemu-img dd (for example
off by one errors).

Fam

> +
> +    echo
> +    echo "== Converting the image with dd with skip=$skip =="
> +
> +    $QEMU_IMG dd if="$TEST_IMG" of="$TEST_IMG.out" skip="$skip" -O "$IMGFMT" \
> +        2> /dev/null
> +    $QEMU_IMG check "$TEST_IMG.out" -f "$IMGFMT" 2>&1  | _filter_testdir | \
> +        _filter_qemu_img_check
> +    dd if="$TEST_IMG" of="$TEST_IMG.out.dd" skip="$skip" status=none
> +    echo
> +    echo "== Compare the images with qemu-img compare =="
> +
> +    $QEMU_IMG compare "$TEST_IMG.out.dd" "$TEST_IMG.out"
> +done

Re: [Qemu-devel] [Qemu-block] [PATCH v3] qemu-img: add skip option to dd

[Stefan Hajnoczi]

On Wed, Jul 27, 2016 at 3:51 PM, Reda Sallahi <fullmanet@gmail.com> wrote:
> -    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
> +    if (dd.flags & C_SKIP && size < in.bsz * in.offset) {
> +        error_report("%s: cannot skip to specified offset", in.filename);
> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);

This looks odd.  What is supposed to happen in this case?  Do you have
a test case for it?

Re: [Qemu-devel] [Qemu-block] [PATCH v3] qemu-img: add skip option to dd

[Reda Sallahi]

On 7/28/16, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Wed, Jul 27, 2016 at 3:51 PM, Reda Sallahi <fullmanet@gmail.com> wrote:
>> -    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
>> +    if (dd.flags & C_SKIP && size < in.bsz * in.offset) {
>> +        error_report("%s: cannot skip to specified offset",
>> in.filename);
>> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
>
> This looks odd.  What is supposed to happen in this case?  Do you have
> a test case for it?
>

We print a warning if we have specified a too big of a value for skip (bigger
than the input size) and set the size to 0 for the output disk image. It's the
same behavior on dd(1).

In the last iteration of the test case it covers such a case. Though without
checking the output on stderr.

-- 
Reda <fullmanet@gmail.com>

Re: [Qemu-devel] [Qemu-block] [PATCH v3] qemu-img: add skip option to dd

[Stefan Hajnoczi]

On Thu, Jul 28, 2016 at 4:51 PM, Reda Sallahi <fullmanet@gmail.com> wrote:
> On 7/28/16, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Wed, Jul 27, 2016 at 3:51 PM, Reda Sallahi <fullmanet@gmail.com> wrote:
>>> -    qemu_opt_set_number(opts, BLOCK_OPT_SIZE, size, &error_abort);
>>> +    if (dd.flags & C_SKIP && size < in.bsz * in.offset) {
>>> +        error_report("%s: cannot skip to specified offset",
>>> in.filename);
>>> +        qemu_opt_set_number(opts, BLOCK_OPT_SIZE, 0, &error_abort);
>>
>> This looks odd.  What is supposed to happen in this case?  Do you have
>> a test case for it?
>>
>
> We print a warning if we have specified a too big of a value for skip (bigger
> than the input size) and set the size to 0 for the output disk image. It's the
> same behavior on dd(1).
>
> In the last iteration of the test case it covers such a case. Though without
> checking the output on stderr.

Good that you have a test case.

Please also add a comment here explaining that this behavior is
compatible with dd and this is a warning rather than an error.

Stefan