The following changes since commit 143c2e0432859826c9e8d5b2baa307355f1a5332:

Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2021-07-19' into staging (2021-07-19 19:06:05 +0100)

are available in the Git repository at:

for you to fetch changes up to d21471696b07f30cb00453709d055a25c1afde85:

iotests/307: Test iothread conflict for exports (2021-07-20 16:49:50 +0200)

Block layer patches

- mirror: Fix active mirror deadlock

- replication: Fix crashes due to operations on wrong BdrvChild

- configure: Add option to use driver whitelist even in tools

- vvfat: Fix crash when opening image read-write

- export: Fix crash in error path with fixed-iothread=false

Kevin Wolf (1):

block: Add option to use driver whitelist even in tools

Lukas Straub (4):

replication: Remove s->active_disk

replication: Reduce usage of s->hidden_disk and s->secondary_disk

replication: Properly attach children

replication: Remove workaround

Max Reitz (2):

block/export: Conditionally ignore set-context error

iotests/307: Test iothread conflict for exports

block/mirror: set .co for active-write MirrorOp objects

iotest 151: add test-case that shows active mirror dead-lock

block/mirror: fix active mirror dead-lock in mirror_wait_on_conflicts

block/vvfat: fix: drop backing

configure | 14 +++++-

block.c | 3 ++

block/export/export.c | 5 +-

block/mirror.c | 13 ++++++

block/replication.c | 111 +++++++++++++++++++++++++++------------------

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

meson.build | 1 +

tests/qemu-iotests/151 | 54 +++++++++++++++++++++-

tests/qemu-iotests/151.out | 4 +-

tests/qemu-iotests/307 | 15 ++++++

tests/qemu-iotests/307.out | 8 ++++

11 files changed, 182 insertions(+), 89 deletions(-)

This field is unused, but it very helpful for debugging.

Message-Id: <20210702211636.228981-2-vsementsov@virtuozzo.com>

block/mirror.c | 1 +

1 file changed, 1 insertion(+)

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

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ static MirrorOp *coroutine_fn active_write_prepare(MirrorBlockJob *s,

.bytes = bytes,

.is_active_write = true,

.is_in_flight = true,

+ .co = qemu_coroutine_self(),

};

qemu_co_queue_init(&op->waiting_requests);

QTAILQ_INSERT_TAIL(&s->ops_in_flight, op, next);

2.31.1

There is a dead-lock in active mirror: when we have parallel

intersecting requests (note that non intersecting requests may be

considered intersecting after aligning to mirror granularity), it may

happen that request A waits request B in mirror_wait_on_conflicts() and

request B waits for A.

Look at the test for details. Test now dead-locks, that's why it's

disabled. Next commit will fix mirror and enable the test.

Message-Id: <20210702211636.228981-3-vsementsov@virtuozzo.com>

tests/qemu-iotests/151 | 62 ++++++++++++++++++++++++++++++++++++--

tests/qemu-iotests/151.out | 4 +--

2 files changed, 62 insertions(+), 4 deletions(-)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/151

+++ b/tests/qemu-iotests/151

@@ -XXX,XX +XXX,XX @@ class TestActiveMirror(iotests.QMPTestCase):

'if': 'none',

'node-name': 'source-node',

'driver': iotests.imgfmt,

- 'file': {'driver': 'file',

- 'filename': source_img}}

+ 'file': {'driver': 'blkdebug',

+ 'image': {'driver': 'file',

+ 'filename': source_img}}}

