Am 07.03.2016 um 19:24 hat Max Reitz geschrieben: > On 07.03.2016 13:26, Kevin Wolf wrote: > > 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 <[email protected]> > > --- > > block.c | 34 ++++++++++++++++++++++++---------- > > blockdev.c | 7 ------- > > include/block/block.h | 1 - > > 3 files changed, 24 insertions(+), 18 deletions(-) > > > > diff --git a/block.c b/block.c > > index ba24b8e..e3fe8ed 100644 > > --- a/block.c > > +++ b/block.c > > @@ -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; > > @@ -1465,7 +1471,6 @@ int bdrv_append_temp_snapshot(BlockDriverState *bs, > > int flags, Error **errp) > > } > > > > /* Prepare a new options QDict for the temporary file */ > > This comment reads a bit weird now.
Good catch, will s/a new// before sending a pull request. > Rest looks good and this is not exactly critical, so: > > Reviewed-by: Max Reitz <[email protected]> Thanks. Kevin
pgp_0ozIZjhaS.pgp
Description: PGP signature
