From patchwork Fri Mar 18 18:21:19 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 599643 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3qRYdK6vqTz9s9G for ; Sat, 19 Mar 2016 05:28:37 +1100 (AEDT) Received: from localhost ([::1]:45623 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agz8W-0008Mf-3U for incoming@patchwork.ozlabs.org; Fri, 18 Mar 2016 14:28:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36540) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agz28-0004AB-Dx for qemu-devel@nongnu.org; Fri, 18 Mar 2016 14:22:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1agz25-00079M-Hw for qemu-devel@nongnu.org; Fri, 18 Mar 2016 14:22:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55025) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1agz21-00077E-Rf; Fri, 18 Mar 2016 14:21:54 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 8DFF31F57B; Fri, 18 Mar 2016 18:21:53 +0000 (UTC) Received: from noname.redhat.com (ovpn-116-83.ams2.redhat.com [10.36.116.83]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u2IILZO1021202; Fri, 18 Mar 2016 14:21:52 -0400 From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 18 Mar 2016 19:21:19 +0100 Message-Id: <1458325289-17848-11-git-send-email-kwolf@redhat.com> In-Reply-To: <1458325289-17848-1-git-send-email-kwolf@redhat.com> References: <1458325289-17848-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com Subject: [Qemu-devel] [PATCH 10/20] block: Move enable_write_cache to BB level X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org 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 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