The following changes since commit b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:

Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' into staging (2018-11-05 10:32:49 +0000)

for you to fetch changes up to 1240ac558d348f6c7a5752b1a57c1da58e4efe3e:

include: Add a comment to explain the origin of sizes' lookup table (2018-11-05 15:29:59 +0100)

- auto-read-only option to fix commit job when used with -blockdev

- Fix help text related qemu-iotests failure (by improving the help text

and updating the reference output)

- quorum: Add missing checks when adding/removing child nodes

- Don't take address of fields in packed structs

- vvfat: Fix crash when reporting error about too many files in directory

Alberto Garcia (7):

block: replace "discard" literal with BDRV_OPT_DISCARD macro

qcow2: Get the request alignment for encrypted images from QCryptoBlock

quorum: Remove quorum_err()

quorum: Return an error if the blkverify mode has invalid settings

iotest: Test the blkverify mode of the Quorum driver

quorum: Forbid adding children in blkverify mode

iotest: Test x-blockdev-change on a Quorum

Cleber Rosa (1):

iotests: make 083 specific to raw

crypto: initialize sector size even when opening with no IO flag

Kevin Wolf (12):

vpc: Don't leak opts in vpc_open()

block: Update flags in bdrv_set_read_only()

block: Add auto-read-only option

rbd: Close image in qemu_rbd_open() error path

block: Require auto-read-only for existing fallbacks

nbd: Support auto-read-only option

file-posix: Support auto-read-only option

curl: Support auto-read-only option

gluster: Support auto-read-only option

iscsi: Support auto-read-only option

block: Make auto-read-only=on default for -drive

qemu-iotests: Test auto-read-only with -drive and -blockdev

Leonid Bloch (2):

vdi: Use a literal number of bytes for DEFAULT_CLUSTER_SIZE

include: Add a comment to explain the origin of sizes' lookup table

Li Qiang (1):

block: change some function return type to bool

Max Reitz (5):

option: Make option help nicer to read

chardev: Indent list of chardevs

qdev-monitor: Make device options help nicer

object: Make option help nicer to read

fw_cfg: Drop newline in @file description

Peter Maydell (5):

block/qcow2: Don't take address of fields in packed structs

block/qcow: Don't take address of fields in packed structs

block/qcow2-bitmap: Don't take address of fields in packed structs

block/vhdx: Don't take address of fields in packed structs

block/vdi: Don't take address of fields in packed structs

Stefan Weil (1):

qemu-io-cmds: Fix two format strings

Thomas Huth (1):

block/vvfat: Fix crash when reporting error about too many files in directory

From: Max Reitz <mreitz@redhat.com>

Following the example of qemu_opts_print_help(), indent all entries in

the list of character devices.

Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

chardev/char.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/chardev/char.c b/chardev/char.c

--- a/chardev/char.c

+++ b/chardev/char.c

@@ -XXX,XX +XXX,XX @@ help_string_append(const char *name, void *opaque)

