1 | The following changes since commit ba29883206d92a29ad5a466e679ccfc2ee6132ef: | 1 | The following changes since commit d922088eb4ba6bc31a99f17b32cf75e59dd306cd: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/borntraeger/tags/s390x-20200310' into staging (2020-03-10 16:50:28 +0000) | 3 | Merge tag 'ui-pull-request' of https://gitlab.com/marcandre.lureau/qemu into staging (2025-02-03 13:42:02 -0500) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://repo.or.cz/qemu/kevin.git tags/for-upstream | 7 | https://repo.or.cz/qemu/kevin.git tags/for-upstream |
8 | 8 | ||
9 | for you to fetch changes up to 8bb3b023f2055054ee119cb45b42d2b14be7fc8a: | 9 | for you to fetch changes up to fc4e394b2887e15d5f83994e4fc7b26c895c627a: |
10 | 10 | ||
11 | qemu-iotests: adding LUKS cleanup for non-UTF8 secret error (2020-03-11 15:54:38 +0100) | 11 | block: remove unused BLOCK_OP_TYPE_DATAPLANE (2025-02-06 14:51:10 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block layer patches: | 14 | Block layer patches |
15 | 15 | ||
16 | - Relax restrictions for blockdev-snapshot (allows libvirt to do live | 16 | - Managing inactive nodes (enables QSD migration with shared storage) |
17 | storage migration with blockdev-mirror) | 17 | - Fix swapped values for BLOCK_IO_ERROR 'device' and 'qom-path' |
18 | - luks: Delete created files when block_crypto_co_create_opts_luks fails | 18 | - vpc: Read images exported from Azure correctly |
19 | - Fix memleaks in qmp_object_add | 19 | - scripts/qemu-gdb: Support coroutine dumps in coredumps |
20 | - Minor cleanups | ||
20 | 21 | ||
21 | ---------------------------------------------------------------- | 22 | ---------------------------------------------------------------- |
22 | Daniel Henrique Barboza (4): | 23 | Fabiano Rosas (1): |
23 | block: introducing 'bdrv_co_delete_file' interface | 24 | block: Fix leak in send_qmp_error_event |
24 | block.c: adding bdrv_co_delete_file | ||
25 | crypto.c: cleanup created file when block_crypto_co_create_opts_luks fails | ||
26 | qemu-iotests: adding LUKS cleanup for non-UTF8 secret error | ||
27 | 25 | ||
28 | Kevin Wolf (6): | 26 | Kevin Wolf (16): |
29 | block: Make bdrv_get_cumulative_perm() public | 27 | block: Add 'active' field to BlockDeviceInfo |
30 | block: Relax restrictions for blockdev-snapshot | 28 | block: Allow inactivating already inactive nodes |
31 | iotests: Fix run_job() with use_log=False | 29 | block: Inactivate external snapshot overlays when necessary |
32 | iotests: Test mirror with temporarily disabled target backing file | 30 | migration/block-active: Remove global active flag |
33 | block: Fix cross-AioContext blockdev-snapshot | 31 | block: Don't attach inactive child to active node |
34 | iotests: Add iothread cases to 155 | 32 | block: Fix crash on block_resize on inactive node |
35 | 33 | block: Add option to create inactive nodes | |
36 | Pan Nengyuan (1): | 34 | block: Add blockdev-set-active QMP command |
37 | qom-qmp-cmds: fix two memleaks in qmp_object_add | 35 | block: Support inactive nodes in blk_insert_bs() |
36 | block/export: Don't ignore image activation error in blk_exp_add() | ||
37 | block: Drain nodes before inactivating them | ||
38 | block/export: Add option to allow export of inactive nodes | ||
39 | nbd/server: Support inactive nodes | ||
40 | iotests: Add filter_qtest() | ||
41 | iotests: Add qsd-migrate case | ||
42 | iotests: Add (NBD-based) tests for inactive nodes | ||
38 | 43 | ||
39 | Peter Krempa (1): | 44 | Peter Krempa (1): |
40 | qapi: Add '@allow-write-only-overlay' feature for 'blockdev-snapshot' | 45 | block-backend: Fix argument order when calling 'qapi_event_send_block_io_error()' |
46 | |||
47 | Peter Xu (3): | ||
48 | scripts/qemu-gdb: Always do full stack dump for python errors | ||
49 | scripts/qemu-gdb: Simplify fs_base fetching for coroutines | ||
50 | scripts/qemu-gdb: Support coroutine dumps in coredumps | ||
41 | 51 | ||
42 | Philippe Mathieu-Daudé (1): | 52 | Philippe Mathieu-Daudé (1): |
43 | tests/qemu-iotests: Fix socket_scm_helper build path | 53 | block: Improve blk_get_attached_dev_id() docstring |
44 | 54 | ||
45 | qapi/block-core.json | 9 ++++- | 55 | Stefan Hajnoczi (1): |
46 | include/block/block.h | 1 + | 56 | block: remove unused BLOCK_OP_TYPE_DATAPLANE |
47 | include/block/block_int.h | 7 ++++ | 57 | |
48 | block.c | 33 ++++++++++++++-- | 58 | Vitaly Kuznetsov (2): |
49 | block/crypto.c | 18 +++++++++ | 59 | vpc: Split off vpc_ignore_current_size() helper |
50 | block/file-posix.c | 23 +++++++++++ | 60 | vpc: Read images exported from Azure correctly |
51 | blockdev.c | 30 ++++----------- | 61 | |
52 | qom/qom-qmp-cmds.c | 16 +++----- | 62 | qapi/block-core.json | 44 +++- |
53 | tests/qemu-iotests/iotests.py | 5 ++- | 63 | qapi/block-export.json | 10 +- |
54 | tests/Makefile.include | 1 + | 64 | include/block/block-common.h | 2 +- |
55 | tests/qemu-iotests/085.out | 4 +- | 65 | include/block/block-global-state.h | 6 + |
56 | tests/qemu-iotests/155 | 88 ++++++++++++++++++++++++++++++++++++------- | 66 | include/block/export.h | 3 + |
57 | tests/qemu-iotests/155.out | 4 +- | 67 | include/system/block-backend-io.h | 7 + |
58 | tests/qemu-iotests/282 | 67 ++++++++++++++++++++++++++++++++ | 68 | migration/migration.h | 3 - |
59 | tests/qemu-iotests/282.out | 11 ++++++ | 69 | block.c | 64 +++++- |
60 | tests/qemu-iotests/group | 1 + | 70 | block/block-backend.c | 32 ++- |
61 | tests/qtest/Makefile.include | 1 - | 71 | block/export/export.c | 29 ++- |
62 | 17 files changed, 262 insertions(+), 57 deletions(-) | 72 | block/monitor/block-hmp-cmds.c | 5 +- |
63 | create mode 100755 tests/qemu-iotests/282 | 73 | block/qapi.c | 1 + |
64 | create mode 100644 tests/qemu-iotests/282.out | 74 | block/replication.c | 1 - |
75 | block/vpc.c | 65 +++--- | ||
76 | blockdev.c | 48 ++++ | ||
77 | blockjob.c | 2 - | ||
78 | hw/block/virtio-blk.c | 9 - | ||
79 | hw/scsi/virtio-scsi.c | 3 - | ||
80 | migration/block-active.c | 46 ---- | ||
81 | migration/migration.c | 8 - | ||
82 | nbd/server.c | 17 ++ | ||
83 | scripts/qemu-gdb.py | 2 + | ||
84 | scripts/qemugdb/coroutine.py | 102 ++++++--- | ||
85 | tests/qemu-iotests/iotests.py | 8 + | ||
86 | tests/qemu-iotests/041 | 4 +- | ||
87 | tests/qemu-iotests/165 | 4 +- | ||
88 | tests/qemu-iotests/184.out | 2 + | ||
89 | tests/qemu-iotests/191.out | 16 ++ | ||
90 | tests/qemu-iotests/273.out | 5 + | ||
91 | tests/qemu-iotests/tests/copy-before-write | 3 +- | ||
92 | tests/qemu-iotests/tests/inactive-node-nbd | 303 +++++++++++++++++++++++++ | ||
93 | tests/qemu-iotests/tests/inactive-node-nbd.out | 239 +++++++++++++++++++ | ||
94 | tests/qemu-iotests/tests/migrate-bitmaps-test | 7 +- | ||
95 | tests/qemu-iotests/tests/qsd-migrate | 140 ++++++++++++ | ||
96 | tests/qemu-iotests/tests/qsd-migrate.out | 59 +++++ | ||
97 | 35 files changed, 1133 insertions(+), 166 deletions(-) | ||
98 | create mode 100755 tests/qemu-iotests/tests/inactive-node-nbd | ||
99 | create mode 100644 tests/qemu-iotests/tests/inactive-node-nbd.out | ||
100 | create mode 100755 tests/qemu-iotests/tests/qsd-migrate | ||
101 | create mode 100644 tests/qemu-iotests/tests/qsd-migrate.out | ||
65 | 102 | ||
66 | 103 | diff view generated by jsdifflib |
1 | From: Daniel Henrique Barboza <danielhb413@gmail.com> | 1 | From: Vitaly Kuznetsov <vkuznets@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Adding to Block Drivers the capability of being able to clean up | 3 | In preparation to making changes to the logic deciding whether CHS or |
4 | its created files can be useful in certain situations. For the | 4 | 'current_size' need to be used in determining the image size, split off |
5 | LUKS driver, for instance, a failure in one of its authentication | 5 | vpc_ignore_current_size() helper. |
6 | steps can leave files in the host that weren't there before. | ||
7 | 6 | ||
8 | This patch adds the 'bdrv_co_delete_file' interface to block | 7 | No functional change intended. |
9 | drivers and add it to the 'file' driver in file-posix.c. The | ||
10 | implementation is given by 'raw_co_delete_file'. | ||
11 | 8 | ||
12 | Suggested-by: Daniel P. Berrangé <berrange@redhat.com> | 9 | Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> |
13 | Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> | 10 | Message-ID: <20241212134504.1983757-2-vkuznets@redhat.com> |
14 | Message-Id: <20200130213907.2830642-2-danielhb413@gmail.com> | 11 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
12 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
16 | --- | 14 | --- |
17 | include/block/block_int.h | 4 ++++ | 15 | block/vpc.c | 67 +++++++++++++++++++++++++++++------------------------ |
18 | block/file-posix.c | 23 +++++++++++++++++++++++ | 16 | 1 file changed, 37 insertions(+), 30 deletions(-) |
19 | 2 files changed, 27 insertions(+) | ||
20 | 17 | ||
21 | diff --git a/include/block/block_int.h b/include/block/block_int.h | 18 | diff --git a/block/vpc.c b/block/vpc.c |
22 | index XXXXXXX..XXXXXXX 100644 | 19 | index XXXXXXX..XXXXXXX 100644 |
23 | --- a/include/block/block_int.h | 20 | --- a/block/vpc.c |
24 | +++ b/include/block/block_int.h | 21 | +++ b/block/vpc.c |
25 | @@ -XXX,XX +XXX,XX @@ struct BlockDriver { | 22 | @@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts, |
26 | */ | 23 | } |
27 | int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs); | ||
28 | |||
29 | + /* Delete a created file. */ | ||
30 | + int coroutine_fn (*bdrv_co_delete_file)(BlockDriverState *bs, | ||
31 | + Error **errp); | ||
32 | + | ||
33 | /* | ||
34 | * Flushes all data that was already written to the OS all the way down to | ||
35 | * the disk (for example file-posix.c calls fsync()). | ||
36 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | ||
38 | --- a/block/file-posix.c | ||
39 | +++ b/block/file-posix.c | ||
40 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_create_opts(const char *filename, QemuOpts *opts, | ||
41 | return raw_co_create(&options, errp); | ||
42 | } | 24 | } |
43 | 25 | ||
44 | +static int coroutine_fn raw_co_delete_file(BlockDriverState *bs, | 26 | +/* |
45 | + Error **errp) | 27 | + * Microsoft Virtual PC and Microsoft Hyper-V produce and read |
28 | + * VHD image sizes differently. VPC will rely on CHS geometry, | ||
29 | + * while Hyper-V and disk2vhd use the size specified in the footer. | ||
30 | + * | ||
31 | + * We use a couple of approaches to try and determine the correct method: | ||
32 | + * look at the Creator App field, and look for images that have CHS | ||
33 | + * geometry that is the maximum value. | ||
34 | + * | ||
35 | + * If the CHS geometry is the maximum CHS geometry, then we assume that | ||
36 | + * the size is the footer->current_size to avoid truncation. Otherwise, | ||
37 | + * we follow the table based on footer->creator_app: | ||
38 | + * | ||
39 | + * Known creator apps: | ||
40 | + * 'vpc ' : CHS Virtual PC (uses disk geometry) | ||
41 | + * 'qemu' : CHS QEMU (uses disk geometry) | ||
42 | + * 'qem2' : current_size QEMU (uses current_size) | ||
43 | + * 'win ' : current_size Hyper-V | ||
44 | + * 'd2v ' : current_size Disk2vhd | ||
45 | + * 'tap\0' : current_size XenServer | ||
46 | + * 'CTXS' : current_size XenConverter | ||
47 | + * | ||
48 | + * The user can override the table values via drive options, however | ||
49 | + * even with an override we will still use current_size for images | ||
50 | + * that have CHS geometry of the maximum size. | ||
51 | + */ | ||
52 | +static bool vpc_ignore_current_size(VHDFooter *footer) | ||
46 | +{ | 53 | +{ |
47 | + struct stat st; | 54 | + return !!strncmp(footer->creator_app, "win ", 4) && |
48 | + int ret; | 55 | + !!strncmp(footer->creator_app, "qem2", 4) && |
49 | + | 56 | + !!strncmp(footer->creator_app, "d2v ", 4) && |
50 | + if (!(stat(bs->filename, &st) == 0) || !S_ISREG(st.st_mode)) { | 57 | + !!strncmp(footer->creator_app, "CTXS", 4) && |
51 | + error_setg_errno(errp, ENOENT, "%s is not a regular file", | 58 | + !!memcmp(footer->creator_app, "tap", 4)); |
52 | + bs->filename); | ||
53 | + return -ENOENT; | ||
54 | + } | ||
55 | + | ||
56 | + ret = unlink(bs->filename); | ||
57 | + if (ret < 0) { | ||
58 | + ret = -errno; | ||
59 | + error_setg_errno(errp, -ret, "Error when deleting file %s", | ||
60 | + bs->filename); | ||
61 | + } | ||
62 | + | ||
63 | + return ret; | ||
64 | +} | 59 | +} |
65 | + | 60 | + |
66 | /* | 61 | static int vpc_open(BlockDriverState *bs, QDict *options, int flags, |
67 | * Find allocation range in @bs around offset @start. | 62 | Error **errp) |
68 | * May change underlying file descriptor's file offset. | 63 | { |
69 | @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = { | 64 | @@ -XXX,XX +XXX,XX @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags, |
70 | .bdrv_co_block_status = raw_co_block_status, | 65 | bs->total_sectors = (int64_t) |
71 | .bdrv_co_invalidate_cache = raw_co_invalidate_cache, | 66 | be16_to_cpu(footer->cyls) * footer->heads * footer->secs_per_cyl; |
72 | .bdrv_co_pwrite_zeroes = raw_co_pwrite_zeroes, | 67 | |
73 | + .bdrv_co_delete_file = raw_co_delete_file, | 68 | - /* Microsoft Virtual PC and Microsoft Hyper-V produce and read |
74 | 69 | - * VHD image sizes differently. VPC will rely on CHS geometry, | |
75 | .bdrv_co_preadv = raw_co_preadv, | 70 | - * while Hyper-V and disk2vhd use the size specified in the footer. |
76 | .bdrv_co_pwritev = raw_co_pwritev, | 71 | - * |
72 | - * We use a couple of approaches to try and determine the correct method: | ||
73 | - * look at the Creator App field, and look for images that have CHS | ||
74 | - * geometry that is the maximum value. | ||
75 | - * | ||
76 | - * If the CHS geometry is the maximum CHS geometry, then we assume that | ||
77 | - * the size is the footer->current_size to avoid truncation. Otherwise, | ||
78 | - * we follow the table based on footer->creator_app: | ||
79 | - * | ||
80 | - * Known creator apps: | ||
81 | - * 'vpc ' : CHS Virtual PC (uses disk geometry) | ||
82 | - * 'qemu' : CHS QEMU (uses disk geometry) | ||
83 | - * 'qem2' : current_size QEMU (uses current_size) | ||
84 | - * 'win ' : current_size Hyper-V | ||
85 | - * 'd2v ' : current_size Disk2vhd | ||
86 | - * 'tap\0' : current_size XenServer | ||
87 | - * 'CTXS' : current_size XenConverter | ||
88 | - * | ||
89 | - * The user can override the table values via drive options, however | ||
90 | - * even with an override we will still use current_size for images | ||
91 | - * that have CHS geometry of the maximum size. | ||
92 | - */ | ||
93 | - use_chs = (!!strncmp(footer->creator_app, "win ", 4) && | ||
94 | - !!strncmp(footer->creator_app, "qem2", 4) && | ||
95 | - !!strncmp(footer->creator_app, "d2v ", 4) && | ||
96 | - !!strncmp(footer->creator_app, "CTXS", 4) && | ||
97 | - !!memcmp(footer->creator_app, "tap", 4)) || s->force_use_chs; | ||
98 | + /* Use CHS or current_size to determine the image size. */ | ||
99 | + use_chs = vpc_ignore_current_size(footer) || s->force_use_chs; | ||
100 | |||
101 | if (!use_chs || bs->total_sectors == VHD_MAX_GEOMETRY || s->force_use_sz) { | ||
102 | bs->total_sectors = be64_to_cpu(footer->current_size) / | ||
77 | -- | 103 | -- |
78 | 2.20.1 | 104 | 2.48.1 |
79 | 105 | ||
80 | 106 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Vitaly Kuznetsov <vkuznets@redhat.com> | ||
1 | 2 | ||
3 | It was found that 'qemu-nbd' is not able to work with some disk images | ||
4 | exported from Azure. Looking at the 512b footer (which contains VPC | ||
5 | metadata): | ||
6 | |||
7 | 00000000 63 6f 6e 65 63 74 69 78 00 00 00 02 00 01 00 00 |conectix........| | ||
8 | 00000010 ff ff ff ff ff ff ff ff 2e c7 9b 96 77 61 00 00 |............wa..| | ||
9 | 00000020 00 07 00 00 57 69 32 6b 00 00 00 01 40 00 00 00 |....Wi2k....@...| | ||
10 | 00000030 00 00 00 01 40 00 00 00 28 a2 10 3f 00 00 00 02 |....@...(..?....| | ||
11 | 00000040 ff ff e7 47 8c 54 df 94 bd 35 71 4c 94 5f e5 44 |...G.T...5qL._.D| | ||
12 | 00000050 44 53 92 1a 00 00 00 00 00 00 00 00 00 00 00 00 |DS..............| | ||
13 | 00000060 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................| | ||
14 | |||
15 | we can see that Azure uses a different 'Creator application' -- | ||
16 | 'wa\0\0' (offset 0x1c, likely reads as 'Windows Azure') and QEMU uses this | ||
17 | field to determine how it can get image size. Apparently, Azure uses 'new' | ||
18 | method, just like Hyper-V. | ||
19 | |||
20 | Overall, it seems that only VPC and old QEMUs need to be ignored as all new | ||
21 | creator apps seem to have reliable current_size. Invert the logic and make | ||
22 | 'current_size' method the default to avoid adding every new creator app to | ||
23 | the list. | ||
24 | |||
25 | Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> | ||
26 | Message-ID: <20241212134504.1983757-3-vkuznets@redhat.com> | ||
27 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
28 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
29 | --- | ||
30 | block/vpc.c | 8 +++----- | ||
31 | 1 file changed, 3 insertions(+), 5 deletions(-) | ||
32 | |||
33 | diff --git a/block/vpc.c b/block/vpc.c | ||
34 | index XXXXXXX..XXXXXXX 100644 | ||
35 | --- a/block/vpc.c | ||
36 | +++ b/block/vpc.c | ||
37 | @@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts, | ||
38 | * 'd2v ' : current_size Disk2vhd | ||
39 | * 'tap\0' : current_size XenServer | ||
40 | * 'CTXS' : current_size XenConverter | ||
41 | + * 'wa\0\0': current_size Azure | ||
42 | * | ||
43 | * The user can override the table values via drive options, however | ||
44 | * even with an override we will still use current_size for images | ||
45 | @@ -XXX,XX +XXX,XX @@ static void vpc_parse_options(BlockDriverState *bs, QemuOpts *opts, | ||
46 | */ | ||
47 | static bool vpc_ignore_current_size(VHDFooter *footer) | ||
48 | { | ||
49 | - return !!strncmp(footer->creator_app, "win ", 4) && | ||
50 | - !!strncmp(footer->creator_app, "qem2", 4) && | ||
51 | - !!strncmp(footer->creator_app, "d2v ", 4) && | ||
52 | - !!strncmp(footer->creator_app, "CTXS", 4) && | ||
53 | - !!memcmp(footer->creator_app, "tap", 4)); | ||
54 | + return !strncmp(footer->creator_app, "vpc ", 4) || | ||
55 | + !strncmp(footer->creator_app, "qemu", 4); | ||
56 | } | ||
57 | |||
58 | static int vpc_open(BlockDriverState *bs, QDict *options, int flags, | ||
59 | -- | ||
60 | 2.48.1 | diff view generated by jsdifflib |
1 | This patch adds test cases for attaching the backing chain to a mirror | 1 | From: Philippe Mathieu-Daudé <philmd@linaro.org> |
---|---|---|---|
2 | job target right before finalising the job, where the image is in a | ||
3 | non-mainloop AioContext (i.e. the backing chain needs to be moved to the | ||
4 | AioContext of the mirror target). | ||
5 | 2 | ||
6 | This requires switching the test case from virtio-blk to virtio-scsi | 3 | Expose the method docstring in the header, and mention |
7 | because virtio-blk only actually starts using the iothreads when the | 4 | returned value must be free'd by caller. |
8 | guest driver initialises the device (which never happens in a test case | ||
9 | without a guest OS). virtio-scsi always keeps its block nodes in the | ||
10 | AioContext of the the requested iothread without guest interaction. | ||
11 | 5 | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 6 | Reported-by: Fabiano Rosas <farosas@suse.de> |
13 | Message-Id: <20200310113831.27293-7-kwolf@redhat.com> | 7 | Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
14 | Reviewed-by: Peter Krempa <pkrempa@redhat.com> | 8 | Message-ID: <20241111170333.43833-2-philmd@linaro.org> |
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
16 | --- | 10 | --- |
17 | tests/qemu-iotests/155 | 32 +++++++++++++++++++++++--------- | 11 | include/system/block-backend-io.h | 7 +++++++ |
18 | tests/qemu-iotests/155.out | 4 ++-- | 12 | block/block-backend.c | 12 ++++++++---- |
19 | 2 files changed, 25 insertions(+), 11 deletions(-) | 13 | 2 files changed, 15 insertions(+), 4 deletions(-) |
20 | 14 | ||
21 | diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155 | 15 | diff --git a/include/system/block-backend-io.h b/include/system/block-backend-io.h |
22 | index XXXXXXX..XXXXXXX 100755 | ||
23 | --- a/tests/qemu-iotests/155 | ||
24 | +++ b/tests/qemu-iotests/155 | ||
25 | @@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt) | ||
26 | # chain opened right away. If False, blockdev-add | ||
27 | # opens it without a backing file and job completion | ||
28 | # is supposed to open the backing chain. | ||
29 | +# use_iothread: If True, an iothread is configured for the virtio-blk device | ||
30 | +# that uses the image being mirrored | ||
31 | |||
32 | class BaseClass(iotests.QMPTestCase): | ||
33 | target_blockdev_backing = None | ||
34 | target_real_backing = None | ||
35 | target_open_with_backing = True | ||
36 | + use_iothread = False | ||
37 | |||
38 | def setUp(self): | ||
39 | qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K') | ||
40 | @@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase): | ||
41 | 'file': {'driver': 'file', | ||
42 | 'filename': source_img}} | ||
43 | self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev)) | ||
44 | - self.vm.add_device('virtio-blk,id=qdev0,drive=source') | ||
45 | + | ||
46 | + if self.use_iothread: | ||
47 | + self.vm.add_object('iothread,id=iothread0') | ||
48 | + iothread = ",iothread=iothread0" | ||
49 | + else: | ||
50 | + iothread = "" | ||
51 | + | ||
52 | + self.vm.add_device('virtio-scsi%s' % iothread) | ||
53 | + self.vm.add_device('scsi-hd,id=qdev0,drive=source') | ||
54 | + | ||
55 | self.vm.launch() | ||
56 | |||
57 | self.assertIntactSourceBackingChain() | ||
58 | @@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass): | ||
59 | def testFull(self): | ||
60 | self.runMirror('full') | ||
61 | |||
62 | - node = self.findBlockNode('target', | ||
63 | - '/machine/peripheral/qdev0/virtio-backend') | ||
64 | + node = self.findBlockNode('target', 'qdev0') | ||
65 | self.assertCorrectBackingImage(node, None) | ||
66 | self.assertIntactSourceBackingChain() | ||
67 | |||
68 | def testTop(self): | ||
69 | self.runMirror('top') | ||
70 | |||
71 | - node = self.findBlockNode('target', | ||
72 | - '/machine/peripheral/qdev0/virtio-backend') | ||
73 | + node = self.findBlockNode('target', 'qdev0') | ||
74 | self.assertCorrectBackingImage(node, back2_img) | ||
75 | self.assertIntactSourceBackingChain() | ||
76 | |||
77 | def testNone(self): | ||
78 | self.runMirror('none') | ||
79 | |||
80 | - node = self.findBlockNode('target', | ||
81 | - '/machine/peripheral/qdev0/virtio-backend') | ||
82 | + node = self.findBlockNode('target', 'qdev0') | ||
83 | self.assertCorrectBackingImage(node, source_img) | ||
84 | self.assertIntactSourceBackingChain() | ||
85 | |||
86 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorReopen(MirrorBaseClass): | ||
87 | backing="backing") | ||
88 | self.assert_qmp(result, 'return', {}) | ||
89 | |||
90 | +class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen): | ||
91 | + use_iothread = True | ||
92 | + | ||
93 | # Attach the backing chain only during completion, with blockdev-snapshot | ||
94 | class TestBlockdevMirrorSnapshot(MirrorBaseClass): | ||
95 | cmd = 'blockdev-mirror' | ||
96 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorSnapshot(MirrorBaseClass): | ||
97 | overlay="target") | ||
98 | self.assert_qmp(result, 'return', {}) | ||
99 | |||
100 | +class TestBlockdevMirrorSnapshotIothread(TestBlockdevMirrorSnapshot): | ||
101 | + use_iothread = True | ||
102 | + | ||
103 | class TestCommit(BaseClass): | ||
104 | existing = False | ||
105 | |||
106 | @@ -XXX,XX +XXX,XX @@ class TestCommit(BaseClass): | ||
107 | |||
108 | self.vm.event_wait('BLOCK_JOB_COMPLETED') | ||
109 | |||
110 | - node = self.findBlockNode(None, | ||
111 | - '/machine/peripheral/qdev0/virtio-backend') | ||
112 | + node = self.findBlockNode(None, 'qdev0') | ||
113 | self.assert_qmp(node, 'image' + '/backing-image' * 0 + '/filename', | ||
114 | back1_img) | ||
115 | self.assert_qmp(node, 'image' + '/backing-image' * 1 + '/filename', | ||
116 | diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out | ||
117 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
118 | --- a/tests/qemu-iotests/155.out | 17 | --- a/include/system/block-backend-io.h |
119 | +++ b/tests/qemu-iotests/155.out | 18 | +++ b/include/system/block-backend-io.h |
120 | @@ -XXX,XX +XXX,XX @@ | 19 | @@ -XXX,XX +XXX,XX @@ void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow); |
121 | -......................... | 20 | void blk_set_disable_request_queuing(BlockBackend *blk, bool disable); |
122 | +............................... | 21 | bool blk_iostatus_is_enabled(const BlockBackend *blk); |
123 | ---------------------------------------------------------------------- | 22 | |
124 | -Ran 25 tests | 23 | +/* |
125 | +Ran 31 tests | 24 | + * Return the qdev ID, or if no ID is assigned the QOM path, |
126 | 25 | + * of the block device attached to the BlockBackend. | |
127 | OK | 26 | + * |
27 | + * The caller is responsible for releasing the value returned | ||
28 | + * with g_free() after use. | ||
29 | + */ | ||
30 | char *blk_get_attached_dev_id(BlockBackend *blk); | ||
31 | |||
32 | BlockAIOCB *blk_aio_pwrite_zeroes(BlockBackend *blk, int64_t offset, | ||
33 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
34 | index XXXXXXX..XXXXXXX 100644 | ||
35 | --- a/block/block-backend.c | ||
36 | +++ b/block/block-backend.c | ||
37 | @@ -XXX,XX +XXX,XX @@ DeviceState *blk_get_attached_dev(BlockBackend *blk) | ||
38 | return blk->dev; | ||
39 | } | ||
40 | |||
41 | +/* | ||
42 | + * The caller is responsible for releasing the value returned | ||
43 | + * with g_free() after use. | ||
44 | + */ | ||
45 | static char *blk_get_attached_dev_id_or_path(BlockBackend *blk, bool want_id) | ||
46 | { | ||
47 | DeviceState *dev = blk->dev; | ||
48 | @@ -XXX,XX +XXX,XX @@ static char *blk_get_attached_dev_id_or_path(BlockBackend *blk, bool want_id) | ||
49 | return object_get_canonical_path(OBJECT(dev)) ?: g_strdup(""); | ||
50 | } | ||
51 | |||
52 | -/* | ||
53 | - * Return the qdev ID, or if no ID is assigned the QOM path, of the block | ||
54 | - * device attached to the BlockBackend. | ||
55 | - */ | ||
56 | char *blk_get_attached_dev_id(BlockBackend *blk) | ||
57 | { | ||
58 | return blk_get_attached_dev_id_or_path(blk, true); | ||
59 | } | ||
60 | |||
61 | +/* | ||
62 | + * The caller is responsible for releasing the value returned | ||
63 | + * with g_free() after use. | ||
64 | + */ | ||
65 | static char *blk_get_attached_dev_path(BlockBackend *blk) | ||
66 | { | ||
67 | return blk_get_attached_dev_id_or_path(blk, false); | ||
128 | -- | 68 | -- |
129 | 2.20.1 | 69 | 2.48.1 |
130 | 70 | ||
131 | 71 | diff view generated by jsdifflib |
1 | From: Pan Nengyuan <pannengyuan@huawei.com> | 1 | From: Fabiano Rosas <farosas@suse.de> |
---|---|---|---|
2 | 2 | ||
3 | 'type/id' forgot to free in qmp_object_add, this patch fix that. | 3 | ASAN detected a leak when running the ahci-test |
4 | /ahci/io/dma/lba28/retry: | ||
4 | 5 | ||
5 | The leak stack: | 6 | Direct leak of 35 byte(s) in 1 object(s) allocated from: |
6 | Direct leak of 84 byte(s) in 6 object(s) allocated from: | 7 | #0 in malloc |
7 | #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768) | 8 | #1 in __vasprintf_internal |
8 | #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445) | 9 | #2 in vasprintf |
9 | #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92) | 10 | #3 in g_vasprintf |
10 | #3 0x56344954e692 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:258 | 11 | #4 in g_strdup_vprintf |
11 | #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132 | 12 | #5 in g_strdup_printf |
12 | #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175 | 13 | #6 in object_get_canonical_path ../qom/object.c:2096:19 |
13 | #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145 | 14 | #7 in blk_get_attached_dev_id_or_path ../block/block-backend.c:1033:12 |
14 | #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234 | 15 | #8 in blk_get_attached_dev_path ../block/block-backend.c:1047:12 |
15 | #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136 | 16 | #9 in send_qmp_error_event ../block/block-backend.c:2140:36 |
17 | #10 in blk_error_action ../block/block-backend.c:2172:9 | ||
18 | #11 in ide_handle_rw_error ../hw/ide/core.c:875:5 | ||
19 | #12 in ide_dma_cb ../hw/ide/core.c:894:13 | ||
20 | #13 in dma_complete ../system/dma-helpers.c:107:9 | ||
21 | #14 in dma_blk_cb ../system/dma-helpers.c:129:9 | ||
22 | #15 in blk_aio_complete ../block/block-backend.c:1552:9 | ||
23 | #16 in blk_aio_write_entry ../block/block-backend.c:1619:5 | ||
24 | #17 in coroutine_trampoline ../util/coroutine-ucontext.c:175:9 | ||
16 | 25 | ||
17 | Direct leak of 54 byte(s) in 6 object(s) allocated from: | 26 | Plug the leak by freeing the device path string. |
18 | #0 0x7fe2a5ebf768 in __interceptor_malloc (/lib64/libasan.so.5+0xef768) | ||
19 | #1 0x7fe2a5044445 in g_malloc (/lib64/libglib-2.0.so.0+0x52445) | ||
20 | #2 0x7fe2a505dd92 in g_strdup (/lib64/libglib-2.0.so.0+0x6bd92) | ||
21 | #3 0x56344954e6c4 in qmp_object_add /mnt/sdb/qemu-new/qemu_test/qemu/qom/qom-qmp-cmds.c:267 | ||
22 | #4 0x563449960f5a in do_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:132 | ||
23 | #5 0x563449960f5a in qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/qapi/qmp-dispatch.c:175 | ||
24 | #6 0x563449498a30 in monitor_qmp_dispatch /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:145 | ||
25 | #7 0x56344949a64f in monitor_qmp_bh_dispatcher /mnt/sdb/qemu-new/qemu_test/qemu/monitor/qmp.c:234 | ||
26 | #8 0x563449a92a3a in aio_bh_call /mnt/sdb/qemu-new/qemu_test/qemu/util/async.c:136 | ||
27 | 27 | ||
28 | Fixes: 5f07c4d60d091320186e7b0edaf9ed2cc16b2d1e | 28 | Signed-off-by: Fabiano Rosas <farosas@suse.de> |
29 | Reported-by: Euler Robot <euler.robot@huawei.com> | 29 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
30 | Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> | 30 | Message-ID: <20241111145214.8261-1-farosas@suse.de> |
31 | Message-Id: <20200310064640.5059-1-pannengyuan@huawei.com> | 31 | [PMD: Use g_autofree] |
32 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | 32 | Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
33 | Acked-by: Igor Mammedov <imammedo@redhat.com> | 33 | Message-ID: <20241111170333.43833-3-philmd@linaro.org> |
34 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 34 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
35 | --- | 35 | --- |
36 | qom/qom-qmp-cmds.c | 16 ++++++---------- | 36 | block/block-backend.c | 4 ++-- |
37 | 1 file changed, 6 insertions(+), 10 deletions(-) | 37 | 1 file changed, 2 insertions(+), 2 deletions(-) |
38 | 38 | ||
39 | diff --git a/qom/qom-qmp-cmds.c b/qom/qom-qmp-cmds.c | 39 | diff --git a/block/block-backend.c b/block/block-backend.c |
40 | index XXXXXXX..XXXXXXX 100644 | 40 | index XXXXXXX..XXXXXXX 100644 |
41 | --- a/qom/qom-qmp-cmds.c | 41 | --- a/block/block-backend.c |
42 | +++ b/qom/qom-qmp-cmds.c | 42 | +++ b/block/block-backend.c |
43 | @@ -XXX,XX +XXX,XX @@ void qmp_object_add(QDict *qdict, QObject **ret_data, Error **errp) | 43 | @@ -XXX,XX +XXX,XX @@ static void send_qmp_error_event(BlockBackend *blk, |
44 | QDict *pdict; | 44 | { |
45 | Visitor *v; | 45 | IoOperationType optype; |
46 | Object *obj; | 46 | BlockDriverState *bs = blk_bs(blk); |
47 | - const char *type; | 47 | + g_autofree char *path = blk_get_attached_dev_path(blk); |
48 | - const char *id; | 48 | |
49 | + g_autofree char *type = NULL; | 49 | optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE; |
50 | + g_autofree char *id = NULL; | 50 | - qapi_event_send_block_io_error(blk_name(blk), |
51 | 51 | - blk_get_attached_dev_path(blk), | |
52 | - type = qdict_get_try_str(qdict, "qom-type"); | 52 | + qapi_event_send_block_io_error(blk_name(blk), path, |
53 | + type = g_strdup(qdict_get_try_str(qdict, "qom-type")); | 53 | bs ? bdrv_get_node_name(bs) : NULL, optype, |
54 | if (!type) { | 54 | action, blk_iostatus_is_enabled(blk), |
55 | error_setg(errp, QERR_MISSING_PARAMETER, "qom-type"); | 55 | error == ENOSPC, strerror(error)); |
56 | return; | ||
57 | - } else { | ||
58 | - type = g_strdup(type); | ||
59 | - qdict_del(qdict, "qom-type"); | ||
60 | } | ||
61 | + qdict_del(qdict, "qom-type"); | ||
62 | |||
63 | - id = qdict_get_try_str(qdict, "id"); | ||
64 | + id = g_strdup(qdict_get_try_str(qdict, "id")); | ||
65 | if (!id) { | ||
66 | error_setg(errp, QERR_MISSING_PARAMETER, "id"); | ||
67 | return; | ||
68 | - } else { | ||
69 | - id = g_strdup(id); | ||
70 | - qdict_del(qdict, "id"); | ||
71 | } | ||
72 | + qdict_del(qdict, "id"); | ||
73 | |||
74 | props = qdict_get(qdict, "props"); | ||
75 | if (props) { | ||
76 | -- | 56 | -- |
77 | 2.20.1 | 57 | 2.48.1 |
78 | 58 | ||
79 | 59 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Xu <peterx@redhat.com> | ||
1 | 2 | ||
3 | It's easier for either debugging plugin errors, or issue reports. | ||
4 | |||
5 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
6 | Message-ID: <20241212204801.1420528-2-peterx@redhat.com> | ||
7 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | scripts/qemu-gdb.py | 2 ++ | ||
11 | 1 file changed, 2 insertions(+) | ||
12 | |||
13 | diff --git a/scripts/qemu-gdb.py b/scripts/qemu-gdb.py | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/scripts/qemu-gdb.py | ||
16 | +++ b/scripts/qemu-gdb.py | ||
17 | @@ -XXX,XX +XXX,XX @@ def __init__(self): | ||
18 | # Default to silently passing through SIGUSR1, because QEMU sends it | ||
19 | # to itself a lot. | ||
20 | gdb.execute('handle SIGUSR1 pass noprint nostop') | ||
21 | +# Always print full stack for python errors, easier to debug and report issues | ||
22 | +gdb.execute('set python print-stack full') | ||
23 | -- | ||
24 | 2.48.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Xu <peterx@redhat.com> | ||
1 | 2 | ||
3 | There're a bunch of code trying to fetch fs_base in different ways. IIUC | ||
4 | the simplest way instead is "$fs_base". It also has the benefit that it'll | ||
5 | work for both live gdb session or coredumps. | ||
6 | |||
7 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
8 | Message-ID: <20241212204801.1420528-3-peterx@redhat.com> | ||
9 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | scripts/qemugdb/coroutine.py | 23 ++--------------------- | ||
13 | 1 file changed, 2 insertions(+), 21 deletions(-) | ||
14 | |||
15 | diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/scripts/qemugdb/coroutine.py | ||
18 | +++ b/scripts/qemugdb/coroutine.py | ||
19 | @@ -XXX,XX +XXX,XX @@ | ||
20 | |||
21 | VOID_PTR = gdb.lookup_type('void').pointer() | ||
22 | |||
23 | -def get_fs_base(): | ||
24 | - '''Fetch %fs base value using arch_prctl(ARCH_GET_FS). This is | ||
25 | - pthread_self().''' | ||
26 | - # %rsp - 120 is scratch space according to the SystemV ABI | ||
27 | - old = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)') | ||
28 | - gdb.execute('call (int)arch_prctl(0x1003, $rsp - 120)', False, True) | ||
29 | - fs_base = gdb.parse_and_eval('*(uint64_t*)($rsp - 120)') | ||
30 | - gdb.execute('set *(uint64_t*)($rsp - 120) = %s' % old, False, True) | ||
31 | - return fs_base | ||
32 | - | ||
33 | def pthread_self(): | ||
34 | - '''Fetch pthread_self() from the glibc start_thread function.''' | ||
35 | - f = gdb.newest_frame() | ||
36 | - while f.name() != 'start_thread': | ||
37 | - f = f.older() | ||
38 | - if f is None: | ||
39 | - return get_fs_base() | ||
40 | - | ||
41 | - try: | ||
42 | - return f.read_var("arg") | ||
43 | - except ValueError: | ||
44 | - return get_fs_base() | ||
45 | + '''Fetch the base address of TLS.''' | ||
46 | + return gdb.parse_and_eval("$fs_base") | ||
47 | |||
48 | def get_glibc_pointer_guard(): | ||
49 | '''Fetch glibc pointer guard value''' | ||
50 | -- | ||
51 | 2.48.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Xu <peterx@redhat.com> | ||
1 | 2 | ||
3 | Dumping coroutines don't yet work with coredumps. Let's make it work. | ||
4 | |||
5 | We still kept most of the old code because they can be either more | ||
6 | flexible, or prettier. Only add the fallbacks when they stop working. | ||
7 | |||
8 | Currently the raw unwind is pretty ugly, but it works, like this: | ||
9 | |||
10 | (gdb) qemu bt | ||
11 | #0 process_incoming_migration_co (opaque=0x0) at ../migration/migration.c:788 | ||
12 | #1 0x000055ae6c0dc4d9 in coroutine_trampoline (i0=-1711718576, i1=21934) at ../util/coroutine-ucontext.c:175 | ||
13 | #2 0x00007f9f59d72f40 in ??? () at /lib64/libc.so.6 | ||
14 | #3 0x00007ffd549214a0 in ??? () | ||
15 | #4 0x0000000000000000 in ??? () | ||
16 | Coroutine at 0x7f9f4c57c748: | ||
17 | #0 0x55ae6c0dc9a8 in qemu_coroutine_switch<+120> () at ../util/coroutine-ucontext.c:321 | ||
18 | #1 0x55ae6c0da2f8 in qemu_aio_coroutine_enter<+356> () at ../util/qemu-coroutine.c:293 | ||
19 | #2 0x55ae6c0da3f1 in qemu_coroutine_enter<+34> () at ../util/qemu-coroutine.c:316 | ||
20 | #3 0x55ae6baf775e in migration_incoming_process<+43> () at ../migration/migration.c:876 | ||
21 | #4 0x55ae6baf7ab4 in migration_ioc_process_incoming<+490> () at ../migration/migration.c:1008 | ||
22 | #5 0x55ae6bae9ae7 in migration_channel_process_incoming<+145> () at ../migration/channel.c:45 | ||
23 | #6 0x55ae6bb18e35 in socket_accept_incoming_migration<+118> () at ../migration/socket.c:132 | ||
24 | #7 0x55ae6be939ef in qio_net_listener_channel_func<+131> () at ../io/net-listener.c:54 | ||
25 | #8 0x55ae6be8ce1a in qio_channel_fd_source_dispatch<+78> () at ../io/channel-watch.c:84 | ||
26 | #9 0x7f9f5b26728c in g_main_context_dispatch_unlocked.lto_priv<+315> () | ||
27 | #10 0x7f9f5b267555 in g_main_context_dispatch<+36> () | ||
28 | #11 0x55ae6c0d91a7 in glib_pollfds_poll<+90> () at ../util/main-loop.c:287 | ||
29 | #12 0x55ae6c0d9235 in os_host_main_loop_wait<+128> () at ../util/main-loop.c:310 | ||
30 | #13 0x55ae6c0d9364 in main_loop_wait<+203> () at ../util/main-loop.c:589 | ||
31 | #14 0x55ae6bac212a in qemu_main_loop<+41> () at ../system/runstate.c:835 | ||
32 | #15 0x55ae6bfdf522 in qemu_default_main<+19> () at ../system/main.c:37 | ||
33 | #16 0x55ae6bfdf55f in main<+40> () at ../system/main.c:48 | ||
34 | #17 0x7f9f59d42248 in __libc_start_call_main<+119> () | ||
35 | #18 0x7f9f59d4230b in __libc_start_main_impl<+138> () | ||
36 | |||
37 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
38 | Message-ID: <20241212204801.1420528-4-peterx@redhat.com> | ||
39 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
40 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
41 | --- | ||
42 | scripts/qemugdb/coroutine.py | 79 +++++++++++++++++++++++++++++++++--- | ||
43 | 1 file changed, 73 insertions(+), 6 deletions(-) | ||
44 | |||
45 | diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py | ||
46 | index XXXXXXX..XXXXXXX 100644 | ||
47 | --- a/scripts/qemugdb/coroutine.py | ||
48 | +++ b/scripts/qemugdb/coroutine.py | ||
49 | @@ -XXX,XX +XXX,XX @@ def get_jmpbuf_regs(jmpbuf): | ||
50 | 'r15': jmpbuf[JB_R15], | ||
51 | 'rip': glibc_ptr_demangle(jmpbuf[JB_PC], pointer_guard) } | ||
52 | |||
53 | -def bt_jmpbuf(jmpbuf): | ||
54 | - '''Backtrace a jmpbuf''' | ||
55 | - regs = get_jmpbuf_regs(jmpbuf) | ||
56 | +def symbol_lookup(addr): | ||
57 | + # Example: "__clone3 + 44 in section .text of /lib64/libc.so.6" | ||
58 | + result = gdb.execute(f"info symbol {hex(addr)}", to_string=True).strip() | ||
59 | + try: | ||
60 | + if "+" in result: | ||
61 | + (func, result) = result.split(" + ") | ||
62 | + (offset, result) = result.split(" in ") | ||
63 | + else: | ||
64 | + offset = "0" | ||
65 | + (func, result) = result.split(" in ") | ||
66 | + func_str = f"{func}<+{offset}> ()" | ||
67 | + except: | ||
68 | + return f"??? ({result})" | ||
69 | + | ||
70 | + # Example: Line 321 of "../util/coroutine-ucontext.c" starts at address | ||
71 | + # 0x55cf3894d993 <qemu_coroutine_switch+99> and ends at 0x55cf3894d9ab | ||
72 | + # <qemu_coroutine_switch+123>. | ||
73 | + result = gdb.execute(f"info line *{hex(addr)}", to_string=True).strip() | ||
74 | + if not result.startswith("Line "): | ||
75 | + return func_str | ||
76 | + result = result[5:] | ||
77 | + | ||
78 | + try: | ||
79 | + result = result.split(" starts ")[0] | ||
80 | + (line, path) = result.split(" of ") | ||
81 | + path = path.replace("\"", "") | ||
82 | + except: | ||
83 | + return func_str | ||
84 | + | ||
85 | + return f"{func_str} at {path}:{line}" | ||
86 | + | ||
87 | +def dump_backtrace(regs): | ||
88 | + ''' | ||
89 | + Backtrace dump with raw registers, mimic GDB command 'bt'. | ||
90 | + ''' | ||
91 | + # Here only rbp and rip that matter.. | ||
92 | + rbp = regs['rbp'] | ||
93 | + rip = regs['rip'] | ||
94 | + i = 0 | ||
95 | + | ||
96 | + while rbp: | ||
97 | + # For all return addresses on stack, we want to look up symbol/line | ||
98 | + # on the CALL command, because the return address is the next | ||
99 | + # instruction instead of the CALL. Here -1 would work for any | ||
100 | + # sized CALL instruction. | ||
101 | + print(f"#{i} {hex(rip)} in {symbol_lookup(rip if i == 0 else rip-1)}") | ||
102 | + rip = gdb.parse_and_eval(f"*(uint64_t *)(uint64_t)({hex(rbp)} + 8)") | ||
103 | + rbp = gdb.parse_and_eval(f"*(uint64_t *)(uint64_t)({hex(rbp)})") | ||
104 | + i += 1 | ||
105 | + | ||
106 | +def dump_backtrace_live(regs): | ||
107 | + ''' | ||
108 | + Backtrace dump with gdb's 'bt' command, only usable in a live session. | ||
109 | + ''' | ||
110 | old = dict() | ||
111 | |||
112 | # remember current stack frame and select the topmost | ||
113 | @@ -XXX,XX +XXX,XX @@ def bt_jmpbuf(jmpbuf): | ||
114 | |||
115 | selected_frame.select() | ||
116 | |||
117 | +def bt_jmpbuf(jmpbuf): | ||
118 | + '''Backtrace a jmpbuf''' | ||
119 | + regs = get_jmpbuf_regs(jmpbuf) | ||
120 | + try: | ||
121 | + # This reuses gdb's "bt" command, which can be slightly prettier | ||
122 | + # but only works with live sessions. | ||
123 | + dump_backtrace_live(regs) | ||
124 | + except: | ||
125 | + # If above doesn't work, fallback to poor man's unwind | ||
126 | + dump_backtrace(regs) | ||
127 | + | ||
128 | def co_cast(co): | ||
129 | return co.cast(gdb.lookup_type('CoroutineUContext').pointer()) | ||
130 | |||
131 | @@ -XXX,XX +XXX,XX @@ def invoke(self, arg, from_tty): | ||
132 | |||
133 | gdb.execute("bt") | ||
134 | |||
135 | - if gdb.parse_and_eval("qemu_in_coroutine()") == False: | ||
136 | - return | ||
137 | + try: | ||
138 | + # This only works with a live session | ||
139 | + co_ptr = gdb.parse_and_eval("qemu_coroutine_self()") | ||
140 | + except: | ||
141 | + # Fallback to use hard-coded ucontext vars if it's coredump | ||
142 | + co_ptr = gdb.parse_and_eval("co_tls_current") | ||
143 | |||
144 | - co_ptr = gdb.parse_and_eval("qemu_coroutine_self()") | ||
145 | + if co_ptr == False: | ||
146 | + return | ||
147 | |||
148 | while True: | ||
149 | co = co_cast(co_ptr) | ||
150 | -- | ||
151 | 2.48.1 | diff view generated by jsdifflib |
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | 1 | From: Peter Krempa <pkrempa@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The socket_scm_helper path got corrupted during the mechanical | 3 | Commit 7452162adec25c10 introduced 'qom-path' argument to BLOCK_IO_ERROR |
4 | refactor moving the qtests files into their own sub-directory. | 4 | event but when the event is instantiated in 'send_qmp_error_event()' the |
5 | arguments for 'device' and 'qom_path' in | ||
6 | qapi_event_send_block_io_error() were reversed : | ||
5 | 7 | ||
6 | Fixes: 1e8a1fae7 ("test: Move qtests to a separate directory") | 8 | Generated code for sending event: |
7 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 9 | |
8 | Message-Id: <20200306165751.18986-1-philmd@redhat.com> | 10 | void qapi_event_send_block_io_error(const char *qom_path, |
9 | Reviewed-by: Laurent Vivier <laurent@vivier.eu> | 11 | const char *device, |
12 | const char *node_name, | ||
13 | IoOperationType operation, | ||
14 | [...] | ||
15 | |||
16 | Call inside send_qmp_error_event(): | ||
17 | |||
18 | qapi_event_send_block_io_error(blk_name(blk), | ||
19 | blk_get_attached_dev_path(blk), | ||
20 | bs ? bdrv_get_node_name(bs) : NULL, optype, | ||
21 | [...] | ||
22 | |||
23 | This results into reporting the QOM path as the device alias and vice | ||
24 | versa which in turn breaks libvirt, which expects the device alias being | ||
25 | either a valid alias or empty (which would make libvirt do the lookup by | ||
26 | node-name instead). | ||
27 | |||
28 | Cc: qemu-stable@nongnu.org | ||
29 | Fixes: 7452162adec2 ("qapi: add qom-path to BLOCK_IO_ERROR event") | ||
30 | Signed-off-by: Peter Krempa <pkrempa@redhat.com> | ||
31 | Message-ID: <09728d784888b38d7a8f09ee5e9e9c542c875e1e.1737973614.git.pkrempa@redhat.com> | ||
32 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> | ||
33 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 34 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
11 | --- | 35 | --- |
12 | tests/Makefile.include | 1 + | 36 | block/block-backend.c | 2 +- |
13 | tests/qtest/Makefile.include | 1 - | 37 | 1 file changed, 1 insertion(+), 1 deletion(-) |
14 | 2 files changed, 1 insertion(+), 1 deletion(-) | ||
15 | 38 | ||
16 | diff --git a/tests/Makefile.include b/tests/Makefile.include | 39 | diff --git a/block/block-backend.c b/block/block-backend.c |
17 | index XXXXXXX..XXXXXXX 100644 | 40 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/tests/Makefile.include | 41 | --- a/block/block-backend.c |
19 | +++ b/tests/Makefile.include | 42 | +++ b/block/block-backend.c |
20 | @@ -XXX,XX +XXX,XX @@ include $(SRC_PATH)/tests/qtest/Makefile.include | 43 | @@ -XXX,XX +XXX,XX @@ static void send_qmp_error_event(BlockBackend *blk, |
21 | tests/test-qga$(EXESUF): qemu-ga$(EXESUF) | 44 | g_autofree char *path = blk_get_attached_dev_path(blk); |
22 | tests/test-qga$(EXESUF): tests/test-qga.o $(qtest-obj-y) | 45 | |
23 | tests/vhost-user-bridge$(EXESUF): tests/vhost-user-bridge.o $(test-util-obj-y) libvhost-user.a | 46 | optype = is_read ? IO_OPERATION_TYPE_READ : IO_OPERATION_TYPE_WRITE; |
24 | +tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o | 47 | - qapi_event_send_block_io_error(blk_name(blk), path, |
25 | 48 | + qapi_event_send_block_io_error(path, blk_name(blk), | |
26 | SPEED = quick | 49 | bs ? bdrv_get_node_name(bs) : NULL, optype, |
27 | 50 | action, blk_iostatus_is_enabled(blk), | |
28 | diff --git a/tests/qtest/Makefile.include b/tests/qtest/Makefile.include | 51 | error == ENOSPC, strerror(error)); |
29 | index XXXXXXX..XXXXXXX 100644 | ||
30 | --- a/tests/qtest/Makefile.include | ||
31 | +++ b/tests/qtest/Makefile.include | ||
32 | @@ -XXX,XX +XXX,XX @@ tests/qtest/usb-hcd-ehci-test$(EXESUF): tests/qtest/usb-hcd-ehci-test.o $(libqos | ||
33 | tests/qtest/usb-hcd-xhci-test$(EXESUF): tests/qtest/usb-hcd-xhci-test.o $(libqos-usb-obj-y) | ||
34 | tests/qtest/cpu-plug-test$(EXESUF): tests/qtest/cpu-plug-test.o | ||
35 | tests/qtest/migration-test$(EXESUF): tests/qtest/migration-test.o tests/qtest/migration-helpers.o | ||
36 | -tests/qtest/qemu-iotests/qtest/socket_scm_helper$(EXESUF): tests/qtest/qemu-iotests/qtest/socket_scm_helper.o | ||
37 | tests/qtest/test-netfilter$(EXESUF): tests/qtest/test-netfilter.o $(qtest-obj-y) | ||
38 | tests/qtest/test-filter-mirror$(EXESUF): tests/qtest/test-filter-mirror.o $(qtest-obj-y) | ||
39 | tests/qtest/test-filter-redirector$(EXESUF): tests/qtest/test-filter-redirector.o $(qtest-obj-y) | ||
40 | -- | 52 | -- |
41 | 2.20.1 | 53 | 2.48.1 |
42 | 54 | ||
43 | 55 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | This allows querying from QMP (and also HMP) whether an image is | ||
2 | currently active or inactive (in the sense of BDRV_O_INACTIVE). | ||
1 | 3 | ||
4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
5 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
6 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
7 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
8 | Message-ID: <20250204211407.381505-2-kwolf@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | qapi/block-core.json | 6 +++++- | ||
12 | include/block/block-global-state.h | 3 +++ | ||
13 | block.c | 4 ++++ | ||
14 | block/monitor/block-hmp-cmds.c | 5 +++-- | ||
15 | block/qapi.c | 1 + | ||
16 | tests/qemu-iotests/184.out | 2 ++ | ||
17 | tests/qemu-iotests/191.out | 16 ++++++++++++++++ | ||
18 | tests/qemu-iotests/273.out | 5 +++++ | ||
19 | 8 files changed, 39 insertions(+), 3 deletions(-) | ||
20 | |||
21 | diff --git a/qapi/block-core.json b/qapi/block-core.json | ||
22 | index XXXXXXX..XXXXXXX 100644 | ||
23 | --- a/qapi/block-core.json | ||
24 | +++ b/qapi/block-core.json | ||
25 | @@ -XXX,XX +XXX,XX @@ | ||
26 | # @backing_file_depth: number of files in the backing file chain | ||
27 | # (since: 1.2) | ||
28 | # | ||
29 | +# @active: true if the backend is active; typical cases for inactive backends | ||
30 | +# are on the migration source instance after migration completes and on the | ||
31 | +# destination before it completes. (since: 10.0) | ||
32 | +# | ||
33 | # @encrypted: true if the backing device is encrypted | ||
34 | # | ||
35 | # @detect_zeroes: detect and optimize zero writes (Since 2.1) | ||
36 | @@ -XXX,XX +XXX,XX @@ | ||
37 | { 'struct': 'BlockDeviceInfo', | ||
38 | 'data': { 'file': 'str', '*node-name': 'str', 'ro': 'bool', 'drv': 'str', | ||
39 | '*backing_file': 'str', 'backing_file_depth': 'int', | ||
40 | - 'encrypted': 'bool', | ||
41 | + 'active': 'bool', 'encrypted': 'bool', | ||
42 | 'detect_zeroes': 'BlockdevDetectZeroesOptions', | ||
43 | 'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int', | ||
44 | 'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int', | ||
45 | diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h | ||
46 | index XXXXXXX..XXXXXXX 100644 | ||
47 | --- a/include/block/block-global-state.h | ||
48 | +++ b/include/block/block-global-state.h | ||
49 | @@ -XXX,XX +XXX,XX @@ BlockDriverState * GRAPH_RDLOCK | ||
50 | check_to_replace_node(BlockDriverState *parent_bs, const char *node_name, | ||
51 | Error **errp); | ||
52 | |||
53 | + | ||
54 | +bool GRAPH_RDLOCK bdrv_is_inactive(BlockDriverState *bs); | ||
55 | + | ||
56 | int no_coroutine_fn GRAPH_RDLOCK | ||
57 | bdrv_activate(BlockDriverState *bs, Error **errp); | ||
58 | |||
59 | diff --git a/block.c b/block.c | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/block.c | ||
62 | +++ b/block.c | ||
63 | @@ -XXX,XX +XXX,XX @@ void bdrv_init_with_whitelist(void) | ||
64 | bdrv_init(); | ||
65 | } | ||
66 | |||
67 | +bool bdrv_is_inactive(BlockDriverState *bs) { | ||
68 | + return bs->open_flags & BDRV_O_INACTIVE; | ||
69 | +} | ||
70 | + | ||
71 | int bdrv_activate(BlockDriverState *bs, Error **errp) | ||
72 | { | ||
73 | BdrvChild *child, *parent; | ||
74 | diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c | ||
75 | index XXXXXXX..XXXXXXX 100644 | ||
76 | --- a/block/monitor/block-hmp-cmds.c | ||
77 | +++ b/block/monitor/block-hmp-cmds.c | ||
78 | @@ -XXX,XX +XXX,XX @@ static void print_block_info(Monitor *mon, BlockInfo *info, | ||
79 | } | ||
80 | |||
81 | if (inserted) { | ||
82 | - monitor_printf(mon, ": %s (%s%s%s)\n", | ||
83 | + monitor_printf(mon, ": %s (%s%s%s%s)\n", | ||
84 | inserted->file, | ||
85 | inserted->drv, | ||
86 | inserted->ro ? ", read-only" : "", | ||
87 | - inserted->encrypted ? ", encrypted" : ""); | ||
88 | + inserted->encrypted ? ", encrypted" : "", | ||
89 | + inserted->active ? "" : ", inactive"); | ||
90 | } else { | ||
91 | monitor_printf(mon, ": [not inserted]\n"); | ||
92 | } | ||
93 | diff --git a/block/qapi.c b/block/qapi.c | ||
94 | index XXXXXXX..XXXXXXX 100644 | ||
95 | --- a/block/qapi.c | ||
96 | +++ b/block/qapi.c | ||
97 | @@ -XXX,XX +XXX,XX @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk, | ||
98 | info->file = g_strdup(bs->filename); | ||
99 | info->ro = bdrv_is_read_only(bs); | ||
100 | info->drv = g_strdup(bs->drv->format_name); | ||
101 | + info->active = !bdrv_is_inactive(bs); | ||
102 | info->encrypted = bs->encrypted; | ||
103 | |||
104 | info->cache = g_new(BlockdevCacheInfo, 1); | ||
105 | diff --git a/tests/qemu-iotests/184.out b/tests/qemu-iotests/184.out | ||
106 | index XXXXXXX..XXXXXXX 100644 | ||
107 | --- a/tests/qemu-iotests/184.out | ||
108 | +++ b/tests/qemu-iotests/184.out | ||
109 | @@ -XXX,XX +XXX,XX @@ Testing: | ||
110 | { | ||
111 | "iops_rd": 0, | ||
112 | "detect_zeroes": "off", | ||
113 | + "active": true, | ||
114 | "image": { | ||
115 | "backing-image": { | ||
116 | "virtual-size": 1073741824, | ||
117 | @@ -XXX,XX +XXX,XX @@ Testing: | ||
118 | { | ||
119 | "iops_rd": 0, | ||
120 | "detect_zeroes": "off", | ||
121 | + "active": true, | ||
122 | "image": { | ||
123 | "virtual-size": 1073741824, | ||
124 | "filename": "null-co://", | ||
125 | diff --git a/tests/qemu-iotests/191.out b/tests/qemu-iotests/191.out | ||
126 | index XXXXXXX..XXXXXXX 100644 | ||
127 | --- a/tests/qemu-iotests/191.out | ||
128 | +++ b/tests/qemu-iotests/191.out | ||
129 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
130 | { | ||
131 | "iops_rd": 0, | ||
132 | "detect_zeroes": "off", | ||
133 | + "active": true, | ||
134 | "image": { | ||
135 | "backing-image": { | ||
136 | "virtual-size": 67108864, | ||
137 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
138 | { | ||
139 | "iops_rd": 0, | ||
140 | "detect_zeroes": "off", | ||
141 | + "active": true, | ||
142 | "image": { | ||
143 | "virtual-size": 197120, | ||
144 | "filename": "TEST_DIR/t.IMGFMT.ovl2", | ||
145 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
146 | { | ||
147 | "iops_rd": 0, | ||
148 | "detect_zeroes": "off", | ||
149 | + "active": true, | ||
150 | "image": { | ||
151 | "backing-image": { | ||
152 | "virtual-size": 67108864, | ||
153 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
154 | { | ||
155 | "iops_rd": 0, | ||
156 | "detect_zeroes": "off", | ||
157 | + "active": true, | ||
158 | "image": { | ||
159 | "virtual-size": 197120, | ||
160 | "filename": "TEST_DIR/t.IMGFMT", | ||
161 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
162 | { | ||
163 | "iops_rd": 0, | ||
164 | "detect_zeroes": "off", | ||
165 | + "active": true, | ||
166 | "image": { | ||
167 | "backing-image": { | ||
168 | "virtual-size": 67108864, | ||
169 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
170 | { | ||
171 | "iops_rd": 0, | ||
172 | "detect_zeroes": "off", | ||
173 | + "active": true, | ||
174 | "image": { | ||
175 | "virtual-size": 393216, | ||
176 | "filename": "TEST_DIR/t.IMGFMT.mid", | ||
177 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
178 | { | ||
179 | "iops_rd": 0, | ||
180 | "detect_zeroes": "off", | ||
181 | + "active": true, | ||
182 | "image": { | ||
183 | "virtual-size": 67108864, | ||
184 | "filename": "TEST_DIR/t.IMGFMT.base", | ||
185 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
186 | { | ||
187 | "iops_rd": 0, | ||
188 | "detect_zeroes": "off", | ||
189 | + "active": true, | ||
190 | "image": { | ||
191 | "virtual-size": 393216, | ||
192 | "filename": "TEST_DIR/t.IMGFMT.base", | ||
193 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
194 | { | ||
195 | "iops_rd": 0, | ||
196 | "detect_zeroes": "off", | ||
197 | + "active": true, | ||
198 | "image": { | ||
199 | "backing-image": { | ||
200 | "virtual-size": 67108864, | ||
201 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
202 | { | ||
203 | "iops_rd": 0, | ||
204 | "detect_zeroes": "off", | ||
205 | + "active": true, | ||
206 | "image": { | ||
207 | "virtual-size": 197120, | ||
208 | "filename": "TEST_DIR/t.IMGFMT.ovl2", | ||
209 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
210 | { | ||
211 | "iops_rd": 0, | ||
212 | "detect_zeroes": "off", | ||
213 | + "active": true, | ||
214 | "image": { | ||
215 | "backing-image": { | ||
216 | "backing-image": { | ||
217 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
218 | { | ||
219 | "iops_rd": 0, | ||
220 | "detect_zeroes": "off", | ||
221 | + "active": true, | ||
222 | "image": { | ||
223 | "virtual-size": 197120, | ||
224 | "filename": "TEST_DIR/t.IMGFMT.ovl3", | ||
225 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
226 | { | ||
227 | "iops_rd": 0, | ||
228 | "detect_zeroes": "off", | ||
229 | + "active": true, | ||
230 | "image": { | ||
231 | "virtual-size": 67108864, | ||
232 | "filename": "TEST_DIR/t.IMGFMT.base", | ||
233 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
234 | { | ||
235 | "iops_rd": 0, | ||
236 | "detect_zeroes": "off", | ||
237 | + "active": true, | ||
238 | "image": { | ||
239 | "virtual-size": 393216, | ||
240 | "filename": "TEST_DIR/t.IMGFMT.base", | ||
241 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
242 | { | ||
243 | "iops_rd": 0, | ||
244 | "detect_zeroes": "off", | ||
245 | + "active": true, | ||
246 | "image": { | ||
247 | "backing-image": { | ||
248 | "virtual-size": 67108864, | ||
249 | @@ -XXX,XX +XXX,XX @@ wrote 65536/65536 bytes at offset 1048576 | ||
250 | { | ||
251 | "iops_rd": 0, | ||
252 | "detect_zeroes": "off", | ||
253 | + "active": true, | ||
254 | "image": { | ||
255 | "virtual-size": 197120, | ||
256 | "filename": "TEST_DIR/t.IMGFMT", | ||
257 | diff --git a/tests/qemu-iotests/273.out b/tests/qemu-iotests/273.out | ||
258 | index XXXXXXX..XXXXXXX 100644 | ||
259 | --- a/tests/qemu-iotests/273.out | ||
260 | +++ b/tests/qemu-iotests/273.out | ||
261 | @@ -XXX,XX +XXX,XX @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev | ||
262 | { | ||
263 | "iops_rd": 0, | ||
264 | "detect_zeroes": "off", | ||
265 | + "active": true, | ||
266 | "image": { | ||
267 | "backing-image": { | ||
268 | "backing-image": { | ||
269 | @@ -XXX,XX +XXX,XX @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev | ||
270 | { | ||
271 | "iops_rd": 0, | ||
272 | "detect_zeroes": "off", | ||
273 | + "active": true, | ||
274 | "image": { | ||
275 | "virtual-size": 197120, | ||
276 | "filename": "TEST_DIR/t.IMGFMT", | ||
277 | @@ -XXX,XX +XXX,XX @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev | ||
278 | { | ||
279 | "iops_rd": 0, | ||
280 | "detect_zeroes": "off", | ||
281 | + "active": true, | ||
282 | "image": { | ||
283 | "backing-image": { | ||
284 | "virtual-size": 197120, | ||
285 | @@ -XXX,XX +XXX,XX @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev | ||
286 | { | ||
287 | "iops_rd": 0, | ||
288 | "detect_zeroes": "off", | ||
289 | + "active": true, | ||
290 | "image": { | ||
291 | "virtual-size": 197120, | ||
292 | "filename": "TEST_DIR/t.IMGFMT.mid", | ||
293 | @@ -XXX,XX +XXX,XX @@ Testing: -blockdev file,node-name=base,filename=TEST_DIR/t.IMGFMT.base -blockdev | ||
294 | { | ||
295 | "iops_rd": 0, | ||
296 | "detect_zeroes": "off", | ||
297 | + "active": true, | ||
298 | "image": { | ||
299 | "virtual-size": 197120, | ||
300 | "filename": "TEST_DIR/t.IMGFMT.base", | ||
301 | -- | ||
302 | 2.48.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | What we wanted to catch with the assertion is cases where the recursion | ||
2 | finds that a child was inactive before its parent. This should never | ||
3 | happen. But if the user tries to inactivate an image that is already | ||
4 | inactive, that's harmless and we don't want to fail the assertion. | ||
1 | 5 | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
10 | Message-ID: <20250204211407.381505-3-kwolf@redhat.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | --- | ||
13 | block.c | 16 ++++++++++++---- | ||
14 | 1 file changed, 12 insertions(+), 4 deletions(-) | ||
15 | |||
16 | diff --git a/block.c b/block.c | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/block.c | ||
19 | +++ b/block.c | ||
20 | @@ -XXX,XX +XXX,XX @@ bdrv_has_bds_parent(BlockDriverState *bs, bool only_active) | ||
21 | return false; | ||
22 | } | ||
23 | |||
24 | -static int GRAPH_RDLOCK bdrv_inactivate_recurse(BlockDriverState *bs) | ||
25 | +static int GRAPH_RDLOCK | ||
26 | +bdrv_inactivate_recurse(BlockDriverState *bs, bool top_level) | ||
27 | { | ||
28 | BdrvChild *child, *parent; | ||
29 | int ret; | ||
30 | @@ -XXX,XX +XXX,XX @@ static int GRAPH_RDLOCK bdrv_inactivate_recurse(BlockDriverState *bs) | ||
31 | return 0; | ||
32 | } | ||
33 | |||
34 | - assert(!(bs->open_flags & BDRV_O_INACTIVE)); | ||
35 | + /* | ||
36 | + * Inactivating an already inactive node on user request is harmless, but if | ||
37 | + * a child is already inactive before its parent, that's bad. | ||
38 | + */ | ||
39 | + if (bs->open_flags & BDRV_O_INACTIVE) { | ||
40 | + assert(top_level); | ||
41 | + return 0; | ||
42 | + } | ||
43 | |||
44 | /* Inactivate this node */ | ||
45 | if (bs->drv->bdrv_inactivate) { | ||
46 | @@ -XXX,XX +XXX,XX @@ static int GRAPH_RDLOCK bdrv_inactivate_recurse(BlockDriverState *bs) | ||
47 | |||
48 | /* Recursively inactivate children */ | ||
49 | QLIST_FOREACH(child, &bs->children, next) { | ||
50 | - ret = bdrv_inactivate_recurse(child->bs); | ||
51 | + ret = bdrv_inactivate_recurse(child->bs, false); | ||
52 | if (ret < 0) { | ||
53 | return ret; | ||
54 | } | ||
55 | @@ -XXX,XX +XXX,XX @@ int bdrv_inactivate_all(void) | ||
56 | if (bdrv_has_bds_parent(bs, false)) { | ||
57 | continue; | ||
58 | } | ||
59 | - ret = bdrv_inactivate_recurse(bs); | ||
60 | + ret = bdrv_inactivate_recurse(bs, true); | ||
61 | if (ret < 0) { | ||
62 | bdrv_next_cleanup(&it); | ||
63 | break; | ||
64 | -- | ||
65 | 2.48.1 | diff view generated by jsdifflib |
1 | blockdev-snapshot returned an error if the overlay was already in use, | 1 | Putting an active block node on top of an inactive one is strictly |
---|---|---|---|
2 | which it defined as having any BlockBackend parent. This is in fact both | 2 | speaking an invalid configuration and the next patch will turn it into a |
3 | too strict (some parents can tolerate the change of visible data caused | 3 | hard error. |
4 | by attaching a backing file) and too loose (some non-BlockBackend | ||
5 | parents may not be happy with it). | ||
6 | 4 | ||
7 | One important use case that is prevented by the too strict check is live | 5 | However, taking a snapshot while disk images are inactive after |
8 | storage migration with blockdev-mirror. Here, the target node is | 6 | completing migration has an important use case: After migrating to a |
9 | usually opened without a backing file so that the active layer is | 7 | file, taking an external snapshot is what is needed to take a full VM |
10 | mirrored while its backing chain can be copied in the background. | 8 | snapshot. |
11 | 9 | ||
12 | The backing chain should be attached to the mirror target node when | 10 | In order for this to keep working after the later patches, change |
13 | finalising the job, just before switching the users of the source node | 11 | creating a snapshot such that it automatically inactivates an overlay |
14 | to the new copy (at which point the mirror job still has a reference to | 12 | that is added on top of an already inactive node. |
15 | the node). drive-mirror did this automatically, but with blockdev-mirror | ||
16 | this is the job of the QMP client, so it needs a way to do this. | ||
17 | |||
18 | blockdev-snapshot is the obvious way, so this patch makes it work in | ||
19 | this scenario. The new condition is that no parent uses CONSISTENT_READ | ||
20 | permissions. This will ensure that the operation will still be blocked | ||
21 | when the node is attached to the guest device, so blockdev-snapshot | ||
22 | remains safe. | ||
23 | |||
24 | (For the sake of completeness, x-blockdev-reopen can be used to achieve | ||
25 | the same, however it is a big hammer, performs the graph change | ||
26 | completely unchecked and is still experimental. So even with the option | ||
27 | of using x-blockdev-reopen, there are reasons why blockdev-snapshot | ||
28 | should be able to perform this operation.) | ||
29 | 13 | ||
30 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
31 | Message-Id: <20200310113831.27293-3-kwolf@redhat.com> | 15 | Acked-by: Fabiano Rosas <farosas@suse.de> |
32 | Reviewed-by: Peter Krempa <pkrempa@redhat.com> | 16 | Reviewed-by: Eric Blake <eblake@redhat.com> |
33 | Tested-by: Peter Krempa <pkrempa@redhat.com> | 17 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
18 | Message-ID: <20250204211407.381505-4-kwolf@redhat.com> | ||
34 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 19 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
35 | --- | 20 | --- |
36 | blockdev.c | 14 ++++++++------ | 21 | blockdev.c | 16 ++++++++++++++++ |
37 | tests/qemu-iotests/085.out | 4 ++-- | 22 | 1 file changed, 16 insertions(+) |
38 | 2 files changed, 10 insertions(+), 8 deletions(-) | ||
39 | 23 | ||
40 | diff --git a/blockdev.c b/blockdev.c | 24 | diff --git a/blockdev.c b/blockdev.c |
41 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
42 | --- a/blockdev.c | 26 | --- a/blockdev.c |
43 | +++ b/blockdev.c | 27 | +++ b/blockdev.c |
44 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common, | 28 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_action(TransactionAction *action, |
45 | TransactionAction *action = common->action; | 29 | return; |
46 | AioContext *aio_context; | ||
47 | AioContext *old_context; | ||
48 | + uint64_t perm, shared; | ||
49 | int ret; | ||
50 | |||
51 | /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar | ||
52 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common, | ||
53 | goto out; | ||
54 | } | 30 | } |
55 | 31 | ||
56 | - if (bdrv_has_blk(state->new_bs)) { | ||
57 | + /* | 32 | + /* |
58 | + * Allow attaching a backing file to an overlay that's already in use only | 33 | + * Older QEMU versions have allowed adding an active parent node to an |
59 | + * if the parents don't assume that they are already seeing a valid image. | 34 | + * inactive child node. This is unsafe in the general case, but there is an |
60 | + * (Specifically, allow it as a mirror target, which is write-only access.) | 35 | + * important use case, which is taking a VM snapshot with migration to file |
36 | + * and then adding an external snapshot while the VM is still stopped and | ||
37 | + * images are inactive. Requiring the user to explicitly create the overlay | ||
38 | + * as inactive would break compatibility, so just do it automatically here | ||
39 | + * to keep this working. | ||
61 | + */ | 40 | + */ |
62 | + bdrv_get_cumulative_perm(state->new_bs, &perm, &shared); | 41 | + if (bdrv_is_inactive(state->old_bs) && !bdrv_is_inactive(state->new_bs)) { |
63 | + if (perm & BLK_PERM_CONSISTENT_READ) { | 42 | + ret = bdrv_inactivate(state->new_bs, errp); |
64 | error_setg(errp, "The overlay is already in use"); | 43 | + if (ret < 0) { |
65 | goto out; | 44 | + return; |
66 | } | 45 | + } |
67 | 46 | + } | |
68 | - if (bdrv_op_is_blocked(state->new_bs, BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, | 47 | + |
69 | - errp)) { | 48 | ret = bdrv_append(state->new_bs, state->old_bs, errp); |
70 | - goto out; | 49 | if (ret < 0) { |
71 | - } | 50 | return; |
72 | - | ||
73 | if (state->new_bs->backing != NULL) { | ||
74 | error_setg(errp, "The overlay already has a backing image"); | ||
75 | goto out; | ||
76 | diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out | ||
77 | index XXXXXXX..XXXXXXX 100644 | ||
78 | --- a/tests/qemu-iotests/085.out | ||
79 | +++ b/tests/qemu-iotests/085.out | ||
80 | @@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f | ||
81 | === Invalid command - cannot create a snapshot using a file BDS === | ||
82 | |||
83 | { 'execute': 'blockdev-snapshot', 'arguments': { 'node':'virtio0', 'overlay':'file_12' } } | ||
84 | -{"error": {"class": "GenericError", "desc": "The overlay does not support backing images"}} | ||
85 | +{"error": {"class": "GenericError", "desc": "The overlay is already in use"}} | ||
86 | |||
87 | === Invalid command - snapshot node used as active layer === | ||
88 | |||
89 | @@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/12-snapshot-v0.IMGFMT', fmt=IMGFMT size=134217728 backing_f | ||
90 | === Invalid command - snapshot node used as backing hd === | ||
91 | |||
92 | { 'execute': 'blockdev-snapshot', 'arguments': { 'node': 'virtio0', 'overlay':'snap_11' } } | ||
93 | -{"error": {"class": "GenericError", "desc": "Node 'snap_11' is busy: node is used as backing hd of 'snap_12'"}} | ||
94 | +{"error": {"class": "GenericError", "desc": "The overlay is already in use"}} | ||
95 | |||
96 | === Invalid command - snapshot node has a backing image === | ||
97 | |||
98 | -- | 51 | -- |
99 | 2.20.1 | 52 | 2.48.1 |
100 | |||
101 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Block devices have an individual active state, a single global flag | ||
2 | can't cover this correctly. This becomes more important as we allow | ||
3 | users to manually manage which nodes are active or inactive. | ||
1 | 4 | ||
5 | Now that it's allowed to call bdrv_inactivate_all() even when some | ||
6 | nodes are already inactive, we can remove the flag and just | ||
7 | unconditionally call bdrv_inactivate_all() and, more importantly, | ||
8 | bdrv_activate_all() before we make use of the nodes. | ||
9 | |||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
12 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Message-ID: <20250204211407.381505-5-kwolf@redhat.com> | ||
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
16 | --- | ||
17 | migration/migration.h | 3 --- | ||
18 | migration/block-active.c | 46 ---------------------------------------- | ||
19 | migration/migration.c | 8 ------- | ||
20 | 3 files changed, 57 deletions(-) | ||
21 | |||
22 | diff --git a/migration/migration.h b/migration/migration.h | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/migration/migration.h | ||
25 | +++ b/migration/migration.h | ||
26 | @@ -XXX,XX +XXX,XX @@ void migration_bitmap_sync_precopy(bool last_stage); | ||
27 | void dirty_bitmap_mig_init(void); | ||
28 | bool should_send_vmdesc(void); | ||
29 | |||
30 | -/* migration/block-active.c */ | ||
31 | -void migration_block_active_setup(bool active); | ||
32 | - | ||
33 | #endif | ||
34 | diff --git a/migration/block-active.c b/migration/block-active.c | ||
35 | index XXXXXXX..XXXXXXX 100644 | ||
36 | --- a/migration/block-active.c | ||
37 | +++ b/migration/block-active.c | ||
38 | @@ -XXX,XX +XXX,XX @@ | ||
39 | #include "qemu/error-report.h" | ||
40 | #include "trace.h" | ||
41 | |||
42 | -/* | ||
43 | - * Migration-only cache to remember the block layer activation status. | ||
44 | - * Protected by BQL. | ||
45 | - * | ||
46 | - * We need this because.. | ||
47 | - * | ||
48 | - * - Migration can fail after block devices are invalidated (during | ||
49 | - * switchover phase). When that happens, we need to be able to recover | ||
50 | - * the block drive status by re-activating them. | ||
51 | - * | ||
52 | - * - Currently bdrv_inactivate_all() is not safe to be invoked on top of | ||
53 | - * invalidated drives (even if bdrv_activate_all() is actually safe to be | ||
54 | - * called any time!). It means remembering this could help migration to | ||
55 | - * make sure it won't invalidate twice in a row, crashing QEMU. It can | ||
56 | - * happen when we migrate a PAUSED VM from host1 to host2, then migrate | ||
57 | - * again to host3 without starting it. TODO: a cleaner solution is to | ||
58 | - * allow safe invoke of bdrv_inactivate_all() at anytime, like | ||
59 | - * bdrv_activate_all(). | ||
60 | - * | ||
61 | - * For freshly started QEMU, the flag is initialized to TRUE reflecting the | ||
62 | - * scenario where QEMU owns block device ownerships. | ||
63 | - * | ||
64 | - * For incoming QEMU taking a migration stream, the flag is initialized to | ||
65 | - * FALSE reflecting that the incoming side doesn't own the block devices, | ||
66 | - * not until switchover happens. | ||
67 | - */ | ||
68 | -static bool migration_block_active; | ||
69 | - | ||
70 | -/* Setup the disk activation status */ | ||
71 | -void migration_block_active_setup(bool active) | ||
72 | -{ | ||
73 | - migration_block_active = active; | ||
74 | -} | ||
75 | - | ||
76 | bool migration_block_activate(Error **errp) | ||
77 | { | ||
78 | ERRP_GUARD(); | ||
79 | |||
80 | assert(bql_locked()); | ||
81 | |||
82 | - if (migration_block_active) { | ||
83 | - trace_migration_block_activation("active-skipped"); | ||
84 | - return true; | ||
85 | - } | ||
86 | - | ||
87 | trace_migration_block_activation("active"); | ||
88 | |||
89 | bdrv_activate_all(errp); | ||
90 | @@ -XXX,XX +XXX,XX @@ bool migration_block_activate(Error **errp) | ||
91 | return false; | ||
92 | } | ||
93 | |||
94 | - migration_block_active = true; | ||
95 | return true; | ||
96 | } | ||
97 | |||
98 | @@ -XXX,XX +XXX,XX @@ bool migration_block_inactivate(void) | ||
99 | |||
100 | assert(bql_locked()); | ||
101 | |||
102 | - if (!migration_block_active) { | ||
103 | - trace_migration_block_activation("inactive-skipped"); | ||
104 | - return true; | ||
105 | - } | ||
106 | - | ||
107 | trace_migration_block_activation("inactive"); | ||
108 | |||
109 | ret = bdrv_inactivate_all(); | ||
110 | @@ -XXX,XX +XXX,XX @@ bool migration_block_inactivate(void) | ||
111 | return false; | ||
112 | } | ||
113 | |||
114 | - migration_block_active = false; | ||
115 | return true; | ||
116 | } | ||
117 | diff --git a/migration/migration.c b/migration/migration.c | ||
118 | index XXXXXXX..XXXXXXX 100644 | ||
119 | --- a/migration/migration.c | ||
120 | +++ b/migration/migration.c | ||
121 | @@ -XXX,XX +XXX,XX @@ void qmp_migrate_incoming(const char *uri, bool has_channels, | ||
122 | return; | ||
123 | } | ||
124 | |||
125 | - /* | ||
126 | - * Newly setup incoming QEMU. Mark the block active state to reflect | ||
127 | - * that the src currently owns the disks. | ||
128 | - */ | ||
129 | - migration_block_active_setup(false); | ||
130 | - | ||
131 | once = false; | ||
132 | } | ||
133 | |||
134 | @@ -XXX,XX +XXX,XX @@ static void migration_instance_init(Object *obj) | ||
135 | ms->state = MIGRATION_STATUS_NONE; | ||
136 | ms->mbps = -1; | ||
137 | ms->pages_per_second = -1; | ||
138 | - /* Freshly started QEMU owns all the block devices */ | ||
139 | - migration_block_active_setup(true); | ||
140 | qemu_sem_init(&ms->pause_sem, 0); | ||
141 | qemu_mutex_init(&ms->error_mutex); | ||
142 | |||
143 | -- | ||
144 | 2.48.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | An active node makes unrestricted use of its children and would possibly | ||
2 | run into assertion failures when it operates on an inactive child node. | ||
1 | 3 | ||
4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
5 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
6 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
7 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
8 | Message-ID: <20250204211407.381505-6-kwolf@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | block.c | 5 +++++ | ||
12 | 1 file changed, 5 insertions(+) | ||
13 | |||
14 | diff --git a/block.c b/block.c | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/block.c | ||
17 | +++ b/block.c | ||
18 | @@ -XXX,XX +XXX,XX @@ bdrv_attach_child_noperm(BlockDriverState *parent_bs, | ||
19 | child_bs->node_name, child_name, parent_bs->node_name); | ||
20 | return NULL; | ||
21 | } | ||
22 | + if (bdrv_is_inactive(child_bs) && !bdrv_is_inactive(parent_bs)) { | ||
23 | + error_setg(errp, "Inactive '%s' can't be a %s child of active '%s'", | ||
24 | + child_bs->node_name, child_name, parent_bs->node_name); | ||
25 | + return NULL; | ||
26 | + } | ||
27 | |||
28 | bdrv_get_cumulative_perm(parent_bs, &perm, &shared_perm); | ||
29 | bdrv_child_perm(parent_bs, child_bs, NULL, child_role, NULL, | ||
30 | -- | ||
31 | 2.48.1 | diff view generated by jsdifflib |
1 | In order for block_resize to fail gracefully on an inactive node instead | ||
---|---|---|---|
2 | of crashing with an assertion failure in bdrv_co_write_req_prepare() | ||
3 | (called from bdrv_co_truncate()), we need to check for inactive nodes | ||
4 | also when they are attached as a root node and make sure that | ||
5 | BLK_PERM_RESIZE isn't among the permissions allowed for inactive nodes. | ||
6 | To this effect, don't enumerate the permissions that are incompatible | ||
7 | with inactive nodes any more, but allow only BLK_PERM_CONSISTENT_READ | ||
8 | for them. | ||
9 | |||
1 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
2 | Message-Id: <20200310113831.27293-2-kwolf@redhat.com> | 11 | Acked-by: Fabiano Rosas <farosas@suse.de> |
3 | Reviewed-by: Peter Krempa <pkrempa@redhat.com> | 12 | Reviewed-by: Eric Blake <eblake@redhat.com> |
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
14 | Message-ID: <20250204211407.381505-7-kwolf@redhat.com> | ||
4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
5 | --- | 16 | --- |
6 | include/block/block_int.h | 3 +++ | 17 | block.c | 7 +++++++ |
7 | block.c | 6 ++---- | 18 | block/block-backend.c | 2 +- |
8 | 2 files changed, 5 insertions(+), 4 deletions(-) | 19 | 2 files changed, 8 insertions(+), 1 deletion(-) |
9 | 20 | ||
10 | diff --git a/include/block/block_int.h b/include/block/block_int.h | ||
11 | index XXXXXXX..XXXXXXX 100644 | ||
12 | --- a/include/block/block_int.h | ||
13 | +++ b/include/block/block_int.h | ||
14 | @@ -XXX,XX +XXX,XX @@ BdrvChild *bdrv_root_attach_child(BlockDriverState *child_bs, | ||
15 | void *opaque, Error **errp); | ||
16 | void bdrv_root_unref_child(BdrvChild *child); | ||
17 | |||
18 | +void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, | ||
19 | + uint64_t *shared_perm); | ||
20 | + | ||
21 | /** | ||
22 | * Sets a BdrvChild's permissions. Avoid if the parent is a BDS; use | ||
23 | * bdrv_child_refresh_perms() instead and make the parent's | ||
24 | diff --git a/block.c b/block.c | 21 | diff --git a/block.c b/block.c |
25 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
26 | --- a/block.c | 23 | --- a/block.c |
27 | +++ b/block.c | 24 | +++ b/block.c |
28 | @@ -XXX,XX +XXX,XX @@ static int bdrv_child_check_perm(BdrvChild *c, BlockReopenQueue *q, | 25 | @@ -XXX,XX +XXX,XX @@ bdrv_attach_child_common(BlockDriverState *child_bs, |
29 | bool *tighten_restrictions, Error **errp); | 26 | assert(child_class->get_parent_desc); |
30 | static void bdrv_child_abort_perm_update(BdrvChild *c); | 27 | GLOBAL_STATE_CODE(); |
31 | static void bdrv_child_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared); | 28 | |
32 | -static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, | 29 | + if (bdrv_is_inactive(child_bs) && (perm & ~BLK_PERM_CONSISTENT_READ)) { |
33 | - uint64_t *shared_perm); | 30 | + g_autofree char *perm_names = bdrv_perm_names(perm); |
34 | 31 | + error_setg(errp, "Permission '%s' unavailable on inactive node", | |
35 | typedef struct BlockReopenQueueEntry { | 32 | + perm_names); |
36 | bool prepared; | 33 | + return NULL; |
37 | @@ -XXX,XX +XXX,XX @@ static void bdrv_set_perm(BlockDriverState *bs, uint64_t cumulative_perms, | 34 | + } |
35 | + | ||
36 | new_child = g_new(BdrvChild, 1); | ||
37 | *new_child = (BdrvChild) { | ||
38 | .bs = NULL, | ||
39 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
40 | index XXXXXXX..XXXXXXX 100644 | ||
41 | --- a/block/block-backend.c | ||
42 | +++ b/block/block-backend.c | ||
43 | @@ -XXX,XX +XXX,XX @@ static bool blk_can_inactivate(BlockBackend *blk) | ||
44 | * guest. For block job BBs that satisfy this, we can just allow | ||
45 | * it. This is the case for mirror job source, which is required | ||
46 | * by libvirt non-shared block migration. */ | ||
47 | - if (!(blk->perm & (BLK_PERM_WRITE | BLK_PERM_WRITE_UNCHANGED))) { | ||
48 | + if (!(blk->perm & ~BLK_PERM_CONSISTENT_READ)) { | ||
49 | return true; | ||
38 | } | 50 | } |
39 | } | 51 | |
40 | |||
41 | -static void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, | ||
42 | - uint64_t *shared_perm) | ||
43 | +void bdrv_get_cumulative_perm(BlockDriverState *bs, uint64_t *perm, | ||
44 | + uint64_t *shared_perm) | ||
45 | { | ||
46 | BdrvChild *c; | ||
47 | uint64_t cumulative_perms = 0; | ||
48 | -- | 52 | -- |
49 | 2.20.1 | 53 | 2.48.1 |
50 | |||
51 | diff view generated by jsdifflib |
1 | From: Peter Krempa <pkrempa@redhat.com> | 1 | In QEMU, nodes are automatically created inactive while expecting an |
---|---|---|---|
2 | incoming migration (i.e. RUN_STATE_INMIGRATE). In qemu-storage-daemon, | ||
3 | the notion of runstates doesn't exist. It also wouldn't necessarily make | ||
4 | sense to introduce it because a single daemon can serve multiple VMs | ||
5 | that can be in different states. | ||
2 | 6 | ||
3 | Anounce that 'blockdev-snapshot' command's permissions allow changing | 7 | Therefore, allow the user to explicitly open images as inactive with a |
4 | of the backing file if the 'consistent_read' permission is not required. | 8 | new option. The default is as before: Nodes are usually active, except |
9 | when created during RUN_STATE_INMIGRATE. | ||
5 | 10 | ||
6 | This is useful for libvirt to allow late opening of the backing chain | ||
7 | during a blockdev-mirror. | ||
8 | |||
9 | Signed-off-by: Peter Krempa <pkrempa@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
11 | Message-Id: <20200310113831.27293-8-kwolf@redhat.com> | 12 | Acked-by: Fabiano Rosas <farosas@suse.de> |
13 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
15 | Message-ID: <20250204211407.381505-8-kwolf@redhat.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
13 | --- | 17 | --- |
14 | qapi/block-core.json | 9 ++++++++- | 18 | qapi/block-core.json | 6 ++++++ |
15 | 1 file changed, 8 insertions(+), 1 deletion(-) | 19 | include/block/block-common.h | 1 + |
20 | block.c | 9 +++++++++ | ||
21 | 3 files changed, 16 insertions(+) | ||
16 | 22 | ||
17 | diff --git a/qapi/block-core.json b/qapi/block-core.json | 23 | diff --git a/qapi/block-core.json b/qapi/block-core.json |
18 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/qapi/block-core.json | 25 | --- a/qapi/block-core.json |
20 | +++ b/qapi/block-core.json | 26 | +++ b/qapi/block-core.json |
21 | @@ -XXX,XX +XXX,XX @@ | 27 | @@ -XXX,XX +XXX,XX @@ |
22 | # | 28 | # |
23 | # For the arguments, see the documentation of BlockdevSnapshot. | 29 | # @cache: cache-related options |
24 | # | 30 | # |
25 | +# Features: | 31 | +# @active: whether the block node should be activated (default: true). |
26 | +# @allow-write-only-overlay: If present, the check whether this operation is safe | 32 | +# Having inactive block nodes is useful primarily for migration because it |
27 | +# was relaxed so that it can be used to change | 33 | +# allows opening an image on the destination while the source is still |
28 | +# backing file of a destination of a blockdev-mirror. | 34 | +# holding locks for it. (Since 10.0) |
29 | +# (since 5.0) | ||
30 | +# | 35 | +# |
31 | # Since: 2.5 | 36 | # @read-only: whether the block device should be read-only (default: |
32 | # | 37 | # false). Note that some block drivers support only read-only |
33 | # Example: | 38 | # access, either generally or in certain configurations. In this |
34 | @@ -XXX,XX +XXX,XX @@ | 39 | @@ -XXX,XX +XXX,XX @@ |
35 | # | 40 | '*node-name': 'str', |
36 | ## | 41 | '*discard': 'BlockdevDiscardOptions', |
37 | { 'command': 'blockdev-snapshot', | 42 | '*cache': 'BlockdevCacheOptions', |
38 | - 'data': 'BlockdevSnapshot' } | 43 | + '*active': 'bool', |
39 | + 'data': 'BlockdevSnapshot', | 44 | '*read-only': 'bool', |
40 | + 'features': [ 'allow-write-only-overlay' ] } | 45 | '*auto-read-only': 'bool', |
41 | 46 | '*force-share': 'bool', | |
42 | ## | 47 | diff --git a/include/block/block-common.h b/include/block/block-common.h |
43 | # @change-backing-file: | 48 | index XXXXXXX..XXXXXXX 100644 |
49 | --- a/include/block/block-common.h | ||
50 | +++ b/include/block/block-common.h | ||
51 | @@ -XXX,XX +XXX,XX @@ typedef enum { | ||
52 | #define BDRV_OPT_AUTO_READ_ONLY "auto-read-only" | ||
53 | #define BDRV_OPT_DISCARD "discard" | ||
54 | #define BDRV_OPT_FORCE_SHARE "force-share" | ||
55 | +#define BDRV_OPT_ACTIVE "active" | ||
56 | |||
57 | |||
58 | #define BDRV_SECTOR_BITS 9 | ||
59 | diff --git a/block.c b/block.c | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/block.c | ||
62 | +++ b/block.c | ||
63 | @@ -XXX,XX +XXX,XX @@ static void update_flags_from_options(int *flags, QemuOpts *opts) | ||
64 | if (qemu_opt_get_bool_del(opts, BDRV_OPT_AUTO_READ_ONLY, false)) { | ||
65 | *flags |= BDRV_O_AUTO_RDONLY; | ||
66 | } | ||
67 | + | ||
68 | + if (!qemu_opt_get_bool_del(opts, BDRV_OPT_ACTIVE, true)) { | ||
69 | + *flags |= BDRV_O_INACTIVE; | ||
70 | + } | ||
71 | } | ||
72 | |||
73 | static void update_options_from_flags(QDict *options, int flags) | ||
74 | @@ -XXX,XX +XXX,XX @@ QemuOptsList bdrv_runtime_opts = { | ||
75 | .type = QEMU_OPT_BOOL, | ||
76 | .help = "Ignore flush requests", | ||
77 | }, | ||
78 | + { | ||
79 | + .name = BDRV_OPT_ACTIVE, | ||
80 | + .type = QEMU_OPT_BOOL, | ||
81 | + .help = "Node is activated", | ||
82 | + }, | ||
83 | { | ||
84 | .name = BDRV_OPT_READ_ONLY, | ||
85 | .type = QEMU_OPT_BOOL, | ||
44 | -- | 86 | -- |
45 | 2.20.1 | 87 | 2.48.1 |
46 | |||
47 | diff view generated by jsdifflib |
1 | From: Daniel Henrique Barboza <danielhb413@gmail.com> | 1 | The system emulator tries to automatically activate and inactivate block |
---|---|---|---|
2 | nodes at the right point during migration. However, there are still | ||
3 | cases where it's necessary that the user can do this manually. | ||
2 | 4 | ||
3 | Using the new 'bdrv_co_delete_file' interface, a pure co_routine function | 5 | Images are only activated on the destination VM of a migration when the |
4 | 'bdrv_co_delete_file' inside block.c can can be used in a way similar of | 6 | VM is actually resumed. If the VM was paused, this doesn't happen |
5 | the existing bdrv_create_file to to clean up a created file. | 7 | automatically. The user may want to perform some operation on a block |
8 | device (e.g. taking a snapshot or starting a block job) without also | ||
9 | resuming the VM yet. This is an example where a manual command is | ||
10 | necessary. | ||
6 | 11 | ||
7 | We're creating a pure co_routine because the only caller of | 12 | Another example is VM migration when the image files are opened by an |
8 | 'bdrv_co_delete_file' will be already in co_routine context, thus there | 13 | external qemu-storage-daemon instance on each side. In this case, the |
9 | is no need to add all the machinery to check for qemu_in_coroutine() and | 14 | process that needs to hand over the images isn't even part of the |
10 | create a separated co_routine to do the job. | 15 | migration and can't know when the migration completes. Management tools |
16 | need a way to explicitly inactivate images on the source and activate | ||
17 | them on the destination. | ||
11 | 18 | ||
12 | Suggested-by: Daniel P. Berrangé <berrange@redhat.com> | 19 | This adds a new blockdev-set-active QMP command that lets the user |
13 | Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> | 20 | change the status of individual nodes (this is necessary in |
14 | Message-Id: <20200130213907.2830642-3-danielhb413@gmail.com> | 21 | qemu-storage-daemon because it could be serving multiple VMs and only |
22 | one of them migrates at a time). For convenience, operating on all | ||
23 | devices (like QEMU does automatically during migration) is offered as an | ||
24 | option, too, and can be used in the context of single VM. | ||
25 | |||
26 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
27 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
28 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
29 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
30 | Message-ID: <20250204211407.381505-9-kwolf@redhat.com> | ||
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 31 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
16 | --- | 32 | --- |
17 | include/block/block.h | 1 + | 33 | qapi/block-core.json | 32 ++++++++++++++++++++++++++++++ |
18 | block.c | 26 ++++++++++++++++++++++++++ | 34 | include/block/block-global-state.h | 3 +++ |
19 | 2 files changed, 27 insertions(+) | 35 | block.c | 21 ++++++++++++++++++++ |
36 | blockdev.c | 32 ++++++++++++++++++++++++++++++ | ||
37 | 4 files changed, 88 insertions(+) | ||
20 | 38 | ||
21 | diff --git a/include/block/block.h b/include/block/block.h | 39 | diff --git a/qapi/block-core.json b/qapi/block-core.json |
22 | index XXXXXXX..XXXXXXX 100644 | 40 | index XXXXXXX..XXXXXXX 100644 |
23 | --- a/include/block/block.h | 41 | --- a/qapi/block-core.json |
24 | +++ b/include/block/block.h | 42 | +++ b/qapi/block-core.json |
25 | @@ -XXX,XX +XXX,XX @@ bool bdrv_is_backing_chain_frozen(BlockDriverState *bs, BlockDriverState *base, | 43 | @@ -XXX,XX +XXX,XX @@ |
26 | int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, | 44 | { 'command': 'blockdev-del', 'data': { 'node-name': 'str' }, |
27 | Error **errp); | 45 | 'allow-preconfig': true } |
28 | void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); | 46 | |
29 | +int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp); | 47 | +## |
30 | 48 | +# @blockdev-set-active: | |
31 | 49 | +# | |
32 | typedef struct BdrvCheckResult { | 50 | +# Activate or inactivate a block device. Use this to manage the handover of |
51 | +# block devices on migration with qemu-storage-daemon. | ||
52 | +# | ||
53 | +# Activating a node automatically activates all of its child nodes first. | ||
54 | +# Inactivating a node automatically inactivates any of its child nodes that are | ||
55 | +# not in use by a still active node. | ||
56 | +# | ||
57 | +# @node-name: Name of the graph node to activate or inactivate. By default, all | ||
58 | +# nodes are affected by the operation. | ||
59 | +# | ||
60 | +# @active: true if the nodes should be active when the command returns success, | ||
61 | +# false if they should be inactive. | ||
62 | +# | ||
63 | +# Since: 10.0 | ||
64 | +# | ||
65 | +# .. qmp-example:: | ||
66 | +# | ||
67 | +# -> { "execute": "blockdev-set-active", | ||
68 | +# "arguments": { | ||
69 | +# "node-name": "node0", | ||
70 | +# "active": false | ||
71 | +# } | ||
72 | +# } | ||
73 | +# <- { "return": {} } | ||
74 | +## | ||
75 | +{ 'command': 'blockdev-set-active', | ||
76 | + 'data': { '*node-name': 'str', 'active': 'bool' }, | ||
77 | + 'allow-preconfig': true } | ||
78 | + | ||
79 | ## | ||
80 | # @BlockdevCreateOptionsFile: | ||
81 | # | ||
82 | diff --git a/include/block/block-global-state.h b/include/block/block-global-state.h | ||
83 | index XXXXXXX..XXXXXXX 100644 | ||
84 | --- a/include/block/block-global-state.h | ||
85 | +++ b/include/block/block-global-state.h | ||
86 | @@ -XXX,XX +XXX,XX @@ bdrv_activate(BlockDriverState *bs, Error **errp); | ||
87 | int coroutine_fn no_co_wrapper_bdrv_rdlock | ||
88 | bdrv_co_activate(BlockDriverState *bs, Error **errp); | ||
89 | |||
90 | +int no_coroutine_fn | ||
91 | +bdrv_inactivate(BlockDriverState *bs, Error **errp); | ||
92 | + | ||
93 | void bdrv_activate_all(Error **errp); | ||
94 | int bdrv_inactivate_all(void); | ||
95 | |||
33 | diff --git a/block.c b/block.c | 96 | diff --git a/block.c b/block.c |
34 | index XXXXXXX..XXXXXXX 100644 | 97 | index XXXXXXX..XXXXXXX 100644 |
35 | --- a/block.c | 98 | --- a/block.c |
36 | +++ b/block.c | 99 | +++ b/block.c |
37 | @@ -XXX,XX +XXX,XX @@ int bdrv_create_file(const char *filename, QemuOpts *opts, Error **errp) | 100 | @@ -XXX,XX +XXX,XX @@ bdrv_inactivate_recurse(BlockDriverState *bs, bool top_level) |
38 | } | 101 | return 0; |
39 | } | 102 | } |
40 | 103 | ||
41 | +int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp) | 104 | +int bdrv_inactivate(BlockDriverState *bs, Error **errp) |
42 | +{ | 105 | +{ |
43 | + Error *local_err = NULL; | ||
44 | + int ret; | 106 | + int ret; |
45 | + | 107 | + |
46 | + assert(bs != NULL); | 108 | + GLOBAL_STATE_CODE(); |
109 | + GRAPH_RDLOCK_GUARD_MAINLOOP(); | ||
47 | + | 110 | + |
48 | + if (!bs->drv) { | 111 | + if (bdrv_has_bds_parent(bs, true)) { |
49 | + error_setg(errp, "Block node '%s' is not opened", bs->filename); | 112 | + error_setg(errp, "Node has active parent node"); |
50 | + return -ENOMEDIUM; | 113 | + return -EPERM; |
51 | + } | 114 | + } |
52 | + | 115 | + |
53 | + if (!bs->drv->bdrv_co_delete_file) { | 116 | + ret = bdrv_inactivate_recurse(bs, true); |
54 | + error_setg(errp, "Driver '%s' does not support image deletion", | 117 | + if (ret < 0) { |
55 | + bs->drv->format_name); | 118 | + error_setg_errno(errp, -ret, "Failed to inactivate node"); |
56 | + return -ENOTSUP; | 119 | + return ret; |
57 | + } | 120 | + } |
58 | + | 121 | + |
59 | + ret = bs->drv->bdrv_co_delete_file(bs, &local_err); | 122 | + return 0; |
60 | + if (ret < 0) { | ||
61 | + error_propagate(errp, local_err); | ||
62 | + } | ||
63 | + | ||
64 | + return ret; | ||
65 | +} | 123 | +} |
66 | + | 124 | + |
67 | /** | 125 | int bdrv_inactivate_all(void) |
68 | * Try to get @bs's logical and physical block size. | 126 | { |
69 | * On success, store them in @bsz struct and return 0. | 127 | BlockDriverState *bs = NULL; |
128 | diff --git a/blockdev.c b/blockdev.c | ||
129 | index XXXXXXX..XXXXXXX 100644 | ||
130 | --- a/blockdev.c | ||
131 | +++ b/blockdev.c | ||
132 | @@ -XXX,XX +XXX,XX @@ void qmp_blockdev_del(const char *node_name, Error **errp) | ||
133 | bdrv_unref(bs); | ||
134 | } | ||
135 | |||
136 | +void qmp_blockdev_set_active(const char *node_name, bool active, Error **errp) | ||
137 | +{ | ||
138 | + int ret; | ||
139 | + | ||
140 | + GLOBAL_STATE_CODE(); | ||
141 | + GRAPH_RDLOCK_GUARD_MAINLOOP(); | ||
142 | + | ||
143 | + if (!node_name) { | ||
144 | + if (active) { | ||
145 | + bdrv_activate_all(errp); | ||
146 | + } else { | ||
147 | + ret = bdrv_inactivate_all(); | ||
148 | + if (ret < 0) { | ||
149 | + error_setg_errno(errp, -ret, "Failed to inactivate all nodes"); | ||
150 | + } | ||
151 | + } | ||
152 | + } else { | ||
153 | + BlockDriverState *bs = bdrv_find_node(node_name); | ||
154 | + if (!bs) { | ||
155 | + error_setg(errp, "Failed to find node with node-name='%s'", | ||
156 | + node_name); | ||
157 | + return; | ||
158 | + } | ||
159 | + | ||
160 | + if (active) { | ||
161 | + bdrv_activate(bs, errp); | ||
162 | + } else { | ||
163 | + bdrv_inactivate(bs, errp); | ||
164 | + } | ||
165 | + } | ||
166 | +} | ||
167 | + | ||
168 | static BdrvChild * GRAPH_RDLOCK | ||
169 | bdrv_find_child(BlockDriverState *parent_bs, const char *child_name) | ||
170 | { | ||
70 | -- | 171 | -- |
71 | 2.20.1 | 172 | 2.48.1 |
72 | |||
73 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Device models have a relatively complex way to set up their block | ||
2 | backends, in which blk_attach_dev() sets blk->disable_perm = true. | ||
3 | We want to support inactive images in exports, too, so that | ||
4 | qemu-storage-daemon can be used with migration. Because they don't use | ||
5 | blk_attach_dev(), they need another way to set this flag. The most | ||
6 | convenient is to do this automatically when an inactive node is attached | ||
7 | to a BlockBackend that can be inactivated. | ||
1 | 8 | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
11 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
12 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
13 | Message-ID: <20250204211407.381505-10-kwolf@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | block/block-backend.c | 14 ++++++++++++-- | ||
17 | 1 file changed, 12 insertions(+), 2 deletions(-) | ||
18 | |||
19 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
20 | index XXXXXXX..XXXXXXX 100644 | ||
21 | --- a/block/block-backend.c | ||
22 | +++ b/block/block-backend.c | ||
23 | @@ -XXX,XX +XXX,XX @@ void blk_remove_bs(BlockBackend *blk) | ||
24 | int blk_insert_bs(BlockBackend *blk, BlockDriverState *bs, Error **errp) | ||
25 | { | ||
26 | ThrottleGroupMember *tgm = &blk->public.throttle_group_member; | ||
27 | + uint64_t perm, shared_perm; | ||
28 | |||
29 | GLOBAL_STATE_CODE(); | ||
30 | bdrv_ref(bs); | ||
31 | bdrv_graph_wrlock(); | ||
32 | + | ||
33 | + if ((bs->open_flags & BDRV_O_INACTIVE) && blk_can_inactivate(blk)) { | ||
34 | + blk->disable_perm = true; | ||
35 | + perm = 0; | ||
36 | + shared_perm = BLK_PERM_ALL; | ||
37 | + } else { | ||
38 | + perm = blk->perm; | ||
39 | + shared_perm = blk->shared_perm; | ||
40 | + } | ||
41 | + | ||
42 | blk->root = bdrv_root_attach_child(bs, "root", &child_root, | ||
43 | BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, | ||
44 | - blk->perm, blk->shared_perm, | ||
45 | - blk, errp); | ||
46 | + perm, shared_perm, blk, errp); | ||
47 | bdrv_graph_wrunlock(); | ||
48 | if (blk->root == NULL) { | ||
49 | return -EPERM; | ||
50 | -- | ||
51 | 2.48.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Currently, block exports can't handle inactive images correctly. | ||
2 | Incoming write requests would run into assertion failures. Make sure | ||
3 | that we return an error when creating an export can't activate the | ||
4 | image. | ||
1 | 5 | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
10 | Message-ID: <20250204211407.381505-11-kwolf@redhat.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | --- | ||
13 | block/export/export.c | 6 +++++- | ||
14 | 1 file changed, 5 insertions(+), 1 deletion(-) | ||
15 | |||
16 | diff --git a/block/export/export.c b/block/export/export.c | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/block/export/export.c | ||
19 | +++ b/block/export/export.c | ||
20 | @@ -XXX,XX +XXX,XX @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) | ||
21 | * ctx was acquired in the caller. | ||
22 | */ | ||
23 | bdrv_graph_rdlock_main_loop(); | ||
24 | - bdrv_activate(bs, NULL); | ||
25 | + ret = bdrv_activate(bs, errp); | ||
26 | + if (ret < 0) { | ||
27 | + bdrv_graph_rdunlock_main_loop(); | ||
28 | + goto fail; | ||
29 | + } | ||
30 | bdrv_graph_rdunlock_main_loop(); | ||
31 | |||
32 | perm = BLK_PERM_CONSISTENT_READ; | ||
33 | -- | ||
34 | 2.48.1 | diff view generated by jsdifflib |
1 | external_snapshot_prepare() tries to move the overlay to the AioContext | 1 | So far the assumption has always been that if we try to inactivate a |
---|---|---|---|
2 | of the backing file (the snapshotted node). However, it's possible that | 2 | node, it is already idle. This doesn't hold true any more if we allow |
3 | this doesn't work, but the backing file can instead be moved to the | 3 | inactivating exported nodes because we can't know when new external |
4 | overlay's AioContext (e.g. opening the backing chain for a mirror | 4 | requests come in. |
5 | target). | ||
6 | 5 | ||
7 | bdrv_append() already indirectly uses bdrv_attach_node(), which takes | 6 | Drain the node around setting BDRV_O_INACTIVE so that requests can't |
8 | care to move nodes to make sure they use the same AioContext and which | 7 | start operating on an active node and then in the middle it suddenly |
9 | tries both directions. | 8 | becomes inactive. With this change, it's enough for exports to check |
10 | 9 | for new requests that they operate on an active node (or, like reads, | |
11 | So the problem has a simple fix: Just delete the unnecessary extra | 10 | are allowed even on an inactive node). |
12 | bdrv_try_set_aio_context() call in external_snapshot_prepare() and | ||
13 | instead assert in bdrv_append() that both nodes were indeed moved to the | ||
14 | same AioContext. | ||
15 | 11 | ||
16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
17 | Message-Id: <20200310113831.27293-6-kwolf@redhat.com> | 13 | Acked-by: Fabiano Rosas <farosas@suse.de> |
18 | Tested-by: Peter Krempa <pkrempa@redhat.com> | 14 | Message-ID: <20250204211407.381505-12-kwolf@redhat.com> |
15 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
16 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
19 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
20 | --- | 18 | --- |
21 | block.c | 1 + | 19 | block.c | 2 ++ |
22 | blockdev.c | 16 ---------------- | 20 | 1 file changed, 2 insertions(+) |
23 | 2 files changed, 1 insertion(+), 16 deletions(-) | ||
24 | 21 | ||
25 | diff --git a/block.c b/block.c | 22 | diff --git a/block.c b/block.c |
26 | index XXXXXXX..XXXXXXX 100644 | 23 | index XXXXXXX..XXXXXXX 100644 |
27 | --- a/block.c | 24 | --- a/block.c |
28 | +++ b/block.c | 25 | +++ b/block.c |
29 | @@ -XXX,XX +XXX,XX @@ void bdrv_replace_node(BlockDriverState *from, BlockDriverState *to, | 26 | @@ -XXX,XX +XXX,XX @@ bdrv_inactivate_recurse(BlockDriverState *bs, bool top_level) |
30 | bdrv_ref(from); | 27 | return -EPERM; |
31 | |||
32 | assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | ||
33 | + assert(bdrv_get_aio_context(from) == bdrv_get_aio_context(to)); | ||
34 | bdrv_drained_begin(from); | ||
35 | |||
36 | /* Put all parents into @list and calculate their cumulative permissions */ | ||
37 | diff --git a/blockdev.c b/blockdev.c | ||
38 | index XXXXXXX..XXXXXXX 100644 | ||
39 | --- a/blockdev.c | ||
40 | +++ b/blockdev.c | ||
41 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common, | ||
42 | DO_UPCAST(ExternalSnapshotState, common, common); | ||
43 | TransactionAction *action = common->action; | ||
44 | AioContext *aio_context; | ||
45 | - AioContext *old_context; | ||
46 | uint64_t perm, shared; | ||
47 | - int ret; | ||
48 | |||
49 | /* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar | ||
50 | * purpose but a different set of parameters */ | ||
51 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common, | ||
52 | goto out; | ||
53 | } | 28 | } |
54 | 29 | ||
55 | - /* Honor bdrv_try_set_aio_context() context acquisition requirements. */ | 30 | + bdrv_drained_begin(bs); |
56 | - old_context = bdrv_get_aio_context(state->new_bs); | 31 | bs->open_flags |= BDRV_O_INACTIVE; |
57 | - aio_context_release(aio_context); | 32 | + bdrv_drained_end(bs); |
58 | - aio_context_acquire(old_context); | 33 | |
59 | - | 34 | /* |
60 | - ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp); | 35 | * Update permissions, they may differ for inactive nodes. |
61 | - | ||
62 | - aio_context_release(old_context); | ||
63 | - aio_context_acquire(aio_context); | ||
64 | - | ||
65 | - if (ret < 0) { | ||
66 | - goto out; | ||
67 | - } | ||
68 | - | ||
69 | /* This removes our old bs and adds the new bs. This is an operation that | ||
70 | * can fail, so we need to do it in .prepare; undoing it for abort is | ||
71 | * always possible. */ | ||
72 | -- | 36 | -- |
73 | 2.20.1 | 37 | 2.48.1 |
74 | |||
75 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Add an option in BlockExportOptions to allow creating an export on an | ||
2 | inactive node without activating the node. This mode needs to be | ||
3 | explicitly supported by the export type (so that it doesn't perform any | ||
4 | operations that are forbidden for inactive nodes), so this patch alone | ||
5 | doesn't allow this option to be successfully used yet. | ||
1 | 6 | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | Acked-by: Fabiano Rosas <farosas@suse.de> | ||
9 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
10 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | Message-ID: <20250204211407.381505-13-kwolf@redhat.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | qapi/block-export.json | 10 +++++++++- | ||
15 | include/block/export.h | 3 +++ | ||
16 | block/export/export.c | 31 +++++++++++++++++++++---------- | ||
17 | 3 files changed, 33 insertions(+), 11 deletions(-) | ||
18 | |||
19 | diff --git a/qapi/block-export.json b/qapi/block-export.json | ||
20 | index XXXXXXX..XXXXXXX 100644 | ||
21 | --- a/qapi/block-export.json | ||
22 | +++ b/qapi/block-export.json | ||
23 | @@ -XXX,XX +XXX,XX @@ | ||
24 | # cannot be moved to the iothread. The default is false. | ||
25 | # (since: 5.2) | ||
26 | # | ||
27 | +# @allow-inactive: If true, the export allows the exported node to be inactive. | ||
28 | +# If it is created for an inactive block node, the node remains inactive. If | ||
29 | +# the export type doesn't support running on an inactive node, an error is | ||
30 | +# returned. If false, inactive block nodes are automatically activated before | ||
31 | +# creating the export and trying to inactivate them later fails. | ||
32 | +# (since: 10.0; default: false) | ||
33 | +# | ||
34 | # Since: 4.2 | ||
35 | ## | ||
36 | { 'union': 'BlockExportOptions', | ||
37 | @@ -XXX,XX +XXX,XX @@ | ||
38 | '*iothread': 'str', | ||
39 | 'node-name': 'str', | ||
40 | '*writable': 'bool', | ||
41 | - '*writethrough': 'bool' }, | ||
42 | + '*writethrough': 'bool', | ||
43 | + '*allow-inactive': 'bool' }, | ||
44 | 'discriminator': 'type', | ||
45 | 'data': { | ||
46 | 'nbd': 'BlockExportOptionsNbd', | ||
47 | diff --git a/include/block/export.h b/include/block/export.h | ||
48 | index XXXXXXX..XXXXXXX 100644 | ||
49 | --- a/include/block/export.h | ||
50 | +++ b/include/block/export.h | ||
51 | @@ -XXX,XX +XXX,XX @@ typedef struct BlockExportDriver { | ||
52 | */ | ||
53 | size_t instance_size; | ||
54 | |||
55 | + /* True if the export type supports running on an inactive node */ | ||
56 | + bool supports_inactive; | ||
57 | + | ||
58 | /* Creates and starts a new block export */ | ||
59 | int (*create)(BlockExport *, BlockExportOptions *, Error **); | ||
60 | |||
61 | diff --git a/block/export/export.c b/block/export/export.c | ||
62 | index XXXXXXX..XXXXXXX 100644 | ||
63 | --- a/block/export/export.c | ||
64 | +++ b/block/export/export.c | ||
65 | @@ -XXX,XX +XXX,XX @@ static const BlockExportDriver *blk_exp_find_driver(BlockExportType type) | ||
66 | BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) | ||
67 | { | ||
68 | bool fixed_iothread = export->has_fixed_iothread && export->fixed_iothread; | ||
69 | + bool allow_inactive = export->has_allow_inactive && export->allow_inactive; | ||
70 | const BlockExportDriver *drv; | ||
71 | BlockExport *exp = NULL; | ||
72 | BlockDriverState *bs; | ||
73 | @@ -XXX,XX +XXX,XX @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) | ||
74 | } | ||
75 | } | ||
76 | |||
77 | - /* | ||
78 | - * Block exports are used for non-shared storage migration. Make sure | ||
79 | - * that BDRV_O_INACTIVE is cleared and the image is ready for write | ||
80 | - * access since the export could be available before migration handover. | ||
81 | - * ctx was acquired in the caller. | ||
82 | - */ | ||
83 | bdrv_graph_rdlock_main_loop(); | ||
84 | - ret = bdrv_activate(bs, errp); | ||
85 | - if (ret < 0) { | ||
86 | - bdrv_graph_rdunlock_main_loop(); | ||
87 | - goto fail; | ||
88 | + if (allow_inactive) { | ||
89 | + if (!drv->supports_inactive) { | ||
90 | + error_setg(errp, "Export type does not support inactive exports"); | ||
91 | + bdrv_graph_rdunlock_main_loop(); | ||
92 | + goto fail; | ||
93 | + } | ||
94 | + } else { | ||
95 | + /* | ||
96 | + * Block exports are used for non-shared storage migration. Make sure | ||
97 | + * that BDRV_O_INACTIVE is cleared and the image is ready for write | ||
98 | + * access since the export could be available before migration handover. | ||
99 | + */ | ||
100 | + ret = bdrv_activate(bs, errp); | ||
101 | + if (ret < 0) { | ||
102 | + bdrv_graph_rdunlock_main_loop(); | ||
103 | + goto fail; | ||
104 | + } | ||
105 | } | ||
106 | bdrv_graph_rdunlock_main_loop(); | ||
107 | |||
108 | @@ -XXX,XX +XXX,XX @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error **errp) | ||
109 | if (!fixed_iothread) { | ||
110 | blk_set_allow_aio_context_change(blk, true); | ||
111 | } | ||
112 | + if (allow_inactive) { | ||
113 | + blk_set_force_allow_inactivate(blk); | ||
114 | + } | ||
115 | |||
116 | ret = blk_insert_bs(blk, bs, errp); | ||
117 | if (ret < 0) { | ||
118 | -- | ||
119 | 2.48.1 | diff view generated by jsdifflib |
1 | From: Daniel Henrique Barboza <danielhb413@gmail.com> | 1 | In order to support running an NBD export on inactive nodes, we must |
---|---|---|---|
2 | make sure to return errors for any operations that aren't allowed on | ||
3 | inactive nodes. Reads are the only operation we know we need for | ||
4 | inactive images, so to err on the side of caution, return errors for | ||
5 | everything else, even if some operations could possibly be okay. | ||
2 | 6 | ||
3 | When using a non-UTF8 secret to create a volume using qemu-img, the | 7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
4 | following error happens: | 8 | Acked-by: Fabiano Rosas <farosas@suse.de> |
5 | 9 | Message-ID: <20250204211407.381505-14-kwolf@redhat.com> | |
6 | $ qemu-img create -f luks --object secret,id=vol_1_encrypt0,file=vol_resize_pool.vol_1.secret.qzVQrI -o key-secret=vol_1_encrypt0 /var/tmp/pool_target/vol_1 10240K | 10 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
7 | 11 | Reviewed-by: Eric Blake <eblake@redhat.com> | |
8 | Formatting '/var/tmp/pool_target/vol_1', fmt=luks size=10485760 key-secret=vol_1_encrypt0 | ||
9 | qemu-img: /var/tmp/pool_target/vol_1: Data from secret vol_1_encrypt0 is not valid UTF-8 | ||
10 | |||
11 | However, the created file '/var/tmp/pool_target/vol_1' is left behind in the | ||
12 | file system after the failure. This behavior can be observed when creating | ||
13 | the volume using Libvirt, via 'virsh vol-create', and then getting "volume | ||
14 | target path already exist" errors when trying to re-create the volume. | ||
15 | |||
16 | The volume file is created inside block_crypto_co_create_opts_luks(), in | ||
17 | block/crypto.c. If the bdrv_create_file() call is successful but any | ||
18 | succeeding step fails*, the existing 'fail' label does not take into | ||
19 | account the created file, leaving it behind. | ||
20 | |||
21 | This patch changes block_crypto_co_create_opts_luks() to delete | ||
22 | 'filename' in case of failure. A failure in this point means that | ||
23 | the volume is now truncated/corrupted, so even if 'filename' was an | ||
24 | existing volume before calling qemu-img, it is now unusable. Deleting | ||
25 | the file it is not much worse than leaving it in the filesystem in | ||
26 | this scenario, and we don't have to deal with checking the file | ||
27 | pre-existence in the code. | ||
28 | |||
29 | * in our case, block_crypto_co_create_generic calls qcrypto_block_create, | ||
30 | which calls qcrypto_block_luks_create, and this function fails when | ||
31 | calling qcrypto_secret_lookup_as_utf8. | ||
32 | |||
33 | Reported-by: Srikanth Aithal <bssrikanth@in.ibm.com> | ||
34 | Suggested-by: Kevin Wolf <kwolf@redhat.com> | ||
35 | Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> | ||
36 | Message-Id: <20200130213907.2830642-4-danielhb413@gmail.com> | ||
37 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
38 | --- | 13 | --- |
39 | block/crypto.c | 18 ++++++++++++++++++ | 14 | nbd/server.c | 17 +++++++++++++++++ |
40 | 1 file changed, 18 insertions(+) | 15 | 1 file changed, 17 insertions(+) |
41 | 16 | ||
42 | diff --git a/block/crypto.c b/block/crypto.c | 17 | diff --git a/nbd/server.c b/nbd/server.c |
43 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
44 | --- a/block/crypto.c | 19 | --- a/nbd/server.c |
45 | +++ b/block/crypto.c | 20 | +++ b/nbd/server.c |
46 | @@ -XXX,XX +XXX,XX @@ | 21 | @@ -XXX,XX +XXX,XX @@ static void nbd_export_delete(BlockExport *blk_exp) |
47 | #include "qapi/error.h" | 22 | const BlockExportDriver blk_exp_nbd = { |
48 | #include "qemu/module.h" | 23 | .type = BLOCK_EXPORT_TYPE_NBD, |
49 | #include "qemu/option.h" | 24 | .instance_size = sizeof(NBDExport), |
50 | +#include "qemu/cutils.h" | 25 | + .supports_inactive = true, |
51 | #include "crypto.h" | 26 | .create = nbd_export_create, |
52 | 27 | .delete = nbd_export_delete, | |
53 | typedef struct BlockCrypto BlockCrypto; | 28 | .request_shutdown = nbd_export_request_shutdown, |
54 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn block_crypto_co_create_opts_luks(const char *filename, | 29 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int nbd_handle_request(NBDClient *client, |
55 | 30 | NBDExport *exp = client->exp; | |
56 | ret = 0; | 31 | char *msg; |
57 | fail: | 32 | size_t i; |
58 | + /* | 33 | + bool inactive; |
59 | + * If an error occurred, delete 'filename'. Even if the file existed | 34 | + |
60 | + * beforehand, it has been truncated and corrupted in the process. | 35 | + WITH_GRAPH_RDLOCK_GUARD() { |
61 | + */ | 36 | + inactive = bdrv_is_inactive(blk_bs(exp->common.blk)); |
62 | + if (ret && bs) { | 37 | + if (inactive) { |
63 | + Error *local_delete_err = NULL; | 38 | + switch (request->type) { |
64 | + int r_del = bdrv_co_delete_file(bs, &local_delete_err); | 39 | + case NBD_CMD_READ: |
65 | + /* | 40 | + /* These commands are allowed on inactive nodes */ |
66 | + * ENOTSUP will happen if the block driver doesn't support | 41 | + break; |
67 | + * the 'bdrv_co_delete_file' interface. This is a predictable | 42 | + default: |
68 | + * scenario and shouldn't be reported back to the user. | 43 | + /* Return an error for the rest */ |
69 | + */ | 44 | + return nbd_send_generic_reply(client, request, -EPERM, |
70 | + if ((r_del < 0) && (r_del != -ENOTSUP)) { | 45 | + "export is inactive", errp); |
71 | + error_report_err(local_delete_err); | 46 | + } |
72 | + } | 47 | + } |
73 | + } | 48 | + } |
74 | + | 49 | |
75 | bdrv_unref(bs); | 50 | switch (request->type) { |
76 | qapi_free_QCryptoBlockCreateOptions(create_opts); | 51 | case NBD_CMD_CACHE: |
77 | qobject_unref(cryptoopts); | ||
78 | -- | 52 | -- |
79 | 2.20.1 | 53 | 2.48.1 |
80 | |||
81 | diff view generated by jsdifflib |
1 | The newly tested scenario is a common live storage migration scenario: | 1 | The open-coded form of this filter has been copied into enough tests |
---|---|---|---|
2 | The target node is opened without a backing file so that the active | 2 | that it's better to move it into iotests.py. |
3 | layer is mirrored while its backing chain can be copied in the | ||
4 | background. | ||
5 | |||
6 | The backing chain should be attached to the mirror target node when | ||
7 | finalising the job, just before switching the users of the source node | ||
8 | to the new copy (at which point the mirror job still has a reference to | ||
9 | the node). drive-mirror did this automatically, but with blockdev-mirror | ||
10 | this is the job of the QMP client. | ||
11 | |||
12 | This patch adds test cases for two ways to achieve the desired result, | ||
13 | using either x-blockdev-reopen or blockdev-snapshot. | ||
14 | 3 | ||
15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
16 | Message-Id: <20200310113831.27293-5-kwolf@redhat.com> | 5 | Acked-by: Fabiano Rosas <farosas@suse.de> |
17 | Reviewed-by: Peter Krempa <pkrempa@redhat.com> | 6 | Reviewed-by: Eric Blake <eblake@redhat.com> |
7 | Message-ID: <20250204211407.381505-15-kwolf@redhat.com> | ||
8 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
18 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
19 | --- | 10 | --- |
20 | tests/qemu-iotests/155 | 56 ++++++++++++++++++++++++++++++++++---- | 11 | tests/qemu-iotests/iotests.py | 4 ++++ |
21 | tests/qemu-iotests/155.out | 4 +-- | 12 | tests/qemu-iotests/041 | 4 +--- |
22 | 2 files changed, 53 insertions(+), 7 deletions(-) | 13 | tests/qemu-iotests/165 | 4 +--- |
14 | tests/qemu-iotests/tests/copy-before-write | 3 +-- | ||
15 | tests/qemu-iotests/tests/migrate-bitmaps-test | 7 +++---- | ||
16 | 5 files changed, 10 insertions(+), 12 deletions(-) | ||
23 | 17 | ||
24 | diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155 | 18 | diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py |
19 | index XXXXXXX..XXXXXXX 100644 | ||
20 | --- a/tests/qemu-iotests/iotests.py | ||
21 | +++ b/tests/qemu-iotests/iotests.py | ||
22 | @@ -XXX,XX +XXX,XX @@ def _filter(_key, value): | ||
23 | def filter_nbd_exports(output: str) -> str: | ||
24 | return re.sub(r'((min|opt|max) block): [0-9]+', r'\1: XXX', output) | ||
25 | |||
26 | +def filter_qtest(output: str) -> str: | ||
27 | + output = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', output) | ||
28 | + output = re.sub(r'\n?\[I \+\d+\.\d+\] CLOSED\n?$', '', output) | ||
29 | + return output | ||
30 | |||
31 | Msg = TypeVar('Msg', Dict[str, Any], List[Any], str) | ||
32 | |||
33 | diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 | ||
25 | index XXXXXXX..XXXXXXX 100755 | 34 | index XXXXXXX..XXXXXXX 100755 |
26 | --- a/tests/qemu-iotests/155 | 35 | --- a/tests/qemu-iotests/041 |
27 | +++ b/tests/qemu-iotests/155 | 36 | +++ b/tests/qemu-iotests/041 |
28 | @@ -XXX,XX +XXX,XX @@ target_img = os.path.join(iotests.test_dir, 'target.' + iotests.imgfmt) | 37 | @@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase): |
29 | # image during runtime, only makes sense if | 38 | |
30 | # target_blockdev_backing is not None | 39 | # Check the full error message now |
31 | # (None: same as target_backing) | 40 | self.vm.shutdown() |
32 | +# target_open_with_backing: If True, the target image is added with its backing | 41 | - log = self.vm.get_log() |
33 | +# chain opened right away. If False, blockdev-add | 42 | - log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) |
34 | +# opens it without a backing file and job completion | 43 | + log = iotests.filter_qtest(self.vm.get_log()) |
35 | +# is supposed to open the backing chain. | 44 | log = re.sub(r'^Formatting.*\n', '', log) |
36 | 45 | - log = re.sub(r'\n\[I \+\d+\.\d+\] CLOSED\n?$', '', log) | |
37 | class BaseClass(iotests.QMPTestCase): | 46 | log = re.sub(r'^%s: ' % os.path.basename(iotests.qemu_prog), '', log) |
38 | target_blockdev_backing = None | 47 | |
39 | target_real_backing = None | 48 | self.assertEqual(log, |
40 | + target_open_with_backing = True | 49 | diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165 |
41 | 50 | index XXXXXXX..XXXXXXX 100755 | |
42 | def setUp(self): | 51 | --- a/tests/qemu-iotests/165 |
43 | qemu_img('create', '-f', iotests.imgfmt, back0_img, '1440K') | 52 | +++ b/tests/qemu-iotests/165 |
44 | @@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase): | 53 | @@ -XXX,XX +XXX,XX @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): |
45 | options = { 'node-name': 'target', | 54 | self.vm.shutdown() |
46 | 'driver': iotests.imgfmt, | 55 | |
47 | 'file': { 'driver': 'file', | 56 | #catch 'Persistent bitmaps are lost' possible error |
48 | + 'node-name': 'target-file', | 57 | - log = self.vm.get_log() |
49 | 'filename': target_img } } | 58 | - log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) |
50 | - if self.target_blockdev_backing: | 59 | - log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log) |
51 | - options['backing'] = self.target_blockdev_backing | 60 | + log = iotests.filter_qtest(self.vm.get_log()) |
52 | + | 61 | if log: |
53 | + if not self.target_open_with_backing: | 62 | print(log) |
54 | + options['backing'] = None | 63 | |
55 | + elif self.target_blockdev_backing: | 64 | diff --git a/tests/qemu-iotests/tests/copy-before-write b/tests/qemu-iotests/tests/copy-before-write |
56 | + options['backing'] = self.target_blockdev_backing | 65 | index XXXXXXX..XXXXXXX 100755 |
57 | 66 | --- a/tests/qemu-iotests/tests/copy-before-write | |
58 | result = self.vm.qmp('blockdev-add', **options) | 67 | +++ b/tests/qemu-iotests/tests/copy-before-write |
59 | self.assert_qmp(result, 'return', {}) | 68 | @@ -XXX,XX +XXX,XX @@ class TestCbwError(iotests.QMPTestCase): |
60 | @@ -XXX,XX +XXX,XX @@ class BaseClass(iotests.QMPTestCase): | 69 | |
61 | # cmd: Mirroring command to execute, either drive-mirror or blockdev-mirror | 70 | self.vm.shutdown() |
62 | 71 | log = self.vm.get_log() | |
63 | class MirrorBaseClass(BaseClass): | 72 | - log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) |
64 | + def openBacking(self): | 73 | - log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log) |
65 | + pass | 74 | + log = iotests.filter_qtest(log) |
66 | + | 75 | log = iotests.filter_qemu_io(log) |
67 | def runMirror(self, sync): | 76 | return log |
68 | if self.cmd == 'blockdev-mirror': | 77 | |
69 | result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source', | 78 | diff --git a/tests/qemu-iotests/tests/migrate-bitmaps-test b/tests/qemu-iotests/tests/migrate-bitmaps-test |
70 | - sync=sync, target='target') | 79 | index XXXXXXX..XXXXXXX 100755 |
71 | + sync=sync, target='target', | 80 | --- a/tests/qemu-iotests/tests/migrate-bitmaps-test |
72 | + auto_finalize=False) | 81 | +++ b/tests/qemu-iotests/tests/migrate-bitmaps-test |
73 | else: | 82 | @@ -XXX,XX +XXX,XX @@ class TestDirtyBitmapMigration(iotests.QMPTestCase): |
74 | if self.existing: | 83 | |
75 | mode = 'existing' | 84 | # catch 'Could not reopen qcow2 layer: Bitmap already exists' |
76 | @@ -XXX,XX +XXX,XX @@ class MirrorBaseClass(BaseClass): | 85 | # possible error |
77 | result = self.vm.qmp(self.cmd, job_id='mirror-job', device='source', | 86 | - log = self.vm_a.get_log() |
78 | sync=sync, target=target_img, | 87 | - log = re.sub(r'^\[I \d+\.\d+\] OPENED\n', '', log) |
79 | format=iotests.imgfmt, mode=mode, | 88 | - log = re.sub(r'^(wrote .* bytes at offset .*\n.*KiB.*ops.*sec.*\n){3}', |
80 | - node_name='target') | 89 | + log = iotests.filter_qtest(self.vm_a.get_log()) |
81 | + node_name='target', auto_finalize=False) | 90 | + log = re.sub(r'^(wrote .* bytes at offset .*\n' |
82 | 91 | + r'.*KiB.*ops.*sec.*\n?){3}', | |
83 | self.assert_qmp(result, 'return', {}) | 92 | '', log) |
84 | 93 | - log = re.sub(r'\[I \+\d+\.\d+\] CLOSED\n?$', '', log) | |
85 | - self.complete_and_wait('mirror-job') | 94 | self.assertEqual(log, '') |
86 | + self.vm.run_job('mirror-job', use_log=False, auto_finalize=False, | 95 | |
87 | + pre_finalize=self.openBacking, auto_dismiss=True) | 96 | # test that bitmap is still persistent |
88 | |||
89 | def testFull(self): | ||
90 | self.runMirror('full') | ||
91 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorForcedBacking(MirrorBaseClass): | ||
92 | target_blockdev_backing = { 'driver': 'null-co' } | ||
93 | target_real_backing = 'null-co://' | ||
94 | |||
95 | +# Attach the backing chain only during completion, with blockdev-reopen | ||
96 | +class TestBlockdevMirrorReopen(MirrorBaseClass): | ||
97 | + cmd = 'blockdev-mirror' | ||
98 | + existing = True | ||
99 | + target_backing = 'null-co://' | ||
100 | + target_open_with_backing = False | ||
101 | + | ||
102 | + def openBacking(self): | ||
103 | + if not self.target_open_with_backing: | ||
104 | + result = self.vm.qmp('blockdev-add', node_name="backing", | ||
105 | + driver="null-co") | ||
106 | + self.assert_qmp(result, 'return', {}) | ||
107 | + result = self.vm.qmp('x-blockdev-reopen', node_name="target", | ||
108 | + driver=iotests.imgfmt, file="target-file", | ||
109 | + backing="backing") | ||
110 | + self.assert_qmp(result, 'return', {}) | ||
111 | + | ||
112 | +# Attach the backing chain only during completion, with blockdev-snapshot | ||
113 | +class TestBlockdevMirrorSnapshot(MirrorBaseClass): | ||
114 | + cmd = 'blockdev-mirror' | ||
115 | + existing = True | ||
116 | + target_backing = 'null-co://' | ||
117 | + target_open_with_backing = False | ||
118 | + | ||
119 | + def openBacking(self): | ||
120 | + if not self.target_open_with_backing: | ||
121 | + result = self.vm.qmp('blockdev-add', node_name="backing", | ||
122 | + driver="null-co") | ||
123 | + self.assert_qmp(result, 'return', {}) | ||
124 | + result = self.vm.qmp('blockdev-snapshot', node="backing", | ||
125 | + overlay="target") | ||
126 | + self.assert_qmp(result, 'return', {}) | ||
127 | |||
128 | class TestCommit(BaseClass): | ||
129 | existing = False | ||
130 | diff --git a/tests/qemu-iotests/155.out b/tests/qemu-iotests/155.out | ||
131 | index XXXXXXX..XXXXXXX 100644 | ||
132 | --- a/tests/qemu-iotests/155.out | ||
133 | +++ b/tests/qemu-iotests/155.out | ||
134 | @@ -XXX,XX +XXX,XX @@ | ||
135 | -................... | ||
136 | +......................... | ||
137 | ---------------------------------------------------------------------- | ||
138 | -Ran 19 tests | ||
139 | +Ran 25 tests | ||
140 | |||
141 | OK | ||
142 | -- | 97 | -- |
143 | 2.20.1 | 98 | 2.48.1 |
144 | |||
145 | diff view generated by jsdifflib |
1 | From: Daniel Henrique Barboza <danielhb413@gmail.com> | 1 | Test that it's possible to migrate a VM that uses an image on shared |
---|---|---|---|
2 | storage through qemu-storage-daemon. | ||
2 | 3 | ||
3 | This patch adds a new test file to exercise the case where | 4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
4 | qemu-img fails to complete for the LUKS format when a non-UTF8 | 5 | Acked-by: Fabiano Rosas <farosas@suse.de> |
5 | secret is used. | 6 | Reviewed-by: Eric Blake <eblake@redhat.com> |
6 | 7 | Message-ID: <20250204211407.381505-16-kwolf@redhat.com> | |
7 | Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com> | 8 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
8 | Message-Id: <20200130213907.2830642-5-danielhb413@gmail.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
10 | --- | 10 | --- |
11 | tests/qemu-iotests/282 | 67 ++++++++++++++++++++++++++++++++++++++ | 11 | tests/qemu-iotests/tests/qsd-migrate | 140 +++++++++++++++++++++++ |
12 | tests/qemu-iotests/282.out | 11 +++++++ | 12 | tests/qemu-iotests/tests/qsd-migrate.out | 59 ++++++++++ |
13 | tests/qemu-iotests/group | 1 + | 13 | 2 files changed, 199 insertions(+) |
14 | 3 files changed, 79 insertions(+) | 14 | create mode 100755 tests/qemu-iotests/tests/qsd-migrate |
15 | create mode 100755 tests/qemu-iotests/282 | 15 | create mode 100644 tests/qemu-iotests/tests/qsd-migrate.out |
16 | create mode 100644 tests/qemu-iotests/282.out | ||
17 | 16 | ||
18 | diff --git a/tests/qemu-iotests/282 b/tests/qemu-iotests/282 | 17 | diff --git a/tests/qemu-iotests/tests/qsd-migrate b/tests/qemu-iotests/tests/qsd-migrate |
19 | new file mode 100755 | 18 | new file mode 100755 |
20 | index XXXXXXX..XXXXXXX | 19 | index XXXXXXX..XXXXXXX |
21 | --- /dev/null | 20 | --- /dev/null |
22 | +++ b/tests/qemu-iotests/282 | 21 | +++ b/tests/qemu-iotests/tests/qsd-migrate |
23 | @@ -XXX,XX +XXX,XX @@ | 22 | @@ -XXX,XX +XXX,XX @@ |
24 | +#!/usr/bin/env bash | 23 | +#!/usr/bin/env python3 |
25 | +# | 24 | +# group: rw quick |
26 | +# Test qemu-img file cleanup for LUKS when using a non-UTF8 secret | 25 | +# |
27 | +# | 26 | +# Copyright (C) Red Hat, Inc. |
28 | +# Copyright (C) 2020, IBM Corporation. | ||
29 | +# | 27 | +# |
30 | +# This program is free software; you can redistribute it and/or modify | 28 | +# This program is free software; you can redistribute it and/or modify |
31 | +# it under the terms of the GNU General Public License as published by | 29 | +# it under the terms of the GNU General Public License as published by |
32 | +# the Free Software Foundation; either version 2 of the License, or | 30 | +# the Free Software Foundation; either version 2 of the License, or |
33 | +# (at your option) any later version. | 31 | +# (at your option) any later version. |
... | ... | ||
38 | +# GNU General Public License for more details. | 36 | +# GNU General Public License for more details. |
39 | +# | 37 | +# |
40 | +# You should have received a copy of the GNU General Public License | 38 | +# You should have received a copy of the GNU General Public License |
41 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | 39 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. |
42 | +# | 40 | +# |
43 | + | 41 | +# Creator/Owner: Kevin Wolf <kwolf@redhat.com> |
44 | +seq=`basename $0` | 42 | + |
45 | +echo "QA output created by $seq" | 43 | +import iotests |
46 | + | 44 | + |
47 | +status=1 # failure is the default! | 45 | +from iotests import filter_qemu_io, filter_qtest |
48 | +TEST_IMAGE_FILE='vol.img' | 46 | + |
49 | + | 47 | +iotests.script_initialize(supported_fmts=['generic'], |
50 | +_cleanup() | 48 | + supported_protocols=['file'], |
51 | +{ | 49 | + supported_platforms=['linux']) |
52 | + _cleanup_test_img | 50 | + |
53 | + rm non_utf8_secret | 51 | +with iotests.FilePath('disk.img') as path, \ |
54 | + rm -f $TEST_IMAGE_FILE | 52 | + iotests.FilePath('nbd-src.sock', base_dir=iotests.sock_dir) as nbd_src, \ |
55 | +} | 53 | + iotests.FilePath('nbd-dst.sock', base_dir=iotests.sock_dir) as nbd_dst, \ |
56 | +trap "_cleanup; exit \$status" 0 1 2 3 15 | 54 | + iotests.FilePath('migrate.sock', base_dir=iotests.sock_dir) as mig_sock, \ |
57 | + | 55 | + iotests.VM(path_suffix="-src") as vm_src, \ |
58 | +# get standard environment, filters and checks | 56 | + iotests.VM(path_suffix="-dst") as vm_dst: |
59 | +. ./common.rc | 57 | + |
60 | +. ./common.filter | 58 | + img_size = '10M' |
61 | + | 59 | + |
62 | +_supported_fmt luks | 60 | + iotests.log('Preparing disk...') |
63 | +_supported_proto generic | 61 | + iotests.qemu_img_create('-f', iotests.imgfmt, path, img_size) |
64 | +_unsupported_proto vxhs | 62 | + |
65 | + | 63 | + iotests.log('Launching source QSD...') |
66 | +echo "== Create non-UTF8 secret ==" | 64 | + qsd_src = iotests.QemuStorageDaemon( |
67 | +echo -n -e '\x3a\x3c\x3b\xff' > non_utf8_secret | 65 | + '--blockdev', f'file,node-name=disk-file,filename={path}', |
68 | +SECRET="secret,id=sec0,file=non_utf8_secret" | 66 | + '--blockdev', f'{iotests.imgfmt},file=disk-file,node-name=disk-fmt', |
69 | + | 67 | + '--nbd-server', f'addr.type=unix,addr.path={nbd_src}', |
70 | +echo "== Throws an error because of invalid UTF-8 secret ==" | 68 | + '--export', 'nbd,id=exp0,node-name=disk-fmt,writable=true,' |
71 | +$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M | 69 | + 'allow-inactive=true', |
72 | + | 70 | + qmp=True, |
73 | +echo "== Image file should not exist after the error ==" | 71 | + ) |
74 | +if test -f "$TEST_IMAGE_FILE"; then | 72 | + |
75 | + exit 1 | 73 | + iotests.log('Launching source VM...') |
76 | +fi | 74 | + vm_src.add_args('-blockdev', f'nbd,node-name=disk,server.type=unix,' |
77 | + | 75 | + f'server.path={nbd_src},export=disk-fmt') |
78 | +echo "== Create a stub image file and run qemu-img again ==" | 76 | + vm_src.add_args('-device', 'virtio-blk,drive=disk,id=virtio0') |
79 | +touch $TEST_IMAGE_FILE | 77 | + vm_src.launch() |
80 | +$QEMU_IMG create -f $IMGFMT --object $SECRET -o "key-secret=sec0" $TEST_IMAGE_FILE 4M | 78 | + |
81 | + | 79 | + iotests.log('Launching destination QSD...') |
82 | +echo "== Pre-existing image file should also be deleted after the error ==" | 80 | + qsd_dst = iotests.QemuStorageDaemon( |
83 | +if test -f "$TEST_IMAGE_FILE"; then | 81 | + '--blockdev', f'file,node-name=disk-file,filename={path},active=off', |
84 | + exit 1 | 82 | + '--blockdev', f'{iotests.imgfmt},file=disk-file,node-name=disk-fmt,' |
85 | +fi | 83 | + f'active=off', |
86 | + | 84 | + '--nbd-server', f'addr.type=unix,addr.path={nbd_dst}', |
87 | +# success, all done | 85 | + '--export', 'nbd,id=exp0,node-name=disk-fmt,writable=true,' |
88 | +echo "*** done" | 86 | + 'allow-inactive=true', |
89 | +rm -f $seq.full | 87 | + qmp=True, |
90 | +status=0 | 88 | + instance_id='b', |
91 | diff --git a/tests/qemu-iotests/282.out b/tests/qemu-iotests/282.out | 89 | + ) |
90 | + | ||
91 | + iotests.log('Launching destination VM...') | ||
92 | + vm_dst.add_args('-blockdev', f'nbd,node-name=disk,server.type=unix,' | ||
93 | + f'server.path={nbd_dst},export=disk-fmt') | ||
94 | + vm_dst.add_args('-device', 'virtio-blk,drive=disk,id=virtio0') | ||
95 | + vm_dst.add_args('-incoming', f'unix:{mig_sock}') | ||
96 | + vm_dst.launch() | ||
97 | + | ||
98 | + iotests.log('\nTest I/O on the source') | ||
99 | + vm_src.hmp_qemu_io('virtio0/virtio-backend', 'write -P 0x11 0 4k', | ||
100 | + use_log=True, qdev=True) | ||
101 | + vm_src.hmp_qemu_io('virtio0/virtio-backend', 'read -P 0x11 0 4k', | ||
102 | + use_log=True, qdev=True) | ||
103 | + | ||
104 | + iotests.log('\nStarting migration...') | ||
105 | + | ||
106 | + mig_caps = [ | ||
107 | + {'capability': 'events', 'state': True}, | ||
108 | + {'capability': 'pause-before-switchover', 'state': True}, | ||
109 | + ] | ||
110 | + vm_src.qmp_log('migrate-set-capabilities', capabilities=mig_caps) | ||
111 | + vm_dst.qmp_log('migrate-set-capabilities', capabilities=mig_caps) | ||
112 | + vm_src.qmp_log('migrate', uri=f'unix:{mig_sock}', | ||
113 | + filters=[iotests.filter_qmp_testfiles]) | ||
114 | + | ||
115 | + vm_src.event_wait('MIGRATION', | ||
116 | + match={'data': {'status': 'pre-switchover'}}) | ||
117 | + | ||
118 | + iotests.log('\nPre-switchover: Reconfigure QSD instances') | ||
119 | + | ||
120 | + iotests.log(qsd_src.qmp('blockdev-set-active', {'active': False})) | ||
121 | + | ||
122 | + # Reading is okay from both sides while the image is inactive. Note that | ||
123 | + # the destination may have stale data until it activates the image, though. | ||
124 | + vm_src.hmp_qemu_io('virtio0/virtio-backend', 'read -P 0x11 0 4k', | ||
125 | + use_log=True, qdev=True) | ||
126 | + vm_dst.hmp_qemu_io('virtio0/virtio-backend', 'read 0 4k', | ||
127 | + use_log=True, qdev=True) | ||
128 | + | ||
129 | + iotests.log(qsd_dst.qmp('blockdev-set-active', {'active': True})) | ||
130 | + | ||
131 | + iotests.log('\nCompleting migration...') | ||
132 | + | ||
133 | + vm_src.qmp_log('migrate-continue', state='pre-switchover') | ||
134 | + vm_dst.event_wait('MIGRATION', match={'data': {'status': 'completed'}}) | ||
135 | + | ||
136 | + iotests.log('\nTest I/O on the destination') | ||
137 | + | ||
138 | + # Now the destination must see what the source wrote | ||
139 | + vm_dst.hmp_qemu_io('virtio0/virtio-backend', 'read -P 0x11 0 4k', | ||
140 | + use_log=True, qdev=True) | ||
141 | + | ||
142 | + # And be able to overwrite it | ||
143 | + vm_dst.hmp_qemu_io('virtio0/virtio-backend', 'write -P 0x22 0 4k', | ||
144 | + use_log=True, qdev=True) | ||
145 | + vm_dst.hmp_qemu_io('virtio0/virtio-backend', 'read -P 0x22 0 4k', | ||
146 | + use_log=True, qdev=True) | ||
147 | + | ||
148 | + iotests.log('\nDone') | ||
149 | + | ||
150 | + vm_src.shutdown() | ||
151 | + iotests.log('\n--- vm_src log ---') | ||
152 | + log = vm_src.get_log() | ||
153 | + if log: | ||
154 | + iotests.log(log, [filter_qtest, filter_qemu_io]) | ||
155 | + qsd_src.stop() | ||
156 | + | ||
157 | + vm_dst.shutdown() | ||
158 | + iotests.log('\n--- vm_dst log ---') | ||
159 | + log = vm_dst.get_log() | ||
160 | + if log: | ||
161 | + iotests.log(log, [filter_qtest, filter_qemu_io]) | ||
162 | + qsd_dst.stop() | ||
163 | diff --git a/tests/qemu-iotests/tests/qsd-migrate.out b/tests/qemu-iotests/tests/qsd-migrate.out | ||
92 | new file mode 100644 | 164 | new file mode 100644 |
93 | index XXXXXXX..XXXXXXX | 165 | index XXXXXXX..XXXXXXX |
94 | --- /dev/null | 166 | --- /dev/null |
95 | +++ b/tests/qemu-iotests/282.out | 167 | +++ b/tests/qemu-iotests/tests/qsd-migrate.out |
96 | @@ -XXX,XX +XXX,XX @@ | 168 | @@ -XXX,XX +XXX,XX @@ |
97 | +QA output created by 282 | 169 | +Preparing disk... |
98 | +== Create non-UTF8 secret == | 170 | +Launching source QSD... |
99 | +== Throws an error because of invalid UTF-8 secret == | 171 | +Launching source VM... |
100 | +qemu-img: vol.img: Data from secret sec0 is not valid UTF-8 | 172 | +Launching destination QSD... |
101 | +Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0 | 173 | +Launching destination VM... |
102 | +== Image file should not exist after the error == | 174 | + |
103 | +== Create a stub image file and run qemu-img again == | 175 | +Test I/O on the source |
104 | +qemu-img: vol.img: Data from secret sec0 is not valid UTF-8 | 176 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"write -P 0x11 0 4k\""}} |
105 | +Formatting 'vol.img', fmt=luks size=4194304 key-secret=sec0 | 177 | +{"return": ""} |
106 | +== Pre-existing image file should also be deleted after the error == | 178 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"read -P 0x11 0 4k\""}} |
107 | + *** done | 179 | +{"return": ""} |
108 | diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group | 180 | + |
109 | index XXXXXXX..XXXXXXX 100644 | 181 | +Starting migration... |
110 | --- a/tests/qemu-iotests/group | 182 | +{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [{"capability": "events", "state": true}, {"capability": "pause-before-switchover", "state": true}]}} |
111 | +++ b/tests/qemu-iotests/group | 183 | +{"return": {}} |
112 | @@ -XXX,XX +XXX,XX @@ | 184 | +{"execute": "migrate-set-capabilities", "arguments": {"capabilities": [{"capability": "events", "state": true}, {"capability": "pause-before-switchover", "state": true}]}} |
113 | 279 rw backing quick | 185 | +{"return": {}} |
114 | 280 rw migration quick | 186 | +{"execute": "migrate", "arguments": {"uri": "unix:SOCK_DIR/PID-migrate.sock"}} |
115 | 281 rw quick | 187 | +{"return": {}} |
116 | +282 rw img quick | 188 | + |
117 | 283 auto quick | 189 | +Pre-switchover: Reconfigure QSD instances |
118 | 284 rw | 190 | +{"return": {}} |
119 | 286 rw quick | 191 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"read -P 0x11 0 4k\""}} |
192 | +{"return": ""} | ||
193 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"read 0 4k\""}} | ||
194 | +{"return": ""} | ||
195 | +{"return": {}} | ||
196 | + | ||
197 | +Completing migration... | ||
198 | +{"execute": "migrate-continue", "arguments": {"state": "pre-switchover"}} | ||
199 | +{"return": {}} | ||
200 | + | ||
201 | +Test I/O on the destination | ||
202 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"read -P 0x11 0 4k\""}} | ||
203 | +{"return": ""} | ||
204 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"write -P 0x22 0 4k\""}} | ||
205 | +{"return": ""} | ||
206 | +{"execute": "human-monitor-command", "arguments": {"command-line": "qemu-io -d virtio0/virtio-backend \"read -P 0x22 0 4k\""}} | ||
207 | +{"return": ""} | ||
208 | + | ||
209 | +Done | ||
210 | + | ||
211 | +--- vm_src log --- | ||
212 | +wrote 4096/4096 bytes at offset 0 | ||
213 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
214 | +read 4096/4096 bytes at offset 0 | ||
215 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
216 | +read 4096/4096 bytes at offset 0 | ||
217 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
218 | + | ||
219 | +--- vm_dst log --- | ||
220 | +read 4096/4096 bytes at offset 0 | ||
221 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
222 | +read 4096/4096 bytes at offset 0 | ||
223 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
224 | +wrote 4096/4096 bytes at offset 0 | ||
225 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
226 | +read 4096/4096 bytes at offset 0 | ||
227 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
120 | -- | 228 | -- |
121 | 2.20.1 | 229 | 2.48.1 |
122 | |||
123 | diff view generated by jsdifflib |
1 | The 'job-complete' QMP command should be run with qmp() rather than | 1 | This tests different types of operations on inactive block nodes |
---|---|---|---|
2 | qmp_log() if use_log=False is passed. | 2 | (including graph changes, block jobs and NBD exports) to make sure that |
3 | users manually activating and inactivating nodes doesn't break things. | ||
4 | |||
5 | Support for inactive nodes in other export types will have to come with | ||
6 | separate test cases because they have different dependencies like blkio | ||
7 | or root permissions and we don't want to disable this basic test when | ||
8 | they are not fulfilled. | ||
3 | 9 | ||
4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
5 | Message-Id: <20200310113831.27293-4-kwolf@redhat.com> | 11 | Acked-by: Fabiano Rosas <farosas@suse.de> |
6 | Reviewed-by: Peter Krempa <pkrempa@redhat.com> | 12 | Message-ID: <20250204211407.381505-17-kwolf@redhat.com> |
13 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 15 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 16 | --- |
9 | tests/qemu-iotests/iotests.py | 5 ++++- | 17 | tests/qemu-iotests/iotests.py | 4 + |
10 | 1 file changed, 4 insertions(+), 1 deletion(-) | 18 | tests/qemu-iotests/tests/inactive-node-nbd | 303 ++++++++++++++++++ |
19 | .../qemu-iotests/tests/inactive-node-nbd.out | 239 ++++++++++++++ | ||
20 | 3 files changed, 546 insertions(+) | ||
21 | create mode 100755 tests/qemu-iotests/tests/inactive-node-nbd | ||
22 | create mode 100644 tests/qemu-iotests/tests/inactive-node-nbd.out | ||
11 | 23 | ||
12 | diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py | 24 | diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py |
13 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/tests/qemu-iotests/iotests.py | 26 | --- a/tests/qemu-iotests/iotests.py |
15 | +++ b/tests/qemu-iotests/iotests.py | 27 | +++ b/tests/qemu-iotests/iotests.py |
16 | @@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine): | 28 | @@ -XXX,XX +XXX,XX @@ def add_incoming(self, addr): |
17 | if use_log: | 29 | self._args.append(addr) |
18 | log('Job failed: %s' % (j['error'])) | 30 | return self |
19 | elif status == 'ready': | 31 | |
20 | - self.qmp_log('job-complete', id=job) | 32 | + def add_paused(self): |
21 | + if use_log: | 33 | + self._args.append('-S') |
22 | + self.qmp_log('job-complete', id=job) | 34 | + return self |
23 | + else: | 35 | + |
24 | + self.qmp('job-complete', id=job) | 36 | def hmp(self, command_line: str, use_log: bool = False) -> QMPMessage: |
25 | elif status == 'pending' and not auto_finalize: | 37 | cmd = 'human-monitor-command' |
26 | if pre_finalize: | 38 | kwargs: Dict[str, Any] = {'command-line': command_line} |
27 | pre_finalize() | 39 | diff --git a/tests/qemu-iotests/tests/inactive-node-nbd b/tests/qemu-iotests/tests/inactive-node-nbd |
40 | new file mode 100755 | ||
41 | index XXXXXXX..XXXXXXX | ||
42 | --- /dev/null | ||
43 | +++ b/tests/qemu-iotests/tests/inactive-node-nbd | ||
44 | @@ -XXX,XX +XXX,XX @@ | ||
45 | +#!/usr/bin/env python3 | ||
46 | +# group: rw quick | ||
47 | +# | ||
48 | +# Copyright (C) Red Hat, Inc. | ||
49 | +# | ||
50 | +# This program is free software; you can redistribute it and/or modify | ||
51 | +# it under the terms of the GNU General Public License as published by | ||
52 | +# the Free Software Foundation; either version 2 of the License, or | ||
53 | +# (at your option) any later version. | ||
54 | +# | ||
55 | +# This program is distributed in the hope that it will be useful, | ||
56 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
57 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
58 | +# GNU General Public License for more details. | ||
59 | +# | ||
60 | +# You should have received a copy of the GNU General Public License | ||
61 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
62 | +# | ||
63 | +# Creator/Owner: Kevin Wolf <kwolf@redhat.com> | ||
64 | + | ||
65 | +import iotests | ||
66 | + | ||
67 | +from iotests import QemuIoInteractive | ||
68 | +from iotests import filter_qemu_io, filter_qtest, filter_qmp_testfiles | ||
69 | + | ||
70 | +iotests.script_initialize(supported_fmts=['generic'], | ||
71 | + supported_protocols=['file'], | ||
72 | + supported_platforms=['linux']) | ||
73 | + | ||
74 | +def get_export(node_name='disk-fmt', allow_inactive=None): | ||
75 | + exp = { | ||
76 | + 'id': 'exp0', | ||
77 | + 'type': 'nbd', | ||
78 | + 'node-name': node_name, | ||
79 | + 'writable': True, | ||
80 | + } | ||
81 | + | ||
82 | + if allow_inactive is not None: | ||
83 | + exp['allow-inactive'] = allow_inactive | ||
84 | + | ||
85 | + return exp | ||
86 | + | ||
87 | +def node_is_active(_vm, node_name): | ||
88 | + nodes = _vm.cmd('query-named-block-nodes', flat=True) | ||
89 | + node = next(n for n in nodes if n['node-name'] == node_name) | ||
90 | + return node['active'] | ||
91 | + | ||
92 | +with iotests.FilePath('disk.img') as path, \ | ||
93 | + iotests.FilePath('snap.qcow2') as snap_path, \ | ||
94 | + iotests.FilePath('snap2.qcow2') as snap2_path, \ | ||
95 | + iotests.FilePath('target.img') as target_path, \ | ||
96 | + iotests.FilePath('nbd.sock', base_dir=iotests.sock_dir) as nbd_sock, \ | ||
97 | + iotests.VM() as vm: | ||
98 | + | ||
99 | + img_size = '10M' | ||
100 | + | ||
101 | + iotests.log('Preparing disk...') | ||
102 | + iotests.qemu_img_create('-f', iotests.imgfmt, path, img_size) | ||
103 | + iotests.qemu_img_create('-f', iotests.imgfmt, target_path, img_size) | ||
104 | + | ||
105 | + iotests.qemu_img_create('-f', 'qcow2', '-b', path, '-F', iotests.imgfmt, | ||
106 | + snap_path) | ||
107 | + iotests.qemu_img_create('-f', 'qcow2', '-b', snap_path, '-F', 'qcow2', | ||
108 | + snap2_path) | ||
109 | + | ||
110 | + iotests.log('Launching VM...') | ||
111 | + vm.add_blockdev(f'file,node-name=disk-file,filename={path}') | ||
112 | + vm.add_blockdev(f'{iotests.imgfmt},file=disk-file,node-name=disk-fmt,' | ||
113 | + 'active=off') | ||
114 | + vm.add_blockdev(f'file,node-name=target-file,filename={target_path}') | ||
115 | + vm.add_blockdev(f'{iotests.imgfmt},file=target-file,node-name=target-fmt') | ||
116 | + vm.add_blockdev(f'file,node-name=snap-file,filename={snap_path}') | ||
117 | + vm.add_blockdev(f'file,node-name=snap2-file,filename={snap2_path}') | ||
118 | + | ||
119 | + # Actually running the VM activates all images | ||
120 | + vm.add_paused() | ||
121 | + | ||
122 | + vm.launch() | ||
123 | + vm.qmp_log('nbd-server-start', | ||
124 | + addr={'type': 'unix', 'data':{'path': nbd_sock}}, | ||
125 | + filters=[filter_qmp_testfiles]) | ||
126 | + | ||
127 | + iotests.log('\n=== Creating export of inactive node ===') | ||
128 | + | ||
129 | + iotests.log('\nExports activate nodes without allow-inactive') | ||
130 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
131 | + vm.qmp_log('block-export-add', **get_export()) | ||
132 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
133 | + vm.qmp_log('query-block-exports') | ||
134 | + vm.qmp_log('block-export-del', id='exp0') | ||
135 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
136 | + vm.qmp_log('query-block-exports') | ||
137 | + | ||
138 | + iotests.log('\nExports activate nodes with allow-inactive=false') | ||
139 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
140 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
141 | + vm.qmp_log('block-export-add', **get_export(allow_inactive=False)) | ||
142 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
143 | + vm.qmp_log('query-block-exports') | ||
144 | + vm.qmp_log('block-export-del', id='exp0') | ||
145 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
146 | + vm.qmp_log('query-block-exports') | ||
147 | + | ||
148 | + iotests.log('\nExport leaves nodes inactive with allow-inactive=true') | ||
149 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
150 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
151 | + vm.qmp_log('block-export-add', **get_export(allow_inactive=True)) | ||
152 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
153 | + vm.qmp_log('query-block-exports') | ||
154 | + vm.qmp_log('block-export-del', id='exp0') | ||
155 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
156 | + vm.qmp_log('query-block-exports') | ||
157 | + | ||
158 | + iotests.log('\n=== Inactivating node with existing export ===') | ||
159 | + | ||
160 | + iotests.log('\nInactivating nodes with an export fails without ' | ||
161 | + 'allow-inactive') | ||
162 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=True) | ||
163 | + vm.qmp_log('block-export-add', **get_export(node_name='disk-fmt')) | ||
164 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
165 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
166 | + vm.qmp_log('query-block-exports') | ||
167 | + vm.qmp_log('block-export-del', id='exp0') | ||
168 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
169 | + vm.qmp_log('query-block-exports') | ||
170 | + | ||
171 | + iotests.log('\nInactivating nodes with an export fails with ' | ||
172 | + 'allow-inactive=false') | ||
173 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=True) | ||
174 | + vm.qmp_log('block-export-add', | ||
175 | + **get_export(node_name='disk-fmt', allow_inactive=False)) | ||
176 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
177 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
178 | + vm.qmp_log('query-block-exports') | ||
179 | + vm.qmp_log('block-export-del', id='exp0') | ||
180 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
181 | + vm.qmp_log('query-block-exports') | ||
182 | + | ||
183 | + iotests.log('\nInactivating nodes with an export works with ' | ||
184 | + 'allow-inactive=true') | ||
185 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=True) | ||
186 | + vm.qmp_log('block-export-add', | ||
187 | + **get_export(node_name='disk-fmt', allow_inactive=True)) | ||
188 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
189 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
190 | + vm.qmp_log('query-block-exports') | ||
191 | + vm.qmp_log('block-export-del', id='exp0') | ||
192 | + vm.event_wait('BLOCK_EXPORT_DELETED') | ||
193 | + vm.qmp_log('query-block-exports') | ||
194 | + | ||
195 | + iotests.log('\n=== Inactive nodes with parent ===') | ||
196 | + | ||
197 | + iotests.log('\nInactivating nodes with an active parent fails') | ||
198 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=True) | ||
199 | + vm.qmp_log('blockdev-set-active', node_name='disk-file', active=False) | ||
200 | + iotests.log('disk-file active: %s' % node_is_active(vm, 'disk-file')) | ||
201 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
202 | + | ||
203 | + iotests.log('\nInactivating nodes with an inactive parent works') | ||
204 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=False) | ||
205 | + vm.qmp_log('blockdev-set-active', node_name='disk-file', active=False) | ||
206 | + iotests.log('disk-file active: %s' % node_is_active(vm, 'disk-file')) | ||
207 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
208 | + | ||
209 | + iotests.log('\nCreating active parent node with an inactive child fails') | ||
210 | + vm.qmp_log('blockdev-add', driver='raw', file='disk-fmt', | ||
211 | + node_name='disk-filter') | ||
212 | + vm.qmp_log('blockdev-add', driver='raw', file='disk-fmt', | ||
213 | + node_name='disk-filter', active=True) | ||
214 | + | ||
215 | + iotests.log('\nCreating inactive parent node with an inactive child works') | ||
216 | + vm.qmp_log('blockdev-add', driver='raw', file='disk-fmt', | ||
217 | + node_name='disk-filter', active=False) | ||
218 | + vm.qmp_log('blockdev-del', node_name='disk-filter') | ||
219 | + | ||
220 | + iotests.log('\n=== Resizing an inactive node ===') | ||
221 | + vm.qmp_log('block_resize', node_name='disk-fmt', size=16*1024*1024) | ||
222 | + | ||
223 | + iotests.log('\n=== Taking a snapshot of an inactive node ===') | ||
224 | + | ||
225 | + iotests.log('\nActive overlay over inactive backing file automatically ' | ||
226 | + 'makes both inactive for compatibility') | ||
227 | + vm.qmp_log('blockdev-add', driver='qcow2', node_name='snap-fmt', | ||
228 | + file='snap-file', backing=None) | ||
229 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
230 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
231 | + vm.qmp_log('blockdev-snapshot', node='disk-fmt', overlay='snap-fmt') | ||
232 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
233 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
234 | + vm.qmp_log('blockdev-del', node_name='snap-fmt') | ||
235 | + | ||
236 | + iotests.log('\nInactive overlay over inactive backing file just works') | ||
237 | + vm.qmp_log('blockdev-add', driver='qcow2', node_name='snap-fmt', | ||
238 | + file='snap-file', backing=None, active=False) | ||
239 | + vm.qmp_log('blockdev-snapshot', node='disk-fmt', overlay='snap-fmt') | ||
240 | + | ||
241 | + iotests.log('\n=== Block jobs with inactive nodes ===') | ||
242 | + | ||
243 | + iotests.log('\nStreaming into an inactive node') | ||
244 | + vm.qmp_log('block-stream', device='snap-fmt', | ||
245 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
246 | + | ||
247 | + iotests.log('\nCommitting an inactive root node (active commit)') | ||
248 | + vm.qmp_log('block-commit', job_id='job0', device='snap-fmt', | ||
249 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
250 | + | ||
251 | + iotests.log('\nCommitting an inactive intermediate node to inactive base') | ||
252 | + vm.qmp_log('blockdev-add', driver='qcow2', node_name='snap2-fmt', | ||
253 | + file='snap2-file', backing='snap-fmt', active=False) | ||
254 | + | ||
255 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
256 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
257 | + iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) | ||
258 | + | ||
259 | + vm.qmp_log('block-commit', job_id='job0', device='snap2-fmt', | ||
260 | + top_node='snap-fmt', | ||
261 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
262 | + | ||
263 | + iotests.log('\nCommitting an inactive intermediate node to active base') | ||
264 | + vm.qmp_log('blockdev-set-active', node_name='disk-fmt', active=True) | ||
265 | + vm.qmp_log('block-commit', job_id='job0', device='snap2-fmt', | ||
266 | + top_node='snap-fmt', | ||
267 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
268 | + | ||
269 | + iotests.log('\nMirror from inactive source to active target') | ||
270 | + vm.qmp_log('blockdev-mirror', job_id='job0', device='snap2-fmt', | ||
271 | + target='target-fmt', sync='full', | ||
272 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
273 | + | ||
274 | + iotests.log('\nMirror from active source to inactive target') | ||
275 | + | ||
276 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
277 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
278 | + iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) | ||
279 | + iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt')) | ||
280 | + | ||
281 | + # Activating snap2-fmt recursively activates the whole backing chain | ||
282 | + vm.qmp_log('blockdev-set-active', node_name='snap2-fmt', active=True) | ||
283 | + vm.qmp_log('blockdev-set-active', node_name='target-fmt', active=False) | ||
284 | + | ||
285 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
286 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
287 | + iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) | ||
288 | + iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt')) | ||
289 | + | ||
290 | + vm.qmp_log('blockdev-mirror', job_id='job0', device='snap2-fmt', | ||
291 | + target='target-fmt', sync='full', | ||
292 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
293 | + | ||
294 | + iotests.log('\nBackup from active source to inactive target') | ||
295 | + | ||
296 | + vm.qmp_log('blockdev-backup', job_id='job0', device='snap2-fmt', | ||
297 | + target='target-fmt', sync='full', | ||
298 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
299 | + | ||
300 | + iotests.log('\nBackup from inactive source to active target') | ||
301 | + | ||
302 | + # Inactivating snap2-fmt recursively inactivates the whole backing chain | ||
303 | + vm.qmp_log('blockdev-set-active', node_name='snap2-fmt', active=False) | ||
304 | + vm.qmp_log('blockdev-set-active', node_name='target-fmt', active=True) | ||
305 | + | ||
306 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
307 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
308 | + iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) | ||
309 | + iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt')) | ||
310 | + | ||
311 | + vm.qmp_log('blockdev-backup', job_id='job0', device='snap2-fmt', | ||
312 | + target='target-fmt', sync='full', | ||
313 | + filters=[iotests.filter_qmp_generated_node_ids]) | ||
314 | + | ||
315 | + iotests.log('\n=== Accessing export on inactive node ===') | ||
316 | + | ||
317 | + # Use the target node because it has the right image format and isn't the | ||
318 | + # (read-only) backing file of a qcow2 node | ||
319 | + vm.qmp_log('blockdev-set-active', node_name='target-fmt', active=False) | ||
320 | + vm.qmp_log('block-export-add', | ||
321 | + **get_export(node_name='target-fmt', allow_inactive=True)) | ||
322 | + | ||
323 | + # The read should succeed, everything else should fail gracefully | ||
324 | + qemu_io = QemuIoInteractive('-f', 'raw', | ||
325 | + f'nbd+unix:///target-fmt?socket={nbd_sock}') | ||
326 | + iotests.log(qemu_io.cmd('read 0 64k'), filters=[filter_qemu_io]) | ||
327 | + iotests.log(qemu_io.cmd('write 0 64k'), filters=[filter_qemu_io]) | ||
328 | + iotests.log(qemu_io.cmd('write -z 0 64k'), filters=[filter_qemu_io]) | ||
329 | + iotests.log(qemu_io.cmd('write -zu 0 64k'), filters=[filter_qemu_io]) | ||
330 | + iotests.log(qemu_io.cmd('discard 0 64k'), filters=[filter_qemu_io]) | ||
331 | + iotests.log(qemu_io.cmd('flush'), filters=[filter_qemu_io]) | ||
332 | + iotests.log(qemu_io.cmd('map'), filters=[filter_qemu_io]) | ||
333 | + qemu_io.close() | ||
334 | + | ||
335 | + iotests.log('\n=== Resuming VM activates all images ===') | ||
336 | + vm.qmp_log('cont') | ||
337 | + | ||
338 | + iotests.log('disk-fmt active: %s' % node_is_active(vm, 'disk-fmt')) | ||
339 | + iotests.log('snap-fmt active: %s' % node_is_active(vm, 'snap-fmt')) | ||
340 | + iotests.log('snap2-fmt active: %s' % node_is_active(vm, 'snap2-fmt')) | ||
341 | + iotests.log('target-fmt active: %s' % node_is_active(vm, 'target-fmt')) | ||
342 | + | ||
343 | + iotests.log('\nShutting down...') | ||
344 | + vm.shutdown() | ||
345 | + log = vm.get_log() | ||
346 | + if log: | ||
347 | + iotests.log(log, [filter_qtest, filter_qemu_io]) | ||
348 | diff --git a/tests/qemu-iotests/tests/inactive-node-nbd.out b/tests/qemu-iotests/tests/inactive-node-nbd.out | ||
349 | new file mode 100644 | ||
350 | index XXXXXXX..XXXXXXX | ||
351 | --- /dev/null | ||
352 | +++ b/tests/qemu-iotests/tests/inactive-node-nbd.out | ||
353 | @@ -XXX,XX +XXX,XX @@ | ||
354 | +Preparing disk... | ||
355 | +Launching VM... | ||
356 | +{"execute": "nbd-server-start", "arguments": {"addr": {"data": {"path": "SOCK_DIR/PID-nbd.sock"}, "type": "unix"}}} | ||
357 | +{"return": {}} | ||
358 | + | ||
359 | +=== Creating export of inactive node === | ||
360 | + | ||
361 | +Exports activate nodes without allow-inactive | ||
362 | +disk-fmt active: False | ||
363 | +{"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
364 | +{"return": {}} | ||
365 | +disk-fmt active: True | ||
366 | +{"execute": "query-block-exports", "arguments": {}} | ||
367 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
368 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
369 | +{"return": {}} | ||
370 | +{"execute": "query-block-exports", "arguments": {}} | ||
371 | +{"return": []} | ||
372 | + | ||
373 | +Exports activate nodes with allow-inactive=false | ||
374 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
375 | +{"return": {}} | ||
376 | +disk-fmt active: False | ||
377 | +{"execute": "block-export-add", "arguments": {"allow-inactive": false, "id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
378 | +{"return": {}} | ||
379 | +disk-fmt active: True | ||
380 | +{"execute": "query-block-exports", "arguments": {}} | ||
381 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
382 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
383 | +{"return": {}} | ||
384 | +{"execute": "query-block-exports", "arguments": {}} | ||
385 | +{"return": []} | ||
386 | + | ||
387 | +Export leaves nodes inactive with allow-inactive=true | ||
388 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
389 | +{"return": {}} | ||
390 | +disk-fmt active: False | ||
391 | +{"execute": "block-export-add", "arguments": {"allow-inactive": true, "id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
392 | +{"return": {}} | ||
393 | +disk-fmt active: False | ||
394 | +{"execute": "query-block-exports", "arguments": {}} | ||
395 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
396 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
397 | +{"return": {}} | ||
398 | +{"execute": "query-block-exports", "arguments": {}} | ||
399 | +{"return": []} | ||
400 | + | ||
401 | +=== Inactivating node with existing export === | ||
402 | + | ||
403 | +Inactivating nodes with an export fails without allow-inactive | ||
404 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "disk-fmt"}} | ||
405 | +{"return": {}} | ||
406 | +{"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
407 | +{"return": {}} | ||
408 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
409 | +{"error": {"class": "GenericError", "desc": "Failed to inactivate node: Operation not permitted"}} | ||
410 | +disk-fmt active: True | ||
411 | +{"execute": "query-block-exports", "arguments": {}} | ||
412 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
413 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
414 | +{"return": {}} | ||
415 | +{"execute": "query-block-exports", "arguments": {}} | ||
416 | +{"return": []} | ||
417 | + | ||
418 | +Inactivating nodes with an export fails with allow-inactive=false | ||
419 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "disk-fmt"}} | ||
420 | +{"return": {}} | ||
421 | +{"execute": "block-export-add", "arguments": {"allow-inactive": false, "id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
422 | +{"return": {}} | ||
423 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
424 | +{"error": {"class": "GenericError", "desc": "Failed to inactivate node: Operation not permitted"}} | ||
425 | +disk-fmt active: True | ||
426 | +{"execute": "query-block-exports", "arguments": {}} | ||
427 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
428 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
429 | +{"return": {}} | ||
430 | +{"execute": "query-block-exports", "arguments": {}} | ||
431 | +{"return": []} | ||
432 | + | ||
433 | +Inactivating nodes with an export works with allow-inactive=true | ||
434 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "disk-fmt"}} | ||
435 | +{"return": {}} | ||
436 | +{"execute": "block-export-add", "arguments": {"allow-inactive": true, "id": "exp0", "node-name": "disk-fmt", "type": "nbd", "writable": true}} | ||
437 | +{"return": {}} | ||
438 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
439 | +{"return": {}} | ||
440 | +disk-fmt active: False | ||
441 | +{"execute": "query-block-exports", "arguments": {}} | ||
442 | +{"return": [{"id": "exp0", "node-name": "disk-fmt", "shutting-down": false, "type": "nbd"}]} | ||
443 | +{"execute": "block-export-del", "arguments": {"id": "exp0"}} | ||
444 | +{"return": {}} | ||
445 | +{"execute": "query-block-exports", "arguments": {}} | ||
446 | +{"return": []} | ||
447 | + | ||
448 | +=== Inactive nodes with parent === | ||
449 | + | ||
450 | +Inactivating nodes with an active parent fails | ||
451 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "disk-fmt"}} | ||
452 | +{"return": {}} | ||
453 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-file"}} | ||
454 | +{"error": {"class": "GenericError", "desc": "Node has active parent node"}} | ||
455 | +disk-file active: True | ||
456 | +disk-fmt active: True | ||
457 | + | ||
458 | +Inactivating nodes with an inactive parent works | ||
459 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-fmt"}} | ||
460 | +{"return": {}} | ||
461 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "disk-file"}} | ||
462 | +{"return": {}} | ||
463 | +disk-file active: False | ||
464 | +disk-fmt active: False | ||
465 | + | ||
466 | +Creating active parent node with an inactive child fails | ||
467 | +{"execute": "blockdev-add", "arguments": {"driver": "raw", "file": "disk-fmt", "node-name": "disk-filter"}} | ||
468 | +{"error": {"class": "GenericError", "desc": "Inactive 'disk-fmt' can't be a file child of active 'disk-filter'"}} | ||
469 | +{"execute": "blockdev-add", "arguments": {"active": true, "driver": "raw", "file": "disk-fmt", "node-name": "disk-filter"}} | ||
470 | +{"error": {"class": "GenericError", "desc": "Inactive 'disk-fmt' can't be a file child of active 'disk-filter'"}} | ||
471 | + | ||
472 | +Creating inactive parent node with an inactive child works | ||
473 | +{"execute": "blockdev-add", "arguments": {"active": false, "driver": "raw", "file": "disk-fmt", "node-name": "disk-filter"}} | ||
474 | +{"return": {}} | ||
475 | +{"execute": "blockdev-del", "arguments": {"node-name": "disk-filter"}} | ||
476 | +{"return": {}} | ||
477 | + | ||
478 | +=== Resizing an inactive node === | ||
479 | +{"execute": "block_resize", "arguments": {"node-name": "disk-fmt", "size": 16777216}} | ||
480 | +{"error": {"class": "GenericError", "desc": "Permission 'resize' unavailable on inactive node"}} | ||
481 | + | ||
482 | +=== Taking a snapshot of an inactive node === | ||
483 | + | ||
484 | +Active overlay over inactive backing file automatically makes both inactive for compatibility | ||
485 | +{"execute": "blockdev-add", "arguments": {"backing": null, "driver": "qcow2", "file": "snap-file", "node-name": "snap-fmt"}} | ||
486 | +{"return": {}} | ||
487 | +disk-fmt active: False | ||
488 | +snap-fmt active: True | ||
489 | +{"execute": "blockdev-snapshot", "arguments": {"node": "disk-fmt", "overlay": "snap-fmt"}} | ||
490 | +{"return": {}} | ||
491 | +disk-fmt active: False | ||
492 | +snap-fmt active: False | ||
493 | +{"execute": "blockdev-del", "arguments": {"node-name": "snap-fmt"}} | ||
494 | +{"return": {}} | ||
495 | + | ||
496 | +Inactive overlay over inactive backing file just works | ||
497 | +{"execute": "blockdev-add", "arguments": {"active": false, "backing": null, "driver": "qcow2", "file": "snap-file", "node-name": "snap-fmt"}} | ||
498 | +{"return": {}} | ||
499 | +{"execute": "blockdev-snapshot", "arguments": {"node": "disk-fmt", "overlay": "snap-fmt"}} | ||
500 | +{"return": {}} | ||
501 | + | ||
502 | +=== Block jobs with inactive nodes === | ||
503 | + | ||
504 | +Streaming into an inactive node | ||
505 | +{"execute": "block-stream", "arguments": {"device": "snap-fmt"}} | ||
506 | +{"error": {"class": "GenericError", "desc": "Could not create node: Inactive 'snap-fmt' can't be a file child of active 'NODE_NAME'"}} | ||
507 | + | ||
508 | +Committing an inactive root node (active commit) | ||
509 | +{"execute": "block-commit", "arguments": {"device": "snap-fmt", "job-id": "job0"}} | ||
510 | +{"error": {"class": "GenericError", "desc": "Inactive 'snap-fmt' can't be a backing child of active 'NODE_NAME'"}} | ||
511 | + | ||
512 | +Committing an inactive intermediate node to inactive base | ||
513 | +{"execute": "blockdev-add", "arguments": {"active": false, "backing": "snap-fmt", "driver": "qcow2", "file": "snap2-file", "node-name": "snap2-fmt"}} | ||
514 | +{"return": {}} | ||
515 | +disk-fmt active: False | ||
516 | +snap-fmt active: False | ||
517 | +snap2-fmt active: False | ||
518 | +{"execute": "block-commit", "arguments": {"device": "snap2-fmt", "job-id": "job0", "top-node": "snap-fmt"}} | ||
519 | +{"error": {"class": "GenericError", "desc": "Inactive 'snap-fmt' can't be a backing child of active 'NODE_NAME'"}} | ||
520 | + | ||
521 | +Committing an inactive intermediate node to active base | ||
522 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "disk-fmt"}} | ||
523 | +{"return": {}} | ||
524 | +{"execute": "block-commit", "arguments": {"device": "snap2-fmt", "job-id": "job0", "top-node": "snap-fmt"}} | ||
525 | +{"error": {"class": "GenericError", "desc": "Inactive 'snap-fmt' can't be a backing child of active 'NODE_NAME'"}} | ||
526 | + | ||
527 | +Mirror from inactive source to active target | ||
528 | +{"execute": "blockdev-mirror", "arguments": {"device": "snap2-fmt", "job-id": "job0", "sync": "full", "target": "target-fmt"}} | ||
529 | +{"error": {"class": "GenericError", "desc": "Inactive 'snap2-fmt' can't be a backing child of active 'NODE_NAME'"}} | ||
530 | + | ||
531 | +Mirror from active source to inactive target | ||
532 | +disk-fmt active: True | ||
533 | +snap-fmt active: False | ||
534 | +snap2-fmt active: False | ||
535 | +target-fmt active: True | ||
536 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "snap2-fmt"}} | ||
537 | +{"return": {}} | ||
538 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "target-fmt"}} | ||
539 | +{"return": {}} | ||
540 | +disk-fmt active: True | ||
541 | +snap-fmt active: True | ||
542 | +snap2-fmt active: True | ||
543 | +target-fmt active: False | ||
544 | +{"execute": "blockdev-mirror", "arguments": {"device": "snap2-fmt", "job-id": "job0", "sync": "full", "target": "target-fmt"}} | ||
545 | +{"error": {"class": "GenericError", "desc": "Permission 'write' unavailable on inactive node"}} | ||
546 | + | ||
547 | +Backup from active source to inactive target | ||
548 | +{"execute": "blockdev-backup", "arguments": {"device": "snap2-fmt", "job-id": "job0", "sync": "full", "target": "target-fmt"}} | ||
549 | +{"error": {"class": "GenericError", "desc": "Could not create node: Inactive 'target-fmt' can't be a target child of active 'NODE_NAME'"}} | ||
550 | + | ||
551 | +Backup from inactive source to active target | ||
552 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "snap2-fmt"}} | ||
553 | +{"return": {}} | ||
554 | +{"execute": "blockdev-set-active", "arguments": {"active": true, "node-name": "target-fmt"}} | ||
555 | +{"return": {}} | ||
556 | +disk-fmt active: False | ||
557 | +snap-fmt active: False | ||
558 | +snap2-fmt active: False | ||
559 | +target-fmt active: True | ||
560 | +{"execute": "blockdev-backup", "arguments": {"device": "snap2-fmt", "job-id": "job0", "sync": "full", "target": "target-fmt"}} | ||
561 | +{"error": {"class": "GenericError", "desc": "Could not create node: Inactive 'snap2-fmt' can't be a file child of active 'NODE_NAME'"}} | ||
562 | + | ||
563 | +=== Accessing export on inactive node === | ||
564 | +{"execute": "blockdev-set-active", "arguments": {"active": false, "node-name": "target-fmt"}} | ||
565 | +{"return": {}} | ||
566 | +{"execute": "block-export-add", "arguments": {"allow-inactive": true, "id": "exp0", "node-name": "target-fmt", "type": "nbd", "writable": true}} | ||
567 | +{"return": {}} | ||
568 | +read 65536/65536 bytes at offset 0 | ||
569 | +64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
570 | + | ||
571 | +write failed: Operation not permitted | ||
572 | + | ||
573 | +write failed: Operation not permitted | ||
574 | + | ||
575 | +write failed: Operation not permitted | ||
576 | + | ||
577 | +discard failed: Operation not permitted | ||
578 | + | ||
579 | + | ||
580 | +qemu-io: Failed to get allocation status: Operation not permitted | ||
581 | + | ||
582 | + | ||
583 | +=== Resuming VM activates all images === | ||
584 | +{"execute": "cont", "arguments": {}} | ||
585 | +{"return": {}} | ||
586 | +disk-fmt active: True | ||
587 | +snap-fmt active: True | ||
588 | +snap2-fmt active: True | ||
589 | +target-fmt active: True | ||
590 | + | ||
591 | +Shutting down... | ||
592 | + | ||
28 | -- | 593 | -- |
29 | 2.20.1 | 594 | 2.48.1 |
30 | |||
31 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Stefan Hajnoczi <stefanha@redhat.com> | ||
1 | 2 | ||
3 | BLOCK_OP_TYPE_DATAPLANE prevents BlockDriverState from being used by | ||
4 | virtio-blk/virtio-scsi with IOThread. Commit b112a65c52aa ("block: | ||
5 | declare blockjobs and dataplane friends!") eliminated the main reason | ||
6 | for this blocker in 2014. | ||
7 | |||
8 | Nowadays the block layer supports I/O from multiple AioContexts, so | ||
9 | there is even less reason to block IOThread users. Any legitimate | ||
10 | reasons related to interference would probably also apply to | ||
11 | non-IOThread users. | ||
12 | |||
13 | The only remaining users are bdrv_op_unblock(BLOCK_OP_TYPE_DATAPLANE) | ||
14 | calls after bdrv_op_block_all(). If we remove BLOCK_OP_TYPE_DATAPLANE | ||
15 | their behavior doesn't change. | ||
16 | |||
17 | Existing bdrv_op_block_all() callers that don't explicitly unblock | ||
18 | BLOCK_OP_TYPE_DATAPLANE seem to do so simply because no one bothered to | ||
19 | rather than because it is necessary to keep BLOCK_OP_TYPE_DATAPLANE | ||
20 | blocked. | ||
21 | |||
22 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
23 | Message-ID: <20250203182529.269066-1-stefanha@redhat.com> | ||
24 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
25 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
26 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
27 | --- | ||
28 | include/block/block-common.h | 1 - | ||
29 | block/replication.c | 1 - | ||
30 | blockjob.c | 2 -- | ||
31 | hw/block/virtio-blk.c | 9 --------- | ||
32 | hw/scsi/virtio-scsi.c | 3 --- | ||
33 | 5 files changed, 16 deletions(-) | ||
34 | |||
35 | diff --git a/include/block/block-common.h b/include/block/block-common.h | ||
36 | index XXXXXXX..XXXXXXX 100644 | ||
37 | --- a/include/block/block-common.h | ||
38 | +++ b/include/block/block-common.h | ||
39 | @@ -XXX,XX +XXX,XX @@ typedef enum BlockOpType { | ||
40 | BLOCK_OP_TYPE_CHANGE, | ||
41 | BLOCK_OP_TYPE_COMMIT_SOURCE, | ||
42 | BLOCK_OP_TYPE_COMMIT_TARGET, | ||
43 | - BLOCK_OP_TYPE_DATAPLANE, | ||
44 | BLOCK_OP_TYPE_DRIVE_DEL, | ||
45 | BLOCK_OP_TYPE_EJECT, | ||
46 | BLOCK_OP_TYPE_EXTERNAL_SNAPSHOT, | ||
47 | diff --git a/block/replication.c b/block/replication.c | ||
48 | index XXXXXXX..XXXXXXX 100644 | ||
49 | --- a/block/replication.c | ||
50 | +++ b/block/replication.c | ||
51 | @@ -XXX,XX +XXX,XX @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, | ||
52 | return; | ||
53 | } | ||
54 | bdrv_op_block_all(top_bs, s->blocker); | ||
55 | - bdrv_op_unblock(top_bs, BLOCK_OP_TYPE_DATAPLANE, s->blocker); | ||
56 | |||
57 | bdrv_graph_wrunlock(); | ||
58 | |||
59 | diff --git a/blockjob.c b/blockjob.c | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/blockjob.c | ||
62 | +++ b/blockjob.c | ||
63 | @@ -XXX,XX +XXX,XX @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, | ||
64 | goto fail; | ||
65 | } | ||
66 | |||
67 | - bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); | ||
68 | - | ||
69 | if (!block_job_set_speed(job, speed, errp)) { | ||
70 | goto fail; | ||
71 | } | ||
72 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
73 | index XXXXXXX..XXXXXXX 100644 | ||
74 | --- a/hw/block/virtio-blk.c | ||
75 | +++ b/hw/block/virtio-blk.c | ||
76 | @@ -XXX,XX +XXX,XX @@ static bool virtio_blk_vq_aio_context_init(VirtIOBlock *s, Error **errp) | ||
77 | error_setg(errp, "ioeventfd is required for iothread"); | ||
78 | return false; | ||
79 | } | ||
80 | - | ||
81 | - /* | ||
82 | - * If ioeventfd is (re-)enabled while the guest is running there could | ||
83 | - * be block jobs that can conflict. | ||
84 | - */ | ||
85 | - if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) { | ||
86 | - error_prepend(errp, "cannot start virtio-blk ioeventfd: "); | ||
87 | - return false; | ||
88 | - } | ||
89 | } | ||
90 | |||
91 | s->vq_aio_context = g_new(AioContext *, conf->num_queues); | ||
92 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
93 | index XXXXXXX..XXXXXXX 100644 | ||
94 | --- a/hw/scsi/virtio-scsi.c | ||
95 | +++ b/hw/scsi/virtio-scsi.c | ||
96 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_hotplug(HotplugHandler *hotplug_dev, DeviceState *dev, | ||
97 | int ret; | ||
98 | |||
99 | if (s->ctx && !s->dataplane_fenced) { | ||
100 | - if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)) { | ||
101 | - return; | ||
102 | - } | ||
103 | ret = blk_set_aio_context(sd->conf.blk, s->ctx, errp); | ||
104 | if (ret < 0) { | ||
105 | return; | ||
106 | -- | ||
107 | 2.48.1 | diff view generated by jsdifflib |