Philippe Mathieu-Daudé (3):

block/block-copy: Fix uninitialized variable in block_copy_task_entry

block/block-copy: Simplify block_copy_do_copy()

hw/ide/ahci: Log lost IRQs

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

include/block/block_int.h | 57 ++--

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

block.c | 601 ++++++++++++++++++++++++---------------

block/backup-top.c | 11 +-

block/blkdebug.c | 10 +-

block/blklogwrites.c | 16 +-

block/blkreplay.c | 8 +-

block/blkverify.c | 10 +-

block/block-backend.c | 30 +-

block/block-copy.c | 14 +-

block/bochs.c | 7 +-

block/cloop.c | 7 +-

block/commit.c | 20 +-

block/copy-on-read.c | 7 +-

block/crypto.c | 8 +-

block/dmg.c | 7 +-

block/filter-compress.c | 7 +-

block/io.c | 22 +-

block/mirror.c | 25 +-

block/parallels.c | 7 +-

block/qcow.c | 7 +-

block/qcow2.c | 20 +-

block/qed.c | 7 +-

block/quorum.c | 8 +-

block/raw-format.c | 128 +++++----

block/replication.c | 23 +-

block/throttle.c | 7 +-

block/vdi.c | 7 +-

block/vhdx.c | 7 +-

block/vmdk.c | 23 +-

block/vpc.c | 7 +-

block/vvfat.c | 17 +-

blockjob.c | 7 +-

hw/ide/ahci.c | 1 +

qemu-img.c | 19 +-

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

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

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

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

tests/qemu-iotests/030 | 6 +-

tests/qemu-iotests/041 | 45 +++

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

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

tests/qemu-iotests/109 | 10 +-

tests/qemu-iotests/109.out | 74 ++---

tests/qemu-iotests/229 | 15 +-

tests/qemu-iotests/229.out | 6 +-

tests/qemu-iotests/297 | 44 +++

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

tests/qemu-iotests/common.filter | 5 +

tests/qemu-iotests/group | 1 +

52 files changed, 995 insertions(+), 573 deletions(-)

create mode 100755 tests/qemu-iotests/297

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

229 relies on the mirror running into an I/O error when the target is

smaller than the source. After changing mirror to catch this condition

while starting the job, this test case won't get a job that is paused

for an I/O error any more. Use blkdebug instead to inject an error.

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

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

Message-Id: <20200511135825.219437-3-kwolf@redhat.com>

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

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

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

---

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

tests/qemu-iotests/229.out | 6 +++---

2 files changed, 14 insertions(+), 7 deletions(-)

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

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/229

+++ b/tests/qemu-iotests/229

@@ -XXX,XX +XXX,XX @@ _cleanup()

_cleanup_test_img

_rm_test_img "$TEST_IMG"

_rm_test_img "$DEST_IMG"

+ rm -f "$TEST_DIR/blkdebug.conf"

}

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

@@ -XXX,XX +XXX,XX @@ _supported_os Linux

DEST_IMG="$TEST_DIR/d.$IMGFMT"

TEST_IMG="$TEST_DIR/b.$IMGFMT"

+BLKDEBUG_CONF="$TEST_DIR/blkdebug.conf"

_make_test_img 2M

-

-# destination for mirror will be too small, causing error

-TEST_IMG=$DEST_IMG _make_test_img 1M

+TEST_IMG=$DEST_IMG _make_test_img 2M

$QEMU_IO -c 'write 0 2M' "$TEST_IMG" | _filter_qemu_io

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

echo '=== Starting drive-mirror, causing error & stop ==='

echo

+cat > "$BLKDEBUG_CONF" <<EOF

+[inject-error]

+event = "write_aio"

+errno = "5"

+once = "on"

+EOF

+

_send_qemu_cmd $QEMU_HANDLE \

"{'execute': 'drive-mirror',

'arguments': {'device': 'testdisk',

'format': '$IMGFMT',

- 'target': '$DEST_IMG',

+ 'target': 'blkdebug:$BLKDEBUG_CONF:$DEST_IMG',

'sync': 'full',

'mode': 'existing',

'on-source-error': 'stop',

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

index XXXXXXX..XXXXXXX 100644

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

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

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

QA output created by 229

Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=2097152

-Formatting 'TEST_DIR/d.IMGFMT', fmt=IMGFMT size=1048576

+Formatting 'TEST_DIR/d.IMGFMT', fmt=IMGFMT size=2097152

wrote 2097152/2097152 bytes at offset 0

2 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)

