From patchwork Fri Oct 12 11:55:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 983019 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=nongnu.org (client-ip=2001:4830:134:3::11; helo=lists.gnu.org; envelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42Wmdt3qh5z9s1x for ; Fri, 12 Oct 2018 23:01:38 +1100 (AEDT) Received: from localhost ([::1]:39696 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gAw8O-0003Ds-3W for incoming@patchwork.ozlabs.org; Fri, 12 Oct 2018 08:01:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38001) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gAw2w-0007do-5d for qemu-devel@nongnu.org; Fri, 12 Oct 2018 07:56:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gAw2t-0005of-Av for qemu-devel@nongnu.org; Fri, 12 Oct 2018 07:55:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:59780) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1gAw2p-0005dB-Lt; Fri, 12 Oct 2018 07:55:51 -0400 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BAED3308AA14; Fri, 12 Oct 2018 11:55:50 +0000 (UTC) Received: from dhcp-200-186.str.redhat.com (dhcp-200-186.str.redhat.com [10.33.200.186]) by smtp.corp.redhat.com (Postfix) with ESMTP id 74CDC6F110; Fri, 12 Oct 2018 11:55:49 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Fri, 12 Oct 2018 13:55:27 +0200 Message-Id: <20181012115532.12645-4-kwolf@redhat.com> In-Reply-To: <20181012115532.12645-1-kwolf@redhat.com> References: <20181012115532.12645-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.41]); Fri, 12 Oct 2018 11:55:50 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v2 3/8] block: Require auto-read-only for existing fallbacks 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, pkrempa@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: "Qemu-devel" Some block drivers have traditionally changed their node to read-only mode without asking the user. This behaviour has been marked deprecated since 2.11, expecting users to provide an explicit read-only=on option. Now that we have auto-read-only=on, enable these drivers to make use of the option. This is the only use of bdrv_set_read_only(), so we can make it a bit more specific and turn it into a bdrv_apply_auto_read_only() that is more convenient for drivers to use. Signed-off-by: Kevin Wolf --- include/block/block.h | 3 ++- block.c | 37 +++++++++++++++++++++++-------------- block/bochs.c | 17 ++++++----------- block/cloop.c | 16 +++++----------- block/dmg.c | 16 +++++----------- block/rbd.c | 14 ++++---------- block/vvfat.c | 12 +++--------- 7 files changed, 48 insertions(+), 67 deletions(-) diff --git a/include/block/block.h b/include/block/block.h index 580b3716c3..7f5453b45b 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -438,7 +438,8 @@ int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base, bool bdrv_is_read_only(BlockDriverState *bs); int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, bool ignore_allow_rdw, Error **errp); -int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp); +int bdrv_apply_auto_read_only(BlockDriverState *bs, const char *errmsg, + Error **errp); bool bdrv_is_writable(BlockDriverState *bs); bool bdrv_is_sg(BlockDriverState *bs); bool bdrv_is_inserted(BlockDriverState *bs); diff --git a/block.c b/block.c index f999393e28..631501bcae 100644 --- a/block.c +++ b/block.c @@ -266,27 +266,36 @@ int bdrv_can_set_read_only(BlockDriverState *bs, bool read_only, return 0; } -/* TODO Remove (deprecated since 2.11) - * Block drivers are not supposed to automatically change bs->read_only. - * Instead, they should just check whether they can provide what the user - * explicitly requested and error out if read-write is requested, but they can - * only provide read-only access. */ -int bdrv_set_read_only(BlockDriverState *bs, bool read_only, Error **errp) +/* + * Called by a driver that can only provide a read-only image. + * + * Returns 0 if the node is already read-only or it could switch the node to + * read-only because BDRV_O_AUTO_RDONLY is set. + * + * Returns -EACCES if the node is read-write and BDRV_O_AUTO_RDONLY is not set. + * If @errmsg is not NULL, it is used as the error message for the Error + * object. + */ +int bdrv_apply_auto_read_only(BlockDriverState *bs, const char *errmsg, + Error **errp) { int ret = 0; - ret = bdrv_can_set_read_only(bs, read_only, false, errp); + if (!(bs->open_flags & BDRV_O_RDWR)) { + return 0; + } + if (!(bs->open_flags & BDRV_O_AUTO_RDONLY)) { + error_setg(errp, "%s", errmsg ?: "Image is read-only"); + return -EACCES; + } + + ret = bdrv_can_set_read_only(bs, true, false, errp); if (ret < 0) { return ret; } - bs->read_only = read_only; - - if (read_only) { - bs->open_flags &= ~BDRV_O_RDWR; - } else { - bs->open_flags |= BDRV_O_RDWR; - } + bs->read_only = true; + bs->open_flags &= ~BDRV_O_RDWR; return 0; } diff --git a/block/bochs.c b/block/bochs.c index 50c630047b..22e7d44211 100644 --- a/block/bochs.c +++ b/block/bochs.c @@ -105,23 +105,18 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, struct bochs_header bochs; int ret; + /* No write support yet */ + ret = bdrv_apply_auto_read_only(bs, NULL, errp); + if (ret < 0) { + return ret; + } + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false, errp); if (!bs->file) { return -EINVAL; } - if (!bdrv_is_read_only(bs)) { - error_report("Opening bochs images without an explicit read-only=on " - "option is deprecated. Future versions will refuse to " - "open the image instead of automatically marking the " - "image read-only."); - ret = bdrv_set_read_only(bs, true, errp); /* no write support yet */ - if (ret < 0) { - return ret; - } - } - ret = bdrv_pread(bs->file, 0, &bochs, sizeof(bochs)); if (ret < 0) { return ret; diff --git a/block/cloop.c b/block/cloop.c index 2be68987bd..df2b85f723 100644 --- a/block/cloop.c +++ b/block/cloop.c @@ -67,23 +67,17 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags, uint32_t offsets_size, max_compressed_block_size = 1, i; int ret; + ret = bdrv_apply_auto_read_only(bs, NULL, errp); + if (ret < 0) { + return ret; + } + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false, errp); if (!bs->file) { return -EINVAL; } - if (!bdrv_is_read_only(bs)) { - error_report("Opening cloop images without an explicit read-only=on " - "option is deprecated. Future versions will refuse to " - "open the image instead of automatically marking the " - "image read-only."); - ret = bdrv_set_read_only(bs, true, errp); - if (ret < 0) { - return ret; - } - } - /* read header */ ret = bdrv_pread(bs->file, 128, &s->block_size, 4); if (ret < 0) { diff --git a/block/dmg.c b/block/dmg.c index c9b3c519c4..1d9283ba2f 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -413,23 +413,17 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, int64_t offset; int ret; + ret = bdrv_apply_auto_read_only(bs, NULL, errp); + if (ret < 0) { + return ret; + } + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false, errp); if (!bs->file) { return -EINVAL; } - if (!bdrv_is_read_only(bs)) { - error_report("Opening dmg images without an explicit read-only=on " - "option is deprecated. Future versions will refuse to " - "open the image instead of automatically marking the " - "image read-only."); - ret = bdrv_set_read_only(bs, true, errp); - if (ret < 0) { - return ret; - } - } - block_module_load_one("dmg-bz2"); s->n_chunks = 0; diff --git a/block/rbd.c b/block/rbd.c index 014c68d629..ee0b4a6941 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -780,16 +780,10 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, /* If we are using an rbd snapshot, we must be r/o, otherwise * leave as-is */ if (s->snap != NULL) { - if (!bdrv_is_read_only(bs)) { - error_report("Opening rbd snapshots without an explicit " - "read-only=on option is deprecated. Future versions " - "will refuse to open the image instead of " - "automatically marking the image read-only."); - r = bdrv_set_read_only(bs, true, &local_err); - if (r < 0) { - error_propagate(errp, local_err); - goto failed_open; - } + r = bdrv_apply_auto_read_only(bs, "rbd snapshots are read-only", errp); + if (r < 0) { + rbd_close(s->image); + goto failed_open; } } diff --git a/block/vvfat.c b/block/vvfat.c index 98ba5e2bac..fd814c39c9 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -1262,16 +1262,10 @@ static int vvfat_open(BlockDriverState *bs, QDict *options, int flags, "Unable to set VVFAT to 'rw' when drive is read-only"); goto fail; } - } else if (!bdrv_is_read_only(bs)) { - error_report("Opening non-rw vvfat images without an explicit " - "read-only=on option is deprecated. Future versions " - "will refuse to open the image instead of " - "automatically marking the image read-only."); - /* read only is the default for safety */ - ret = bdrv_set_read_only(bs, true, &local_err); + } else { + ret = bdrv_apply_auto_read_only(bs, NULL, errp); if (ret < 0) { - error_propagate(errp, local_err); - goto fail; + return ret; } }