* [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend
@ 2016-03-18 18:21 Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode() Kevin Wolf
` (19 more replies)
0 siblings, 20 replies; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
I already sent (and merged) patches that are meant to prevent users from
setting the writethrough option on non-root nodes, where we won't be able to
support the feature in the long term because we discovered that writethrough
mode only makes sense as a BlockBackend feature. The intention was to allow
moving the feature where it belongs in the following release.
However, moving the feature involves another few changes in behaviour in some
corner cases (most of which became only possible with patches after 2.5), so it
seems to be actually preferable to do the full thing now instead of having
behaviour changes in two consecutive releases.
This series removes (or is supposed to remove, at least) all traces of
writethrough handling from the BlockDriverState layer and implements it in
BlockBackend instead.
Kevin Wolf (20):
block: Add bdrv_parse_cache_mode()
qemu-nbd: Call blk_set_enable_write_cache() explicitly
qemu-io: Call blk_set_enable_write_cache() explicitly
qemu-img: Expand all BDRV_O_FLAGS uses
qemu-img: Call blk_set_enable_write_cache() explicitly
xen_disk: Call blk_set_enable_write_cache() explicitly
block: blockdev_init(): Call blk_set_enable_write_cache() explicitly
block: Always set writeback mode in blk_new_open()
block: Handle flush error in bdrv_pwrite_sync()
block: Move enable_write_cache to BB level
block/qapi: Use blk_enable_write_cache()
block: Introduce bdrv_co_writev_flags()
iscsi: Support BDRV_REQ_FUA
nbd: Support BDRV_REQ_FUA
raw: Support BDRV_REQ_FUA
block: Use bdrv_parse_cache_mode() in drive_init()
qemu-io: Use bdrv_parse_cache_mode() in reopen_f()
block: Remove bdrv_parse_cache_flags()
block: Remove BDRV_O_CACHE_WB
block: Remove bdrv_(set_)enable_write_cache()
block.c | 78 +++++---------------------------
block/backup.c | 1 -
block/block-backend.c | 45 ++++++++++--------
block/io.c | 15 ++++--
block/iscsi.c | 24 +++-------
block/mirror.c | 1 -
block/nbd-client.c | 13 +++---
block/nbd-client.h | 2 +-
block/nbd.c | 26 +++++++++--
block/parallels.c | 3 +-
block/qapi.c | 7 +--
block/qcow.c | 3 +-
block/qcow2.c | 9 ++--
block/qed.c | 3 +-
block/raw_bsd.c | 11 +++--
block/sheepdog.c | 5 +-
block/vdi.c | 3 +-
block/vhdx.c | 3 +-
block/vmdk.c | 8 ++--
block/vpc.c | 3 +-
block/vvfat.c | 3 +-
blockdev.c | 35 ++++++--------
hw/block/xen_disk.c | 5 +-
include/block/block.h | 8 ++--
include/block/block_int.h | 8 ++--
include/block/qapi.h | 3 +-
qemu-img.c | 84 ++++++++++++++++++++--------------
qemu-io-cmds.c | 13 +++++-
qemu-io.c | 21 +++++----
qemu-nbd.c | 5 +-
tests/qemu-iotests/051 | 2 +-
tests/qemu-iotests/051.pc.out | 10 ++--
tests/qemu-iotests/142 | 19 +++++---
tests/qemu-iotests/142.out | 103 ++++++++++++++++++++++++++++--------------
34 files changed, 304 insertions(+), 278 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 17:05 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly Kevin Wolf
` (18 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
It's like bdrv_parse_cache_flags(), except that writethrough mode isn't
included in the flags, but returned as a separate bool.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 17 +++++++++++++++++
include/block/block.h | 1 +
2 files changed, 18 insertions(+)
diff --git a/block.c b/block.c
index d319f86..172f865 100644
--- a/block.c
+++ b/block.c
@@ -661,6 +661,23 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
return 0;
}
+int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
+{
+ int ret = bdrv_parse_cache_flags(mode, flags);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (*flags & BDRV_O_CACHE_WB) {
+ *flags &= ~BDRV_O_CACHE_WB;
+ *writethrough = false;
+ } else {
+ *writethrough = true;
+ }
+
+ return 0;
+}
+
/*
* Returns the options and flags that a temporary snapshot should get, based on
* the originally requested flags (the originally requested image will have
diff --git a/include/block/block.h b/include/block/block.h
index 4adb1e9..823f4d7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -206,6 +206,7 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old,
BlockDriverState *new);
int bdrv_parse_cache_flags(const char *mode, int *flags);
+int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
int bdrv_parse_discard_flags(const char *mode, int *flags);
BdrvChild *bdrv_open_child(const char *filename,
QDict *options, const char *bdref_key,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 17:09 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 03/20] qemu-io: " Kevin Wolf
` (17 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-nbd.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/qemu-nbd.c b/qemu-nbd.c
index d5f8473..3a32140 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -507,6 +507,7 @@ int main(int argc, char **argv)
const char *export_name = NULL;
const char *tlscredsid = NULL;
bool imageOpts = false;
+ bool writethrough = true;
/* The client thread uses SIGTERM to interrupt the server. A signal
* handler ensures that "qemu-nbd -v -c" exits with a nice status code.
@@ -533,7 +534,7 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
seen_cache = true;
- if (bdrv_parse_cache_flags(optarg, &flags) == -1) {
+ if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) == -1) {
error_report("Invalid cache mode `%s'", optarg);
exit(EXIT_FAILURE);
}
@@ -847,6 +848,8 @@ int main(int argc, char **argv)
}
bs = blk_bs(blk);
+ blk_set_enable_write_cache(blk, !writethrough);
+
if (sn_opts) {
ret = bdrv_snapshot_load_tmp(bs,
qemu_opt_get(sn_opts, SNAPSHOT_OPT_ID),
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 03/20] qemu-io: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode() Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 17:18 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses Kevin Wolf
` (16 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-io.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index d7c2f26..260b024 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -50,7 +50,7 @@ static const cmdinfo_t close_cmd = {
.oneline = "close the current open file",
};
-static int openfile(char *name, int flags, QDict *opts)
+static int openfile(char *name, int flags, bool writethrough, QDict *opts)
{
Error *local_err = NULL;
BlockDriverState *bs;
@@ -82,6 +82,7 @@ static int openfile(char *name, int flags, QDict *opts)
}
}
+ blk_set_enable_write_cache(qemuio_blk, !writethrough);
return 0;
@@ -136,6 +137,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
{
int flags = 0;
int readonly = 0;
+ bool writethrough = true;
int c;
QemuOpts *qopts;
QDict *opts;
@@ -146,7 +148,8 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
flags |= BDRV_O_SNAPSHOT;
break;
case 'n':
- flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
+ flags |= BDRV_O_NOCACHE;
+ writethrough = false;
break;
case 'r':
readonly = 1;
@@ -184,9 +187,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv)
qemu_opts_reset(&empty_opts);
if (optind == argc - 1) {
- return openfile(argv[optind], flags, opts);
+ return openfile(argv[optind], flags, writethrough, opts);
} else if (optind == argc) {
- return openfile(NULL, flags, opts);
+ return openfile(NULL, flags, writethrough, opts);
} else {
QDECREF(opts);
return qemuio_command_usage(&open_cmd);
@@ -427,6 +430,7 @@ int main(int argc, char **argv)
int c;
int opt_index = 0;
int flags = BDRV_O_UNMAP;
+ bool writethrough = true;
Error *local_error = NULL;
QDict *opts = NULL;
const char *format = NULL;
@@ -448,7 +452,8 @@ int main(int argc, char **argv)
flags |= BDRV_O_SNAPSHOT;
break;
case 'n':
- flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
+ flags |= BDRV_O_NOCACHE;
+ writethrough = false;
break;
case 'd':
if (bdrv_parse_discard_flags(optarg, &flags) < 0) {
@@ -472,7 +477,7 @@ int main(int argc, char **argv)
flags |= BDRV_O_NATIVE_AIO;
break;
case 't':
- if (bdrv_parse_cache_flags(optarg, &flags) < 0) {
+ if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
exit(1);
}
@@ -554,13 +559,13 @@ int main(int argc, char **argv)
exit(1);
}
opts = qemu_opts_to_qdict(qopts, NULL);
- openfile(NULL, flags, opts);
+ openfile(NULL, flags, writethrough, opts);
} else {
if (format) {
opts = qdict_new();
qdict_put(opts, "driver", qstring_from_str(format));
}
- openfile(argv[optind], flags, opts);
+ openfile(argv[optind], flags, writethrough, opts);
}
}
command_loop();
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (2 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 03/20] qemu-io: " Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 17:34 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly Kevin Wolf
` (15 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
It always only set the BDRV_O_CACHE_WB flag, which is going to go away.
In order to make the next changes more local for better reviewability
this patches expands the macro.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 29eae2a..839e05b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -58,8 +58,7 @@ typedef enum OutputFormat {
OFORMAT_HUMAN,
} OutputFormat;
-/* Default to cache=writeback as data integrity is not important for qemu-tcg. */
-#define BDRV_O_FLAGS BDRV_O_CACHE_WB
+/* Default to cache=writeback as data integrity is not important for qemu-img */
#define BDRV_DEFAULT_CACHE "writeback"
static void format_print(void *opaque, const char *name)
@@ -460,7 +459,7 @@ static int img_create(int argc, char **argv)
}
bdrv_img_create(filename, fmt, base_filename, base_fmt,
- options, img_size, BDRV_O_FLAGS, &local_err, quiet);
+ options, img_size, BDRV_O_CACHE_WB, &local_err, quiet);
if (local_err) {
error_reportf_err(local_err, "%s: ", filename);
goto fail;
@@ -590,7 +589,7 @@ static int img_check(int argc, char **argv)
BlockBackend *blk;
BlockDriverState *bs;
int fix = 0;
- int flags = BDRV_O_FLAGS | BDRV_O_CHECK;
+ int flags = BDRV_O_CACHE_WB | BDRV_O_CHECK;
ImageCheck *check;
bool quiet = false;
Error *local_err = NULL;
@@ -1202,7 +1201,7 @@ static int img_compare(int argc, char **argv)
/* Initialize before goto out */
qemu_progress_init(progress, 2.0);
- flags = BDRV_O_FLAGS;
+ flags = BDRV_O_CACHE_WB;
ret = bdrv_parse_cache_flags(cache, &flags);
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
@@ -1882,7 +1881,7 @@ static int img_convert(int argc, char **argv)
goto out;
}
- src_flags = BDRV_O_FLAGS;
+ src_flags = BDRV_O_CACHE_WB;
ret = bdrv_parse_cache_flags(src_cache, &src_flags);
if (ret < 0) {
error_report("Invalid source cache option: %s", src_cache);
@@ -2235,7 +2234,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
g_hash_table_insert(filenames, (gpointer)filename, NULL);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_FLAGS | BDRV_O_NO_BACKING,
+ BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
false, false);
if (!blk) {
goto err;
@@ -2566,7 +2565,7 @@ static int img_map(int argc, char **argv)
return 1;
}
- blk = img_open(image_opts, filename, fmt, BDRV_O_FLAGS, true, false);
+ blk = img_open(image_opts, filename, fmt, BDRV_O_CACHE_WB, true, false);
if (!blk) {
return 1;
}
@@ -2630,7 +2629,7 @@ static int img_snapshot(int argc, char **argv)
Error *err = NULL;
bool image_opts = false;
- bdrv_oflags = BDRV_O_FLAGS | BDRV_O_RDWR;
+ bdrv_oflags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
/* Parse commandline parameters */
for(;;) {
static const struct option long_options[] = {
@@ -2869,7 +2868,7 @@ static int img_rebase(int argc, char **argv)
goto out;
}
- src_flags = BDRV_O_FLAGS;
+ src_flags = BDRV_O_CACHE_WB;
ret = bdrv_parse_cache_flags(src_cache, &src_flags);
if (ret < 0) {
error_report("Invalid source cache option: %s", src_cache);
@@ -3220,7 +3219,7 @@ static int img_resize(int argc, char **argv)
qemu_opts_del(param);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_FLAGS | BDRV_O_RDWR, true, quiet);
+ BDRV_O_CACHE_WB | BDRV_O_RDWR, true, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3372,7 +3371,7 @@ static int img_amend(int argc, char **argv)
goto out;
}
- flags = BDRV_O_FLAGS | BDRV_O_RDWR;
+ flags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
ret = bdrv_parse_cache_flags(cache, &flags);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (3 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 17:54 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 06/20] xen_disk: " Kevin Wolf
` (14 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
qemu-img.c | 79 ++++++++++++++++++++++++++++++++++++++------------------------
1 file changed, 48 insertions(+), 31 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 839e05b..96b51d4 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -245,7 +245,7 @@ static int img_open_password(BlockBackend *blk, const char *filename,
static BlockBackend *img_open_opts(const char *optstr,
- QemuOpts *opts, int flags,
+ QemuOpts *opts, int flags, bool writethrough,
bool require_io, bool quiet)
{
QDict *options;
@@ -257,6 +257,7 @@ static BlockBackend *img_open_opts(const char *optstr,
error_reportf_err(local_err, "Could not open '%s'", optstr);
return NULL;
}
+ blk_set_enable_write_cache(blk, !writethrough);
if (img_open_password(blk, optstr, require_io, quiet) < 0) {
blk_unref(blk);
@@ -267,7 +268,8 @@ static BlockBackend *img_open_opts(const char *optstr,
static BlockBackend *img_open_file(const char *filename,
const char *fmt, int flags,
- bool require_io, bool quiet)
+ bool writethrough, bool require_io,
+ bool quiet)
{
BlockBackend *blk;
Error *local_err = NULL;
@@ -283,6 +285,7 @@ static BlockBackend *img_open_file(const char *filename,
error_reportf_err(local_err, "Could not open '%s': ", filename);
return NULL;
}
+ blk_set_enable_write_cache(blk, !writethrough);
if (img_open_password(blk, filename, require_io, quiet) < 0) {
blk_unref(blk);
@@ -294,7 +297,7 @@ static BlockBackend *img_open_file(const char *filename,
static BlockBackend *img_open(bool image_opts,
const char *filename,
- const char *fmt, int flags,
+ const char *fmt, int flags, bool writethrough,
bool require_io, bool quiet)
{
BlockBackend *blk;
@@ -309,9 +312,9 @@ static BlockBackend *img_open(bool image_opts,
if (!opts) {
return NULL;
}
- blk = img_open_opts(filename, opts, flags, true, quiet);
+ blk = img_open_opts(filename, opts, flags, writethrough, true, quiet);
} else {
- blk = img_open_file(filename, fmt, flags, true, quiet);
+ blk = img_open_file(filename, fmt, flags, writethrough, true, quiet);
}
return blk;
}
@@ -589,7 +592,8 @@ static int img_check(int argc, char **argv)
BlockBackend *blk;
BlockDriverState *bs;
int fix = 0;
- int flags = BDRV_O_CACHE_WB | BDRV_O_CHECK;
+ int flags = BDRV_O_CHECK;
+ bool writethrough;
ImageCheck *check;
bool quiet = false;
Error *local_err = NULL;
@@ -598,6 +602,7 @@ static int img_check(int argc, char **argv)
fmt = NULL;
output = NULL;
cache = BDRV_DEFAULT_CACHE;
+
for(;;) {
int option_index = 0;
static const struct option long_options[] = {
@@ -677,13 +682,13 @@ static int img_check(int argc, char **argv)
return 1;
}
- ret = bdrv_parse_cache_flags(cache, &flags);
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
return 1;
}
- blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, true, quiet);
if (!blk) {
return 1;
}
@@ -793,6 +798,7 @@ static int img_commit(int argc, char **argv)
BlockBackend *blk;
BlockDriverState *bs, *base_bs;
bool progress = false, quiet = false, drop = false;
+ bool writethrough;
Error *local_err = NULL;
CommonBlockJobCBInfo cbi;
bool image_opts = false;
@@ -869,13 +875,13 @@ static int img_commit(int argc, char **argv)
}
flags = BDRV_O_RDWR | BDRV_O_UNMAP;
- ret = bdrv_parse_cache_flags(cache, &flags);
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
return 1;
}
- blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, true, quiet);
if (!blk) {
return 1;
}
@@ -1119,6 +1125,7 @@ static int img_compare(int argc, char **argv)
int ret = 0; /* return value - 0 Ident, 1 Different, >1 Error */
bool progress = false, quiet = false, strict = false;
int flags;
+ bool writethrough;
int64_t total_sectors;
int64_t sector_num = 0;
int64_t nb_sectors;
@@ -1201,21 +1208,23 @@ static int img_compare(int argc, char **argv)
/* Initialize before goto out */
qemu_progress_init(progress, 2.0);
- flags = BDRV_O_CACHE_WB;
- ret = bdrv_parse_cache_flags(cache, &flags);
+ flags = 0;
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid source cache option: %s", cache);
ret = 2;
goto out3;
}
- blk1 = img_open(image_opts, filename1, fmt1, flags, true, quiet);
+ blk1 = img_open(image_opts, filename1, fmt1, flags, writethrough, true,
+ quiet);
if (!blk1) {
ret = 2;
goto out3;
}
- blk2 = img_open(image_opts, filename2, fmt2, flags, true, quiet);
+ blk2 = img_open(image_opts, filename2, fmt2, flags, writethrough, true,
+ quiet);
if (!blk2) {
ret = 2;
goto out2;
@@ -1709,6 +1718,7 @@ static int img_convert(int argc, char **argv)
int c, bs_n, bs_i, compress, cluster_sectors, skip_create;
int64_t ret = 0;
int progress = 0, flags, src_flags;
+ bool writethrough, src_writethrough;
const char *fmt, *out_fmt, *cache, *src_cache, *out_baseimg, *out_filename;
BlockDriver *drv, *proto_drv;
BlockBackend **blk = NULL, *out_blk = NULL;
@@ -1881,8 +1891,8 @@ static int img_convert(int argc, char **argv)
goto out;
}
- src_flags = BDRV_O_CACHE_WB;
- ret = bdrv_parse_cache_flags(src_cache, &src_flags);
+ src_flags = 0;
+ ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
if (ret < 0) {
error_report("Invalid source cache option: %s", src_cache);
goto out;
@@ -1897,7 +1907,7 @@ static int img_convert(int argc, char **argv)
total_sectors = 0;
for (bs_i = 0; bs_i < bs_n; bs_i++) {
blk[bs_i] = img_open(image_opts, argv[optind + bs_i],
- fmt, src_flags, true, quiet);
+ fmt, src_flags, src_writethrough, true, quiet);
if (!blk[bs_i]) {
ret = -1;
goto out;
@@ -2031,7 +2041,7 @@ static int img_convert(int argc, char **argv)
}
flags = min_sparse ? (BDRV_O_RDWR | BDRV_O_UNMAP) : BDRV_O_RDWR;
- ret = bdrv_parse_cache_flags(cache, &flags);
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
goto out;
@@ -2042,7 +2052,8 @@ static int img_convert(int argc, char **argv)
* the bdrv_create() call which takes different params.
* Not critical right now, so fix can wait...
*/
- out_blk = img_open_file(out_filename, out_fmt, flags, true, quiet);
+ out_blk = img_open_file(out_filename, out_fmt, flags, writethrough, true,
+ quiet);
if (!out_blk) {
ret = -1;
goto out;
@@ -2234,7 +2245,7 @@ static ImageInfoList *collect_image_info_list(bool image_opts,
g_hash_table_insert(filenames, (gpointer)filename, NULL);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
+ BDRV_O_NO_BACKING, false,
false, false);
if (!blk) {
goto err;
@@ -2565,7 +2576,7 @@ static int img_map(int argc, char **argv)
return 1;
}
- blk = img_open(image_opts, filename, fmt, BDRV_O_CACHE_WB, true, false);
+ blk = img_open(image_opts, filename, fmt, 0, false, true, false);
if (!blk) {
return 1;
}
@@ -2629,7 +2640,7 @@ static int img_snapshot(int argc, char **argv)
Error *err = NULL;
bool image_opts = false;
- bdrv_oflags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
+ bdrv_oflags = BDRV_O_RDWR;
/* Parse commandline parameters */
for(;;) {
static const struct option long_options[] = {
@@ -2710,7 +2721,7 @@ static int img_snapshot(int argc, char **argv)
}
/* Open the image */
- blk = img_open(image_opts, filename, NULL, bdrv_oflags, true, quiet);
+ blk = img_open(image_opts, filename, NULL, bdrv_oflags, false, true, quiet);
if (!blk) {
return 1;
}
@@ -2772,6 +2783,7 @@ static int img_rebase(int argc, char **argv)
char *filename;
const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
int c, flags, src_flags, ret;
+ bool writethrough, src_writethrough;
int unsafe = 0;
int progress = 0;
bool quiet = false;
@@ -2862,26 +2874,30 @@ static int img_rebase(int argc, char **argv)
qemu_progress_print(0, 100);
flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
- ret = bdrv_parse_cache_flags(cache, &flags);
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
goto out;
}
- src_flags = BDRV_O_CACHE_WB;
- ret = bdrv_parse_cache_flags(src_cache, &src_flags);
+ src_flags = 0;
+ ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
if (ret < 0) {
error_report("Invalid source cache option: %s", src_cache);
goto out;
}
+ /* The source files are opened read-only, don't care about WCE */
+ assert((src_writethrough & BDRV_O_RDWR) == 0);
+ (void) src_writethrough;
+
/*
* Open the images.
*
* Ignore the old backing file for unsafe rebase in case we want to correct
* the reference to a renamed or moved backing file.
*/
- blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, true, writethrough, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3219,7 +3235,7 @@ static int img_resize(int argc, char **argv)
qemu_opts_del(param);
blk = img_open(image_opts, filename, fmt,
- BDRV_O_CACHE_WB | BDRV_O_RDWR, true, quiet);
+ BDRV_O_RDWR, false, true, quiet);
if (!blk) {
ret = -1;
goto out;
@@ -3275,6 +3291,7 @@ static int img_amend(int argc, char **argv)
QemuOpts *opts = NULL;
const char *fmt = NULL, *filename, *cache;
int flags;
+ bool writethrough;
bool quiet = false, progress = false;
BlockBackend *blk = NULL;
BlockDriverState *bs = NULL;
@@ -3371,14 +3388,14 @@ static int img_amend(int argc, char **argv)
goto out;
}
- flags = BDRV_O_CACHE_WB | BDRV_O_RDWR;
- ret = bdrv_parse_cache_flags(cache, &flags);
+ flags = BDRV_O_RDWR;
+ ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
if (ret < 0) {
error_report("Invalid cache option: %s", cache);
goto out;
}
- blk = img_open(image_opts, filename, fmt, flags, true, quiet);
+ blk = img_open(image_opts, filename, fmt, flags, writethrough, true, quiet);
if (!blk) {
ret = -1;
goto out;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 06/20] xen_disk: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (4 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-22 11:08 ` Stefano Stabellini
2016-03-26 17:59 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 07/20] block: blockdev_init(): " Kevin Wolf
` (13 subsequent siblings)
19 siblings, 2 replies; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
hw/block/xen_disk.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
index 635328f..c358709 100644
--- a/hw/block/xen_disk.c
+++ b/hw/block/xen_disk.c
@@ -888,12 +888,14 @@ static int blk_connect(struct XenDevice *xendev)
struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
int pers, index, qflags;
bool readonly = true;
+ bool writethrough = true;
/* read-only ? */
if (blkdev->directiosafe) {
qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
} else {
- qflags = BDRV_O_CACHE_WB;
+ qflags = 0;
+ writethrough = false;
}
if (strcmp(blkdev->mode, "w") == 0) {
qflags |= BDRV_O_RDWR;
@@ -925,6 +927,7 @@ static int blk_connect(struct XenDevice *xendev)
error_free(local_err);
return -1;
}
+ blk_set_enable_write_cache(blkdev->blk, !writethrough);
} else {
/* setup via qemu cmdline -> already setup for us */
xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 07/20] block: blockdev_init(): Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (5 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 06/20] xen_disk: " Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 18:13 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open() Kevin Wolf
` (12 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/blockdev.c b/blockdev.c
index b18861e..70c3add 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -467,6 +467,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
int bdrv_flags = 0;
int on_read_error, on_write_error;
bool account_invalid, account_failed;
+ bool writethrough;
BlockBackend *blk;
BlockDriverState *bs;
ThrottleConfig cfg;
@@ -505,6 +506,8 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
account_invalid = qemu_opt_get_bool(opts, "stats-account-invalid", true);
account_failed = qemu_opt_get_bool(opts, "stats-account-failed", true);
+ writethrough = !qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, true);
+
qdict_extract_subqdict(bs_opts, &interval_dict, "stats-intervals.");
qdict_array_split(interval_dict, &interval_list);
@@ -590,7 +593,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
/* bdrv_open() defaults to the values in bdrv_flags (for compatibility
* with other callers) rather than what we want as the real defaults.
* Apply the defaults here instead. */
- qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, "on");
+ qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, writethrough ? "off" : "on");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
@@ -628,6 +631,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
}
}
+ blk_set_enable_write_cache(blk, !writethrough);
blk_set_on_error(blk, on_read_error, on_write_error);
if (!monitor_add_blk(blk, qemu_opts_id(opts), errp)) {
@@ -4129,6 +4133,10 @@ QemuOptsList qemu_common_drive_opts = {
.type = QEMU_OPT_STRING,
.help = "host AIO implementation (threads, native)",
},{
+ .name = BDRV_OPT_CACHE_WB,
+ .type = QEMU_OPT_BOOL,
+ .help = "Enable writeback mode",
+ },{
.name = "format",
.type = QEMU_OPT_STRING,
.help = "disk format (raw, qcow2, ...)",
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (6 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 07/20] block: blockdev_init(): " Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 18:36 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync() Kevin Wolf
` (11 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
All callers of blk_new_open() either don't rely on the WCE bit set after
blk_new_open() because they explicitly set it anyway, or they pass
BDRV_O_CACHE_WB unconditionally.
This patch changes blk_new_open() so that it always enables writeback
mode and asserts that BDRV_O_CACHE_WB is clear. For those callers that
used to pass BDRV_O_CACHE_WB unconditionally, the flag is removed now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/block-backend.c | 4 ++++
block/parallels.c | 3 +--
block/qcow.c | 3 +--
block/qcow2.c | 9 +++------
block/qed.c | 3 +--
block/sheepdog.c | 5 ++---
block/vdi.c | 3 +--
block/vhdx.c | 3 +--
block/vmdk.c | 8 +++-----
block/vpc.c | 3 +--
blockdev.c | 1 +
11 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index dca21d1..ffa5856 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -147,6 +147,8 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
BlockBackend *blk;
int ret;
+ assert((flags & BDRV_O_CACHE_WB) == 0);
+
blk = blk_new_with_bs(errp);
if (!blk) {
QDECREF(options);
@@ -159,6 +161,8 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
return NULL;
}
+ blk_set_enable_write_cache(blk, true);
+
return blk;
}
diff --git a/block/parallels.c b/block/parallels.c
index b322d05..fc18bd7 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -479,8 +479,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
}
file = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (file == NULL) {
error_propagate(errp, local_err);
return -EIO;
diff --git a/block/qcow.c b/block/qcow.c
index 73cf8a7..07dd867 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -794,8 +794,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
}
qcow_blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (qcow_blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/block/qcow2.c b/block/qcow2.c
index 5f4fea6..4363f78 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2160,8 +2160,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
return -EIO;
@@ -2225,8 +2224,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
options = qdict_new();
qdict_put(options, "driver", qstring_from_str("qcow2"));
blk = blk_new_open(filename, NULL, options,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_NO_FLUSH, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
@@ -2287,8 +2285,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
options = qdict_new();
qdict_put(options, "driver", qstring_from_str("qcow2"));
blk = blk_new_open(filename, NULL, options,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_BACKING,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_NO_BACKING, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/block/qed.c b/block/qed.c
index 5b24a97..091d207 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -575,8 +575,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
return -EIO;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index a3aeae4..4b1bfc9 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1647,8 +1647,7 @@ static int sd_prealloc(const char *filename, Error **errp)
int ret;
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- errp);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, errp);
if (blk == NULL) {
ret = -EIO;
goto out_with_err_set;
@@ -1844,7 +1843,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
}
blk = blk_new_open(backing_file, NULL, NULL,
- BDRV_O_PROTOCOL | BDRV_O_CACHE_WB, errp);
+ BDRV_O_PROTOCOL, errp);
if (blk == NULL) {
ret = -EIO;
goto out;
diff --git a/block/vdi.c b/block/vdi.c
index df9fa47..1791f22 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -769,8 +769,7 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/block/vhdx.c b/block/vhdx.c
index 78fe56c..7d1bddc 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1839,8 +1839,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp)
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/block/vmdk.c b/block/vmdk.c
index 80f0338..60f4d79 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1662,8 +1662,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
@@ -1947,7 +1946,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
}
blk = blk_new_open(full_backing, NULL, NULL,
- BDRV_O_NO_BACKING | BDRV_O_CACHE_WB, errp);
+ BDRV_O_NO_BACKING, errp);
g_free(full_backing);
if (blk == NULL) {
ret = -EIO;
@@ -2019,8 +2018,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
}
new_blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (new_blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/block/vpc.c b/block/vpc.c
index 8435205..0429528 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -889,8 +889,7 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp)
}
blk = blk_new_open(filename, NULL, NULL,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_PROTOCOL,
- &local_err);
+ BDRV_O_RDWR | BDRV_O_PROTOCOL, &local_err);
if (blk == NULL) {
error_propagate(errp, local_err);
ret = -EIO;
diff --git a/blockdev.c b/blockdev.c
index 70c3add..10eefda 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -596,6 +596,7 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, writethrough ? "off" : "on");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
+ assert((bdrv_flags & BDRV_O_CACHE_MASK) == 0);
if (runstate_check(RUN_STATE_INMIGRATE)) {
bdrv_flags |= BDRV_O_INACTIVE;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (7 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 18:46 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level Kevin Wolf
` (10 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
We don't want to silently ignore a flush error.
Also, there is little point in avoiding the flush for writethrough modes
and once WCE is moved to the BB layer, we definitely need the flush here
because bdrv_pwrite() won't involve one any more.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/io.c b/block/io.c
index 41d954ca..575da22 100644
--- a/block/io.c
+++ b/block/io.c
@@ -745,9 +745,9 @@ int bdrv_pwrite_sync(BlockDriverState *bs, int64_t offset,
return ret;
}
- /* No flush needed for cache modes that already do it */
- if (bs->enable_write_cache) {
- bdrv_flush(bs);
+ ret = bdrv_flush(bs);
+ if (ret < 0) {
+ return ret;
}
return 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (8 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 19:54 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache() Kevin Wolf
` (9 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Whether a write cache is used or not is a decision that concerns the
user (e.g. the guest device) rather than the backend. It was already
logically part of the BB level as bdrv_move_feature_fields() always kept
it on top of the BDS tree; with this patch, the core of it (the actual
flag and the additional flushes) is also implemented there.
Direct callers of bdrv_open() must pass BDRV_O_CACHE_WB now if bs
doesn't have a BlockBackend attached.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 26 +++++++++++++++++---------
block/block-backend.c | 42 +++++++++++++++++++++++++++---------------
block/io.c | 2 +-
block/iscsi.c | 2 +-
include/block/block.h | 1 +
include/block/block_int.h | 3 ---
tests/qemu-iotests/142 | 4 ++--
tests/qemu-iotests/142.out | 8 ++++----
8 files changed, 53 insertions(+), 35 deletions(-)
diff --git a/block.c b/block.c
index 172f865..9271dbb 100644
--- a/block.c
+++ b/block.c
@@ -2038,6 +2038,11 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
goto error;
}
}
+ if (!reopen_state->bs->blk && !(reopen_state->flags & BDRV_O_CACHE_WB)) {
+ error_setg(errp, "Cannot disable cache.writeback: No BlockBackend");
+ ret = -EINVAL;
+ goto error;
+ }
/* node-name and driver must be unchanged. Put them back into the QDict, so
* that they are checked at the end of this function. */
@@ -2138,10 +2143,10 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
reopen_state->bs->explicit_options = reopen_state->explicit_options;
reopen_state->bs->open_flags = reopen_state->flags;
- reopen_state->bs->enable_write_cache = !!(reopen_state->flags &
- BDRV_O_CACHE_WB);
reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
+ bdrv_set_enable_write_cache(reopen_state->bs,
+ !!(reopen_state->flags & BDRV_O_CACHE_WB));
bdrv_refresh_limits(reopen_state->bs, NULL);
}
@@ -2271,9 +2276,6 @@ static void bdrv_move_feature_fields(BlockDriverState *bs_dest,
BlockDriverState *bs_src)
{
/* move some fields that need to stay attached to the device */
-
- /* dev info */
- bs_dest->enable_write_cache = bs_src->enable_write_cache;
}
static void change_parent_backing_link(BlockDriverState *from,
@@ -2753,12 +2755,18 @@ int bdrv_is_sg(BlockDriverState *bs)
int bdrv_enable_write_cache(BlockDriverState *bs)
{
- return bs->enable_write_cache;
+ if (bs->blk) {
+ return blk_enable_write_cache(bs->blk);
+ } else {
+ return true;
+ }
}
void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
{
- bs->enable_write_cache = wce;
+ if (bs->blk) {
+ blk_set_enable_write_cache(bs->blk, wce);
+ }
/* so a reopen() will preserve wce */
if (wce) {
@@ -3618,8 +3626,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
}
/* backing files always opened read-only */
- back_flags =
- flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
+ back_flags = flags | BDRV_O_CACHE_WB;
+ back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
if (backing_fmt) {
backing_options = qdict_new();
diff --git a/block/block-backend.c b/block/block-backend.c
index ffa5856..4ef4b03 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -46,6 +46,8 @@ struct BlockBackend {
* can be used to restore those options in the new BDS on insert) */
BlockBackendRootState root_state;
+ bool enable_write_cache;
+
/* I/O stats (display with "info blockstats"). */
BlockAcctStats stats;
@@ -715,11 +717,17 @@ static int coroutine_fn blk_co_pwritev(BlockBackend *blk, int64_t offset,
unsigned int bytes, QEMUIOVector *qiov,
BdrvRequestFlags flags)
{
- int ret = blk_check_byte_request(blk, offset, bytes);
+ int ret;
+
+ ret = blk_check_byte_request(blk, offset, bytes);
if (ret < 0) {
return ret;
}
+ if (!blk->enable_write_cache) {
+ flags |= BDRV_REQ_FUA;
+ }
+
return bdrv_co_do_pwritev(blk_bs(blk), offset, bytes, qiov, flags);
}
@@ -1226,26 +1234,19 @@ int blk_is_sg(BlockBackend *blk)
int blk_enable_write_cache(BlockBackend *blk)
{
- BlockDriverState *bs = blk_bs(blk);
-
- if (bs) {
- return bdrv_enable_write_cache(bs);
- } else {
- return !!(blk->root_state.open_flags & BDRV_O_CACHE_WB);
- }
+ return blk->enable_write_cache;
}
void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
{
- BlockDriverState *bs = blk_bs(blk);
+ blk->enable_write_cache = wce;
- if (bs) {
- bdrv_set_enable_write_cache(bs, wce);
- } else {
+ /* TODO Remove this when BDRV_O_CACHE_WB isn't used any more */
+ if (blk->root) {
if (wce) {
- blk->root_state.open_flags |= BDRV_O_CACHE_WB;
+ blk->root->bs->open_flags |= BDRV_O_CACHE_WB;
} else {
- blk->root_state.open_flags &= ~BDRV_O_CACHE_WB;
+ blk->root->bs->open_flags &= ~BDRV_O_CACHE_WB;
}
}
}
@@ -1508,11 +1509,22 @@ int blk_discard(BlockBackend *blk, int64_t sector_num, int nb_sectors)
int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
int64_t pos, int size)
{
+ int ret;
+
if (!blk_is_available(blk)) {
return -ENOMEDIUM;
}
- return bdrv_save_vmstate(blk_bs(blk), buf, pos, size);
+ ret = bdrv_save_vmstate(blk_bs(blk), buf, pos, size);
+ if (ret < 0) {
+ return ret;
+ }
+
+ if (ret == size && !blk->enable_write_cache) {
+ ret = bdrv_flush(blk_bs(blk));
+ }
+
+ return ret < 0 ? ret : size;
}
int blk_load_vmstate(BlockBackend *blk, uint8_t *buf, int64_t pos, int size)
diff --git a/block/io.c b/block/io.c
index 575da22..14f12c8 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1156,7 +1156,7 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
}
bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);
- if (ret == 0 && !bs->enable_write_cache) {
+ if (ret == 0 && (flags & BDRV_REQ_FUA)) {
ret = bdrv_co_flush(bs);
}
diff --git a/block/iscsi.c b/block/iscsi.c
index 128ea79..3b54536 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -476,7 +476,7 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
iscsi_co_init_iscsitask(iscsilun, &iTask);
retry:
- fua = iscsilun->dpofua && !bs->enable_write_cache;
+ fua = iscsilun->dpofua && !bdrv_enable_write_cache(bs);
iTask.force_next_flush = !fua;
if (iscsilun->use_16_for_rw) {
iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
diff --git a/include/block/block.h b/include/block/block.h
index 823f4d7..1b8a1c7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -64,6 +64,7 @@ typedef enum {
*/
BDRV_REQ_MAY_UNMAP = 0x4,
BDRV_REQ_NO_SERIALISING = 0x8,
+ BDRV_REQ_FUA = 0x10,
} BdrvRequestFlags;
typedef struct BlockSizes {
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ba6e9ac..ce9d764 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -435,9 +435,6 @@ struct BlockDriverState {
/* Alignment requirement for offset/length of I/O requests */
unsigned int request_alignment;
- /* do we need to tell the quest if we have a volatile write cache? */
- int enable_write_cache;
-
/* the following member gives a name to every node on the bs graph. */
char node_name[32];
/* element of the list of named nodes building the graph */
diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
index 80834b5..517fb30 100755
--- a/tests/qemu-iotests/142
+++ b/tests/qemu-iotests/142
@@ -338,8 +338,8 @@ echo
# TODO Implement node-name support for 'qemu-io' HMP command for -c
# Can use only -o to access child node options for now
-hmp_cmds="qemu-io none0 \"reopen -o file.cache.writeback=off,file.cache.direct=off,file.cache.no-flush=off\"
-qemu-io none0 \"reopen -o backing.file.cache.writeback=on,backing.file.cache.direct=off,backing.file.cache.no-flush=on\"
+hmp_cmds="qemu-io none0 \"reopen -o file.cache.direct=off,file.cache.no-flush=off\"
+qemu-io none0 \"reopen -o backing.file.cache.direct=off,backing.file.cache.no-flush=on\"
qemu-io none0 \"reopen -c none\"
info block image
info block file
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
index 5dd5bd0..32dc802 100644
--- a/tests/qemu-iotests/142.out
+++ b/tests/qemu-iotests/142.out
@@ -132,7 +132,7 @@ cache.direct=on on backing-file
cache.writeback=off on none0
Cache mode: writethrough
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
@@ -342,7 +342,7 @@ cache.direct=on on backing-file
cache.writeback=off on none0
Cache mode: writeback, direct
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
@@ -503,7 +503,7 @@ cache.direct=on on backing-file
cache.writeback=off on blk
Cache mode: writethrough
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
@@ -707,7 +707,7 @@ cache.no-flush=on on backing-file
--- Change cache mode after reopening child ---
Cache mode: writeback, direct
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, ignore flushes
*** done
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (9 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:14 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags() Kevin Wolf
` (8 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Now that WCE is handled on the BlockBackend level, the flag is
meaningless for BDSes. As the schema requires us to fill the field,
we return an enabled write cache for them.
Note that this means that querying the BlockBackend name may return
writethrough as the cache information, whereas querying the node-name of
the root of that same BlockBackend will return writeback.
This may appear odd at first, but it actually makes sense because it
correctly repesents the layer that implements the WCE handling. This
becomes more apparent when you consider nodes that are the root node of
multiple BlockBackends, where each BB can have its own WCE setting.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 2 +-
block/qapi.c | 7 +++---
include/block/qapi.h | 3 ++-
tests/qemu-iotests/142 | 7 +++++-
tests/qemu-iotests/142.out | 57 ++++++++++++++++++++++++++++++++++++++--------
5 files changed, 60 insertions(+), 16 deletions(-)
diff --git a/block.c b/block.c
index 9271dbb..4e95d01 100644
--- a/block.c
+++ b/block.c
@@ -2917,7 +2917,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(Error **errp)
list = NULL;
QTAILQ_FOREACH(bs, &graph_bdrv_states, node_list) {
- BlockDeviceInfo *info = bdrv_block_device_info(bs, errp);
+ BlockDeviceInfo *info = bdrv_block_device_info(NULL, bs, errp);
if (!info) {
qapi_free_BlockDeviceInfoList(list);
return NULL;
diff --git a/block/qapi.c b/block/qapi.c
index 6a4869a..f01894a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -33,7 +33,8 @@
#include "qapi/qmp/types.h"
#include "sysemu/block-backend.h"
-BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)
+BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
+ BlockDriverState *bs, Error **errp)
{
ImageInfo **p_image_info;
BlockDriverState *bs0;
@@ -47,7 +48,7 @@ BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp)
info->cache = g_new(BlockdevCacheInfo, 1);
*info->cache = (BlockdevCacheInfo) {
- .writeback = bdrv_enable_write_cache(bs),
+ .writeback = blk ? blk_enable_write_cache(blk) : true,
.direct = !!(bs->open_flags & BDRV_O_NOCACHE),
.no_flush = !!(bs->open_flags & BDRV_O_NO_FLUSH),
};
@@ -342,7 +343,7 @@ static void bdrv_query_info(BlockBackend *blk, BlockInfo **p_info,
if (bs && bs->drv) {
info->has_inserted = true;
- info->inserted = bdrv_block_device_info(bs, errp);
+ info->inserted = bdrv_block_device_info(blk, bs, errp);
if (info->inserted == NULL) {
goto err;
}
diff --git a/include/block/qapi.h b/include/block/qapi.h
index 327549d..82ba4b6 100644
--- a/include/block/qapi.h
+++ b/include/block/qapi.h
@@ -29,7 +29,8 @@
#include "block/block.h"
#include "block/snapshot.h"
-BlockDeviceInfo *bdrv_block_device_info(BlockDriverState *bs, Error **errp);
+BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
+ BlockDriverState *bs, Error **errp);
int bdrv_query_snapshot_info_list(BlockDriverState *bs,
SnapshotInfoList **p_list,
Error **errp);
diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
index 517fb30..8bbbfde 100755
--- a/tests/qemu-iotests/142
+++ b/tests/qemu-iotests/142
@@ -134,7 +134,8 @@ echo
# First check the inherited cache mode after opening the image.
-hmp_cmds="info block image
+hmp_cmds="info block none0
+info block image
info block file
info block backing
info block backing-file"
@@ -164,6 +165,7 @@ echo
# new cache mode is specified in the flags, not as an option.
hmp_cmds='qemu-io none0 "reopen -c none"
+info block none0
info block image
info block file
info block backing
@@ -179,6 +181,7 @@ echo
# new cache mode is specified as an option, not in the flags.
hmp_cmds='qemu-io none0 "reopen -o cache.direct=on"
+info block none0
info block image
info block file
info block backing
@@ -214,6 +217,7 @@ echo
# options from its parent node.
hmp_cmds="qemu-io none0 \"reopen -o cache.writeback=off,cache.direct=on,cache.no-flush=on\"
+info block none0
info block image
info block blkdebug
info block file"
@@ -321,6 +325,7 @@ echo "--- Basic reopen ---"
echo
hmp_cmds='qemu-io none0 "reopen -o backing.cache.direct=on"
+info block none0
info block image
info block file
info block backing
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
index 32dc802..c922490 100644
--- a/tests/qemu-iotests/142.out
+++ b/tests/qemu-iotests/142.out
@@ -39,9 +39,11 @@ cache.direct=on on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on file
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback
Cache mode: writeback
@@ -49,6 +51,7 @@ cache.direct=on on file
cache.direct=on on backing
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -56,6 +59,7 @@ cache.direct=on on backing-file
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
@@ -64,6 +68,7 @@ cache.writeback=off on none0
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
cache.writeback=off on file
QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
@@ -80,9 +85,11 @@ cache.no-flush=on on none0
Cache mode: writeback, ignore flushes
Cache mode: writeback, ignore flushes
Cache mode: writeback, ignore flushes
+ Cache mode: writeback, ignore flushes
cache.no-flush=on on file
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, ignore flushes
Cache mode: writeback
Cache mode: writeback
@@ -90,6 +97,7 @@ cache.no-flush=on on file
cache.no-flush=on on backing
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, ignore flushes
Cache mode: writeback, ignore flushes
@@ -97,6 +105,7 @@ cache.no-flush=on on backing-file
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, ignore flushes
--- Cache modes after reopen (live snapshot) ---
@@ -182,24 +191,28 @@ cache.direct=on on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on backing
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on backing-file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.writeback=off on none0
@@ -207,6 +220,7 @@ cache.writeback=off on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.writeback=off on file
QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
@@ -223,9 +237,11 @@ cache.no-flush=on on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.no-flush=on on file
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -233,6 +249,7 @@ cache.no-flush=on on file
cache.no-flush=on on backing
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
@@ -240,6 +257,7 @@ cache.no-flush=on on backing-file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
--- Change cache modes with reopen (qemu-io command, options) ---
@@ -249,24 +267,28 @@ cache.direct=on on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on backing
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on backing-file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.writeback=off on none0
@@ -274,6 +296,7 @@ cache.writeback=off on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.writeback=off on file
QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
@@ -290,9 +313,11 @@ cache.no-flush=on on none0
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
+ Cache mode: writeback, direct, ignore flushes
cache.no-flush=on on file
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -300,6 +325,7 @@ cache.no-flush=on on file
cache.no-flush=on on backing
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
@@ -307,6 +333,7 @@ cache.no-flush=on on backing-file
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
--- Change cache modes after snapshot ---
@@ -389,6 +416,7 @@ cache.no-flush=on on backing-file
Cache mode: writethrough, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
+ Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, ignore flushes
=== Check that referenced BDSes don't inherit ===
@@ -422,28 +450,28 @@ cache.direct=on on backing-file
cache.writeback=off on blk
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
cache.writeback=off on file
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
cache.writeback=off on backing
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
cache.writeback=off on backing-file
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
cache.no-flush=on on blk
@@ -511,7 +539,7 @@ cache.writeback=off on blk
cache.writeback=off on file
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
@@ -519,7 +547,7 @@ cache.writeback=off on backing
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
cache.writeback=off on backing-file
@@ -527,7 +555,7 @@ cache.writeback=off on backing-file
Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
cache.no-flush=on on blk
@@ -593,21 +621,21 @@ cache.writeback=off on blk
cache.writeback=off on file
Cache mode: writeback, direct
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
Cache mode: writeback
cache.writeback=off on backing
Cache mode: writeback, direct
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
Cache mode: writeback
cache.writeback=off on backing-file
Cache mode: writeback, direct
Cache mode: writeback
Cache mode: writeback
- Cache mode: writethrough
+ Cache mode: writeback
cache.no-flush=on on blk
@@ -644,9 +672,11 @@ cache.direct=on on none0
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
+ Cache mode: writeback, direct
cache.direct=on on file
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -654,12 +684,14 @@ cache.direct=on on file
cache.direct=on on backing
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct
cache.direct=on on backing-file
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -667,6 +699,7 @@ cache.direct=on on backing-file
cache.writeback=off on none0
Cache mode: writethrough
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -683,11 +716,13 @@ QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t
cache.no-flush=on on none0
Cache mode: writeback, ignore flushes
Cache mode: writeback, ignore flushes
+ Cache mode: writeback, ignore flushes
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
cache.no-flush=on on file
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, ignore flushes
Cache mode: writeback, direct
Cache mode: writeback, direct
@@ -695,12 +730,14 @@ cache.no-flush=on on file
cache.no-flush=on on backing
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
cache.no-flush=on on backing-file
Cache mode: writeback
Cache mode: writeback
+ Cache mode: writeback
Cache mode: writeback, direct
Cache mode: writeback, direct, ignore flushes
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (10 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:24 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA Kevin Wolf
` (7 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
This function will allow drivers to implement BDRV_REQ_FUA natively
instead of sending a separate flush after the write.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 9 ++++++++-
include/block/block_int.h | 5 +++++
2 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index 14f12c8..cce508a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -1150,13 +1150,20 @@ static int coroutine_fn bdrv_aligned_pwritev(BlockDriverState *bs,
} else if (flags & BDRV_REQ_ZERO_WRITE) {
bdrv_debug_event(bs, BLKDBG_PWRITEV_ZERO);
ret = bdrv_co_do_write_zeroes(bs, sector_num, nb_sectors, flags);
+ } else if (drv->bdrv_co_writev_flags) {
+ bdrv_debug_event(bs, BLKDBG_PWRITEV);
+ ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
+ flags);
} else {
+ assert(drv->supported_write_flags == 0);
bdrv_debug_event(bs, BLKDBG_PWRITEV);
ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov);
}
bdrv_debug_event(bs, BLKDBG_PWRITEV_DONE);
- if (ret == 0 && (flags & BDRV_REQ_FUA)) {
+ if (ret == 0 && (flags & BDRV_REQ_FUA) &&
+ !(drv->supported_write_flags & BDRV_REQ_FUA))
+ {
ret = bdrv_co_flush(bs);
}
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ce9d764..d090646 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -155,6 +155,11 @@ struct BlockDriver {
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
int coroutine_fn (*bdrv_co_writev)(BlockDriverState *bs,
int64_t sector_num, int nb_sectors, QEMUIOVector *qiov);
+ int coroutine_fn (*bdrv_co_writev_flags)(BlockDriverState *bs,
+ int64_t sector_num, int nb_sectors, QEMUIOVector *qiov, int flags);
+
+ int supported_write_flags;
+
/*
* Efficiently zero a region of the disk image. Typically an image format
* would use a compact metadata representation to implement this. This
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (11 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:33 ` Max Reitz
2016-03-26 20:44 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 14/20] nbd: " Kevin Wolf
` (6 subsequent siblings)
19 siblings, 2 replies; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
This replaces the existing hack in the iscsi driver that sent the FUA
bit in writethrough mode and ignored the following flush in order to
optimise the number of roundtrips (see commit 73b5394e).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/iscsi.c | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/block/iscsi.c b/block/iscsi.c
index 3b54536..4f75204 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -70,7 +70,6 @@ typedef struct IscsiLun {
bool lbprz;
bool dpofua;
bool has_write_same;
- bool force_next_flush;
bool request_timed_out;
} IscsiLun;
@@ -84,7 +83,6 @@ typedef struct IscsiTask {
QEMUBH *bh;
IscsiLun *iscsilun;
QEMUTimer retry_timer;
- bool force_next_flush;
int err_code;
} IscsiTask;
@@ -282,8 +280,6 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
}
iTask->err_code = iscsi_translate_sense(&task->sense);
error_report("iSCSI Failure: %s", iscsi_get_error(iscsi));
- } else {
- iTask->iscsilun->force_next_flush |= iTask->force_next_flush;
}
out:
@@ -452,15 +448,15 @@ static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num,
}
}
-static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
- int64_t sector_num, int nb_sectors,
- QEMUIOVector *iov)
+static int coroutine_fn
+iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+ QEMUIOVector *iov, int flags)
{
IscsiLun *iscsilun = bs->opaque;
struct IscsiTask iTask;
uint64_t lba;
uint32_t num_sectors;
- int fua;
+ bool fua;
if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
return -EINVAL;
@@ -476,8 +472,7 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
iscsi_co_init_iscsitask(iscsilun, &iTask);
retry:
- fua = iscsilun->dpofua && !bdrv_enable_write_cache(bs);
- iTask.force_next_flush = !fua;
+ fua = iscsilun->dpofua && (flags & BDRV_REQ_FUA);
if (iscsilun->use_16_for_rw) {
iTask.task = iscsi_write16_task(iscsilun->iscsi, iscsilun->lun, lba,
NULL, num_sectors * iscsilun->block_size,
@@ -715,11 +710,6 @@ static int coroutine_fn iscsi_co_flush(BlockDriverState *bs)
IscsiLun *iscsilun = bs->opaque;
struct IscsiTask iTask;
- if (!iscsilun->force_next_flush) {
- return 0;
- }
- iscsilun->force_next_flush = false;
-
iscsi_co_init_iscsitask(iscsilun, &iTask);
retry:
if (iscsi_synchronizecache10_task(iscsilun->iscsi, iscsilun->lun, 0, 0, 0,
@@ -1019,7 +1009,6 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
}
iscsi_co_init_iscsitask(iscsilun, &iTask);
- iTask.force_next_flush = true;
retry:
if (use_16_for_ws) {
iTask.task = iscsi_writesame16_task(iscsilun->iscsi, iscsilun->lun, lba,
@@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi = {
.bdrv_co_discard = iscsi_co_discard,
.bdrv_co_write_zeroes = iscsi_co_write_zeroes,
.bdrv_co_readv = iscsi_co_readv,
- .bdrv_co_writev = iscsi_co_writev,
+ .bdrv_co_writev_flags = iscsi_co_writev_flags,
+ .supported_write_flags = BDRV_REQ_FUA,
.bdrv_co_flush_to_disk = iscsi_co_flush,
#ifdef __linux__
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 14/20] nbd: Support BDRV_REQ_FUA
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (12 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:46 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 15/20] raw: " Kevin Wolf
` (5 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
The NBD server already used to send a FUA flag when the writethrough
mode was set. This code was a remnant from the times where protocol
drivers actually had to implement writethrough modes. Since nowadays the
block layer sends flushes in writethrough mode and non-root nodes are
always writeback, this was mostly dead code - only mostly because if NBD
was configured to be used without a format, we sent _both_ FUA and an
explicit flush afterwards, which makes the code not technically dead,
but useless overhead.
This patch changes the code so that the block layer's FUA flag is
recognised and translated into a NBD FUA flag. The additional flush is
avoided now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/nbd-client.c | 13 +++++++------
block/nbd-client.h | 2 +-
block/nbd.c | 26 +++++++++++++++++++++-----
3 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/block/nbd-client.c b/block/nbd-client.c
index 6a9b4c7..021a88b 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -243,15 +243,15 @@ static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov,
- int offset)
+ int offset, int *flags)
{
NbdClientSession *client = nbd_get_client_session(bs);
struct nbd_request request = { .type = NBD_CMD_WRITE };
struct nbd_reply reply;
ssize_t ret;
- if (!bdrv_enable_write_cache(bs) &&
- (client->nbdflags & NBD_FLAG_SEND_FUA)) {
+ if ((*flags & BDRV_REQ_FUA) && (client->nbdflags & NBD_FLAG_SEND_FUA)) {
+ *flags &= ~BDRV_REQ_FUA;
request.type |= NBD_CMD_FLAG_FUA;
}
@@ -291,12 +291,13 @@ int nbd_client_co_readv(BlockDriverState *bs, int64_t sector_num,
}
int nbd_client_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov, int *flags)
{
int offset = 0;
int ret;
while (nb_sectors > NBD_MAX_SECTORS) {
- ret = nbd_co_writev_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
+ ret = nbd_co_writev_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset,
+ flags);
if (ret < 0) {
return ret;
}
@@ -304,7 +305,7 @@ int nbd_client_co_writev(BlockDriverState *bs, int64_t sector_num,
sector_num += NBD_MAX_SECTORS;
nb_sectors -= NBD_MAX_SECTORS;
}
- return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset);
+ return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset, flags);
}
int nbd_client_co_flush(BlockDriverState *bs)
diff --git a/block/nbd-client.h b/block/nbd-client.h
index 53f116d..bc7aec0 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -48,7 +48,7 @@ int nbd_client_co_discard(BlockDriverState *bs, int64_t sector_num,
int nb_sectors);
int nbd_client_co_flush(BlockDriverState *bs);
int nbd_client_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov);
+ int nb_sectors, QEMUIOVector *qiov, int *flags);
int nbd_client_co_readv(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, QEMUIOVector *qiov);
diff --git a/block/nbd.c b/block/nbd.c
index 9f333c9..87887c4 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -355,9 +355,22 @@ static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
}
static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+ int nb_sectors, QEMUIOVector *qiov, int flags)
{
- return nbd_client_co_writev(bs, sector_num, nb_sectors, qiov);
+ int ret;
+
+ ret = nbd_client_co_writev(bs, sector_num, nb_sectors, qiov, &flags);
+ if (ret < 0) {
+ return ret;
+ }
+
+ /* The flag wasn't sent to the server, so we need to emulate it with an
+ * explicit flush */
+ if (flags & BDRV_REQ_FUA) {
+ ret = nbd_client_co_flush(bs);
+ }
+
+ return ret;
}
static int nbd_co_flush(BlockDriverState *bs)
@@ -456,7 +469,8 @@ static BlockDriver bdrv_nbd = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
+ .bdrv_co_writev_flags = nbd_co_writev,
+ .supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_discard = nbd_co_discard,
@@ -474,7 +488,8 @@ static BlockDriver bdrv_nbd_tcp = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
+ .bdrv_co_writev_flags = nbd_co_writev,
+ .supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_discard = nbd_co_discard,
@@ -492,7 +507,8 @@ static BlockDriver bdrv_nbd_unix = {
.bdrv_parse_filename = nbd_parse_filename,
.bdrv_file_open = nbd_open,
.bdrv_co_readv = nbd_co_readv,
- .bdrv_co_writev = nbd_co_writev,
+ .bdrv_co_writev_flags = nbd_co_writev,
+ .supported_write_flags = BDRV_REQ_FUA,
.bdrv_close = nbd_close,
.bdrv_co_flush_to_os = nbd_co_flush,
.bdrv_co_discard = nbd_co_discard,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 15/20] raw: Support BDRV_REQ_FUA
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (13 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 14/20] nbd: " Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:49 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init() Kevin Wolf
` (4 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Pass through the FUA flag to the lower layer so that the separate flush
can be saved in practically relevant cases where a (raw) format driver
sits on top of the protocol driver.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/raw_bsd.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/block/raw_bsd.c b/block/raw_bsd.c
index ea16a23..4e802a7 100644
--- a/block/raw_bsd.c
+++ b/block/raw_bsd.c
@@ -56,8 +56,9 @@ static int coroutine_fn raw_co_readv(BlockDriverState *bs, int64_t sector_num,
return bdrv_co_readv(bs->file->bs, sector_num, nb_sectors, qiov);
}
-static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
- int nb_sectors, QEMUIOVector *qiov)
+static int coroutine_fn
+raw_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors,
+ QEMUIOVector *qiov, int flags)
{
void *buf = NULL;
BlockDriver *drv;
@@ -103,7 +104,8 @@ static int coroutine_fn raw_co_writev(BlockDriverState *bs, int64_t sector_num,
}
BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
- ret = bdrv_co_writev(bs->file->bs, sector_num, nb_sectors, qiov);
+ ret = bdrv_co_do_pwritev(bs->file->bs, sector_num * BDRV_SECTOR_SIZE,
+ nb_sectors * BDRV_SECTOR_SIZE, qiov, flags);
fail:
if (qiov == &local_qiov) {
@@ -246,7 +248,8 @@ BlockDriver bdrv_raw = {
.bdrv_close = &raw_close,
.bdrv_create = &raw_create,
.bdrv_co_readv = &raw_co_readv,
- .bdrv_co_writev = &raw_co_writev,
+ .bdrv_co_writev_flags = &raw_co_writev_flags,
+ .supported_write_flags = BDRV_REQ_FUA,
.bdrv_co_write_zeroes = &raw_co_write_zeroes,
.bdrv_co_discard = &raw_co_discard,
.bdrv_co_get_block_status = &raw_co_get_block_status,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (14 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 15/20] raw: " Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 20:53 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() Kevin Wolf
` (3 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockdev.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/blockdev.c b/blockdev.c
index 10eefda..ed2a5c7 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -897,8 +897,9 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
value = qemu_opt_get(all_opts, "cache");
if (value) {
int flags = 0;
+ bool writethrough;
- if (bdrv_parse_cache_flags(value, &flags) != 0) {
+ if (bdrv_parse_cache_mode(value, &flags, &writethrough) != 0) {
error_report("invalid cache option");
return NULL;
}
@@ -906,7 +907,7 @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
/* Specific options take precedence */
if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_WB)) {
qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_WB,
- !!(flags & BDRV_O_CACHE_WB), &error_abort);
+ !writethrough, &error_abort);
}
if (!qemu_opt_get(all_opts, BDRV_OPT_CACHE_DIRECT)) {
qemu_opt_set_bool(all_opts, BDRV_OPT_CACHE_DIRECT,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (15 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 21:05 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags() Kevin Wolf
` (2 subsequent siblings)
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
We must forbid changing the WCE flag in bdrv_reopen() in the same patch,
as otherwise the behaviour would change so that the flag takes
precedence over the explicitly specified option.
The correct value of the WCE flag depends on the BlockBackend user (e.g.
guest device) and isn't a decision that the QMP client makes, so this
change is what we want.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 18 ++++++------------
qemu-io-cmds.c | 14 +++++++++++++-
tests/qemu-iotests/142 | 2 +-
tests/qemu-iotests/142.out | 2 +-
4 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/block.c b/block.c
index 4e95d01..3775e65 100644
--- a/block.c
+++ b/block.c
@@ -2028,18 +2028,12 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
update_flags_from_options(&reopen_state->flags, opts);
- /* If a guest device is attached, it owns WCE */
- if (reopen_state->bs->blk && blk_get_attached_dev(reopen_state->bs->blk)) {
- bool old_wce = bdrv_enable_write_cache(reopen_state->bs);
- bool new_wce = (reopen_state->flags & BDRV_O_CACHE_WB);
- if (old_wce != new_wce) {
- error_setg(errp, "Cannot change cache.writeback: Device attached");
- ret = -EINVAL;
- goto error;
- }
- }
- if (!reopen_state->bs->blk && !(reopen_state->flags & BDRV_O_CACHE_WB)) {
- error_setg(errp, "Cannot disable cache.writeback: No BlockBackend");
+ /* WCE is a BlockBackend level option, can't change it */
+ bool old_wce = bdrv_enable_write_cache(reopen_state->bs);
+ bool new_wce = (reopen_state->flags & BDRV_O_CACHE_WB);
+
+ if (old_wce != new_wce) {
+ error_setg(errp, "Cannot change cache.writeback");
ret = -EINVAL;
goto error;
}
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index e929d24..7de3754 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2104,6 +2104,7 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
QDict *opts;
int c;
int flags = bs->open_flags;
+ bool writethrough = !blk_enable_write_cache(blk);
BlockReopenQueue *brq;
Error *local_err = NULL;
@@ -2111,7 +2112,7 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
while ((c = getopt(argc, argv, "c:o:r")) != -1) {
switch (c) {
case 'c':
- if (bdrv_parse_cache_flags(optarg, &flags) < 0) {
+ if (bdrv_parse_cache_mode(optarg, &flags, &writethrough) < 0) {
error_report("Invalid cache option: %s", optarg);
return 0;
}
@@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
return qemuio_command_usage(&reopen_cmd);
}
+ if (writethrough != blk_enable_write_cache(blk) &&
+ blk_get_attached_dev(blk))
+ {
+ error_report("Cannot change cache.writeback: Device attached");
+ qemu_opts_reset(&reopen_opts);
+ return 0;
+ }
+
qopts = qemu_opts_find(&reopen_opts, NULL);
opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
qemu_opts_reset(&reopen_opts);
+ flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
brq = bdrv_reopen_queue(NULL, bs, opts, flags);
bdrv_reopen_multiple(brq, &local_err);
if (local_err) {
error_report_err(local_err);
+ } else {
+ blk_set_enable_write_cache(blk, !writethrough);
}
return 0;
diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
index 8bbbfde..a035747 100755
--- a/tests/qemu-iotests/142
+++ b/tests/qemu-iotests/142
@@ -216,7 +216,7 @@ echo
# BDS initialised with the json: pseudo-protocol, but still have it inherit
# options from its parent node.
-hmp_cmds="qemu-io none0 \"reopen -o cache.writeback=off,cache.direct=on,cache.no-flush=on\"
+hmp_cmds="qemu-io none0 \"reopen -o cache.direct=on,cache.no-flush=on\"
info block none0
info block image
info block blkdebug
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
index c922490..3d5ef5f 100644
--- a/tests/qemu-iotests/142.out
+++ b/tests/qemu-iotests/142.out
@@ -414,7 +414,7 @@ cache.no-flush=on on backing-file
--- Change cache mode in parent, child has explicit option in JSON ---
- Cache mode: writethrough, direct, ignore flushes
+ Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, direct, ignore flushes
Cache mode: writeback, ignore flushes
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (16 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 21:06 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache() Kevin Wolf
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
All users are converted to bdrv_parse_cache_mode() now.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 29 +++++++----------------------
include/block/block.h | 1 -
2 files changed, 7 insertions(+), 23 deletions(-)
diff --git a/block.c b/block.c
index 3775e65..7cae704 100644
--- a/block.c
+++ b/block.c
@@ -639,21 +639,23 @@ int bdrv_parse_discard_flags(const char *mode, int *flags)
*
* Return 0 on success, -1 if the cache mode was invalid.
*/
-int bdrv_parse_cache_flags(const char *mode, int *flags)
+int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
{
*flags &= ~BDRV_O_CACHE_MASK;
if (!strcmp(mode, "off") || !strcmp(mode, "none")) {
- *flags |= BDRV_O_NOCACHE | BDRV_O_CACHE_WB;
+ *writethrough = false;
+ *flags |= BDRV_O_NOCACHE;
} else if (!strcmp(mode, "directsync")) {
+ *writethrough = true;
*flags |= BDRV_O_NOCACHE;
} else if (!strcmp(mode, "writeback")) {
- *flags |= BDRV_O_CACHE_WB;
+ *writethrough = false;
} else if (!strcmp(mode, "unsafe")) {
- *flags |= BDRV_O_CACHE_WB;
+ *writethrough = false;
*flags |= BDRV_O_NO_FLUSH;
} else if (!strcmp(mode, "writethrough")) {
- /* this is the default */
+ *writethrough = true;
} else {
return -1;
}
@@ -661,23 +663,6 @@ int bdrv_parse_cache_flags(const char *mode, int *flags)
return 0;
}
-int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough)
-{
- int ret = bdrv_parse_cache_flags(mode, flags);
- if (ret < 0) {
- return ret;
- }
-
- if (*flags & BDRV_O_CACHE_WB) {
- *flags &= ~BDRV_O_CACHE_WB;
- *writethrough = false;
- } else {
- *writethrough = true;
- }
-
- return 0;
-}
-
/*
* Returns the options and flags that a temporary snapshot should get, based on
* the originally requested flags (the originally requested image will have
diff --git a/include/block/block.h b/include/block/block.h
index 1b8a1c7..c69204a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -206,7 +206,6 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
void bdrv_replace_in_backing_chain(BlockDriverState *old,
BlockDriverState *new);
-int bdrv_parse_cache_flags(const char *mode, int *flags);
int bdrv_parse_cache_mode(const char *mode, int *flags, bool *writethrough);
int bdrv_parse_discard_flags(const char *mode, int *flags);
BdrvChild *bdrv_open_child(const char *filename,
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (17 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags() Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 21:23 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache() Kevin Wolf
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
The previous patches have successively made blk->enable_write_cache the
true source for the information whether a writethrough mode must be
implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
we're carrying around, so now's the time to remove it.
At the same time, we remove the 'cache.writeback' option parsing on the
BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.
This change requires test cases that explicitly enabled the option to
drop it. Other than that and the change of the error message when
writethrough is enabled on the BDS level (from "Can't set writethrough
mode" to "doesn't support the option"), there should be no change in
behaviour.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 48 ++-----------------------------------------
block/block-backend.c | 11 ----------
block/vvfat.c | 3 +--
blockdev.c | 21 ++-----------------
include/block/block.h | 3 +--
qemu-img.c | 2 +-
qemu-io-cmds.c | 1 -
tests/qemu-iotests/051 | 2 +-
tests/qemu-iotests/051.pc.out | 10 ++++-----
tests/qemu-iotests/142 | 6 +++---
tests/qemu-iotests/142.out | 36 ++++++++++++++++----------------
11 files changed, 34 insertions(+), 109 deletions(-)
diff --git a/block.c b/block.c
index 7cae704..0e7cbb6 100644
--- a/block.c
+++ b/block.c
@@ -674,7 +674,6 @@ static void bdrv_temp_snapshot_options(int *child_flags, QDict *child_options,
*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");
}
@@ -699,7 +698,6 @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,
/* Our block drivers take care to send flushes and respect unmap policy,
* so we can default to enable both on lower layers regardless of the
* corresponding parent options. */
- qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
flags |= BDRV_O_UNMAP;
/* Clear flags that only apply to the top layer */
@@ -741,7 +739,6 @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,
/* The cache mode is inherited unmodified for backing files; except WCE,
* which is only applied on the top level (BlockBackend) */
- qdict_set_default_str(child_options, BDRV_OPT_CACHE_WB, "on");
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_DIRECT);
qdict_copy_default(child_options, parent_options, BDRV_OPT_CACHE_NO_FLUSH);
@@ -760,7 +757,7 @@ static const BdrvChildRole child_backing = {
static int bdrv_open_flags(BlockDriverState *bs, int flags)
{
- int open_flags = flags | BDRV_O_CACHE_WB;
+ int open_flags = flags;
/*
* Clear flags that are internal to the block layer before opening the
@@ -782,11 +779,6 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
{
*flags &= ~BDRV_O_CACHE_MASK;
- assert(qemu_opt_find(opts, BDRV_OPT_CACHE_WB));
- if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_WB, false)) {
- *flags |= BDRV_O_CACHE_WB;
- }
-
assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH));
if (qemu_opt_get_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) {
*flags |= BDRV_O_NO_FLUSH;
@@ -800,10 +792,6 @@ static void update_flags_from_options(int *flags, QemuOpts *opts)
static void update_options_from_flags(QDict *options, int flags)
{
- if (!qdict_haskey(options, BDRV_OPT_CACHE_WB)) {
- qdict_put(options, BDRV_OPT_CACHE_WB,
- qbool_from_bool(flags & BDRV_O_CACHE_WB));
- }
if (!qdict_haskey(options, BDRV_OPT_CACHE_DIRECT)) {
qdict_put(options, BDRV_OPT_CACHE_DIRECT,
qbool_from_bool(flags & BDRV_O_NOCACHE));
@@ -866,11 +854,6 @@ static QemuOptsList bdrv_runtime_opts = {
.help = "Block driver to use for the node",
},
{
- .name = BDRV_OPT_CACHE_WB,
- .type = QEMU_OPT_BOOL,
- .help = "Enable writeback mode",
- },
- {
.name = BDRV_OPT_CACHE_DIRECT,
.type = QEMU_OPT_BOOL,
.help = "Bypass software writeback cache on the host",
@@ -977,14 +960,6 @@ static int bdrv_open_common(BlockDriverState *bs, BdrvChild *file,
/* Apply cache mode options */
update_flags_from_options(&bs->open_flags, opts);
- if (!bs->blk && (bs->open_flags & BDRV_O_CACHE_WB) == 0) {
- error_setg(errp, "Can't set writethrough mode except for the root");
- ret = -EINVAL;
- goto free_and_fail;
- }
-
- bdrv_set_enable_write_cache(bs, bs->open_flags & BDRV_O_CACHE_WB);
-
/* Open the image, either directly or using a protocol */
open_flags = bdrv_open_flags(bs, bs->open_flags);
if (drv->bdrv_file_open) {
@@ -2013,16 +1988,6 @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,
update_flags_from_options(&reopen_state->flags, opts);
- /* WCE is a BlockBackend level option, can't change it */
- bool old_wce = bdrv_enable_write_cache(reopen_state->bs);
- bool new_wce = (reopen_state->flags & BDRV_O_CACHE_WB);
-
- if (old_wce != new_wce) {
- error_setg(errp, "Cannot change cache.writeback");
- ret = -EINVAL;
- goto error;
- }
-
/* node-name and driver must be unchanged. Put them back into the QDict, so
* that they are checked at the end of this function. */
value = qemu_opt_get(opts, "node-name");
@@ -2124,8 +2089,6 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
reopen_state->bs->open_flags = reopen_state->flags;
reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
- bdrv_set_enable_write_cache(reopen_state->bs,
- !!(reopen_state->flags & BDRV_O_CACHE_WB));
bdrv_refresh_limits(reopen_state->bs, NULL);
}
@@ -2746,13 +2709,6 @@ void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
if (bs->blk) {
blk_set_enable_write_cache(bs->blk, wce);
}
-
- /* so a reopen() will preserve wce */
- if (wce) {
- bs->open_flags |= BDRV_O_CACHE_WB;
- } else {
- bs->open_flags &= ~BDRV_O_CACHE_WB;
- }
}
int bdrv_is_encrypted(BlockDriverState *bs)
@@ -3605,7 +3561,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
}
/* backing files always opened read-only */
- back_flags = flags | BDRV_O_CACHE_WB;
+ back_flags = flags;
back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
if (backing_fmt) {
diff --git a/block/block-backend.c b/block/block-backend.c
index 4ef4b03..5ca4e8d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -149,8 +149,6 @@ BlockBackend *blk_new_open(const char *filename, const char *reference,
BlockBackend *blk;
int ret;
- assert((flags & BDRV_O_CACHE_WB) == 0);
-
blk = blk_new_with_bs(errp);
if (!blk) {
QDECREF(options);
@@ -1240,15 +1238,6 @@ int blk_enable_write_cache(BlockBackend *blk)
void blk_set_enable_write_cache(BlockBackend *blk, bool wce)
{
blk->enable_write_cache = wce;
-
- /* TODO Remove this when BDRV_O_CACHE_WB isn't used any more */
- if (blk->root) {
- if (wce) {
- blk->root->bs->open_flags |= BDRV_O_CACHE_WB;
- } else {
- blk->root->bs->open_flags &= ~BDRV_O_CACHE_WB;
- }
- }
}
void blk_invalidate_cache(BlockBackend *blk, Error **errp)
diff --git a/block/vvfat.c b/block/vvfat.c
index b8d29e1..d5dbb89 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2956,8 +2956,7 @@ static int enable_write_target(BDRVVVFATState *s, Error **errp)
options = qdict_new();
qdict_put(options, "driver", qstring_from_str("qcow"));
ret = bdrv_open(&s->qcow, s->qcow_filename, NULL, options,
- BDRV_O_RDWR | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH,
- errp);
+ BDRV_O_RDWR | BDRV_O_NO_FLUSH, errp);
if (ret < 0) {
goto err;
}
diff --git a/blockdev.c b/blockdev.c
index ed2a5c7..d23256a 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -593,7 +593,6 @@ static BlockBackend *blockdev_init(const char *file, QDict *bs_opts,
/* bdrv_open() defaults to the values in bdrv_flags (for compatibility
* with other callers) rather than what we want as the real defaults.
* Apply the defaults here instead. */
- qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, writethrough ? "off" : "on");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
assert((bdrv_flags & BDRV_O_CACHE_MASK) == 0);
@@ -689,7 +688,6 @@ static BlockDriverState *bds_tree_init(QDict *bs_opts, Error **errp)
/* bdrv_open() defaults to the values in bdrv_flags (for compatibility
* with other callers) rather than what we want as the real defaults.
* Apply the defaults here instead. */
- qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_WB, "on");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_DIRECT, "off");
qdict_set_default_str(bs_opts, BDRV_OPT_CACHE_NO_FLUSH, "off");
@@ -1777,12 +1775,6 @@ static void external_snapshot_prepare(BlkActionState *common,
flags |= BDRV_O_NO_BACKING;
}
- /* There is no BB attached during bdrv_open(), so we can't set a
- * writethrough mode. bdrv_append() will swap the WCE setting so that the
- * backing file becomes unconditionally writeback (which is what backing
- * files should always be) and the new overlay gets the original setting. */
- flags |= BDRV_O_CACHE_WB;
-
assert(state->new_bs == NULL);
ret = bdrv_open(&state->new_bs, new_image_file, snapshot_ref, options,
flags, errp);
@@ -2527,7 +2519,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
BlockBackend *blk;
BlockDriverState *medium_bs = NULL;
int bdrv_flags, ret;
- bool writethrough;
QDict *options = NULL;
Error *err = NULL;
@@ -2546,12 +2537,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
bdrv_flags &= ~(BDRV_O_TEMPORARY | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING |
BDRV_O_PROTOCOL);
- /* Must open the image in writeback mode as long as no BlockBackend is
- * attached. The right mode can be set as the final step after changing the
- * medium. */
- writethrough = !(bdrv_flags & BDRV_O_CACHE_WB);
- bdrv_flags |= BDRV_O_CACHE_WB;
-
if (!has_read_only) {
read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;
}
@@ -2609,8 +2594,6 @@ void qmp_blockdev_change_medium(const char *device, const char *filename,
goto fail;
}
- bdrv_set_enable_write_cache(medium_bs, !writethrough);
-
qmp_blockdev_close_tray(device, errp);
fail:
@@ -3236,7 +3219,7 @@ static void do_drive_backup(const char *device, const char *target,
goto out;
}
- flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR;
+ flags = bs->open_flags | BDRV_O_RDWR;
/* See if we have a backing HD we can use to create our new image
* on top of. */
@@ -3531,7 +3514,7 @@ void qmp_drive_mirror(const char *device, const char *target,
format = mode == NEW_IMAGE_MODE_EXISTING ? NULL : bs->drv->format_name;
}
- flags = bs->open_flags | BDRV_O_CACHE_WB | BDRV_O_RDWR;
+ flags = bs->open_flags | BDRV_O_RDWR;
source = backing_bs(bs);
if (!source && sync == MIRROR_SYNC_MODE_TOP) {
sync = MIRROR_SYNC_MODE_FULL;
diff --git a/include/block/block.h b/include/block/block.h
index c69204a..bd596b1 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -82,7 +82,6 @@ typedef struct HDGeometry {
#define BDRV_O_SNAPSHOT 0x0008 /* open the file read only and save writes in a snapshot */
#define BDRV_O_TEMPORARY 0x0010 /* delete the file after use */
#define BDRV_O_NOCACHE 0x0020 /* do not use the host page cache */
-#define BDRV_O_CACHE_WB 0x0040 /* use write-back caching */
#define BDRV_O_NATIVE_AIO 0x0080 /* use native AIO instead of the thread pool */
#define BDRV_O_NO_BACKING 0x0100 /* don't open the backing file */
#define BDRV_O_NO_FLUSH 0x0200 /* disable flushing on this disk */
@@ -95,7 +94,7 @@ typedef struct HDGeometry {
select an appropriate protocol driver,
ignoring the format layer */
-#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_CACHE_WB | BDRV_O_NO_FLUSH)
+#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)
/* Option names of options parsed by the block layer */
diff --git a/qemu-img.c b/qemu-img.c
index 96b51d4..c91a322 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -462,7 +462,7 @@ static int img_create(int argc, char **argv)
}
bdrv_img_create(filename, fmt, base_filename, base_fmt,
- options, img_size, BDRV_O_CACHE_WB, &local_err, quiet);
+ options, img_size, 0, &local_err, quiet);
if (local_err) {
error_reportf_err(local_err, "%s: ", filename);
goto fail;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 7de3754..35ee50b 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2149,7 +2149,6 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
qemu_opts_reset(&reopen_opts);
- flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
brq = bdrv_reopen_queue(NULL, bs, opts, flags);
bdrv_reopen_multiple(brq, &local_err);
if (local_err) {
diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
index 7bfe9ff..88b3d91 100755
--- a/tests/qemu-iotests/051
+++ b/tests/qemu-iotests/051
@@ -218,7 +218,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
for cache in writeback writethrough unsafe invalid_value; do
echo -e "info block\ninfo block file\ninfo block backing\ninfo block backing-file" | \
- run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
+ run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
done
echo
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 73cc15a..ec6d222 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -239,7 +239,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
Testing: -drive driver=null-co,cache=invalid_value
QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
-Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -259,7 +259,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Cache mode: writeback, ignore flushes
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
-Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -279,7 +279,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Cache mode: writeback, ignore flushes
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
-Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
QEMU X.Y.Z monitor - type 'help' for more information
(qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
@@ -299,8 +299,8 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
Cache mode: writeback, ignore flushes
(qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
-Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
-QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
+Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
+QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
=== Specifying the protocol layer ===
diff --git a/tests/qemu-iotests/142 b/tests/qemu-iotests/142
index a035747..3828c23 100755
--- a/tests/qemu-iotests/142
+++ b/tests/qemu-iotests/142
@@ -110,11 +110,11 @@ function check_cache_all()
echo -e "\n\ncache.writeback=off on none0"
echo "$hmp_cmds" | run_qemu -drive "$files","$ids",cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
echo -e "\ncache.writeback=off on file"
- echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+ echo "$hmp_cmds" | run_qemu -drive "$files","$ids",file.cache.writeback=off | grep -e "doesn't" -e "does not"
echo -e "\ncache.writeback=off on backing"
- echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+ echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.cache.writeback=off | grep -e "doesn't" -e "does not"
echo -e "\ncache.writeback=off on backing-file"
- echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep -e "Cache" -e "[Cc]annot\|[Cc]ould not\|[Cc]an't"
+ echo "$hmp_cmds" | run_qemu -drive "$files","$ids",backing.file.cache.writeback=off | grep -e "doesn't" -e "does not"
# cache.no-flush is supposed to be inherited by both bs->file and bs->backing
diff --git a/tests/qemu-iotests/142.out b/tests/qemu-iotests/142.out
index 3d5ef5f..600beca 100644
--- a/tests/qemu-iotests/142.out
+++ b/tests/qemu-iotests/142.out
@@ -71,13 +71,13 @@ cache.writeback=off on none0
Cache mode: writeback
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
@@ -147,13 +147,13 @@ cache.writeback=off on none0
Cache mode: writeback
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
@@ -223,13 +223,13 @@ cache.writeback=off on none0
Cache mode: writeback, direct
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
@@ -299,13 +299,13 @@ cache.writeback=off on none0
Cache mode: writeback, direct
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
@@ -375,13 +375,13 @@ cache.writeback=off on none0
Cache mode: writeback
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
@@ -704,13 +704,13 @@ cache.writeback=off on none0
Cache mode: writeback, direct
cache.writeback=off on file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,file.cache.writeback=off: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.writeback=off on backing
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.cache.writeback=off: Could not open backing file: Block format 'qcow2' does not support the option 'cache.writeback'
cache.writeback=off on backing-file
-QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Can't set writethrough mode except for the root
+QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2,backing.file.filename=TEST_DIR/t.qcow2.base,node-name=image,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,backing.file.cache.writeback=off: Could not open backing file: Block protocol 'file' doesn't support the option 'cache.writeback'
cache.no-flush=on on none0
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache()
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
` (18 preceding siblings ...)
2016-03-18 18:21 ` [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB Kevin Wolf
@ 2016-03-18 18:21 ` Kevin Wolf
2016-03-26 21:25 ` Max Reitz
19 siblings, 1 reply; 45+ messages in thread
From: Kevin Wolf @ 2016-03-18 18:21 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
The only remaining users were block jobs (mirror and backup) which
unconditionally enabled WCE on the BlockBackend of the target image. As
these block jobs don't go through BlockBackend for their I/O requests,
they aren't affected by this setting anyway but always get a writeback
mode, so that call can be removed.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 16 ----------------
block/backup.c | 1 -
block/mirror.c | 1 -
include/block/block.h | 2 --
4 files changed, 20 deletions(-)
diff --git a/block.c b/block.c
index 0e7cbb6..c22cb3f 100644
--- a/block.c
+++ b/block.c
@@ -2695,22 +2695,6 @@ int bdrv_is_sg(BlockDriverState *bs)
return bs->sg;
}
-int bdrv_enable_write_cache(BlockDriverState *bs)
-{
- if (bs->blk) {
- return blk_enable_write_cache(bs->blk);
- } else {
- return true;
- }
-}
-
-void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce)
-{
- if (bs->blk) {
- blk_set_enable_write_cache(bs->blk, wce);
- }
-}
-
int bdrv_is_encrypted(BlockDriverState *bs)
{
if (bs->backing && bs->backing->bs->encrypted) {
diff --git a/block/backup.c b/block/backup.c
index ab3e345..10397e2 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -402,7 +402,6 @@ static void coroutine_fn backup_run(void *opaque)
job->done_bitmap = bitmap_new(end);
- bdrv_set_enable_write_cache(target, true);
if (target->blk) {
blk_set_on_error(target->blk, on_target_error, on_target_error);
blk_iostatus_enable(target->blk);
diff --git a/block/mirror.c b/block/mirror.c
index 9635fa8..da18c0b 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -855,7 +855,6 @@ static void mirror_start_job(BlockDriverState *bs, BlockDriverState *target,
bdrv_op_block_all(s->target, s->common.blocker);
- bdrv_set_enable_write_cache(s->target, true);
if (s->target->blk) {
blk_set_on_error(s->target->blk, on_target_error, on_target_error);
blk_iostatus_enable(s->target->blk);
diff --git a/include/block/block.h b/include/block/block.h
index bd596b1..3eb281a 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -393,8 +393,6 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
int bdrv_is_read_only(BlockDriverState *bs);
int bdrv_is_sg(BlockDriverState *bs);
-int bdrv_enable_write_cache(BlockDriverState *bs);
-void bdrv_set_enable_write_cache(BlockDriverState *bs, bool wce);
bool bdrv_is_inserted(BlockDriverState *bs);
int bdrv_media_changed(BlockDriverState *bs);
void bdrv_lock_medium(BlockDriverState *bs, bool locked);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 06/20] xen_disk: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 06/20] xen_disk: " Kevin Wolf
@ 2016-03-22 11:08 ` Stefano Stabellini
2016-03-26 17:59 ` Max Reitz
1 sibling, 0 replies; 45+ messages in thread
From: Stefano Stabellini @ 2016-03-22 11:08 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, qemu-block, mreitz
On Fri, 18 Mar 2016, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> hw/block/xen_disk.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/block/xen_disk.c b/hw/block/xen_disk.c
> index 635328f..c358709 100644
> --- a/hw/block/xen_disk.c
> +++ b/hw/block/xen_disk.c
> @@ -888,12 +888,14 @@ static int blk_connect(struct XenDevice *xendev)
> struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
> int pers, index, qflags;
> bool readonly = true;
> + bool writethrough = true;
>
> /* read-only ? */
> if (blkdev->directiosafe) {
> qflags = BDRV_O_NOCACHE | BDRV_O_NATIVE_AIO;
> } else {
> - qflags = BDRV_O_CACHE_WB;
> + qflags = 0;
You might as well initialize qflags to 0 above and leave it unchanged
here. In any case:
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> + writethrough = false;
> }
> if (strcmp(blkdev->mode, "w") == 0) {
> qflags |= BDRV_O_RDWR;
> @@ -925,6 +927,7 @@ static int blk_connect(struct XenDevice *xendev)
> error_free(local_err);
> return -1;
> }
> + blk_set_enable_write_cache(blkdev->blk, !writethrough);
> } else {
> /* setup via qemu cmdline -> already setup for us */
> xen_be_printf(&blkdev->xendev, 2, "get configured bdrv (cmdline setup)\n");
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode() Kevin Wolf
@ 2016-03-26 17:05 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:05 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 401 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> It's like bdrv_parse_cache_flags(), except that writethrough mode isn't
> included in the flags, but returned as a separate bool.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 17 +++++++++++++++++
> include/block/block.h | 1 +
> 2 files changed, 18 insertions(+)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly Kevin Wolf
@ 2016-03-26 17:09 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:09 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 219 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-nbd.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 03/20] qemu-io: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 03/20] qemu-io: " Kevin Wolf
@ 2016-03-26 17:18 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:18 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 237 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-io.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses
2016-03-18 18:21 ` [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses Kevin Wolf
@ 2016-03-26 17:34 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:34 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 428 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> It always only set the BDRV_O_CACHE_WB flag, which is going to go away.
> In order to make the next changes more local for better reviewability
> this patches expands the macro.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-img.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly Kevin Wolf
@ 2016-03-26 17:54 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1904 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> qemu-img.c | 79 ++++++++++++++++++++++++++++++++++++++------------------------
> 1 file changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 839e05b..96b51d4 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
[...]
> @@ -2862,26 +2874,30 @@ static int img_rebase(int argc, char **argv)
> qemu_progress_print(0, 100);
>
> flags = BDRV_O_RDWR | (unsafe ? BDRV_O_NO_BACKING : 0);
> - ret = bdrv_parse_cache_flags(cache, &flags);
> + ret = bdrv_parse_cache_mode(cache, &flags, &writethrough);
> if (ret < 0) {
> error_report("Invalid cache option: %s", cache);
> goto out;
> }
>
> - src_flags = BDRV_O_CACHE_WB;
> - ret = bdrv_parse_cache_flags(src_cache, &src_flags);
> + src_flags = 0;
> + ret = bdrv_parse_cache_mode(src_cache, &src_flags, &src_writethrough);
> if (ret < 0) {
> error_report("Invalid source cache option: %s", src_cache);
> goto out;
> }
>
> + /* The source files are opened read-only, don't care about WCE */
> + assert((src_writethrough & BDRV_O_RDWR) == 0);
Well, yeah, that is a trivial assertion to make because BDRV_O_RDWR is 2.
I guess you meant s/src_writethrough/src_flags/.
With that fixed:
Reviewed-by: Max Reitz <mreitz@redhat.com>
> + (void) src_writethrough;
> +
> /*
> * Open the images.
> *
> * Ignore the old backing file for unsafe rebase in case we want to correct
> * the reference to a renamed or moved backing file.
> */
> - blk = img_open(image_opts, filename, fmt, flags, true, quiet);
> + blk = img_open(image_opts, filename, fmt, flags, true, writethrough, quiet);
> if (!blk) {
> ret = -1;
> goto out;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 06/20] xen_disk: Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 06/20] xen_disk: " Kevin Wolf
2016-03-22 11:08 ` Stefano Stabellini
@ 2016-03-26 17:59 ` Max Reitz
1 sibling, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 17:59 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 228 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> hw/block/xen_disk.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 07/20] block: blockdev_init(): Call blk_set_enable_write_cache() explicitly
2016-03-18 18:21 ` [Qemu-devel] [PATCH 07/20] block: blockdev_init(): " Kevin Wolf
@ 2016-03-26 18:13 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 18:13 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 225 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open() Kevin Wolf
@ 2016-03-26 18:36 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 18:36 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 996 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> All callers of blk_new_open() either don't rely on the WCE bit set after
> blk_new_open() because they explicitly set it anyway, or they pass
> BDRV_O_CACHE_WB unconditionally.
>
> This patch changes blk_new_open() so that it always enables writeback
> mode and asserts that BDRV_O_CACHE_WB is clear. For those callers that
> used to pass BDRV_O_CACHE_WB unconditionally, the flag is removed now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/block-backend.c | 4 ++++
> block/parallels.c | 3 +--
> block/qcow.c | 3 +--
> block/qcow2.c | 9 +++------
> block/qed.c | 3 +--
> block/sheepdog.c | 5 ++---
> block/vdi.c | 3 +--
> block/vhdx.c | 3 +--
> block/vmdk.c | 8 +++-----
> block/vpc.c | 3 +--
> blockdev.c | 1 +
> 11 files changed, 19 insertions(+), 26 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync() Kevin Wolf
@ 2016-03-26 18:46 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 18:46 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 485 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> We don't want to silently ignore a flush error.
>
> Also, there is little point in avoiding the flush for writethrough modes
> and once WCE is moved to the BB layer, we definitely need the flush here
> because bdrv_pwrite() won't involve one any more.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level
2016-03-18 18:21 ` [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level Kevin Wolf
@ 2016-03-26 19:54 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 19:54 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 2301 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Whether a write cache is used or not is a decision that concerns the
> user (e.g. the guest device) rather than the backend. It was already
> logically part of the BB level as bdrv_move_feature_fields() always kept
> it on top of the BDS tree; with this patch, the core of it (the actual
> flag and the additional flushes) is also implemented there.
>
> Direct callers of bdrv_open() must pass BDRV_O_CACHE_WB now if bs
> doesn't have a BlockBackend attached.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 26 +++++++++++++++++---------
> block/block-backend.c | 42 +++++++++++++++++++++++++++---------------
> block/io.c | 2 +-
> block/iscsi.c | 2 +-
> include/block/block.h | 1 +
> include/block/block_int.h | 3 ---
> tests/qemu-iotests/142 | 4 ++--
> tests/qemu-iotests/142.out | 8 ++++----
> 8 files changed, 53 insertions(+), 35 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
I'm not so sure about the state bdrv_{set_,}enable_write_cache() are in
after this patch (e.g. the NBD client will always think the write cache
is enabled; and bdrv_set_enable_write_cache() can be used to unset
BDRV_O_CACHE_WB on BDSs), but looking at the following patches' titles,
they'll clear that up.
It appears to me that multiwrite will ignore the writethrough status,
but then again, qemu-io seems to be the only multiwrite user.
> diff --git a/block.c b/block.c
> index 172f865..9271dbb 100644
> --- a/block.c
> +++ b/block.c
[...]
> @@ -3618,8 +3626,8 @@ void bdrv_img_create(const char *filename, const char *fmt,
> }
>
> /* backing files always opened read-only */
> - back_flags =
> - flags & ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
> + back_flags = flags | BDRV_O_CACHE_WB;
> + back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
Actually, this is the only thing the @flags parameter of this function
is used for. Maybe it can be dropped since we already regulate the
back_flags pretty strictly.
>
> if (backing_fmt) {
> backing_options = qdict_new();
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache() Kevin Wolf
@ 2016-03-26 20:14 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:14 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1503 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Now that WCE is handled on the BlockBackend level, the flag is
> meaningless for BDSes. As the schema requires us to fill the field,
> we return an enabled write cache for them.
>
> Note that this means that querying the BlockBackend name may return
> writethrough as the cache information, whereas querying the node-name of
> the root of that same BlockBackend will return writeback.
>
> This may appear odd at first,
Yeah, intuitively I'd think that a BDS shares the writeback mode of the
BB at the tree root, but...
> but it actually makes sense because it
> correctly repesents the layer that implements the WCE handling.
...when you actually access the BDS directly, i.e., not through the
whole tree and the BB at its root, then you will indeed get WB behavior.
> This
> becomes more apparent when you consider nodes that are the root node of
> multiple BlockBackends, where each BB can have its own WCE setting.
True.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 2 +-
> block/qapi.c | 7 +++---
> include/block/qapi.h | 3 ++-
> tests/qemu-iotests/142 | 7 +++++-
> tests/qemu-iotests/142.out | 57 ++++++++++++++++++++++++++++++++++++++--------
> 5 files changed, 60 insertions(+), 16 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags() Kevin Wolf
@ 2016-03-26 20:24 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:24 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 411 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> This function will allow drivers to implement BDRV_REQ_FUA natively
> instead of sending a separate flush after the write.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/io.c | 9 ++++++++-
> include/block/block_int.h | 5 +++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA
2016-03-18 18:21 ` [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA Kevin Wolf
@ 2016-03-26 20:33 ` Max Reitz
2016-03-26 20:44 ` Max Reitz
1 sibling, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:33 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 453 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> This replaces the existing hack in the iscsi driver that sent the FUA
> bit in writethrough mode and ignored the following flush in order to
> optimise the number of roundtrips (see commit 73b5394e).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/iscsi.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA
2016-03-18 18:21 ` [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA Kevin Wolf
2016-03-26 20:33 ` Max Reitz
@ 2016-03-26 20:44 ` Max Reitz
2016-03-29 11:02 ` Kevin Wolf
1 sibling, 1 reply; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:44 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1792 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> This replaces the existing hack in the iscsi driver that sent the FUA
> bit in writethrough mode and ignored the following flush in order to
> optimise the number of roundtrips (see commit 73b5394e).
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/iscsi.c | 24 +++++++-----------------
> 1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 3b54536..4f75204 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
[...]
> @@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi = {
> .bdrv_co_discard = iscsi_co_discard,
> .bdrv_co_write_zeroes = iscsi_co_write_zeroes,
> .bdrv_co_readv = iscsi_co_readv,
> - .bdrv_co_writev = iscsi_co_writev,
> + .bdrv_co_writev_flags = iscsi_co_writev_flags,
> + .supported_write_flags = BDRV_REQ_FUA,
> .bdrv_co_flush_to_disk = iscsi_co_flush,
>
> #ifdef __linux__
>
Hm, wait, maybe not R-b. I can see three places in block/io.c which call
bdrv_co_writev(), and only one of them diverts to bdrv_co_writev_flags()
if that is available. Maybe we don't need to care about the
bounce-buffer case for write_zeroes, but I do think we need to care
about the COR case.
Of course bdrv_co_writev() can trivially be forwarded to
bdrv_co_writev_flags(), but I'm not sure who is supposed to do this
forwarding. I can imagine three ways:
(1) Keep a wrapper per block driver. Simple, but not so elegant.
(2) Make all bdrv_co_writev() callers call bdrv_co_writev_flags() if
the former is not available but the latter is.
(3) Introduce a generic function replacing every drv->bdrv_co_writev()
call which then decides which driver function to invoke.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 14/20] nbd: Support BDRV_REQ_FUA
2016-03-18 18:21 ` [Qemu-devel] [PATCH 14/20] nbd: " Kevin Wolf
@ 2016-03-26 20:46 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:46 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 1052 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> The NBD server already used to send a FUA flag when the writethrough
> mode was set. This code was a remnant from the times where protocol
> drivers actually had to implement writethrough modes. Since nowadays the
> block layer sends flushes in writethrough mode and non-root nodes are
> always writeback, this was mostly dead code - only mostly because if NBD
> was configured to be used without a format, we sent _both_ FUA and an
> explicit flush afterwards, which makes the code not technically dead,
> but useless overhead.
>
> This patch changes the code so that the block layer's FUA flag is
> recognised and translated into a NBD FUA flag. The additional flush is
> avoided now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/nbd-client.c | 13 +++++++------
> block/nbd-client.h | 2 +-
> block/nbd.c | 26 +++++++++++++++++++++-----
> 3 files changed, 29 insertions(+), 12 deletions(-)
Looks good, but I have the same issue as with patch 13.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 15/20] raw: Support BDRV_REQ_FUA
2016-03-18 18:21 ` [Qemu-devel] [PATCH 15/20] raw: " Kevin Wolf
@ 2016-03-26 20:49 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:49 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 435 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Pass through the FUA flag to the lower layer so that the separate flush
> can be saved in practically relevant cases where a (raw) format driver
> sits on top of the protocol driver.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block/raw_bsd.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
Looks good, modulo the bdrv_co_writev() issue.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init() Kevin Wolf
@ 2016-03-26 20:53 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 20:53 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 220 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> blockdev.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() Kevin Wolf
@ 2016-03-26 21:05 ` Max Reitz
2016-03-29 10:16 ` Kevin Wolf
0 siblings, 1 reply; 45+ messages in thread
From: Max Reitz @ 2016-03-26 21:05 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 2067 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> We must forbid changing the WCE flag in bdrv_reopen() in the same patch,
> as otherwise the behaviour would change so that the flag takes
> precedence over the explicitly specified option.
>
> The correct value of the WCE flag depends on the BlockBackend user (e.g.
> guest device) and isn't a decision that the QMP client makes, so this
> change is what we want.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 18 ++++++------------
> qemu-io-cmds.c | 14 +++++++++++++-
> tests/qemu-iotests/142 | 2 +-
> tests/qemu-iotests/142.out | 2 +-
> 4 files changed, 21 insertions(+), 15 deletions(-)
>
[...]
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index e929d24..7de3754 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
[...]
> @@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
> return qemuio_command_usage(&reopen_cmd);
> }
>
> + if (writethrough != blk_enable_write_cache(blk) &&
> + blk_get_attached_dev(blk))
> + {
> + error_report("Cannot change cache.writeback: Device attached");
> + qemu_opts_reset(&reopen_opts);
> + return 0;
> + }
> +
> qopts = qemu_opts_find(&reopen_opts, NULL);
> opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
> qemu_opts_reset(&reopen_opts);
>
> + flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
Shouldn't this be bdrv_enable_write_cache(bs)?
With blk_enable_write_cache(blk), reopening a non-WB BB should always
fail because bdrv_reopen_multiple() accuses us of trying to change the
WB mode (whereas we really don't want to change the BDS's mode).
Max
> brq = bdrv_reopen_queue(NULL, bs, opts, flags);
> bdrv_reopen_multiple(brq, &local_err);
> if (local_err) {
> error_report_err(local_err);
> + } else {
> + blk_set_enable_write_cache(blk, !writethrough);
> }
>
> return 0;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags() Kevin Wolf
@ 2016-03-26 21:06 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 21:06 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 354 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> All users are converted to bdrv_parse_cache_mode() now.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 29 +++++++----------------------
> include/block/block.h | 1 -
> 2 files changed, 7 insertions(+), 23 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB
2016-03-18 18:21 ` [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB Kevin Wolf
@ 2016-03-26 21:23 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 21:23 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 7615 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> The previous patches have successively made blk->enable_write_cache the
> true source for the information whether a writethrough mode must be
> implemented. The corresponding BDRV_O_CACHE_WB is only useless baggage
> we're carrying around, so now's the time to remove it.
>
> At the same time, we remove the 'cache.writeback' option parsing on the
> BDS level as the only effect was setting the BDRV_O_CACHE_WB flag.
>
> This change requires test cases that explicitly enabled the option to
> drop it. Other than that and the change of the error message when
> writethrough is enabled on the BDS level (from "Can't set writethrough
> mode" to "doesn't support the option"), there should be no change in
> behaviour.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 48 ++-----------------------------------------
> block/block-backend.c | 11 ----------
> block/vvfat.c | 3 +--
> blockdev.c | 21 ++-----------------
> include/block/block.h | 3 +--
> qemu-img.c | 2 +-
> qemu-io-cmds.c | 1 -
> tests/qemu-iotests/051 | 2 +-
> tests/qemu-iotests/051.pc.out | 10 ++++-----
> tests/qemu-iotests/142 | 6 +++---
> tests/qemu-iotests/142.out | 36 ++++++++++++++++----------------
> 11 files changed, 34 insertions(+), 109 deletions(-)
>
[...]
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 7de3754..35ee50b 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -2149,7 +2149,6 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
> opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
> qemu_opts_reset(&reopen_opts);
>
> - flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
Well... :-P
> brq = bdrv_reopen_queue(NULL, bs, opts, flags);
> bdrv_reopen_multiple(brq, &local_err);
> if (local_err) {
> diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051
> index 7bfe9ff..88b3d91 100755
> --- a/tests/qemu-iotests/051
> +++ b/tests/qemu-iotests/051
> @@ -218,7 +218,7 @@ run_qemu -drive driver=null-co,cache=invalid_value
>
> for cache in writeback writethrough unsafe invalid_value; do
> echo -e "info block\ninfo block file\ninfo block backing\ninfo block backing-file" | \
> - run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
> + run_qemu -drive file="$TEST_IMG",cache=$cache,backing.file.filename="$TEST_IMG.base",backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=$device_id -nodefaults
> done
>
> echo
> diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
> index 73cc15a..ec6d222 100644
> --- a/tests/qemu-iotests/051.pc.out
> +++ b/tests/qemu-iotests/051.pc.out
051.out needs the same changes.
Aside from that: Looks good.
Max
> @@ -239,7 +239,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
> Testing: -drive driver=null-co,cache=invalid_value
> QEMU_PROG: -drive driver=null-co,cache=invalid_value: invalid cache option
>
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> QEMU X.Y.Z monitor - type 'help' for more information
> (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
> drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -259,7 +259,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
> Cache mode: writeback, ignore flushes
> (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> QEMU X.Y.Z monitor - type 'help' for more information
> (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
> drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -279,7 +279,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
> Cache mode: writeback, ignore flushes
> (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=unsafe,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> QEMU X.Y.Z monitor - type 'help' for more information
> (qemu) i^[[K^[[Din^[[K^[[D^[[Dinf^[[K^[[D^[[D^[[Dinfo^[[K^[[D^[[D^[[D^[[Dinfo ^[[K^[[D^[[D^[[D^[[D^[[Dinfo b^[[K^[[D^[[D^[[D^[[D^[[D^[[Dinfo bl^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo blo^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo bloc^[[K^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[D^[[Dinfo block^[[K
> drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
> @@ -299,8 +299,8 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
> Cache mode: writeback, ignore flushes
> (qemu) q^[[K^[[Dqu^[[K^[[D^[[Dqui^[[K^[[D^[[D^[[Dquit^[[K
>
> -Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> -QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.cache.writeback=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
> +Testing: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0 -nodefaults
> +QEMU_PROG: -drive file=TEST_DIR/t.qcow2,cache=invalid_value,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0: invalid cache option
>
>
> === Specifying the protocol layer ===
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache()
2016-03-18 18:21 ` [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache() Kevin Wolf
@ 2016-03-26 21:25 ` Max Reitz
0 siblings, 0 replies; 45+ messages in thread
From: Max Reitz @ 2016-03-26 21:25 UTC (permalink / raw)
To: Kevin Wolf, qemu-block; +Cc: qemu-devel
[-- Attachment #1.1: Type: text/plain, Size: 663 bytes --]
On 18.03.2016 19:21, Kevin Wolf wrote:
> The only remaining users were block jobs (mirror and backup) which
> unconditionally enabled WCE on the BlockBackend of the target image. As
> these block jobs don't go through BlockBackend for their I/O requests,
> they aren't affected by this setting anyway but always get a writeback
> mode, so that call can be removed.
>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
> block.c | 16 ----------------
> block/backup.c | 1 -
> block/mirror.c | 1 -
> include/block/block.h | 2 --
> 4 files changed, 20 deletions(-)
Reviewed-by: Max Reitz <mreitz@redhat.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f()
2016-03-26 21:05 ` Max Reitz
@ 2016-03-29 10:16 ` Kevin Wolf
0 siblings, 0 replies; 45+ messages in thread
From: Kevin Wolf @ 2016-03-29 10:16 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]
Am 26.03.2016 um 22:05 hat Max Reitz geschrieben:
> On 18.03.2016 19:21, Kevin Wolf wrote:
> > We must forbid changing the WCE flag in bdrv_reopen() in the same patch,
> > as otherwise the behaviour would change so that the flag takes
> > precedence over the explicitly specified option.
> >
> > The correct value of the WCE flag depends on the BlockBackend user (e.g.
> > guest device) and isn't a decision that the QMP client makes, so this
> > change is what we want.
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > block.c | 18 ++++++------------
> > qemu-io-cmds.c | 14 +++++++++++++-
> > tests/qemu-iotests/142 | 2 +-
> > tests/qemu-iotests/142.out | 2 +-
> > 4 files changed, 21 insertions(+), 15 deletions(-)
> >
>
> [...]
>
> > diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> > index e929d24..7de3754 100644
> > --- a/qemu-io-cmds.c
> > +++ b/qemu-io-cmds.c
>
> [...]
>
> > @@ -2136,14 +2137,25 @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
> > return qemuio_command_usage(&reopen_cmd);
> > }
> >
> > + if (writethrough != blk_enable_write_cache(blk) &&
> > + blk_get_attached_dev(blk))
> > + {
> > + error_report("Cannot change cache.writeback: Device attached");
> > + qemu_opts_reset(&reopen_opts);
> > + return 0;
> > + }
> > +
> > qopts = qemu_opts_find(&reopen_opts, NULL);
> > opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
> > qemu_opts_reset(&reopen_opts);
> >
> > + flags |= blk_enable_write_cache(blk) ? BDRV_O_CACHE_WB : 0;
>
> Shouldn't this be bdrv_enable_write_cache(bs)?
>
> With blk_enable_write_cache(blk), reopening a non-WB BB should always
> fail because bdrv_reopen_multiple() accuses us of trying to change the
> WB mode (whereas we really don't want to change the BDS's mode).
bdrv_enable_write_cache() is only a wrapper around
blk_enable_write_cache(), so that wouldn't make any difference. It also
means that bdrv_reopen() checks the cache mode of the BlockBackend
rather than always returning true. This line exists precisely to avoid
the error message you mention.
And as you noticed, the line goes away anyway when BDRV_O_CACHE_WB is
removed later in the series.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA
2016-03-26 20:44 ` Max Reitz
@ 2016-03-29 11:02 ` Kevin Wolf
0 siblings, 0 replies; 45+ messages in thread
From: Kevin Wolf @ 2016-03-29 11:02 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-devel, qemu-block
[-- Attachment #1: Type: text/plain, Size: 2521 bytes --]
Am 26.03.2016 um 21:44 hat Max Reitz geschrieben:
> On 18.03.2016 19:21, Kevin Wolf wrote:
> > This replaces the existing hack in the iscsi driver that sent the FUA
> > bit in writethrough mode and ignored the following flush in order to
> > optimise the number of roundtrips (see commit 73b5394e).
> >
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > ---
> > block/iscsi.c | 24 +++++++-----------------
> > 1 file changed, 7 insertions(+), 17 deletions(-)
> >
> > diff --git a/block/iscsi.c b/block/iscsi.c
> > index 3b54536..4f75204 100644
> > --- a/block/iscsi.c
> > +++ b/block/iscsi.c
>
> [...]
>
> > @@ -1851,7 +1840,8 @@ static BlockDriver bdrv_iscsi = {
> > .bdrv_co_discard = iscsi_co_discard,
> > .bdrv_co_write_zeroes = iscsi_co_write_zeroes,
> > .bdrv_co_readv = iscsi_co_readv,
> > - .bdrv_co_writev = iscsi_co_writev,
> > + .bdrv_co_writev_flags = iscsi_co_writev_flags,
> > + .supported_write_flags = BDRV_REQ_FUA,
> > .bdrv_co_flush_to_disk = iscsi_co_flush,
> >
> > #ifdef __linux__
> >
>
> Hm, wait, maybe not R-b. I can see three places in block/io.c which call
> bdrv_co_writev(), and only one of them diverts to bdrv_co_writev_flags()
> if that is available. Maybe we don't need to care about the
> bounce-buffer case for write_zeroes, but I do think we need to care
> about the COR case.
>
> Of course bdrv_co_writev() can trivially be forwarded to
> bdrv_co_writev_flags(), but I'm not sure who is supposed to do this
> forwarding. I can imagine three ways:
>
> (1) Keep a wrapper per block driver. Simple, but not so elegant.
> (2) Make all bdrv_co_writev() callers call bdrv_co_writev_flags() if
> the former is not available but the latter is.
> (3) Introduce a generic function replacing every drv->bdrv_co_writev()
> call which then decides which driver function to invoke.
Good catch, thanks!
Going for (1) for now, because I think (2) is even less elegant and
while I have a slight preference for (3) from the code perspective, it
could be argued that it impacts the hot write path of raw images and
I don't want to deal with potential performance changes that late in the
cycle.
And now that I'm writing this, I realise that the hot path already calls
.bdrv_co_writev_flags, so that's not a real argument. But I've already
implemented (1), so I'll leave it at that... The long term plan is
anyway to convert everything to .bdrv_co_writev_flags.
Kevin
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2016-03-29 11:02 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-18 18:21 [Qemu-devel] [PATCH 00/20] block: Implement writethrough in BlockBackend Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 01/20] block: Add bdrv_parse_cache_mode() Kevin Wolf
2016-03-26 17:05 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 02/20] qemu-nbd: Call blk_set_enable_write_cache() explicitly Kevin Wolf
2016-03-26 17:09 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 03/20] qemu-io: " Kevin Wolf
2016-03-26 17:18 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 04/20] qemu-img: Expand all BDRV_O_FLAGS uses Kevin Wolf
2016-03-26 17:34 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 05/20] qemu-img: Call blk_set_enable_write_cache() explicitly Kevin Wolf
2016-03-26 17:54 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 06/20] xen_disk: " Kevin Wolf
2016-03-22 11:08 ` Stefano Stabellini
2016-03-26 17:59 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 07/20] block: blockdev_init(): " Kevin Wolf
2016-03-26 18:13 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 08/20] block: Always set writeback mode in blk_new_open() Kevin Wolf
2016-03-26 18:36 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 09/20] block: Handle flush error in bdrv_pwrite_sync() Kevin Wolf
2016-03-26 18:46 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level Kevin Wolf
2016-03-26 19:54 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 11/20] block/qapi: Use blk_enable_write_cache() Kevin Wolf
2016-03-26 20:14 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 12/20] block: Introduce bdrv_co_writev_flags() Kevin Wolf
2016-03-26 20:24 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 13/20] iscsi: Support BDRV_REQ_FUA Kevin Wolf
2016-03-26 20:33 ` Max Reitz
2016-03-26 20:44 ` Max Reitz
2016-03-29 11:02 ` Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 14/20] nbd: " Kevin Wolf
2016-03-26 20:46 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 15/20] raw: " Kevin Wolf
2016-03-26 20:49 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 16/20] block: Use bdrv_parse_cache_mode() in drive_init() Kevin Wolf
2016-03-26 20:53 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 17/20] qemu-io: Use bdrv_parse_cache_mode() in reopen_f() Kevin Wolf
2016-03-26 21:05 ` Max Reitz
2016-03-29 10:16 ` Kevin Wolf
2016-03-18 18:21 ` [Qemu-devel] [PATCH 18/20] block: Remove bdrv_parse_cache_flags() Kevin Wolf
2016-03-26 21:06 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 19/20] block: Remove BDRV_O_CACHE_WB Kevin Wolf
2016-03-26 21:23 ` Max Reitz
2016-03-18 18:21 ` [Qemu-devel] [PATCH 20/20] block: Remove bdrv_(set_)enable_write_cache() Kevin Wolf
2016-03-26 21:25 ` Max Reitz
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.