The following changes since commit eda1df0345f5a1e337e30367124dcb0e802bdfde:

Merge remote-tracking branch 'remotes/armbru/tags/pull-pflash-2019-03-11' into staging (2019-03-12 11:12:36 +0000)

for you to fetch changes up to c31dfeb02a1d155bdb961edeb61a137a589c174b:

qemu-iotests: Test the x-blockdev-reopen QMP command (2019-03-12 17:58:37 +0100)

- file-posix: Make auto-read-only dynamic

- Add x-blockdev-reopen QMP command

- Finalize block-latency-histogram QMP command

- gluster: Build fixes for newer lib version

Alberto Garcia (13):

block: Allow freezing BdrvChild links

block: Freeze the backing chain for the duration of the commit job

block: Freeze the backing chain for the duration of the mirror job

block: Freeze the backing chain for the duration of the stream job

block: Add 'keep_old_opts' parameter to bdrv_reopen_queue()

block: Handle child references in bdrv_reopen_queue()

block: Allow omitting the 'backing' option in certain cases

block: Allow changing the backing file on reopen

block: Add a 'mutable_opts' field to BlockDriver

block: Add bdrv_reset_options_allowed()

block: Remove the AioContext parameter from bdrv_reopen_multiple()

block: Add an 'x-blockdev-reopen' QMP command

qemu-iotests: Test the x-blockdev-reopen QMP command

Keith Busch (1):

nvme: fix write zeroes offset and count

Kevin Wolf (10):

tests/virtio-blk-test: Disable auto-read-only

qemu-iotests: commit to backing file with auto-read-only

block: Avoid useless local_err

block: Make permission changes in reopen less wrong

file-posix: Fix bdrv_open_flags() for snapshot=on

file-posix: Factor out raw_reconfigure_getfd()

file-posix: Store BDRVRawState.reopen_state during reopen

file-posix: Lock new fd in raw_reopen_prepare()

file-posix: Prepare permission code for fd switching

file-posix: Make auto-read-only dynamic

Niels de Vos (1):

gluster: the glfs_io_cbk callback function pointer adds pre/post stat args

Prasanna Kumar Kalever (1):

gluster: Handle changed glfs_ftruncate signature

Vladimir Sementsov-Ogievskiy (2):

qapi: move to QOM path for x-block-latency-histogram-set

qapi: drop x- from x-block-latency-histogram-set

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

configure | 42 ++

include/block/block.h | 13 +-

include/block/block_int.h | 14 +

block.c | 440 +++++++++++++++++--

block/commit.c | 16 +

block/file-posix.c | 254 ++++++++---

block/gluster.c | 10 +-

block/mirror.c | 8 +

block/qapi.c | 12 +-

block/qcow2.c | 25 ++

block/raw-format.c | 3 +

block/replication.c | 7 +-

block/stream.c | 21 +

blockdev.c | 61 ++-

hw/block/nvme.c | 6 +-

qemu-io-cmds.c | 4 +-

tests/virtio-blk-test.c | 2 +-

tests/qemu-iotests/051 | 7 +

tests/qemu-iotests/051.out | 9 +

tests/qemu-iotests/051.pc.out | 9 +

tests/qemu-iotests/232 | 31 ++

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

tests/qemu-iotests/245 | 991 ++++++++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/245.out | 5 +

tests/qemu-iotests/group | 1 +

26 files changed, 1929 insertions(+), 160 deletions(-)

create mode 100644 tests/qemu-iotests/245

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

From: Prasanna Kumar Kalever <prasanna.kalever@redhat.com>

configure | 18 ++++++++++++++++++

block/gluster.c | 4 ++++

2 files changed, 22 insertions(+)

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

#include "qemu/option.h"

#include "qemu/cutils.h"

+#ifdef CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT

+# define glfs_ftruncate(fd, offset) glfs_ftruncate(fd, offset, NULL, NULL)

+#endif

#define GLUSTER_OPT_FILENAME "filename"

#define GLUSTER_OPT_VOLUME "volume"

#define GLUSTER_OPT_PATH "path"

2.20.1

From: Niels de Vos <ndevos@redhat.com>

The glfs_*_async() functions do a callback once finished. This callback

has changed its arguments, pre- and post-stat structures have been

added. This makes it possible to improve caching, which is useful for

Samba and NFS-Ganesha, but not so much for QEMU. Gluster 6 is the first

release that includes these new arguments.

With an additional detection in ./configure, the new arguments can

conditionally get included in the glfs_io_cbk handler.

Signed-off-by: Niels de Vos <ndevos@redhat.com>

configure | 24 ++++++++++++++++++++++++

block/gluster.c | 6 +++++-

2 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/configure b/configure

--- a/configure

+++ b/configure

@@ -XXX,XX +XXX,XX @@ glusterfs_discard="no"

glusterfs_fallocate="no"

glusterfs_zerofill="no"

glusterfs_ftruncate_has_stat="no"

+glusterfs_iocb_has_stat="no"

gtk=""

gtk_gl="no"

tls_priority="NORMAL"

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

if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then

glusterfs_ftruncate_has_stat="yes"

fi

+ cat > $TMPC << EOF

+#include <glusterfs/api/glfs.h>

+

+/* new glfs_io_cbk() passes two additional glfs_stat structs */

+static void

+glusterfs_iocb(glfs_fd_t *fd, ssize_t ret, struct glfs_stat *prestat, struct glfs_stat *poststat, void *data)

+{}

+

+int

+main(void)

+{

+    glfs_io_cbk iocb = &glusterfs_iocb;

+    iocb(NULL, 0 , NULL, NULL, NULL);

+    return 0;

+}

+EOF

+ if compile_prog "$glusterfs_cflags" "$glusterfs_libs" ; then

+ glusterfs_iocb_has_stat="yes"

+ fi

else

if test "$glusterfs" = "yes" ; then

feature_not_found "GlusterFS backend support" \

@@ -XXX,XX +XXX,XX @@ if test "$glusterfs_ftruncate_has_stat" = "yes" ; then

echo "CONFIG_GLUSTERFS_FTRUNCATE_HAS_STAT=y" >> $config_host_mak

fi

+if test "$glusterfs_iocb_has_stat" = "yes" ; then

+ echo "CONFIG_GLUSTERFS_IOCB_HAS_STAT=y" >> $config_host_mak

+fi

+

if test "$libssh2" = "yes" ; then

echo "CONFIG_LIBSSH2=m" >> $config_host_mak

echo "LIBSSH2_CFLAGS=$libssh2_cflags" >> $config_host_mak

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

index XXXXXXX..XXXXXXX 100644

--- a/block/gluster.c

+++ b/block/gluster.c

@@ -XXX,XX +XXX,XX @@ static struct glfs *qemu_gluster_init(BlockdevOptionsGluster *gconf,

/*

* AIO callback routine called from GlusterFS thread.

*/

-static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret, void *arg)

+static void gluster_finish_aiocb(struct glfs_fd *fd, ssize_t ret,

+#ifdef CONFIG_GLUSTERFS_IOCB_HAS_STAT

+ struct glfs_stat *pre, struct glfs_stat *post,

+#endif

+ void *arg)

{

GlusterAIOCB *acb = (GlusterAIOCB *)arg;

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

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

blockdev.c | 12 ++++++------

2 files changed, 8 insertions(+), 8 deletions(-)

@@ -XXX,XX +XXX,XX @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,

}

void qmp_x_block_latency_histogram_set(

- const char *device,

+ const char *id,

bool has_boundaries, uint64List *boundaries,

bool has_boundaries_read, uint64List *boundaries_read,

bool has_boundaries_write, uint64List *boundaries_write,

bool has_boundaries_flush, uint64List *boundaries_flush,

Error **errp)

{

- BlockBackend *blk = blk_by_name(device);

+ BlockBackend *blk = qmp_get_blk(NULL, id, errp);

BlockAcctStats *stats;

int ret;

if (!blk) {

- error_setg(errp, "Device '%s' not found", device);

return;

}

+

stats = blk_get_stats(blk);

if (!has_boundaries && !has_boundaries_read && !has_boundaries_write &&

@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(

stats, BLOCK_ACCT_READ,

has_boundaries_read ? boundaries_read : boundaries);

if (ret) {

- error_setg(errp, "Device '%s' set read boundaries fail", device);

+ error_setg(errp, "Device '%s' set read boundaries fail", id);

return;

}

}

@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(

stats, BLOCK_ACCT_WRITE,

has_boundaries_write ? boundaries_write : boundaries);

if (ret) {

- error_setg(errp, "Device '%s' set write boundaries fail", device);

+ error_setg(errp, "Device '%s' set write boundaries fail", id);

return;

}

}

@@ -XXX,XX +XXX,XX @@ void qmp_x_block_latency_histogram_set(

stats, BLOCK_ACCT_FLUSH,

has_boundaries_flush ? boundaries_flush : boundaries);

if (ret) {

- error_setg(errp, "Device '%s' set flush boundaries fail", device);

+ error_setg(errp, "Device '%s' set flush boundaries fail", id);

return;

}

}

2.20.1

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Drop x- and x_ prefixes for latency histograms and update version to

3.1

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

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

block/qapi.c | 12 ++++++------

blockdev.c | 2 +-

3 files changed, 17 insertions(+), 17 deletions(-)

# +------------------

# 10 50 100

#

-# Since: 2.12

+# Since: 4.0

##

{ 'struct': 'BlockLatencyHistogramInfo',

'data': {'boundaries': ['uint64'], 'bins': ['uint64'] } }

##

-# @x-block-latency-histogram-set:

+# @block-latency-histogram-set:

#

# Manage read, write and flush latency histograms for the device.

#

#

# Returns: error if device is not found or any boundary arrays are invalid.

#

-# Since: 2.12

+# Since: 4.0

#

# Example: set new histograms for all io types with intervals

# [0, 10), [10, 50), [50, 100), [100, +inf):

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

# "arguments": { "device": "drive0" } }

# <- { "return": {} }

##

-{ 'command': 'x-block-latency-histogram-set',

+{ 'command': 'block-latency-histogram-set',

'data': {'id': 'str',

'*boundaries': ['uint64'],

'*boundaries-read': ['uint64'],

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

# @timed_stats: Statistics specific to the set of previously defined

# intervals of time (Since 2.5)

#

-# @x_rd_latency_histogram: @BlockLatencyHistogramInfo. (Since 2.12)

+# @rd_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)

#

-# @x_wr_latency_histogram: @BlockLatencyHistogramInfo. (Since 2.12)

+# @wr_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)

#

-# @x_flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 2.12)

+# @flush_latency_histogram: @BlockLatencyHistogramInfo. (Since 4.0)

#

# Since: 0.14.0

##

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

'invalid_wr_operations': 'int', 'invalid_flush_operations': 'int',

'account_invalid': 'bool', 'account_failed': 'bool',

'timed_stats': ['BlockDeviceTimedStats'],

- '*x_rd_latency_histogram': 'BlockLatencyHistogramInfo',

- '*x_wr_latency_histogram': 'BlockLatencyHistogramInfo',

- '*x_flush_latency_histogram': 'BlockLatencyHistogramInfo' } }

+ '*rd_latency_histogram': 'BlockLatencyHistogramInfo',

+ '*wr_latency_histogram': 'BlockLatencyHistogramInfo',

+ '*flush_latency_histogram': 'BlockLatencyHistogramInfo' } }

##

# @BlockStats:

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

--- a/block/qapi.c

+++ b/block/qapi.c

@@ -XXX,XX +XXX,XX @@ static void bdrv_query_blk_stats(BlockDeviceStats *ds, BlockBackend *blk)

bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_READ],

- &ds->has_x_rd_latency_histogram,

- &ds->x_rd_latency_histogram);

+ &ds->has_rd_latency_histogram,

+ &ds->rd_latency_histogram);

bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_WRITE],

- &ds->has_x_wr_latency_histogram,

- &ds->x_wr_latency_histogram);

+ &ds->has_wr_latency_histogram,

+ &ds->wr_latency_histogram);

bdrv_latency_histogram_stats(&stats->latency_histogram[BLOCK_ACCT_FLUSH],

- &ds->has_x_flush_latency_histogram,

- &ds->x_flush_latency_histogram);

+ &ds->has_flush_latency_histogram,

+ &ds->flush_latency_histogram);

static BlockStats *bdrv_query_bds_stats(BlockDriverState *bs,

@@ -XXX,XX +XXX,XX @@ void qmp_x_blockdev_set_iothread(const char *node_name, StrOrNull *iothread,

aio_context_release(old_context);

}

-void qmp_x_block_latency_histogram_set(

+void qmp_block_latency_histogram_set(

const char *id,

bool has_boundaries, uint64List *boundaries,

bool has_boundaries_read, uint64List *boundaries_read,

2.20.1

tests/virtio-blk-test uses a temporary image file that it deletes while

QEMU is still running, so it can't be reopened when writers are

attached or detached. Disable auto-read-only to keep it always writable.

tests/virtio-blk-test.c | 2 +-

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

diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c

--- a/tests/virtio-blk-test.c

+++ b/tests/virtio-blk-test.c

@@ -XXX,XX +XXX,XX @@ static void *virtio_blk_test_setup(GString *cmd_line, void *arg)

char *tmp_path = drive_create();

g_string_append_printf(cmd_line,

- " -drive if=none,id=drive0,file=%s,format=raw "

+ " -drive if=none,id=drive0,file=%s,format=raw,auto-read-only=off "

"-drive if=none,id=drive1,file=null-co://,format=raw ",

tmp_path);

2.20.1

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

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

2 files changed, 51 insertions(+)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/232

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

@@ -XXX,XX +XXX,XX @@ status=1    # failure is the default!

_cleanup()

{

_cleanup_test_img

+ rm -f $TEST_IMG.[01234]

}

trap "_cleanup; exit \$status" 0 1 2 3 15

@@ -XXX,XX +XXX,XX @@ run_qemu_info_block -blockdev driver=file,filename="$TEST_IMG",node-name=node0,a

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 "=== Try commit to backing file with auto-read-only ==="

+echo

+

+TEST_IMG="$TEST_IMG.0" _make_test_img $size

+TEST_IMG="$TEST_IMG.1" _make_test_img $size

+TEST_IMG="$TEST_IMG.2" _make_test_img $size

+TEST_IMG="$TEST_IMG.3" _make_test_img $size

+TEST_IMG="$TEST_IMG.4" _make_test_img $size

+

+(cat <<EOF

+{"execute":"qmp_capabilities"}

+{"execute":"block-commit",

+ "arguments":{"device":"format-4", "top-node": "format-2", "base-node":"format-0", "job-id":"job0"}}

+EOF

+sleep 1

+echo '{"execute":"quit"}'

+) | $QEMU -qmp stdio -nographic -nodefaults \

+ -blockdev file,node-name=file-0,filename=$TEST_IMG.0,auto-read-only=on \

+ -blockdev qcow2,node-name=format-0,file=file-0,read-only=on \

+ -blockdev file,node-name=file-1,filename=$TEST_IMG.1,auto-read-only=on \

+ -blockdev qcow2,node-name=format-1,file=file-1,read-only=on,backing=format-0 \

+ -blockdev file,node-name=file-2,filename=$TEST_IMG.2,auto-read-only=on \

+ -blockdev qcow2,node-name=format-2,file=file-2,read-only=on,backing=format-1 \

+ -blockdev file,node-name=file-3,filename=$TEST_IMG.3,auto-read-only=on \

+ -blockdev qcow2,node-name=format-3,file=file-3,read-only=on,backing=format-2 \

+ -blockdev file,node-name=file-4,filename=$TEST_IMG.4,auto-read-only=on \

+ -blockdev qcow2,node-name=format-4,file=file-4,read-only=on,backing=format-3 |

+ _filter_qmp

+

# success, all done

echo "*** done"

rm -f $seq.full

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

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

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

@@ -XXX,XX +XXX,XX @@ QEMU_PROG: -blockdev driver=file,filename=TEST_DIR/t.IMGFMT,node-name=node0,read

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

+

+=== Try commit to backing file with auto-read-only ===

+

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

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

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

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

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

+QMP_VERSION

+{"return": {}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}

+{"return": {}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "waiting", "id": "job0"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "pending", "id": "job0"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_COMPLETED", "data": {"device": "job0", "len": 134217728, "offset": 134217728, "speed": 0, "type": "commit"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "concluded", "id": "job0"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "null", "id": "job0"}}

+{"return": {}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}

*** done

2.20.1

block.c | 4 +---

1 file changed, 1 insertion(+), 3 deletions(-)

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

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er

{

int ret = -1;

BlockReopenQueueEntry *bs_entry, *next;

- Error *local_err = NULL;

assert(bs_queue != NULL);

QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {

assert(bs_entry->state.bs->quiesce_counter > 0);

- if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {

- error_propagate(errp, local_err);

+ if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, errp)) {

goto cleanup;

}

bs_entry->prepared = true;

2.20.1

The way that reopen interacts with permission changes has one big

problem: Both operations are recursive, and the permissions are changes

for each node in the reopen queue.

For a simple graph that consists just of parent and child,

.bdrv_check_perm will be called twice for the child, once recursively

when adjusting the permissions of parent, and once again when the child

itself is reopened.

Even worse, the first .bdrv_check_perm call happens before

.bdrv_reopen_prepare was called for the child and the second one is

called afterwards.

Making sure that .bdrv_check_perm (and the other permission callbacks)

are called only once is hard. We can cope with multiple calls right now,

but as soon as file-posix gets a dynamic auto-read-only that may need to

open a new file descriptor, we get the additional requirement that all

of them are after the .bdrv_reopen_prepare call.

So reorder things in bdrv_reopen_multiple() to first call

.bdrv_reopen_prepare for all involved nodes and only then adjust

permissions.

block.c | 35 ++++++++++++++++++++++++-----------

1 file changed, 24 insertions(+), 11 deletions(-)

@@ -XXX,XX +XXX,XX @@ static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared);

typedef struct BlockReopenQueueEntry {

bool prepared;

+ bool perms_checked;

BDRVReopenState state;

QSIMPLEQ_ENTRY(BlockReopenQueueEntry) entry;

} BlockReopenQueueEntry;

@@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er

bs_entry->prepared = true;

+ QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {

+ BDRVReopenState *state = &bs_entry->state;

+ ret = bdrv_check_perm(state->bs, bs_queue, state->perm,

+ state->shared_perm, NULL, errp);

+ if (ret < 0) {

+ goto cleanup_perm;

+ }

+ bs_entry->perms_checked = true;

/* If we reach this point, we have success and just need to apply the

* changes

*/

@@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er

}

ret = 0;

+cleanup_perm:

+ QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {

+ BDRVReopenState *state = &bs_entry->state;

+

+ if (!bs_entry->perms_checked) {

+ continue;

+ }

+ if (ret == 0) {

+ bdrv_set_perm(state->bs, state->perm, state->shared_perm);

+ } else {

+ bdrv_abort_perm_update(state->bs);

+ }

+ }

cleanup:

QSIMPLEQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) {

if (ret) {

@@ -XXX,XX +XXX,XX @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue,

} while ((entry = qdict_next(reopen_state->options, entry)));

}

- ret = bdrv_check_perm(reopen_state->bs, queue, reopen_state->perm,

- reopen_state->shared_perm, NULL, errp);

- if (ret < 0) {

- goto error;

- }

-

ret = 0;

/* Restore the original reopen_state->options QDict */

@@ -XXX,XX +XXX,XX @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)

bdrv_refresh_limits(bs, NULL);

- bdrv_set_perm(reopen_state->bs, reopen_state->perm,

- reopen_state->shared_perm);

-

new_can_write =

!bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);

if (!old_can_write && new_can_write && drv->bdrv_reopen_bitmaps_rw) {

@@ -XXX,XX +XXX,XX @@ void bdrv_reopen_abort(BDRVReopenState *reopen_state)

if (drv->bdrv_reopen_abort) {

drv->bdrv_reopen_abort(reopen_state);

}

-

- bdrv_abort_perm_update(reopen_state->bs);

2.20.1

Using a different read-only setting for bs->open_flags than for the

flags to the driver's open function is just inconsistent and a bad idea.

After this patch, the temporary snapshot keeps being opened read-only if

read-only=on,snapshot=on is passed.

If we wanted to change this behaviour to make only the orginal image

file read-only, but the temporary overlay read-write (as the comment in

the removed code suggests), that change would have to be made in

bdrv_temp_snapshot_options() (where the comment suggests otherwise).

Addressing this inconsistency before introducing dynamic auto-read-only

is important because otherwise we would immediately try to reopen the

temporary overlay even though the file is already unlinked.

block.c | 7 -------

tests/qemu-iotests/051 | 7 +++++++

tests/qemu-iotests/051.out | 9 +++++++++

tests/qemu-iotests/051.pc.out | 9 +++++++++

4 files changed, 25 insertions(+), 7 deletions(-)

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

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ static int bdrv_open_flags(BlockDriverState *bs, int flags)

*/

open_flags &= ~(BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING | BDRV_O_PROTOCOL);

- /*

- * Snapshots should be writable.

- */

- if (flags & BDRV_O_TEMPORARY) {

- open_flags |= BDRV_O_RDWR;

- }

-

return open_flags;

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/051

+++ b/tests/qemu-iotests/051

@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c "read -P 0x33 0 4k" "$TEST_IMG" | _filter_qemu_io

# Using snapshot=on with a non-existent TMPDIR

TMPDIR=/nonexistent run_qemu -drive driver=null-co,snapshot=on

+# Using snapshot=on together with read-only=on

+echo "info block" |

+ run_qemu -drive file="$TEST_IMG",snapshot=on,read-only=on,if=none,id=$device_id |

+ _filter_qemu_io |

+ sed -e 's#"/[^"]*/vl\.[A-Za-z]\{6\}"#SNAPSHOT_PATH#g'

+

+

# success, all done

echo "*** done"

rm -f $seq.full

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ read 4096/4096 bytes at offset 0

Testing: -drive driver=null-co,snapshot=on

QEMU_PROG: -drive driver=null-co,snapshot=on: Could not get temporary filename: No such file or directory

+Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on,read-only=on,if=none,id=drive0

+QEMU X.Y.Z monitor - type 'help' for more information

+(qemu) info block

+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}}, "driver": "qcow2", "file": {"driver": "file", "filename": SNAPSHOT_PATH}} (qcow2, read-only)

+ Removable device: not locked, tray closed

+ Cache mode: writeback, ignore flushes

+ Backing file: TEST_DIR/t.qcow2 (chain depth: 1)

+(qemu) quit

+

*** done

diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/051.pc.out

+++ b/tests/qemu-iotests/051.pc.out

@@ -XXX,XX +XXX,XX @@ read 4096/4096 bytes at offset 0

Testing: -drive driver=null-co,snapshot=on

QEMU_PROG: -drive driver=null-co,snapshot=on: Could not get temporary filename: No such file or directory

+Testing: -drive file=TEST_DIR/t.qcow2,snapshot=on,read-only=on,if=none,id=drive0

+QEMU X.Y.Z monitor - type 'help' for more information

+(qemu) info block

+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": "file", "filename": "TEST_DIR/t.qcow2"}}, "driver": "qcow2", "file": {"driver": "file", "filename": SNAPSHOT_PATH}} (qcow2, read-only)

+ Removable device: not locked, tray closed

+ Cache mode: writeback, ignore flushes

+ Backing file: TEST_DIR/t.qcow2 (chain depth: 1)

+(qemu) quit

+

*** done

2.20.1

block/file-posix.c | 107 ++++++++++++++++++++++++++-------------------

1 file changed, 62 insertions(+), 45 deletions(-)

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

