From patchwork Tue May 3 22:39:06 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Blake X-Patchwork-Id: 9008701 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id C87129F372 for ; Tue, 3 May 2016 22:40:17 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D49762037F for ; Tue, 3 May 2016 22:40:16 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CA0862035D for ; Tue, 3 May 2016 22:40:15 +0000 (UTC) Received: from localhost ([::1]:44287 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axizC-0001wM-HO for patchwork-qemu-devel@patchwork.kernel.org; Tue, 03 May 2016 18:40:10 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54633) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axiyy-0001hO-VH for qemu-devel@nongnu.org; Tue, 03 May 2016 18:40:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1axiym-000083-NV for qemu-devel@nongnu.org; Tue, 03 May 2016 18:39:51 -0400 Received: from resqmta-po-06v.sys.comcast.net ([2001:558:fe16:19:96:114:154:165]:60368) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1axiyl-0008Tk-8x for qemu-devel@nongnu.org; Tue, 03 May 2016 18:39:44 -0400 Received: from resomta-po-08v.sys.comcast.net ([96.114.154.232]) by comcast with SMTP id xiyWamwSf9sFTxiyZaMpND; Tue, 03 May 2016 22:39:31 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net; s=q20140121; t=1462315171; bh=yfiLLiJ8s2U0WjclUxmkBhrmxWCHMWColKYAOv8ahsc=; h=Received:Received:From:To:Subject:Date:Message-Id; b=SseogQ8UXegiUuYTUPZRuo8aNwnjUxxdYSLUp6Nob5iYh9sjz6xNbOz0orUbejj/J EtI/VHWh3XzsKVnF+6KvB956cu+xfhVb+8rdN7wHmn7ka0B6mgiKplCa3TRjRDEStR tsy/VBihVkk9Gji36Qk7w6fKUWU6GvHm6bM8uBYdjSRusFsaOoFsqkG5Kf12GWjSXb ffZnPbzgNJW/Tw7OIVXf0RabgBtxIt0uhmXN1XMqxaSbIpOu5kOgR4wgGRFHiGQzqy oNYOOWk2wqpA0DGddj9rPW7GxVCbDIkrdceRsF2B45RkBuXwRaASqa6xtOk9ADwBIM zC0v2IuM5p6JQ== Received: from red.redhat.com ([24.10.254.122]) by resomta-po-08v.sys.comcast.net with comcast id pyf91s00H2fD5rL01yfVoG; Tue, 03 May 2016 22:39:31 +0000 From: Eric Blake To: qemu-devel@nongnu.org Date: Tue, 3 May 2016 16:39:06 -0600 Message-Id: <1462315148-15504-2-git-send-email-eblake@redhat.com> X-Mailer: git-send-email 2.5.5 In-Reply-To: <1462315148-15504-1-git-send-email-eblake@redhat.com> References: <1462315148-15504-1-git-send-email-eblake@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:558:fe16:19:96:114:154:165 Subject: [Qemu-devel] [PATCHv2 1/3] block: Make supported_write_flags a per-bds property X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, Fam Zheng , Stefan Hajnoczi , qemu-block@nongnu.org, Peter Lieven , Max Reitz , Ronnie Sahlberg , Paolo Bonzini Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Pre-patch, .supported_write_flags lives at the driver level, which means we are blindly declaring that all block devices using a given driver will either equally support FUA, or that we need a fallback at the block layer. But there are drivers where FUA support is a per-block decision: the NBD block driver is dependent on the remote server advertising NBD_FLAG_SEND_FUA (and has fallback code to duplicate the flush that the block layer would do if NBD had not set .supported_write_flags); and the iscsi block driver is dependent on the mode sense bits advertised by the underlying device (and is currently silently ignoring FUA requests if the underlying device does not support FUA). The fix is to make supported flags as a per-BDS option, set during .bdrv_open(). This patch moves the variable and fixes NBD and iscsi to set it only conditionally; later patches will then further simplify the NBD driver to quit duplicating work done at the block layer, as well as tackle the fact that SCSI does not support FUA semantics on WRITESAME(10/16) but only on WRITE(10/16). Signed-off-by: Eric Blake --- include/block/block_int.h | 4 ++-- block/io.c | 9 ++++----- block/iscsi.c | 10 +++++++--- block/nbd-client.c | 3 +++ block/nbd.c | 3 --- block/raw_bsd.c | 2 +- 6 files changed, 17 insertions(+), 14 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index c512074..63e49ba 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -158,8 +158,6 @@ struct BlockDriver { int coroutine_fn (*bdrv_co_pwritev)(BlockDriverState *bs, uint64_t offset, uint64_t bytes, 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 @@ -445,6 +443,8 @@ struct BlockDriverState { /* Alignment requirement for offset/length of I/O requests */ unsigned int request_alignment; + /* Flags honored during pwrite (so far: BDRV_REQ_FUA) */ + unsigned int supported_write_flags; /* the following member gives a name to every node on the bs graph. */ char node_name[32]; diff --git a/block/io.c b/block/io.c index 0db1146..1fb7afe 100644 --- a/block/io.c +++ b/block/io.c @@ -841,9 +841,10 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs, if (drv->bdrv_co_writev_flags) { ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov, - flags); + flags & bs->supported_write_flags); + flags &= ~bs->supported_write_flags; } else if (drv->bdrv_co_writev) { - assert(drv->supported_write_flags == 0); + assert(!bs->supported_write_flags); ret = drv->bdrv_co_writev(bs, sector_num, nb_sectors, qiov); } else { BlockAIOCB *acb; @@ -862,9 +863,7 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs, } emulate_flags: - if (ret == 0 && (flags & BDRV_REQ_FUA) && - !(drv->supported_write_flags & BDRV_REQ_FUA)) - { + if (ret == 0 && (flags & BDRV_REQ_FUA)) { ret = bdrv_co_flush(bs); } diff --git a/block/iscsi.c b/block/iscsi.c index 4f75204..6d5c1f6 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -456,8 +456,11 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors, struct IscsiTask iTask; uint64_t lba; uint32_t num_sectors; - bool fua; + bool fua = flags & BDRV_REQ_FUA; + if (fua) { + assert(iscsilun->dpofua); + } if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) { return -EINVAL; } @@ -472,7 +475,6 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t sector_num, int nb_sectors, num_sectors = sector_qemu2lun(nb_sectors, iscsilun); iscsi_co_init_iscsitask(iscsilun, &iTask); retry: - 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, @@ -1548,6 +1550,9 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags, task = NULL; iscsi_modesense_sync(iscsilun); + if (iscsilun->dpofua) { + bs->supported_write_flags = BDRV_REQ_FUA; + } /* Check the write protect flag of the LUN if we want to write */ if (iscsilun->type == TYPE_DISK && (flags & BDRV_O_RDWR) && @@ -1841,7 +1846,6 @@ static BlockDriver bdrv_iscsi = { .bdrv_co_write_zeroes = iscsi_co_write_zeroes, .bdrv_co_readv = iscsi_co_readv, .bdrv_co_writev_flags = iscsi_co_writev_flags, - .supported_write_flags = BDRV_REQ_FUA, .bdrv_co_flush_to_disk = iscsi_co_flush, #ifdef __linux__ diff --git a/block/nbd-client.c b/block/nbd-client.c index 878e879..5fc96e9 100644 --- a/block/nbd-client.c +++ b/block/nbd-client.c @@ -414,6 +414,9 @@ int nbd_client_init(BlockDriverState *bs, logout("Failed to negotiate with the NBD server\n"); return ret; } + if (client->nbdflags & NBD_FLAG_SEND_FUA) { + bs->supported_write_flags = BDRV_REQ_FUA; + } qemu_co_mutex_init(&client->send_mutex); qemu_co_mutex_init(&client->free_sema); diff --git a/block/nbd.c b/block/nbd.c index fccbfef..a4fba91 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -471,7 +471,6 @@ static BlockDriver bdrv_nbd = { .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev_flags = nbd_co_writev_flags, - .supported_write_flags = BDRV_REQ_FUA, .bdrv_close = nbd_close, .bdrv_co_flush_to_os = nbd_co_flush, .bdrv_co_discard = nbd_co_discard, @@ -490,7 +489,6 @@ static BlockDriver bdrv_nbd_tcp = { .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev_flags = nbd_co_writev_flags, - .supported_write_flags = BDRV_REQ_FUA, .bdrv_close = nbd_close, .bdrv_co_flush_to_os = nbd_co_flush, .bdrv_co_discard = nbd_co_discard, @@ -509,7 +507,6 @@ static BlockDriver bdrv_nbd_unix = { .bdrv_file_open = nbd_open, .bdrv_co_readv = nbd_co_readv, .bdrv_co_writev_flags = nbd_co_writev_flags, - .supported_write_flags = BDRV_REQ_FUA, .bdrv_close = nbd_close, .bdrv_co_flush_to_os = nbd_co_flush, .bdrv_co_discard = nbd_co_discard, diff --git a/block/raw_bsd.c b/block/raw_bsd.c index 5e65fb0..1a1618e 100644 --- a/block/raw_bsd.c +++ b/block/raw_bsd.c @@ -204,6 +204,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags, Error **errp) { bs->sg = bs->file->bs->sg; + bs->supported_write_flags = BDRV_REQ_FUA; if (bs->probed && !bdrv_is_read_only(bs)) { fprintf(stderr, @@ -250,7 +251,6 @@ BlockDriver bdrv_raw = { .bdrv_create = &raw_create, .bdrv_co_readv = &raw_co_readv, .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,