OpenE2K/qemu-e2k
13
4
Fork 0
Code Issues 4 Pull Requests Projects 2 Releases Activity

block: Fix snapshot=on cache modes

Browse Source 
Since commit 91a097e, we end up with a somewhat weird cache mode
configuration with snapshot=on: The commit broke the cache mode
inheritance for the snapshot overlay so that it is opened as
writethrough instead of unsafe now. The following bdrv_append() call to
put it on top of the tree swaps the WCE flag with the snapshot's backing
file (i.e. the originally given file), so what we eventually get is
cache=writeback on the temporary overlay and
cache=writethrough,cache.no-flush=on on the real image file.

This patch changes things so that the temporary overlay gets
cache=unsafe again like it used to, and the real images get whatever the
user specified. This means that cache.direct is now respected even with
snapshot=on, and in the case of committing changes, the final flush is
no longer ignored except explicitly requested by the user.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
This commit is contained in:
Kevin Wolf 2016-03-07 13:02:15 +01:00
parent f86b8b584b
commit 73176bee99
3 changed files with 25 additions and 19 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
block.c
View File

@ -687,13 +687,19 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
}
/*
* Returns the flags that a temporary snapshot should get, based on the
* originally requested flags (the originally requested image will have flags
* like a backing file)
* Returns the options and flags that a temporary snapshot should get, based on
* the originally requested flags (the originally requested image will have
* flags like a backing file)
*/
static int bdrv_temp_snapshot_flags(int flags)
static void bdrv_temp_snapshot_options(int *child_flags, QDict *child_options,
int parent_flags, QDict *parent_options)
{
return (flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
*child_flags = (parent_flags & ~BDRV_O_SNAPSHOT) | BDRV_O_TEMPORARY;
/* For temporary files, unconditional cache=unsafe is fine */
qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
qdict_set_default_str(child_options, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");
}
/*
@ -1424,13 +1430,13 @@ done:
return c;
}
int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
static int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags,
QDict *snapshot_options, Error **errp)
{
/* TODO: extra byte is a hack to ensure MAX_PATH space on Windows. */
char *tmp_filename = g_malloc0(PATH_MAX + 1);
int64_t total_size;
QemuOpts *opts = NULL;
QDict *snapshot_options;
BlockDriverState *bs_snapshot;
Error *local_err = NULL;
int ret;
@ -1464,8 +1470,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
goto out;
}
/* Prepare a new options QDict for the temporary file */
snapshot_options = qdict_new();
/* Prepare options QDict for the temporary file */
qdict_put(snapshot_options, "file.driver",
qstring_from_str("file"));
qdict_put(snapshot_options, "file.filename",
@ -1477,6 +1482,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
ret = bdrv_open(&bs_snapshot, NULL, NULL, snapshot_options,
flags, &local_err);
snapshot_options = NULL;
if (ret < 0) {
error_propagate(errp, local_err);
goto out;
@ -1485,6 +1491,7 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp)
bdrv_append(bs_snapshot, bs);
out:
QDECREF(snapshot_options);
g_free(tmp_filename);
return ret;
}
@ -1516,6 +1523,7 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
const char *drvname;
const char *backing;
Error *local_err = NULL;
QDict *snapshot_options = NULL;
int snapshot_flags = 0;
assert(pbs);
@ -1607,7 +1615,9 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
flags |= BDRV_O_ALLOW_RDWR;
}
if (flags & BDRV_O_SNAPSHOT) {
snapshot_flags = bdrv_temp_snapshot_flags(flags);
snapshot_options = qdict_new();
bdrv_temp_snapshot_options(&snapshot_flags, snapshot_options,
flags, options);
bdrv_backing_options(&flags, options, flags, options);
}
@ -1709,7 +1719,9 @@ static int bdrv_open_inherit(BlockDriverState **pbs, const char *filename,
/* For snapshot=on, create a temporary qcow2 overlay. bs points to the
* temporary snapshot afterwards. */
if (snapshot_flags) {
ret = bdrv_append_temp_snapshot(bs, snapshot_flags, &local_err);
ret = bdrv_append_temp_snapshot(bs, snapshot_flags, snapshot_options,
&local_err);
snapshot_options = NULL;
if (local_err) {
goto close_and_fail;
}
@ -1721,6 +1733,7 @@ fail:
if (file != NULL) {
bdrv_unref_child(bs, file);
}
QDECREF(snapshot_options);
QDECREF(bs->explicit_options);
QDECREF(bs->options);
QDECREF(options);
@ -1743,6 +1756,7 @@ close_and_fail:
} else {
bdrv_unref(bs);
}
QDECREF(snapshot_options);
QDECREF(options);
if (local_err) {
error_propagate(errp, local_err);

7
blockdev.c
View File

@ -593,13 +593,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
if (snapshot) {
/* always use cache=unsafe with snapshot */
qdict_put(bs_opts, BDRV_OPT_CACHE_WB, qstring_from_str("on"));
qdict_put(bs_opts, BDRV_OPT_CACHE_DIRECT, qstring_from_str("off"));
qdict_put(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, qstring_from_str("on"));
}
if (runstate_check(RUN_STATE_INMIGRATE)) {
bdrv_flags |= BDRV_O_INACTIVE;
}

1
include/block/block.h
View File

@ -215,7 +215,6 @@ BdrvChild *bdrv_open_child(const char *filename,
void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd);
int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp);
int bdrv_append_temp_snapshot(BlockDriverState *bs, int flags, Error **errp);
int bdrv_open(BlockDriverState **pbs, const char *filename,
const char *reference, QDict *options, int flags, Error **errp);
BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,