From bfd18d1e0b06608226fcef1982c3a439c49c3366 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Fri, 4 Mar 2016 14:28:01 +0100 Subject: block: Move enable_write_cache to BB level 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 Reviewed-by: Max Reitz --- 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 14d2ebc35a..71061fbbfc 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 048f48e935..a263636a58 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -47,6 +47,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; @@ -699,11 +701,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); } @@ -1210,26 +1218,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; } } } @@ -1492,11 +1493,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 b58843594e..939e4f1559 100644 --- a/block/io.c +++ b/block/io.c @@ -1160,7 +1160,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 128ea79c13..3b5453687d 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 4fd6c050f0..ddfd50abd5 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 1177c25aab..48846091ac 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -442,9 +442,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 80834b5d8f..517fb306c8 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 5dd5bd0cd4..32dc802a18 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 -- cgit v1.2.1