blk_target = {'node-name': 'target-node',

'driver': iotests.imgfmt,

@@ -XXX,XX +XXX,XX @@ class TestActiveMirror(iotests.QMPTestCase):

self.potential_writes_in_flight = False

+ def testIntersectingActiveIO(self):

+ # FIXME: test-case is dead-locking. To reproduce dead-lock just drop

+ # this return statement

+ return

+

+ # Fill the source image

+ result = self.vm.hmp_qemu_io('source', 'write -P 1 0 2M')

+

+ # Start the block job (very slowly)

+ result = self.vm.qmp('blockdev-mirror',

+ job_id='mirror',

+ filter_node_name='mirror-node',

+ device='source-node',

+ target='target-node',

+ sync='full',

+ copy_mode='write-blocking',

+ speed=1)

+

+ self.vm.hmp_qemu_io('source', 'break write_aio A')

+ self.vm.hmp_qemu_io('source', 'aio_write 0 1M') # 1

+ self.vm.hmp_qemu_io('source', 'wait_break A')

+ self.vm.hmp_qemu_io('source', 'aio_write 0 2M') # 2

+ self.vm.hmp_qemu_io('source', 'aio_write 0 2M') # 3

+

+ # Now 2 and 3 are in mirror_wait_on_conflicts, waiting for 1

+

+ self.vm.hmp_qemu_io('source', 'break write_aio B')

+ self.vm.hmp_qemu_io('source', 'aio_write 1M 2M') # 4

+ self.vm.hmp_qemu_io('source', 'wait_break B')

+

+ # 4 doesn't wait for 2 and 3, because they didn't yet set

+ # in_flight_bitmap. So, nothing prevents 4 to go except for our

+ # break-point B.

+

+ self.vm.hmp_qemu_io('source', 'resume A')

+

+ # Now we resumed 1, so 2 and 3 goes to the next iteration of while loop

+ # in mirror_wait_on_conflicts(). They don't exit, as bitmap is dirty

+ # due to request 4. And they start to wait: 2 wait for 3, 3 wait for 2

+ # - DEAD LOCK.

+ # Note that it's important that we add request 4 at last: requests are

+ # appended to the list, so we are sure that 4 is last in the list, so 2

+ # and 3 now waits for each other, not for 4.

+

+ self.vm.hmp_qemu_io('source', 'resume B')

+

+ # Resuming 4 doesn't help, 2 and 3 already dead-locked

+ # To check the dead-lock run:

+ # gdb -p $(pidof qemu-system-x86_64) -ex 'set $job=(MirrorBlockJob *)jobs.lh_first' -ex 'p *$job->ops_in_flight.tqh_first' -ex 'p *$job->ops_in_flight.tqh_first->next.tqe_next'

+ # You'll see two MirrorOp objects waiting on each other

+

+ result = self.vm.qmp('block-job-set-speed', device='mirror', speed=0)

+ self.assert_qmp(result, 'return', {})

+ self.complete_and_wait(drive='mirror')

+

+ self.potential_writes_in_flight = False

+

if __name__ == '__main__':

iotests.main(supported_fmts=['qcow2', 'raw'],

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

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

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

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

-...

+....

----------------------------------------------------------------------

-Ran 3 tests

+Ran 4 tests

OK

2.31.1

It's possible that requests start to wait each other in

mirror_wait_on_conflicts(). To avoid it let's use same technique as in

block/io.c in bdrv_wait_serialising_requests_locked() /

bdrv_find_conflicting_request(): don't wait on intersecting request if

it is already waiting for some other request.

Message-Id: <20210702211636.228981-4-vsementsov@virtuozzo.com>

block/mirror.c | 12 ++++++++++++

tests/qemu-iotests/151 | 18 +++++-------------

2 files changed, 17 insertions(+), 13 deletions(-)

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

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ struct MirrorOp {

bool is_in_flight;

CoQueue waiting_requests;

Coroutine *co;

+ MirrorOp *waiting_for_op;

QTAILQ_ENTRY(MirrorOp) next;

};

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn mirror_wait_on_conflicts(MirrorOp *self,

if (ranges_overlap(self_start_chunk, self_nb_chunks,

op_start_chunk, op_nb_chunks))

{

+ /*

+ * If the operation is already (indirectly) waiting for us, or

+ * will wait for us as soon as it wakes up, then just go on

+ * (instead of producing a deadlock in the former case).

+ */

+ if (op->waiting_for_op) {

+ continue;

+ }

+

+ self->waiting_for_op = op;

qemu_co_queue_wait(&op->waiting_requests, NULL);

+ self->waiting_for_op = NULL;

break;

}

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/151

+++ b/tests/qemu-iotests/151

@@ -XXX,XX +XXX,XX @@ class TestActiveMirror(iotests.QMPTestCase):

self.potential_writes_in_flight = False

def testIntersectingActiveIO(self):

- # FIXME: test-case is dead-locking. To reproduce dead-lock just drop

- # this return statement

- return

-

# Fill the source image

result = self.vm.hmp_qemu_io('source', 'write -P 1 0 2M')

@@ -XXX,XX +XXX,XX @@ class TestActiveMirror(iotests.QMPTestCase):

# Now we resumed 1, so 2 and 3 goes to the next iteration of while loop

# in mirror_wait_on_conflicts(). They don't exit, as bitmap is dirty

- # due to request 4. And they start to wait: 2 wait for 3, 3 wait for 2

- # - DEAD LOCK.

- # Note that it's important that we add request 4 at last: requests are

- # appended to the list, so we are sure that 4 is last in the list, so 2

- # and 3 now waits for each other, not for 4.

+ # due to request 4.

+ # In the past at that point 2 and 3 would wait for each other producing

+ # a dead-lock. Now this is fixed and they will wait for request 4.

self.vm.hmp_qemu_io('source', 'resume B')

- # Resuming 4 doesn't help, 2 and 3 already dead-locked

- # To check the dead-lock run:

- # gdb -p $(pidof qemu-system-x86_64) -ex 'set $job=(MirrorBlockJob *)jobs.lh_first' -ex 'p *$job->ops_in_flight.tqh_first' -ex 'p *$job->ops_in_flight.tqh_first->next.tqe_next'

- # You'll see two MirrorOp objects waiting on each other

+ # After resuming 4, one of 2 and 3 goes first and set in_flight_bitmap,

+ # so the other will wait for it.

result = self.vm.qmp('block-job-set-speed', device='mirror', speed=0)

self.assert_qmp(result, 'return', {})

2.31.1

Currently, the block driver whitelists are only applied for the system

emulator. All other binaries still give unrestricted access to all block

drivers. There are use cases where this made sense because the main

concern was avoiding customers running VMs on less optimised block

drivers and getting bad performance. Allowing the same image format e.g.

as a target for 'qemu-img convert' is not a problem then.

However, if the concern is the supportability of the driver in general,

either in full or when used read-write, not applying the list driver

whitelist in tools doesn't help - especially since qemu-nbd and

qemu-storage-daemon now give access to more or less the same operations

in block drivers as running a system emulator.

In order to address this, introduce a new configure option that enforces

the driver whitelist in all binaries.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-Id: <20210709164141.254097-1-kwolf@redhat.com>

Reviewed-by: Eric Blake <eblake@redhat.com>

configure | 14 ++++++++++++--

block.c | 3 +++

meson.build | 1 +

3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/configure b/configure

index XXXXXXX..XXXXXXX 100755

--- a/configure

+++ b/configure

@@ -XXX,XX +XXX,XX @@ cross_prefix=""

audio_drv_list=""

block_drv_rw_whitelist=""

block_drv_ro_whitelist=""

+block_drv_whitelist_tools="no"

host_cc="cc"

audio_win_int=""

libs_qga=""

@@ -XXX,XX +XXX,XX @@ for opt do

;;

--block-drv-ro-whitelist=*) block_drv_ro_whitelist=$(echo "$optarg" | sed -e 's/,/ /g')

;;

+ --enable-block-drv-whitelist-in-tools) block_drv_whitelist_tools="yes"

+ ;;

+ --disable-block-drv-whitelist-in-tools) block_drv_whitelist_tools="no"

+ ;;

--enable-debug-tcg) debug_tcg="yes"

;;

--disable-debug-tcg) debug_tcg="no"

@@ -XXX,XX +XXX,XX @@ Advanced options (experts only):

--block-drv-whitelist=L Same as --block-drv-rw-whitelist=L

--block-drv-rw-whitelist=L

set block driver read-write whitelist

- (affects only QEMU, not qemu-img)

+ (by default affects only QEMU, not tools like qemu-img)

--block-drv-ro-whitelist=L

set block driver read-only whitelist

- (affects only QEMU, not qemu-img)

+ (by default affects only QEMU, not tools like qemu-img)

+ --enable-block-drv-whitelist-in-tools

+ use block whitelist also in tools instead of only QEMU

--enable-trace-backends=B Set trace backend

Available backends: $trace_backend_list

--with-trace-file=NAME Full PATH,NAME of file to store traces

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

fi

echo "CONFIG_BDRV_RW_WHITELIST=$block_drv_rw_whitelist" >> $config_host_mak

echo "CONFIG_BDRV_RO_WHITELIST=$block_drv_ro_whitelist" >> $config_host_mak

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

+ echo "CONFIG_BDRV_WHITELIST_TOOLS=y" >> $config_host_mak

+fi

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

echo "CONFIG_XFS=y" >> $config_host_mak