{'execute': 'qmp_capabilities'}

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

=== Starting drive-mirror, causing error & stop ===

-{'execute': 'drive-mirror', 'arguments': {'device': 'testdisk', 'format': 'IMGFMT', 'target': 'TEST_DIR/d.IMGFMT', 'sync': 'full', 'mode': 'existing', 'on-source-error': 'stop', 'on-target-error': 'stop' }}

+{'execute': 'drive-mirror', 'arguments': {'device': 'testdisk', 'format': 'IMGFMT', 'target': 'blkdebug:TEST_DIR/blkdebug.conf:TEST_DIR/d.IMGFMT', 'sync': 'full', 'mode': 'existing', 'on-source-error': 'stop', 'on-target-error': 'stop' }}

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

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

{"return": {}}

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

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

{"return": {}}

{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "aborting", "id": "testdisk"}}

-{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152, "offset": 1048576, "speed": 0, "type": "mirror"}}

+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_JOB_CANCELLED", "data": {"device": "testdisk", "len": 2097152, "offset": 2097152, "speed": 0, "type": "mirror"}}

*** done

--

2.25.4

This tests that the mirror job catches situations where the target node

has a different size than the source node. It must also forbid resize

operations when the job is already running.

From: Lukas Straub <lukasstraub2@web.de>

If qemu in colo secondary mode is stopped, it crashes because

s->backup_job is canceled twice: First with job_cancel_sync_all()

in qemu_cleanup() and then in replication_stop().

Fix this by assigning NULL to s->backup_job when the job completes

so replication_stop() and replication_do_checkpoint() won't touch

the job.

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

Message-Id: <20200511090801.7ed5d8f3@luklap>

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

---

block/replication.c | 2 ++

1 file changed, 2 insertions(+)

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

index XXXXXXX..XXXXXXX 100644

--- a/block/replication.c

+++ b/block/replication.c

@@ -XXX,XX +XXX,XX @@ static void backup_job_cleanup(BlockDriverState *bs)

BDRVReplicationState *s = bs->opaque;

BlockDriverState *top_bs;

+ s->backup_job = NULL;

+

top_bs = bdrv_lookup_bs(s->top_id, s->top_id, NULL);

if (!top_bs) {

return;

--

2.25.4

We need to fix only a few places so that iotests.py can pass

mypy --disallow-incomplete-defs, which seems to be a desirable option to

have enabled in the long run.

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

Message-Id: <20200511163529.349329-2-kwolf@redhat.com>

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

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

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

---

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

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

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 _verify_cache_mode(supported_cache_modes: Sequence[str] = ()) -> None:

if supported_cache_modes and (cachemode not in supported_cache_modes):

notrun('not suitable for this cache mode: %s' % cachemode)

-def _verify_aio_mode(supported_aio_modes: Sequence[str] = ()):

+def _verify_aio_mode(supported_aio_modes: Sequence[str] = ()) -> None:

if supported_aio_modes and (aiomode not in supported_aio_modes):

notrun('not suitable for this aio mode: %s' % aiomode)

@@ -XXX,XX +XXX,XX @@ def skip_if_unsupported(required_formats=(), read_only=False):

'''Skip Test Decorator

Runs the test if all the required formats are whitelisted'''

def skip_test_decorator(func):

- def func_wrapper(test_case: QMPTestCase, *args, **kwargs):

+ def func_wrapper(test_case: QMPTestCase, *args: List[Any],

+ **kwargs: Dict[str, Any]) -> None:

if callable(required_formats):

fmts = required_formats(test_case)

else:

@@ -XXX,XX +XXX,XX @@ def skip_if_unsupported(required_formats=(), read_only=False):

if usf_list:

msg = f'{test_case}: formats {usf_list} are not whitelisted'

test_case.case_skip(msg)

- return None

else:

- return func(test_case, *args, **kwargs)

+ func(test_case, *args, **kwargs)

return func_wrapper

return skip_test_decorator

--

2.25.4

We made sure that iotests.py passes pylint. It would be a shame if we

allowed new patches in that break this again, so let's just add a

meta-test case that runs pylint on it.

While we don't pass mypy --strict yet, we can already run it with a few

options that would be part of --strict to make sure that we won't

regress on these aspects at least until we can enable the full thing.

From: Max Reitz <mreitz@redhat.com>

Right now, all users of bdrv_make_empty() call the BlockDriver method

directly. That is not only bad style, it is also wrong, unless the

caller has a BdrvChild with a WRITE or WRITE_UNCHANGED permission.

(WRITE_UNCHANGED suffices, because callers generally use this function

to clear a node with a backing file after a commit operation.)

Introduce bdrv_make_empty() that verifies that it does.

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

Message-Id: <20200429141126.85159-2-mreitz@redhat.com>

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

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

---

include/block/block.h | 1 +

block.c | 23 +++++++++++++++++++++++

2 files changed, 24 insertions(+)

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 @@ BlockMeasureInfo *bdrv_measure(BlockDriver *drv, QemuOpts *opts,

void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);

void bdrv_refresh_limits(BlockDriverState *bs, Error **errp);

int bdrv_commit(BlockDriverState *bs);

+int bdrv_make_empty(BdrvChild *c, Error **errp);

int bdrv_change_backing_file(BlockDriverState *bs,

const char *backing_file, const char *backing_fmt);

void bdrv_register(BlockDriver *bdrv);

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

index XXXXXXX..XXXXXXX 100644

--- a/block.c

+++ b/block.c

@@ -XXX,XX +XXX,XX @@ void bdrv_del_child(BlockDriverState *parent_bs, BdrvChild *child, Error **errp)

parent_bs->drv->bdrv_del_child(parent_bs, child, errp);

}

+

+int bdrv_make_empty(BdrvChild *c, Error **errp)

+{

+ BlockDriver *drv = c->bs->drv;

+ int ret;

+

+ assert(c->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED));

+

+ if (!drv->bdrv_make_empty) {

+ error_setg(errp, "%s does not support emptying nodes",

+ drv->format_name);

+ return -ENOTSUP;

+ }

+

+ ret = drv->bdrv_make_empty(c->bs);

+ if (ret < 0) {

+ error_setg_errno(errp, -ret, "Failed to empty %s",

+ c->bs->filename);

+ return ret;

+ }

+

+ return 0;

+}

--

2.25.4

From: Max Reitz <mreitz@redhat.com>

Two callers of BlockDriver.bdrv_make_empty() remain that should not call

this method directly. Both do not have access to a BdrvChild, but they

can use a BlockBackend, so we add this function that lets them use it.

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

Message-Id: <20200429141126.85159-4-mreitz@redhat.com>

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

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

---

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

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

2 files changed, 12 insertions(+)

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

index XXXXXXX..XXXXXXX 100644

--- a/include/sysemu/block-backend.h

+++ b/include/sysemu/block-backend.h

@@ -XXX,XX +XXX,XX @@ int coroutine_fn blk_co_copy_range(BlockBackend *blk_in, int64_t off_in,

const BdrvChild *blk_root(BlockBackend *blk);

+int blk_make_empty(BlockBackend *blk, Error **errp);

+

#endif

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

index XXXXXXX..XXXXXXX 100644

--- a/block/block-backend.c

+++ b/block/block-backend.c

@@ -XXX,XX +XXX,XX @@ const BdrvChild *blk_root(BlockBackend *blk)

{

return blk->root;

}

+

+int blk_make_empty(BlockBackend *blk, Error **errp)

+{

+ if (!blk_is_available(blk)) {

+ error_setg(errp, "No medium inserted");

+ return -ENOMEDIUM;

+ }

+

+ return bdrv_make_empty(blk->root, errp);

+}

--

2.25.4

After this commit, bdrv_make_empty() is the only remaining caller of

BlockDriver.bdrv_make_empty().

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

Message-Id: <20200429141126.85159-5-mreitz@redhat.com>

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

[kwolf: Fixed up reference output for 098]