Re: [Qemu-devel] [Qemu-block] [PATCH] block: Deprecate bdrv_set_read_only() and users

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2780 bytes --]

Am 08.11.2017 um 11:04 hat Kevin Wolf geschrieben:
> Am 07.11.2017 um 21:29 hat Eric Blake geschrieben:
> > On 11/07/2017 11:26 AM, Kevin Wolf wrote:
> > > bdrv_set_read_only() is used by some block drivers to override the
> > > read-only option given by the user. This is not how read-only images
> > > generally work in QEMU: Instead of second guessing what the user really
> > > meant (which currently includes making an image read-only even if the
> > > user didn't only use the default, but explicitly said read-only=off), we
> > > should error out if we can't provide what the user requested.
> > > 
> > > This adds deprecation warnings to all callers of bdrv_set_read_only() so
> > > that the behaviour can be corrected after the usual deprecation period.
> > > 
> > > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > > ---
> > >  block.c       |  5 +++++
> > >  block/bochs.c | 13 ++++++++++---
> > >  block/cloop.c | 13 ++++++++++---
> > >  block/dmg.c   | 12 +++++++++---
> > >  block/rbd.c   | 14 ++++++++++----
> > >  block/vvfat.c |  6 +++++-
> > >  6 files changed, 49 insertions(+), 14 deletions(-)
> > 
> > Dan pointed out the missing documentation, but for the code itself, the
> > approach looks sane (especially since it was my attempt to make it worse
> > by extending the idiom to NBD that triggered you to write this patch).
> > 
> > Other documentation: In qapi/block-core.json, @BlockdevOptions, we
> > probably ought to mention under @read-only that some block drivers
> > require the use of an explicit read-only.
> 
> Well, they don't only need an explicitly set option, but the important
> point is that they don't work with the default value. But I can add
> something to this effect.

I'll squash this in if it looks good to you:

diff --git a/qapi/block-core.json b/qapi/block-core.json
index ab96e348e6..76bf50f813 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3134,8 +3134,11 @@
 #                 This option is required on the top level of blockdev-add.
 # @discard:       discard-related options (default: ignore)
 # @cache:         cache-related options
-# @read-only:     whether the block device should be read-only
-#                 (default: false)
+# @read-only:     whether the block device should be read-only (default: false).
+#                 Note that some block drivers support only read-only access,
+#                 either generally or in certain configurations. In this case,
+#                 the default value does not work and the option must be
+#                 specified explicitly.
 # @detect-zeroes: detect and optimize zero writes (Since 2.1)
 #                 (default: off)
 # @force-share:   force share all permission on added nodes.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]