fi

@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,

void bdrv_init(void)

{

+#ifdef CONFIG_BDRV_WHITELIST_TOOLS

+ use_bdrv_whitelist = 1;

+#endif

module_call_init(MODULE_INIT_BLOCK);

}

diff --git a/meson.build b/meson.build

--- a/meson.build

+++ b/meson.build

@@ -XXX,XX +XXX,XX @@ summary_info += {'coroutine pool': config_host['CONFIG_COROUTINE_POOL'] == '1

if have_block

summary_info += {'Block whitelist (rw)': config_host['CONFIG_BDRV_RW_WHITELIST']}

summary_info += {'Block whitelist (ro)': config_host['CONFIG_BDRV_RO_WHITELIST']}

+ summary_info += {'Use block whitelist in tools': config_host.has_key('CONFIG_BDRV_WHITELIST_TOOLS')}

summary_info += {'VirtFS support': have_virtfs}

summary_info += {'build virtiofs daemon': have_virtiofsd}

summary_info += {'Live block migration': config_host.has_key('CONFIG_LIVE_BLOCK_MIGRATION')}

2.31.1

From: Lukas Straub <lukasstraub2@web.de>

s->active_disk is bs->file. Remove it and use local variables instead.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>

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

Message-Id: <2534f867ea9be5b666dfce19744b7d4e2b96c976.1626619393.git.lukasstraub2@web.de>

Reviewed-by: Zhang Chen <chen.zhang@intel.com>

block/replication.c | 34 +++++++++++++++++-----------------

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

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

--- a/block/replication.c

+++ b/block/replication.c

@@ -XXX,XX +XXX,XX @@ typedef enum {

typedef struct BDRVReplicationState {

ReplicationMode mode;

ReplicationStage stage;

- BdrvChild *active_disk;

BlockJob *commit_job;

BdrvChild *hidden_disk;

BdrvChild *secondary_disk;

@@ -XXX,XX +XXX,XX @@ out:

return ret;

}

-static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)

+static void secondary_do_checkpoint(BlockDriverState *bs, Error **errp)

{

+ BDRVReplicationState *s = bs->opaque;

+ BdrvChild *active_disk = bs->file;

Error *local_err = NULL;

int ret;

@@ -XXX,XX +XXX,XX @@ static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)

- if (!s->active_disk->bs->drv) {

+ if (!active_disk->bs->drv) {

error_setg(errp, "Active disk %s is ejected",

- s->active_disk->bs->node_name);

+ active_disk->bs->node_name);

- ret = bdrv_make_empty(s->active_disk, errp);

+ ret = bdrv_make_empty(active_disk, errp);

if (ret < 0) {