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 <kw...@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. > +++ b/block/vvfat.c > @@ -1259,7 +1259,11 @@ static int vvfat_open(BlockDriverState *bs, QDict > *options, int flags, > "Unable to set VVFAT to 'rw' when drive is > read-only"); > goto fail; > } > - } else { > + } 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); Is this also a good time to deprecate vvfat's duplication of rw vs. read-only, and consolidate that into a single option? No other device defaults to read-only, so the deprecation period is a good point to warn that a future version may default to read-write without an explicit read-only. I guess vvfat is the only driver with a device-specific QAPI change (for 'rw') that might be impacted if you make that additional change. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature