The following changes since commit ad88e4252f09c2956b99c90de39e95bab2e8e7af:

Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-jun-1-2019' into staging (2019-06-03 10:25:12 +0100)

for you to fetch changes up to 9593db8ccd27800ce4a17f1d5b735b9130c541a2:

iotests: Fix duplicated diff output on failure (2019-06-03 16:33:20 +0200)

- block: AioContext management, part 2

- Avoid recursive block_status call (i.e. lseek() calls) if possible

- linux-aio: Drop unused BlockAIOCB submission method

- nvme: add Get/Set Feature Timestamp support

- Fix crash on commit job start with active I/O on base node

- Fix crash in bdrv_drained_end

- Fix integer overflow in qcow2 discard

John Snow (1):

blockdev: fix missed target unref for drive-backup

Julia Suvorova (1):

block/linux-aio: Drop unused BlockAIOCB submission method

Kenneth Heitke (1):

nvme: add Get/Set Feature Timestamp support

Kevin Wolf (19):

block: Drain source node in bdrv_replace_node()

iotests: Test commit job start with concurrent I/O

test-block-iothread: Check filter node in test_propagate_mirror

nbd-server: Call blk_set_allow_aio_context_change()

block: Add Error to blk_set_aio_context()

block: Add BlockBackend.ctx

block: Add qdev_prop_drive_iothread property type

scsi-disk: Use qdev_prop_drive_iothread

block: Adjust AioContexts when attaching nodes

test-block-iothread: Test adding parent to iothread node

test-block-iothread: BlockBackend AioContext across root node change

block: Move node without parents to main AioContext

blockdev: Use bdrv_try_set_aio_context() for monitor commands

block: Remove wrong bdrv_set_aio_context() calls

virtio-scsi-test: Test attaching new overlay with iothreads

iotests: Attach new devices to node in non-default iothread

test-bdrv-drain: Use bdrv_try_set_aio_context()

block: Remove bdrv_set_aio_context()

iotests: Fix duplicated diff output on failure

Max Reitz (2):

block/io: Delay decrementing the quiesce_counter

iotests: Test cancelling a job and closing the VM

Vladimir Sementsov-Ogievskiy (4):

tests/perf: Test lseek influence on qcow2 block-status

block: avoid recursive block_status call if possible

block/qcow2-refcount: add trace-point to qcow2_process_discards

block/io: bdrv_pdiscard: support int64_t bytes parameter

docs/devel/multiple-iothreads.txt | 4 +-

block/qcow2.h | 4 +

hw/block/nvme.h | 2 +

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

include/block/block_int.h | 1 +

include/block/nvme.h | 2 +

include/block/raw-aio.h | 3 -

include/hw/block/block.h | 7 +-

include/hw/qdev-properties.h | 3 +

include/hw/scsi/scsi.h | 1 +

include/sysemu/block-backend.h | 5 +-

tests/libqtest.h | 11 +++

block.c | 79 ++++++++++++-----

block/backup.c | 3 +-

block/block-backend.c | 47 ++++++----

block/commit.c | 13 +--

block/crypto.c | 3 +-

block/io.c | 28 +++---

block/linux-aio.c | 72 +++------------

block/mirror.c | 4 +-

block/parallels.c | 3 +-

block/qcow.c | 3 +-

block/qcow2-refcount.c | 39 ++++++++-

block/qcow2.c | 17 +++-

block/qed.c | 3 +-

block/sheepdog.c | 3 +-

block/vdi.c | 3 +-

block/vhdx.c | 3 +-

block/vmdk.c | 3 +-

block/vpc.c | 3 +-

blockdev.c | 61 +++++++------

blockjob.c | 12 ++-

hmp.c | 3 +-

hw/block/dataplane/virtio-blk.c | 12 ++-

hw/block/dataplane/xen-block.c | 6 +-

hw/block/fdc.c | 2 +-

hw/block/nvme.c | 106 +++++++++++++++++++++-

hw/block/xen-block.c | 2 +-

hw/core/qdev-properties-system.c | 41 ++++++++-

hw/ide/qdev.c | 2 +-

hw/scsi/scsi-disk.c | 24 +++--

hw/scsi/virtio-scsi.c | 25 +++---

migration/block.c | 3 +-

nbd/server.c | 6 +-

qemu-img.c | 6 +-

tests/libqtest.c | 19 ++++

tests/test-bdrv-drain.c | 50 ++++++-----

tests/test-bdrv-graph-mod.c | 5 +-

tests/test-block-backend.c | 6 +-

tests/test-block-iothread.c | 104 ++++++++++++++++++----

tests/test-blockjob.c | 2 +-

tests/test-throttle.c | 6 +-

tests/virtio-scsi-test.c | 62 +++++++++++++

block/trace-events | 3 +

hw/block/trace-events | 2 +

tests/perf/block/qcow2/convert-blockstatus | 71 +++++++++++++++

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

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

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

tests/qemu-iotests/102 | 2 +-

tests/qemu-iotests/102.out | 3 +-

tests/qemu-iotests/141.out | 2 +-

tests/qemu-iotests/144.out | 2 +-

tests/qemu-iotests/240 | 21 +++++

tests/qemu-iotests/240.out | 15 +++-

tests/qemu-iotests/255 | 135 +++++++++++++++++++++++++++++

tests/qemu-iotests/255.out | 40 +++++++++

tests/qemu-iotests/check | 1 -

tests/qemu-iotests/group | 1 +

tests/qemu-iotests/iotests.py | 10 ++-

70 files changed, 1048 insertions(+), 272 deletions(-)

create mode 100755 tests/perf/block/qcow2/convert-blockstatus

create mode 100755 tests/qemu-iotests/255

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

Instead of just asserting that no requests are in flight in

bdrv_replace_node(), which is a requirement that most callers ignore, we

can just drain the source node right there. This fixes at least starting

a commit job while I/O is active on the backing chain, but probably

other callers, too.

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

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

1 file changed, 4 insertions(+), 3 deletions(-)

@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,

uint64_t perm = 0, shared = BLK_PERM_ALL;

int ret;

- assert(!atomic_read(&from->in_flight));

- assert(!atomic_read(&to->in_flight));

-

/* Make sure that @from doesn't go away until we have successfully attached

* all of its parents to @to. */

bdrv_ref(from);

+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());

+ bdrv_drained_begin(from);

/* Put all parents into @list and calculate their cumulative permissions */

QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {

assert(c->bs == from);

@@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to,

out:

g_slist_free(list);

+ bdrv_drained_end(from);

bdrv_unref(from);

2.20.1

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

tests/qemu-iotests/255 | 83 +++++++++++++++++++++++++++++++++++

tests/qemu-iotests/255.out | 16 +++++++

tests/qemu-iotests/group | 1 +

tests/qemu-iotests/iotests.py | 10 ++++-

4 files changed, 109 insertions(+), 1 deletion(-)

create mode 100755 tests/qemu-iotests/255

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

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

+++ b/tests/qemu-iotests/255

+#!/usr/bin/env python

+# Test commit job graph modifications while requests are active

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

+#

+# Creator/Owner: Kevin Wolf <kwolf@redhat.com>

+import iotests

+from iotests import imgfmt

+iotests.verify_image_format(supported_fmts=['qcow2'])

+def blockdev_create(vm, options):

+ result = vm.qmp_log('blockdev-create',

+ filters=[iotests.filter_qmp_testfiles],

+ job_id='job0', options=options)

+ if 'return' in result:

+ assert result['return'] == {}

+ vm.run_job('job0')

+ iotests.log("")

+with iotests.FilePath('t.qcow2') as disk_path, \

+ iotests.FilePath('t.qcow2.mid') as mid_path, \

+ iotests.FilePath('t.qcow2.base') as base_path, \

+ iotests.VM() as vm:

+ iotests.log("=== Create backing chain and start VM ===")

+ iotests.log("")

+ size = 128 * 1024 * 1024

+ size_str = str(size)

+ iotests.create_image(base_path, size)

+ iotests.qemu_img_log('create', '-f', iotests.imgfmt, mid_path, size_str)

+ iotests.qemu_img_log('create', '-f', iotests.imgfmt, disk_path, size_str)

+ # Create a backing chain like this:

+ # base <- [throttled: bps-read=4096] <- mid <- overlay

+ vm.add_object('throttle-group,x-bps-read=4096,id=throttle0')

+ vm.add_blockdev('file,filename=%s,node-name=base' % (base_path))

+ vm.add_blockdev('throttle,throttle-group=throttle0,file=base,node-name=throttled')

+ vm.add_blockdev('file,filename=%s,node-name=mid-file' % (mid_path))

+ vm.add_blockdev('qcow2,file=mid-file,node-name=mid,backing=throttled')

+ vm.add_drive_raw('if=none,id=overlay,driver=qcow2,file=%s,backing=mid' % (disk_path))

+ vm.launch()

+ iotests.log("=== Start background read requests ===")

+ iotests.log("")

+ def start_requests():

+ vm.hmp_qemu_io('overlay', 'aio_read 0 4k')

+ vm.hmp_qemu_io('overlay', 'aio_read 0 4k')

+ start_requests()

+ iotests.log("=== Run a commit job ===")

+ iotests.log("")

+ result = vm.qmp_log('block-commit', job_id='job0', auto_finalize=False,

+ device='overlay', top_node='mid')

+ vm.run_job('job0', auto_finalize=False, pre_finalize=start_requests,

+ auto_dismiss=True)

+ vm.shutdown()

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

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

+=== Create backing chain and start VM ===

+Formatting 'TEST_DIR/PID-t.qcow2.mid', fmt=qcow2 size=134217728 cluster_size=65536 lazy_refcounts=off refcount_bits=16

+Formatting 'TEST_DIR/PID-t.qcow2', fmt=qcow2 size=134217728 cluster_size=65536 lazy_refcounts=off refcount_bits=16

+=== Start background read requests ===

+=== Run a commit job ===

+{"execute": "block-commit", "arguments": {"auto-finalize": false, "device": "overlay", "job-id": "job0", "top-node": "mid"}}

+{"return": {}}

+{"execute": "job-finalize", "arguments": {"id": "job0"}}

+{"return": {}}

+{"data": {"id": "job0", "type": "commit"}, "event": "BLOCK_JOB_PENDING", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}

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

252 rw auto backing quick

253 rw auto quick

254 rw auto backing quick

+255 rw auto quick

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ def qemu_img_pipe(*args):

sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))

return subp.communicate()[0]

+def qemu_img_log(*args):

+ result = qemu_img_pipe(*args)

+ log(result, filters=[filter_testfiles])

+ return result

+

def img_info_log(filename, filter_path=None, imgopts=False, extra_args=[]):

args = [ 'info' ]

if imgopts:

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

return result

# Returns None on success, and an error string on failure

- def run_job(self, job, auto_finalize=True, auto_dismiss=False):

+ def run_job(self, job, auto_finalize=True, auto_dismiss=False,

+ pre_finalize=None):

error = None

while True:

for ev in self.get_qmp_events_filtered(wait=True):

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

error = j['error']

log('Job failed: %s' % (j['error']))

elif status == 'pending' and not auto_finalize:

+ if pre_finalize:

+ pre_finalize()

self.qmp_log('job-finalize', id=job)

elif status == 'concluded' and not auto_dismiss:

self.qmp_log('job-dismiss', id=job)

2.20.1

From: John Snow <jsnow@redhat.com>

If the bitmap can't be used for whatever reason, we skip putting down

the reference. Fix that.

In practice, this means that if you attempt to gracefully exit QEMU

after a backup command being rejected, bdrv_close_all will fail and

tell you some unpleasant things via assert().

Reported-by: aihua liang <aliang@redhat.com>

Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1703916

Signed-off-by: John Snow <jsnow@redhat.com>

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

blockdev.c | 13 ++++++-------

1 file changed, 6 insertions(+), 7 deletions(-)

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

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,

if (set_backing_hd) {

bdrv_set_backing_hd(target_bs, source, &local_err);

if (local_err) {

- bdrv_unref(target_bs);

- goto out;

+ goto unref;

@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,

bmap = bdrv_find_dirty_bitmap(bs, backup->bitmap);

if (!bmap) {

error_setg(errp, "Bitmap '%s' could not be found", backup->bitmap);

- bdrv_unref(target_bs);

- goto out;

+ goto unref;

2.20.1

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

Block layer may recursively check block_status in file child of qcow2,

if qcow2 driver returned DATA. There are several test cases to check

influence of lseek on block_status performance. To see real difference

run on tmpfs.

Tests originally created by Kevin, I just refactored and put them

together into one executable file with simple output.

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

tests/perf/block/qcow2/convert-blockstatus | 71 ++++++++++++++++++++++

1 file changed, 71 insertions(+)

create mode 100755 tests/perf/block/qcow2/convert-blockstatus

diff --git a/tests/perf/block/qcow2/convert-blockstatus b/tests/perf/block/qcow2/convert-blockstatus

new file mode 100755

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/perf/block/qcow2/convert-blockstatus

+#!/bin/bash

+#

+# Test lseek influence on qcow2 block-status

+#

+# Block layer may recursively check block_status in file child of qcow2, if

+# qcow2 driver returned DATA. There are several test cases to check influence

+# of lseek on block_status performance. To see real difference run on tmpfs.

+#

+# Copyright (c) 2019 Virtuozzo International GmbH. All rights reserved.

+#

+# Tests originally written by Kevin Wolf

+#

+# This program is free software; you can redistribute it and/or modify

+# it under the terms of the GNU General Public License as published by

+# the Free Software Foundation; either version 2 of the License, or

+# (at your option) any later version.

+#

+# This program is distributed in the hope that it will be useful,

+# but WITHOUT ANY WARRANTY; without even the implied warranty of

+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the

+# GNU General Public License for more details.

+#

+# You should have received a copy of the GNU General Public License

+# along with this program. If not, see <http://www.gnu.org/licenses/>.

+#

+if [ "$#" -lt 1 ]; then

+ echo "Usage: $0 SOURCE_FILE"

+ exit 1

+fi

+

+ROOT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )/../../../.." >/dev/null 2>&1 && pwd )"

+QEMU_IMG="$ROOT_DIR/qemu-img"

+QEMU_IO="$ROOT_DIR/qemu-io"

+

+size=1G

+src="$1"

+

+# test-case plain

+

+(

+$QEMU_IMG create -f qcow2 "$src" $size

+for i in $(seq 16384 -1 0); do

+ echo "write $((i * 65536)) 64k"

+done | $QEMU_IO "$src"

+) > /dev/null

+

+echo -n "plain: "

+/usr/bin/time -f %e $QEMU_IMG convert -n "$src" null-co://

+

+# test-case forward

+

+(

+$QEMU_IMG create -f qcow2 "$src" $size

+for i in $(seq 0 2 16384); do

+ echo "write $((i * 65536)) 64k"

+done | $QEMU_IO "$src"

+for i in $(seq 1 2 16384); do

+ echo "write $((i * 65536)) 64k"

+done | $QEMU_IO "$src"

+) > /dev/null

+

+echo -n "forward: "

+/usr/bin/time -f %e $QEMU_IMG convert -n "$src" null-co://

+

+# test-case prealloc

+

+$QEMU_IMG create -f qcow2 -o preallocation=metadata "$src" $size > /dev/null

+

+echo -n "prealloc: "

+/usr/bin/time -f %e $QEMU_IMG convert -n "$src" null-co://

2.20.1

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

drv_co_block_status digs bs->file for additional, more accurate search

for hole inside region, reported as DATA by bs since 5daa74a6ebc.

This accuracy is not free: assume we have qcow2 disk. Actually, qcow2

knows, where are holes and where is data. But every block_status

request calls lseek additionally. Assume a big disk, full of

data, in any iterative copying block job (or img convert) we'll call

lseek(HOLE) on every iteration, and each of these lseeks will have to

iterate through all metadata up to the end of file. It's obviously

ineffective behavior. And for many scenarios we don't need this lseek

at all.

However, lseek is needed when we have metadata-preallocated image.

So, let's detect metadata-preallocation case and don't dig qcow2's

protocol file in other cases.

The idea is to compare allocation size in POV of filesystem with

allocations size in POV of Qcow2 (by refcounts). If allocation in fs is

significantly lower, consider it as metadata-preallocation case.

102 iotest changed, as our detector can't detect shrinked file as

metadata-preallocation, which don't seem to be wrong, as with metadata

preallocation we always have valid file length.

Two other iotests have a slight change in their QMP output sequence:

Active 'block-commit' returns earlier because the job coroutine yields

earlier on a blocking operation. This operation is loading the refcount

blocks in qcow2_detect_metadata_preallocation().

Suggested-by: Denis V. Lunev <den@openvz.org>

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

block/qcow2.h | 4 ++++

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

block/io.c | 9 ++++++++-

block/qcow2-refcount.c | 32 ++++++++++++++++++++++++++++++++

block/qcow2.c | 11 +++++++++++

tests/qemu-iotests/102 | 2 +-

tests/qemu-iotests/102.out | 3 ++-

tests/qemu-iotests/141.out | 2 +-

tests/qemu-iotests/144.out | 2 +-

9 files changed, 67 insertions(+), 6 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.h

+++ b/block/qcow2.h

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

int nb_threads;

BdrvChild *data_file;

+

+ bool metadata_preallocation_checked;

+ bool metadata_preallocation;

} BDRVQcow2State;

typedef struct Qcow2COWRegion {

@@ -XXX,XX +XXX,XX @@ int qcow2_change_refcount_order(BlockDriverState *bs, int refcount_order,

void *cb_opaque, Error **errp);

int qcow2_shrink_reftable(BlockDriverState *bs);

int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);

+int qcow2_detect_metadata_preallocation(BlockDriverState *bs);

/* qcow2-cluster.c functions */

int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,

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 {

* BDRV_BLOCK_EOF: the returned pnum covers through end of file for this

* layer, set by block layer

*

- * Internal flag:

+ * Internal flags:

* BDRV_BLOCK_RAW: for use by passthrough drivers, such as raw, to request

* that the block layer recompute the answer from the returned

* BDS; must be accompanied by just BDRV_BLOCK_OFFSET_VALID.

+ * BDRV_BLOCK_RECURSE: request that the block layer will recursively search for

+ * zeroes in file child of current block node inside

+ * returned region. Only valid together with both

+ * BDRV_BLOCK_DATA and BDRV_BLOCK_OFFSET_VALID. Should not

+ * appear with BDRV_BLOCK_ZERO.

*

* If BDRV_BLOCK_OFFSET_VALID is set, the map parameter represents the

* host offset within the returned BDS that is allocated for the

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

#define BDRV_BLOCK_RAW 0x08

#define BDRV_BLOCK_ALLOCATED 0x10

#define BDRV_BLOCK_EOF 0x20

+#define BDRV_BLOCK_RECURSE 0x40

#define BDRV_BLOCK_OFFSET_MASK BDRV_SECTOR_MASK

typedef QSIMPLEQ_HEAD(BlockReopenQueue, BlockReopenQueueEntry) BlockReopenQueue;

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

index XXXXXXX..XXXXXXX 100644

--- a/block/io.c

+++ b/block/io.c

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

*/

assert(*pnum && QEMU_IS_ALIGNED(*pnum, align) &&

align > offset - aligned_offset);

+ if (ret & BDRV_BLOCK_RECURSE) {

+ assert(ret & BDRV_BLOCK_DATA);

+ assert(ret & BDRV_BLOCK_OFFSET_VALID);

+ assert(!(ret & BDRV_BLOCK_ZERO));

+ }

+

*pnum -= offset - aligned_offset;

if (*pnum > bytes) {

*pnum = bytes;

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

}

}

- if (want_zero && local_file && local_file != bs &&

+ if (want_zero && ret & BDRV_BLOCK_RECURSE &&

+ local_file && local_file != bs &&

(ret & BDRV_BLOCK_DATA) && !(ret & BDRV_BLOCK_ZERO) &&

(ret & BDRV_BLOCK_OFFSET_VALID)) {

int64_t file_pnum;

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

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2-refcount.c

+++ b/block/qcow2-refcount.c

@@ -XXX,XX +XXX,XX @@ int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)

"There are no references in the refcount table.");

return -EIO;

}

+

+int qcow2_detect_metadata_preallocation(BlockDriverState *bs)

+{

+ BDRVQcow2State *s = bs->opaque;

+ int64_t i, end_cluster, cluster_count = 0, threshold;

+ int64_t file_length, real_allocation, real_clusters;

+

+ file_length = bdrv_getlength(bs->file->bs);

+ if (file_length < 0) {

+ return file_length;

+ }

+

+ real_allocation = bdrv_get_allocated_file_size(bs->file->bs);

+ if (real_allocation < 0) {

+ return real_allocation;

+ }

+

+ real_clusters = real_allocation / s->cluster_size;

+ threshold = MAX(real_clusters * 10 / 9, real_clusters + 2);

+

+ end_cluster = size_to_clusters(s, file_length);

+ for (i = 0; i < end_cluster && cluster_count < threshold; i++) {

+ uint64_t refcount;

+ int ret = qcow2_get_refcount(bs, i, &refcount);

+ if (ret < 0) {

+ return ret;

+ }

+ cluster_count += !!refcount;

+ }

+

+ return cluster_count >= threshold;

+}

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

unsigned int bytes;

int status = 0;

+ if (!s->metadata_preallocation_checked) {

+ ret = qcow2_detect_metadata_preallocation(bs);

+ s->metadata_preallocation = (ret == 1);

+ s->metadata_preallocation_checked = true;

+ }

+

bytes = MIN(INT_MAX, count);

qemu_co_mutex_lock(&s->lock);

ret = qcow2_get_cluster_offset(bs, offset, &bytes, &cluster_offset);

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

} else if (ret != QCOW2_CLUSTER_UNALLOCATED) {

status |= BDRV_BLOCK_DATA;

+ if (s->metadata_preallocation && (status & BDRV_BLOCK_DATA) &&

+ (status & BDRV_BLOCK_OFFSET_VALID))

+ {

+ status |= BDRV_BLOCK_RECURSE;

+ }

return status;

}

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/102

+++ b/tests/qemu-iotests/102

@@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'write 0 64k' "$TEST_IMG" | _filter_qemu_io

$QEMU_IMG resize -f raw --shrink "$TEST_IMG" $((5 * 64 * 1024))

$QEMU_IO -c map "$TEST_IMG"

-$QEMU_IMG map "$TEST_IMG"

+$QEMU_IMG map "$TEST_IMG" | _filter_qemu_img_map

echo

echo '=== Testing map on an image file truncated outside of qemu ==='

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 0

64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

Image resized.

64 KiB (0x10000) bytes allocated at offset 0 bytes (0x0)

-Offset Length Mapped to File

+Offset Length File

+0 0x10000 TEST_DIR/t.IMGFMT

=== Testing map on an image file truncated outside of qemu ===

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.

{"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": "ready", "id": "job0"}}

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

-{"return": {}}

{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: block device is in use by block job: commit"}}

{"return": {}}

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

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/tmp.qcow2', fmt=qcow2 size=536870912 backing_file=TEST_DIR/

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

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

+{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "ready", "id": "virtio0"}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_READY", "data": {"device": "virtio0", "len": 0, "offset": 0, "speed": 0, "type": "commit"}}

{"return": {}}

-{"return": {}}

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

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

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

2.20.1

When ending a drained section, bdrv_do_drained_end() currently first

decrements the quiesce_counter, and only then actually ends the drain.

The bdrv_drain_invoke(bs, false) call may cause graph changes. Say the

graph change involves replacing an existing BB's ("blk") BDS

(blk_bs(blk)) by @bs. Let us introducing the following values:

- bs_oqc = old_quiesce_counter

(so bs->quiesce_counter == bs_oqc - 1)

- obs_qc = blk_bs(blk)->quiesce_counter (before bdrv_drain_invoke())

Let us assume there is no blk_pread_unthrottled() involved, so

blk->quiesce_counter == obs_qc (before bdrv_drain_invoke()).

Now replacing blk_bs(blk) by @bs will reduce blk->quiesce_counter by

obs_qc (making it 0) and increase it by bs_oqc-1 (making it bs_oqc-1).

bdrv_drain_invoke() returns and we invoke bdrv_parent_drained_end().

This will decrement blk->quiesce_counter by one, so it would be -1 --

were there not an assertion against that in blk_root_drained_end().

We therefore have to keep the quiesce_counter up at least until

bdrv_drain_invoke() returns, so that bdrv_parent_drained_end() does the

right thing for the parents @bs got during bdrv_drain_invoke().

But let us delay it even further, namely until bdrv_parent_drained_end()

returns, because then it mirrors bdrv_do_drained_begin(): There, we

first increment the quiesce_counter, then begin draining the parents,

and then call bdrv_drain_invoke(). It makes sense to let

bdrv_do_drained_end() unravel this exactly in reverse.

block/io.c | 3 ++-

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

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

--- a/block/io.c

+++ b/block/io.c

@@ -XXX,XX +XXX,XX @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,

return;

}

assert(bs->quiesce_counter > 0);

- old_quiesce_counter = atomic_fetch_dec(&bs->quiesce_counter);

/* Re-enable things in child-to-parent order */

bdrv_drain_invoke(bs, false);

bdrv_parent_drained_end(bs, parent, ignore_bds_parents);

+

+ old_quiesce_counter = atomic_fetch_dec(&bs->quiesce_counter);

if (old_quiesce_counter == 1) {

aio_enable_external(bdrv_get_aio_context(bs));

}

2.20.1

This patch adds a test where we cancel a throttled mirror job and

immediately close the VM before it can be cancelled. Doing so will

invoke bdrv_drain_all() while the mirror job tries to drain the

throttled node. When bdrv_drain_all_end() tries to lift its drain on

the throttle node, the job will exit and replace the current root node

of the BB drive0 (which is the job's filter node) by the throttle node.

Before the previous patch, this replacement did not increase drive0's

quiesce_counter by a sufficient amount, so when

bdrv_parent_drained_end() (invoked by bdrv_do_drained_end(), invoked by

bdrv_drain_all_end()) tried to end the drain on all of the throttle

node's parents, it decreased drive0's quiesce_counter below 0 -- which

fails an assertion.

tests/qemu-iotests/255 | 52 ++++++++++++++++++++++++++++++++++++++

tests/qemu-iotests/255.out | 24 ++++++++++++++++++

2 files changed, 76 insertions(+)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/255

+++ b/tests/qemu-iotests/255

@@ -XXX,XX +XXX,XX @@ def blockdev_create(vm, options):

vm.run_job('job0')

iotests.log("")

+iotests.log('Finishing a commit job with background reads')

+iotests.log('============================================')

+iotests.log('')