* [Qemu-devel] [PULL 00/29] Block patches
@ 2018-06-11 14:25 Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 01/29] block/file-posix: Pass FD to locking helpers Max Reitz
` (29 more replies)
0 siblings, 30 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
The following changes since commit 9f55925b8f50a962d1d08d815044db7767ae3838:
Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-3.0-pull-request' into staging (2018-06-11 12:46:16 +0100)
are available in the Git repository at:
git://github.com/XanClic/qemu.git tags/pull-block-2018-06-11
for you to fetch changes up to c50abd175a88cd41c2c08339de91f6f6e4a7b162:
iotests: Add case for a corrupted inactive image (2018-06-11 16:18:45 +0200)
----------------------------------------------------------------
Block patches:
- Various bug fixes
- Removal of qemu-img convert's deprecated -s option
- qemu-io now exits with an error when a command failed
----------------------------------------------------------------
Alberto Garcia (1):
throttle: Fix crash on reopen
Max Reitz (25):
block/file-posix: Pass FD to locking helpers
block/file-posix: File locking during creation
iotests: Add creation test to 153
qemu-img: Amendment support implies create_opts
block: Add Error parameter to bdrv_amend_options
qemu-option: Pull out "Supported options" print
qemu-img: Add print_amend_option_help()
qemu-img: Recognize no creation support in -o help
iotests: Test help option for unsupporting formats
iotests: Rework 113
qcow2: Repair OFLAG_COPIED when fixing leaks
iotests: Repairing error during snapshot deletion
qemu-io: Drop command functions' return values
qemu-io: Let command functions return error code
qemu-io: Exit with error when a command failed
iotests.py: Add qemu_io_silent
iotests: Let 216 make use of qemu-io's exit code
qemu-img: Resolve relative backing paths in rebase
iotests: Add test for rebasing with relative paths
qemu-img: Special post-backing convert handling
iotests: Test post-backing convert target behavior
iotests: Fix 219's timing
block: Make bdrv_is_writable() public
qcow2: Do not mark inactive images corrupt
iotests: Add case for a corrupted inactive image
Thomas Huth (1):
qemu-img: Remove deprecated -s snapshot_id_or_name option
Vladimir Sementsov-Ogievskiy (2):
iotests: improve pause_job
block/qcow2-bitmap: fix free_bitmap_clusters
include/block/block.h | 4 +-
include/block/block_int.h | 3 +-
include/qemu-io.h | 9 +-
block.c | 25 ++-
block/file-posix.c | 64 ++++++--
block/qcow2-bitmap.c | 1 -
block/qcow2-refcount.c | 25 ++-
block/qcow2.c | 74 +++++----
block/throttle.c | 54 ++++---
qemu-img.c | 108 +++++++++++--
qemu-io-cmds.c | 276 +++++++++++++++++++---------------
qemu-io.c | 62 +++++---
util/qemu-option.c | 1 -
qemu-doc.texi | 7 -
qemu-img-cmds.hx | 4 +-
qemu-img.texi | 7 +-
tests/qemu-iotests/024 | 82 +++++++++-
tests/qemu-iotests/024.out | 30 ++++
tests/qemu-iotests/029 | 2 +-
tests/qemu-iotests/060 | 30 ++++
tests/qemu-iotests/060.out | 18 ++-
tests/qemu-iotests/061.out | 7 -
tests/qemu-iotests/080 | 4 +-
tests/qemu-iotests/080.out | 4 +-
tests/qemu-iotests/082 | 9 ++
tests/qemu-iotests/082.out | 53 ++++---
tests/qemu-iotests/112.out | 5 +-
tests/qemu-iotests/113 | 19 ++-
tests/qemu-iotests/113.out | 7 +-
tests/qemu-iotests/122 | 42 ++++++
tests/qemu-iotests/122.out | 18 +++
tests/qemu-iotests/153 | 18 +++
tests/qemu-iotests/153.out | 13 ++
tests/qemu-iotests/216 | 23 +--
tests/qemu-iotests/216.out | 17 +--
tests/qemu-iotests/217 | 90 +++++++++++
tests/qemu-iotests/217.out | 42 ++++++
tests/qemu-iotests/219 | 26 +++-
tests/qemu-iotests/219.out | 10 +-
tests/qemu-iotests/group | 1 +
tests/qemu-iotests/iotests.py | 18 ++-
41 files changed, 970 insertions(+), 342 deletions(-)
create mode 100755 tests/qemu-iotests/217
create mode 100644 tests/qemu-iotests/217.out
--
2.17.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 01/29] block/file-posix: Pass FD to locking helpers
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 02/29] block/file-posix: File locking during creation Max Reitz
` (28 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
raw_apply_lock_bytes() and raw_check_lock_bytes() currently take a
BDRVRawState *, but they only use the lock_fd field. During image
creation, we do not have a BDRVRawState, but we do have an FD; so if we
want to reuse the functions there, we should modify them to receive only
the FD.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180509215336.31304-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 27 ++++++++++++++-------------
1 file changed, 14 insertions(+), 13 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 513d371bb1..7583bbfbb3 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -643,7 +643,7 @@ typedef enum {
* file; if @unlock == true, also unlock the unneeded bytes.
* @shared_perm_lock_bits is the mask of all permissions that are NOT shared.
*/
-static int raw_apply_lock_bytes(BDRVRawState *s,
+static int raw_apply_lock_bytes(int fd,
uint64_t perm_lock_bits,
uint64_t shared_perm_lock_bits,
bool unlock, Error **errp)
@@ -654,13 +654,13 @@ static int raw_apply_lock_bytes(BDRVRawState *s,
PERM_FOREACH(i) {
int off = RAW_LOCK_PERM_BASE + i;
if (perm_lock_bits & (1ULL << i)) {
- ret = qemu_lock_fd(s->lock_fd, off, 1, false);
+ ret = qemu_lock_fd(fd, off, 1, false);
if (ret) {
error_setg(errp, "Failed to lock byte %d", off);
return ret;
}
} else if (unlock) {
- ret = qemu_unlock_fd(s->lock_fd, off, 1);
+ ret = qemu_unlock_fd(fd, off, 1);
if (ret) {
error_setg(errp, "Failed to unlock byte %d", off);
return ret;
@@ -670,13 +670,13 @@ static int raw_apply_lock_bytes(BDRVRawState *s,
PERM_FOREACH(i) {
int off = RAW_LOCK_SHARED_BASE + i;
if (shared_perm_lock_bits & (1ULL << i)) {
- ret = qemu_lock_fd(s->lock_fd, off, 1, false);
+ ret = qemu_lock_fd(fd, off, 1, false);
if (ret) {
error_setg(errp, "Failed to lock byte %d", off);
return ret;
}
} else if (unlock) {
- ret = qemu_unlock_fd(s->lock_fd, off, 1);
+ ret = qemu_unlock_fd(fd, off, 1);
if (ret) {
error_setg(errp, "Failed to unlock byte %d", off);
return ret;
@@ -687,8 +687,7 @@ static int raw_apply_lock_bytes(BDRVRawState *s,
}
/* Check "unshared" bytes implied by @perm and ~@shared_perm in the file. */
-static int raw_check_lock_bytes(BDRVRawState *s,
- uint64_t perm, uint64_t shared_perm,
+static int raw_check_lock_bytes(int fd, uint64_t perm, uint64_t shared_perm,
Error **errp)
{
int ret;
@@ -698,7 +697,7 @@ static int raw_check_lock_bytes(BDRVRawState *s,
int off = RAW_LOCK_SHARED_BASE + i;
uint64_t p = 1ULL << i;
if (perm & p) {
- ret = qemu_lock_fd_test(s->lock_fd, off, 1, true);
+ ret = qemu_lock_fd_test(fd, off, 1, true);
if (ret) {
char *perm_name = bdrv_perm_names(p);
error_setg(errp,
@@ -715,7 +714,7 @@ static int raw_check_lock_bytes(BDRVRawState *s,
int off = RAW_LOCK_PERM_BASE + i;
uint64_t p = 1ULL << i;
if (!(shared_perm & p)) {
- ret = qemu_lock_fd_test(s->lock_fd, off, 1, true);
+ ret = qemu_lock_fd_test(fd, off, 1, true);
if (ret) {
char *perm_name = bdrv_perm_names(p);
error_setg(errp,
@@ -752,11 +751,11 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
switch (op) {
case RAW_PL_PREPARE:
- ret = raw_apply_lock_bytes(s, s->perm | new_perm,
+ ret = raw_apply_lock_bytes(s->lock_fd, s->perm | new_perm,
~s->shared_perm | ~new_shared,
false, errp);
if (!ret) {
- ret = raw_check_lock_bytes(s, new_perm, new_shared, errp);
+ ret = raw_check_lock_bytes(s->lock_fd, new_perm, new_shared, errp);
if (!ret) {
return 0;
}
@@ -764,7 +763,8 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
op = RAW_PL_ABORT;
/* fall through to unlock bytes. */
case RAW_PL_ABORT:
- raw_apply_lock_bytes(s, s->perm, ~s->shared_perm, true, &local_err);
+ raw_apply_lock_bytes(s->lock_fd, s->perm, ~s->shared_perm,
+ true, &local_err);
if (local_err) {
/* Theoretically the above call only unlocks bytes and it cannot
* fail. Something weird happened, report it.
@@ -773,7 +773,8 @@ static int raw_handle_perm_lock(BlockDriverState *bs,
}
break;
case RAW_PL_COMMIT:
- raw_apply_lock_bytes(s, new_perm, ~new_shared, true, &local_err);
+ raw_apply_lock_bytes(s->lock_fd, new_perm, ~new_shared,
+ true, &local_err);
if (local_err) {
/* Theoretically the above call only unlocks bytes and it cannot
* fail. Something weird happened, report it.
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 02/29] block/file-posix: File locking during creation
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 01/29] block/file-posix: Pass FD to locking helpers Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 03/29] iotests: Add creation test to 153 Max Reitz
` (27 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
When creating a file, we should take the WRITE and RESIZE permissions.
We do not need either for the creation itself, but we do need them for
clearing and resizing it. So we can take the proper permissions by
replacing O_TRUNC with an explicit truncation to 0, and by taking the
appropriate file locks between those two steps.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180509215336.31304-3-mreitz@redhat.com
Reviewed-by: Fam Zheng <famz@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 37 +++++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 7583bbfbb3..07bb061fe4 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2076,6 +2076,7 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
{
BlockdevCreateOptionsFile *file_opts;
int fd;
+ int perm, shared;
int result = 0;
/* Validate options and set default values */
@@ -2090,14 +2091,44 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
}
/* Create file */
- fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY,
- 0644);
+ fd = qemu_open(file_opts->filename, O_RDWR | O_CREAT | O_BINARY, 0644);
if (fd < 0) {
result = -errno;
error_setg_errno(errp, -result, "Could not create file");
goto out;
}
+ /* Take permissions: We want to discard everything, so we need
+ * BLK_PERM_WRITE; and truncation to the desired size requires
+ * BLK_PERM_RESIZE.
+ * On the other hand, we cannot share the RESIZE permission
+ * because we promise that after this function, the file has the
+ * size given in the options. If someone else were to resize it
+ * concurrently, we could not guarantee that.
+ * Note that after this function, we can no longer guarantee that
+ * the file is not touched by a third party, so it may be resized
+ * then. */
+ perm = BLK_PERM_WRITE | BLK_PERM_RESIZE;
+ shared = BLK_PERM_ALL & ~BLK_PERM_RESIZE;
+
+ /* Step one: Take locks */
+ result = raw_apply_lock_bytes(fd, perm, shared, false, errp);
+ if (result < 0) {
+ goto out_close;
+ }
+
+ /* Step two: Check that nobody else has taken conflicting locks */
+ result = raw_check_lock_bytes(fd, perm, shared, errp);
+ if (result < 0) {
+ goto out_close;
+ }
+
+ /* Clear the file by truncating it to 0 */
+ result = raw_regular_truncate(fd, 0, PREALLOC_MODE_OFF, errp);
+ if (result < 0) {
+ goto out_close;
+ }
+
if (file_opts->nocow) {
#ifdef __linux__
/* Set NOCOW flag to solve performance issue on fs like btrfs.
@@ -2113,6 +2144,8 @@ static int raw_co_create(BlockdevCreateOptions *options, Error **errp)
#endif
}
+ /* Resize and potentially preallocate the file to the desired
+ * final size */
result = raw_regular_truncate(fd, file_opts->size, file_opts->preallocation,
errp);
if (result < 0) {
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 03/29] iotests: Add creation test to 153
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 01/29] block/file-posix: Pass FD to locking helpers Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 02/29] block/file-posix: File locking during creation Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 04/29] qemu-img: Amendment support implies create_opts Max Reitz
` (26 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This patch adds a test case to 153 which tries to overwrite an image
(using qemu-img create) while it is in use. Without the original user
explicitly sharing the necessary permissions (writing and truncation),
this should not be allowed.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20180509215336.31304-4-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/153 | 18 ++++++++++++++++++
tests/qemu-iotests/153.out | 13 +++++++++++++
2 files changed, 31 insertions(+)
diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153
index ec508c758f..673813cd39 100755
--- a/tests/qemu-iotests/153
+++ b/tests/qemu-iotests/153
@@ -137,6 +137,24 @@ for opts1 in "" "read-only=on" "read-only=on,force-share=on"; do
_run_cmd $QEMU_IMG dd $L if="${TEST_IMG}" of="${TEST_IMG}.convert" bs=512 count=1
_run_cmd $QEMU_IMG bench $L -c 1 "${TEST_IMG}"
_run_cmd $QEMU_IMG bench $L -w -c 1 "${TEST_IMG}"
+
+ # qemu-img create does not support -U
+ if [ -z "$L" ]; then
+ _run_cmd $QEMU_IMG create -f $IMGFMT "${TEST_IMG}" \
+ -b ${TEST_IMG}.base
+ # Read the file format. It used to be the case that
+ # file-posix simply truncated the file, but the qcow2
+ # driver then failed to format it because it was unable
+ # to acquire the necessary WRITE permission. However, the
+ # truncation was already wrong, and the whole process
+ # resulted in the file being completely empty and thus its
+ # format would be detected to be raw.
+ # So we read it here to see that creation either completed
+ # successfully (thus the format is qcow2) or it aborted
+ # before the file was changed at all (thus the format stays
+ # qcow2).
+ _img_info -U | grep 'file format'
+ fi
done
_send_qemu_cmd $h "{ 'execute': 'quit', }" ""
echo
diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out
index 2510762ba1..3492ba7a29 100644
--- a/tests/qemu-iotests/153.out
+++ b/tests/qemu-iotests/153.out
@@ -92,6 +92,11 @@ _qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock
Is another process using the image?
+_qemu_img_wrapper create -f qcow2 TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: TEST_DIR/t.qcow2: Failed to get "write" lock
+Is another process using the image?
+file format: IMGFMT
+
== Running utility commands -U ==
_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
@@ -209,6 +214,11 @@ _qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
qemu-img: Could not open 'TEST_DIR/t.qcow2': Failed to get "write" lock
Is another process using the image?
+_qemu_img_wrapper create -f qcow2 TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+qemu-img: TEST_DIR/t.qcow2: Failed to get "write" lock
+Is another process using the image?
+file format: IMGFMT
+
== Running utility commands -U ==
_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
@@ -309,6 +319,9 @@ _qemu_img_wrapper bench -c 1 TEST_DIR/t.qcow2
_qemu_img_wrapper bench -w -c 1 TEST_DIR/t.qcow2
+_qemu_img_wrapper create -f qcow2 TEST_DIR/t.qcow2 -b TEST_DIR/t.qcow2.base
+file format: IMGFMT
+
== Running utility commands -U ==
_qemu_io_wrapper -U -c read 0 512 TEST_DIR/t.qcow2
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 04/29] qemu-img: Amendment support implies create_opts
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (2 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 03/29] iotests: Add creation test to 153 Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 05/29] block: Add Error parameter to bdrv_amend_options Max Reitz
` (25 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Instead of checking whether a driver has a non-NULL create_opts we
should check whether it supports image amendment in the first place. If
it does, it must have create_opts.
On the other hand, if it does not have create_opts (so it does not
support amendment either), the error message "does not support any
options" is a bit useless. Stating clearly that the driver has no
amendment support whatsoever is probably better.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509210023.20283-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 75f1610aa0..0be11b3852 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3789,13 +3789,16 @@ static int img_amend(int argc, char **argv)
goto out;
}
- if (!bs->drv->create_opts) {
- error_report("Format driver '%s' does not support any options to amend",
+ if (!bs->drv->bdrv_amend_options) {
+ error_report("Format driver '%s' does not support option amendment",
fmt);
ret = -1;
goto out;
}
+ /* Every driver supporting amendment must have create_opts */
+ assert(bs->drv->create_opts);
+
create_opts = qemu_opts_append(create_opts, bs->drv->create_opts);
opts = qemu_opts_create(create_opts, NULL, 0, &error_abort);
qemu_opts_do_parse(opts, options, NULL, &err);
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 05/29] block: Add Error parameter to bdrv_amend_options
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (3 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 04/29] qemu-img: Amendment support implies create_opts Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 06/29] qemu-option: Pull out "Supported options" print Max Reitz
` (24 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Looking at the qcow2 code that is riddled with error_report() calls,
this is really how it should have been from the start.
Along the way, turn the target_version/current_version comparisons at
the beginning of qcow2_downgrade() into assertions (the caller has to
make sure these conditions are met), and rephrase the error message on
using compat=1.1 to get refcount widths other than 16 bits.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180509210023.20283-3-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block.h | 3 +-
include/block/block_int.h | 3 +-
block.c | 8 +++--
block/qcow2.c | 72 ++++++++++++++++++++++----------------
qemu-img.c | 4 +--
tests/qemu-iotests/060.out | 4 +--
tests/qemu-iotests/061.out | 7 ----
tests/qemu-iotests/080.out | 4 +--
tests/qemu-iotests/112.out | 5 +--
9 files changed, 58 insertions(+), 52 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 6cc6c7e699..4dd4f1eab2 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -343,7 +343,8 @@ int bdrv_check(BlockDriverState *bs, BdrvCheckResult *res, BdrvCheckMode fix);
typedef void BlockDriverAmendStatusCB(BlockDriverState *bs, int64_t offset,
int64_t total_work_size, void *opaque);
int bdrv_amend_options(BlockDriverState *bs_new, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb, void *cb_opaque);
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+ Error **errp);
/* external snapshots */
bool bdrv_recurse_is_first_non_filter(BlockDriverState *bs,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 888b7f7bff..327e478a73 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -353,7 +353,8 @@ struct BlockDriver {
int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
- void *cb_opaque);
+ void *cb_opaque,
+ Error **errp);
void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
diff --git a/block.c b/block.c
index 501b64c819..9d577f65bb 100644
--- a/block.c
+++ b/block.c
@@ -4996,15 +4996,19 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs,
}
int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts,
- BlockDriverAmendStatusCB *status_cb, void *cb_opaque)
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+ Error **errp)
{
if (!bs->drv) {
+ error_setg(errp, "Node is ejected");
return -ENOMEDIUM;
}
if (!bs->drv->bdrv_amend_options) {
+ error_setg(errp, "Block driver '%s' does not support option amendment",
+ bs->drv->format_name);
return -ENOTSUP;
}
- return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque);
+ return bs->drv->bdrv_amend_options(bs, opts, status_cb, cb_opaque, errp);
}
/* This function will be called by the bdrv_recurse_is_first_non_filter method
diff --git a/block/qcow2.c b/block/qcow2.c
index 549fee9b69..6b2d88759d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4215,22 +4215,21 @@ static int qcow2_load_vmstate(BlockDriverState *bs, QEMUIOVector *qiov,
* have to be removed.
*/
static int qcow2_downgrade(BlockDriverState *bs, int target_version,
- BlockDriverAmendStatusCB *status_cb, void *cb_opaque)
+ BlockDriverAmendStatusCB *status_cb, void *cb_opaque,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int current_version = s->qcow_version;
int ret;
- if (target_version == current_version) {
- return 0;
- } else if (target_version > current_version) {
- return -EINVAL;
- } else if (target_version != 2) {
- return -EINVAL;
- }
+ /* This is qcow2_downgrade(), not qcow2_upgrade() */
+ assert(target_version < current_version);
+
+ /* There are no other versions (now) that you can downgrade to */
+ assert(target_version == 2);
if (s->refcount_order != 4) {
- error_report("compat=0.10 requires refcount_bits=16");
+ error_setg(errp, "compat=0.10 requires refcount_bits=16");
return -ENOTSUP;
}
@@ -4238,6 +4237,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
if (s->incompatible_features & QCOW2_INCOMPAT_DIRTY) {
ret = qcow2_mark_clean(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to make the image clean");
return ret;
}
}
@@ -4247,6 +4247,8 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
* best thing to do anyway */
if (s->incompatible_features) {
+ error_setg(errp, "Cannot downgrade an image with incompatible features "
+ "%#" PRIx64 " set", s->incompatible_features);
return -ENOTSUP;
}
@@ -4260,6 +4262,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
ret = qcow2_expand_zero_clusters(bs, status_cb, cb_opaque);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to turn zero into data clusters");
return ret;
}
@@ -4267,6 +4270,7 @@ static int qcow2_downgrade(BlockDriverState *bs, int target_version,
ret = qcow2_update_header(bs);
if (ret < 0) {
s->qcow_version = current_version;
+ error_setg_errno(errp, -ret, "Failed to update the image header");
return ret;
}
return 0;
@@ -4344,7 +4348,8 @@ static void qcow2_amend_helper_cb(BlockDriverState *bs,
static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
BlockDriverAmendStatusCB *status_cb,
- void *cb_opaque)
+ void *cb_opaque,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int old_version = s->qcow_version, new_version = old_version;
@@ -4356,7 +4361,6 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
bool encrypt;
int encformat;
int refcount_bits = s->refcount_bits;
- Error *local_err = NULL;
int ret;
QemuOptDesc *desc = opts->list->desc;
Qcow2AmendHelperCBInfo helper_cb_info;
@@ -4377,11 +4381,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
} else if (!strcmp(compat, "1.1")) {
new_version = 3;
} else {
- error_report("Unknown compatibility level %s", compat);
+ error_setg(errp, "Unknown compatibility level %s", compat);
return -EINVAL;
}
} else if (!strcmp(desc->name, BLOCK_OPT_PREALLOC)) {
- error_report("Cannot change preallocation mode");
+ error_setg(errp, "Cannot change preallocation mode");
return -ENOTSUP;
} else if (!strcmp(desc->name, BLOCK_OPT_SIZE)) {
new_size = qemu_opt_get_size(opts, BLOCK_OPT_SIZE, 0);
@@ -4394,7 +4398,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
!!s->crypto);
if (encrypt != !!s->crypto) {
- error_report("Changing the encryption flag is not supported");
+ error_setg(errp,
+ "Changing the encryption flag is not supported");
return -ENOTSUP;
}
} else if (!strcmp(desc->name, BLOCK_OPT_ENCRYPT_FORMAT)) {
@@ -4402,17 +4407,19 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
qemu_opt_get(opts, BLOCK_OPT_ENCRYPT_FORMAT));
if (encformat != s->crypt_method_header) {
- error_report("Changing the encryption format is not supported");
+ error_setg(errp,
+ "Changing the encryption format is not supported");
return -ENOTSUP;
}
} else if (g_str_has_prefix(desc->name, "encrypt.")) {
- error_report("Changing the encryption parameters is not supported");
+ error_setg(errp,
+ "Changing the encryption parameters is not supported");
return -ENOTSUP;
} else if (!strcmp(desc->name, BLOCK_OPT_CLUSTER_SIZE)) {
cluster_size = qemu_opt_get_size(opts, BLOCK_OPT_CLUSTER_SIZE,
cluster_size);
if (cluster_size != s->cluster_size) {
- error_report("Changing the cluster size is not supported");
+ error_setg(errp, "Changing the cluster size is not supported");
return -ENOTSUP;
}
} else if (!strcmp(desc->name, BLOCK_OPT_LAZY_REFCOUNTS)) {
@@ -4425,8 +4432,8 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (refcount_bits <= 0 || refcount_bits > 64 ||
!is_power_of_2(refcount_bits))
{
- error_report("Refcount width must be a power of two and may "
- "not exceed 64 bits");
+ error_setg(errp, "Refcount width must be a power of two and "
+ "may not exceed 64 bits");
return -EINVAL;
}
} else {
@@ -4451,6 +4458,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
ret = qcow2_update_header(bs);
if (ret < 0) {
s->qcow_version = old_version;
+ error_setg_errno(errp, -ret, "Failed to update the image header");
return ret;
}
}
@@ -4459,18 +4467,17 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
int refcount_order = ctz32(refcount_bits);
if (new_version < 3 && refcount_bits != 16) {
- error_report("Different refcount widths than 16 bits require "
- "compatibility level 1.1 or above (use compat=1.1 or "
- "greater)");
+ error_setg(errp, "Refcount widths other than 16 bits require "
+ "compatibility level 1.1 or above (use compat=1.1 or "
+ "greater)");
return -EINVAL;
}
helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER;
ret = qcow2_change_refcount_order(bs, refcount_order,
&qcow2_amend_helper_cb,
- &helper_cb_info, &local_err);
+ &helper_cb_info, errp);
if (ret < 0) {
- error_report_err(local_err);
return ret;
}
}
@@ -4480,6 +4487,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
backing_file ?: s->image_backing_file,
backing_format ?: s->image_backing_format);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to change the backing file");
return ret;
}
}
@@ -4487,14 +4495,16 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (s->use_lazy_refcounts != lazy_refcounts) {
if (lazy_refcounts) {
if (new_version < 3) {
- error_report("Lazy refcounts only supported with compatibility "
- "level 1.1 and above (use compat=1.1 or greater)");
+ error_setg(errp, "Lazy refcounts only supported with "
+ "compatibility level 1.1 and above (use compat=1.1 "
+ "or greater)");
return -EINVAL;
}
s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
ret = qcow2_update_header(bs);
if (ret < 0) {
s->compatible_features &= ~QCOW2_COMPAT_LAZY_REFCOUNTS;
+ error_setg_errno(errp, -ret, "Failed to update the image header");
return ret;
}
s->use_lazy_refcounts = true;
@@ -4502,6 +4512,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
/* make image clean first */
ret = qcow2_mark_clean(bs);
if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to make the image clean");
return ret;
}
/* now disallow lazy refcounts */
@@ -4509,6 +4520,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
ret = qcow2_update_header(bs);
if (ret < 0) {
s->compatible_features |= QCOW2_COMPAT_LAZY_REFCOUNTS;
+ error_setg_errno(errp, -ret, "Failed to update the image header");
return ret;
}
s->use_lazy_refcounts = false;
@@ -4517,17 +4529,15 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (new_size) {
BlockBackend *blk = blk_new(BLK_PERM_RESIZE, BLK_PERM_ALL);
- ret = blk_insert_bs(blk, bs, &local_err);
+ ret = blk_insert_bs(blk, bs, errp);
if (ret < 0) {
- error_report_err(local_err);
blk_unref(blk);
return ret;
}
- ret = blk_truncate(blk, new_size, PREALLOC_MODE_OFF, &local_err);
+ ret = blk_truncate(blk, new_size, PREALLOC_MODE_OFF, errp);
blk_unref(blk);
if (ret < 0) {
- error_report_err(local_err);
return ret;
}
}
@@ -4536,7 +4546,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
if (new_version < old_version) {
helper_cb_info.current_operation = QCOW2_DOWNGRADING;
ret = qcow2_downgrade(bs, new_version, &qcow2_amend_helper_cb,
- &helper_cb_info);
+ &helper_cb_info, errp);
if (ret < 0) {
return ret;
}
diff --git a/qemu-img.c b/qemu-img.c
index 0be11b3852..df3ec5b7fe 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3810,10 +3810,10 @@ static int img_amend(int argc, char **argv)
/* In case the driver does not call amend_status_cb() */
qemu_progress_print(0.f, 0);
- ret = bdrv_amend_options(bs, opts, &amend_status_cb, NULL);
+ ret = bdrv_amend_options(bs, opts, &amend_status_cb, NULL, &err);
qemu_progress_print(100.f, 0);
if (ret < 0) {
- error_report("Error while amending options: %s", strerror(-ret));
+ error_report_err(err);
goto out;
}
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 25d5c3938b..5f4264cff6 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -129,7 +129,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qcow2: Marking image as corrupt: L2 table offset 0x42a00 unaligned (L1 index: 0); further corruption events will be suppressed
-qemu-img: Error while amending options: Input/output error
+qemu-img: Failed to turn zero into data clusters: Input/output error
=== Testing unaligned L2 entry ===
@@ -145,7 +145,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
wrote 65536/65536 bytes at offset 0
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed
-qemu-img: Error while amending options: Input/output error
+qemu-img: Failed to turn zero into data clusters: Input/output error
=== Testing unaligned reftable entry ===
diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out
index e857ef9a7d..183f7dd690 100644
--- a/tests/qemu-iotests/061.out
+++ b/tests/qemu-iotests/061.out
@@ -358,18 +358,12 @@ No errors were found on the image.
Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
-qemu-img: Error while amending options: Invalid argument
qemu-img: Lazy refcounts only supported with compatibility level 1.1 and above (use compat=1.1 or greater)
-qemu-img: Error while amending options: Invalid argument
qemu-img: Unknown compatibility level 0.42
-qemu-img: Error while amending options: Invalid argument
qemu-img: Invalid parameter 'foo'
qemu-img: Changing the cluster size is not supported
-qemu-img: Error while amending options: Operation not supported
qemu-img: Changing the encryption flag is not supported
-qemu-img: Error while amending options: Operation not supported
qemu-img: Cannot change preallocation mode
-qemu-img: Error while amending options: Operation not supported
=== Testing correct handling of unset value ===
@@ -377,7 +371,6 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
Should work:
Should not work:
qemu-img: Changing the cluster size is not supported
-qemu-img: Error while amending options: Operation not supported
=== Testing zero expansion on inactive clusters ===
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 4e0f7f7b92..281c7e0d1d 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -65,7 +65,7 @@ wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: Failed to load snapshot: Snapshot L1 table offset invalid
qemu-img: Snapshot L1 table offset invalid
-qemu-img: Error while amending options: Invalid argument
+qemu-img: Failed to turn zero into data clusters: Invalid argument
Failed to flush the refcount block cache: Invalid argument
write failed: Invalid argument
qemu-img: Snapshot L1 table offset invalid
@@ -88,7 +88,7 @@ wrote 512/512 bytes at offset 0
512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: Failed to load snapshot: Snapshot L1 table too large
qemu-img: Snapshot L1 table too large
-qemu-img: Error while amending options: File too large
+qemu-img: Failed to turn zero into data clusters: File too large
Failed to flush the refcount block cache: File too large
write failed: File too large
qemu-img: Snapshot L1 table too large
diff --git a/tests/qemu-iotests/112.out b/tests/qemu-iotests/112.out
index 86f041075d..ae0318cabe 100644
--- a/tests/qemu-iotests/112.out
+++ b/tests/qemu-iotests/112.out
@@ -99,13 +99,11 @@ refcount bits: 64
=== Amend to compat=0.10 ===
qemu-img: compat=0.10 requires refcount_bits=16
-qemu-img: Error while amending options: Operation not supported
refcount bits: 64
No errors were found on the image.
refcount bits: 16
refcount bits: 16
-qemu-img: Different refcount widths than 16 bits require compatibility level 1.1 or above (use compat=1.1 or greater)
-qemu-img: Error while amending options: Invalid argument
+qemu-img: Refcount widths other than 16 bits require compatibility level 1.1 or above (use compat=1.1 or greater)
refcount bits: 16
=== Amend with snapshot ===
@@ -113,7 +111,6 @@ refcount bits: 16
wrote 16777216/16777216 bytes at offset 0
16 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
qemu-img: Cannot decrease refcount entry width to 1 bits: Cluster at offset 0x50000 has a refcount of 2
-qemu-img: Error while amending options: Invalid argument
No errors were found on the image.
refcount bits: 16
No errors were found on the image.
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 06/29] qemu-option: Pull out "Supported options" print
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (4 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 05/29] block: Add Error parameter to bdrv_amend_options Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 07/29] qemu-img: Add print_amend_option_help() Max Reitz
` (23 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
It really is up to the caller to decide what this list of options means.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509210023.20283-4-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 1 +
util/qemu-option.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index df3ec5b7fe..52008c5647 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -260,6 +260,7 @@ static int print_block_option_help(const char *filename, const char *fmt)
create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
}
+ printf("Supported options:\n");
qemu_opts_print_help(create_opts);
qemu_opts_free(create_opts);
return 0;
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 58d1c23893..ba44a0895c 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -220,7 +220,6 @@ void qemu_opts_print_help(QemuOptsList *list)
assert(list);
desc = list->desc;
- printf("Supported options:\n");
while (desc && desc->name) {
printf("%-16s %s\n", desc->name,
desc->help ? desc->help : "No description available");
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 07/29] qemu-img: Add print_amend_option_help()
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (5 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 06/29] qemu-option: Pull out "Supported options" print Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 08/29] qemu-img: Recognize no creation support in -o help Max Reitz
` (22 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
The more generic print_block_option_help() function is not really
suitable for qemu-img amend, for a couple of reasons:
(1) We do not need to append the protocol-level options, as amendment
happens only on one node and does not descend downwards to its
children.
(2) print_block_option_help() says those options are "supported". For
option amendment, we do not really know that. So this new function
explicitly says that those options are the creation options, and not
all of them may be supported.
(3) If the driver does not support option amendment, we should not print
anything (except for an error message that amendment is not
supported).
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1537956
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509210023.20283-5-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 30 ++++++++++++++++++++++++--
tests/qemu-iotests/082.out | 44 +++++++++++++++++++++++---------------
2 files changed, 55 insertions(+), 19 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 52008c5647..07935cb232 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3658,6 +3658,32 @@ static void amend_status_cb(BlockDriverState *bs,
qemu_progress_print(100.f * offset / total_work_size, 0);
}
+static int print_amend_option_help(const char *format)
+{
+ BlockDriver *drv;
+
+ /* Find driver and parse its options */
+ drv = bdrv_find_format(format);
+ if (!drv) {
+ error_report("Unknown file format '%s'", format);
+ return 1;
+ }
+
+ if (!drv->bdrv_amend_options) {
+ error_report("Format driver '%s' does not support option amendment",
+ format);
+ return 1;
+ }
+
+ /* Every driver supporting amendment must have create_opts */
+ assert(drv->create_opts);
+
+ printf("Creation options for '%s':\n", format);
+ qemu_opts_print_help(drv->create_opts);
+ printf("\nNote that not all of these options may be amendable.\n");
+ return 0;
+}
+
static int img_amend(int argc, char **argv)
{
Error *err = NULL;
@@ -3757,7 +3783,7 @@ static int img_amend(int argc, char **argv)
if (fmt && has_help_option(options)) {
/* If a format is explicitly specified (and possibly no filename is
* given), print option help here */
- ret = print_block_option_help(filename, fmt);
+ ret = print_amend_option_help(fmt);
goto out;
}
@@ -3786,7 +3812,7 @@ static int img_amend(int argc, char **argv)
if (has_help_option(options)) {
/* If the format was auto-detected, print option help here */
- ret = print_block_option_help(filename, fmt);
+ ret = print_amend_option_help(fmt);
goto out;
}
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 1527fbe1b7..4e52dce8d6 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -546,7 +546,7 @@ cluster_size: 65536
=== amend: help for -o ===
Testing: amend -f qcow2 -o help TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -564,10 +564,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o ? TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -585,10 +586,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o cluster_size=4k,help TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -606,10 +608,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o cluster_size=4k,? TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -627,10 +630,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o help,cluster_size=4k TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -648,10 +652,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o ?,cluster_size=4k TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -669,10 +674,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o cluster_size=4k -o help TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -690,10 +696,11 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o cluster_size=4k -o ? TEST_DIR/t.qcow2
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -711,7 +718,8 @@ cluster_size qcow2 cluster size
preallocation Preallocation mode (allowed values: off, metadata, falloc, full)
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
-nocow Turn off copy-on-write (valid only on btrfs)
+
+Note that not all of these options may be amendable.
Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2
@@ -731,7 +739,7 @@ Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2 -o ,, -o help TEST_DIR/
qemu-img: Invalid option list: ,,
Testing: amend -f qcow2 -o help
-Supported options:
+Creation options for 'qcow2':
size Virtual disk size
compat Compatibility level (0.10 or 1.1)
backing_file File name of a base image
@@ -750,6 +758,8 @@ preallocation Preallocation mode (allowed values: off, metadata, falloc, full
lazy_refcounts Postpone refcount updates
refcount_bits Width of a reference count entry in bits
+Note that not all of these options may be amendable.
+
Testing: convert -o help
Supported options:
size Virtual disk size
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 08/29] qemu-img: Recognize no creation support in -o help
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (6 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 07/29] qemu-img: Add print_amend_option_help() Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 09/29] iotests: Test help option for unsupporting formats Max Reitz
` (21 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
The only users of print_block_option_help() are qemu-img create and
qemu-img convert for the output image, so this function is always used
for image creation (it used to be used for amendment also, but that is
no longer the case).
So if image creation is not supported by either the format or the
protocol, there is no need to print any option description, because the
user cannot create an image like this anyway.
This also fixes an assertion failure:
$ qemu-img create -f bochs -o help
Supported options:
qemu-img: util/qemu-option.c:219:
qemu_opts_print_help: Assertion `list' failed.
[1] 24831 abort (core dumped) qemu-img create -f bochs -o help
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509210023.20283-6-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/qemu-img.c b/qemu-img.c
index 07935cb232..b1c1e484d2 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -249,6 +249,11 @@ static int print_block_option_help(const char *filename, const char *fmt)
return 1;
}
+ if (!drv->create_opts) {
+ error_report("Format driver '%s' does not support image creation", fmt);
+ return 1;
+ }
+
create_opts = qemu_opts_append(create_opts, drv->create_opts);
if (filename) {
proto_drv = bdrv_find_protocol(filename, true, &local_err);
@@ -257,6 +262,11 @@ static int print_block_option_help(const char *filename, const char *fmt)
qemu_opts_free(create_opts);
return 1;
}
+ if (!proto_drv->create_opts) {
+ error_report("Protocal driver '%s' does not support image creation",
+ proto_drv->format_name);
+ return 1;
+ }
create_opts = qemu_opts_append(create_opts, proto_drv->create_opts);
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 09/29] iotests: Test help option for unsupporting formats
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (7 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 08/29] qemu-img: Recognize no creation support in -o help Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 10/29] iotests: Rework 113 Max Reitz
` (20 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This adds test cases to 082 for qemu-img create/convert/amend "-o help"
on formats that do not support creation or amendment, respectively.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509210023.20283-7-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/082 | 9 +++++++++
tests/qemu-iotests/082.out | 9 +++++++++
2 files changed, 18 insertions(+)
diff --git a/tests/qemu-iotests/082 b/tests/qemu-iotests/082
index d5c83d45ed..a872f771a6 100755
--- a/tests/qemu-iotests/082
+++ b/tests/qemu-iotests/082
@@ -97,6 +97,9 @@ run_qemu_img create -f $IMGFMT -o backing_file="$TEST_IMG" -o ,, -o help "$TEST_
run_qemu_img create -f $IMGFMT -o help
run_qemu_img create -o help
+# Try help option for a format that does not support creation
+run_qemu_img create -f bochs -o help
+
echo
echo === convert: Options specified more than once ===
@@ -151,6 +154,9 @@ run_qemu_img convert -O $IMGFMT -o backing_file="$TEST_IMG" -o ,, -o help "$TEST
run_qemu_img convert -O $IMGFMT -o help
run_qemu_img convert -o help
+# Try help option for a format that does not support creation
+run_qemu_img convert -O bochs -o help
+
echo
echo === amend: Options specified more than once ===
@@ -201,6 +207,9 @@ run_qemu_img amend -f $IMGFMT -o backing_file="$TEST_IMG" -o ,, -o help "$TEST_I
run_qemu_img amend -f $IMGFMT -o help
run_qemu_img convert -o help
+# Try help option for a format that does not support amendment
+run_qemu_img amend -f bochs -o help
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out
index 4e52dce8d6..60ef87c276 100644
--- a/tests/qemu-iotests/082.out
+++ b/tests/qemu-iotests/082.out
@@ -249,6 +249,9 @@ Testing: create -o help
Supported options:
size Virtual disk size
+Testing: create -f bochs -o help
+qemu-img: Format driver 'bochs' does not support image creation
+
=== convert: Options specified more than once ===
Testing: create -f qcow2 TEST_DIR/t.qcow2 128M
@@ -502,6 +505,9 @@ Testing: convert -o help
Supported options:
size Virtual disk size
+Testing: convert -O bochs -o help
+qemu-img: Format driver 'bochs' does not support image creation
+
=== amend: Options specified more than once ===
Testing: amend -f foo -f qcow2 -o lazy_refcounts=on TEST_DIR/t.qcow2
@@ -763,4 +769,7 @@ Note that not all of these options may be amendable.
Testing: convert -o help
Supported options:
size Virtual disk size
+
+Testing: amend -f bochs -o help
+qemu-img: Format driver 'bochs' does not support option amendment
*** done
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 10/29] iotests: Rework 113
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (8 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 09/29] iotests: Test help option for unsupporting formats Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 11/29] qcow2: Repair OFLAG_COPIED when fixing leaks Max Reitz
` (19 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This test case has been broken since 398e6ad014df261d (roughly half a
year). qemu-img amend requires its output image to be R/W, so it opens
it as such; the node is then turned into an read-only node automatically
which is now accompanied by a warning, however. This warning has not
been part of the reference output.
For one thing, this warning shows that we cannot keep the test case as
it is. We would need a format that has no create_opts but that does
have write support -- we do not have such a format, though.
Another thing is that qemu now actually checks whether an image format
supports amendment instead of whether it has create_opts (since the
former always implies the latter). So we can now use any format that
does not support amendment (even if it supports creation) and thus test
the same code path.
The reason nobody has noticed the breakage until now of course is the
fact that nobody runs the iotests for nbd+bochs. There actually was
never any reason to set the protocol to "nbd" but because that was
technically correct; functionally it made no difference. So that is the
first thing we are going to change: Make the protocol "file" instead so
that people might actually notice breakage here.
Secondly, now that bochs no longer works for the amend test case, we
have to change the format there anyway. Set let us just bend the truth
a bit, declare this test a raw test. In fact, that does not even
concern the bochs test cases, other than the output now reading 'bochs'
instead of 'IMGFMT'.
So with this test now being a raw test, we can rework the amend test
case to use raw instead.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 20180509210023.20283-8-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/113 | 19 +++++++++----------
tests/qemu-iotests/113.out | 7 ++++---
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/tests/qemu-iotests/113 b/tests/qemu-iotests/113
index 19b68b2727..4e09810905 100755
--- a/tests/qemu-iotests/113
+++ b/tests/qemu-iotests/113
@@ -38,16 +38,17 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
. ./common.rc
. ./common.filter
-# We can only test one format here because we need its sample file
-_supported_fmt bochs
-_supported_proto nbd
+# Some of these test cases use bochs, but others do use raw, so this
+# is only half a lie.
+_supported_fmt raw
+_supported_proto file
_supported_os Linux
echo
echo '=== Unsupported image creation in qemu-img create ==='
echo
-$QEMU_IMG create -f $IMGFMT nbd://example.com 2>&1 64M | _filter_imgfmt
+$QEMU_IMG create -f bochs nbd://example.com 2>&1 64M
echo
echo '=== Unsupported image creation in qemu-img convert ==='
@@ -56,17 +57,15 @@ echo
# We could use any input image format here, but this is a bochs test, so just
# use the bochs image
_use_sample_img empty.bochs.bz2
-$QEMU_IMG convert -f $IMGFMT -O $IMGFMT "$TEST_IMG" nbd://example.com 2>&1 \
- | _filter_imgfmt
+$QEMU_IMG convert -f bochs -O bochs "$TEST_IMG" nbd://example.com
echo
echo '=== Unsupported format in qemu-img amend ==='
echo
-# The protocol does not matter here
-_use_sample_img empty.bochs.bz2
-$QEMU_IMG amend -f $IMGFMT -o foo=bar "$TEST_IMG" 2>&1 | _filter_imgfmt
-
+TEST_IMG="$TEST_DIR/t.$IMGFMT"
+_make_test_img 1M
+$QEMU_IMG amend -f $IMGFMT -o size=2M "$TEST_IMG" 2>&1 | _filter_imgfmt
# success, all done
echo
diff --git a/tests/qemu-iotests/113.out b/tests/qemu-iotests/113.out
index 00bdfd6887..3557e2bbf0 100644
--- a/tests/qemu-iotests/113.out
+++ b/tests/qemu-iotests/113.out
@@ -2,14 +2,15 @@ QA output created by 113
=== Unsupported image creation in qemu-img create ===
-qemu-img: nbd://example.com: Format driver 'IMGFMT' does not support image creation
+qemu-img: nbd://example.com: Format driver 'bochs' does not support image creation
=== Unsupported image creation in qemu-img convert ===
-qemu-img: Format driver 'IMGFMT' does not support image creation
+qemu-img: Format driver 'bochs' does not support image creation
=== Unsupported format in qemu-img amend ===
-qemu-img: Format driver 'IMGFMT' does not support any options to amend
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576
+qemu-img: Format driver 'IMGFMT' does not support option amendment
*** done
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 11/29] qcow2: Repair OFLAG_COPIED when fixing leaks
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (9 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 10/29] iotests: Rework 113 Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 12/29] iotests: Repairing error during snapshot deletion Max Reitz
` (18 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Repairing OFLAG_COPIED is usually safe because it is done after the
refcounts have been repaired. Therefore, it we did not find anyone else
referencing a data or L2 cluster, it makes no sense to not set
OFLAG_COPIED -- and the other direction (clearing OFLAG_COPIED) is
always safe, anyway, it may just induce leaks.
Furthermore, if OFLAG_COPIED is actually consistent with a wrong (leaky)
refcount, we will decrement the refcount with -r leaks, but OFLAG_COPIED
will then be wrong. qemu-img check should not produce images that are
more corrupted afterwards then they were before.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1527085
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509200059.31125-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-refcount.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 403236256b..18c729aa27 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1799,6 +1799,19 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
int ret;
uint64_t refcount;
int i, j;
+ bool repair;
+
+ if (fix & BDRV_FIX_ERRORS) {
+ /* Always repair */
+ repair = true;
+ } else if (fix & BDRV_FIX_LEAKS) {
+ /* Repair only if that seems safe: This function is always
+ * called after the refcounts have been fixed, so the refcount
+ * is accurate if that repair was successful */
+ repair = !res->check_errors && !res->corruptions && !res->leaks;
+ } else {
+ repair = false;
+ }
for (i = 0; i < s->l1_size; i++) {
uint64_t l1_entry = s->l1_table[i];
@@ -1818,10 +1831,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d "
"l1_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" :
- "ERROR",
- i, l1_entry, refcount);
- if (fix & BDRV_FIX_ERRORS) {
+ repair ? "Repairing" : "ERROR", i, l1_entry, refcount);
+ if (repair) {
s->l1_table[i] = refcount == 1
? l1_entry | QCOW_OFLAG_COPIED
: l1_entry & ~QCOW_OFLAG_COPIED;
@@ -1862,10 +1873,8 @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res,
if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) {
fprintf(stderr, "%s OFLAG_COPIED data cluster: "
"l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n",
- fix & BDRV_FIX_ERRORS ? "Repairing" :
- "ERROR",
- l2_entry, refcount);
- if (fix & BDRV_FIX_ERRORS) {
+ repair ? "Repairing" : "ERROR", l2_entry, refcount);
+ if (repair) {
l2_table[j] = cpu_to_be64(refcount == 1
? l2_entry | QCOW_OFLAG_COPIED
: l2_entry & ~QCOW_OFLAG_COPIED);
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 12/29] iotests: Repairing error during snapshot deletion
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (10 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 11/29] qcow2: Repair OFLAG_COPIED when fixing leaks Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 13/29] qemu-io: Drop command functions' return values Max Reitz
` (17 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This adds a test for an I/O error during snapshot deletion, and maybe
more importantly, for how to repair the resulting image. If the
snapshot has been deleted before the error occurs, the only negative
result will be leaked clusters -- and those should be repairable with
qemu-img check -r leaks.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509200059.31125-3-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/217 | 90 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/217.out | 42 ++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 133 insertions(+)
create mode 100755 tests/qemu-iotests/217
create mode 100644 tests/qemu-iotests/217.out
diff --git a/tests/qemu-iotests/217 b/tests/qemu-iotests/217
new file mode 100755
index 0000000000..d3ab5d72be
--- /dev/null
+++ b/tests/qemu-iotests/217
@@ -0,0 +1,90 @@
+#!/bin/bash
+#
+# I/O errors when working with internal qcow2 snapshots, and repairing
+# the result
+#
+# 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/>.
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+ rm -f "$TEST_DIR/blkdebug.conf"
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This test is specific to qcow2
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+# This test needs clusters with at least a refcount of 2 so that
+# OFLAG_COPIED is not set. refcount_bits=1 is therefore unsupported.
+_unsupported_imgopts 'refcount_bits=1[^0-9]'
+
+echo
+echo '=== Simulating an I/O error during snapshot deletion ==='
+echo
+
+_make_test_img 64M
+$QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io
+
+# Create the snapshot
+$QEMU_IMG snapshot -c foo "$TEST_IMG"
+
+# Verify the snapshot is there
+echo
+_img_info | grep 'Snapshot list'
+echo '(Snapshot filtered)'
+echo
+
+# Try to delete the snapshot (with an error happening when freeing the
+# then leaked clusters)
+cat > "$TEST_DIR/blkdebug.conf" <<EOF
+[inject-error]
+event = "cluster_free"
+errno = "5"
+EOF
+$QEMU_IMG snapshot -d foo "blkdebug:$TEST_DIR/blkdebug.conf:$TEST_IMG"
+
+# Verify the snapshot is gone
+echo
+_img_info | grep 'Snapshot list'
+
+# Should only show leaks
+echo '--- Checking test image ---'
+_check_test_img
+
+echo
+
+# As there are only leaks, this should be able to fully repair the
+# image
+echo '--- Repairing test image ---'
+_check_test_img -r leaks
+
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/217.out b/tests/qemu-iotests/217.out
new file mode 100644
index 0000000000..e3fc40a8c7
--- /dev/null
+++ b/tests/qemu-iotests/217.out
@@ -0,0 +1,42 @@
+QA output created by 217
+
+=== Simulating an I/O error during snapshot deletion ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Snapshot list:
+(Snapshot filtered)
+
+qcow2_free_clusters failed: Input/output error
+qemu-img: Could not delete snapshot 'foo': Failed to free the cluster and L1 table: Input/output error
+
+--- Checking test image ---
+Leaked cluster 4 refcount=2 reference=1
+Leaked cluster 5 refcount=2 reference=1
+Leaked cluster 6 refcount=1 reference=0
+Leaked cluster 7 refcount=1 reference=0
+
+4 leaked clusters were found on the image.
+This means waste of disk space, but no harm to data.
+
+--- Repairing test image ---
+Leaked cluster 4 refcount=2 reference=1
+Leaked cluster 5 refcount=2 reference=1
+Leaked cluster 6 refcount=1 reference=0
+Leaked cluster 7 refcount=1 reference=0
+Repairing cluster 4 refcount=2 reference=1
+Repairing cluster 5 refcount=2 reference=1
+Repairing cluster 6 refcount=1 reference=0
+Repairing cluster 7 refcount=1 reference=0
+Repairing OFLAG_COPIED L2 cluster: l1_index=0 l1_entry=40000 refcount=1
+Repairing OFLAG_COPIED data cluster: l2_entry=50000 refcount=1
+The following inconsistencies were found and repaired:
+
+ 4 leaked clusters
+ 2 corruptions
+
+Double checking the fixed image now...
+No errors were found on the image.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 93f93d71ba..0914c922d7 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -215,5 +215,6 @@
214 rw auto
215 rw auto quick
216 rw auto quick
+217 rw auto quick
218 rw auto quick
219 rw auto
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 13/29] qemu-io: Drop command functions' return values
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (11 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 12/29] iotests: Repairing error during snapshot deletion Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 14/29] qemu-io: Let command functions return error code Max Reitz
` (16 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
For qemu-io, a function returns an integer with two possible values: 0
for "qemu-io may continue execution", or 1 for "qemu-io should exit".
However, there is only a single command that returns 1, and that is
"quit".
So let's turn this case into a global variable instead so we can make
better use of the return value in a later patch.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509194302.21585-2-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/qemu-io.h | 6 +-
qemu-io-cmds.c | 294 +++++++++++++++++++++-------------------------
qemu-io.c | 36 +++---
3 files changed, 157 insertions(+), 179 deletions(-)
diff --git a/include/qemu-io.h b/include/qemu-io.h
index 196fde0f3a..06cdfbf660 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -22,7 +22,7 @@
#define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
-typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
typedef void (*helpfunc_t)(void);
typedef struct cmdinfo {
@@ -41,10 +41,10 @@ typedef struct cmdinfo {
extern bool qemuio_misalign;
-bool qemuio_command(BlockBackend *blk, const char *cmd);
+void qemuio_command(BlockBackend *blk, const char *cmd);
void qemuio_add_command(const cmdinfo_t *ci);
-int qemuio_command_usage(const cmdinfo_t *ci);
+void qemuio_command_usage(const cmdinfo_t *ci);
void qemuio_complete_command(const char *input,
void (*fn)(const char *cmd, void *opaque),
void *opaque);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 9b3cd00af6..c2fbaae0fd 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -48,10 +48,9 @@ void qemuio_add_command(const cmdinfo_t *ci)
qsort(cmdtab, ncmds, sizeof(*cmdtab), compare_cmdname);
}
-int qemuio_command_usage(const cmdinfo_t *ci)
+void qemuio_command_usage(const cmdinfo_t *ci)
{
printf("%s %s -- %s\n", ci->name, ci->args, ci->oneline);
- return 0;
}
static int init_check_command(BlockBackend *blk, const cmdinfo_t *ct)
@@ -66,13 +65,13 @@ static int init_check_command(BlockBackend *blk, const cmdinfo_t *ct)
return 1;
}
-static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
- char **argv)
+static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
+ char **argv)
{
char *cmd = argv[0];
if (!init_check_command(blk, ct)) {
- return 0;
+ return;
}
if (argc - 1 < ct->argmin || (ct->argmax != -1 && argc - 1 > ct->argmax)) {
@@ -89,7 +88,7 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
"bad argument count %d to %s, expected between %d and %d arguments\n",
argc-1, cmd, ct->argmin, ct->argmax);
}
- return 0;
+ return;
}
/* Request additional permissions if necessary for this command. The caller
@@ -109,13 +108,13 @@ static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
ret = blk_set_perm(blk, new_perm, orig_shared_perm, &local_err);
if (ret < 0) {
error_report_err(local_err);
- return 0;
+ return;
}
}
}
optind = 0;
- return ct->cfunc(blk, argc, argv);
+ ct->cfunc(blk, argc, argv);
}
static const cmdinfo_t *find_command(const char *cmd)
@@ -634,7 +633,7 @@ static void read_help(void)
"\n");
}
-static int read_f(BlockBackend *blk, int argc, char **argv);
+static void read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t read_cmd = {
.name = "read",
@@ -647,7 +646,7 @@ static const cmdinfo_t read_cmd = {
.help = read_help,
};
-static int read_f(BlockBackend *blk, int argc, char **argv)
+static void read_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, vflag = false;
@@ -674,7 +673,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
pattern_count = cvtnum(optarg);
if (pattern_count < 0) {
print_cvtnum_err(pattern_count, optarg);
- return 0;
+ return;
}
break;
case 'p':
@@ -684,7 +683,7 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return 0;
+ return;
}
break;
case 'q':
@@ -695,40 +694,43 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
pattern_offset = cvtnum(optarg);
if (pattern_offset < 0) {
print_cvtnum_err(pattern_offset, optarg);
- return 0;
+ return;
}
break;
case 'v':
vflag = true;
break;
default:
- return qemuio_command_usage(&read_cmd);
+ qemuio_command_usage(&read_cmd);
+ return;
}
}
if (optind != argc - 2) {
- return qemuio_command_usage(&read_cmd);
+ qemuio_command_usage(&read_cmd);
+ return;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return 0;
+ return;
}
optind++;
count = cvtnum(argv[optind]);
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
- return 0;
+ return;
} else if (count > BDRV_REQUEST_MAX_BYTES) {
printf("length cannot exceed %" PRIu64 ", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
- return 0;
+ return;
}
if (!Pflag && (lflag || sflag)) {
- return qemuio_command_usage(&read_cmd);
+ qemuio_command_usage(&read_cmd);
+ return;
}
if (!lflag) {
@@ -737,19 +739,19 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
if ((pattern_count < 0) || (pattern_count + pattern_offset > count)) {
printf("pattern verification range exceeds end of read data\n");
- return 0;
+ return;
}
if (bflag) {
if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
offset);
- return 0;
+ return;
}
if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
printf("%"PRId64" is not a sector-aligned value for 'count'\n",
count);
- return 0;
+ return;
}
}
@@ -793,8 +795,6 @@ static int read_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_io_free(buf);
-
- return 0;
}
static void readv_help(void)
@@ -816,7 +816,7 @@ static void readv_help(void)
"\n");
}
-static int readv_f(BlockBackend *blk, int argc, char **argv);
+static void readv_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t readv_cmd = {
.name = "readv",
@@ -828,7 +828,7 @@ static const cmdinfo_t readv_cmd = {
.help = readv_help,
};
-static int readv_f(BlockBackend *blk, int argc, char **argv)
+static void readv_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, vflag = false;
@@ -851,7 +851,7 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return 0;
+ return;
}
break;
case 'q':
@@ -861,26 +861,28 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
vflag = true;
break;
default:
- return qemuio_command_usage(&readv_cmd);
+ qemuio_command_usage(&readv_cmd);
+ return;
}
}
if (optind > argc - 2) {
- return qemuio_command_usage(&readv_cmd);
+ qemuio_command_usage(&readv_cmd);
+ return;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return 0;
+ return;
}
optind++;
nr_iov = argc - optind;
buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab);
if (buf == NULL) {
- return 0;
+ return;
}
gettimeofday(&t1, NULL);
@@ -917,7 +919,6 @@ static int readv_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
- return 0;
}
static void write_help(void)
@@ -943,7 +944,7 @@ static void write_help(void)
"\n");
}
-static int write_f(BlockBackend *blk, int argc, char **argv);
+static void write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t write_cmd = {
.name = "write",
@@ -957,7 +958,7 @@ static const cmdinfo_t write_cmd = {
.help = write_help,
};
-static int write_f(BlockBackend *blk, int argc, char **argv)
+static void write_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, bflag = false;
@@ -992,7 +993,7 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return 0;
+ return;
}
break;
case 'q':
@@ -1005,62 +1006,64 @@ static int write_f(BlockBackend *blk, int argc, char **argv)
zflag = true;
break;
default:
- return qemuio_command_usage(&write_cmd);
+ qemuio_command_usage(&write_cmd);
+ return;
}
}
if (optind != argc - 2) {
- return qemuio_command_usage(&write_cmd);
+ qemuio_command_usage(&write_cmd);
+ return;
}
if (bflag && zflag) {
printf("-b and -z cannot be specified at the same time\n");
- return 0;
+ return;
}
if ((flags & BDRV_REQ_FUA) && (bflag || cflag)) {
printf("-f and -b or -c cannot be specified at the same time\n");
- return 0;
+ return;
}
if ((flags & BDRV_REQ_MAY_UNMAP) && !zflag) {
printf("-u requires -z to be specified\n");
- return 0;
+ return;
}
if (zflag && Pflag) {
printf("-z and -P cannot be specified at the same time\n");
- return 0;
+ return;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return 0;
+ return;
}
optind++;
count = cvtnum(argv[optind]);
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
- return 0;
+ return;
} else if (count > BDRV_REQUEST_MAX_BYTES) {
printf("length cannot exceed %" PRIu64 ", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
- return 0;
+ return;
}
if (bflag || cflag) {
if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
offset);
- return 0;
+ return;
}
if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
printf("%"PRId64" is not a sector-aligned value for 'count'\n",
count);
- return 0;
+ return;
}
}
@@ -1097,8 +1100,6 @@ out:
if (!zflag) {
qemu_io_free(buf);
}
-
- return 0;
}
static void
@@ -1120,7 +1121,7 @@ writev_help(void)
"\n");
}
-static int writev_f(BlockBackend *blk, int argc, char **argv);
+static void writev_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t writev_cmd = {
.name = "writev",
@@ -1133,7 +1134,7 @@ static const cmdinfo_t writev_cmd = {
.help = writev_help,
};
-static int writev_f(BlockBackend *blk, int argc, char **argv)
+static void writev_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false;
@@ -1161,29 +1162,31 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
case 'P':
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return 0;
+ return;
}
break;
default:
- return qemuio_command_usage(&writev_cmd);
+ qemuio_command_usage(&writev_cmd);
+ return;
}
}
if (optind > argc - 2) {
- return qemuio_command_usage(&writev_cmd);
+ qemuio_command_usage(&writev_cmd);
+ return;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return 0;
+ return;
}
optind++;
nr_iov = argc - optind;
buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern);
if (buf == NULL) {
- return 0;
+ return;
}
gettimeofday(&t1, NULL);
@@ -1205,7 +1208,6 @@ static int writev_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
- return 0;
}
struct aio_ctx {
@@ -1320,7 +1322,7 @@ static void aio_read_help(void)
"\n");
}
-static int aio_read_f(BlockBackend *blk, int argc, char **argv);
+static void aio_read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_read_cmd = {
.name = "aio_read",
@@ -1332,7 +1334,7 @@ static const cmdinfo_t aio_read_cmd = {
.help = aio_read_help,
};
-static int aio_read_f(BlockBackend *blk, int argc, char **argv)
+static void aio_read_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
@@ -1348,14 +1350,14 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
ctx->pattern = parse_pattern(optarg);
if (ctx->pattern < 0) {
g_free(ctx);
- return 0;
+ return;
}
break;
case 'i':
printf("injecting invalid read request\n");
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
g_free(ctx);
- return 0;
+ return;
case 'q':
ctx->qflag = true;
break;
@@ -1364,20 +1366,22 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
break;
default:
g_free(ctx);
- return qemuio_command_usage(&aio_read_cmd);
+ qemuio_command_usage(&aio_read_cmd);
+ return;
}
}
if (optind > argc - 2) {
g_free(ctx);
- return qemuio_command_usage(&aio_read_cmd);
+ qemuio_command_usage(&aio_read_cmd);
+ return;
}
ctx->offset = cvtnum(argv[optind]);
if (ctx->offset < 0) {
print_cvtnum_err(ctx->offset, argv[optind]);
g_free(ctx);
- return 0;
+ return;
}
optind++;
@@ -1386,14 +1390,13 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv)
if (ctx->buf == NULL) {
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
g_free(ctx);
- return 0;
+ return;
}
gettimeofday(&ctx->t1, NULL);
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_READ);
blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx);
- return 0;
}
static void aio_write_help(void)
@@ -1420,7 +1423,7 @@ static void aio_write_help(void)
"\n");
}
-static int aio_write_f(BlockBackend *blk, int argc, char **argv);
+static void aio_write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_write_cmd = {
.name = "aio_write",
@@ -1433,7 +1436,7 @@ static const cmdinfo_t aio_write_cmd = {
.help = aio_write_help,
};
-static int aio_write_f(BlockBackend *blk, int argc, char **argv)
+static void aio_write_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
int pattern = 0xcd;
@@ -1459,51 +1462,53 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
pattern = parse_pattern(optarg);
if (pattern < 0) {
g_free(ctx);
- return 0;
+ return;
}
break;
case 'i':
printf("injecting invalid write request\n");
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
g_free(ctx);
- return 0;
+ return;
case 'z':
ctx->zflag = true;
break;
default:
g_free(ctx);
- return qemuio_command_usage(&aio_write_cmd);
+ qemuio_command_usage(&aio_write_cmd);
+ return;
}
}
if (optind > argc - 2) {
g_free(ctx);
- return qemuio_command_usage(&aio_write_cmd);
+ qemuio_command_usage(&aio_write_cmd);
+ return;
}
if (ctx->zflag && optind != argc - 2) {
printf("-z supports only a single length parameter\n");
g_free(ctx);
- return 0;
+ return;
}
if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {
printf("-u requires -z to be specified\n");
g_free(ctx);
- return 0;
+ return;
}
if (ctx->zflag && ctx->Pflag) {
printf("-z and -P cannot be specified at the same time\n");
g_free(ctx);
- return 0;
+ return;
}
ctx->offset = cvtnum(argv[optind]);
if (ctx->offset < 0) {
print_cvtnum_err(ctx->offset, argv[optind]);
g_free(ctx);
- return 0;
+ return;
}
optind++;
@@ -1512,7 +1517,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
g_free(ctx);
- return 0;
+ return;
}
ctx->qiov.size = count;
@@ -1525,7 +1530,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
if (ctx->buf == NULL) {
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
g_free(ctx);
- return 0;
+ return;
}
gettimeofday(&ctx->t1, NULL);
@@ -1535,16 +1540,14 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv)
blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, flags, aio_write_done,
ctx);
}
- return 0;
}
-static int aio_flush_f(BlockBackend *blk, int argc, char **argv)
+static void aio_flush_f(BlockBackend *blk, int argc, char **argv)
{
BlockAcctCookie cookie;
block_acct_start(blk_get_stats(blk), &cookie, 0, BLOCK_ACCT_FLUSH);
blk_drain_all();
block_acct_done(blk_get_stats(blk), &cookie);
- return 0;
}
static const cmdinfo_t aio_flush_cmd = {
@@ -1553,10 +1556,9 @@ static const cmdinfo_t aio_flush_cmd = {
.oneline = "completes all outstanding aio requests"
};
-static int flush_f(BlockBackend *blk, int argc, char **argv)
+static void flush_f(BlockBackend *blk, int argc, char **argv)
{
blk_flush(blk);
- return 0;
}
static const cmdinfo_t flush_cmd = {
@@ -1566,7 +1568,7 @@ static const cmdinfo_t flush_cmd = {
.oneline = "flush all in-core file state to disk",
};
-static int truncate_f(BlockBackend *blk, int argc, char **argv)
+static void truncate_f(BlockBackend *blk, int argc, char **argv)
{
Error *local_err = NULL;
int64_t offset;
@@ -1575,16 +1577,14 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
offset = cvtnum(argv[1]);
if (offset < 0) {
print_cvtnum_err(offset, argv[1]);
- return 0;
+ return;
}
ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
- return 0;
+ return;
}
-
- return 0;
}
static const cmdinfo_t truncate_cmd = {
@@ -1598,7 +1598,7 @@ static const cmdinfo_t truncate_cmd = {
.oneline = "truncates the current file at the given offset",
};
-static int length_f(BlockBackend *blk, int argc, char **argv)
+static void length_f(BlockBackend *blk, int argc, char **argv)
{
int64_t size;
char s1[64];
@@ -1606,12 +1606,11 @@ static int length_f(BlockBackend *blk, int argc, char **argv)
size = blk_getlength(blk);
if (size < 0) {
printf("getlength: %s\n", strerror(-size));
- return 0;
+ return;
}
cvtstr(size, s1, sizeof(s1));
printf("%s\n", s1);
- return 0;
}
@@ -1623,7 +1622,7 @@ static const cmdinfo_t length_cmd = {
};
-static int info_f(BlockBackend *blk, int argc, char **argv)
+static void info_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
BlockDriverInfo bdi;
@@ -1640,7 +1639,7 @@ static int info_f(BlockBackend *blk, int argc, char **argv)
ret = bdrv_get_info(bs, &bdi);
if (ret) {
- return 0;
+ return;
}
cvtstr(bdi.cluster_size, s1, sizeof(s1));
@@ -1655,8 +1654,6 @@ static int info_f(BlockBackend *blk, int argc, char **argv)
bdrv_image_info_specific_dump(fprintf, stdout, spec_info);
qapi_free_ImageInfoSpecific(spec_info);
}
-
- return 0;
}
@@ -1683,7 +1680,7 @@ static void discard_help(void)
"\n");
}
-static int discard_f(BlockBackend *blk, int argc, char **argv);
+static void discard_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t discard_cmd = {
.name = "discard",
@@ -1697,7 +1694,7 @@ static const cmdinfo_t discard_cmd = {
.help = discard_help,
};
-static int discard_f(BlockBackend *blk, int argc, char **argv)
+static void discard_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false;
@@ -1713,30 +1710,32 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
qflag = true;
break;
default:
- return qemuio_command_usage(&discard_cmd);
+ qemuio_command_usage(&discard_cmd);
+ return;
}
}
if (optind != argc - 2) {
- return qemuio_command_usage(&discard_cmd);
+ qemuio_command_usage(&discard_cmd);
+ return;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return 0;
+ return;
}
optind++;
bytes = cvtnum(argv[optind]);
if (bytes < 0) {
print_cvtnum_err(bytes, argv[optind]);
- return 0;
+ return;
} else if (bytes >> BDRV_SECTOR_BITS > BDRV_REQUEST_MAX_SECTORS) {
printf("length cannot exceed %"PRIu64", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS,
argv[optind]);
- return 0;
+ return;
}
gettimeofday(&t1, NULL);
@@ -1745,7 +1744,7 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
if (ret < 0) {
printf("discard failed: %s\n", strerror(-ret));
- goto out;
+ return;
}
/* Finally, report back -- -C gives a parsable format */
@@ -1753,12 +1752,9 @@ static int discard_f(BlockBackend *blk, int argc, char **argv)
t2 = tsub(t2, t1);
print_report("discard", &t2, offset, bytes, bytes, 1, Cflag);
}
-
-out:
- return 0;
}
-static int alloc_f(BlockBackend *blk, int argc, char **argv)
+static void alloc_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
int64_t offset, start, remaining, count;
@@ -1769,14 +1765,14 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
start = offset = cvtnum(argv[1]);
if (offset < 0) {
print_cvtnum_err(offset, argv[1]);
- return 0;
+ return;
}
if (argc == 3) {
count = cvtnum(argv[2]);
if (count < 0) {
print_cvtnum_err(count, argv[2]);
- return 0;
+ return;
}
} else {
count = BDRV_SECTOR_SIZE;
@@ -1788,7 +1784,7 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
ret = bdrv_is_allocated(bs, offset, remaining, &num);
if (ret < 0) {
printf("is_allocated failed: %s\n", strerror(-ret));
- return 0;
+ return;
}
offset += num;
remaining -= num;
@@ -1805,7 +1801,6 @@ static int alloc_f(BlockBackend *blk, int argc, char **argv)
printf("%"PRId64"/%"PRId64" bytes allocated at offset %s\n",
sum_alloc, count, s1);
- return 0;
}
static const cmdinfo_t alloc_cmd = {
@@ -1851,7 +1846,7 @@ static int map_is_allocated(BlockDriverState *bs, int64_t offset,
return firstret;
}
-static int map_f(BlockBackend *blk, int argc, char **argv)
+static void map_f(BlockBackend *blk, int argc, char **argv)
{
int64_t offset, bytes;
char s1[64], s2[64];
@@ -1863,17 +1858,17 @@ static int map_f(BlockBackend *blk, int argc, char **argv)
bytes = blk_getlength(blk);
if (bytes < 0) {
error_report("Failed to query image length: %s", strerror(-bytes));
- return 0;
+ return;
}
while (bytes) {
ret = map_is_allocated(blk_bs(blk), offset, bytes, &num);
if (ret < 0) {
error_report("Failed to get allocation status: %s", strerror(-ret));
- return 0;
+ return;
} else if (!num) {
error_report("Unexpected end of image");
- return 0;
+ return;
}
retstr = ret ? " allocated" : "not allocated";
@@ -1885,8 +1880,6 @@ static int map_f(BlockBackend *blk, int argc, char **argv)
offset += num;
bytes -= num;
}
-
- return 0;
}
static const cmdinfo_t map_cmd = {
@@ -1914,7 +1907,7 @@ static void reopen_help(void)
"\n");
}
-static int reopen_f(BlockBackend *blk, int argc, char **argv);
+static void reopen_f(BlockBackend *blk, int argc, char **argv);
static QemuOptsList reopen_opts = {
.name = "reopen",
@@ -1936,7 +1929,7 @@ static const cmdinfo_t reopen_cmd = {
.help = reopen_help,
};
-static int reopen_f(BlockBackend *blk, int argc, char **argv)
+static void reopen_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
QemuOpts *qopts;
@@ -1954,19 +1947,19 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
case 'c':
if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
- return 0;
+ return;
}
break;
case 'o':
if (!qemu_opts_parse_noisily(&reopen_opts, optarg, 0)) {
qemu_opts_reset(&reopen_opts);
- return 0;
+ return;
}
break;
case 'r':
if (has_rw_option) {
error_report("Only one -r/-w option may be given");
- return 0;
+ return;
}
flags &= ~BDRV_O_RDWR;
has_rw_option = true;
@@ -1974,20 +1967,22 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
case 'w':
if (has_rw_option) {
error_report("Only one -r/-w option may be given");
- return 0;
+ return;
}
flags |= BDRV_O_RDWR;
has_rw_option = true;
break;
default:
qemu_opts_reset(&reopen_opts);
- return qemuio_command_usage(&reopen_cmd);
+ qemuio_command_usage(&reopen_cmd);
+ return;
}
}
if (optind != argc) {
qemu_opts_reset(&reopen_opts);
- return qemuio_command_usage(&reopen_cmd);
+ qemuio_command_usage(&reopen_cmd);
+ return;
}
if (writethrough != blk_enable_write_cache(blk) &&
@@ -1995,7 +1990,7 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
{
error_report("Cannot change cache.writeback: Device attached");
qemu_opts_reset(&reopen_opts);
- return 0;
+ return;
}
if (!(flags & BDRV_O_RDWR)) {
@@ -2024,11 +2019,9 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
} else {
blk_set_enable_write_cache(blk, !writethrough);
}
-
- return 0;
}
-static int break_f(BlockBackend *blk, int argc, char **argv)
+static void break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
@@ -2036,11 +2029,9 @@ static int break_f(BlockBackend *blk, int argc, char **argv)
if (ret < 0) {
printf("Could not set breakpoint: %s\n", strerror(-ret));
}
-
- return 0;
}
-static int remove_break_f(BlockBackend *blk, int argc, char **argv)
+static void remove_break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
@@ -2048,8 +2039,6 @@ static int remove_break_f(BlockBackend *blk, int argc, char **argv)
if (ret < 0) {
printf("Could not remove breakpoint %s: %s\n", argv[1], strerror(-ret));
}
-
- return 0;
}
static const cmdinfo_t break_cmd = {
@@ -2071,7 +2060,7 @@ static const cmdinfo_t remove_break_cmd = {
.oneline = "remove a breakpoint by tag",
};
-static int resume_f(BlockBackend *blk, int argc, char **argv)
+static void resume_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
@@ -2079,8 +2068,6 @@ static int resume_f(BlockBackend *blk, int argc, char **argv)
if (ret < 0) {
printf("Could not resume request: %s\n", strerror(-ret));
}
-
- return 0;
}
static const cmdinfo_t resume_cmd = {
@@ -2092,13 +2079,11 @@ static const cmdinfo_t resume_cmd = {
.oneline = "resumes the request tagged as tag",
};
-static int wait_break_f(BlockBackend *blk, int argc, char **argv)
+static void wait_break_f(BlockBackend *blk, int argc, char **argv)
{
while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
aio_poll(blk_get_aio_context(blk), true);
}
-
- return 0;
}
static const cmdinfo_t wait_break_cmd = {
@@ -2110,7 +2095,7 @@ static const cmdinfo_t wait_break_cmd = {
.oneline = "waits for the suspension of a request",
};
-static int abort_f(BlockBackend *blk, int argc, char **argv)
+static void abort_f(BlockBackend *blk, int argc, char **argv)
{
abort();
}
@@ -2136,7 +2121,7 @@ static void sigraise_help(void)
"\n", SIGTERM);
}
-static int sigraise_f(BlockBackend *blk, int argc, char **argv);
+static void sigraise_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t sigraise_cmd = {
.name = "sigraise",
@@ -2149,16 +2134,16 @@ static const cmdinfo_t sigraise_cmd = {
.help = sigraise_help,
};
-static int sigraise_f(BlockBackend *blk, int argc, char **argv)
+static void sigraise_f(BlockBackend *blk, int argc, char **argv)
{
int64_t sig = cvtnum(argv[1]);
if (sig < 0) {
print_cvtnum_err(sig, argv[1]);
- return 0;
+ return;
} else if (sig > NSIG) {
printf("signal argument '%s' is too large to be a valid signal\n",
argv[1]);
- return 0;
+ return;
}
/* Using raise() to kill this process does not necessarily flush all open
@@ -2168,7 +2153,6 @@ static int sigraise_f(BlockBackend *blk, int argc, char **argv)
fflush(stderr);
raise(sig);
- return 0;
}
static void sleep_cb(void *opaque)
@@ -2177,7 +2161,7 @@ static void sleep_cb(void *opaque)
*expired = true;
}
-static int sleep_f(BlockBackend *blk, int argc, char **argv)
+static void sleep_f(BlockBackend *blk, int argc, char **argv)
{
char *endptr;
long ms;
@@ -2187,7 +2171,7 @@ static int sleep_f(BlockBackend *blk, int argc, char **argv)
ms = strtol(argv[1], &endptr, 0);
if (ms < 0 || *endptr != '\0') {
printf("%s is not a valid number\n", argv[1]);
- return 0;
+ return;
}
timer = timer_new_ns(QEMU_CLOCK_HOST, sleep_cb, &expired);
@@ -2198,8 +2182,6 @@ static int sleep_f(BlockBackend *blk, int argc, char **argv)
}
timer_free(timer);
-
- return 0;
}
static const cmdinfo_t sleep_cmd = {
@@ -2246,23 +2228,22 @@ static void help_all(void)
printf("\nUse 'help commandname' for extended help.\n");
}
-static int help_f(BlockBackend *blk, int argc, char **argv)
+static void help_f(BlockBackend *blk, int argc, char **argv)
{
const cmdinfo_t *ct;
if (argc == 1) {
help_all();
- return 0;
+ return;
}
ct = find_command(argv[1]);
if (ct == NULL) {
printf("command %s not found\n", argv[1]);
- return 0;
+ return;
}
help_onecmd(argv[1], ct);
- return 0;
}
static const cmdinfo_t help_cmd = {
@@ -2276,14 +2257,13 @@ static const cmdinfo_t help_cmd = {
.oneline = "help for one or all commands",
};
-bool qemuio_command(BlockBackend *blk, const char *cmd)
+void qemuio_command(BlockBackend *blk, const char *cmd)
{
AioContext *ctx;
char *input;
const cmdinfo_t *ct;
char **v;
int c;
- bool done = false;
input = g_strdup(cmd);
v = breakline(input, &c);
@@ -2292,7 +2272,7 @@ bool qemuio_command(BlockBackend *blk, const char *cmd)
if (ct) {
ctx = blk ? blk_get_aio_context(blk) : qemu_get_aio_context();
aio_context_acquire(ctx);
- done = command(blk, ct, c, v);
+ command(blk, ct, c, v);
aio_context_release(ctx);
} else {
fprintf(stderr, "command \"%s\" not found\n", v[0]);
@@ -2300,8 +2280,6 @@ bool qemuio_command(BlockBackend *blk, const char *cmd)
}
g_free(input);
g_free(v);
-
- return done;
}
static void __attribute((constructor)) init_qemuio_commands(void)
diff --git a/qemu-io.c b/qemu-io.c
index 73c638ff8b..02a67c929a 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -37,6 +37,7 @@
static char *progname;
static BlockBackend *qemuio_blk;
+static bool quit_qemu_io;
/* qemu-io commands passed using -c */
static int ncmdline;
@@ -65,11 +66,10 @@ static int get_eof_char(void)
#endif
}
-static int close_f(BlockBackend *blk, int argc, char **argv)
+static void close_f(BlockBackend *blk, int argc, char **argv)
{
blk_unref(qemuio_blk);
qemuio_blk = NULL;
- return 0;
}
static const cmdinfo_t close_cmd = {
@@ -136,7 +136,7 @@ static void open_help(void)
"\n");
}
-static int open_f(BlockBackend *blk, int argc, char **argv);
+static void open_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t open_cmd = {
.name = "open",
@@ -160,7 +160,7 @@ static QemuOptsList empty_opts = {
},
};
-static int open_f(BlockBackend *blk, int argc, char **argv)
+static void open_f(BlockBackend *blk, int argc, char **argv)
{
int flags = BDRV_O_UNMAP;
int readonly = 0;
@@ -192,25 +192,25 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
qemu_opts_reset(&empty_opts);
- return 0;
+ return;
}
break;
case 'd':
if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
error_report("Invalid discard option: %s", optarg);
qemu_opts_reset(&empty_opts);
- return 0;
+ return;
}
break;
case 'o':
if (imageOpts) {
printf("--image-opts and 'open -o' are mutually exclusive\n");
qemu_opts_reset(&empty_opts);
- return 0;
+ return;
}
if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
qemu_opts_reset(&empty_opts);
- return 0;
+ return;
}
break;
case 'U':
@@ -218,7 +218,8 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
break;
default:
qemu_opts_reset(&empty_opts);
- return qemuio_command_usage(&open_cmd);
+ qemuio_command_usage(&open_cmd);
+ return;
}
}
@@ -229,7 +230,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
if (imageOpts && (optind == argc - 1)) {
if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
qemu_opts_reset(&empty_opts);
- return 0;
+ return;
}
optind++;
}
@@ -246,12 +247,11 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
qobject_unref(opts);
qemuio_command_usage(&open_cmd);
}
- return 0;
}
-static int quit_f(BlockBackend *blk, int argc, char **argv)
+static void quit_f(BlockBackend *blk, int argc, char **argv)
{
- return 1;
+ quit_qemu_io = true;
}
static const cmdinfo_t quit_cmd = {
@@ -392,18 +392,18 @@ static void prep_fetchline(void *opaque)
static void command_loop(void)
{
- int i, done = 0, fetchable = 0, prompted = 0;
+ int i, fetchable = 0, prompted = 0;
char *input;
- for (i = 0; !done && i < ncmdline; i++) {
- done = qemuio_command(qemuio_blk, cmdline[i]);
+ for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
+ qemuio_command(qemuio_blk, cmdline[i]);
}
if (cmdline) {
g_free(cmdline);
return;
}
- while (!done) {
+ while (!quit_qemu_io) {
if (!prompted) {
printf("%s", get_prompt());
fflush(stdout);
@@ -421,7 +421,7 @@ static void command_loop(void)
if (input == NULL) {
break;
}
- done = qemuio_command(qemuio_blk, input);
+ qemuio_command(qemuio_blk, input);
g_free(input);
prompted = 0;
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 14/29] qemu-io: Let command functions return error code
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (12 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 13/29] qemu-io: Drop command functions' return values Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 15/29] qemu-io: Exit with error when a command failed Max Reitz
` (15 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This is basically what everything else in the qemu code base does, so we
can do it here, too.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509194302.21585-3-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/qemu-io.h | 9 +-
qemu-io-cmds.c | 346 +++++++++++++++++++++++++++-------------------
qemu-io.c | 34 +++--
3 files changed, 232 insertions(+), 157 deletions(-)
diff --git a/include/qemu-io.h b/include/qemu-io.h
index 06cdfbf660..7433239372 100644
--- a/include/qemu-io.h
+++ b/include/qemu-io.h
@@ -22,7 +22,12 @@
#define CMD_FLAG_GLOBAL ((int)0x80000000) /* don't iterate "args" */
-typedef void (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+/* Implement a qemu-io command.
+ * Operate on @blk using @argc/@argv as the command's arguments, and
+ * return 0 on success or negative errno on failure.
+ */
+typedef int (*cfunc_t)(BlockBackend *blk, int argc, char **argv);
+
typedef void (*helpfunc_t)(void);
typedef struct cmdinfo {
@@ -41,7 +46,7 @@ typedef struct cmdinfo {
extern bool qemuio_misalign;
-void qemuio_command(BlockBackend *blk, const char *cmd);
+int qemuio_command(BlockBackend *blk, const char *cmd);
void qemuio_add_command(const cmdinfo_t *ci);
void qemuio_command_usage(const cmdinfo_t *ci);
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index c2fbaae0fd..5bf5f28178 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -65,13 +65,13 @@ static int init_check_command(BlockBackend *blk, const cmdinfo_t *ct)
return 1;
}
-static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
- char **argv)
+static int command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
+ char **argv)
{
char *cmd = argv[0];
if (!init_check_command(blk, ct)) {
- return;
+ return -EINVAL;
}
if (argc - 1 < ct->argmin || (ct->argmax != -1 && argc - 1 > ct->argmax)) {
@@ -88,7 +88,7 @@ static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
"bad argument count %d to %s, expected between %d and %d arguments\n",
argc-1, cmd, ct->argmin, ct->argmax);
}
- return;
+ return -EINVAL;
}
/* Request additional permissions if necessary for this command. The caller
@@ -108,13 +108,13 @@ static void command(BlockBackend *blk, const cmdinfo_t *ct, int argc,
ret = blk_set_perm(blk, new_perm, orig_shared_perm, &local_err);
if (ret < 0) {
error_report_err(local_err);
- return;
+ return ret;
}
}
}
optind = 0;
- ct->cfunc(blk, argc, argv);
+ return ct->cfunc(blk, argc, argv);
}
static const cmdinfo_t *find_command(const char *cmd)
@@ -633,7 +633,7 @@ static void read_help(void)
"\n");
}
-static void read_f(BlockBackend *blk, int argc, char **argv);
+static int read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t read_cmd = {
.name = "read",
@@ -646,12 +646,12 @@ static const cmdinfo_t read_cmd = {
.help = read_help,
};
-static void read_f(BlockBackend *blk, int argc, char **argv)
+static int read_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, vflag = false;
bool Pflag = false, sflag = false, lflag = false, bflag = false;
- int c, cnt;
+ int c, cnt, ret;
char *buf;
int64_t offset;
int64_t count;
@@ -673,7 +673,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
pattern_count = cvtnum(optarg);
if (pattern_count < 0) {
print_cvtnum_err(pattern_count, optarg);
- return;
+ return pattern_count;
}
break;
case 'p':
@@ -683,7 +683,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return;
+ return -EINVAL;
}
break;
case 'q':
@@ -694,7 +694,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
pattern_offset = cvtnum(optarg);
if (pattern_offset < 0) {
print_cvtnum_err(pattern_offset, optarg);
- return;
+ return pattern_offset;
}
break;
case 'v':
@@ -702,35 +702,35 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
break;
default:
qemuio_command_usage(&read_cmd);
- return;
+ return -EINVAL;
}
}
if (optind != argc - 2) {
qemuio_command_usage(&read_cmd);
- return;
+ return -EINVAL;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return;
+ return offset;
}
optind++;
count = cvtnum(argv[optind]);
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
- return;
+ return count;
} else if (count > BDRV_REQUEST_MAX_BYTES) {
printf("length cannot exceed %" PRIu64 ", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
- return;
+ return -EINVAL;
}
if (!Pflag && (lflag || sflag)) {
qemuio_command_usage(&read_cmd);
- return;
+ return -EINVAL;
}
if (!lflag) {
@@ -739,19 +739,19 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
if ((pattern_count < 0) || (pattern_count + pattern_offset > count)) {
printf("pattern verification range exceeds end of read data\n");
- return;
+ return -EINVAL;
}
if (bflag) {
if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
offset);
- return;
+ return -EINVAL;
}
if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
printf("%"PRId64" is not a sector-aligned value for 'count'\n",
count);
- return;
+ return -EINVAL;
}
}
@@ -759,16 +759,19 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
gettimeofday(&t1, NULL);
if (bflag) {
- cnt = do_load_vmstate(blk, buf, offset, count, &total);
+ ret = do_load_vmstate(blk, buf, offset, count, &total);
} else {
- cnt = do_pread(blk, buf, offset, count, &total);
+ ret = do_pread(blk, buf, offset, count, &total);
}
gettimeofday(&t2, NULL);
- if (cnt < 0) {
- printf("read failed: %s\n", strerror(-cnt));
+ if (ret < 0) {
+ printf("read failed: %s\n", strerror(-ret));
goto out;
}
+ cnt = ret;
+
+ ret = 0;
if (Pflag) {
void *cmp_buf = g_malloc(pattern_count);
@@ -777,6 +780,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
printf("Pattern verification failed at offset %"
PRId64 ", %"PRId64" bytes\n",
offset + pattern_offset, pattern_count);
+ ret = -EINVAL;
}
g_free(cmp_buf);
}
@@ -795,6 +799,7 @@ static void read_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_io_free(buf);
+ return ret;
}
static void readv_help(void)
@@ -816,7 +821,7 @@ static void readv_help(void)
"\n");
}
-static void readv_f(BlockBackend *blk, int argc, char **argv);
+static int readv_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t readv_cmd = {
.name = "readv",
@@ -828,11 +833,11 @@ static const cmdinfo_t readv_cmd = {
.help = readv_help,
};
-static void readv_f(BlockBackend *blk, int argc, char **argv)
+static int readv_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, vflag = false;
- int c, cnt;
+ int c, cnt, ret;
char *buf;
int64_t offset;
/* Some compilers get confused and warn if this is not initialized. */
@@ -851,7 +856,7 @@ static void readv_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return;
+ return -EINVAL;
}
break;
case 'q':
@@ -862,37 +867,40 @@ static void readv_f(BlockBackend *blk, int argc, char **argv)
break;
default:
qemuio_command_usage(&readv_cmd);
- return;
+ return -EINVAL;
}
}
if (optind > argc - 2) {
qemuio_command_usage(&readv_cmd);
- return;
+ return -EINVAL;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return;
+ return offset;
}
optind++;
nr_iov = argc - optind;
buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab);
if (buf == NULL) {
- return;
+ return -EINVAL;
}
gettimeofday(&t1, NULL);
- cnt = do_aio_readv(blk, &qiov, offset, &total);
+ ret = do_aio_readv(blk, &qiov, offset, &total);
gettimeofday(&t2, NULL);
- if (cnt < 0) {
- printf("readv failed: %s\n", strerror(-cnt));
+ if (ret < 0) {
+ printf("readv failed: %s\n", strerror(-ret));
goto out;
}
+ cnt = ret;
+
+ ret = 0;
if (Pflag) {
void *cmp_buf = g_malloc(qiov.size);
@@ -900,6 +908,7 @@ static void readv_f(BlockBackend *blk, int argc, char **argv)
if (memcmp(buf, cmp_buf, qiov.size)) {
printf("Pattern verification failed at offset %"
PRId64 ", %zd bytes\n", offset, qiov.size);
+ ret = -EINVAL;
}
g_free(cmp_buf);
}
@@ -919,6 +928,7 @@ static void readv_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
+ return ret;
}
static void write_help(void)
@@ -944,7 +954,7 @@ static void write_help(void)
"\n");
}
-static void write_f(BlockBackend *blk, int argc, char **argv);
+static int write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t write_cmd = {
.name = "write",
@@ -958,13 +968,13 @@ static const cmdinfo_t write_cmd = {
.help = write_help,
};
-static void write_f(BlockBackend *blk, int argc, char **argv)
+static int write_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false, bflag = false;
bool Pflag = false, zflag = false, cflag = false;
int flags = 0;
- int c, cnt;
+ int c, cnt, ret;
char *buf = NULL;
int64_t offset;
int64_t count;
@@ -993,7 +1003,7 @@ static void write_f(BlockBackend *blk, int argc, char **argv)
Pflag = true;
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return;
+ return -EINVAL;
}
break;
case 'q':
@@ -1007,63 +1017,63 @@ static void write_f(BlockBackend *blk, int argc, char **argv)
break;
default:
qemuio_command_usage(&write_cmd);
- return;
+ return -EINVAL;
}
}
if (optind != argc - 2) {
qemuio_command_usage(&write_cmd);
- return;
+ return -EINVAL;
}
if (bflag && zflag) {
printf("-b and -z cannot be specified at the same time\n");
- return;
+ return -EINVAL;
}
if ((flags & BDRV_REQ_FUA) && (bflag || cflag)) {
printf("-f and -b or -c cannot be specified at the same time\n");
- return;
+ return -EINVAL;
}
if ((flags & BDRV_REQ_MAY_UNMAP) && !zflag) {
printf("-u requires -z to be specified\n");
- return;
+ return -EINVAL;
}
if (zflag && Pflag) {
printf("-z and -P cannot be specified at the same time\n");
- return;
+ return -EINVAL;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return;
+ return offset;
}
optind++;
count = cvtnum(argv[optind]);
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
- return;
+ return count;
} else if (count > BDRV_REQUEST_MAX_BYTES) {
printf("length cannot exceed %" PRIu64 ", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_BYTES, argv[optind]);
- return;
+ return -EINVAL;
}
if (bflag || cflag) {
if (!QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE)) {
printf("%" PRId64 " is not a sector-aligned value for 'offset'\n",
offset);
- return;
+ return -EINVAL;
}
if (!QEMU_IS_ALIGNED(count, BDRV_SECTOR_SIZE)) {
printf("%"PRId64" is not a sector-aligned value for 'count'\n",
count);
- return;
+ return -EINVAL;
}
}
@@ -1073,20 +1083,23 @@ static void write_f(BlockBackend *blk, int argc, char **argv)
gettimeofday(&t1, NULL);
if (bflag) {
- cnt = do_save_vmstate(blk, buf, offset, count, &total);
+ ret = do_save_vmstate(blk, buf, offset, count, &total);
} else if (zflag) {
- cnt = do_co_pwrite_zeroes(blk, offset, count, flags, &total);
+ ret = do_co_pwrite_zeroes(blk, offset, count, flags, &total);
} else if (cflag) {
- cnt = do_write_compressed(blk, buf, offset, count, &total);
+ ret = do_write_compressed(blk, buf, offset, count, &total);
} else {
- cnt = do_pwrite(blk, buf, offset, count, flags, &total);
+ ret = do_pwrite(blk, buf, offset, count, flags, &total);
}
gettimeofday(&t2, NULL);
- if (cnt < 0) {
- printf("write failed: %s\n", strerror(-cnt));
+ if (ret < 0) {
+ printf("write failed: %s\n", strerror(-ret));
goto out;
}
+ cnt = ret;
+
+ ret = 0;
if (qflag) {
goto out;
@@ -1100,6 +1113,7 @@ out:
if (!zflag) {
qemu_io_free(buf);
}
+ return ret;
}
static void
@@ -1121,7 +1135,7 @@ writev_help(void)
"\n");
}
-static void writev_f(BlockBackend *blk, int argc, char **argv);
+static int writev_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t writev_cmd = {
.name = "writev",
@@ -1134,12 +1148,12 @@ static const cmdinfo_t writev_cmd = {
.help = writev_help,
};
-static void writev_f(BlockBackend *blk, int argc, char **argv)
+static int writev_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false;
int flags = 0;
- int c, cnt;
+ int c, cnt, ret;
char *buf;
int64_t offset;
/* Some compilers get confused and warn if this is not initialized. */
@@ -1162,41 +1176,44 @@ static void writev_f(BlockBackend *blk, int argc, char **argv)
case 'P':
pattern = parse_pattern(optarg);
if (pattern < 0) {
- return;
+ return -EINVAL;
}
break;
default:
qemuio_command_usage(&writev_cmd);
- return;
+ return -EINVAL;
}
}
if (optind > argc - 2) {
qemuio_command_usage(&writev_cmd);
- return;
+ return -EINVAL;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return;
+ return offset;
}
optind++;
nr_iov = argc - optind;
buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern);
if (buf == NULL) {
- return;
+ return -EINVAL;
}
gettimeofday(&t1, NULL);
- cnt = do_aio_writev(blk, &qiov, offset, flags, &total);
+ ret = do_aio_writev(blk, &qiov, offset, flags, &total);
gettimeofday(&t2, NULL);
- if (cnt < 0) {
- printf("writev failed: %s\n", strerror(-cnt));
+ if (ret < 0) {
+ printf("writev failed: %s\n", strerror(-ret));
goto out;
}
+ cnt = ret;
+
+ ret = 0;
if (qflag) {
goto out;
@@ -1208,6 +1225,7 @@ static void writev_f(BlockBackend *blk, int argc, char **argv)
out:
qemu_iovec_destroy(&qiov);
qemu_io_free(buf);
+ return ret;
}
struct aio_ctx {
@@ -1314,6 +1332,9 @@ static void aio_read_help(void)
" standard output stream (with -v option) for subsequent inspection.\n"
" The read is performed asynchronously and the aio_flush command must be\n"
" used to ensure all outstanding aio requests have been completed.\n"
+" Note that due to its asynchronous nature, this command will be\n"
+" considered successful once the request is submitted, independently\n"
+" of potential I/O errors or pattern mismatches.\n"
" -C, -- report statistics in a machine parsable format\n"
" -P, -- use a pattern to verify read data\n"
" -i, -- treat request as invalid, for exercising stats\n"
@@ -1322,7 +1343,7 @@ static void aio_read_help(void)
"\n");
}
-static void aio_read_f(BlockBackend *blk, int argc, char **argv);
+static int aio_read_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_read_cmd = {
.name = "aio_read",
@@ -1334,7 +1355,7 @@ static const cmdinfo_t aio_read_cmd = {
.help = aio_read_help,
};
-static void aio_read_f(BlockBackend *blk, int argc, char **argv)
+static int aio_read_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
@@ -1350,14 +1371,14 @@ static void aio_read_f(BlockBackend *blk, int argc, char **argv)
ctx->pattern = parse_pattern(optarg);
if (ctx->pattern < 0) {
g_free(ctx);
- return;
+ return -EINVAL;
}
break;
case 'i':
printf("injecting invalid read request\n");
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
g_free(ctx);
- return;
+ return 0;
case 'q':
ctx->qflag = true;
break;
@@ -1367,21 +1388,22 @@ static void aio_read_f(BlockBackend *blk, int argc, char **argv)
default:
g_free(ctx);
qemuio_command_usage(&aio_read_cmd);
- return;
+ return -EINVAL;
}
}
if (optind > argc - 2) {
g_free(ctx);
qemuio_command_usage(&aio_read_cmd);
- return;
+ return -EINVAL;
}
ctx->offset = cvtnum(argv[optind]);
if (ctx->offset < 0) {
- print_cvtnum_err(ctx->offset, argv[optind]);
+ int ret = ctx->offset;
+ print_cvtnum_err(ret, argv[optind]);
g_free(ctx);
- return;
+ return ret;
}
optind++;
@@ -1390,13 +1412,14 @@ static void aio_read_f(BlockBackend *blk, int argc, char **argv)
if (ctx->buf == NULL) {
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ);
g_free(ctx);
- return;
+ return -EINVAL;
}
gettimeofday(&ctx->t1, NULL);
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_READ);
blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx);
+ return 0;
}
static void aio_write_help(void)
@@ -1413,6 +1436,9 @@ static void aio_write_help(void)
" filled with a set pattern (0xcdcdcdcd).\n"
" The write is performed asynchronously and the aio_flush command must be\n"
" used to ensure all outstanding aio requests have been completed.\n"
+" Note that due to its asynchronous nature, this command will be\n"
+" considered successful once the request is submitted, independently\n"
+" of potential I/O errors or pattern mismatches.\n"
" -P, -- use different pattern to fill file\n"
" -C, -- report statistics in a machine parsable format\n"
" -f, -- use Force Unit Access semantics\n"
@@ -1423,7 +1449,7 @@ static void aio_write_help(void)
"\n");
}
-static void aio_write_f(BlockBackend *blk, int argc, char **argv);
+static int aio_write_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t aio_write_cmd = {
.name = "aio_write",
@@ -1436,7 +1462,7 @@ static const cmdinfo_t aio_write_cmd = {
.help = aio_write_help,
};
-static void aio_write_f(BlockBackend *blk, int argc, char **argv)
+static int aio_write_f(BlockBackend *blk, int argc, char **argv)
{
int nr_iov, c;
int pattern = 0xcd;
@@ -1462,53 +1488,54 @@ static void aio_write_f(BlockBackend *blk, int argc, char **argv)
pattern = parse_pattern(optarg);
if (pattern < 0) {
g_free(ctx);
- return;
+ return -EINVAL;
}
break;
case 'i':
printf("injecting invalid write request\n");
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
g_free(ctx);
- return;
+ return 0;
case 'z':
ctx->zflag = true;
break;
default:
g_free(ctx);
qemuio_command_usage(&aio_write_cmd);
- return;
+ return -EINVAL;
}
}
if (optind > argc - 2) {
g_free(ctx);
qemuio_command_usage(&aio_write_cmd);
- return;
+ return -EINVAL;
}
if (ctx->zflag && optind != argc - 2) {
printf("-z supports only a single length parameter\n");
g_free(ctx);
- return;
+ return -EINVAL;
}
if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) {
printf("-u requires -z to be specified\n");
g_free(ctx);
- return;
+ return -EINVAL;
}
if (ctx->zflag && ctx->Pflag) {
printf("-z and -P cannot be specified at the same time\n");
g_free(ctx);
- return;
+ return -EINVAL;
}
ctx->offset = cvtnum(argv[optind]);
if (ctx->offset < 0) {
- print_cvtnum_err(ctx->offset, argv[optind]);
+ int ret = ctx->offset;
+ print_cvtnum_err(ret, argv[optind]);
g_free(ctx);
- return;
+ return ret;
}
optind++;
@@ -1517,7 +1544,7 @@ static void aio_write_f(BlockBackend *blk, int argc, char **argv)
if (count < 0) {
print_cvtnum_err(count, argv[optind]);
g_free(ctx);
- return;
+ return count;
}
ctx->qiov.size = count;
@@ -1530,7 +1557,7 @@ static void aio_write_f(BlockBackend *blk, int argc, char **argv)
if (ctx->buf == NULL) {
block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE);
g_free(ctx);
- return;
+ return -EINVAL;
}
gettimeofday(&ctx->t1, NULL);
@@ -1540,14 +1567,17 @@ static void aio_write_f(BlockBackend *blk, int argc, char **argv)
blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, flags, aio_write_done,
ctx);
}
+
+ return 0;
}
-static void aio_flush_f(BlockBackend *blk, int argc, char **argv)
+static int aio_flush_f(BlockBackend *blk, int argc, char **argv)
{
BlockAcctCookie cookie;
block_acct_start(blk_get_stats(blk), &cookie, 0, BLOCK_ACCT_FLUSH);
blk_drain_all();
block_acct_done(blk_get_stats(blk), &cookie);
+ return 0;
}
static const cmdinfo_t aio_flush_cmd = {
@@ -1556,9 +1586,9 @@ static const cmdinfo_t aio_flush_cmd = {
.oneline = "completes all outstanding aio requests"
};
-static void flush_f(BlockBackend *blk, int argc, char **argv)
+static int flush_f(BlockBackend *blk, int argc, char **argv)
{
- blk_flush(blk);
+ return blk_flush(blk);
}
static const cmdinfo_t flush_cmd = {
@@ -1568,7 +1598,7 @@ static const cmdinfo_t flush_cmd = {
.oneline = "flush all in-core file state to disk",
};
-static void truncate_f(BlockBackend *blk, int argc, char **argv)
+static int truncate_f(BlockBackend *blk, int argc, char **argv)
{
Error *local_err = NULL;
int64_t offset;
@@ -1577,14 +1607,16 @@ static void truncate_f(BlockBackend *blk, int argc, char **argv)
offset = cvtnum(argv[1]);
if (offset < 0) {
print_cvtnum_err(offset, argv[1]);
- return;
+ return offset;
}
ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
- return;
+ return ret;
}
+
+ return 0;
}
static const cmdinfo_t truncate_cmd = {
@@ -1598,7 +1630,7 @@ static const cmdinfo_t truncate_cmd = {
.oneline = "truncates the current file at the given offset",
};
-static void length_f(BlockBackend *blk, int argc, char **argv)
+static int length_f(BlockBackend *blk, int argc, char **argv)
{
int64_t size;
char s1[64];
@@ -1606,11 +1638,12 @@ static void length_f(BlockBackend *blk, int argc, char **argv)
size = blk_getlength(blk);
if (size < 0) {
printf("getlength: %s\n", strerror(-size));
- return;
+ return size;
}
cvtstr(size, s1, sizeof(s1));
printf("%s\n", s1);
+ return 0;
}
@@ -1622,7 +1655,7 @@ static const cmdinfo_t length_cmd = {
};
-static void info_f(BlockBackend *blk, int argc, char **argv)
+static int info_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
BlockDriverInfo bdi;
@@ -1639,7 +1672,7 @@ static void info_f(BlockBackend *blk, int argc, char **argv)
ret = bdrv_get_info(bs, &bdi);
if (ret) {
- return;
+ return ret;
}
cvtstr(bdi.cluster_size, s1, sizeof(s1));
@@ -1654,6 +1687,8 @@ static void info_f(BlockBackend *blk, int argc, char **argv)
bdrv_image_info_specific_dump(fprintf, stdout, spec_info);
qapi_free_ImageInfoSpecific(spec_info);
}
+
+ return 0;
}
@@ -1680,7 +1715,7 @@ static void discard_help(void)
"\n");
}
-static void discard_f(BlockBackend *blk, int argc, char **argv);
+static int discard_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t discard_cmd = {
.name = "discard",
@@ -1694,7 +1729,7 @@ static const cmdinfo_t discard_cmd = {
.help = discard_help,
};
-static void discard_f(BlockBackend *blk, int argc, char **argv)
+static int discard_f(BlockBackend *blk, int argc, char **argv)
{
struct timeval t1, t2;
bool Cflag = false, qflag = false;
@@ -1711,31 +1746,31 @@ static void discard_f(BlockBackend *blk, int argc, char **argv)
break;
default:
qemuio_command_usage(&discard_cmd);
- return;
+ return -EINVAL;
}
}
if (optind != argc - 2) {
qemuio_command_usage(&discard_cmd);
- return;
+ return -EINVAL;
}
offset = cvtnum(argv[optind]);
if (offset < 0) {
print_cvtnum_err(offset, argv[optind]);
- return;
+ return offset;
}
optind++;
bytes = cvtnum(argv[optind]);
if (bytes < 0) {
print_cvtnum_err(bytes, argv[optind]);
- return;
+ return bytes;
} else if (bytes >> BDRV_SECTOR_BITS > BDRV_REQUEST_MAX_SECTORS) {
printf("length cannot exceed %"PRIu64", given %s\n",
(uint64_t)BDRV_REQUEST_MAX_SECTORS << BDRV_SECTOR_BITS,
argv[optind]);
- return;
+ return -EINVAL;
}
gettimeofday(&t1, NULL);
@@ -1744,7 +1779,7 @@ static void discard_f(BlockBackend *blk, int argc, char **argv)
if (ret < 0) {
printf("discard failed: %s\n", strerror(-ret));
- return;
+ return ret;
}
/* Finally, report back -- -C gives a parsable format */
@@ -1752,9 +1787,11 @@ static void discard_f(BlockBackend *blk, int argc, char **argv)
t2 = tsub(t2, t1);
print_report("discard", &t2, offset, bytes, bytes, 1, Cflag);
}
+
+ return 0;
}
-static void alloc_f(BlockBackend *blk, int argc, char **argv)
+static int alloc_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
int64_t offset, start, remaining, count;
@@ -1765,14 +1802,14 @@ static void alloc_f(BlockBackend *blk, int argc, char **argv)
start = offset = cvtnum(argv[1]);
if (offset < 0) {
print_cvtnum_err(offset, argv[1]);
- return;
+ return offset;
}
if (argc == 3) {
count = cvtnum(argv[2]);
if (count < 0) {
print_cvtnum_err(count, argv[2]);
- return;
+ return count;
}
} else {
count = BDRV_SECTOR_SIZE;
@@ -1784,7 +1821,7 @@ static void alloc_f(BlockBackend *blk, int argc, char **argv)
ret = bdrv_is_allocated(bs, offset, remaining, &num);
if (ret < 0) {
printf("is_allocated failed: %s\n", strerror(-ret));
- return;
+ return ret;
}
offset += num;
remaining -= num;
@@ -1801,6 +1838,7 @@ static void alloc_f(BlockBackend *blk, int argc, char **argv)
printf("%"PRId64"/%"PRId64" bytes allocated at offset %s\n",
sum_alloc, count, s1);
+ return 0;
}
static const cmdinfo_t alloc_cmd = {
@@ -1846,7 +1884,7 @@ static int map_is_allocated(BlockDriverState *bs, int64_t offset,
return firstret;
}
-static void map_f(BlockBackend *blk, int argc, char **argv)
+static int map_f(BlockBackend *blk, int argc, char **argv)
{
int64_t offset, bytes;
char s1[64], s2[64];
@@ -1858,17 +1896,17 @@ static void map_f(BlockBackend *blk, int argc, char **argv)
bytes = blk_getlength(blk);
if (bytes < 0) {
error_report("Failed to query image length: %s", strerror(-bytes));
- return;
+ return bytes;
}
while (bytes) {
ret = map_is_allocated(blk_bs(blk), offset, bytes, &num);
if (ret < 0) {
error_report("Failed to get allocation status: %s", strerror(-ret));
- return;
+ return ret;
} else if (!num) {
error_report("Unexpected end of image");
- return;
+ return -EIO;
}
retstr = ret ? " allocated" : "not allocated";
@@ -1880,6 +1918,8 @@ static void map_f(BlockBackend *blk, int argc, char **argv)
offset += num;
bytes -= num;
}
+
+ return 0;
}
static const cmdinfo_t map_cmd = {
@@ -1907,7 +1947,7 @@ static void reopen_help(void)
"\n");
}
-static void reopen_f(BlockBackend *blk, int argc, char **argv);
+static int reopen_f(BlockBackend *blk, int argc, char **argv);
static QemuOptsList reopen_opts = {
.name = "reopen",
@@ -1929,7 +1969,7 @@ static const cmdinfo_t reopen_cmd = {
.help = reopen_help,
};
-static void reopen_f(BlockBackend *blk, int argc, char **argv)
+static int reopen_f(BlockBackend *blk, int argc, char **argv)
{
BlockDriverState *bs = blk_bs(blk);
QemuOpts *qopts;
@@ -1947,19 +1987,19 @@ static void reopen_f(BlockBackend *blk, int argc, char **argv)
case 'c':
if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
- return;
+ return -EINVAL;
}
break;
case 'o':
if (!qemu_opts_parse_noisily(&reopen_opts, optarg, 0)) {
qemu_opts_reset(&reopen_opts);
- return;
+ return -EINVAL;
}
break;
case 'r':
if (has_rw_option) {
error_report("Only one -r/-w option may be given");
- return;
+ return -EINVAL;
}
flags &= ~BDRV_O_RDWR;
has_rw_option = true;
@@ -1967,7 +2007,7 @@ static void reopen_f(BlockBackend *blk, int argc, char **argv)
case 'w':
if (has_rw_option) {
error_report("Only one -r/-w option may be given");
- return;
+ return -EINVAL;
}
flags |= BDRV_O_RDWR;
has_rw_option = true;
@@ -1975,14 +2015,14 @@ static void reopen_f(BlockBackend *blk, int argc, char **argv)
default:
qemu_opts_reset(&reopen_opts);
qemuio_command_usage(&reopen_cmd);
- return;
+ return -EINVAL;
}
}
if (optind != argc) {
qemu_opts_reset(&reopen_opts);
qemuio_command_usage(&reopen_cmd);
- return;
+ return -EINVAL;
}
if (writethrough != blk_enable_write_cache(blk) &&
@@ -1990,7 +2030,7 @@ static void reopen_f(BlockBackend *blk, int argc, char **argv)
{
error_report("Cannot change cache.writeback: Device attached");
qemu_opts_reset(&reopen_opts);
- return;
+ return -EBUSY;
}
if (!(flags & BDRV_O_RDWR)) {
@@ -2016,29 +2056,37 @@ static void reopen_f(BlockBackend *blk, int argc, char **argv)
if (local_err) {
error_report_err(local_err);
- } else {
- blk_set_enable_write_cache(blk, !writethrough);
+ return -EINVAL;
}
+
+ blk_set_enable_write_cache(blk, !writethrough);
+ return 0;
}
-static void break_f(BlockBackend *blk, int argc, char **argv)
+static int break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
ret = bdrv_debug_breakpoint(blk_bs(blk), argv[1], argv[2]);
if (ret < 0) {
printf("Could not set breakpoint: %s\n", strerror(-ret));
+ return ret;
}
+
+ return 0;
}
-static void remove_break_f(BlockBackend *blk, int argc, char **argv)
+static int remove_break_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
ret = bdrv_debug_remove_breakpoint(blk_bs(blk), argv[1]);
if (ret < 0) {
printf("Could not remove breakpoint %s: %s\n", argv[1], strerror(-ret));
+ return ret;
}
+
+ return 0;
}
static const cmdinfo_t break_cmd = {
@@ -2060,14 +2108,17 @@ static const cmdinfo_t remove_break_cmd = {
.oneline = "remove a breakpoint by tag",
};
-static void resume_f(BlockBackend *blk, int argc, char **argv)
+static int resume_f(BlockBackend *blk, int argc, char **argv)
{
int ret;
ret = bdrv_debug_resume(blk_bs(blk), argv[1]);
if (ret < 0) {
printf("Could not resume request: %s\n", strerror(-ret));
+ return ret;
}
+
+ return 0;
}
static const cmdinfo_t resume_cmd = {
@@ -2079,11 +2130,12 @@ static const cmdinfo_t resume_cmd = {
.oneline = "resumes the request tagged as tag",
};
-static void wait_break_f(BlockBackend *blk, int argc, char **argv)
+static int wait_break_f(BlockBackend *blk, int argc, char **argv)
{
while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
aio_poll(blk_get_aio_context(blk), true);
}
+ return 0;
}
static const cmdinfo_t wait_break_cmd = {
@@ -2095,7 +2147,7 @@ static const cmdinfo_t wait_break_cmd = {
.oneline = "waits for the suspension of a request",
};
-static void abort_f(BlockBackend *blk, int argc, char **argv)
+static int abort_f(BlockBackend *blk, int argc, char **argv)
{
abort();
}
@@ -2121,7 +2173,7 @@ static void sigraise_help(void)
"\n", SIGTERM);
}
-static void sigraise_f(BlockBackend *blk, int argc, char **argv);
+static int sigraise_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t sigraise_cmd = {
.name = "sigraise",
@@ -2134,16 +2186,16 @@ static const cmdinfo_t sigraise_cmd = {
.help = sigraise_help,
};
-static void sigraise_f(BlockBackend *blk, int argc, char **argv)
+static int sigraise_f(BlockBackend *blk, int argc, char **argv)
{
int64_t sig = cvtnum(argv[1]);
if (sig < 0) {
print_cvtnum_err(sig, argv[1]);
- return;
+ return sig;
} else if (sig > NSIG) {
printf("signal argument '%s' is too large to be a valid signal\n",
argv[1]);
- return;
+ return -EINVAL;
}
/* Using raise() to kill this process does not necessarily flush all open
@@ -2153,6 +2205,8 @@ static void sigraise_f(BlockBackend *blk, int argc, char **argv)
fflush(stderr);
raise(sig);
+
+ return 0;
}
static void sleep_cb(void *opaque)
@@ -2161,7 +2215,7 @@ static void sleep_cb(void *opaque)
*expired = true;
}
-static void sleep_f(BlockBackend *blk, int argc, char **argv)
+static int sleep_f(BlockBackend *blk, int argc, char **argv)
{
char *endptr;
long ms;
@@ -2171,7 +2225,7 @@ static void sleep_f(BlockBackend *blk, int argc, char **argv)
ms = strtol(argv[1], &endptr, 0);
if (ms < 0 || *endptr != '\0') {
printf("%s is not a valid number\n", argv[1]);
- return;
+ return -EINVAL;
}
timer = timer_new_ns(QEMU_CLOCK_HOST, sleep_cb, &expired);
@@ -2182,6 +2236,7 @@ static void sleep_f(BlockBackend *blk, int argc, char **argv)
}
timer_free(timer);
+ return 0;
}
static const cmdinfo_t sleep_cmd = {
@@ -2228,22 +2283,23 @@ static void help_all(void)
printf("\nUse 'help commandname' for extended help.\n");
}
-static void help_f(BlockBackend *blk, int argc, char **argv)
+static int help_f(BlockBackend *blk, int argc, char **argv)
{
const cmdinfo_t *ct;
if (argc == 1) {
help_all();
- return;
+ return 0;
}
ct = find_command(argv[1]);
if (ct == NULL) {
printf("command %s not found\n", argv[1]);
- return;
+ return -EINVAL;
}
help_onecmd(argv[1], ct);
+ return 0;
}
static const cmdinfo_t help_cmd = {
@@ -2257,13 +2313,14 @@ static const cmdinfo_t help_cmd = {
.oneline = "help for one or all commands",
};
-void qemuio_command(BlockBackend *blk, const char *cmd)
+int qemuio_command(BlockBackend *blk, const char *cmd)
{
AioContext *ctx;
char *input;
const cmdinfo_t *ct;
char **v;
int c;
+ int ret = 0;
input = g_strdup(cmd);
v = breakline(input, &c);
@@ -2272,14 +2329,17 @@ void qemuio_command(BlockBackend *blk, const char *cmd)
if (ct) {
ctx = blk ? blk_get_aio_context(blk) : qemu_get_aio_context();
aio_context_acquire(ctx);
- command(blk, ct, c, v);
+ ret = command(blk, ct, c, v);
aio_context_release(ctx);
} else {
fprintf(stderr, "command \"%s\" not found\n", v[0]);
+ ret = -EINVAL;
}
}
g_free(input);
g_free(v);
+
+ return ret;
}
static void __attribute((constructor)) init_qemuio_commands(void)
diff --git a/qemu-io.c b/qemu-io.c
index 02a67c929a..ec6683803f 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -66,10 +66,11 @@ static int get_eof_char(void)
#endif
}
-static void close_f(BlockBackend *blk, int argc, char **argv)
+static int close_f(BlockBackend *blk, int argc, char **argv)
{
blk_unref(qemuio_blk);
qemuio_blk = NULL;
+ return 0;
}
static const cmdinfo_t close_cmd = {
@@ -136,7 +137,7 @@ static void open_help(void)
"\n");
}
-static void open_f(BlockBackend *blk, int argc, char **argv);
+static int open_f(BlockBackend *blk, int argc, char **argv);
static const cmdinfo_t open_cmd = {
.name = "open",
@@ -160,12 +161,13 @@ static QemuOptsList empty_opts = {
},
};
-static void open_f(BlockBackend *blk, int argc, char **argv)
+static int open_f(BlockBackend *blk, int argc, char **argv)
{
int flags = BDRV_O_UNMAP;
int readonly = 0;
bool writethrough = true;
int c;
+ int ret;
QemuOpts *qopts;
QDict *opts;
bool force_share = false;
@@ -192,25 +194,25 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
qemu_opts_reset(&empty_opts);
- return;
+ return -EINVAL;
}
break;
case 'd':
if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
error_report("Invalid discard option: %s", optarg);
qemu_opts_reset(&empty_opts);
- return;
+ return -EINVAL;
}
break;
case 'o':
if (imageOpts) {
printf("--image-opts and 'open -o' are mutually exclusive\n");
qemu_opts_reset(&empty_opts);
- return;
+ return -EINVAL;
}
if (!qemu_opts_parse_noisily(&empty_opts, optarg, false)) {
qemu_opts_reset(&empty_opts);
- return;
+ return -EINVAL;
}
break;
case 'U':
@@ -219,7 +221,7 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
default:
qemu_opts_reset(&empty_opts);
qemuio_command_usage(&open_cmd);
- return;
+ return -EINVAL;
}
}
@@ -230,7 +232,7 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
if (imageOpts && (optind == argc - 1)) {
if (!qemu_opts_parse_noisily(&empty_opts, argv[optind], false)) {
qemu_opts_reset(&empty_opts);
- return;
+ return -EINVAL;
}
optind++;
}
@@ -240,18 +242,26 @@ static void open_f(BlockBackend *blk, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- openfile(argv[optind], flags, writethrough, force_share, opts);
+ ret = openfile(argv[optind], flags, writethrough, force_share, opts);
} else if (optind == argc) {
- openfile(NULL, flags, writethrough, force_share, opts);
+ ret = openfile(NULL, flags, writethrough, force_share, opts);
} else {
qobject_unref(opts);
qemuio_command_usage(&open_cmd);
+ return -EINVAL;
+ }
+
+ if (ret) {
+ return -EINVAL;
}
+
+ return 0;
}
-static void quit_f(BlockBackend *blk, int argc, char **argv)
+static int quit_f(BlockBackend *blk, int argc, char **argv)
{
quit_qemu_io = true;
+ return 0;
}
static const cmdinfo_t quit_cmd = {
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 15/29] qemu-io: Exit with error when a command failed
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (13 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 14/29] qemu-io: Let command functions return error code Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 16/29] iotests.py: Add qemu_io_silent Max Reitz
` (14 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Currently, qemu-io basically always returns success when it gets to
interactive mode (so once the whole command line has been parsed; even
before the commands on the command line are interpreted). That is not
very useful.
This patch makes qemu-io return failure when any of the executed
commands failed.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1519617
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509194302.21585-4-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-io.c | 28 ++++++++++++++++++++++------
1 file changed, 22 insertions(+), 6 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index ec6683803f..13829f5e21 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -400,17 +400,21 @@ static void prep_fetchline(void *opaque)
*fetchable= 1;
}
-static void command_loop(void)
+static int command_loop(void)
{
int i, fetchable = 0, prompted = 0;
+ int ret, last_error = 0;
char *input;
for (i = 0; !quit_qemu_io && i < ncmdline; i++) {
- qemuio_command(qemuio_blk, cmdline[i]);
+ ret = qemuio_command(qemuio_blk, cmdline[i]);
+ if (ret < 0) {
+ last_error = ret;
+ }
}
if (cmdline) {
g_free(cmdline);
- return;
+ return last_error;
}
while (!quit_qemu_io) {
@@ -431,13 +435,19 @@ static void command_loop(void)
if (input == NULL) {
break;
}
- qemuio_command(qemuio_blk, input);
+ ret = qemuio_command(qemuio_blk, input);
g_free(input);
+ if (ret < 0) {
+ last_error = ret;
+ }
+
prompted = 0;
fetchable = 0;
}
qemu_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL);
+
+ return last_error;
}
static void add_user_command(char *optarg)
@@ -502,6 +512,7 @@ int main(int argc, char **argv)
int c;
int opt_index = 0;
int flags = BDRV_O_UNMAP;
+ int ret;
bool writethrough = true;
Error *local_error = NULL;
QDict *opts = NULL;
@@ -663,7 +674,7 @@ int main(int argc, char **argv)
}
}
}
- command_loop();
+ ret = command_loop();
/*
* Make sure all outstanding requests complete before the program exits.
@@ -672,5 +683,10 @@ int main(int argc, char **argv)
blk_unref(qemuio_blk);
g_free(readline_state);
- return 0;
+
+ if (ret < 0) {
+ return 1;
+ } else {
+ return 0;
+ }
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 16/29] iotests.py: Add qemu_io_silent
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (14 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 15/29] qemu-io: Exit with error when a command failed Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code Max Reitz
` (13 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
With qemu-io now returning a useful exit code, some tests may find it
sufficient to just query that instead of logging (and filtering) the
whole output.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509194302.21585-5-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index fdbdd8b300..0b204dc220 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -133,6 +133,15 @@ def qemu_io(*args):
sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
return subp.communicate()[0]
+def qemu_io_silent(*args):
+ '''Run qemu-io and return the exit code, suppressing stdout'''
+ args = qemu_io_args + list(args)
+ exitcode = subprocess.call(args, stdout=open('/dev/null', 'w'))
+ if exitcode < 0:
+ sys.stderr.write('qemu-io received signal %i: %s\n' %
+ (-exitcode, ' '.join(args)))
+ return exitcode
+
class QemuIoInteractive:
def __init__(self, *args):
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (15 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 16/29] iotests.py: Add qemu_io_silent Max Reitz
@ 2018-06-11 14:25 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 18/29] qemu-img: Resolve relative backing paths in rebase Max Reitz
` (12 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:25 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
As a showcase of how you can use qemu-io's exit code to determine
success or failure (same for qemu-img), this test is changed to use
qemu_io_silent() instead of qemu_io(), and to assert the exit code
instead of logging the filtered result.
One real advantage of this is that in case of an error, you get a
backtrace that helps you locate the issue in the test file quickly.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509194302.21585-6-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/216 | 23 ++++++++++++-----------
tests/qemu-iotests/216.out | 17 ++---------------
2 files changed, 14 insertions(+), 26 deletions(-)
diff --git a/tests/qemu-iotests/216 b/tests/qemu-iotests/216
index ca9b47a7fd..3c0ae54b44 100755
--- a/tests/qemu-iotests/216
+++ b/tests/qemu-iotests/216
@@ -20,7 +20,7 @@
# Creator/Owner: Max Reitz <mreitz@redhat.com>
import iotests
-from iotests import log, qemu_img_pipe, qemu_io, filter_qemu_io
+from iotests import log, qemu_img, qemu_io_silent
# Need backing file support
iotests.verify_image_format(supported_fmts=['qcow2', 'qcow', 'qed', 'vmdk'])
@@ -50,14 +50,13 @@ with iotests.FilePath('base.img') as base_img_path, \
log('--- Setting up images ---')
log('')
- qemu_img_pipe('create', '-f', iotests.imgfmt, base_img_path, '64M')
+ assert qemu_img('create', '-f', iotests.imgfmt, base_img_path, '64M') == 0
+ assert qemu_io_silent(base_img_path, '-c', 'write -P 1 0M 1M') == 0
+ assert qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
+ top_img_path) == 0
+ assert qemu_io_silent(top_img_path, '-c', 'write -P 2 1M 1M') == 0
- log(filter_qemu_io(qemu_io(base_img_path, '-c', 'write -P 1 0M 1M')))
-
- qemu_img_pipe('create', '-f', iotests.imgfmt, '-b', base_img_path,
- top_img_path)
-
- log(filter_qemu_io(qemu_io(top_img_path, '-c', 'write -P 2 1M 1M')))
+ log('Done')
log('')
log('--- Doing COR ---')
@@ -110,6 +109,8 @@ with iotests.FilePath('base.img') as base_img_path, \
log('--- Checking COR result ---')
log('')
- log(filter_qemu_io(qemu_io(base_img_path, '-c', 'discard 0 64M')))
- log(filter_qemu_io(qemu_io(top_img_path, '-c', 'read -P 1 0M 1M')))
- log(filter_qemu_io(qemu_io(top_img_path, '-c', 'read -P 2 1M 1M')))
+ assert qemu_io_silent(base_img_path, '-c', 'discard 0 64M') == 0
+ assert qemu_io_silent(top_img_path, '-c', 'read -P 1 0M 1M') == 0
+ assert qemu_io_silent(top_img_path, '-c', 'read -P 2 1M 1M') == 0
+
+ log('Done')
diff --git a/tests/qemu-iotests/216.out b/tests/qemu-iotests/216.out
index d3fc590d29..45ea857ee1 100644
--- a/tests/qemu-iotests/216.out
+++ b/tests/qemu-iotests/216.out
@@ -3,12 +3,7 @@
--- Setting up images ---
-wrote 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-wrote 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
--- Doing COR ---
@@ -17,12 +12,4 @@ wrote 1048576/1048576 bytes at offset 1048576
--- Checking COR result ---
-discard 67108864/67108864 bytes at offset 0
-64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 0
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-read 1048576/1048576 bytes at offset 1048576
-1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
+Done
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 18/29] qemu-img: Resolve relative backing paths in rebase
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (16 preceding siblings ...)
2018-06-11 14:25 ` [Qemu-devel] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 19/29] iotests: Add test for rebasing with relative paths Max Reitz
` (11 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Currently, rebase interprets a relative path for the new backing image
as follows:
(1) Open the new backing image with the given relative path (thus relative to
qemu-img's working directory).
(2) Write it directly into the overlay's backing path field (thus
relative to the overlay).
If the overlay is not in qemu-img's working directory, both will be
different interpretations, which may either lead to an error somewhere
(either rebase fails because it cannot open the new backing image, or
your overlay becomes unusable because its backing path does not point to
a file), or, even worse, it may result in your rebase being performed
for a different backing file than what your overlay will point to after
the rebase.
Fix this by interpreting the target backing path as relative to the
overlay, like qemu-img does everywhere else.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1569835
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180509182002.8044-2-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index b1c1e484d2..ebe1b866da 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3251,6 +3251,9 @@ static int img_rebase(int argc, char **argv)
}
if (out_baseimg[0]) {
+ const char *overlay_filename;
+ char *out_real_path;
+
options = qdict_new();
if (out_basefmt) {
qdict_put_str(options, "driver", out_basefmt);
@@ -3259,8 +3262,26 @@ static int img_rebase(int argc, char **argv)
qdict_put_bool(options, BDRV_OPT_FORCE_SHARE, true);
}
- blk_new_backing = blk_new_open(out_baseimg, NULL,
+ overlay_filename = bs->exact_filename[0] ? bs->exact_filename
+ : bs->filename;
+ out_real_path = g_malloc(PATH_MAX);
+
+ bdrv_get_full_backing_filename_from_filename(overlay_filename,
+ out_baseimg,
+ out_real_path,
+ PATH_MAX,
+ &local_err);
+ if (local_err) {
+ error_reportf_err(local_err,
+ "Could not resolve backing filename: ");
+ ret = -1;
+ g_free(out_real_path);
+ goto out;
+ }
+
+ blk_new_backing = blk_new_open(out_real_path, NULL,
options, src_flags, &local_err);
+ g_free(out_real_path);
if (!blk_new_backing) {
error_reportf_err(local_err,
"Could not open new backing file '%s': ",
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 19/29] iotests: Add test for rebasing with relative paths
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (17 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 18/29] qemu-img: Resolve relative backing paths in rebase Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 20/29] qemu-img: Special post-backing convert handling Max Reitz
` (10 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 20180509182002.8044-3-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/024 | 82 ++++++++++++++++++++++++++++++++++++--
tests/qemu-iotests/024.out | 30 ++++++++++++++
2 files changed, 109 insertions(+), 3 deletions(-)
diff --git a/tests/qemu-iotests/024 b/tests/qemu-iotests/024
index e0d77ce2f5..4071ed6093 100755
--- a/tests/qemu-iotests/024
+++ b/tests/qemu-iotests/024
@@ -29,9 +29,14 @@ status=1 # failure is the default!
_cleanup()
{
- _cleanup_test_img
- rm -f "$TEST_DIR/t.$IMGFMT.base_old"
- rm -f "$TEST_DIR/t.$IMGFMT.base_new"
+ _cleanup_test_img
+ rm -f "$TEST_DIR/t.$IMGFMT.base_old"
+ rm -f "$TEST_DIR/t.$IMGFMT.base_new"
+
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT"
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT.base_old"
+ rm -f "$TEST_DIR/subdir/t.$IMGFMT.base_new"
+ rmdir "$TEST_DIR/subdir" 2> /dev/null
}
trap "_cleanup; exit \$status" 0 1 2 3 15
@@ -123,6 +128,77 @@ io_pattern readv $((13 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
io_pattern readv $((14 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x11
io_pattern readv $((15 * CLUSTER_SIZE)) $CLUSTER_SIZE 0 1 0x00
+echo
+echo "=== Test rebase in a subdirectory of the working directory ==="
+echo
+
+# Clean up the old images beforehand so they do not interfere with
+# this test
+_cleanup
+
+mkdir "$TEST_DIR/subdir"
+
+# Relative to the overlay
+BASE_OLD_OREL="t.$IMGFMT.base_old"
+BASE_NEW_OREL="t.$IMGFMT.base_new"
+
+# Relative to $TEST_DIR (which is going to be our working directory)
+OVERLAY_WREL="subdir/t.$IMGFMT"
+
+BASE_OLD="$TEST_DIR/subdir/$BASE_OLD_OREL"
+BASE_NEW="$TEST_DIR/subdir/$BASE_NEW_OREL"
+OVERLAY="$TEST_DIR/$OVERLAY_WREL"
+
+# Test done here:
+#
+# Backing (old): 11 11 -- 11
+# Backing (new): -- 22 22 11
+# Overlay: -- -- -- --
+#
+# Rebasing works, we have verified that above. Here, we just want to
+# see that rebasing is done for the correct target backing file.
+
+TEST_IMG=$BASE_OLD _make_test_img 1M
+TEST_IMG=$BASE_NEW _make_test_img 1M
+TEST_IMG=$OVERLAY _make_test_img -b "$BASE_OLD_OREL" 1M
+
+echo
+
+$QEMU_IO "$BASE_OLD" \
+ -c "write -P 0x11 $((0 * CLUSTER_SIZE)) $((2 * CLUSTER_SIZE))" \
+ -c "write -P 0x11 $((3 * CLUSTER_SIZE)) $((1 * CLUSTER_SIZE))" \
+ | _filter_qemu_io
+
+$QEMU_IO "$BASE_NEW" \
+ -c "write -P 0x22 $((1 * CLUSTER_SIZE)) $((2 * CLUSTER_SIZE))" \
+ -c "write -P 0x11 $((3 * CLUSTER_SIZE)) $((1 * CLUSTER_SIZE))" \
+ | _filter_qemu_io
+
+echo
+
+pushd "$TEST_DIR" >/dev/null
+$QEMU_IMG rebase -f "$IMGFMT" -b "$BASE_NEW_OREL" "$OVERLAY_WREL"
+popd >/dev/null
+
+# Verify the backing path is correct
+TEST_IMG=$OVERLAY _img_info | grep '^backing file'
+
+echo
+
+# Verify the data is correct
+$QEMU_IO "$OVERLAY" \
+ -c "read -P 0x11 $((0 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x11 $((1 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x00 $((2 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ -c "read -P 0x11 $((3 * CLUSTER_SIZE)) $CLUSTER_SIZE" \
+ | _filter_qemu_io
+
+echo
+
+# Verify that cluster #3 is not allocated (because it is the same in
+# $BASE_OLD and $BASE_NEW)
+$QEMU_IMG map "$OVERLAY" | _filter_qemu_img_map
+
# success, all done
echo "*** done"
diff --git a/tests/qemu-iotests/024.out b/tests/qemu-iotests/024.out
index 33cfaf5cfc..024dc786b3 100644
--- a/tests/qemu-iotests/024.out
+++ b/tests/qemu-iotests/024.out
@@ -141,4 +141,34 @@ read 65536/65536 bytes at offset 917504
=== IO: pattern 0x00
read 65536/65536 bytes at offset 983040
64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Test rebase in a subdirectory of the working directory ===
+
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_old', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/subdir/t.IMGFMT.base_new', fmt=IMGFMT size=1048576
+Formatting 'TEST_DIR/subdir/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=t.IMGFMT.base_old
+
+wrote 131072/131072 bytes at offset 0
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 131072/131072 bytes at offset 65536
+128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+backing file: t.IMGFMT.base_new (actual path: TEST_DIR/subdir/t.IMGFMT.base_new)
+
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 65536
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 131072
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 65536/65536 bytes at offset 196608
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+Offset Length File
+0 0x30000 TEST_DIR/subdir/t.IMGFMT
+0x30000 0x10000 TEST_DIR/subdir/t.IMGFMT.base_new
*** done
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 20/29] qemu-img: Special post-backing convert handling
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (18 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 19/29] iotests: Add test for rebasing with relative paths Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 21/29] iotests: Test post-backing convert target behavior Max Reitz
` (9 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Currently, qemu-img convert writes zeroes when it reads zeroes.
Sometimes it does not because the target is initialized to zeroes
anyway, so we do not need to overwrite (and thus potentially allocate)
it. This is never the case for targets with backing files, though. But
even they may have an area that is initialized to zeroes, and that is
the area past the end of the backing file (if that is shorter than the
overlay).
So if the target format's unallocated blocks are zero and there is a gap
between the target's backing file's end and the target's end, we do not
have to explicitly write zeroes there.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1527898
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180501165750.19242-2-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/qemu-img.c b/qemu-img.c
index ebe1b866da..ae4acb655b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1556,7 +1556,9 @@ typedef struct ImgConvertState {
BlockBackend *target;
bool has_zero_init;
bool compressed;
+ bool unallocated_blocks_are_zero;
bool target_has_backing;
+ int64_t target_backing_sectors; /* negative if unknown */
bool wr_in_order;
bool copy_range;
int min_sparse;
@@ -1586,12 +1588,23 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
{
int64_t src_cur_offset;
int ret, n, src_cur;
+ bool post_backing_zero = false;
convert_select_part(s, sector_num, &src_cur, &src_cur_offset);
assert(s->total_sectors > sector_num);
n = MIN(s->total_sectors - sector_num, BDRV_REQUEST_MAX_SECTORS);
+ if (s->target_backing_sectors >= 0) {
+ if (sector_num >= s->target_backing_sectors) {
+ post_backing_zero = s->unallocated_blocks_are_zero;
+ } else if (sector_num + n > s->target_backing_sectors) {
+ /* Split requests around target_backing_sectors (because
+ * starting from there, zeros are handled differently) */
+ n = s->target_backing_sectors - sector_num;
+ }
+ }
+
if (s->sector_next_status <= sector_num) {
int64_t count = n * BDRV_SECTOR_SIZE;
@@ -1613,7 +1626,7 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num)
n = DIV_ROUND_UP(count, BDRV_SECTOR_SIZE);
if (ret & BDRV_BLOCK_ZERO) {
- s->status = BLK_ZERO;
+ s->status = post_backing_zero ? BLK_BACKING_FILE : BLK_ZERO;
} else if (ret & BDRV_BLOCK_DATA) {
s->status = BLK_DATA;
} else {
@@ -2379,6 +2392,16 @@ static int img_convert(int argc, char **argv)
}
}
+ if (s.target_has_backing) {
+ /* Errors are treated as "backing length unknown" (which means
+ * s.target_backing_sectors has to be negative, which it will
+ * be automatically). The backing file length is used only
+ * for optimizations, so such a case is not fatal. */
+ s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs);
+ } else {
+ s.target_backing_sectors = -1;
+ }
+
ret = bdrv_get_info(out_bs, &bdi);
if (ret < 0) {
if (s.compressed) {
@@ -2388,6 +2411,7 @@ static int img_convert(int argc, char **argv)
} else {
s.compressed = s.compressed || bdi.needs_compressed_writes;
s.cluster_sectors = bdi.cluster_size / BDRV_SECTOR_SIZE;
+ s.unallocated_blocks_are_zero = bdi.unallocated_blocks_are_zero;
}
ret = convert_do_copy(&s);
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 21/29] iotests: Test post-backing convert target behavior
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (19 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 20/29] qemu-img: Special post-backing convert handling Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 22/29] iotests: improve pause_job Max Reitz
` (8 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This adds a test case to 122 for what happens when you convert to a
target with a backing file that is shorter than the target, and the
image format does not support efficient zero writes (as is the case with
qcow2 v2).
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180501165750.19242-3-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/122 | 42 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/122.out | 18 ++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/tests/qemu-iotests/122 b/tests/qemu-iotests/122
index 45b359c2ba..d8c8ad722d 100755
--- a/tests/qemu-iotests/122
+++ b/tests/qemu-iotests/122
@@ -76,6 +76,48 @@ $QEMU_IMG convert -O $IMGFMT -c -B "$TEST_IMG".base "$TEST_IMG" "$TEST_IMG".orig
$QEMU_IO -c "read -P 0 0 3M" "$TEST_IMG".orig 2>&1 | _filter_qemu_io | _filter_testdir
+echo
+echo "=== Converting to an overlay larger than its backing file ==="
+echo
+
+TEST_IMG="$TEST_IMG".base _make_test_img 256M
+# Needs to be at least how much an L2 table covers
+# (64 kB/entry * 64 kB / 8 B/entry = 512 MB)
+# That way, qcow2 will yield at least two status request responses.
+# With just a single response, it would always say "Allocated in the
+# backing file", so the optimization qemu-img convert tries to do is
+# done automatically. Once it has to be queried twice, however (and
+# one of the queries is completely after the end of the backing file),
+# the block layer will automatically add a ZERO flag that qemu-img
+# convert used to follow up with a zero write to the target.
+# We do not want such a zero write, however, because we are past the
+# end of the backing file on the target as well, so we do not need to
+# write anything there.
+_make_test_img -b "$TEST_IMG".base 768M
+
+# Use compat=0.10 as the output so there is no zero cluster support
+$QEMU_IMG convert -O $IMGFMT -B "$TEST_IMG".base -o compat=0.10 \
+ "$TEST_IMG" "$TEST_IMG".orig
+# See that nothing has been allocated past 64M
+$QEMU_IMG map "$TEST_IMG".orig | _filter_qemu_img_map
+
+echo
+
+# Just before the end of the backing file
+$QEMU_IO -c 'write -P 0x11 255M 1M' "$TEST_IMG".base 2>&1 | _filter_qemu_io
+# Somewhere in the second L2 table
+$QEMU_IO -c 'write -P 0x22 600M 1M' "$TEST_IMG" 2>&1 | _filter_qemu_io
+
+$QEMU_IMG convert -O $IMGFMT -B "$TEST_IMG".base -o compat=0.10 \
+ "$TEST_IMG" "$TEST_IMG".orig
+
+$QEMU_IMG map "$TEST_IMG".orig | _filter_qemu_img_map
+$QEMU_IO -c 'read -P 0x11 255M 1M' \
+ -c 'read -P 0x22 600M 1M' \
+ "$TEST_IMG".orig \
+ | _filter_qemu_io
+
+
echo
echo "=== Concatenate multiple source images ==="
echo
diff --git a/tests/qemu-iotests/122.out b/tests/qemu-iotests/122.out
index 47d8656db8..6c7ee1da6c 100644
--- a/tests/qemu-iotests/122.out
+++ b/tests/qemu-iotests/122.out
@@ -28,6 +28,24 @@ read 3145728/3145728 bytes at offset 0
read 3145728/3145728 bytes at offset 0
3 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+=== Converting to an overlay larger than its backing file ===
+
+Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=268435456
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=805306368 backing_file=TEST_DIR/t.IMGFMT.base
+Offset Length File
+
+wrote 1048576/1048576 bytes at offset 267386880
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 1048576/1048576 bytes at offset 629145600
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset Length File
+0xff00000 0x100000 TEST_DIR/t.IMGFMT.base
+0x25800000 0x100000 TEST_DIR/t.IMGFMT.orig
+read 1048576/1048576 bytes at offset 267386880
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1048576/1048576 bytes at offset 629145600
+1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
=== Concatenate multiple source images ===
Formatting 'TEST_DIR/t.IMGFMT.1', fmt=IMGFMT size=4194304
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 22/29] iotests: improve pause_job
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (20 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 21/29] iotests: Test post-backing convert target behavior Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 23/29] iotests: Fix 219's timing Max Reitz
` (7 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
It's possible, that job was finished during waiting. In this case we
will see error message "Timeout waiting for job to pause" which is not
very informative. So, let's check during waiting iteration that the job
exists.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180601115923.17159-1-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/iotests.py | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 0b204dc220..2f22fab2a7 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -590,9 +590,14 @@ class QMPTestCase(unittest.TestCase):
with Timeout(1, "Timeout waiting for job to pause"):
while True:
result = self.vm.qmp('query-block-jobs')
+ found = False
for job in result['return']:
- if job['device'] == job_id and job['paused'] == True and job['busy'] == False:
- return job
+ if job['device'] == job_id:
+ found = True
+ if job['paused'] == True and job['busy'] == False:
+ return job
+ break
+ assert found
def pause_job(self, job_id='job0', wait=True):
result = self.vm.qmp('block-job-pause', device=job_id)
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 23/29] iotests: Fix 219's timing
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (21 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 22/29] iotests: improve pause_job Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 24/29] qemu-img: Remove deprecated -s snapshot_id_or_name option Max Reitz
` (6 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
219 has two issues that may lead to sporadic failure, both of which are
the result of issuing query-jobs too early after a job has been
modified. This can then lead to different results based on whether the
modification has taken effect already or not.
First, query-jobs is issued right after the job has been created.
Besides its current progress possibly being in any random state (which
has already been taken care of), its total progress too is basically
arbitrary, because the job may not yet have been able to determine it.
This patch addresses this by just filtering the total progress, like
what has been done for the current progress already. However, for more
clarity, the filtering is changed to replace the values by a string
'FILTERED' instead of deleting them.
Secondly, query-jobs is issued right after a job has been resumed. The
job may or may not yet have had the time to actually perform any I/O,
and thus its current progress may or may not have advanced. To make
sure it has indeed advanced (which is what the reference output already
assumes), keep querying it until it has.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180606190628.8170-1-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/219 | 26 ++++++++++++++++++++------
tests/qemu-iotests/219.out | 10 +++++-----
2 files changed, 25 insertions(+), 11 deletions(-)
diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
index 898a26eef0..c03bbdb294 100755
--- a/tests/qemu-iotests/219
+++ b/tests/qemu-iotests/219
@@ -42,11 +42,24 @@ def test_pause_resume(vm):
iotests.log(vm.qmp(pause_cmd, **{pause_arg: 'job0'}))
pause_wait(vm, 'job0')
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
- iotests.log(vm.qmp('query-jobs'))
+ result = vm.qmp('query-jobs')
+ iotests.log(result)
+
+ old_progress = result['return'][0]['current-progress']
+ total_progress = result['return'][0]['total-progress']
iotests.log(vm.qmp(resume_cmd, **{resume_arg: 'job0'}))
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
- iotests.log(vm.qmp('query-jobs'))
+ if old_progress < total_progress:
+ # Wait for the job to advance
+ while result['return'][0]['current-progress'] == old_progress:
+ result = vm.qmp('query-jobs')
+ iotests.log(result)
+ else:
+ # Already reached the end, so the job cannot advance
+ # any further; therefore, the query-jobs result can be
+ # logged immediately
+ iotests.log(vm.qmp('query-jobs'))
def test_job_lifecycle(vm, job, job_args, has_ready=False):
iotests.log('')
@@ -58,12 +71,13 @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):
iotests.log(vm.qmp(job, job_id='job0', **job_args))
# Depending on the storage, the first request may or may not have completed
- # yet, so filter out the progress. Later query-job calls don't need the
- # filtering because the progress is made deterministic by the block job
- # speed
+ # yet (and the total progress may not have been fully determined yet), so
+ # filter out the progress. Later query-job calls don't need the filtering
+ # because the progress is made deterministic by the block job speed
result = vm.qmp('query-jobs')
for j in result['return']:
- del j['current-progress']
+ j['current-progress'] = 'FILTERED'
+ j['total-progress'] = 'FILTERED'
iotests.log(result)
# undefined -> created -> running
diff --git a/tests/qemu-iotests/219.out b/tests/qemu-iotests/219.out
index 346801b655..6dc07bc41e 100644
--- a/tests/qemu-iotests/219.out
+++ b/tests/qemu-iotests/219.out
@@ -3,7 +3,7 @@ Launching VM...
Starting block job: drive-mirror (auto-finalize: True; auto-dismiss: True)
{u'return': {}}
-{u'return': [{u'status': u'running', u'total-progress': 4194304, u'id': u'job0', u'type': u'mirror'}]}
+{u'return': [{u'status': u'running', u'current-progress': 'FILTERED', u'total-progress': 'FILTERED', u'id': u'job0', u'type': u'mirror'}]}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'created', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'running', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
@@ -93,7 +93,7 @@ Waiting for PENDING state...
Starting block job: drive-backup (auto-finalize: True; auto-dismiss: True)
{u'return': {}}
-{u'return': [{u'status': u'running', u'total-progress': 4194304, u'id': u'job0', u'type': u'backup'}]}
+{u'return': [{u'status': u'running', u'current-progress': 'FILTERED', u'total-progress': 'FILTERED', u'id': u'job0', u'type': u'backup'}]}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'created', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'running', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
@@ -144,7 +144,7 @@ Waiting for PENDING state...
Starting block job: drive-backup (auto-finalize: True; auto-dismiss: False)
{u'return': {}}
-{u'return': [{u'status': u'running', u'total-progress': 4194304, u'id': u'job0', u'type': u'backup'}]}
+{u'return': [{u'status': u'running', u'current-progress': 'FILTERED', u'total-progress': 'FILTERED', u'id': u'job0', u'type': u'backup'}]}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'created', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'running', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
@@ -203,7 +203,7 @@ Waiting for PENDING state...
Starting block job: drive-backup (auto-finalize: False; auto-dismiss: True)
{u'return': {}}
-{u'return': [{u'status': u'running', u'total-progress': 4194304, u'id': u'job0', u'type': u'backup'}]}
+{u'return': [{u'status': u'running', u'current-progress': 'FILTERED', u'total-progress': 'FILTERED', u'id': u'job0', u'type': u'backup'}]}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'created', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'running', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
@@ -262,7 +262,7 @@ Waiting for PENDING state...
Starting block job: drive-backup (auto-finalize: False; auto-dismiss: False)
{u'return': {}}
-{u'return': [{u'status': u'running', u'total-progress': 4194304, u'id': u'job0', u'type': u'backup'}]}
+{u'return': [{u'status': u'running', u'current-progress': 'FILTERED', u'total-progress': 'FILTERED', u'id': u'job0', u'type': u'backup'}]}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'created', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
{u'timestamp': {u'seconds': 'SECS', u'microseconds': 'USECS'}, u'data': {u'status': u'running', u'id': u'job0'}, u'event': u'JOB_STATUS_CHANGE'}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 24/29] qemu-img: Remove deprecated -s snapshot_id_or_name option
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (22 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 23/29] iotests: Fix 219's timing Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 25/29] block/qcow2-bitmap: fix free_bitmap_clusters Max Reitz
` (5 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
From: Thomas Huth <thuth@redhat.com>
It has been marked as deprecated since QEMU v2.0 already, so it
is time now to finally remove it.
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-id: 1528288551-31641-1-git-send-email-thuth@redhat.com
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 7 +------
qemu-doc.texi | 7 -------
qemu-img-cmds.hx | 4 ++--
qemu-img.texi | 7 ++-----
tests/qemu-iotests/029 | 2 +-
tests/qemu-iotests/080 | 4 ++--
6 files changed, 8 insertions(+), 23 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index ae4acb655b..1dcdd47254 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -148,8 +148,6 @@ static void QEMU_NORETURN help(void)
" 'snapshot_param' is param used for internal snapshot, format\n"
" is 'snapshot.id=[ID],snapshot.name=[NAME]', or\n"
" '[ID_OR_NAME]'\n"
- " 'snapshot_id_or_name' is deprecated, use 'snapshot_param'\n"
- " instead\n"
" '-c' indicates that target image must be compressed (qcow format only)\n"
" '-u' allows unsafe backing chains. For rebasing, it is assumed that old and\n"
" new backing file match exactly. The image doesn't need a working\n"
@@ -2018,7 +2016,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:co:s:l:S:pt:T:qnm:WU",
+ c = getopt_long(argc, argv, ":hf:O:B:co:l:S:pt:T:qnm:WU",
long_options, NULL);
if (c == -1) {
break;
@@ -2059,9 +2057,6 @@ static int img_convert(int argc, char **argv)
g_free(old_options);
}
break;
- case 's':
- snapshot_name = optarg;
- break;
case 'l':
if (strstart(optarg, SNAPSHOT_OPT_BASE, NULL)) {
sn_opts = qemu_opts_parse_noisily(&internal_snapshot_opts,
diff --git a/qemu-doc.texi b/qemu-doc.texi
index 2effe66d6b..9aff6b4ea9 100644
--- a/qemu-doc.texi
+++ b/qemu-doc.texi
@@ -2927,13 +2927,6 @@ Option @option{-virtioconsole} has been replaced by
The @code{-clock} option is ignored since QEMU version 1.7.0. There is no
replacement since it is not needed anymore.
-@section qemu-img command line arguments
-
-@subsection convert -s (since 2.0.0)
-
-The ``convert -s snapshot_id_or_name'' argument is obsoleted
-by the ``convert -l snapshot_param'' argument instead.
-
@section QEMU Machine Protocol (QMP) commands
@subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 3d2f7b26eb..69758fb6e8 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] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename")
+ "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-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}] [-s @var{snapshot_id_or_name}] [-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] @var{filename} [@var{filename2} [...]] @var{output_filename}
ETEXI
DEF("create", img_create,
diff --git a/qemu-img.texi b/qemu-img.texi
index 2be8206a05..aeb1b9e66c 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -61,9 +61,6 @@ by the used format or see the format descriptions below for details.
is param used for internal snapshot, format is
'snapshot.id=[ID],snapshot.name=[NAME]' or '[ID_OR_NAME]'
-@item snapshot_id_or_name
-is deprecated, use snapshot_param instead
-
@end table
@table @option
@@ -322,9 +319,9 @@ Error on reading data
@end table
-@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}] [-s @var{snapshot_id_or_name}] [-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] @var{filename} [@var{filename2} [...]] @var{output_filename}
-Convert the disk image @var{filename} or a snapshot @var{snapshot_param}(@var{snapshot_id_or_name} is deprecated)
+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}
option) or use any format specific options like encryption (@code{-o} option).
diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
index 30bab24dc0..5cff6875bf 100755
--- a/tests/qemu-iotests/029
+++ b/tests/qemu-iotests/029
@@ -92,7 +92,7 @@ _make_test_img 64M
{ $QEMU_IMG snapshot -c foo $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
poke_file "$TEST_IMG" "$offset_size" "\x00\x00\x00\x00\x00\x00\x02\x00"
poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
-{ $QEMU_IMG convert -s foo $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IMG convert -l foo $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_qemu_io | _filter_testdir
# success, all done
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 4dbe68e950..f0eb42f390 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -176,7 +176,7 @@ _make_test_img 64M
{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
poke_file "$TEST_IMG" "$offset_snap1_l1_offset" "\x00\x00\x00\x00\x00\x40\x02\x00"
-{ $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
+{ $QEMU_IMG convert -l test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
{ $QEMU_IMG amend -o compat=0.10 $TEST_IMG; } 2>&1 | _filter_testdir
{ $QEMU_IO -c "open -o overlap-check.inactive-l2=on $TEST_IMG" \
-c 'write 0 4k'; } 2>&1 | _filter_qemu_io | _filter_testdir
@@ -190,7 +190,7 @@ _make_test_img 64M
{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
poke_file "$TEST_IMG" "$offset_snap1_l1_size" "\x10\x00\x00\x00"
-{ $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
+{ $QEMU_IMG convert -l test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
{ $QEMU_IMG amend -o compat=0.10 $TEST_IMG; } 2>&1 | _filter_testdir
{ $QEMU_IO -c "open -o overlap-check.inactive-l2=on $TEST_IMG" \
-c 'write 0 4k'; } 2>&1 | _filter_qemu_io | _filter_testdir
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 25/29] block/qcow2-bitmap: fix free_bitmap_clusters
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (23 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 24/29] qemu-img: Remove deprecated -s snapshot_id_or_name option Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 26/29] throttle: Fix crash on reopen Max Reitz
` (4 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
This assert may fail, because bitmap_table is not initialized. Just
drop it, as it's obvious, that bitmap_table_load sets bitmap_table
parameter only when returning zero.
Reported-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20180608101225.2575-1-vsementsov@virtuozzo.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2-bitmap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c
index 60d5290f10..69485aa1de 100644
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@@ -254,7 +254,6 @@ static int free_bitmap_clusters(BlockDriverState *bs, Qcow2BitmapTable *tb)
ret = bitmap_table_load(bs, tb, &bitmap_table);
if (ret < 0) {
- assert(bitmap_table == NULL);
return ret;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 26/29] throttle: Fix crash on reopen
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (24 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 25/29] block/qcow2-bitmap: fix free_bitmap_clusters Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 27/29] block: Make bdrv_is_writable() public Max Reitz
` (3 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
From: Alberto Garcia <berto@igalia.com>
The throttle block filter can be reopened, and with this it is
possible to change the throttle group that the filter belongs to.
The way the code does that is the following:
- On throttle_reopen_prepare(): create a new ThrottleGroupMember
and attach it to the new throttle group.
- On throttle_reopen_commit(): detach the old ThrottleGroupMember,
delete it and replace it with the new one.
The problem with this is that by replacing the ThrottleGroupMember the
previous value of io_limits_disabled is lost, causing an assertion
failure in throttle_co_drain_end().
This problem can be reproduced by reopening a throttle node:
$QEMU -monitor stdio
-object throttle-group,id=tg0,x-iops-total=1000 \
-blockdev node-name=hd0,driver=qcow2,file.driver=file,file.filename=hd.qcow2 \
-blockdev node-name=root,driver=throttle,throttle-group=tg0,file=hd0,read-only=on
(qemu) block_stream root
block/throttle.c:214: throttle_co_drain_end: Assertion `tgm->io_limits_disabled' failed.
Since we only want to change the throttle group on reopen there's no
need to create a ThrottleGroupMember and discard the old one. It's
easier if we simply detach it from its current group and attach it to
the new one.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 20180608151536.7378-1-berto@igalia.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/throttle.c | 54 +++++++++++++++++++++++++++++-------------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/block/throttle.c b/block/throttle.c
index e298827f95..f617f23a12 100644
--- a/block/throttle.c
+++ b/block/throttle.c
@@ -36,9 +36,12 @@ static QemuOptsList throttle_opts = {
},
};
-static int throttle_configure_tgm(BlockDriverState *bs,
- ThrottleGroupMember *tgm,
- QDict *options, Error **errp)
+/*
+ * If this function succeeds then the throttle group name is stored in
+ * @group and must be freed by the caller.
+ * If there's an error then @group remains unmodified.
+ */
+static int throttle_parse_options(QDict *options, char **group, Error **errp)
{
int ret;
const char *group_name;
@@ -63,8 +66,7 @@ static int throttle_configure_tgm(BlockDriverState *bs,
goto fin;
}
- /* Register membership to group with name group_name */
- throttle_group_register_tgm(tgm, group_name, bdrv_get_aio_context(bs));
+ *group = g_strdup(group_name);
ret = 0;
fin:
qemu_opts_del(opts);
@@ -75,6 +77,8 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
int flags, Error **errp)
{
ThrottleGroupMember *tgm = bs->opaque;
+ char *group;
+ int ret;
bs->file = bdrv_open_child(NULL, options, "file", bs,
&child_file, false, errp);
@@ -86,7 +90,14 @@ static int throttle_open(BlockDriverState *bs, QDict *options,
bs->supported_zero_flags = bs->file->bs->supported_zero_flags |
BDRV_REQ_WRITE_UNCHANGED;
- return throttle_configure_tgm(bs, tgm, options, errp);
+ ret = throttle_parse_options(options, &group, errp);
+ if (ret == 0) {
+ /* Register membership to group with name group_name */
+ throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
+ g_free(group);
+ }
+
+ return ret;
}
static void throttle_close(BlockDriverState *bs)
@@ -162,35 +173,36 @@ static void throttle_attach_aio_context(BlockDriverState *bs,
static int throttle_reopen_prepare(BDRVReopenState *reopen_state,
BlockReopenQueue *queue, Error **errp)
{
- ThrottleGroupMember *tgm;
+ int ret;
+ char *group = NULL;
assert(reopen_state != NULL);
assert(reopen_state->bs != NULL);
- reopen_state->opaque = g_new0(ThrottleGroupMember, 1);
- tgm = reopen_state->opaque;
-
- return throttle_configure_tgm(reopen_state->bs, tgm, reopen_state->options,
- errp);
+ ret = throttle_parse_options(reopen_state->options, &group, errp);
+ reopen_state->opaque = group;
+ return ret;
}
static void throttle_reopen_commit(BDRVReopenState *reopen_state)
{
- ThrottleGroupMember *old_tgm = reopen_state->bs->opaque;
- ThrottleGroupMember *new_tgm = reopen_state->opaque;
+ BlockDriverState *bs = reopen_state->bs;
+ ThrottleGroupMember *tgm = bs->opaque;
+ char *group = reopen_state->opaque;
+
+ assert(group);
- throttle_group_unregister_tgm(old_tgm);
- g_free(old_tgm);
- reopen_state->bs->opaque = new_tgm;
+ if (strcmp(group, throttle_group_get_name(tgm))) {
+ throttle_group_unregister_tgm(tgm);
+ throttle_group_register_tgm(tgm, group, bdrv_get_aio_context(bs));
+ }
+ g_free(reopen_state->opaque);
reopen_state->opaque = NULL;
}
static void throttle_reopen_abort(BDRVReopenState *reopen_state)
{
- ThrottleGroupMember *tgm = reopen_state->opaque;
-
- throttle_group_unregister_tgm(tgm);
- g_free(tgm);
+ g_free(reopen_state->opaque);
reopen_state->opaque = NULL;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 27/29] block: Make bdrv_is_writable() public
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (25 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 26/29] throttle: Fix crash on reopen Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 28/29] qcow2: Do not mark inactive images corrupt Max Reitz
` (2 subsequent siblings)
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
This is a useful function for the whole block layer, so make it public.
At the same time, users outside of block.c probably do not need to make
use of the reopen functionality, so rename the current function to
bdrv_is_writable_after_reopen() create a new bdrv_is_writable() function
that just passes NULL to it for the reopen queue.
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180606193702.7113-2-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block.h | 1 +
block.c | 17 ++++++++++++++---
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 4dd4f1eab2..e677080c4e 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -408,6 +408,7 @@ bool bdrv_is_read_only(BlockDriverState *bs);
int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only,
bool ignore_allow_rdw, Error **errp);
int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp);
+bool bdrv_is_writable(BlockDriverState *bs);
bool bdrv_is_sg(BlockDriverState *bs);
bool bdrv_is_inserted(BlockDriverState *bs);
void bdrv_lock_medium(BlockDriverState *bs, bool locked);
diff --git a/block.c b/block.c
index 9d577f65bb..50887087f3 100644
--- a/block.c
+++ b/block.c
@@ -1620,13 +1620,24 @@ static int bdrv_reopen_get_flags(BlockReopenQueue *q, BlockDriverState *bs)
/* Returns whether the image file can be written to after the reopen queue @q
* has been successfully applied, or right now if @q is NULL. */
-static bool bdrv_is_writable(BlockDriverState *bs, BlockReopenQueue *q)
+static bool bdrv_is_writable_after_reopen(BlockDriverState *bs,
+ BlockReopenQueue *q)
{
int flags = bdrv_reopen_get_flags(q, bs);
return (flags & (BDRV_O_RDWR | BDRV_O_INACTIVE)) == BDRV_O_RDWR;
}
+/*
+ * Return whether the BDS can be written to. This is not necessarily
+ * the same as !bdrv_is_read_only(bs), as inactivated images may not
+ * be written to but do not count as read-only images.
+ */
+bool bdrv_is_writable(BlockDriverState *bs)
+{
+ return bdrv_is_writable_after_reopen(bs, NULL);
+}
+
static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs,
BdrvChild *c, const BdrvChildRole *role,
BlockReopenQueue *reopen_queue,
@@ -1664,7 +1675,7 @@ static int bdrv_check_perm(BlockDriverState *bs, BlockReopenQueue *q,
/* Write permissions never work with read-only images */
if ((cumulative_perms & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED)) &&
- !bdrv_is_writable(bs, q))
+ !bdrv_is_writable_after_reopen(bs, q))
{
error_setg(errp, "Block node is read-only");
return -EPERM;
@@ -1956,7 +1967,7 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
&perm, &shared);
/* Format drivers may touch metadata even if the guest doesn't write */
- if (bdrv_is_writable(bs, reopen_queue)) {
+ if (bdrv_is_writable_after_reopen(bs, reopen_queue)) {
perm |= BLK_PERM_WRITE | BLK_PERM_RESIZE;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 28/29] qcow2: Do not mark inactive images corrupt
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (26 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 27/29] block: Make bdrv_is_writable() public Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 29/29] iotests: Add case for a corrupted inactive image Max Reitz
2018-06-11 15:20 ` [Qemu-devel] [PULL 00/29] Block patches Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
When signaling a corruption on a read-only image, qcow2 already makes
fatal events non-fatal (i.e., they will not result in the image being
closed, and the image header's corrupt flag will not be set). This is
necessary because we cannot set the corrupt flag on read-only images,
and it is possible because further corruption of read-only images is
impossible.
Inactive images are effectively read-only, too, so we should do the same
for them. bdrv_is_writable() can tell us whether an image can actually
be written to, so use its result instead of !bs->read_only.
(Otherwise, the assert(!(bs->open_flags & BDRV_O_INACTIVE)) in
bdrv_co_pwritev() will fail, crashing qemu.)
Cc: qemu-stable@nongnu.org
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180606193702.7113-3-mreitz@redhat.com
Reviewed-by: John Snow <jsnow@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 6b2d88759d..6fa5e1d71a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -4569,7 +4569,7 @@ void qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
char *message;
va_list ap;
- fatal = fatal && !bs->read_only;
+ fatal = fatal && bdrv_is_writable(bs);
if (s->signaled_corruption &&
(!fatal || (s->incompatible_features & QCOW2_INCOMPAT_CORRUPT)))
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [Qemu-devel] [PULL 29/29] iotests: Add case for a corrupted inactive image
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (27 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 28/29] qcow2: Do not mark inactive images corrupt Max Reitz
@ 2018-06-11 14:26 ` Max Reitz
2018-06-11 15:20 ` [Qemu-devel] [PULL 00/29] Block patches Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Max Reitz @ 2018-06-11 14:26 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf, Peter Maydell
Reviewed-by: John Snow <jsnow@redhat.com>
Tested-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20180606193702.7113-4-mreitz@redhat.com
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/060 | 30 ++++++++++++++++++++++++++++++
tests/qemu-iotests/060.out | 14 ++++++++++++++
2 files changed, 44 insertions(+)
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 6c7407f499..7bdf609f3f 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -440,6 +440,36 @@ echo "{'execute': 'qmp_capabilities'}
-drive if=none,node-name=drive,file="$TEST_IMG",driver=qcow2 \
| _filter_qmp | _filter_qemu_io
+echo
+echo "=== Testing incoming inactive corrupted image ==="
+echo
+
+_make_test_img 64M
+# Create an unaligned L1 entry, so qemu will signal a corruption when
+# reading from the covered area
+poke_file "$TEST_IMG" "$l1_offset" "\x00\x00\x00\x00\x2a\x2a\x2a\x2a"
+
+# Inactive images are effectively read-only images, so this should be a
+# non-fatal corruption (which does not modify the image)
+echo "{'execute': 'qmp_capabilities'}
+ {'execute': 'human-monitor-command',
+ 'arguments': {'command-line': 'qemu-io drive \"read 0 512\"'}}
+ {'execute': 'quit'}" \
+ | $QEMU -qmp stdio -nographic -nodefaults \
+ -blockdev "{'node-name': 'drive',
+ 'driver': 'qcow2',
+ 'file': {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG'
+ }}" \
+ -incoming exec:'cat /dev/null' \
+ 2>&1 \
+ | _filter_qmp | _filter_qemu_io
+
+echo
+# Image should not have been marked corrupt
+_img_info --format-specific | grep 'corrupt:'
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 5f4264cff6..bff023d889 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -420,4 +420,18 @@ write failed: Input/output error
{"return": ""}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+=== Testing incoming inactive corrupted image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+QMP_VERSION
+{"return": {}}
+qcow2: Image is corrupt: L2 table offset 0x2a2a2a00 unaligned (L1 index: 0); further non-fatal corruption events will be suppressed
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_IMAGE_CORRUPTED", "data": {"device": "", "msg": "L2 table offset 0x2a2a2a00 unaligned (L1 index: 0)", "node-name": "drive", "fatal": false}}
+read failed: Input/output error
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
+
+ corrupt: false
*** done
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [Qemu-devel] [PULL 00/29] Block patches
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
` (28 preceding siblings ...)
2018-06-11 14:26 ` [Qemu-devel] [PULL 29/29] iotests: Add case for a corrupted inactive image Max Reitz
@ 2018-06-11 15:20 ` Peter Maydell
29 siblings, 0 replies; 31+ messages in thread
From: Peter Maydell @ 2018-06-11 15:20 UTC (permalink / raw)
To: Max Reitz; +Cc: Qemu-block, QEMU Developers, Kevin Wolf
On 11 June 2018 at 15:25, Max Reitz <mreitz@redhat.com> wrote:
> The following changes since commit 9f55925b8f50a962d1d08d815044db7767ae3838:
>
> Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-3.0-pull-request' into staging (2018-06-11 12:46:16 +0100)
>
> are available in the Git repository at:
>
> git://github.com/XanClic/qemu.git tags/pull-block-2018-06-11
>
> for you to fetch changes up to c50abd175a88cd41c2c08339de91f6f6e4a7b162:
>
> iotests: Add case for a corrupted inactive image (2018-06-11 16:18:45 +0200)
>
> ----------------------------------------------------------------
> Block patches:
> - Various bug fixes
> - Removal of qemu-img convert's deprecated -s option
> - qemu-io now exits with an error when a command failed
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2018-06-11 15:21 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-11 14:25 [Qemu-devel] [PULL 00/29] Block patches Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 01/29] block/file-posix: Pass FD to locking helpers Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 02/29] block/file-posix: File locking during creation Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 03/29] iotests: Add creation test to 153 Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 04/29] qemu-img: Amendment support implies create_opts Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 05/29] block: Add Error parameter to bdrv_amend_options Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 06/29] qemu-option: Pull out "Supported options" print Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 07/29] qemu-img: Add print_amend_option_help() Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 08/29] qemu-img: Recognize no creation support in -o help Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 09/29] iotests: Test help option for unsupporting formats Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 10/29] iotests: Rework 113 Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 11/29] qcow2: Repair OFLAG_COPIED when fixing leaks Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 12/29] iotests: Repairing error during snapshot deletion Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 13/29] qemu-io: Drop command functions' return values Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 14/29] qemu-io: Let command functions return error code Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 15/29] qemu-io: Exit with error when a command failed Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 16/29] iotests.py: Add qemu_io_silent Max Reitz
2018-06-11 14:25 ` [Qemu-devel] [PULL 17/29] iotests: Let 216 make use of qemu-io's exit code Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 18/29] qemu-img: Resolve relative backing paths in rebase Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 19/29] iotests: Add test for rebasing with relative paths Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 20/29] qemu-img: Special post-backing convert handling Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 21/29] iotests: Test post-backing convert target behavior Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 22/29] iotests: improve pause_job Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 23/29] iotests: Fix 219's timing Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 24/29] qemu-img: Remove deprecated -s snapshot_id_or_name option Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 25/29] block/qcow2-bitmap: fix free_bitmap_clusters Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 26/29] throttle: Fix crash on reopen Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 27/29] block: Make bdrv_is_writable() public Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 28/29] qcow2: Do not mark inactive images corrupt Max Reitz
2018-06-11 14:26 ` [Qemu-devel] [PULL 29/29] iotests: Add case for a corrupted inactive image Max Reitz
2018-06-11 15:20 ` [Qemu-devel] [PULL 00/29] Block patches Peter Maydell
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.