From nobody Fri May 17 01:43:51 2024 Delivered-To: importer@patchew.org Received-SPF: temperror (zoho.com: Error in retrieving data from DNS) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=temperror (zoho.com: Error in retrieving data from DNS) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (209.51.188.17 [209.51.188.17]) by mx.zohomail.com with SMTPS id 1554037054012641.4094655888215; Sun, 31 Mar 2019 05:57:34 -0700 (PDT) Received: from localhost ([127.0.0.1]:58488 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAa1f-0003ke-JT for importer@patchew.org; Sun, 31 Mar 2019 08:57:27 -0400 Received: from eggs.gnu.org ([209.51.188.92]:34057) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAa0k-0003K4-Ll for qemu-devel@nongnu.org; Sun, 31 Mar 2019 08:56:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hAa0j-00072j-5x for qemu-devel@nongnu.org; Sun, 31 Mar 2019 08:56:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42160) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hAa0f-0006ub-Nb; Sun, 31 Mar 2019 08:56:26 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D7BB436807; Sun, 31 Mar 2019 12:56:23 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-116.ams2.redhat.com [10.36.116.116]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A1B1619C56; Sun, 31 Mar 2019 12:56:21 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 058E61138648; Sun, 31 Mar 2019 14:56:19 +0200 (CEST) From: Markus Armbruster To: Kevin Wolf References: <20190328182810.21497-1-kwolf@redhat.com> <20190328182810.21497-2-kwolf@redhat.com> <2fc10494-0f79-8d18-610d-be21cfc6c30b@redhat.com> <87ftr5iy8i.fsf@dusky.pond.sub.org> <20190329170439.GK5081@localhost.localdomain> <87o95sa40r.fsf@dusky.pond.sub.org> Date: Sun, 31 Mar 2019 14:56:19 +0200 In-Reply-To: <87o95sa40r.fsf@dusky.pond.sub.org> (Markus Armbruster's message of "Sat, 30 Mar 2019 14:24:52 +0100") Message-ID: <87y34v42z0.fsf_-_@dusky.pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Sun, 31 Mar 2019 12:56:24 +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] Options for 4.0 (was: [PATCH 1/2] qmp: Add query-qemu-features) 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: pkrempa@redhat.com, qemu-block@nongnu.org, stefanha@redhat.com, qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Let's review our options for 4.0. Please note my analysis is handicapped by incomplete information, in particular on libvirt's needs. Terminology: * "Hard read-write" semantics: open read/write. * "Hard read-only" semantics: open read-only. * "Dynamic read-only" semantics: open read-only, reopen read/write when a writer attaches, reopen read-only when the last writer detaches. * "Fallback read-only" semantics:. try to open read/write, fall back to read-only. We have a use case for dynamic read-only: libvirt. I'm not aware of a use case for fallback read-only. Behavior before 3.1: * read-only=3Don: hard read-only * read-only=3Doff: hard read-write Behavior in 3.1: new parameter auto-read-only, default off. * read-only=3Don: hard read-only (no change) * read-only=3Don,auto-read-only=3Don: hard read-only (auto-read-only=3Don silently ignored) * read-only=3Doff: hard read-write * read-only=3Doff,auto-read-only=3Don: depends on the driver - file-posix.c's drivers: fallback read-only - some other drivers: fallback read-only - remaining drivers: hard read/write Behavior in 4.0 so far: * read-only=3Don: hard read-only (no change) * read-only=3Don,auto-read-only=3Don: hard read-only (no change) * read-only=3Doff: hard read-write (no change) * read-only=3Doff,auto-read-only=3Don: depends on the driver - file-posix.c's drivers: dynamic read-only - some other drivers: fallback read-only (no change) - remaining drivers: hard read/write (no change) Option 1: Rename @auto-read-only. Rationale: we released auto-read-only in v3.1 with unproven fallback read-only semantics. It turned out not to be useful. Admit the mistake, retract it. Release our next attempt in 4.0 under a suitable new name with fallback read-only semantics. CON: * Compatibility break. No-go if there are users. Users seem quite unlikely, though. * Still unproven, albeit less so: this time we have some unreleased (unfinished?) libvirt code. * Semantics are still largely left to drivers, and the schema can't tell which one does what. Awkward. * Unless we're fairly confident we won't upgrade drivers from "hard" to "fallback" to "dynamic", this merely kicks the can down the road: we'll face the exact same "how can libvirt find out" problem on every upgrade. PRO: * When libvirt sees the new name, it can rely on file-posix.c's drivers providing dynamic read-only semantics. Option 2: Add query-qemu-features command, return file-dynamic-auto-read-only #ifdef CONFIG_POSIX. Rationale: we released auto-read-only in v3.1 with unproven fallback read-only semantics. It turned out not to be useful. Admit the mistake, and patch it up in 4.0. Libirt needs to know whether it's patche up, and this is a simple way to tell it. CON: * All of option 1's, except for the compatibility break * Uses query-qemu-features to expose a property of the build. I consider that a mistake. PRO * Libvirt can use either query-qmp-schema or query-qemu-features to find out whether it can can rely on file-posix.c's drivers providing dynamic read-only semantics. To make query-qemu-features usable, we need to promise query-qemu-features never returns false for it. To make query-qemu-features usable, we need to promise the value will remain valid for the remainder of the QEMU run (defeats caching) or for any future run of the same QEMU binary (enables caching). Option 3: Add @dynamic-read-only to the drivers that provide dynamic read-only, default to value of auto-read-only, promise we'll never add it to drivers that don't. Rationale: give users explicit control over dynamic vs. fallback for all drivers that can provide dynamic. This makes some sense as an interface, as long as you ignore the fact that no driver implements both dynamic and fallback. I can't see why a driver couldn't implement both. It also makes dynamic support visible in the schema. Behavior (three bools -> eight cases): * read-only=3Don: hard read-only (no change) Shorthand for read-only=3Don,auto-read-only=3Doff,dynamic-read-only=3Doff * read-only=3Don,auto-read-only=3Don: hard read-only (no change) Shorthand for read-only=3Don,auto-read-only=3Don,dynamic-read-only=3Don * read-only=3Doff: hard read-write (no change) Shorthand for read-only=3Doff,auto-read-only=3Doff,dynamic-read-only=3Doff * read-only=3Doff,auto-read-only=3Don: Shorthand for read-only=3Doff,auto-read-only=3Don,dynamic-read-only=3Don - file-posix.c's drivers: dynamic read-only (no change, dynamic-read-only=3Don is the default) - some other drivers: fallback read-only (no change) - remaining drivers: hard read/write (no change) * read-only=3Doff,auto-read-only=3Don,dynamic-read-only=3Doff - file-posix.c's drivers: error - all other drivers: N/A * read-only=3Doff,auto-read-only=3Doff,dynamic-read-only=3Don - file-posix.c's drivers: error - all other drivers: N/A * read-only=3Don,dynamic-read-only=3Don Shorthand for read-only=3Don,auto-read-only=3Doff,dynamic-read-only=3Don - file-posix.c's drivers: error - all other drivers: N/A * read-only=3Don,auto-read-only=3Don,dynamic-read-only=3Doff Shorthand for read-only=3Don,auto-read-only=3Don,dynamic-read-only=3Doff - file-posix.c's drivers: error - all other drivers: N/A CON: * Two bools (read-only and auto-read-only) to select from three choices was already ugly. Three bools (the same plus dynamic-read-only) to select from four choices is even uglier. * The explicit control is just a facade so far: since only the default setting is implemented, it doesn't actually control anything. PRO: * When libvirt sees a driver providing a dynamic-read-only parameter, it knows it can rely on the driver providing dynamic read-only semantics. * Adding dynamic read-only capability to drivers creates no introspection problems: we simply add dynamic-read-only to their parameters. Code sketch appended From 6c5f539818a817947545bed6457a8971dccb6464 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Sun, 31 Mar 2019 09:19:11 +0200 Subject: [PATCH] dynamic-read-only WIP --- block/file-posix.c | 12 ++++++++++++ qapi/block-core.json | 3 +++ 2 files changed, 15 insertions(+) diff --git a/block/file-posix.c b/block/file-posix.c index db4cccbe51..d62681df14 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -420,6 +420,11 @@ static QemuOptsList raw_runtime_opts =3D { .type =3D QEMU_OPT_STRING, .help =3D "File name of the image", }, + { + .name =3D "dynamic-read-only", + .type =3D QEMU_OPT_BOOL, + .help =3D "FIXME", + }, { .name =3D "aio", .type =3D QEMU_OPT_STRING, @@ -540,6 +545,13 @@ static int raw_open_common(BlockDriverState *bs, QDict= *options, s->open_flags =3D open_flags; raw_parse_flags(bdrv_flags, &s->open_flags, false); =20 + if (qemu_opt_get_bool(opts, "dynamic-read-only", + bdrv_flags & BDRV_O_AUTO_RDONLY) + =3D=3D !(bdrv_flags & BDRV_O_AUTO_RDONLY)) { + error_setg(errp, "FIXME"); + ret =3D - EINVAL; + } + s->fd =3D -1; fd =3D qemu_open(filename, s->open_flags, 0644); ret =3D fd < 0 ? -errno : 0; diff --git a/qapi/block-core.json b/qapi/block-core.json index 7ccbfff9d0..0cf15477ce 100644 --- a/qapi/block-core.json +++ b/qapi/block-core.json @@ -2827,6 +2827,7 @@ # Driver specific block device options for the file backend. # # @filename: path to the image file +# @dynamic-read-only: FIXME document (since 4.0) # @pr-manager: the id for the object that will handle persistent reservat= ions # for this device (default: none, forward the commands via S= G_IO; # since 2.11) @@ -2847,6 +2848,8 @@ ## { 'struct': 'BlockdevOptionsFile', 'data': { 'filename': 'str', + '*dynamic-read-only': {'type': 'bool', + 'if': 'defined(CONFIG_POSIX)'}, '*pr-manager': 'str', '*locking': 'OnOffAuto', '*aio': 'BlockdevAioOptions', --=20 2.17.2