--- a/block/file-posix.c

+++ b/block/file-posix.c

@@ -XXX,XX +XXX,XX @@ static int raw_handle_perm_lock(BlockDriverState *bs,

return ret;

+static int raw_reconfigure_getfd(BlockDriverState *bs, int flags,

+ int *open_flags, Error **errp)

+{

+ BDRVRawState *s = bs->opaque;

+ int fd = -1;

+ int ret;

+ int fcntl_flags = O_APPEND | O_NONBLOCK;

+#ifdef O_NOATIME

+ fcntl_flags |= O_NOATIME;

+#endif

+

+ *open_flags = 0;

+ if (s->type == FTYPE_CD) {

+ *open_flags |= O_NONBLOCK;

+ }

+

+ raw_parse_flags(flags, open_flags);

+

+#ifdef O_ASYNC

+ /* Not all operating systems have O_ASYNC, and those that don't

+ * will not let us track the state into rs->open_flags (typically

+ * you achieve the same effect with an ioctl, for example I_SETSIG

+ * on Solaris). But we do not use O_ASYNC, so that's fine.

+ */

+ assert((s->open_flags & O_ASYNC) == 0);

+#endif

+

+ if ((*open_flags & ~fcntl_flags) == (s->open_flags & ~fcntl_flags)) {

+ /* dup the original fd */

+ fd = qemu_dup(s->fd);

+ if (fd >= 0) {

+ ret = fcntl_setfl(fd, *open_flags);

+ if (ret) {

+ qemu_close(fd);

+ fd = -1;