{

GString *str = opaque;

- g_string_append_printf(str, "\n%s", name);

+ g_string_append_printf(str, "\n %s", name);

static const char *chardev_alias_translate(const char *name)

2.19.1

From: Cleber Rosa <crosa@redhat.com>

While testing the Python 3 changes which touch the 083 test, I noticed

that it would fail with qcow2. Expanding the testing, I noticed it

had nothing to do with the Python 3 changes, and in fact, it would not

pass on anything but raw:

raw: pass

bochs: not generic

cloop: not generic

parallels: fail

qcow: fail

qcow2: fail

qed: fail

vdi: fail

vhdx: fail

vmdk: fail

vpc: fail

luks: fail

The errors are a mixture I/O and "image not in xxx format", such as:

=== Check disconnect before data ===

Unexpected end-of-file before all bytes were read

-read failed: Input/output error

+can't open device nbd+tcp://127.0.0.1:PORT/foo: Could not open 'nbd://127.0.0.1:PORT/foo': Input/output error

=== Check disconnect after data ===

-read 512/512 bytes at offset 0

-512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

+can't open device nbd+tcp://127.0.0.1:PORT/foo: Image not in qcow format

I'm not aware if there's a quick fix, so, for the time being, it looks

like the honest approach is to make the test known to work on raw

only.

Signed-off-by: Cleber Rosa <crosa@redhat.com>

tests/qemu-iotests/083 | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083

--- a/tests/qemu-iotests/083

+++ b/tests/qemu-iotests/083

@@ -XXX,XX +XXX,XX @@ trap "_cleanup; exit \$status" 0 1 2 3 15

. ./common.rc

. ./common.filter

-_supported_fmt generic

+_supported_fmt raw

_supported_proto nbd

2.19.1

When using the vvfat driver with a directory that contains too many files,

QEMU currently crashes. This can be triggered like this for example:

mkdir /tmp/vvfattest

cd /tmp/vvfattest

for ((x=0;x<=513;x++)); do mkdir $x; done

qemu-system-x86_64 -drive \

file.driver=vvfat,file.dir=.,read-only=on,media=cdrom

Seems like read_directory() is changing the mapping->path variable. Make

sure we use the right pointer instead.

block/vvfat.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/block/vvfat.c b/block/vvfat.c

index XXXXXXX..XXXXXXX 100644

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ static int init_directories(BDRVVVFATState* s,

mapping = array_get(&(s->mapping), i);

if (mapping->mode & MODE_DIRECTORY) {

+ char *path = mapping->path;

mapping->begin = cluster;

if(read_directory(s, i)) {

- error_setg(errp, "Could not read directory %s",

- mapping->path);

+ error_setg(errp, "Could not read directory %s", path);

return -1;

}

mapping = array_get(&(s->mapping), i);

2.19.1

If a management application builds the block graph node by node, the

protocol layer doesn't inherit its read-only option from the format

layer any more, so it must be set explicitly.

Backing files should work on read-only storage, but at the same time, a

block job like commit should be able to reopen them read-write if they

are on read-write storage. However, without option inheritance, reopen

only changes the read-only option for the root node (typically the

format layer), but not the protocol layer, so reopening fails (the

format layer wants to get write permissions, but the protocol layer is

still read-only).

A simple workaround for the problem in the management tool would be to

open the protocol layer always read-write and to make only the format

layer read-only for backing files. However, sometimes the file is

actually stored on read-only storage and we don't know whether the image

can be opened read-write (for example, for NBD it depends on the server

we're trying to connect to). This adds an option that makes QEMU try to

open the image read-write, but allows it to degrade to a read-only mode

without returning an error.

The documentation for this option is consciously phrased in a way that

allows QEMU to switch to a better model eventually: Instead of trying

when the image is first opened, making the read-only flag dynamic and

changing it automatically whenever the first BLK_PERM_WRITE user is

attached or the last one is detached would be much more useful

behaviour.

Unfortunately, this more useful behaviour is also a lot harder to

implement, and libvirt needs a solution now before it can switch to

-blockdev, so let's start with this easier approach for now.

Instead of adding a new auto-read-only option, turning the existing

read-only into an enum (with a bool alternate for compatibility) was

considered, but it complicated the implementation to the point that it

didn't seem to be worth it.

qapi/block-core.json | 7 +++++++

include/block/block.h | 2 ++

block.c | 17 +++++++++++++++++

block/vvfat.c | 1 +

blockdev.c | 2 +-

5 files changed, 28 insertions(+), 1 deletion(-)

# either generally or in certain configurations. In this case,

# the default value does not work and the option must be

# specified explicitly.

+# @auto-read-only: if true and @read-only is false, QEMU may automatically

+# decide not to open the image read-write as requested, but

+# fall back to read-only instead (and switch between the modes

+# later), e.g. depending on whether the image file is writable

+# or whether a writing user is attached to the node

+# (default: false, since 3.1)

# @detect-zeroes: detect and optimize zero writes (Since 2.1)

# (default: off)

# @force-share: force share all permission on added nodes.

@@ -XXX,XX +XXX,XX @@

'*discard': 'BlockdevDiscardOptions',

'*cache': 'BlockdevCacheOptions',

'*read-only': 'bool',

+ '*auto-read-only': 'bool',

'*force-share': 'bool',

'*detect-zeroes': 'BlockdevDetectZeroesOptions' },

'discriminator': 'driver',

diff --git a/include/block/block.h b/include/block/block.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/block.h

+++ b/include/block/block.h

@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {

select an appropriate protocol driver,

ignoring the format layer */

#define BDRV_O_NO_IO 0x10000 /* don't initialize for I/O */

+#define BDRV_O_AUTO_RDONLY 0x20000 /* degrade to read-only if opening read-write fails */

#define BDRV_O_CACHE_MASK (BDRV_O_NOCACHE | BDRV_O_NO_FLUSH)

@@ -XXX,XX +XXX,XX @@ typedef struct HDGeometry {

#define BDRV_OPT_CACHE_DIRECT "cache.direct"

#define BDRV_OPT_CACHE_NO_FLUSH "cache.no-flush"

#define BDRV_OPT_READ_ONLY "read-only"

+#define BDRV_OPT_AUTO_READ_ONLY "auto-read-only"

#define BDRV_OPT_DISCARD "discard"

#define BDRV_OPT_FORCE_SHARE "force-share"

diff --git a/block.c b/block.c

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ static void bdrv_inherited_options(int *child_flags, QDict *child_options,

/* Inherit the read-only option from the parent if it's not set */

qdict_copy_default(child_options, parent_options, BDRV_OPT_READ_ONLY);

+ qdict_copy_default(child_options, parent_options, BDRV_OPT_AUTO_READ_ONLY);

/* Our block drivers take care to send flushes and respect unmap policy,

* so we can default to enable both on lower layers regardless of the

@@ -XXX,XX +XXX,XX @@ static void bdrv_backing_options(int *child_flags, QDict *child_options,

/* backing files always opened read-only */

qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "on");

+ qdict_set_default_str(child_options, BDRV_OPT_AUTO_READ_ONLY, "off");

flags &= ~BDRV_O_COPY_ON_READ;

/* snapshot=on is handled on the top layer */

@@ -XXX,XX +XXX,XX @@ static void update_flags_from_options(int *flags, QemuOpts *opts)

*flags |= BDRV_O_RDWR;

}

+ assert(qemu_opt_find(opts, BDRV_OPT_AUTO_READ_ONLY));

+ if (qemu_opt_get_bool_del(opts, BDRV_OPT_AUTO_READ_ONLY, false)) {

+ *flags |= BDRV_O_AUTO_RDONLY;

+ }

}

static void update_options_from_flags(QDict *options, int flags)

@@ -XXX,XX +XXX,XX @@ static void update_options_from_flags(QDict *options, int flags)

if (!qdict_haskey(options, BDRV_OPT_READ_ONLY)) {

qdict_put_bool(options, BDRV_OPT_READ_ONLY, !(flags & BDRV_O_RDWR));

}

+ if (!qdict_haskey(options, BDRV_OPT_AUTO_READ_ONLY)) {

+ qdict_put_bool(options, BDRV_OPT_AUTO_READ_ONLY,

+ flags & BDRV_O_AUTO_RDONLY);

+ }

}

static void bdrv_assign_node_name(BlockDriverState *bs,

@@ -XXX,XX +XXX,XX @@ QemuOptsList bdrv_runtime_opts = {

.type = QEMU_OPT_BOOL,

.help = "Node is opened in read-only mode",

},

+ {

+ .name = BDRV_OPT_AUTO_READ_ONLY,

+ .type = QEMU_OPT_BOOL,

+ .help = "Node can become read-only if opening read-write fails",

+ },

{

.name = "detect-zeroes",

.type = QEMU_OPT_STRING,

@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_open_blockdev_ref(BlockdevRef *ref, Error **errp)

qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");

qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");

qdict_set_default_str(qdict, BDRV_OPT_READ_ONLY, "off");

+ qdict_set_default_str(qdict, BDRV_OPT_AUTO_READ_ONLY, "off");

+

}

bs = bdrv_open_inherit(NULL, reference, qdict, 0, NULL, NULL, errp);

diff --git a/block/vvfat.c b/block/vvfat.c

index XXXXXXX..XXXXXXX 100644

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ static void vvfat_qcow_options(int *child_flags, QDict *child_options,

int parent_flags, QDict *parent_options)

{

qdict_set_default_str(child_options, BDRV_OPT_READ_ONLY, "off");

+ qdict_set_default_str(child_options, BDRV_OPT_AUTO_READ_ONLY, "off");

qdict_set_default_str(child_options, BDRV_OPT_CACHE_NO_FLUSH, "on");

}

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ void qmp_blockdev_change_medium(bool has_device, const char *device,

bdrv_flags = blk_get_open_flags_from_root_state(blk);

bdrv_flags &= ~(BDRV_O_TEMPORARY | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING |

- BDRV_O_PROTOCOL);

+ BDRV_O_PROTOCOL | BDRV_O_AUTO_RDONLY);

if (!has_read_only) {

read_only = BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN;

2.19.1

tests/qemu-iotests/232 | 147 +++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/232.out | 59 +++++++++++++++

tests/qemu-iotests/group | 1 +

3 files changed, 207 insertions(+)

create mode 100755 tests/qemu-iotests/232

create mode 100644 tests/qemu-iotests/232.out

diff --git a/tests/qemu-iotests/232 b/tests/qemu-iotests/232

+++ b/tests/qemu-iotests/232

+#!/bin/bash

+# Test for auto-read-only

+# Copyright (C) 2018 Red Hat, Inc.

+# creator

+owner=kwolf@redhat.com

+

+seq=`basename $0`

+ rm -f $TEST_IMG.snap

+_supported_fmt generic

+function do_run_qemu()

+{

+ echo Testing: "$@"

+ (

+ if ! test -t 0; then

+ while read cmd; do

+ echo $cmd

+ done

+ fi

+ echo quit

+ ) | $QEMU -nographic -monitor stdio -nodefaults "$@"

+ echo

+}

+

+function run_qemu()

+{

+ do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qemu | _filter_hmp |

+ _filter_generated_node_ids | _filter_imgfmt

+}

+

+function run_qemu_info_block()

+{

+ echo "info block -n" | run_qemu "$@" | grep -e "(file" -e "QEMU_PROG"

+}

+

+size=128M

+

+_make_test_img $size

+echo "=== -drive with read-write image: read-only/auto-read-only combinations ==="

+echo

+

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on

+echo

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off

+echo

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none

+echo "=== -drive with read-only image: read-only/auto-read-only combinations ==="

+echo

+

+chmod a-w $TEST_IMG

+

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=on

+echo

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,read-only=off

+echo

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,auto-read-only=off

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none,auto-read-only=on

+run_qemu_info_block -drive driver=file,file="$TEST_IMG",if=none

+

+echo

+echo "=== -blockdev with read-write image: read-only/auto-read-only combinations ==="

+echo

+

+chmod a+w $TEST_IMG

+

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on

+echo

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off

+echo

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0

+

+echo

+echo "=== -blockdev with read-only image: read-only/auto-read-only combinations ==="

+echo

+

+chmod a-w $TEST_IMG

+

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=on

+echo

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,read-only=off

+echo

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,auto-read-only=off

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,auto-read-only=on

+run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0

diff --git a/tests/qemu-iotests/232.out b/tests/qemu-iotests/232.out

+++ b/tests/qemu-iotests/232.out

+QA output created by 232

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728

+=== -drive with read-write image: read-only/auto-read-only combinations ===

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+NODE_NAME: TEST_DIR/t.IMGFMT (file)

+=== -drive with read-only image: read-only/auto-read-only combinations ===

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+

+QEMU_PROG: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none,read-only=off,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+

+QEMU_PROG: -drive driver=file,file=TEST_DIR/t.IMGFMT,if=none,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+NODE_NAME: TEST_DIR/t.IMGFMT (file, read-only)

+

+=== -blockdev with read-write image: read-only/auto-read-only combinations ===

+

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+

+node0: TEST_DIR/t.IMGFMT (file)

+node0: TEST_DIR/t.IMGFMT (file)

+node0: TEST_DIR/t.IMGFMT (file)

+

+node0: TEST_DIR/t.IMGFMT (file)

+node0: TEST_DIR/t.IMGFMT (file)

+node0: TEST_DIR/t.IMGFMT (file)

+

+=== -blockdev with read-only image: read-only/auto-read-only combinations ===

+

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+

+QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read-only=off,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

+

+QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,auto-read-only=off: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

+node0: TEST_DIR/t.IMGFMT (file, read-only)

+QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0: Could not open 'TEST_DIR/t.IMGFMT': Permission denied

227 auto quick

229 auto quick

231 auto quick

+232 auto quick

2.19.1

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.

include/block/block.h | 3 ++-

block.c | 42 +++++++++++++++++++++++++++---------------

block/bochs.c | 17 ++++++-----------

block/cloop.c | 16 +++++-----------

block/dmg.c | 16 +++++-----------

block/rbd.c | 15 ++++-----------

block/vvfat.c | 10 ++--------

7 files changed, 51 insertions(+), 68 deletions(-)

@@ -XXX,XX +XXX,XX @@ 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

+ * or bdrv_can_set_read_only() forbids making the node read-only. 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 (ret < 0) {

- return ret;

+ if (!(bs->open_flags & BDRV_O_RDWR)) {

+ return 0;

+ }

+ if (!(bs->open_flags & BDRV_O_AUTO_RDONLY)) {

+ goto fail;

- bs->read_only = read_only;

-

- if (read_only) {

- bs->open_flags &= ~BDRV_O_RDWR;

- } else {

- bs->open_flags |= BDRV_O_RDWR;

+ ret = bdrv_can_set_read_only(bs, true, false, NULL);

+ if (ret < 0) {

+ goto fail;

+ bs->read_only = true;

+ bs->open_flags &= ~BDRV_O_RDWR;

+

return 0;

+

+fail:

+ error_setg(errp, "%s", errmsg ?: "Image is read-only");

+ return -EACCES;

}

void bdrv_get_full_backing_filename_from_filename(const char *backed,

diff --git a/block/bochs.c b/block/bochs.c

index XXXXXXX..XXXXXXX 100644

--- a/block/bochs.c

+++ b/block/bochs.c

@@ -XXX,XX +XXX,XX @@ 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

--- a/block/cloop.c

+++ b/block/cloop.c

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/block/dmg.c

+++ b/block/dmg.c

@@ -XXX,XX +XXX,XX @@ 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 XXXXXXX..XXXXXXX 100644

--- a/block/rbd.c

+++ b/block/rbd.c

@@ -XXX,XX +XXX,XX @@ 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) {

- rbd_close(s->image);

- 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 XXXXXXX..XXXXXXX 100644

--- a/block/vvfat.c

+++ b/block/vvfat.c

@@ -XXX,XX +XXX,XX @@ 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;

}

}

2.19.1

From: Max Reitz <mreitz@redhat.com>

This adds some whitespace into the option help (including indentation)

and puts angle brackets around the type names. Furthermore, the list

name is no longer printed as part of every line, but only once in

advance, and only if the caller did not print a caption already.

This patch also restores the description alignment we had before commit

9cbef9d68ee1d8d0, just at 24 instead of 16 characters like we used to.

This increase is because now we have the type and two spaces of

indentation before the description, and with a usual type name length of

three chracters, this sums up to eight additional characters -- which

means that we now need 24 characters to get the same amount of padding

for most options. Also, 24 is a third of 80, which makes it kind of a

round number in terminal terms.

Finally, this patch amends the reference output of iotest 082 to match

the changes (and thus makes it pass again).

include/qemu/option.h | 2 +-

qemu-img.c | 4 +-

util/qemu-option.c | 32 +-

tests/qemu-iotests/082.out | 956 ++++++++++++++++++-------------------

4 files changed, 507 insertions(+), 487 deletions(-)

diff --git a/include/qemu/option.h b/include/qemu/option.h

--- a/include/qemu/option.h

+++ b/include/qemu/option.h

@@ -XXX,XX +XXX,XX @@ typedef int (*qemu_opts_loopfunc)(void *opaque, QemuOpts *opts, Error **errp);

int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func,

void *opaque, Error **errp);

void qemu_opts_print(QemuOpts *opts, const char *sep);

-void qemu_opts_print_help(QemuOptsList *list);

+void qemu_opts_print_help(QemuOptsList *list, bool print_caption);

void qemu_opts_free(QemuOptsList *list);

QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);

diff --git a/qemu-img.c b/qemu-img.c

index XXXXXXX..XXXXXXX 100644

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static int print_block_option_help(const char *filename, const char *fmt)

printf("Supported options:\n");

- qemu_opts_print_help(create_opts);

+ qemu_opts_print_help(create_opts, false);

qemu_opts_free(create_opts);

return 0;

}

@@ -XXX,XX +XXX,XX @@ static int print_amend_option_help(const char *format)

assert(drv->create_opts);

printf("Creation options for '%s':\n", format);

- qemu_opts_print_help(drv->create_opts);

+ qemu_opts_print_help(drv->create_opts, false);

printf("\nNote that not all of these options may be amendable.\n");

return 0;

}

diff --git a/util/qemu-option.c b/util/qemu-option.c

index XXXXXXX..XXXXXXX 100644

--- a/util/qemu-option.c

+++ b/util/qemu-option.c

@@ -XXX,XX +XXX,XX @@ static const char *opt_type_to_string(enum QemuOptType type)

g_assert_not_reached();

}

-void qemu_opts_print_help(QemuOptsList *list)

+/**

+ * Print the list of options available in the given list. If

+ * @print_caption is true, a caption (including the list name, if it

+ * exists) is printed. The options itself will be indented, so

+ * @print_caption should only be set to false if the caller prints its

+ * own custom caption (so that the indentation makes sense).

+ */

+void qemu_opts_print_help(QemuOptsList *list, bool print_caption)

{

QemuOptDesc *desc;

int i;

@@ -XXX,XX +XXX,XX @@ void qemu_opts_print_help(QemuOptsList *list)

desc = list->desc;

while (desc && desc->name) {

GString *str = g_string_new(NULL);

- if (list->name) {

- g_string_append_printf(str, "%s.", list->name);

- }

- g_string_append_printf(str, "%s=%s", desc->name,

+ g_string_append_printf(str, " %s=<%s>", desc->name,

opt_type_to_string(desc->type));

if (desc->help) {

+ if (str->len < 24) {

+ g_string_append_printf(str, "%*s", 24 - (int)str->len, "");

+ }

g_string_append_printf(str, " - %s", desc->help);

}

g_ptr_array_add(array, g_string_free(str, false));

@@ -XXX,XX +XXX,XX @@ void qemu_opts_print_help(QemuOptsList *list)

g_ptr_array_sort(array, (GCompareFunc)qemu_pstrcmp0);

+ if (print_caption && array->len > 0) {

+ if (list->name) {

+ printf("%s options:\n", list->name);

+ } else {

+ printf("Options:\n");

+ }

+ } else if (array->len == 0) {

+ if (list->name) {

+ printf("There are no options for %s.\n", list->name);

+ } else {

+ printf("No options available.\n");

+ }

+ }

for (i = 0; i < array->len; i++) {

printf("%s\n", (char *)array->pdata[i]);

}

@@ -XXX,XX +XXX,XX @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,

opts = opts_parse(list, params, permit_abbrev, false, &invalidp, &err);

if (err) {

if (invalidp && has_help_option(params)) {

- qemu_opts_print_help(list);

+ qemu_opts_print_help(list, true);

error_free(err);

} else {

error_report_err(err);

diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/082.out

+++ b/tests/qemu-iotests/082.out

@@ -XXX,XX +XXX,XX @@ cluster_size: 8192

Testing: create -f qcow2 -o help TEST_DIR/t.qcow2 128M

Supported options:

-size Virtual disk size

-compat Compatibility level (0.10 or 1.1)

-backing_file File name of a base image

-backing_fmt Image format of the base image

-encryption Encrypt the image with format 'aes'. (Deprecated in favor of encrypt.format=aes)

-encrypt.format Encrypt the image, format choices: 'aes', 'luks'

-encrypt.key-secret ID of secret providing qcow AES key or LUKS passphrase

-encrypt.cipher-alg Name of encryption cipher algorithm

-encrypt.cipher-mode Name of encryption cipher mode

-encrypt.ivgen-alg Name of IV generator algorithm

-encrypt.ivgen-hash-alg Name of IV generator hash algorithm

-encrypt.hash-alg Name of encryption hash algorithm

-encrypt.iter-time Time to spend in PBKDF in milliseconds

-cluster_size qcow2 cluster size

-preallocation Preallocation mode (allowed values: off, metadata, falloc, full)

-lazy_refcounts Postpone refcount updates

-refcount_bits Width of a reference count entry in bits

-nocow Turn off copy-on-write (valid only on btrfs)

+ backing_file=<str> - File name of a base image

+ backing_fmt=<str> - Image format of the base image

+ cluster_size=<size> - qcow2 cluster size

+ compat=<str> - Compatibility level (0.10 or 1.1)

+ encrypt.cipher-alg=<str> - Name of encryption cipher algorithm

+ encrypt.cipher-mode=<str> - Name of encryption cipher mode