Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging
# gpg: Signature made Tue 21 Nov 2017 17:01:33 GMT
# gpg: using RSA key 0xBDBE7B27C0DE3057
# gpg: Good signature from "Jeffrey Cody <jcody@redhat.com>"
# gpg: aka "Jeffrey Cody <jeff@codyprime.org>"
# gpg: aka "Jeffrey Cody <codyprime@gmail.com>"
# Primary key fingerprint: 9957 4B4D 3474 90E7 9D98 D624 BDBE 7B27 C0DE 3057
* remotes/cody/tags/block-pull-request:
qemu-iotest: add test for blockjob coroutine race condition
qemu-iotests: add option in common.qemu for mismatch only
coroutine: abort if we try to schedule or enter a pending coroutine
blockjob: do not allow coroutine double entry or entry-after-completion
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
diff --git a/block.c b/block.c
index 6c8ef98..9a1a0d1 100644
--- a/block.c
+++ b/block.c
@@ -2579,7 +2579,10 @@
goto fail;
}
if (file_bs != NULL) {
- file = blk_new(BLK_PERM_CONSISTENT_READ, BLK_PERM_ALL);
+ /* Not requesting BLK_PERM_CONSISTENT_READ because we're only
+ * looking at the header to guess the image format. This works even
+ * in cases where a guest would not see a consistent state. */
+ file = blk_new(0, BLK_PERM_ALL);
blk_insert_bs(file, file_bs, &local_err);
bdrv_unref(file_bs);
if (local_err) {
@@ -3195,6 +3198,7 @@
static void bdrv_close(BlockDriverState *bs)
{
BdrvAioNotifier *ban, *ban_next;
+ BdrvChild *child, *next;
assert(!bs->job);
assert(!bs->refcnt);
@@ -3204,43 +3208,41 @@
bdrv_drain(bs); /* in case flush left pending I/O */
if (bs->drv) {
- BdrvChild *child, *next;
-
bs->drv->bdrv_close(bs);
bs->drv = NULL;
-
- bdrv_set_backing_hd(bs, NULL, &error_abort);
-
- if (bs->file != NULL) {
- bdrv_unref_child(bs, bs->file);
- bs->file = NULL;
- }
-
- QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
- /* TODO Remove bdrv_unref() from drivers' close function and use
- * bdrv_unref_child() here */
- if (child->bs->inherits_from == bs) {
- child->bs->inherits_from = NULL;
- }
- bdrv_detach_child(child);
- }
-
- g_free(bs->opaque);
- bs->opaque = NULL;
- atomic_set(&bs->copy_on_read, 0);
- bs->backing_file[0] = '\0';
- bs->backing_format[0] = '\0';
- bs->total_sectors = 0;
- bs->encrypted = false;
- bs->sg = false;
- QDECREF(bs->options);
- QDECREF(bs->explicit_options);
- bs->options = NULL;
- bs->explicit_options = NULL;
- QDECREF(bs->full_open_options);
- bs->full_open_options = NULL;
}
+ bdrv_set_backing_hd(bs, NULL, &error_abort);
+
+ if (bs->file != NULL) {
+ bdrv_unref_child(bs, bs->file);
+ bs->file = NULL;
+ }
+
+ QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
+ /* TODO Remove bdrv_unref() from drivers' close function and use
+ * bdrv_unref_child() here */
+ if (child->bs->inherits_from == bs) {
+ child->bs->inherits_from = NULL;
+ }
+ bdrv_detach_child(child);
+ }
+
+ g_free(bs->opaque);
+ bs->opaque = NULL;
+ atomic_set(&bs->copy_on_read, 0);
+ bs->backing_file[0] = '\0';
+ bs->backing_format[0] = '\0';
+ bs->total_sectors = 0;
+ bs->encrypted = false;
+ bs->sg = false;
+ QDECREF(bs->options);
+ QDECREF(bs->explicit_options);
+ bs->options = NULL;
+ bs->explicit_options = NULL;
+ QDECREF(bs->full_open_options);
+ bs->full_open_options = NULL;
+
bdrv_release_named_dirty_bitmaps(bs);
assert(QLIST_EMPTY(&bs->dirty_bitmaps));
diff --git a/block/block-backend.c b/block/block-backend.c
index 5836cb3..baef8e7 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -299,7 +299,7 @@
{
BlockBackend *blk;
BlockDriverState *bs;
- uint64_t perm;
+ uint64_t perm = 0;
/* blk_new_open() is mainly used in .bdrv_create implementations and the
* tools where sharing isn't a concern because the BDS stays private, so we
@@ -309,9 +309,11 @@
* caller of blk_new_open() doesn't make use of the permissions, but they
* shouldn't hurt either. We can still share everything here because the
* guest devices will add their own blockers if they can't share. */
- perm = BLK_PERM_CONSISTENT_READ;
- if (flags & BDRV_O_RDWR) {
- perm |= BLK_PERM_WRITE;
+ if ((flags & BDRV_O_NO_IO) == 0) {
+ perm |= BLK_PERM_CONSISTENT_READ;
+ if (flags & BDRV_O_RDWR) {
+ perm |= BLK_PERM_WRITE;
+ }
}
if (flags & BDRV_O_RESIZE) {
perm |= BLK_PERM_RESIZE;
diff --git a/block/snapshot.c b/block/snapshot.c
index be0743a..8cb70db 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -177,36 +177,35 @@
}
int bdrv_snapshot_goto(BlockDriverState *bs,
- const char *snapshot_id)
+ const char *snapshot_id,
+ Error **errp)
{
BlockDriver *drv = bs->drv;
int ret, open_ret;
- int64_t len;
if (!drv) {
+ error_setg(errp, "Block driver is closed");
return -ENOMEDIUM;
}
- len = bdrv_getlength(bs);
- if (len < 0) {
- return len;
+ if (!QLIST_EMPTY(&bs->dirty_bitmaps)) {
+ error_setg(errp, "Device has active dirty bitmaps");
+ return -EBUSY;
}
- /* We should set all bits in all enabled dirty bitmaps, because dirty
- * bitmaps reflect active state of disk and snapshot switch operation
- * actually dirties active state.
- * TODO: It may make sense not to set all bits but analyze block status of
- * current state and destination snapshot and do not set bits corresponding
- * to both-zero or both-unallocated areas. */
- bdrv_set_dirty(bs, 0, len);
if (drv->bdrv_snapshot_goto) {
- return drv->bdrv_snapshot_goto(bs, snapshot_id);
+ ret = drv->bdrv_snapshot_goto(bs, snapshot_id);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Failed to load snapshot");
+ }
+ return ret;
}
if (bs->file) {
BlockDriverState *file;
QDict *options = qdict_clone_shallow(bs->options);
QDict *file_options;
+ Error *local_err = NULL;
file = bs->file->bs;
/* Prevent it from getting deleted when detached from bs */
@@ -220,13 +219,15 @@
bdrv_unref_child(bs, bs->file);
bs->file = NULL;
- ret = bdrv_snapshot_goto(file, snapshot_id);
- open_ret = drv->bdrv_open(bs, options, bs->open_flags, NULL);
+ ret = bdrv_snapshot_goto(file, snapshot_id, errp);
+ open_ret = drv->bdrv_open(bs, options, bs->open_flags, &local_err);
QDECREF(options);
if (open_ret < 0) {
bdrv_unref(file);
bs->drv = NULL;
- return open_ret;
+ /* A bdrv_snapshot_goto() error takes precedence */
+ error_propagate(errp, local_err);
+ return ret < 0 ? ret : open_ret;
}
assert(bs->file->bs == file);
@@ -234,6 +235,7 @@
return ret;
}
+ error_setg(errp, "Block driver does not support snapshots");
return -ENOTSUP;
}
@@ -456,9 +458,10 @@
}
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
+ Error **errp)
{
- int err = 0;
+ int ret = 0;
BlockDriverState *bs;
BdrvNextIterator it;
@@ -467,10 +470,10 @@
aio_context_acquire(ctx);
if (bdrv_can_snapshot(bs)) {
- err = bdrv_snapshot_goto(bs, name);
+ ret = bdrv_snapshot_goto(bs, name, errp);
}
aio_context_release(ctx);
- if (err < 0) {
+ if (ret < 0) {
bdrv_next_cleanup(&it);
goto fail;
}
@@ -478,7 +481,7 @@
fail:
*first_bad_bs = bs;
- return err;
+ return ret;
}
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs)
diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index e5c0553..9407799 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -57,7 +57,8 @@
int bdrv_snapshot_create(BlockDriverState *bs,
QEMUSnapshotInfo *sn_info);
int bdrv_snapshot_goto(BlockDriverState *bs,
- const char *snapshot_id);
+ const char *snapshot_id,
+ Error **errp);
int bdrv_snapshot_delete(BlockDriverState *bs,
const char *snapshot_id,
const char *name,
@@ -83,7 +84,8 @@
bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
Error **err);
-int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bsd_bs);
+int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
+ Error **errp);
int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
int bdrv_all_create_snapshot(QEMUSnapshotInfo *sn,
BlockDriverState *vm_state_bs,
diff --git a/migration/savevm.c b/migration/savevm.c
index 4a88228..192f2d8 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2346,10 +2346,10 @@
/* Flush all IO requests so they don't interfere with the new state. */
bdrv_drain_all_begin();
- ret = bdrv_all_goto_snapshot(name, &bs);
+ ret = bdrv_all_goto_snapshot(name, &bs, errp);
if (ret < 0) {
- error_setg(errp, "Error %d while activating snapshot '%s' on '%s'",
- ret, name, bdrv_get_device_name(bs));
+ error_prepend(errp, "Could not load snapshot '%s' on '%s': ",
+ name, bdrv_get_device_name(bs));
goto err_drain;
}
diff --git a/qemu-img.c b/qemu-img.c
index 02a6e27..68b375f 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2989,10 +2989,10 @@
break;
case SNAPSHOT_APPLY:
- ret = bdrv_snapshot_goto(bs, snapshot_name);
+ ret = bdrv_snapshot_goto(bs, snapshot_name, &err);
if (ret) {
- error_report("Could not apply snapshot '%s': %d (%s)",
- snapshot_name, ret, strerror(-ret));
+ error_reportf_err(err, "Could not apply snapshot '%s': ",
+ snapshot_name);
}
break;
diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060
index 1eca094..14797dd 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -426,6 +426,19 @@
_check_test_img -q -r all
_check_test_img -r all
+echo
+echo "=== Testing the QEMU shutdown with a corrupted image ==="
+echo
+_make_test_img 64M
+poke_file "$TEST_IMG" "$rt_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
+echo "{'execute': 'qmp_capabilities'}
+ {'execute': 'human-monitor-command',
+ 'arguments': {'command-line': 'qemu-io drive \"write 0 512\"'}}
+ {'execute': 'quit'}" \
+ | $QEMU -qmp stdio -nographic -nodefaults \
+ -drive if=none,node-name=drive,file="$TEST_IMG",driver=qcow2 \
+ | _filter_qmp | _filter_qemu_io
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out
index 56f5eb1..c4cb7c6 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -399,4 +399,16 @@
Double checking the fixed image now...
No errors were found on the image.
+
+=== Testing the QEMU shutdown with a corrupted image ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+qcow2: Marking image as corrupt: Preventing invalid write on metadata (overlaps with refcount table); further corruption events will be suppressed
+QMP_VERSION
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_IMAGE_CORRUPTED", "data": {"device": "none0", "msg": "Preventing invalid write on metadata (overlaps with refcount table)", "offset": 65536, "node-name": "drive", "fatal": true, "size": 65536}}
+write failed: Input/output error
+{"return": ""}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
*** done
diff --git a/tests/qemu-iotests/176 b/tests/qemu-iotests/176
index 0f31a20..b8dc17c 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -52,7 +52,8 @@
function run_qemu()
{
$QEMU -nographic -qmp stdio -serial none "$@" 2>&1 \
- | _filter_testdir | _filter_qmp | _filter_qemu
+ | _filter_testdir | _filter_qmp | _filter_qemu \
+ | sed 's/"sha256": ".\{64\}"/"sha256": HASH/'
}
for reason in snapshot bitmap; do
diff --git a/tests/qemu-iotests/176.out b/tests/qemu-iotests/176.out
index e62085c..f03a2e7 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -205,7 +205,7 @@
QMP_VERSION
{"return": {}}
{"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
@@ -255,7 +255,7 @@
QMP_VERSION
{"return": {}}
{"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
@@ -305,7 +305,7 @@
QMP_VERSION
{"return": {}}
{"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
@@ -352,7 +352,7 @@
QMP_VERSION
{"return": {}}
{"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
*** done
