All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/2] qemu-img convert: ignore read errors
@ 2019-04-09 14:56 ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, mreitz, den, vsementsov, andrey.shinkevich

The 'qemu-img convert' new command option 'force read' with the key '-R'
allows converting a damaged image to get all the available information
in case of the read errors. The program reports read errors and continue
the image conversion. The users should keep in their minds that the
resulting image is inconsistent.

Andrey Shinkevich (2):
  qemu-img convert: ignore read errors
  iotests: new test 253 check qemu-img convert force read

 qemu-img-cmds.hx           |  4 +--
 qemu-img.c                 | 18 ++++++++++--
 qemu-img.texi              |  2 +-
 tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/253.out |  4 +++
 tests/qemu-iotests/group   |  1 +
 6 files changed, 93 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 0/2] qemu-img convert: ignore read errors
@ 2019-04-09 14:56 ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, den, vsementsov, andrey.shinkevich, mreitz

The 'qemu-img convert' new command option 'force read' with the key '-R'
allows converting a damaged image to get all the available information
in case of the read errors. The program reports read errors and continue
the image conversion. The users should keep in their minds that the
resulting image is inconsistent.

Andrey Shinkevich (2):
  qemu-img convert: ignore read errors
  iotests: new test 253 check qemu-img convert force read

 qemu-img-cmds.hx           |  4 +--
 qemu-img.c                 | 18 ++++++++++--
 qemu-img.texi              |  2 +-
 tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/253.out |  4 +++
 tests/qemu-iotests/group   |  1 +
 6 files changed, 93 insertions(+), 5 deletions(-)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/2] qemu-img convert: ignore read errors
@ 2019-04-09 14:56   ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, mreitz, den, vsementsov, andrey.shinkevich

The 'qemu-img convert' new command option 'force read' with the key '-R'
allows converting a damaged image to get all the available information
in case of the read errors. The program reports read errors and continue
the image conversion. The users should keep in their minds that the
resulting image is inconsistent.

