1 | The following changes since commit b55a69fe5f0a504dac6359bb7e99a72b130c3661: | 1 | The following changes since commit cf7ca7d5b9faca13f1f8e3ea92cfb2f741eb0c0e: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/juanquintela/tags/migration/20170607' into staging (2017-06-07 15:06:42 +0100) | 3 | Merge remote-tracking branch 'remotes/stefanha-gitlab/tags/tracing-pull-request' into staging (2021-02-01 16:28:00 +0000) |
4 | 4 | ||
5 | are available in the git repository at: | 5 | are available in the Git repository at: |
6 | |||
7 | 6 | ||
8 | git://repo.or.cz/qemu/kevin.git tags/for-upstream | 7 | git://repo.or.cz/qemu/kevin.git tags/for-upstream |
9 | 8 | ||
10 | for you to fetch changes up to 2cb8f869e43406ce829ebe3b16fecc78d367dc7f: | 9 | for you to fetch changes up to 26513a01741f51650f5dd716681995359794ba6f: |
11 | 10 | ||
12 | block: fix external snapshot abort permission error (2017-06-07 18:51:51 +0200) | 11 | block: Fix VM size column width in bdrv_snapshot_dump() (2021-02-02 17:23:55 +0100) |
13 | 12 | ||
14 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
15 | Block layer patches | 14 | Block layer patches: |
15 | |||
16 | - Fix double processing of nodes in bdrv_set_aio_context() | ||
17 | - Fix potential hang in block export shutdown | ||
18 | - block/nvme: Minor tracing improvements | ||
19 | - iotests: Some more fixups for the 'check' rewrite | ||
20 | - MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs | ||
16 | 21 | ||
17 | ---------------------------------------------------------------- | 22 | ---------------------------------------------------------------- |
18 | Jeff Cody (1): | 23 | Kevin Wolf (3): |
19 | block: fix external snapshot abort permission error | 24 | iotests: Revert emulator selection to old behaviour |
25 | iotests: Fix -makecheck output | ||
26 | block: Fix VM size column width in bdrv_snapshot_dump() | ||
20 | 27 | ||
21 | Kevin Wolf (6): | 28 | Philippe Mathieu-Daudé (2): |
22 | block: Fix anonymous BBs in blk_root_inactivate() | 29 | block/nvme: Properly display doorbell stride length in trace event |
23 | migration: Inactivate images after .save_live_complete_precopy() | 30 | block/nvme: Trace NVMe spec version supported by the controller |
24 | migration/block: Clean up BBs in block_save_complete() | ||
25 | qemu-iotests: Block migration test | ||
26 | commit: Fix use after free in completion | ||
27 | qemu-iotests: Test automatic commit job cancel on hot unplug | ||
28 | 31 | ||
29 | Peter Maydell (1): | 32 | Sergio Lopez (2): |
30 | block/qcow.c: Fix memory leak in qcow_create() | 33 | block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() |
34 | block: move blk_exp_close_all() to qemu_cleanup() | ||
31 | 35 | ||
32 | block/block-backend.c | 2 +- | 36 | Vladimir Sementsov-Ogievskiy (3): |
33 | block/commit.c | 7 +++ | 37 | MAINTAINERS: Add Vladimir as co-maintainer for Block Jobs |
34 | block/qcow.c | 1 + | 38 | iotests/297: pylint: ignore too many statements |
35 | blockdev.c | 4 ++ | 39 | iotests: check: return 1 on failure |
36 | migration/block.c | 22 +++++-- | ||
37 | migration/migration.c | 12 ++-- | ||
38 | tests/qemu-iotests/040 | 35 +++++++++++- | ||
39 | tests/qemu-iotests/040.out | 4 +- | ||
40 | tests/qemu-iotests/183 | 140 +++++++++++++++++++++++++++++++++++++++++++++ | ||
41 | tests/qemu-iotests/183.out | 46 +++++++++++++++ | ||
42 | tests/qemu-iotests/group | 1 + | ||
43 | 11 files changed, 259 insertions(+), 15 deletions(-) | ||
44 | create mode 100755 tests/qemu-iotests/183 | ||
45 | create mode 100644 tests/qemu-iotests/183.out | ||
46 | 40 | ||
41 | block.c | 35 +++++++++++++++++++++++++++-------- | ||
42 | block/nvme.c | 8 +++++++- | ||
43 | block/qapi.c | 4 ++-- | ||
44 | qemu-nbd.c | 1 + | ||
45 | softmmu/runstate.c | 9 +++++++++ | ||
46 | storage-daemon/qemu-storage-daemon.c | 1 + | ||
47 | tests/qemu-iotests/testenv.py | 2 +- | ||
48 | tests/qemu-iotests/testrunner.py | 10 +++++++--- | ||
49 | MAINTAINERS | 10 ++++++++++ | ||
50 | block/trace-events | 1 + | ||
51 | tests/qemu-iotests/check | 5 ++++- | ||
52 | tests/qemu-iotests/pylintrc | 2 ++ | ||
53 | 12 files changed, 72 insertions(+), 16 deletions(-) | ||
54 | |||
55 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
1 | 2 | ||
3 | I'm developing Qemu backup for several years, and finally new backup | ||
4 | architecture, including block-copy generic engine and backup-top filter | ||
5 | landed upstream, great thanks to reviewers and especially to | ||
6 | Max Reitz! | ||
7 | |||
8 | I also have plans of moving other block-jobs onto block-copy, so that | ||
9 | we finally have one generic block copying path, fast and well-formed. | ||
10 | |||
11 | So, now I suggest to bring all parts of backup architecture into | ||
12 | "Block Jobs" subsystem (actually, aio_task is shared with qcow2 and | ||
13 | qemu-co-shared-resource can be reused somewhere else, but I'd keep an | ||
14 | eye on them in context of block-jobs) and add myself as co-maintainer. | ||
15 | |||
16 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
17 | Message-Id: <20210128144144.27617-1-vsementsov@virtuozzo.com> | ||
18 | Reviewed-by: Markus Armbruster <armbru@redhat.com> | ||
19 | Reviewed-by: John Snow <jsnow@redhat.com> | ||
20 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
21 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
22 | --- | ||
23 | MAINTAINERS | 10 ++++++++++ | ||
24 | 1 file changed, 10 insertions(+) | ||
25 | |||
26 | diff --git a/MAINTAINERS b/MAINTAINERS | ||
27 | index XXXXXXX..XXXXXXX 100644 | ||
28 | --- a/MAINTAINERS | ||
29 | +++ b/MAINTAINERS | ||
30 | @@ -XXX,XX +XXX,XX @@ F: scsi/* | ||
31 | |||
32 | Block Jobs | ||
33 | M: John Snow <jsnow@redhat.com> | ||
34 | +M: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
35 | L: qemu-block@nongnu.org | ||
36 | S: Supported | ||
37 | F: blockjob.c | ||
38 | @@ -XXX,XX +XXX,XX @@ F: block/commit.c | ||
39 | F: block/stream.c | ||
40 | F: block/mirror.c | ||
41 | F: qapi/job.json | ||
42 | +F: block/block-copy.c | ||
43 | +F: include/block/block-copy.c | ||
44 | +F: block/backup-top.h | ||
45 | +F: block/backup-top.c | ||
46 | +F: include/block/aio_task.h | ||
47 | +F: block/aio_task.c | ||
48 | +F: util/qemu-co-shared-resource.c | ||
49 | +F: include/qemu/co-shared-resource.h | ||
50 | T: git https://gitlab.com/jsnow/qemu.git jobs | ||
51 | +T: git https://src.openvz.org/scm/~vsementsov/qemu.git jobs | ||
52 | |||
53 | Block QAPI, monitor, command line | ||
54 | M: Markus Armbruster <armbru@redhat.com> | ||
55 | -- | ||
56 | 2.29.2 | ||
57 | |||
58 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Sergio Lopez <slp@redhat.com> | ||
1 | 2 | ||
3 | Some graphs may contain an indirect reference to the first BDS in the | ||
4 | chain that can be reached while walking it bottom->up from one its | ||
5 | children. | ||
6 | |||
7 | Doubling-processing of a BDS is especially problematic for the | ||
8 | aio_notifiers, as they might attempt to work on both the old and the | ||
9 | new AIO contexts. | ||
10 | |||
11 | To avoid this problem, add every child and parent to the ignore list | ||
12 | before actually processing them. | ||
13 | |||
14 | Suggested-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | Signed-off-by: Sergio Lopez <slp@redhat.com> | ||
16 | Message-Id: <20210201125032.44713-2-slp@redhat.com> | ||
17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
18 | --- | ||
19 | block.c | 34 +++++++++++++++++++++++++++------- | ||
20 | 1 file changed, 27 insertions(+), 7 deletions(-) | ||
21 | |||
22 | diff --git a/block.c b/block.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/block.c | ||
25 | +++ b/block.c | ||
26 | @@ -XXX,XX +XXX,XX @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs, | ||
27 | AioContext *new_context, GSList **ignore) | ||
28 | { | ||
29 | AioContext *old_context = bdrv_get_aio_context(bs); | ||
30 | - BdrvChild *child; | ||
31 | + GSList *children_to_process = NULL; | ||
32 | + GSList *parents_to_process = NULL; | ||
33 | + GSList *entry; | ||
34 | + BdrvChild *child, *parent; | ||
35 | |||
36 | g_assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | ||
37 | |||
38 | @@ -XXX,XX +XXX,XX @@ void bdrv_set_aio_context_ignore(BlockDriverState *bs, | ||
39 | continue; | ||
40 | } | ||
41 | *ignore = g_slist_prepend(*ignore, child); | ||
42 | - bdrv_set_aio_context_ignore(child->bs, new_context, ignore); | ||
43 | + children_to_process = g_slist_prepend(children_to_process, child); | ||
44 | } | ||
45 | - QLIST_FOREACH(child, &bs->parents, next_parent) { | ||
46 | - if (g_slist_find(*ignore, child)) { | ||
47 | + | ||
48 | + QLIST_FOREACH(parent, &bs->parents, next_parent) { | ||
49 | + if (g_slist_find(*ignore, parent)) { | ||
50 | continue; | ||
51 | } | ||
52 | - assert(child->klass->set_aio_ctx); | ||
53 | - *ignore = g_slist_prepend(*ignore, child); | ||
54 | - child->klass->set_aio_ctx(child, new_context, ignore); | ||
55 | + *ignore = g_slist_prepend(*ignore, parent); | ||
56 | + parents_to_process = g_slist_prepend(parents_to_process, parent); | ||
57 | + } | ||
58 | + | ||
59 | + for (entry = children_to_process; | ||
60 | + entry != NULL; | ||
61 | + entry = g_slist_next(entry)) { | ||
62 | + child = entry->data; | ||
63 | + bdrv_set_aio_context_ignore(child->bs, new_context, ignore); | ||
64 | + } | ||
65 | + g_slist_free(children_to_process); | ||
66 | + | ||
67 | + for (entry = parents_to_process; | ||
68 | + entry != NULL; | ||
69 | + entry = g_slist_next(entry)) { | ||
70 | + parent = entry->data; | ||
71 | + assert(parent->klass->set_aio_ctx); | ||
72 | + parent->klass->set_aio_ctx(parent, new_context, ignore); | ||
73 | } | ||
74 | + g_slist_free(parents_to_process); | ||
75 | |||
76 | bdrv_detach_aio_context(bs); | ||
77 | |||
78 | -- | ||
79 | 2.29.2 | ||
80 | |||
81 | diff view generated by jsdifflib |
1 | We need to release any block migrations BlockBackends on the source | 1 | From: Sergio Lopez <slp@redhat.com> |
---|---|---|---|
2 | before successfully completing the migration because otherwise | ||
3 | inactivating the images will fail (inactivation only tolerates device | ||
4 | BBs). | ||
5 | 2 | ||
3 | Move blk_exp_close_all() from bdrv_close() to qemu_cleanup(), before | ||
4 | bdrv_drain_all_begin(). | ||
5 | |||
6 | Export drivers may have coroutines yielding at some point in the block | ||
7 | layer, so we need to shut them down before draining the block layer, | ||
8 | as otherwise they may get stuck blk_wait_while_drained(). | ||
9 | |||
10 | RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1900505 | ||
11 | Signed-off-by: Sergio Lopez <slp@redhat.com> | ||
12 | Message-Id: <20210201125032.44713-3-slp@redhat.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
7 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
10 | --- | 14 | --- |
11 | migration/block.c | 22 +++++++++++++++++----- | 15 | block.c | 1 - |
12 | 1 file changed, 17 insertions(+), 5 deletions(-) | 16 | qemu-nbd.c | 1 + |
17 | softmmu/runstate.c | 9 +++++++++ | ||
18 | storage-daemon/qemu-storage-daemon.c | 1 + | ||
19 | 4 files changed, 11 insertions(+), 1 deletion(-) | ||
13 | 20 | ||
14 | diff --git a/migration/block.c b/migration/block.c | 21 | diff --git a/block.c b/block.c |
15 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/migration/block.c | 23 | --- a/block.c |
17 | +++ b/migration/block.c | 24 | +++ b/block.c |
18 | @@ -XXX,XX +XXX,XX @@ static int64_t get_remaining_dirty(void) | 25 | @@ -XXX,XX +XXX,XX @@ static void bdrv_close(BlockDriverState *bs) |
19 | return dirty << BDRV_SECTOR_BITS; | 26 | void bdrv_close_all(void) |
27 | { | ||
28 | assert(job_next(NULL) == NULL); | ||
29 | - blk_exp_close_all(); | ||
30 | |||
31 | /* Drop references from requests still in flight, such as canceled block | ||
32 | * jobs whose AIO context has not been polled yet */ | ||
33 | diff --git a/qemu-nbd.c b/qemu-nbd.c | ||
34 | index XXXXXXX..XXXXXXX 100644 | ||
35 | --- a/qemu-nbd.c | ||
36 | +++ b/qemu-nbd.c | ||
37 | @@ -XXX,XX +XXX,XX @@ static const char *socket_activation_validate_opts(const char *device, | ||
38 | static void qemu_nbd_shutdown(void) | ||
39 | { | ||
40 | job_cancel_sync_all(); | ||
41 | + blk_exp_close_all(); | ||
42 | bdrv_close_all(); | ||
20 | } | 43 | } |
21 | 44 | ||
22 | -/* Called with iothread lock taken. */ | 45 | diff --git a/softmmu/runstate.c b/softmmu/runstate.c |
23 | 46 | index XXXXXXX..XXXXXXX 100644 | |
24 | -static void block_migration_cleanup(void *opaque) | 47 | --- a/softmmu/runstate.c |
48 | +++ b/softmmu/runstate.c | ||
49 | @@ -XXX,XX +XXX,XX @@ | ||
50 | #include "qemu/osdep.h" | ||
51 | #include "audio/audio.h" | ||
52 | #include "block/block.h" | ||
53 | +#include "block/export.h" | ||
54 | #include "chardev/char.h" | ||
55 | #include "crypto/cipher.h" | ||
56 | #include "crypto/init.h" | ||
57 | @@ -XXX,XX +XXX,XX @@ void qemu_cleanup(void) | ||
58 | */ | ||
59 | migration_shutdown(); | ||
60 | |||
61 | + /* | ||
62 | + * Close the exports before draining the block layer. The export | ||
63 | + * drivers may have coroutines yielding on it, so we need to clean | ||
64 | + * them up before the drain, as otherwise they may be get stuck in | ||
65 | + * blk_wait_while_drained(). | ||
66 | + */ | ||
67 | + blk_exp_close_all(); | ||
25 | + | 68 | + |
26 | +/* Called with iothread lock taken. */ | 69 | /* |
27 | +static void block_migration_cleanup_bmds(void) | 70 | * We must cancel all block jobs while the block layer is drained, |
28 | { | 71 | * or cancelling will be affected by throttling and thus may block |
29 | BlkMigDevState *bmds; | 72 | diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c |
30 | - BlkMigBlock *blk; | 73 | index XXXXXXX..XXXXXXX 100644 |
31 | AioContext *ctx; | 74 | --- a/storage-daemon/qemu-storage-daemon.c |
32 | 75 | +++ b/storage-daemon/qemu-storage-daemon.c | |
33 | - bdrv_drain_all(); | 76 | @@ -XXX,XX +XXX,XX @@ int main(int argc, char *argv[]) |
34 | - | 77 | main_loop_wait(false); |
35 | unset_dirty_tracking(); | ||
36 | |||
37 | while ((bmds = QSIMPLEQ_FIRST(&block_mig_state.bmds_list)) != NULL) { | ||
38 | @@ -XXX,XX +XXX,XX @@ static void block_migration_cleanup(void *opaque) | ||
39 | g_free(bmds->aio_bitmap); | ||
40 | g_free(bmds); | ||
41 | } | 78 | } |
42 | +} | 79 | |
43 | + | 80 | + blk_exp_close_all(); |
44 | +/* Called with iothread lock taken. */ | 81 | bdrv_drain_all_begin(); |
45 | +static void block_migration_cleanup(void *opaque) | 82 | bdrv_close_all(); |
46 | +{ | ||
47 | + BlkMigBlock *blk; | ||
48 | + | ||
49 | + bdrv_drain_all(); | ||
50 | + | ||
51 | + block_migration_cleanup_bmds(); | ||
52 | |||
53 | blk_mig_lock(); | ||
54 | while ((blk = QSIMPLEQ_FIRST(&block_mig_state.blk_list)) != NULL) { | ||
55 | @@ -XXX,XX +XXX,XX @@ static int block_save_complete(QEMUFile *f, void *opaque) | ||
56 | |||
57 | qemu_put_be64(f, BLK_MIG_FLAG_EOS); | ||
58 | |||
59 | + /* Make sure that our BlockBackends are gone, so that the block driver | ||
60 | + * nodes can be inactivated. */ | ||
61 | + block_migration_cleanup_bmds(); | ||
62 | + | ||
63 | return 0; | ||
64 | } | ||
65 | 83 | ||
66 | -- | 84 | -- |
67 | 1.8.3.1 | 85 | 2.29.2 |
68 | 86 | ||
69 | 87 | diff view generated by jsdifflib |
1 | From: Peter Maydell <peter.maydell@linaro.org> | 1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
---|---|---|---|
2 | 2 | ||
3 | Coverity points out that the code path in qcow_create() for | 3 | Ignore two complains, which now lead to 297 failure on testenv.py and |
4 | the magic "fat:" backing file name leaks the memory used to | 4 | testrunner.py. |
5 | store the filename (CID 1307771). Free the memory before | ||
6 | we overwrite the pointer. | ||
7 | 5 | ||
8 | Signed-off-by: Peter Maydell <peter.maydell@linaro.org> | 6 | Fixes: 2e5a2f57db481f18fcf70be2a36b1417370b8476 |
9 | Reviewed-by: Eric Blake <eblake@redhat.com> | 7 | Fixes: d74c754c924ca34e90b7c96ce2f5609d82c0e628 |
10 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> | 8 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
9 | Message-Id: <20210129161323.615027-1-vsementsov@virtuozzo.com> | ||
10 | Reviewed-by: John Snow <jsnow@redhat.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | --- | 12 | --- |
13 | block/qcow.c | 1 + | 13 | tests/qemu-iotests/pylintrc | 2 ++ |
14 | 1 file changed, 1 insertion(+) | 14 | 1 file changed, 2 insertions(+) |
15 | 15 | ||
16 | diff --git a/block/qcow.c b/block/qcow.c | 16 | diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc |
17 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/block/qcow.c | 18 | --- a/tests/qemu-iotests/pylintrc |
19 | +++ b/block/qcow.c | 19 | +++ b/tests/qemu-iotests/pylintrc |
20 | @@ -XXX,XX +XXX,XX @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) | 20 | @@ -XXX,XX +XXX,XX @@ disable=invalid-name, |
21 | header_size += backing_filename_len; | 21 | unsubscriptable-object, |
22 | } else { | 22 | # These are temporary, and should be removed: |
23 | /* special backing file for vvfat */ | 23 | missing-docstring, |
24 | + g_free(backing_file); | 24 | + too-many-return-statements, |
25 | backing_file = NULL; | 25 | + too-many-statements |
26 | } | 26 | |
27 | header.cluster_bits = 9; /* 512 byte cluster to avoid copying | 27 | [FORMAT] |
28 | |||
28 | -- | 29 | -- |
29 | 1.8.3.1 | 30 | 2.29.2 |
30 | 31 | ||
31 | 32 | diff view generated by jsdifflib |
1 | The final bdrv_set_backing_hd() could be working on already freed nodes | 1 | If the qemu-system-{arch} binary for the host architecture can't be |
---|---|---|---|
2 | because the commit job drops its references (through BlockBackends) to | 2 | found, the old 'check' implementation selected the alphabetically first |
3 | both overlay_bs and top already a bit earlier. | 3 | system emulator binary that it could find. The new Python implementation |
4 | just uses the first result of glob.iglob(), which has an undefined | ||
5 | order. | ||
4 | 6 | ||
5 | One way to trigger the bug is hot unplugging a disk for which | 7 | This is a problem that breaks CI because the iotests aren't actually |
6 | blockdev_mark_auto_del() cancels the block job. | 8 | prepared to run on any emulator. They should be, so this is really a bug |
7 | 9 | in the failing test cases that should be fixed there, but as a quick | |
8 | Fix this by taking BDS-level references while we're still using the | 10 | fix, let's revert to the old behaviour to let CI runs succeed again. |
9 | nodes. | ||
10 | 11 | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | Reviewed-by: John Snow <jsnow@redhat.com> | 13 | Message-Id: <20210202142802.119999-1-kwolf@redhat.com> |
14 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
15 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
16 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
17 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
18 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | 19 | --- |
14 | block/commit.c | 7 +++++++ | 20 | tests/qemu-iotests/testenv.py | 2 +- |
15 | 1 file changed, 7 insertions(+) | 21 | 1 file changed, 1 insertion(+), 1 deletion(-) |
16 | 22 | ||
17 | diff --git a/block/commit.c b/block/commit.c | 23 | diff --git a/tests/qemu-iotests/testenv.py b/tests/qemu-iotests/testenv.py |
18 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/block/commit.c | 25 | --- a/tests/qemu-iotests/testenv.py |
20 | +++ b/block/commit.c | 26 | +++ b/tests/qemu-iotests/testenv.py |
21 | @@ -XXX,XX +XXX,XX @@ static void commit_complete(BlockJob *job, void *opaque) | 27 | @@ -XXX,XX +XXX,XX @@ class TestEnv(ContextManager['TestEnv']): |
22 | int ret = data->ret; | 28 | if not os.path.exists(self.qemu_prog): |
23 | bool remove_commit_top_bs = false; | 29 | pattern = root('qemu-system-*') |
24 | 30 | try: | |
25 | + /* Make sure overlay_bs and top stay around until bdrv_set_backing_hd() */ | 31 | - progs = glob.iglob(pattern) |
26 | + bdrv_ref(top); | 32 | + progs = sorted(glob.iglob(pattern)) |
27 | + bdrv_ref(overlay_bs); | 33 | self.qemu_prog = next(p for p in progs if isxfile(p)) |
28 | + | 34 | except StopIteration: |
29 | /* Remove base node parent that still uses BLK_PERM_WRITE/RESIZE before | 35 | sys.exit("Not found any Qemu executable binary by pattern " |
30 | * the normal backing chain can be restored. */ | ||
31 | blk_unref(s->base); | ||
32 | @@ -XXX,XX +XXX,XX @@ static void commit_complete(BlockJob *job, void *opaque) | ||
33 | if (remove_commit_top_bs) { | ||
34 | bdrv_set_backing_hd(overlay_bs, top, &error_abort); | ||
35 | } | ||
36 | + | ||
37 | + bdrv_unref(overlay_bs); | ||
38 | + bdrv_unref(top); | ||
39 | } | ||
40 | |||
41 | static void coroutine_fn commit_run(void *opaque) | ||
42 | -- | 36 | -- |
43 | 1.8.3.1 | 37 | 2.29.2 |
44 | 38 | ||
45 | 39 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
---|---|---|---|
2 | |||
3 | We should indicate failure by exit code, not only output. | ||
4 | |||
5 | Reported-by: Peter Maydell | ||
6 | Fixes: f203080bbd9f9e5b31041b1f2afcd6040c5aaec5 | ||
7 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Message-Id: <20210201085041.3079-1-vsementsov@virtuozzo.com> | ||
1 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
2 | Reviewed-by: John Snow <jsnow@redhat.com> | ||
3 | --- | 10 | --- |
4 | tests/qemu-iotests/040 | 35 +++++++++++++++++++++++++++++++++-- | 11 | tests/qemu-iotests/testrunner.py | 4 +++- |
5 | tests/qemu-iotests/040.out | 4 ++-- | 12 | tests/qemu-iotests/check | 5 ++++- |
6 | 2 files changed, 35 insertions(+), 4 deletions(-) | 13 | 2 files changed, 7 insertions(+), 2 deletions(-) |
7 | 14 | ||
8 | diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 | 15 | diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py |
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/tests/qemu-iotests/testrunner.py | ||
18 | +++ b/tests/qemu-iotests/testrunner.py | ||
19 | @@ -XXX,XX +XXX,XX @@ class TestRunner(ContextManager['TestRunner']): | ||
20 | |||
21 | return res | ||
22 | |||
23 | - def run_tests(self, tests: List[str]) -> None: | ||
24 | + def run_tests(self, tests: List[str]) -> bool: | ||
25 | n_run = 0 | ||
26 | failed = [] | ||
27 | notrun = [] | ||
28 | @@ -XXX,XX +XXX,XX @@ class TestRunner(ContextManager['TestRunner']): | ||
29 | if failed: | ||
30 | print('Failures:', ' '.join(failed)) | ||
31 | print(f'Failed {len(failed)} of {n_run} iotests') | ||
32 | + return False | ||
33 | else: | ||
34 | print(f'Passed all {n_run} iotests') | ||
35 | + return True | ||
36 | diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check | ||
9 | index XXXXXXX..XXXXXXX 100755 | 37 | index XXXXXXX..XXXXXXX 100755 |
10 | --- a/tests/qemu-iotests/040 | 38 | --- a/tests/qemu-iotests/check |
11 | +++ b/tests/qemu-iotests/040 | 39 | +++ b/tests/qemu-iotests/check |
12 | @@ -XXX,XX +XXX,XX @@ class ImageCommitTestCase(iotests.QMPTestCase): | 40 | @@ -XXX,XX +XXX,XX @@ if __name__ == '__main__': |
13 | self.wait_for_complete() | 41 | else: |
14 | 42 | with TestRunner(env, makecheck=args.makecheck, | |
15 | class TestSingleDrive(ImageCommitTestCase): | 43 | color=args.color) as tr: |
16 | - image_len = 1 * 1024 * 1024 | 44 | - tr.run_tests([os.path.join(env.source_iotests, t) for t in tests]) |
17 | + # Need some space after the copied data so that throttling is effective in | 45 | + paths = [os.path.join(env.source_iotests, t) for t in tests] |
18 | + # tests that use it rather than just completing the job immediately | 46 | + ok = tr.run_tests(paths) |
19 | + image_len = 2 * 1024 * 1024 | 47 | + if not ok: |
20 | test_len = 1 * 1024 * 256 | 48 | + sys.exit(1) |
21 | |||
22 | def setUp(self): | ||
23 | @@ -XXX,XX +XXX,XX @@ class TestSingleDrive(ImageCommitTestCase): | ||
24 | qemu_img('create', '-f', iotests.imgfmt, '-o', 'backing_file=%s' % mid_img, test_img) | ||
25 | qemu_io('-f', 'raw', '-c', 'write -P 0xab 0 524288', backing_img) | ||
26 | qemu_io('-f', iotests.imgfmt, '-c', 'write -P 0xef 524288 524288', mid_img) | ||
27 | - self.vm = iotests.VM().add_drive(test_img) | ||
28 | + self.vm = iotests.VM().add_drive(test_img, interface="none") | ||
29 | + self.vm.add_device("virtio-scsi-pci") | ||
30 | + self.vm.add_device("scsi-hd,id=scsi0,drive=drive0") | ||
31 | self.vm.launch() | ||
32 | |||
33 | def tearDown(self): | ||
34 | @@ -XXX,XX +XXX,XX @@ class TestSingleDrive(ImageCommitTestCase): | ||
35 | self.assert_qmp(result, 'error/class', 'GenericError') | ||
36 | self.assert_qmp(result, 'error/desc', 'Base \'%s\' not found' % mid_img) | ||
37 | |||
38 | + # When the job is running on a BB that is automatically deleted on hot | ||
39 | + # unplug, the job is cancelled when the device disappears | ||
40 | + def test_hot_unplug(self): | ||
41 | + if self.image_len == 0: | ||
42 | + return | ||
43 | + | ||
44 | + self.assert_no_active_block_jobs() | ||
45 | + result = self.vm.qmp('block-commit', device='drive0', top=mid_img, | ||
46 | + base=backing_img, speed=(self.image_len / 4)) | ||
47 | + self.assert_qmp(result, 'return', {}) | ||
48 | + result = self.vm.qmp('device_del', id='scsi0') | ||
49 | + self.assert_qmp(result, 'return', {}) | ||
50 | + | ||
51 | + cancelled = False | ||
52 | + deleted = False | ||
53 | + while not cancelled or not deleted: | ||
54 | + for event in self.vm.get_qmp_events(wait=True): | ||
55 | + if event['event'] == 'DEVICE_DELETED': | ||
56 | + self.assert_qmp(event, 'data/device', 'scsi0') | ||
57 | + deleted = True | ||
58 | + elif event['event'] == 'BLOCK_JOB_CANCELLED': | ||
59 | + self.assert_qmp(event, 'data/device', 'drive0') | ||
60 | + cancelled = True | ||
61 | + else: | ||
62 | + self.fail("Unexpected event %s" % (event['event'])) | ||
63 | + | ||
64 | + self.assert_no_active_block_jobs() | ||
65 | |||
66 | class TestRelativePaths(ImageCommitTestCase): | ||
67 | image_len = 1 * 1024 * 1024 | ||
68 | diff --git a/tests/qemu-iotests/040.out b/tests/qemu-iotests/040.out | ||
69 | index XXXXXXX..XXXXXXX 100644 | ||
70 | --- a/tests/qemu-iotests/040.out | ||
71 | +++ b/tests/qemu-iotests/040.out | ||
72 | @@ -XXX,XX +XXX,XX @@ | ||
73 | -......................... | ||
74 | +........................... | ||
75 | ---------------------------------------------------------------------- | ||
76 | -Ran 25 tests | ||
77 | +Ran 27 tests | ||
78 | |||
79 | OK | ||
80 | -- | 49 | -- |
81 | 1.8.3.1 | 50 | 2.29.2 |
82 | 51 | ||
83 | 52 | diff view generated by jsdifflib |
1 | For -makecheck, the old 'check' implementation skipped the output when | ||
---|---|---|---|
2 | starting a test. It only had the condensed output at the end of a test. | ||
3 | |||
4 | testrunner.py prints the normal output when starting a test even for | ||
5 | -makecheck. This output contains '\r' at the end so that it can be | ||
6 | overwritten with the result at the end of the test. However, for | ||
7 | -makecheck this is shorter output in a different format, so effectively | ||
8 | we end up with garbled output that mixes both output forms. | ||
9 | |||
10 | Revert to the old behaviour of only printing a message after the test | ||
11 | had completed in -makecheck mode. | ||
12 | |||
13 | Fixes: d74c754c924ca34e90b7c96ce2f5609d82c0e628 | ||
1 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
2 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 15 | Message-Id: <20210201161024.127921-1-kwolf@redhat.com> |
3 | Reviewed-by: Eric Blake <eblake@redhat.com> | 16 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
4 | --- | 18 | --- |
5 | tests/qemu-iotests/183 | 140 +++++++++++++++++++++++++++++++++++++++++++++ | 19 | tests/qemu-iotests/testrunner.py | 6 ++++-- |
6 | tests/qemu-iotests/183.out | 46 +++++++++++++++ | 20 | 1 file changed, 4 insertions(+), 2 deletions(-) |
7 | tests/qemu-iotests/group | 1 + | ||
8 | 3 files changed, 187 insertions(+) | ||
9 | create mode 100755 tests/qemu-iotests/183 | ||
10 | create mode 100644 tests/qemu-iotests/183.out | ||
11 | 21 | ||
12 | diff --git a/tests/qemu-iotests/183 b/tests/qemu-iotests/183 | 22 | diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py |
13 | new file mode 100755 | ||
14 | index XXXXXXX..XXXXXXX | ||
15 | --- /dev/null | ||
16 | +++ b/tests/qemu-iotests/183 | ||
17 | @@ -XXX,XX +XXX,XX @@ | ||
18 | +#!/bin/bash | ||
19 | +# | ||
20 | +# Test old-style block migration (migrate -b) | ||
21 | +# | ||
22 | +# Copyright (C) 2017 Red Hat, Inc. | ||
23 | +# | ||
24 | +# This program is free software; you can redistribute it and/or modify | ||
25 | +# it under the terms of the GNU General Public License as published by | ||
26 | +# the Free Software Foundation; either version 2 of the License, or | ||
27 | +# (at your option) any later version. | ||
28 | +# | ||
29 | +# This program is distributed in the hope that it will be useful, | ||
30 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
31 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
32 | +# GNU General Public License for more details. | ||
33 | +# | ||
34 | +# You should have received a copy of the GNU General Public License | ||
35 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
36 | +# | ||
37 | + | ||
38 | +# creator | ||
39 | +owner=kwolf@redhat.com | ||
40 | + | ||
41 | +seq=`basename $0` | ||
42 | +echo "QA output created by $seq" | ||
43 | + | ||
44 | +here=`pwd` | ||
45 | +status=1 # failure is the default! | ||
46 | + | ||
47 | +MIG_SOCKET="${TEST_DIR}/migrate" | ||
48 | + | ||
49 | +_cleanup() | ||
50 | +{ | ||
51 | + rm -f "${MIG_SOCKET}" | ||
52 | + rm -f "${TEST_IMG}.dest" | ||
53 | + _cleanup_test_img | ||
54 | + _cleanup_qemu | ||
55 | +} | ||
56 | +trap "_cleanup; exit \$status" 0 1 2 3 15 | ||
57 | + | ||
58 | +# get standard environment, filters and checks | ||
59 | +. ./common.rc | ||
60 | +. ./common.filter | ||
61 | +. ./common.qemu | ||
62 | + | ||
63 | +_supported_fmt qcow2 raw qed dmg quorum | ||
64 | +_supported_proto file | ||
65 | +_supported_os Linux | ||
66 | + | ||
67 | +size=64M | ||
68 | +_make_test_img $size | ||
69 | +TEST_IMG="${TEST_IMG}.dest" _make_test_img $size | ||
70 | + | ||
71 | +echo | ||
72 | +echo === Starting VMs === | ||
73 | +echo | ||
74 | + | ||
75 | +qemu_comm_method="qmp" | ||
76 | + | ||
77 | +_launch_qemu \ | ||
78 | + -drive file="${TEST_IMG}",cache=$CACHEMODE,driver=$IMGFMT,id=disk | ||
79 | +src=$QEMU_HANDLE | ||
80 | +_send_qemu_cmd $src "{ 'execute': 'qmp_capabilities' }" 'return' | ||
81 | + | ||
82 | +_launch_qemu \ | ||
83 | + -drive file="${TEST_IMG}.dest",cache=$CACHEMODE,driver=$IMGFMT,id=disk \ | ||
84 | + -incoming "unix:${MIG_SOCKET}" | ||
85 | +dest=$QEMU_HANDLE | ||
86 | +_send_qemu_cmd $dest "{ 'execute': 'qmp_capabilities' }" 'return' | ||
87 | + | ||
88 | +echo | ||
89 | +echo === Write something on the source === | ||
90 | +echo | ||
91 | + | ||
92 | +_send_qemu_cmd $src \ | ||
93 | + "{ 'execute': 'human-monitor-command', | ||
94 | + 'arguments': { 'command-line': | ||
95 | + 'qemu-io disk \"write -P 0x55 0 64k\"' } }" \ | ||
96 | + 'return' | ||
97 | +_send_qemu_cmd $src \ | ||
98 | + "{ 'execute': 'human-monitor-command', | ||
99 | + 'arguments': { 'command-line': | ||
100 | + 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \ | ||
101 | + 'return' | ||
102 | + | ||
103 | +echo | ||
104 | +echo === Do block migration to destination === | ||
105 | +echo | ||
106 | + | ||
107 | +reply="$(_send_qemu_cmd $src \ | ||
108 | + "{ 'execute': 'migrate', | ||
109 | + 'arguments': { 'uri': 'unix:${MIG_SOCKET}', 'blk': true } }" \ | ||
110 | + 'return\|error')" | ||
111 | +echo "$reply" | ||
112 | +if echo "$reply" | grep "compiled without old-style" > /dev/null; then | ||
113 | + _notrun "migrate -b support not compiled in" | ||
114 | +fi | ||
115 | + | ||
116 | +QEMU_COMM_TIMEOUT=0.1 qemu_cmd_repeat=50 silent=yes \ | ||
117 | + _send_qemu_cmd $src "{ 'execute': 'query-migrate' }" '"status": "completed"' | ||
118 | +_send_qemu_cmd $src "{ 'execute': 'query-status' }" "return" | ||
119 | + | ||
120 | +echo | ||
121 | +echo === Do some I/O on the destination === | ||
122 | +echo | ||
123 | + | ||
124 | +# It is important that we use the BlockBackend of the guest device here instead | ||
125 | +# of the node name, which would create a new BlockBackend and not test whether | ||
126 | +# the guest has the necessary permissions to access the image now | ||
127 | +silent=yes _send_qemu_cmd $dest "" "100 %" | ||
128 | +_send_qemu_cmd $dest "{ 'execute': 'query-status' }" "return" | ||
129 | +_send_qemu_cmd $dest \ | ||
130 | + "{ 'execute': 'human-monitor-command', | ||
131 | + 'arguments': { 'command-line': | ||
132 | + 'qemu-io disk \"read -P 0x55 0 64k\"' } }" \ | ||
133 | + 'return' | ||
134 | +_send_qemu_cmd $dest \ | ||
135 | + "{ 'execute': 'human-monitor-command', | ||
136 | + 'arguments': { 'command-line': | ||
137 | + 'qemu-io disk \"write -P 0x66 1M 64k\"' } }" \ | ||
138 | + 'return' | ||
139 | + | ||
140 | +echo | ||
141 | +echo === Shut down and check image === | ||
142 | +echo | ||
143 | + | ||
144 | +_send_qemu_cmd $src '{"execute":"quit"}' 'return' | ||
145 | +_send_qemu_cmd $dest '{"execute":"quit"}' 'return' | ||
146 | +wait=1 _cleanup_qemu | ||
147 | + | ||
148 | +_check_test_img | ||
149 | +TEST_IMG="${TEST_IMG}.dest" _check_test_img | ||
150 | + | ||
151 | +$QEMU_IO -c 'write -P 0x66 1M 64k' "$TEST_IMG" | _filter_qemu_io | ||
152 | +$QEMU_IMG compare "$TEST_IMG.dest" "$TEST_IMG" | ||
153 | + | ||
154 | +# success, all done | ||
155 | +echo "*** done" | ||
156 | +rm -f $seq.full | ||
157 | +status=0 | ||
158 | diff --git a/tests/qemu-iotests/183.out b/tests/qemu-iotests/183.out | ||
159 | new file mode 100644 | ||
160 | index XXXXXXX..XXXXXXX | ||
161 | --- /dev/null | ||
162 | +++ b/tests/qemu-iotests/183.out | ||
163 | @@ -XXX,XX +XXX,XX @@ | ||
164 | +QA output created by 183 | ||
165 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 | ||
166 | +Formatting 'TEST_DIR/t.IMGFMT.dest', fmt=IMGFMT size=67108864 | ||
167 | + | ||
168 | +=== Starting VMs === | ||
169 | + | ||
170 | +{"return": {}} | ||
171 | +{"return": {}} | ||
172 | + | ||
173 | +=== Write something on the source === | ||
174 | + | ||
175 | +wrote 65536/65536 bytes at offset 0 | ||
176 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
177 | +{"return": ""} | ||
178 | +read 65536/65536 bytes at offset 0 | ||
179 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
180 | +{"return": ""} | ||
181 | + | ||
182 | +=== Do block migration to destination === | ||
183 | + | ||
184 | +{"return": {}} | ||
185 | +{"return": {"status": "postmigrate", "singlestep": false, "running": false}} | ||
186 | + | ||
187 | +=== Do some I/O on the destination === | ||
188 | + | ||
189 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "RESUME"} | ||
190 | +{"return": {"status": "running", "singlestep": false, "running": true}} | ||
191 | +read 65536/65536 bytes at offset 0 | ||
192 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
193 | +{"return": ""} | ||
194 | +wrote 65536/65536 bytes at offset 1048576 | ||
195 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
196 | +{"return": ""} | ||
197 | + | ||
198 | +=== Shut down and check image === | ||
199 | + | ||
200 | +{"return": {}} | ||
201 | +{"return": {}} | ||
202 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} | ||
203 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}} | ||
204 | +No errors were found on the image. | ||
205 | +No errors were found on the image. | ||
206 | +wrote 65536/65536 bytes at offset 1048576 | ||
207 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
208 | +Images are identical. | ||
209 | +*** done | ||
210 | diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group | ||
211 | index XXXXXXX..XXXXXXX 100644 | 23 | index XXXXXXX..XXXXXXX 100644 |
212 | --- a/tests/qemu-iotests/group | 24 | --- a/tests/qemu-iotests/testrunner.py |
213 | +++ b/tests/qemu-iotests/group | 25 | +++ b/tests/qemu-iotests/testrunner.py |
214 | @@ -XXX,XX +XXX,XX @@ | 26 | @@ -XXX,XX +XXX,XX @@ class TestRunner(ContextManager['TestRunner']): |
215 | 179 rw auto quick | 27 | last_el = self.last_elapsed.get(test) |
216 | 181 rw auto migration | 28 | start = datetime.datetime.now().strftime('%H:%M:%S') |
217 | 182 rw auto quick | 29 | |
218 | +183 rw auto migration | 30 | - self.test_print_one_line(test=test, starttime=start, lasttime=last_el, |
31 | - end='\r', test_field_width=test_field_width) | ||
32 | + if not self.makecheck: | ||
33 | + self.test_print_one_line(test=test, starttime=start, | ||
34 | + lasttime=last_el, end='\r', | ||
35 | + test_field_width=test_field_width) | ||
36 | |||
37 | res = self.do_run_test(test) | ||
38 | |||
219 | -- | 39 | -- |
220 | 1.8.3.1 | 40 | 2.29.2 |
221 | 41 | ||
222 | 42 | diff view generated by jsdifflib |
1 | blk->name isn't an array, but a pointer that can be NULL. Checking for | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | an anonymous BB must involve a NULL check first, otherwise we get | ||
3 | crashes. | ||
4 | 2 | ||
3 | Commit 15b2260bef3 ("block/nvme: Trace controller capabilities") | ||
4 | misunderstood the doorbell stride value from the datasheet, use | ||
5 | the correct one. The 'doorbell_scale' variable used few lines | ||
6 | later is correct. | ||
7 | |||
8 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
9 | Message-Id: <20210127212137.3482291-2-philmd@redhat.com> | ||
10 | Reviewed-by: Klaus Jensen <k.jensen@samsung.com> | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
6 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
7 | Reviewed-by: Juan Quintela <quintela@redhat.com> | ||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
10 | --- | 12 | --- |
11 | block/block-backend.c | 2 +- | 13 | block/nvme.c | 2 +- |
12 | 1 file changed, 1 insertion(+), 1 deletion(-) | 14 | 1 file changed, 1 insertion(+), 1 deletion(-) |
13 | 15 | ||
14 | diff --git a/block/block-backend.c b/block/block-backend.c | 16 | diff --git a/block/nvme.c b/block/nvme.c |
15 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/block/block-backend.c | 18 | --- a/block/nvme.c |
17 | +++ b/block/block-backend.c | 19 | +++ b/block/nvme.c |
18 | @@ -XXX,XX +XXX,XX @@ static int blk_root_inactivate(BdrvChild *child) | 20 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, |
19 | * this point because the VM is stopped) and unattached monitor-owned | 21 | trace_nvme_controller_capability("Contiguous Queues Required", |
20 | * BlockBackends. If there is still any other user like a block job, then | 22 | NVME_CAP_CQR(cap)); |
21 | * we simply can't inactivate the image. */ | 23 | trace_nvme_controller_capability("Doorbell Stride", |
22 | - if (!blk->dev && !blk->name[0]) { | 24 | - 2 << (2 + NVME_CAP_DSTRD(cap))); |
23 | + if (!blk->dev && !blk_name(blk)[0]) { | 25 | + 1 << (2 + NVME_CAP_DSTRD(cap))); |
24 | return -EPERM; | 26 | trace_nvme_controller_capability("Subsystem Reset Supported", |
25 | } | 27 | NVME_CAP_NSSRS(cap)); |
26 | 28 | trace_nvme_controller_capability("Memory Page Size Minimum", | |
27 | -- | 29 | -- |
28 | 1.8.3.1 | 30 | 2.29.2 |
29 | 31 | ||
30 | 32 | diff view generated by jsdifflib |
1 | Block migration may still access the image during its | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | .save_live_complete_precopy() implementation, so we should only | ||
3 | inactivate the image afterwards. | ||
4 | 2 | ||
5 | Another reason for the change is that inactivating an image fails when | 3 | NVMe controllers implement different versions of the spec, |
6 | there is still a non-device BlockBackend using it, which includes the | 4 | and different features of it. It is useful to gather this |
7 | BBs used by block migration. We want to give block migration a chance to | 5 | information when debugging. |
8 | release the BBs before trying to inactivate the image (this will be done | ||
9 | in another patch). | ||
10 | 6 | ||
7 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
8 | Message-Id: <20210127212137.3482291-3-philmd@redhat.com> | ||
9 | Reviewed-by: Klaus Jensen <k.jensen@samsung.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
13 | Reviewed-by: Juan Quintela <quintela@redhat.com> | ||
14 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
15 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
16 | --- | 11 | --- |
17 | migration/migration.c | 12 +++++++----- | 12 | block/nvme.c | 6 ++++++ |
18 | 1 file changed, 7 insertions(+), 5 deletions(-) | 13 | block/trace-events | 1 + |
14 | 2 files changed, 7 insertions(+) | ||
19 | 15 | ||
20 | diff --git a/migration/migration.c b/migration/migration.c | 16 | diff --git a/block/nvme.c b/block/nvme.c |
21 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/migration/migration.c | 18 | --- a/block/nvme.c |
23 | +++ b/migration/migration.c | 19 | +++ b/block/nvme.c |
24 | @@ -XXX,XX +XXX,XX @@ static void migration_completion(MigrationState *s, int current_active_state, | 20 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, |
25 | 21 | AioContext *aio_context = bdrv_get_aio_context(bs); | |
26 | if (!ret) { | 22 | int ret; |
27 | ret = vm_stop_force_state(RUN_STATE_FINISH_MIGRATE); | 23 | uint64_t cap; |
28 | + if (ret >= 0) { | 24 | + uint32_t ver; |
29 | + qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); | 25 | uint64_t timeout_ms; |
30 | + qemu_savevm_state_complete_precopy(s->to_dst_file, false); | 26 | uint64_t deadline, now; |
31 | + } | 27 | volatile NvmeBar *regs = NULL; |
32 | /* | 28 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(BlockDriverState *bs, const char *device, int namespace, |
33 | * Don't mark the image with BDRV_O_INACTIVE flag if | 29 | bs->bl.request_alignment = s->page_size; |
34 | * we will go into COLO stage later. | 30 | timeout_ms = MIN(500 * NVME_CAP_TO(cap), 30000); |
35 | */ | 31 | |
36 | if (ret >= 0 && !migrate_colo_enabled()) { | 32 | + ver = le32_to_cpu(regs->vs); |
37 | ret = bdrv_inactivate_all(); | 33 | + trace_nvme_controller_spec_version(extract32(ver, 16, 16), |
38 | - } | 34 | + extract32(ver, 8, 8), |
39 | - if (ret >= 0) { | 35 | + extract32(ver, 0, 8)); |
40 | - qemu_file_set_rate_limit(s->to_dst_file, INT64_MAX); | 36 | + |
41 | - qemu_savevm_state_complete_precopy(s->to_dst_file, false); | 37 | /* Reset device to get a clean state. */ |
42 | - s->block_inactive = true; | 38 | regs->cc = cpu_to_le32(le32_to_cpu(regs->cc) & 0xFE); |
43 | + if (ret >= 0) { | 39 | /* Wait for CSTS.RDY = 0. */ |
44 | + s->block_inactive = true; | 40 | diff --git a/block/trace-events b/block/trace-events |
45 | + } | 41 | index XXXXXXX..XXXXXXX 100644 |
46 | } | 42 | --- a/block/trace-events |
47 | } | 43 | +++ b/block/trace-events |
48 | qemu_mutex_unlock_iothread(); | 44 | @@ -XXX,XX +XXX,XX @@ qed_aio_write_main(void *s, void *acb, int ret, uint64_t offset, size_t len) "s |
45 | # nvme.c | ||
46 | nvme_controller_capability_raw(uint64_t value) "0x%08"PRIx64 | ||
47 | nvme_controller_capability(const char *desc, uint64_t value) "%s: %"PRIu64 | ||
48 | +nvme_controller_spec_version(uint32_t mjr, uint32_t mnr, uint32_t ter) "Specification supported: %u.%u.%u" | ||
49 | nvme_kick(void *s, unsigned q_index) "s %p q #%u" | ||
50 | nvme_dma_flush_queue_wait(void *s) "s %p" | ||
51 | nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" | ||
49 | -- | 52 | -- |
50 | 1.8.3.1 | 53 | 2.29.2 |
51 | 54 | ||
52 | 55 | diff view generated by jsdifflib |
1 | From: Jeff Cody <jcody@redhat.com> | 1 | size_to_str() can return a size like "4.24 MiB", with a single digit |
---|---|---|---|
2 | integer part and two fractional digits. This is eight characters, but | ||
3 | commit b39847a5 changed the format string to only reserve seven | ||
4 | characters for the column. | ||
2 | 5 | ||
3 | In external_snapshot_abort(), we try to undo what was done in | 6 | This can result in unaligned columns, which in turn changes the output of |
4 | external_snapshot_prepare() calling bdrv_replace_node() to swap the | 7 | iotests case 267 because exceeding the column size defeats the attempt |
5 | nodes back. However, we receive a permissions error as writers are | 8 | to filter the size out of the output (observed with the ppc64 emulator). |
6 | blocked on the old node, which is now the new node backing file. | 9 | The resulting change is only a whitespace change, but since commit |
10 | f203080b this is enough for iotests to consider the test failed. | ||
7 | 11 | ||
8 | An easy fix (initially suggested by Kevin Wolf) is to call | 12 | Taking a character away from the tag name column and adding it to the VM |
9 | bdrv_set_backing_hd() on the new node, to set the backing node to NULL. | 13 | size column doesn't change anything in the common case (the tag name is |
14 | left justified, the VM size is right justified), but fixes this case. | ||
10 | 15 | ||
11 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 16 | Fixes: b39847a50553b7679d6d7fefbe6a108a17aacf8d |
17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
18 | Message-Id: <20210202155911.179865-1-kwolf@redhat.com> | ||
12 | Reviewed-by: Eric Blake <eblake@redhat.com> | 19 | Reviewed-by: Eric Blake <eblake@redhat.com> |
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 20 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
14 | --- | 21 | --- |
15 | blockdev.c | 4 ++++ | 22 | block/qapi.c | 4 ++-- |
16 | 1 file changed, 4 insertions(+) | 23 | 1 file changed, 2 insertions(+), 2 deletions(-) |
17 | 24 | ||
18 | diff --git a/blockdev.c b/blockdev.c | 25 | diff --git a/block/qapi.c b/block/qapi.c |
19 | index XXXXXXX..XXXXXXX 100644 | 26 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/blockdev.c | 27 | --- a/block/qapi.c |
21 | +++ b/blockdev.c | 28 | +++ b/block/qapi.c |
22 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common) | 29 | @@ -XXX,XX +XXX,XX @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn) |
23 | DO_UPCAST(ExternalSnapshotState, common, common); | 30 | char *sizing = NULL; |
24 | if (state->new_bs) { | 31 | |
25 | if (state->overlay_appended) { | 32 | if (!sn) { |
26 | + bdrv_ref(state->old_bs); /* we can't let bdrv_set_backind_hd() | 33 | - qemu_printf("%-10s%-18s%7s%20s%13s%11s", |
27 | + close state->old_bs; we need it */ | 34 | + qemu_printf("%-10s%-17s%8s%20s%13s%11s", |
28 | + bdrv_set_backing_hd(state->new_bs, NULL, &error_abort); | 35 | "ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT"); |
29 | bdrv_replace_node(state->new_bs, state->old_bs, &error_abort); | 36 | } else { |
30 | + bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */ | 37 | ti = sn->date_sec; |
38 | @@ -XXX,XX +XXX,XX @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn) | ||
39 | snprintf(icount_buf, sizeof(icount_buf), | ||
40 | "%"PRId64, sn->icount); | ||
31 | } | 41 | } |
32 | } | 42 | - qemu_printf("%-9s %-17s %7s%20s%13s%11s", |
33 | } | 43 | + qemu_printf("%-9s %-16s %8s%20s%13s%11s", |
44 | sn->id_str, sn->name, | ||
45 | sizing, | ||
46 | date_buf, | ||
34 | -- | 47 | -- |
35 | 1.8.3.1 | 48 | 2.29.2 |
36 | 49 | ||
37 | 50 | diff view generated by jsdifflib |