diff --git a/block.c b/block.c
index 6c8ef98dfaf713b43ebd6f53becf5efcce2678b5..9a1a0d1e73c44209b5d8993c1e9aa5bee456a1ad 100644
--- a/block.c
+++ b/block.c
@@ -2579,7 +2579,10 @@ static BlockDriverState *bdrv_open_inherit(const char *filename,
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 @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)
static void bdrv_close(BlockDriverState *bs)
{
BdrvAioNotifier *ban, *ban_next;
+ BdrvChild *child, *next;
assert(!bs->job);
assert(!bs->refcnt);
@@ -3204,43 +3208,41 @@ static void bdrv_close(BlockDriverState *bs)
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);
+ bdrv_set_backing_hd(bs, NULL, &error_abort);
- if (bs->file != NULL) {
- bdrv_unref_child(bs, bs->file);
- bs->file = NULL;
- }
+ 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);
+ 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;
}
-
- 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_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 5836cb30876fe673430e808865f8cc41c20989b0..baef8e7abc2f738661bbd4be92e769ada7ebe75d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -299,7 +299,7 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
{
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 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
* 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 be0743abac874b21a10e8375e6ababc94084276a..8cb70dbad5909e10bc8f319d9d26f2ce9e00b209 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -177,36 +177,35 @@ int bdrv_snapshot_create(BlockDriverState *bs,
}
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 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
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 @@ int bdrv_snapshot_goto(BlockDriverState *bs,
return ret;
}
+ error_setg(errp, "Block driver does not support snapshots");
return -ENOTSUP;
}
@@ -456,9 +458,10 @@ fail:
}
-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 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
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 @@ int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs)
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 e5c055311557e86f219503d629be4b88c94e1fe5..940779994111a39c84744633c4e3033217e4acc8 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -57,7 +57,8 @@ int bdrv_can_snapshot(BlockDriverState *bs);
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 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
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 4a882286148f8d618eb60c750ef09f4a0285f322..192f2d82cd56c164f6820274d746a63527259bdb 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2346,10 +2346,10 @@ int load_snapshot(const char *name, Error **errp)
/* 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 02a6e27beb1357593e69a0f20e6d22b97da596be..68b375f998c067a15551b8dbb23b5eadddce6727 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2989,10 +2989,10 @@ static int img_snapshot(int argc, char **argv)
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 1eca09417ba157c4e6f6ea5c49ef7d47d107ed46..14797dd3b0e74c452c1b3963c7f9d40bed89cd60 100755
--- a/tests/qemu-iotests/060
+++ b/tests/qemu-iotests/060
@@ -426,6 +426,19 @@ echo '--- Repairing ---'
_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 56f5eb15d8744b342822b56d3693bc92a453cd4e..c4cb7c665e90007d0e883a96eeac450c3f21cb78 100644
--- a/tests/qemu-iotests/060.out
+++ b/tests/qemu-iotests/060.out
@@ -399,4 +399,16 @@ The following inconsistencies were found and repaired:
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 0f31a20294f80a5bc81c7bdd34690c7e964a696b..b8dc17c59205d2a8a44e3a6d79b035cf09503ac8 100755
--- a/tests/qemu-iotests/176
+++ b/tests/qemu-iotests/176
@@ -52,7 +52,8 @@ _supported_os Linux
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 e62085cd0ae92750106d2ba41fd2d449105b9957..f03a2e776c62ca48b48edd88c361b5db6e90b1e9 100644
--- a/tests/qemu-iotests/176.out
+++ b/tests/qemu-iotests/176.out
@@ -205,7 +205,7 @@ Offset Length File
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 @@ Offset Length File
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 @@ Offset Length File
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 @@ Offset Length File
QMP_VERSION
{"return": {}}
{"return": {}}
-{"return": {"sha256": "e12600978d86b5a453861ae5c17d275204673fef3874b7c3c5433c6153d84706"}}
+{"return": {"sha256": HASH}}
{"return": {}}
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
*** done