Suggested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c       | 18 ++++++++++++++++--
 qemu-img.texi    |  2 +-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1526f32..09af9cb 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -44,9 +44,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("create", img_create,
diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f..494e880 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -175,6 +175,7 @@ static void QEMU_NORETURN help(void)
            "Parameters to convert subcommand:\n"
            "  '-m' specifies how many coroutines work in parallel during the convert\n"
            "       process (defaults to 8)\n"
+           "  '-R' continue the image conversion in case of the sector read error\n"
            "  '-W' allow to write to the target out of order rather than sequential\n"
            "\n"
            "Parameters to snapshot subcommand:\n"
@@ -1579,6 +1580,7 @@ typedef struct ImgConvertState {
     int64_t wait_sector_num[MAX_COROUTINES];
     CoMutex lock;
     int ret;
+    bool force_read;
 } ImgConvertState;
 
 static void convert_select_part(ImgConvertState *s, int64_t sector_num,
@@ -1693,7 +1695,15 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
                 blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
                 n << BDRV_SECTOR_BITS, &qiov, 0);
         if (ret < 0) {
-            return ret;
+            if (!s->force_read) {
+                return ret;
+            }
+            int64_t offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
+            unsigned int bytes = n << BDRV_SECTOR_BITS;
+            error_report("failed to read %d bytes at offset %" PRId64 ": %s",
+                         bytes, offset, strerror(-ret));
+            memset(buf, 0, n * BDRV_SECTOR_SIZE);
+            /* TODO: retry to read smaller chunks */
         }
 
         sector_num += n;
@@ -2018,6 +2028,7 @@ static int img_convert(int argc, char **argv)
         .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
         .wr_in_order        = true,
         .num_coroutines     = 8,
+        .force_read         = false,
     };
 
     for(;;) {
@@ -2029,7 +2040,7 @@ static int img_convert(int argc, char **argv)
             {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
+        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:RWU",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -2132,6 +2143,9 @@ static int img_convert(int argc, char **argv)
         case 'U':
             force_share = true;
             break;
+        case 'R':
+            s.force_read = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *object_opts;
             object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6710a..94f7d80 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -325,7 +325,7 @@ Error on reading data
 
 @end table
 
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
 to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 1/2] qemu-img convert: ignore read errors
@ 2019-04-09 14:56   ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, den, vsementsov, andrey.shinkevich, mreitz

The 'qemu-img convert' new command option 'force read' with the key '-R'
allows converting a damaged image to get all the available information
in case of the read errors. The program reports read errors and continue
the image conversion. The users should keep in their minds that the
resulting image is inconsistent.

Suggested-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 qemu-img-cmds.hx |  4 ++--
 qemu-img.c       | 18 ++++++++++++++++--
 qemu-img.texi    |  2 +-
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1526f32..09af9cb 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -44,9 +44,9 @@ STEXI
 ETEXI
 
 DEF("convert", img_convert,
-    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("create", img_create,
diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f..494e880 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -175,6 +175,7 @@ static void QEMU_NORETURN help(void)
            "Parameters to convert subcommand:\n"
            "  '-m' specifies how many coroutines work in parallel during the convert\n"
            "       process (defaults to 8)\n"
+           "  '-R' continue the image conversion in case of the sector read error\n"
            "  '-W' allow to write to the target out of order rather than sequential\n"
            "\n"
            "Parameters to snapshot subcommand:\n"
@@ -1579,6 +1580,7 @@ typedef struct ImgConvertState {
     int64_t wait_sector_num[MAX_COROUTINES];
     CoMutex lock;
     int ret;
+    bool force_read;
 } ImgConvertState;
 
 static void convert_select_part(ImgConvertState *s, int64_t sector_num,
@@ -1693,7 +1695,15 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
                 blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
                 n << BDRV_SECTOR_BITS, &qiov, 0);
         if (ret < 0) {
-            return ret;
+            if (!s->force_read) {
+                return ret;
+            }
+            int64_t offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
+            unsigned int bytes = n << BDRV_SECTOR_BITS;
+            error_report("failed to read %d bytes at offset %" PRId64 ": %s",
+                         bytes, offset, strerror(-ret));
+            memset(buf, 0, n * BDRV_SECTOR_SIZE);
+            /* TODO: retry to read smaller chunks */
         }
 
         sector_num += n;
@@ -2018,6 +2028,7 @@ static int img_convert(int argc, char **argv)
         .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
         .wr_in_order        = true,
         .num_coroutines     = 8,
+        .force_read         = false,
     };
 
     for(;;) {
@@ -2029,7 +2040,7 @@ static int img_convert(int argc, char **argv)
             {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
             {0, 0, 0, 0}
         };
-        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
+        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:RWU",
                         long_options, NULL);
         if (c == -1) {
             break;
@@ -2132,6 +2143,9 @@ static int img_convert(int argc, char **argv)
         case 'U':
             force_share = true;
             break;
+        case 'R':
+            s.force_read = true;
+            break;
         case OPTION_OBJECT: {
             QemuOpts *object_opts;
             object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6710a..94f7d80 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -325,7 +325,7 @@ Error on reading data
 
 @end table
 
-@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
+@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
 to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/2] iotests: new test 253 check qemu-img convert force read
@ 2019-04-09 14:56   ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, mreitz, den, vsementsov, andrey.shinkevich

A new test for the patch 'qemu-img convert: ignore read errors'

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/253.out |  4 +++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 74 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
new file mode 100755
index 0000000..671a3c4
--- /dev/null
+++ b/tests/qemu-iotests/253
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+#
+# Test of the 'qemu-img convert' for a damaged image.
+#
+# Copyright (c) 2019 Virtuozzo International GmbH
+#
+# 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/>.
+#
+
+import iotests
+import os
+from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+    file_path, log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+disk = file_path('disk')
+chunk = 256 * 1024
+bad_sector = 768
+blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
+
+
+def create_blkdebug_file():
+    file = open(blkdebug_file, 'w')
+    file.write('''
+[inject-error]
+event = "read_aio"
+errno = "5"
+once ="off"
+sector = "%d"
+''' % (bad_sector))
+    file.close()
+
+
+def write_to_disk(offset, size):
+    write = 'write -P 7 {} {}'.format(offset, size)
+    qemu_io('-c', write, disk)
+
+
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+for num in range(1, 4):
+    write_to_disk((num-1) * chunk, chunk)
+
+create_blkdebug_file()
+disk_converted = disk + '_converted'
+log(qemu_img_pipe('convert', '-O', iotests.imgfmt, 'blkdebug:%s:%s'
+                  % (blkdebug_file, disk), disk_converted))
+log(qemu_img_pipe('convert', '-O', iotests.imgfmt, '-R', 'blkdebug:%s:%s'
+                  % (blkdebug_file, disk), disk_converted))
+
+# TODO: with a class, self.assertNotEqual(
+#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk),
+#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk_converted),
+#               'image file map matches the copied file after conversion')
+
+os.remove(disk_converted)
+os.remove(blkdebug_file)
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
new file mode 100644
index 0000000..b972541
--- /dev/null
+++ b/tests/qemu-iotests/253.out
@@ -0,0 +1,4 @@
+qemu-img: error while reading sector 0: Input/output error
+
+qemu-img: failed to read 786432 bytes at offset 0: Input/output error
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bae7718..4cd2fe8 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -248,3 +248,4 @@
 246 rw auto quick
 247 rw auto quick
 248 rw auto quick
+253 rw auto quick
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH 2/2] iotests: new test 253 check qemu-img convert force read
@ 2019-04-09 14:56   ` Andrey Shinkevich
  0 siblings, 0 replies; 11+ messages in thread
From: Andrey Shinkevich @ 2019-04-09 14:56 UTC (permalink / raw)
  To: qemu-devel, qemu-block; +Cc: kwolf, den, vsementsov, andrey.shinkevich, mreitz

A new test for the patch 'qemu-img convert: ignore read errors'

Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
---
 tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/253.out |  4 +++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 74 insertions(+)
 create mode 100755 tests/qemu-iotests/253
 create mode 100644 tests/qemu-iotests/253.out

diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
new file mode 100755
index 0000000..671a3c4
--- /dev/null
+++ b/tests/qemu-iotests/253
@@ -0,0 +1,69 @@
+#!/usr/bin/env python
+#
+# Test of the 'qemu-img convert' for a damaged image.
+#
+# Copyright (c) 2019 Virtuozzo International GmbH
+#
+# 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/>.
+#
+
+import iotests
+import os
+from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+    file_path, log
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+disk = file_path('disk')
+chunk = 256 * 1024
+bad_sector = 768
+blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
+
+
+def create_blkdebug_file():
+    file = open(blkdebug_file, 'w')
+    file.write('''
+[inject-error]
+event = "read_aio"
+errno = "5"
+once ="off"
+sector = "%d"
+''' % (bad_sector))
+    file.close()
+
+
+def write_to_disk(offset, size):
+    write = 'write -P 7 {} {}'.format(offset, size)
+    qemu_io('-c', write, disk)
+
+
+qemu_img_create('-f', iotests.imgfmt, disk, '1M')
+
+for num in range(1, 4):
+    write_to_disk((num-1) * chunk, chunk)
+
+create_blkdebug_file()
+disk_converted = disk + '_converted'
+log(qemu_img_pipe('convert', '-O', iotests.imgfmt, 'blkdebug:%s:%s'
+                  % (blkdebug_file, disk), disk_converted))
+log(qemu_img_pipe('convert', '-O', iotests.imgfmt, '-R', 'blkdebug:%s:%s'
+                  % (blkdebug_file, disk), disk_converted))
+
+# TODO: with a class, self.assertNotEqual(
+#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk),
+#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk_converted),
+#               'image file map matches the copied file after conversion')
+
+os.remove(disk_converted)
+os.remove(blkdebug_file)
diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
new file mode 100644
index 0000000..b972541
--- /dev/null
+++ b/tests/qemu-iotests/253.out
@@ -0,0 +1,4 @@
+qemu-img: error while reading sector 0: Input/output error
+
+qemu-img: failed to read 786432 bytes at offset 0: Input/output error
+
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bae7718..4cd2fe8 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -248,3 +248,4 @@
 246 rw auto quick
 247 rw auto quick
 248 rw auto quick
+253 rw auto quick
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qemu-img convert: ignore read errors
@ 2019-04-10  8:58     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-10  8:58 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, mreitz, Denis Lunev

09.04.2019 17:56, Andrey Shinkevich wrote:
> The 'qemu-img convert' new command option 'force read' with the key '-R'
> allows converting a damaged image to get all the available information
> in case of the read errors. The program reports read errors and continue
> the image conversion. The users should keep in their minds that the
> resulting image is inconsistent.
> 
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   qemu-img-cmds.hx |  4 ++--
>   qemu-img.c       | 18 ++++++++++++++++--
>   qemu-img.texi    |  2 +-
>   3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 1526f32..09af9cb 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -44,9 +44,9 @@ STEXI
>   ETEXI
>   
>   DEF("convert", img_convert,
> -    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
> +    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
>   STEXI
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
>   ETEXI
>   
>   DEF("create", img_create,
> diff --git a/qemu-img.c b/qemu-img.c
> index aa6f81f..494e880 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -175,6 +175,7 @@ static void QEMU_NORETURN help(void)
>              "Parameters to convert subcommand:\n"
>              "  '-m' specifies how many coroutines work in parallel during the convert\n"
>              "       process (defaults to 8)\n"
> +           "  '-R' continue the image conversion in case of the sector read error\n"
>              "  '-W' allow to write to the target out of order rather than sequential\n"
>              "\n"
>              "Parameters to snapshot subcommand:\n"
> @@ -1579,6 +1580,7 @@ typedef struct ImgConvertState {
>       int64_t wait_sector_num[MAX_COROUTINES];
>       CoMutex lock;
>       int ret;
> +    bool force_read;
>   } ImgConvertState;
>   
>   static void convert_select_part(ImgConvertState *s, int64_t sector_num,
> @@ -1693,7 +1695,15 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
>                   blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
>                   n << BDRV_SECTOR_BITS, &qiov, 0);
>           if (ret < 0) {
> -            return ret;
> +            if (!s->force_read) {
> +                return ret;
> +            }
> +            int64_t offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
> +            unsigned int bytes = n << BDRV_SECTOR_BITS;

declarations should be at start of block and better separated by one empty line.
On the other hand, if we add these variables, I think better to declare them in while
block and reuse for blk_co_preadv too.

with at least this fixed (but note, also, that I'm not good in cmd line option defining. Looks like we
have too many places where something should be added):
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> +            error_report("failed to read %d bytes at offset %" PRId64 ": %s",
> +                         bytes, offset, strerror(-ret));
> +            memset(buf, 0, n * BDRV_SECTOR_SIZE);

hmm, may be some non-zero pattern would be better.

> +            /* TODO: retry to read smaller chunks */
>           }
>   
>           sector_num += n;
> @@ -2018,6 +2028,7 @@ static int img_convert(int argc, char **argv)
>           .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
>           .wr_in_order        = true,
>           .num_coroutines     = 8,
> +        .force_read         = false,

It is initialized by zero by default, as all other fields. ".copy_range = false" is
bad example here, I think.

>       };
>   
>       for(;;) {
> @@ -2029,7 +2040,7 @@ static int img_convert(int argc, char **argv)
>               {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
>               {0, 0, 0, 0}
>           };
> -        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
> +        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:RWU",
>                           long_options, NULL);
>           if (c == -1) {
>               break;
> @@ -2132,6 +2143,9 @@ static int img_convert(int argc, char **argv)
>           case 'U':
>               force_share = true;
>               break;
> +        case 'R':
> +            s.force_read = true;
> +            break;
>           case OPTION_OBJECT: {
>               QemuOpts *object_opts;
>               object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 3b6710a..94f7d80 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -325,7 +325,7 @@ Error on reading data
>   
>   @end table
>   
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
>   
>   Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
>   to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
> 


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 1/2] qemu-img convert: ignore read errors
@ 2019-04-10  8:58     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-10  8:58 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, Denis Lunev, mreitz

09.04.2019 17:56, Andrey Shinkevich wrote:
> The 'qemu-img convert' new command option 'force read' with the key '-R'
> allows converting a damaged image to get all the available information
> in case of the read errors. The program reports read errors and continue
> the image conversion. The users should keep in their minds that the
> resulting image is inconsistent.
> 
> Suggested-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   qemu-img-cmds.hx |  4 ++--
>   qemu-img.c       | 18 ++++++++++++++++--
>   qemu-img.texi    |  2 +-
>   3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
> index 1526f32..09af9cb 100644
> --- a/qemu-img-cmds.hx
> +++ b/qemu-img-cmds.hx
> @@ -44,9 +44,9 @@ STEXI
>   ETEXI
>   
>   DEF("convert", img_convert,
> -    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
> +    "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
>   STEXI
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
>   ETEXI
>   
>   DEF("create", img_create,
> diff --git a/qemu-img.c b/qemu-img.c
> index aa6f81f..494e880 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -175,6 +175,7 @@ static void QEMU_NORETURN help(void)
>              "Parameters to convert subcommand:\n"
>              "  '-m' specifies how many coroutines work in parallel during the convert\n"
>              "       process (defaults to 8)\n"
> +           "  '-R' continue the image conversion in case of the sector read error\n"
>              "  '-W' allow to write to the target out of order rather than sequential\n"
>              "\n"
>              "Parameters to snapshot subcommand:\n"
> @@ -1579,6 +1580,7 @@ typedef struct ImgConvertState {
>       int64_t wait_sector_num[MAX_COROUTINES];
>       CoMutex lock;
>       int ret;
> +    bool force_read;
>   } ImgConvertState;
>   
>   static void convert_select_part(ImgConvertState *s, int64_t sector_num,
> @@ -1693,7 +1695,15 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
>                   blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
>                   n << BDRV_SECTOR_BITS, &qiov, 0);
>           if (ret < 0) {
> -            return ret;
> +            if (!s->force_read) {
> +                return ret;
> +            }
> +            int64_t offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
> +            unsigned int bytes = n << BDRV_SECTOR_BITS;

declarations should be at start of block and better separated by one empty line.
On the other hand, if we add these variables, I think better to declare them in while
block and reuse for blk_co_preadv too.

with at least this fixed (but note, also, that I'm not good in cmd line option defining. Looks like we
have too many places where something should be added):
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

> +            error_report("failed to read %d bytes at offset %" PRId64 ": %s",
> +                         bytes, offset, strerror(-ret));
> +            memset(buf, 0, n * BDRV_SECTOR_SIZE);

hmm, may be some non-zero pattern would be better.

> +            /* TODO: retry to read smaller chunks */
>           }
>   
>           sector_num += n;
> @@ -2018,6 +2028,7 @@ static int img_convert(int argc, char **argv)
>           .buf_sectors        = IO_BUF_SIZE / BDRV_SECTOR_SIZE,
>           .wr_in_order        = true,
>           .num_coroutines     = 8,
> +        .force_read         = false,

It is initialized by zero by default, as all other fields. ".copy_range = false" is
bad example here, I think.

>       };
>   
>       for(;;) {
> @@ -2029,7 +2040,7 @@ static int img_convert(int argc, char **argv)
>               {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
>               {0, 0, 0, 0}
>           };
> -        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
> +        c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:RWU",
>                           long_options, NULL);
>           if (c == -1) {
>               break;
> @@ -2132,6 +2143,9 @@ static int img_convert(int argc, char **argv)
>           case 'U':
>               force_share = true;
>               break;
> +        case 'R':
> +            s.force_read = true;
> +            break;
>           case OPTION_OBJECT: {
>               QemuOpts *object_opts;
>               object_opts = qemu_opts_parse_noisily(&qemu_object_opts,
> diff --git a/qemu-img.texi b/qemu-img.texi
> index 3b6710a..94f7d80 100644
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -325,7 +325,7 @@ Error on reading data
>   
>   @end table
>   
> -@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
> +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-R] [-C] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename}
>   
>   Convert the disk image @var{filename} or a snapshot @var{snapshot_param}
>   to disk image @var{output_filename} using format @var{output_fmt}. It can be optionally compressed (@code{-c}
> 


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] iotests: new test 253 check qemu-img convert force read
@ 2019-04-10  8:59     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-10  8:59 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, mreitz, Denis Lunev

09.04.2019 17:56, Andrey Shinkevich wrote:
> A new test for the patch 'qemu-img convert: ignore read errors'
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/253.out |  4 +++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 74 insertions(+)
>   create mode 100755 tests/qemu-iotests/253
>   create mode 100644 tests/qemu-iotests/253.out
> 
> diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
> new file mode 100755
> index 0000000..671a3c4
> --- /dev/null
> +++ b/tests/qemu-iotests/253
> @@ -0,0 +1,69 @@
> +#!/usr/bin/env python
> +#
> +# Test of the 'qemu-img convert' for a damaged image.
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH
> +#
> +# 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/>.
> +#
> +
> +import iotests
> +import os
> +from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
> +    file_path, log
> +
> +iotests.verify_image_format(supported_fmts=['qcow2'])
> +
> +disk = file_path('disk')
> +chunk = 256 * 1024
> +bad_sector = 768
> +blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
> +
> +
> +def create_blkdebug_file():
> +    file = open(blkdebug_file, 'w')
> +    file.write('''
> +[inject-error]
> +event = "read_aio"
> +errno = "5"
> +once ="off"
> +sector = "%d"
> +''' % (bad_sector))
> +    file.close()
> +
> +
> +def write_to_disk(offset, size):
> +    write = 'write -P 7 {} {}'.format(offset, size)
> +    qemu_io('-c', write, disk)
> +
> +
> +qemu_img_create('-f', iotests.imgfmt, disk, '1M')
> +
> +for num in range(1, 4):
> +    write_to_disk((num-1) * chunk, chunk)
> +
> +create_blkdebug_file()
> +disk_converted = disk + '_converted'
> +log(qemu_img_pipe('convert', '-O', iotests.imgfmt, 'blkdebug:%s:%s'
> +                  % (blkdebug_file, disk), disk_converted))
> +log(qemu_img_pipe('convert', '-O', iotests.imgfmt, '-R', 'blkdebug:%s:%s'
> +                  % (blkdebug_file, disk), disk_converted))
> +
> +# TODO: with a class, self.assertNotEqual(
> +#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk),
> +#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk_converted),
> +#               'image file map matches the copied file after conversion')

You may use just assert. But maps will not be different. I think better is to
check resulting image by qemu-io read -P.

> +
> +os.remove(disk_converted)
> +os.remove(blkdebug_file)
> diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
> new file mode 100644
> index 0000000..b972541
> --- /dev/null
> +++ b/tests/qemu-iotests/253.out
> @@ -0,0 +1,4 @@
> +qemu-img: error while reading sector 0: Input/output error
> +
> +qemu-img: failed to read 786432 bytes at offset 0: Input/output error
> +
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index bae7718..4cd2fe8 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -248,3 +248,4 @@
>   246 rw auto quick
>   247 rw auto quick
>   248 rw auto quick
> +253 rw auto quick
> 


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 2/2] iotests: new test 253 check qemu-img convert force read
@ 2019-04-10  8:59     ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 11+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-10  8:59 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, Denis Lunev, mreitz

09.04.2019 17:56, Andrey Shinkevich wrote:
> A new test for the patch 'qemu-img convert: ignore read errors'
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> ---
>   tests/qemu-iotests/253     | 69 ++++++++++++++++++++++++++++++++++++++++++++++
>   tests/qemu-iotests/253.out |  4 +++
>   tests/qemu-iotests/group   |  1 +
>   3 files changed, 74 insertions(+)
>   create mode 100755 tests/qemu-iotests/253
>   create mode 100644 tests/qemu-iotests/253.out
> 
> diff --git a/tests/qemu-iotests/253 b/tests/qemu-iotests/253
> new file mode 100755
> index 0000000..671a3c4
> --- /dev/null
> +++ b/tests/qemu-iotests/253
> @@ -0,0 +1,69 @@
> +#!/usr/bin/env python
> +#
> +# Test of the 'qemu-img convert' for a damaged image.
> +#
> +# Copyright (c) 2019 Virtuozzo International GmbH
> +#
> +# 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/>.
> +#
> +
> +import iotests
> +import os
> +from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
> +    file_path, log
> +
> +iotests.verify_image_format(supported_fmts=['qcow2'])
> +
> +disk = file_path('disk')
> +chunk = 256 * 1024
> +bad_sector = 768
> +blkdebug_file = os.path.join(iotests.test_dir, 'blkdebug.conf')
> +
> +
> +def create_blkdebug_file():
> +    file = open(blkdebug_file, 'w')
> +    file.write('''
> +[inject-error]
> +event = "read_aio"
> +errno = "5"
> +once ="off"
> +sector = "%d"
> +''' % (bad_sector))
> +    file.close()
> +
> +
> +def write_to_disk(offset, size):
> +    write = 'write -P 7 {} {}'.format(offset, size)
> +    qemu_io('-c', write, disk)
> +
> +
> +qemu_img_create('-f', iotests.imgfmt, disk, '1M')
> +
> +for num in range(1, 4):
> +    write_to_disk((num-1) * chunk, chunk)
> +
> +create_blkdebug_file()
> +disk_converted = disk + '_converted'
> +log(qemu_img_pipe('convert', '-O', iotests.imgfmt, 'blkdebug:%s:%s'
> +                  % (blkdebug_file, disk), disk_converted))
> +log(qemu_img_pipe('convert', '-O', iotests.imgfmt, '-R', 'blkdebug:%s:%s'
> +                  % (blkdebug_file, disk), disk_converted))
> +
> +# TODO: with a class, self.assertNotEqual(
> +#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk),
> +#       qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', disk_converted),
> +#               'image file map matches the copied file after conversion')

You may use just assert. But maps will not be different. I think better is to
check resulting image by qemu-io read -P.

> +
> +os.remove(disk_converted)
> +os.remove(blkdebug_file)
> diff --git a/tests/qemu-iotests/253.out b/tests/qemu-iotests/253.out
> new file mode 100644
> index 0000000..b972541
> --- /dev/null
> +++ b/tests/qemu-iotests/253.out
> @@ -0,0 +1,4 @@
> +qemu-img: error while reading sector 0: Input/output error
> +
> +qemu-img: failed to read 786432 bytes at offset 0: Input/output error
> +
> diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
> index bae7718..4cd2fe8 100644
> --- a/tests/qemu-iotests/group
> +++ b/tests/qemu-iotests/group
> @@ -248,3 +248,4 @@
>   246 rw auto quick
>   247 rw auto quick
>   248 rw auto quick
> +253 rw auto quick
> 


-- 
Best regards,
Vladimir

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH 0/2] qemu-img convert: ignore read errors
  2019-04-09 14:56 ` Andrey Shinkevich
                   ` (2 preceding siblings ...)
  (?)
@ 2019-04-10 15:26 ` Max Reitz
  -1 siblings, 0 replies; 11+ messages in thread
From: Max Reitz @ 2019-04-10 15:26 UTC (permalink / raw)
  To: Andrey Shinkevich, qemu-devel, qemu-block; +Cc: kwolf, den, vsementsov

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

On 09.04.19 16:56, Andrey Shinkevich wrote:
> The 'qemu-img convert' new command option 'force read' with the key '-R'
> allows converting a damaged image to get all the available information
> in case of the read errors. The program reports read errors and continue
> the image conversion. The users should keep in their minds that the
> resulting image is inconsistent.

I’ve already sent a series “Add salvaging mode to convert” back in December.

Max


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-04-10 15:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-09 14:56 [Qemu-devel] [PATCH 0/2] qemu-img convert: ignore read errors Andrey Shinkevich
2019-04-09 14:56 ` Andrey Shinkevich
2019-04-09 14:56 ` [Qemu-devel] [PATCH 1/2] " Andrey Shinkevich
2019-04-09 14:56   ` Andrey Shinkevich
2019-04-10  8:58   ` Vladimir Sementsov-Ogievskiy
2019-04-10  8:58     ` Vladimir Sementsov-Ogievskiy
2019-04-09 14:56 ` [Qemu-devel] [PATCH 2/2] iotests: new test 253 check qemu-img convert force read Andrey Shinkevich
2019-04-09 14:56   ` Andrey Shinkevich
2019-04-10  8:59   ` Vladimir Sementsov-Ogievskiy
2019-04-10  8:59     ` Vladimir Sementsov-Ogievskiy
2019-04-10 15:26 ` [Qemu-devel] [PATCH 0/2] qemu-img convert: ignore read errors Max Reitz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.