* [Qemu-devel] [PATCH v2 0/6] qemu-img: Add salvaging mode to convert
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
Hi,
This series adds a --salvage option to qemu-img convert. With this,
qemu-img will not abort when it encounters an I/O error. Instead, it
tries to narrow it down and will treat the affected sectors as being
completely 0 (and print a warning).
Testing this is not so easy, because while real I/O errors during read
operations should be treated as described above, errors encountered
during bdrv_block_status() should just be ignored and the affected
sectors should be considered allocated. But blkdebug does not yet have
a way to intercept this, and:
(1) Just adding a new block-status event would be silly, because I don't
want an event, I want it to fail on a certain kind of operation, on
a certain sector range, independently of any events, so why can't we
just do that? See patch 4.
(2) If we just make blkdebug intercept .bdrv_co_block_status() like all
other kinds of operations, at least iotest 041 fails, which does
exactly that silly thing: It uses the read_aio event to wait for any
read. But it turns out that there may be a bdrv_*block_status()
call in between, so suddenly the wrong operation yields an error.
As I said, the real fault here is that it does not really make sense
to pray that the operation you want to fail is the one that is
immediately executed after some event that you hope will trigger
that operation.
See patch 3.
So patch 3 allows blkdebug users to select which kind of I/O operation
they actually want to make fail, and patch 4 allows them to not use any
event, but to have a rule active all the time.
Together, we can then enable error injection for block-status in patch 5
and make use of event=none iotype=block-status in patch 6.
v2:
- Patch 2:
- 2058c2ad261de7f58fae01d63d3d0efa484caf2a has added an error message
when message when *block_status() fails. Continue using this
message (with the problematic offset instead of the sector index),
and adjust my new messages to fit its style.
- Context conflict due to c075a0af22442e88226a6cef22da3fafccfb5d7e
- Patch 3: %s/4\.0/4.1/
- Patch 4: %s/4\.0/4.1/
- Patch 6: Amendments necessitated by the changes to the warnings added
in patch 2
git-backport-diff against v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/6:[----] [--] 'qemu-img: Move quiet into ImgConvertState'
002/6:[0017] [FC] 'qemu-img: Add salvaging mode to convert'
003/6:[0004] [FC] 'blkdebug: Add @iotype error option'
004/6:[0002] [FC] 'blkdebug: Add "none" event'
005/6:[----] [-C] 'blkdebug: Inject errors on .bdrv_co_block_status()'
006/6:[0035] [FC] 'iotests: Test qemu-img convert --salvage'
Max Reitz (6):
qemu-img: Move quiet into ImgConvertState
qemu-img: Add salvaging mode to convert
blkdebug: Add @iotype error option
blkdebug: Add "none" event
blkdebug: Inject errors on .bdrv_co_block_status()
iotests: Test qemu-img convert --salvage
qapi/block-core.json | 33 +++++++-
block/blkdebug.c | 60 ++++++++++++--
qemu-img.c | 98 ++++++++++++++++------
qemu-img-cmds.hx | 4 +-
qemu-img.texi | 5 ++
tests/qemu-iotests/249 | 163 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/249.out | 43 ++++++++++
tests/qemu-iotests/group | 1 +
8 files changed, 368 insertions(+), 39 deletions(-)
create mode 100755 tests/qemu-iotests/249
create mode 100644 tests/qemu-iotests/249.out
--
2.20.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 0/6] qemu-img: Add salvaging mode to convert
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
Hi,
This series adds a --salvage option to qemu-img convert. With this,
qemu-img will not abort when it encounters an I/O error. Instead, it
tries to narrow it down and will treat the affected sectors as being
completely 0 (and print a warning).
Testing this is not so easy, because while real I/O errors during read
operations should be treated as described above, errors encountered
during bdrv_block_status() should just be ignored and the affected
sectors should be considered allocated. But blkdebug does not yet have
a way to intercept this, and:
(1) Just adding a new block-status event would be silly, because I don't
want an event, I want it to fail on a certain kind of operation, on
a certain sector range, independently of any events, so why can't we
just do that? See patch 4.
(2) If we just make blkdebug intercept .bdrv_co_block_status() like all
other kinds of operations, at least iotest 041 fails, which does
exactly that silly thing: It uses the read_aio event to wait for any
read. But it turns out that there may be a bdrv_*block_status()
call in between, so suddenly the wrong operation yields an error.
As I said, the real fault here is that it does not really make sense
to pray that the operation you want to fail is the one that is
immediately executed after some event that you hope will trigger
that operation.
See patch 3.
So patch 3 allows blkdebug users to select which kind of I/O operation
they actually want to make fail, and patch 4 allows them to not use any
event, but to have a rule active all the time.
Together, we can then enable error injection for block-status in patch 5
and make use of event=none iotype=block-status in patch 6.
v2:
- Patch 2:
- 2058c2ad261de7f58fae01d63d3d0efa484caf2a has added an error message
when message when *block_status() fails. Continue using this
message (with the problematic offset instead of the sector index),
and adjust my new messages to fit its style.
- Context conflict due to c075a0af22442e88226a6cef22da3fafccfb5d7e
- Patch 3: %s/4\.0/4.1/
- Patch 4: %s/4\.0/4.1/
- Patch 6: Amendments necessitated by the changes to the warnings added
in patch 2
git-backport-diff against v2:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/6:[----] [--] 'qemu-img: Move quiet into ImgConvertState'
002/6:[0017] [FC] 'qemu-img: Add salvaging mode to convert'
003/6:[0004] [FC] 'blkdebug: Add @iotype error option'
004/6:[0002] [FC] 'blkdebug: Add "none" event'
005/6:[----] [-C] 'blkdebug: Inject errors on .bdrv_co_block_status()'
006/6:[0035] [FC] 'iotests: Test qemu-img convert --salvage'
Max Reitz (6):
qemu-img: Move quiet into ImgConvertState
qemu-img: Add salvaging mode to convert
blkdebug: Add @iotype error option
blkdebug: Add "none" event
blkdebug: Inject errors on .bdrv_co_block_status()
iotests: Test qemu-img convert --salvage
qapi/block-core.json | 33 +++++++-
block/blkdebug.c | 60 ++++++++++++--
qemu-img.c | 98 ++++++++++++++++------
qemu-img-cmds.hx | 4 +-
qemu-img.texi | 5 ++
tests/qemu-iotests/249 | 163 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/249.out | 43 ++++++++++
tests/qemu-iotests/group | 1 +
8 files changed, 368 insertions(+), 39 deletions(-)
create mode 100755 tests/qemu-iotests/249
create mode 100644 tests/qemu-iotests/249.out
--
2.20.1
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 1/6] qemu-img: Move quiet into ImgConvertState
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
Move img_convert()'s quiet flag into the ImgConvertState so it is
accessible by nested functions. -q dictates that it suppresses anything
but errors, so if those functions want to emit warnings, they need to
query this flag first. (There currently are no such warnings, but there
will be as of the next patch.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qemu-img.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f1ea..c53666aa41 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1569,6 +1569,7 @@ typedef struct ImgConvertState {
int64_t target_backing_sectors; /* negative if unknown */
bool wr_in_order;
bool copy_range;
+ bool quiet;
int min_sparse;
int alignment;
size_t cluster_sectors;
@@ -2005,7 +2006,7 @@ static int img_convert(int argc, char **argv)
QDict *open_opts = NULL;
char *options = NULL;
Error *local_err = NULL;
- bool writethrough, src_writethrough, quiet = false, image_opts = false,
+ bool writethrough, src_writethrough, image_opts = false,
skip_create = false, progress = false, tgt_image_opts = false;
int64_t ret = -EINVAL;
bool force_share = false;
@@ -2113,7 +2114,7 @@ static int img_convert(int argc, char **argv)
src_cache = optarg;
break;
case 'q':
- quiet = true;
+ s.quiet = true;
break;
case 'n':
skip_create = true;
@@ -2202,7 +2203,7 @@ static int img_convert(int argc, char **argv)
}
/* Initialize before goto out */
- if (quiet) {
+ if (s.quiet) {
progress = false;
}
qemu_progress_init(progress, 1.0);
@@ -2213,7 +2214,7 @@ static int img_convert(int argc, char **argv)
for (bs_i = 0; bs_i < s.src_num; bs_i++) {
s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
- fmt, src_flags, src_writethrough, quiet,
+ fmt, src_flags, src_writethrough, s.quiet,
force_share);
if (!s.src[bs_i]) {
ret = -1;
@@ -2376,7 +2377,7 @@ static int img_convert(int argc, char **argv)
if (skip_create) {
s.target = img_open(tgt_image_opts, out_filename, out_fmt,
- flags, writethrough, quiet, false);
+ flags, writethrough, s.quiet, false);
} else {
/* TODO ultimately we should allow --target-image-opts
* to be used even when -n is not given.
@@ -2384,7 +2385,7 @@ static int img_convert(int argc, char **argv)
* to allow filenames in option syntax
*/
s.target = img_open_file(out_filename, open_opts, out_fmt,
- flags, writethrough, quiet, false);
+ flags, writethrough, s.quiet, false);
open_opts = NULL; /* blk_new_open will have freed it */
}
if (!s.target) {
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 1/6] qemu-img: Move quiet into ImgConvertState
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
Move img_convert()'s quiet flag into the ImgConvertState so it is
accessible by nested functions. -q dictates that it suppresses anything
but errors, so if those functions want to emit warnings, they need to
query this flag first. (There currently are no such warnings, but there
will be as of the next patch.)
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qemu-img.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index aa6f81f1ea..c53666aa41 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1569,6 +1569,7 @@ typedef struct ImgConvertState {
int64_t target_backing_sectors; /* negative if unknown */
bool wr_in_order;
bool copy_range;
+ bool quiet;
int min_sparse;
int alignment;
size_t cluster_sectors;
@@ -2005,7 +2006,7 @@ static int img_convert(int argc, char **argv)
QDict *open_opts = NULL;
char *options = NULL;
Error *local_err = NULL;
- bool writethrough, src_writethrough, quiet = false, image_opts = false,
+ bool writethrough, src_writethrough, image_opts = false,
skip_create = false, progress = false, tgt_image_opts = false;
int64_t ret = -EINVAL;
bool force_share = false;
@@ -2113,7 +2114,7 @@ static int img_convert(int argc, char **argv)
src_cache = optarg;
break;
case 'q':
- quiet = true;
+ s.quiet = true;
break;
case 'n':
skip_create = true;
@@ -2202,7 +2203,7 @@ static int img_convert(int argc, char **argv)
}
/* Initialize before goto out */
- if (quiet) {
+ if (s.quiet) {
progress = false;
}
qemu_progress_init(progress, 1.0);
@@ -2213,7 +2214,7 @@ static int img_convert(int argc, char **argv)
for (bs_i = 0; bs_i < s.src_num; bs_i++) {
s.src[bs_i] = img_open(image_opts, argv[optind + bs_i],
- fmt, src_flags, src_writethrough, quiet,
+ fmt, src_flags, src_writethrough, s.quiet,
force_share);
if (!s.src[bs_i]) {
ret = -1;
@@ -2376,7 +2377,7 @@ static int img_convert(int argc, char **argv)
if (skip_create) {
s.target = img_open(tgt_image_opts, out_filename, out_fmt,
- flags, writethrough, quiet, false);
+ flags, writethrough, s.quiet, false);
} else {
/* TODO ultimately we should allow --target-image-opts
* to be used even when -n is not given.
@@ -2384,7 +2385,7 @@ static int img_convert(int argc, char **argv)
* to allow filenames in option syntax
*/
s.target = img_open_file(out_filename, open_opts, out_fmt,
- flags, writethrough, quiet, false);
+ flags, writethrough, s.quiet, false);
open_opts = NULL; /* blk_new_open will have freed it */
}
if (!s.target) {
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
This adds a salvaging mode (--salvage) to qemu-img convert which ignores
read errors and treats the respective areas as containing only zeroes.
This can be used for instance to at least partially recover the data
from terminally corrupted qcow2 images.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 85 ++++++++++++++++++++++++++++++++++++------------
qemu-img-cmds.hx | 4 +--
qemu-img.texi | 5 +++
3 files changed, 71 insertions(+), 23 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index c53666aa41..c2216e67a6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -66,6 +66,7 @@ enum {
OPTION_SIZE = 264,
OPTION_PREALLOCATION = 265,
OPTION_SHRINK = 266,
+ OPTION_SALVAGE = 267,
};
typedef enum OutputFormat {
@@ -1569,6 +1570,7 @@ typedef struct ImgConvertState {
int64_t target_backing_sectors; /* negative if unknown */
bool wr_in_order;
bool copy_range;
+ bool salvage;
bool quiet;
int min_sparse;
int alignment;
@@ -1616,25 +1618,44 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
}
if (s->sector_next_status <= sector_num) {
- int64_t count = n * BDRV_SECTOR_SIZE;
+ uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
+ int64_t count;
- if (s->target_has_backing) {
+ do {
+ count = n * BDRV_SECTOR_SIZE;
+
+ if (s->target_has_backing) {
+ ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
+ count, &count, NULL, NULL);
+ } else {
+ ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
+ offset, count, &count, NULL,
+ NULL);
+ }
+
+ if (ret < 0) {
+ if (s->salvage) {
+ if (n == 1) {
+ if (!s->quiet) {
+ warn_report("error while reading block status at "
+ "offset %" PRIu64 ": %s", offset,
+ strerror(-ret));
+ }
+ /* Just try to read the data, then */
+ ret = BDRV_BLOCK_DATA;
+ count = BDRV_SECTOR_SIZE;
+ } else {
+ /* Retry on a shorter range */
+ n = DIV_ROUND_UP(n, 4);
+ }
+ } else {
+ error_report("error while reading block status at offset "
+ "%" PRIu64 ": %s", offset, strerror(-ret));
+ return ret;
+ }
+ }
+ } while (ret < 0);
- ret = bdrv_block_status(blk_bs(s->src[src_cur]),
- (sector_num - src_cur_offset) *
- BDRV_SECTOR_SIZE,
- count, &count, NULL, NULL);
- } else {
- ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
- (sector_num - src_cur_offset) *
- BDRV_SECTOR_SIZE,
- count, &count, NULL, NULL);
- }
- if (ret < 0) {
- error_report("error while reading block status of sector %" PRId64
- ": %s", sector_num, strerror(-ret));
- return ret;
- }
n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
if (ret & BDRV_BLOCK_ZERO) {
@@ -1671,6 +1692,7 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
int nb_sectors, uint8_t *buf)
{
+ uint64_t single_read_until = 0;
int n, ret;
QEMUIOVector qiov;
@@ -1679,6 +1701,7 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
BlockBackend *blk;
int src_cur;
int64_t bs_sectors, src_cur_offset;
+ uint64_t offset;
/* In the case of compression with multiple source files, we can get a
* nb_sectors that spreads into the next part. So we must be able to
@@ -1687,14 +1710,30 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
blk = s->src[src_cur];
bs_sectors = s->src_sectors[src_cur];
+ offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
+
n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset));
+ if (single_read_until > offset) {
+ n = 1;
+ }
qemu_iovec_init_buf(&qiov, buf, n << BDRV_SECTOR_BITS);
- ret = blk_co_preadv(
- blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
- n << BDRV_SECTOR_BITS, &qiov, 0);
+ ret = blk_co_preadv(blk, offset, n << BDRV_SECTOR_BITS, &qiov, 0);
if (ret < 0) {
- return ret;
+ if (s->salvage) {
+ if (n > 1) {
+ single_read_until = offset + (n << BDRV_SECTOR_BITS);
+ continue;
+ } else {
+ if (!s->quiet) {
+ warn_report("error while reading offset %" PRIu64
+ ": %s", offset, strerror(-ret));
+ }
+ memset(buf, 0, BDRV_SECTOR_SIZE);
+ }
+ } else {
+ return ret;
+ }
}
sector_num += n;
@@ -2028,6 +2067,7 @@ static int img_convert(int argc, char **argv)
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{"force-share", no_argument, 0, 'U'},
{"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
+ {"salvage", no_argument, 0, OPTION_SALVAGE},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -2145,6 +2185,9 @@ static int img_convert(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+ case OPTION_SALVAGE:
+ s.salvage = true;
+ break;
case OPTION_TARGET_IMAGE_OPTS:
tgt_image_opts = true;
break;
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1526f327a5..d6f444586c 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] [-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] [--salvage] 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] [-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] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("create", img_create,
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6710a580..827ee9fe32 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
but will not automatically sparsify zero sectors, and may result in a fully
allocated target image depending on the host support for getting allocation
information.
+@item --salvage
+Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
+will still be printed. Areas that cannot be read from the source will be
+treated as containing only zeroes. This option has no effect in copy offloading
+mode (@code{-C}).
@end table
Parameters to dd subcommand:
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
This adds a salvaging mode (--salvage) to qemu-img convert which ignores
read errors and treats the respective areas as containing only zeroes.
This can be used for instance to at least partially recover the data
from terminally corrupted qcow2 images.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 85 ++++++++++++++++++++++++++++++++++++------------
qemu-img-cmds.hx | 4 +--
qemu-img.texi | 5 +++
3 files changed, 71 insertions(+), 23 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index c53666aa41..c2216e67a6 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -66,6 +66,7 @@ enum {
OPTION_SIZE = 264,
OPTION_PREALLOCATION = 265,
OPTION_SHRINK = 266,
+ OPTION_SALVAGE = 267,
};
typedef enum OutputFormat {
@@ -1569,6 +1570,7 @@ typedef struct ImgConvertState {
int64_t target_backing_sectors; /* negative if unknown */
bool wr_in_order;
bool copy_range;
+ bool salvage;
bool quiet;
int min_sparse;
int alignment;
@@ -1616,25 +1618,44 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
}
if (s->sector_next_status <= sector_num) {
- int64_t count = n * BDRV_SECTOR_SIZE;
+ uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
+ int64_t count;
- if (s->target_has_backing) {
+ do {
+ count = n * BDRV_SECTOR_SIZE;
+
+ if (s->target_has_backing) {
+ ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
+ count, &count, NULL, NULL);
+ } else {
+ ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
+ offset, count, &count, NULL,
+ NULL);
+ }
+
+ if (ret < 0) {
+ if (s->salvage) {
+ if (n == 1) {
+ if (!s->quiet) {
+ warn_report("error while reading block status at "
+ "offset %" PRIu64 ": %s", offset,
+ strerror(-ret));
+ }
+ /* Just try to read the data, then */
+ ret = BDRV_BLOCK_DATA;
+ count = BDRV_SECTOR_SIZE;
+ } else {
+ /* Retry on a shorter range */
+ n = DIV_ROUND_UP(n, 4);
+ }
+ } else {
+ error_report("error while reading block status at offset "
+ "%" PRIu64 ": %s", offset, strerror(-ret));
+ return ret;
+ }
+ }
+ } while (ret < 0);
- ret = bdrv_block_status(blk_bs(s->src[src_cur]),
- (sector_num - src_cur_offset) *
- BDRV_SECTOR_SIZE,
- count, &count, NULL, NULL);
- } else {
- ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
- (sector_num - src_cur_offset) *
- BDRV_SECTOR_SIZE,
- count, &count, NULL, NULL);
- }
- if (ret < 0) {
- error_report("error while reading block status of sector %" PRId64
- ": %s", sector_num, strerror(-ret));
- return ret;
- }
n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
if (ret & BDRV_BLOCK_ZERO) {
@@ -1671,6 +1692,7 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
int nb_sectors, uint8_t *buf)
{
+ uint64_t single_read_until = 0;
int n, ret;
QEMUIOVector qiov;
@@ -1679,6 +1701,7 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
BlockBackend *blk;
int src_cur;
int64_t bs_sectors, src_cur_offset;
+ uint64_t offset;
/* In the case of compression with multiple source files, we can get a
* nb_sectors that spreads into the next part. So we must be able to
@@ -1687,14 +1710,30 @@ static int coroutine_fn convert_co_read(ImgConvertState *s, int64_t sector_num,
blk = s->src[src_cur];
bs_sectors = s->src_sectors[src_cur];
+ offset = (sector_num - src_cur_offset) << BDRV_SECTOR_BITS;
+
n = MIN(nb_sectors, bs_sectors - (sector_num - src_cur_offset));
+ if (single_read_until > offset) {
+ n = 1;
+ }
qemu_iovec_init_buf(&qiov, buf, n << BDRV_SECTOR_BITS);
- ret = blk_co_preadv(
- blk, (sector_num - src_cur_offset) << BDRV_SECTOR_BITS,
- n << BDRV_SECTOR_BITS, &qiov, 0);
+ ret = blk_co_preadv(blk, offset, n << BDRV_SECTOR_BITS, &qiov, 0);
if (ret < 0) {
- return ret;
+ if (s->salvage) {
+ if (n > 1) {
+ single_read_until = offset + (n << BDRV_SECTOR_BITS);
+ continue;
+ } else {
+ if (!s->quiet) {
+ warn_report("error while reading offset %" PRIu64
+ ": %s", offset, strerror(-ret));
+ }
+ memset(buf, 0, BDRV_SECTOR_SIZE);
+ }
+ } else {
+ return ret;
+ }
}
sector_num += n;
@@ -2028,6 +2067,7 @@ static int img_convert(int argc, char **argv)
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
{"force-share", no_argument, 0, 'U'},
{"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS},
+ {"salvage", no_argument, 0, OPTION_SALVAGE},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, ":hf:O:B:Cco:l:S:pt:T:qnm:WU",
@@ -2145,6 +2185,9 @@ static int img_convert(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+ case OPTION_SALVAGE:
+ s.salvage = true;
+ break;
case OPTION_TARGET_IMAGE_OPTS:
tgt_image_opts = true;
break;
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 1526f327a5..d6f444586c 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] [-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] [--salvage] 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] [-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] [--salvage] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("create", img_create,
diff --git a/qemu-img.texi b/qemu-img.texi
index 3b6710a580..827ee9fe32 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
but will not automatically sparsify zero sectors, and may result in a fully
allocated target image depending on the host support for getting allocation
information.
+@item --salvage
+Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
+will still be printed. Areas that cannot be read from the source will be
+treated as containing only zeroes. This option has no effect in copy offloading
+mode (@code{-C}).
@end table
Parameters to dd subcommand:
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 3/6] blkdebug: Add @iotype error option
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
This new error option allows users of blkdebug to inject errors only on
certain kinds of I/O operations. Users usually want to make a very
specific operation fail, not just any; but right now they simply hope
that the event that triggers the error injection is followed up with
that very operation. That may not be true, however, because the block
layer is changing (including blkdebug, which may increase the number of
types of I/O operations on which to inject errors).
The new option's default has been chosen to keep backwards
compatibility.
Note that similar to the internal representation, we could choose to
expose this option as a list of I/O types. But there is no practical
use for this, because as described above, users usually know exactly
which kind of operation they want to make fail, so there is no need to
specify multiple I/O types at once. In addition, exposing this option
as a list would require non-trivial changes to qemu_opts_absorb_qdict().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 26 +++++++++++++++++++++++
block/blkdebug.c | 50 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ccbfff9d0..5141e91172 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3235,6 +3235,26 @@
'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
'cor_write'] }
+##
+# @BlkdebugIOType:
+#
+# Kinds of I/O that blkdebug can inject errors in.
+#
+# @read: .bdrv_co_preadv()
+#
+# @write: .bdrv_co_pwritev()
+#
+# @write-zeroes: .bdrv_co_pwrite_zeroes()
+#
+# @discard: .bdrv_co_pdiscard()
+#
+# @flush: .bdrv_co_flush_to_disk()
+#
+# Since: 4.1
+##
+{ 'enum': 'BlkdebugIOType',
+ 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+
##
# @BlkdebugInjectErrorOptions:
#
@@ -3245,6 +3265,11 @@
# @state: the state identifier blkdebug needs to be in to
# actually trigger the event; defaults to "any"
#
+# @iotype: the type of I/O operations on which this error should
+# be injected; defaults to "all read, write,
+# write-zeroes, discard, and flush operations"
+# (since: 4.1)
+#
# @errno: error identifier (errno) to be returned; defaults to
# EIO
#
@@ -3262,6 +3287,7 @@
{ 'struct': 'BlkdebugInjectErrorOptions',
'data': { 'event': 'BlkdebugEvent',
'*state': 'int',
+ '*iotype': 'BlkdebugIOType',
'*errno': 'int',
'*sector': 'int',
'*once': 'bool',
diff --git a/block/blkdebug.c b/block/blkdebug.c
index efd9441625..ca84b12e32 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -75,6 +75,7 @@ typedef struct BlkdebugRule {
int state;
union {
struct {
+ uint64_t iotype_mask;
int error;
int immediately;
int once;
@@ -91,6 +92,9 @@ typedef struct BlkdebugRule {
QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
} BlkdebugRule;
+QEMU_BUILD_BUG_MSG(BLKDEBUGIO_TYPE__MAX > 64,
+ "BlkdebugIOType mask does not fit into an uint64_t");
+
static QemuOptsList inject_error_opts = {
.name = "inject-error",
.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
@@ -103,6 +107,10 @@ static QemuOptsList inject_error_opts = {
.name = "state",
.type = QEMU_OPT_NUMBER,
},
+ {
+ .name = "iotype",
+ .type = QEMU_OPT_STRING,
+ },
{
.name = "errno",
.type = QEMU_OPT_NUMBER,
@@ -162,6 +170,8 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
int event;
struct BlkdebugRule *rule;
int64_t sector;
+ BlkdebugIOType iotype;
+ Error *local_error = NULL;
/* Find the right event for the rule */
event_name = qemu_opt_get(opts, "event");
@@ -192,6 +202,26 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
sector = qemu_opt_get_number(opts, "sector", -1);
rule->options.inject.offset =
sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
+
+ iotype = qapi_enum_parse(&BlkdebugIOType_lookup,
+ qemu_opt_get(opts, "iotype"),
+ BLKDEBUGIO_TYPE__MAX, &local_error);
+ if (local_error) {
+ error_propagate(errp, local_error);
+ return -1;
+ }
+ if (iotype != BLKDEBUGIO_TYPE__MAX) {
+ rule->options.inject.iotype_mask = (1ull << iotype);
+ } else {
+ /* Apply the default */
+ rule->options.inject.iotype_mask =
+ (1ull << BLKDEBUGIO_TYPE_READ)
+ | (1ull << BLKDEBUGIO_TYPE_WRITE)
+ | (1ull << BLKDEBUGIO_TYPE_WRITE_ZEROES)
+ | (1ull << BLKDEBUGIO_TYPE_DISCARD)
+ | (1ull << BLKDEBUGIO_TYPE_FLUSH);
+ }
+
break;
case ACTION_SET_STATE:
@@ -470,7 +500,8 @@ out:
return ret;
}
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ BlkdebugIOType iotype)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugRule *rule = NULL;
@@ -480,9 +511,10 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
uint64_t inject_offset = rule->options.inject.offset;
- if (inject_offset == -1 ||
- (bytes && inject_offset >= offset &&
- inject_offset < offset + bytes))
+ if ((inject_offset == -1 ||
+ (bytes && inject_offset >= offset &&
+ inject_offset < offset + bytes)) &&
+ (rule->options.inject.iotype_mask & (1ull << iotype)))
{
break;
}
@@ -521,7 +553,7 @@ blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
assert(bytes <= bs->bl.max_transfer);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_READ);
if (err) {
return err;
}
@@ -542,7 +574,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
assert(bytes <= bs->bl.max_transfer);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_WRITE);
if (err) {
return err;
}
@@ -552,7 +584,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
static int blkdebug_co_flush(BlockDriverState *bs)
{
- int err = rule_check(bs, 0, 0);
+ int err = rule_check(bs, 0, 0, BLKDEBUGIO_TYPE_FLUSH);
if (err) {
return err;
@@ -586,7 +618,7 @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
assert(bytes <= bs->bl.max_pwrite_zeroes);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_WRITE_ZEROES);
if (err) {
return err;
}
@@ -620,7 +652,7 @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
assert(bytes <= bs->bl.max_pdiscard);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_DISCARD);
if (err) {
return err;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 3/6] blkdebug: Add @iotype error option
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
This new error option allows users of blkdebug to inject errors only on
certain kinds of I/O operations. Users usually want to make a very
specific operation fail, not just any; but right now they simply hope
that the event that triggers the error injection is followed up with
that very operation. That may not be true, however, because the block
layer is changing (including blkdebug, which may increase the number of
types of I/O operations on which to inject errors).
The new option's default has been chosen to keep backwards
compatibility.
Note that similar to the internal representation, we could choose to
expose this option as a list of I/O types. But there is no practical
use for this, because as described above, users usually know exactly
which kind of operation they want to make fail, so there is no need to
specify multiple I/O types at once. In addition, exposing this option
as a list would require non-trivial changes to qemu_opts_absorb_qdict().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 26 +++++++++++++++++++++++
block/blkdebug.c | 50 ++++++++++++++++++++++++++++++++++++--------
2 files changed, 67 insertions(+), 9 deletions(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 7ccbfff9d0..5141e91172 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3235,6 +3235,26 @@
'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
'cor_write'] }
+##
+# @BlkdebugIOType:
+#
+# Kinds of I/O that blkdebug can inject errors in.
+#
+# @read: .bdrv_co_preadv()
+#
+# @write: .bdrv_co_pwritev()
+#
+# @write-zeroes: .bdrv_co_pwrite_zeroes()
+#
+# @discard: .bdrv_co_pdiscard()
+#
+# @flush: .bdrv_co_flush_to_disk()
+#
+# Since: 4.1
+##
+{ 'enum': 'BlkdebugIOType',
+ 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+
##
# @BlkdebugInjectErrorOptions:
#
@@ -3245,6 +3265,11 @@
# @state: the state identifier blkdebug needs to be in to
# actually trigger the event; defaults to "any"
#
+# @iotype: the type of I/O operations on which this error should
+# be injected; defaults to "all read, write,
+# write-zeroes, discard, and flush operations"
+# (since: 4.1)
+#
# @errno: error identifier (errno) to be returned; defaults to
# EIO
#
@@ -3262,6 +3287,7 @@
{ 'struct': 'BlkdebugInjectErrorOptions',
'data': { 'event': 'BlkdebugEvent',
'*state': 'int',
+ '*iotype': 'BlkdebugIOType',
'*errno': 'int',
'*sector': 'int',
'*once': 'bool',
diff --git a/block/blkdebug.c b/block/blkdebug.c
index efd9441625..ca84b12e32 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -75,6 +75,7 @@ typedef struct BlkdebugRule {
int state;
union {
struct {
+ uint64_t iotype_mask;
int error;
int immediately;
int once;
@@ -91,6 +92,9 @@ typedef struct BlkdebugRule {
QSIMPLEQ_ENTRY(BlkdebugRule) active_next;
} BlkdebugRule;
+QEMU_BUILD_BUG_MSG(BLKDEBUGIO_TYPE__MAX > 64,
+ "BlkdebugIOType mask does not fit into an uint64_t");
+
static QemuOptsList inject_error_opts = {
.name = "inject-error",
.head = QTAILQ_HEAD_INITIALIZER(inject_error_opts.head),
@@ -103,6 +107,10 @@ static QemuOptsList inject_error_opts = {
.name = "state",
.type = QEMU_OPT_NUMBER,
},
+ {
+ .name = "iotype",
+ .type = QEMU_OPT_STRING,
+ },
{
.name = "errno",
.type = QEMU_OPT_NUMBER,
@@ -162,6 +170,8 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
int event;
struct BlkdebugRule *rule;
int64_t sector;
+ BlkdebugIOType iotype;
+ Error *local_error = NULL;
/* Find the right event for the rule */
event_name = qemu_opt_get(opts, "event");
@@ -192,6 +202,26 @@ static int add_rule(void *opaque, QemuOpts *opts, Error **errp)
sector = qemu_opt_get_number(opts, "sector", -1);
rule->options.inject.offset =
sector == -1 ? -1 : sector * BDRV_SECTOR_SIZE;
+
+ iotype = qapi_enum_parse(&BlkdebugIOType_lookup,
+ qemu_opt_get(opts, "iotype"),
+ BLKDEBUGIO_TYPE__MAX, &local_error);
+ if (local_error) {
+ error_propagate(errp, local_error);
+ return -1;
+ }
+ if (iotype != BLKDEBUGIO_TYPE__MAX) {
+ rule->options.inject.iotype_mask = (1ull << iotype);
+ } else {
+ /* Apply the default */
+ rule->options.inject.iotype_mask =
+ (1ull << BLKDEBUGIO_TYPE_READ)
+ | (1ull << BLKDEBUGIO_TYPE_WRITE)
+ | (1ull << BLKDEBUGIO_TYPE_WRITE_ZEROES)
+ | (1ull << BLKDEBUGIO_TYPE_DISCARD)
+ | (1ull << BLKDEBUGIO_TYPE_FLUSH);
+ }
+
break;
case ACTION_SET_STATE:
@@ -470,7 +500,8 @@ out:
return ret;
}
-static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
+static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
+ BlkdebugIOType iotype)
{
BDRVBlkdebugState *s = bs->opaque;
BlkdebugRule *rule = NULL;
@@ -480,9 +511,10 @@ static int rule_check(BlockDriverState *bs, uint64_t offset, uint64_t bytes)
QSIMPLEQ_FOREACH(rule, &s->active_rules, active_next) {
uint64_t inject_offset = rule->options.inject.offset;
- if (inject_offset == -1 ||
- (bytes && inject_offset >= offset &&
- inject_offset < offset + bytes))
+ if ((inject_offset == -1 ||
+ (bytes && inject_offset >= offset &&
+ inject_offset < offset + bytes)) &&
+ (rule->options.inject.iotype_mask & (1ull << iotype)))
{
break;
}
@@ -521,7 +553,7 @@ blkdebug_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
assert(bytes <= bs->bl.max_transfer);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_READ);
if (err) {
return err;
}
@@ -542,7 +574,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
assert(bytes <= bs->bl.max_transfer);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_WRITE);
if (err) {
return err;
}
@@ -552,7 +584,7 @@ blkdebug_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes,
static int blkdebug_co_flush(BlockDriverState *bs)
{
- int err = rule_check(bs, 0, 0);
+ int err = rule_check(bs, 0, 0, BLKDEBUGIO_TYPE_FLUSH);
if (err) {
return err;
@@ -586,7 +618,7 @@ static int coroutine_fn blkdebug_co_pwrite_zeroes(BlockDriverState *bs,
assert(bytes <= bs->bl.max_pwrite_zeroes);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_WRITE_ZEROES);
if (err) {
return err;
}
@@ -620,7 +652,7 @@ static int coroutine_fn blkdebug_co_pdiscard(BlockDriverState *bs,
assert(bytes <= bs->bl.max_pdiscard);
}
- err = rule_check(bs, offset, bytes);
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_DISCARD);
if (err) {
return err;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 4/6] blkdebug: Add "none" event
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
Together with @iotypes and @sector, this can be used to trap e.g. the
first read or write access to a certain sector without having to know
what happens internally in the block layer, i.e. which "real" events
happen right before such an access.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 4 +++-
block/blkdebug.c | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5141e91172..717b13f7f5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3215,6 +3215,8 @@
#
# @cor_write: a write due to copy-on-read (since 2.11)
#
+# @none: triggers once at creation of the blkdebug node (since 4.1)
+#
# Since: 2.9
##
{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -3233,7 +3235,7 @@
'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
- 'cor_write'] }
+ 'cor_write', 'none' ] }
##
# @BlkdebugIOType:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index ca84b12e32..69c608be6f 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -491,6 +491,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
+ bdrv_debug_event(bs, BLKDBG_NONE);
+
ret = 0;
out:
if (ret < 0) {
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 4/6] blkdebug: Add "none" event
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
Together with @iotypes and @sector, this can be used to trap e.g. the
first read or write access to a certain sector without having to know
what happens internally in the block layer, i.e. which "real" events
happen right before such an access.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qapi/block-core.json | 4 +++-
block/blkdebug.c | 2 ++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 5141e91172..717b13f7f5 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3215,6 +3215,8 @@
#
# @cor_write: a write due to copy-on-read (since 2.11)
#
+# @none: triggers once at creation of the blkdebug node (since 4.1)
+#
# Since: 2.9
##
{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -3233,7 +3235,7 @@
'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
- 'cor_write'] }
+ 'cor_write', 'none' ] }
##
# @BlkdebugIOType:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index ca84b12e32..69c608be6f 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -491,6 +491,8 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
goto out;
}
+ bdrv_debug_event(bs, BLKDBG_NONE);
+
ret = 0;
out:
if (ret < 0) {
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 5/6] blkdebug: Inject errors on .bdrv_co_block_status()
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-core.json | 5 ++++-
block/blkdebug.c | 8 ++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 717b13f7f5..2aa675a192 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3252,10 +3252,13 @@
#
# @flush: .bdrv_co_flush_to_disk()
#
+# @block-status: .bdrv_co_block_status()
+#
# Since: 4.1
##
{ 'enum': 'BlkdebugIOType',
- 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+ 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush',
+ 'block-status' ] }
##
# @BlkdebugInjectErrorOptions:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 69c608be6f..df9c8b1673 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -670,7 +670,15 @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
int64_t *map,
BlockDriverState **file)
{
+ int err;
+
assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
+
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_BLOCK_STATUS);
+ if (err) {
+ return err;
+ }
+
return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,
pnum, map, file);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 5/6] blkdebug: Inject errors on .bdrv_co_block_status()
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
qapi/block-core.json | 5 ++++-
block/blkdebug.c | 8 ++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 717b13f7f5..2aa675a192 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3252,10 +3252,13 @@
#
# @flush: .bdrv_co_flush_to_disk()
#
+# @block-status: .bdrv_co_block_status()
+#
# Since: 4.1
##
{ 'enum': 'BlkdebugIOType',
- 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush' ] }
+ 'data': [ 'read', 'write', 'write-zeroes', 'discard', 'flush',
+ 'block-status' ] }
##
# @BlkdebugInjectErrorOptions:
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 69c608be6f..df9c8b1673 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -670,7 +670,15 @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
int64_t *map,
BlockDriverState **file)
{
+ int err;
+
assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
+
+ err = rule_check(bs, offset, bytes, BLKDEBUGIO_TYPE_BLOCK_STATUS);
+ if (err) {
+ return err;
+ }
+
return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,
pnum, map, file);
}
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 6/6] iotests: Test qemu-img convert --salvage
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: qemu-devel, Max Reitz, Kevin Wolf, Eric Blake, Andrey Shinkevich,
Denis V . Lunev, Vladimir Sementsov-Ogievskiy
This test converts a simple image to another, but blkdebug injects
block_status and read faults at some offsets. The resulting image
should be the same as the input image, except that sectors that could
not be read have to be 0.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/249 | 163 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/249.out | 43 ++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 207 insertions(+)
create mode 100755 tests/qemu-iotests/249
create mode 100644 tests/qemu-iotests/249.out
diff --git a/tests/qemu-iotests/249 b/tests/qemu-iotests/249
new file mode 100755
index 0000000000..d3883d75f3
--- /dev/null
+++ b/tests/qemu-iotests/249
@@ -0,0 +1,163 @@
+#!/bin/bash
+#
+# Test qemu-img convert --salvage
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# 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/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+
+TEST_IMG="$TEST_IMG.orig" _make_test_img 64M
+
+$QEMU_IO -c 'write -P 42 0 64M' "$TEST_IMG.orig" | _filter_qemu_io
+
+
+sector_size=512
+
+# Offsets on which to fail block-status. Keep in ascending order so
+# the indexing done by _filter_offsets will appear in ascending order
+# in the output as well.
+status_fail_offsets="$((16 * 1024 * 1024 + 8192))
+ $((33 * 1024 * 1024 + 512))"
+
+# Offsets on which to fail reads. Keep in ascending order for the
+# same reason.
+# Element 1 is shared with $status_fail_offsets on purpose.
+# Elements 2 and later test what happens when a continuous range of
+# sectors is inaccessible.
+read_fail_offsets="$((32 * 1024 * 1024 - 65536))
+ $((33 * 1024 * 1024 + 512))
+ $(seq $((34 * 1024 * 1024)) $sector_size \
+ $((34 * 1024 * 1024 + 4096 - $sector_size)))"
+
+
+# blkdebug must be above the format layer so it can intercept all
+# block-status events
+source_img="json:{'driver': 'blkdebug',
+ 'image': {
+ 'driver': '$IMGFMT',
+ 'file': {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG.orig'
+ }
+ },
+ 'inject-error': ["
+
+for ofs in $status_fail_offsets
+do
+ source_img+="{ 'event': 'none',
+ 'iotype': 'block-status',
+ 'errno': 5,
+ 'sector': $((ofs / sector_size)) },"
+done
+
+for ofs in $read_fail_offsets
+do
+ source_img+="{ 'event': 'none',
+ 'iotype': 'read',
+ 'errno': 5,
+ 'sector': $((ofs / sector_size)) },"
+done
+
+# Remove the trailing comma and terminate @inject-error and json:{}
+source_img="${source_img%,} ] }"
+
+
+echo
+
+
+_filter_offsets() {
+ filters=
+
+ index=0
+ for ofs in $2
+ do
+ filters+=" -e s/$(printf "$1" $ofs)/status_fail_offset_$index/"
+ index=$((index + 1))
+ done
+
+ index=0
+ for ofs in $3
+ do
+ filters+=" -e s/$(printf "$1" $ofs)/read_fail_offset_$index/"
+ index=$((index + 1))
+ done
+
+ sed $filters
+}
+
+# While determining the number of allocated sectors in the input
+# image, we should see one block status warning per element of
+# $status_fail_offsets.
+#
+# Then, the image is read. Since the block status is queried in
+# basically the same way, the same warnings as in the previous step
+# should reappear. Interleaved with those we should see a read
+# warning per element of $read_fail_offsets.
+# Note that $read_fail_offsets and $status_fail_offsets share an
+# element (read_fail_offset_1 == status_fail_offset_1), so
+# "status_fail_offset_1" in the output is the same as
+# "read_fail_offset_1".
+$QEMU_IMG convert --salvage "$source_img" "$TEST_IMG" 2>&1 \
+ | _filter_offsets '%i' "$status_fail_offsets" "$read_fail_offsets"
+
+echo
+
+# The offsets where the block status could not be determined should
+# have been treated as containing data and thus should be correct in
+# the output image.
+# The offsets where reading failed altogether should be 0. Make them
+# 0 in the input image, too, so we can compare both images.
+for ofs in $read_fail_offsets
+do
+ $QEMU_IO -c "write -z $ofs $sector_size" "$TEST_IMG.orig" \
+ | _filter_qemu_io \
+ | _filter_offsets '%i' '' "$read_fail_offsets"
+done
+
+echo
+
+# These should be equal now.
+$QEMU_IMG compare "$TEST_IMG.orig" "$TEST_IMG"
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/249.out b/tests/qemu-iotests/249.out
new file mode 100644
index 0000000000..9213d3e450
--- /dev/null
+++ b/tests/qemu-iotests/249.out
@@ -0,0 +1,43 @@
+QA output created by 249
+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_2: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_3: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_4: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_5: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_6: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_7: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_8: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_9: Input/output error
+
+wrote 512/512 bytes at offset read_fail_offset_0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_1
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_2
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_3
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_4
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_5
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_6
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_7
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_8
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_9
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bae7718380..a2e2f507e8 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
+249 auto quick
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH v2 6/6] iotests: Test qemu-img convert --salvage
@ 2019-04-10 20:57 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-10 20:57 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Vladimir Sementsov-Ogievskiy, qemu-devel, Max Reitz,
Denis V . Lunev, Andrey Shinkevich
This test converts a simple image to another, but blkdebug injects
block_status and read faults at some offsets. The resulting image
should be the same as the input image, except that sectors that could
not be read have to be 0.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/249 | 163 +++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/249.out | 43 ++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 207 insertions(+)
create mode 100755 tests/qemu-iotests/249
create mode 100644 tests/qemu-iotests/249.out
diff --git a/tests/qemu-iotests/249 b/tests/qemu-iotests/249
new file mode 100755
index 0000000000..d3883d75f3
--- /dev/null
+++ b/tests/qemu-iotests/249
@@ -0,0 +1,163 @@
+#!/bin/bash
+#
+# Test qemu-img convert --salvage
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# 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/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.qemu
+
+_supported_fmt generic
+_supported_proto file
+_supported_os Linux
+
+
+TEST_IMG="$TEST_IMG.orig" _make_test_img 64M
+
+$QEMU_IO -c 'write -P 42 0 64M' "$TEST_IMG.orig" | _filter_qemu_io
+
+
+sector_size=512
+
+# Offsets on which to fail block-status. Keep in ascending order so
+# the indexing done by _filter_offsets will appear in ascending order
+# in the output as well.
+status_fail_offsets="$((16 * 1024 * 1024 + 8192))
+ $((33 * 1024 * 1024 + 512))"
+
+# Offsets on which to fail reads. Keep in ascending order for the
+# same reason.
+# Element 1 is shared with $status_fail_offsets on purpose.
+# Elements 2 and later test what happens when a continuous range of
+# sectors is inaccessible.
+read_fail_offsets="$((32 * 1024 * 1024 - 65536))
+ $((33 * 1024 * 1024 + 512))
+ $(seq $((34 * 1024 * 1024)) $sector_size \
+ $((34 * 1024 * 1024 + 4096 - $sector_size)))"
+
+
+# blkdebug must be above the format layer so it can intercept all
+# block-status events
+source_img="json:{'driver': 'blkdebug',
+ 'image': {
+ 'driver': '$IMGFMT',
+ 'file': {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG.orig'
+ }
+ },
+ 'inject-error': ["
+
+for ofs in $status_fail_offsets
+do
+ source_img+="{ 'event': 'none',
+ 'iotype': 'block-status',
+ 'errno': 5,
+ 'sector': $((ofs / sector_size)) },"
+done
+
+for ofs in $read_fail_offsets
+do
+ source_img+="{ 'event': 'none',
+ 'iotype': 'read',
+ 'errno': 5,
+ 'sector': $((ofs / sector_size)) },"
+done
+
+# Remove the trailing comma and terminate @inject-error and json:{}
+source_img="${source_img%,} ] }"
+
+
+echo
+
+
+_filter_offsets() {
+ filters=
+
+ index=0
+ for ofs in $2
+ do
+ filters+=" -e s/$(printf "$1" $ofs)/status_fail_offset_$index/"
+ index=$((index + 1))
+ done
+
+ index=0
+ for ofs in $3
+ do
+ filters+=" -e s/$(printf "$1" $ofs)/read_fail_offset_$index/"
+ index=$((index + 1))
+ done
+
+ sed $filters
+}
+
+# While determining the number of allocated sectors in the input
+# image, we should see one block status warning per element of
+# $status_fail_offsets.
+#
+# Then, the image is read. Since the block status is queried in
+# basically the same way, the same warnings as in the previous step
+# should reappear. Interleaved with those we should see a read
+# warning per element of $read_fail_offsets.
+# Note that $read_fail_offsets and $status_fail_offsets share an
+# element (read_fail_offset_1 == status_fail_offset_1), so
+# "status_fail_offset_1" in the output is the same as
+# "read_fail_offset_1".
+$QEMU_IMG convert --salvage "$source_img" "$TEST_IMG" 2>&1 \
+ | _filter_offsets '%i' "$status_fail_offsets" "$read_fail_offsets"
+
+echo
+
+# The offsets where the block status could not be determined should
+# have been treated as containing data and thus should be correct in
+# the output image.
+# The offsets where reading failed altogether should be 0. Make them
+# 0 in the input image, too, so we can compare both images.
+for ofs in $read_fail_offsets
+do
+ $QEMU_IO -c "write -z $ofs $sector_size" "$TEST_IMG.orig" \
+ | _filter_qemu_io \
+ | _filter_offsets '%i' '' "$read_fail_offsets"
+done
+
+echo
+
+# These should be equal now.
+$QEMU_IMG compare "$TEST_IMG.orig" "$TEST_IMG"
+
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/249.out b/tests/qemu-iotests/249.out
new file mode 100644
index 0000000000..9213d3e450
--- /dev/null
+++ b/tests/qemu-iotests/249.out
@@ -0,0 +1,43 @@
+QA output created by 249
+Formatting 'TEST_DIR/t.IMGFMT.orig', fmt=IMGFMT size=67108864
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_0: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_0: Input/output error
+qemu-img: warning: error while reading block status at offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset status_fail_offset_1: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_2: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_3: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_4: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_5: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_6: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_7: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_8: Input/output error
+qemu-img: warning: error while reading offset read_fail_offset_9: Input/output error
+
+wrote 512/512 bytes at offset read_fail_offset_0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_1
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_2
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_3
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_4
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_5
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_6
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_7
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_8
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 512/512 bytes at offset read_fail_offset_9
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Images are identical.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bae7718380..a2e2f507e8 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
+249 auto quick
--
2.20.1
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] qemu-img: Move quiet into ImgConvertState
@ 2019-04-11 13:48 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-11 13:48 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: qemu-devel, Kevin Wolf, Eric Blake, Andrey Shinkevich, Denis Lunev
10.04.2019 23:57, Max Reitz wrote:
> Move img_convert()'s quiet flag into the ImgConvertState so it is
> accessible by nested functions. -q dictates that it suppresses anything
> but errors, so if those functions want to emit warnings, they need to
> query this flag first. (There currently are no such warnings, but there
> will be as of the next patch.)
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/6] qemu-img: Move quiet into ImgConvertState
@ 2019-04-11 13:48 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-11 13:48 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Andrey Shinkevich, qemu-devel, Denis Lunev
10.04.2019 23:57, Max Reitz wrote:
> Move img_convert()'s quiet flag into the ImgConvertState so it is
> accessible by nested functions. -q dictates that it suppresses anything
> but errors, so if those functions want to emit warnings, they need to
> query this flag first. (There currently are no such warnings, but there
> will be as of the next patch.)
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-11 14:33 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-11 14:33 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: qemu-devel, Kevin Wolf, Eric Blake, Andrey Shinkevich, Denis Lunev
10.04.2019 23:57, Max Reitz wrote:
> This adds a salvaging mode (--salvage) to qemu-img convert which ignores
> read errors and treats the respective areas as containing only zeroes.
> This can be used for instance to at least partially recover the data
> from terminally corrupted qcow2 images.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
[..]
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
> but will not automatically sparsify zero sectors, and may result in a fully
> allocated target image depending on the host support for getting allocation
> information.
> +@item --salvage
> +Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
> +will still be printed. Areas that cannot be read from the source will be
> +treated as containing only zeroes. This option has no effect in copy offloading
> +mode (@code{-C}).
Isn't it better to restrict such it, instead of creating meaningless combination?
And drop the restriction if we support it in future, instead of silent move from
noop to the feature?
Otherwise, looks OK for me. I dislike mixing byte-based and sector-based logic, but it
is not about the series, we should move to byte-based like in other places, and it may
be done later.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-11 14:33 ` Vladimir Sementsov-Ogievskiy
0 siblings, 0 replies; 20+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-04-11 14:33 UTC (permalink / raw)
To: Max Reitz, qemu-block
Cc: Kevin Wolf, Andrey Shinkevich, qemu-devel, Denis Lunev
10.04.2019 23:57, Max Reitz wrote:
> This adds a salvaging mode (--salvage) to qemu-img convert which ignores
> read errors and treats the respective areas as containing only zeroes.
> This can be used for instance to at least partially recover the data
> from terminally corrupted qcow2 images.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
[..]
> --- a/qemu-img.texi
> +++ b/qemu-img.texi
> @@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
> but will not automatically sparsify zero sectors, and may result in a fully
> allocated target image depending on the host support for getting allocation
> information.
> +@item --salvage
> +Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
> +will still be printed. Areas that cannot be read from the source will be
> +treated as containing only zeroes. This option has no effect in copy offloading
> +mode (@code{-C}).
Isn't it better to restrict such it, instead of creating meaningless combination?
And drop the restriction if we support it in future, instead of silent move from
noop to the feature?
Otherwise, looks OK for me. I dislike mixing byte-based and sector-based logic, but it
is not about the series, we should move to byte-based like in other places, and it may
be done later.
--
Best regards,
Vladimir
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-13 1:01 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-13 1:01 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: qemu-devel, Kevin Wolf, Eric Blake, Andrey Shinkevich, Denis Lunev
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
On 11.04.19 16:33, Vladimir Sementsov-Ogievskiy wrote:
> 10.04.2019 23:57, Max Reitz wrote:
>> This adds a salvaging mode (--salvage) to qemu-img convert which ignores
>> read errors and treats the respective areas as containing only zeroes.
>> This can be used for instance to at least partially recover the data
>> from terminally corrupted qcow2 images.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>
> [..]
>
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
>> but will not automatically sparsify zero sectors, and may result in a fully
>> allocated target image depending on the host support for getting allocation
>> information.
>> +@item --salvage
>> +Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
>> +will still be printed. Areas that cannot be read from the source will be
>> +treated as containing only zeroes. This option has no effect in copy offloading
>> +mode (@code{-C}).
>
> Isn't it better to restrict such it, instead of creating meaningless combination?
> And drop the restriction if we support it in future, instead of silent move from
> noop to the feature?
You’re right, I should do that.
> Otherwise, looks OK for me. I dislike mixing byte-based and sector-based logic, but it
> is not about the series, we should move to byte-based like in other places, and it may
> be done later.
Thanks for having a look!
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert
@ 2019-04-13 1:01 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-04-13 1:01 UTC (permalink / raw)
To: Vladimir Sementsov-Ogievskiy, qemu-block
Cc: Kevin Wolf, Andrey Shinkevich, qemu-devel, Denis Lunev
[-- Attachment #1: Type: text/plain, Size: 1542 bytes --]
On 11.04.19 16:33, Vladimir Sementsov-Ogievskiy wrote:
> 10.04.2019 23:57, Max Reitz wrote:
>> This adds a salvaging mode (--salvage) to qemu-img convert which ignores
>> read errors and treats the respective areas as containing only zeroes.
>> This can be used for instance to at least partially recover the data
>> from terminally corrupted qcow2 images.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>
> [..]
>
>> --- a/qemu-img.texi
>> +++ b/qemu-img.texi
>> @@ -175,6 +175,11 @@ improve performance if the data is remote, such as with NFS or iSCSI backends,
>> but will not automatically sparsify zero sectors, and may result in a fully
>> allocated target image depending on the host support for getting allocation
>> information.
>> +@item --salvage
>> +Try to ignore I/O errors when reading. Unless in quiet mode (@code{-q}), errors
>> +will still be printed. Areas that cannot be read from the source will be
>> +treated as containing only zeroes. This option has no effect in copy offloading
>> +mode (@code{-C}).
>
> Isn't it better to restrict such it, instead of creating meaningless combination?
> And drop the restriction if we support it in future, instead of silent move from
> noop to the feature?
You’re right, I should do that.
> Otherwise, looks OK for me. I dislike mixing byte-based and sector-based logic, but it
> is not about the series, we should move to byte-based like in other places, and it may
> be done later.
Thanks for having a look!
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-04-13 1:03 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-10 20:57 [Qemu-devel] [PATCH v2 0/6] qemu-img: Add salvaging mode to convert Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 1/6] qemu-img: Move quiet into ImgConvertState Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-11 13:48 ` Vladimir Sementsov-Ogievskiy
2019-04-11 13:48 ` Vladimir Sementsov-Ogievskiy
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 2/6] qemu-img: Add salvaging mode to convert Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-11 14:33 ` Vladimir Sementsov-Ogievskiy
2019-04-11 14:33 ` Vladimir Sementsov-Ogievskiy
2019-04-13 1:01 ` Max Reitz
2019-04-13 1:01 ` Max Reitz
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 3/6] blkdebug: Add @iotype error option Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 4/6] blkdebug: Add "none" event Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 5/6] blkdebug: Inject errors on .bdrv_co_block_status() Max Reitz
2019-04-10 20:57 ` Max Reitz
2019-04-10 20:57 ` [Qemu-devel] [PATCH v2 6/6] iotests: Test qemu-img convert --salvage Max Reitz
2019-04-10 20:57 ` 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.