1 | The following changes since commit 19eb2d4e736dc895f31fbd6b520e514f10cc08e0: | 1 | The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/thibault/tags/samuel-thibault' into staging (2019-05-07 10:43:32 +0100) | 3 | Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | https://git.xanclic.moe/XanClic/qemu.git tags/pull-block-2019-05-07 | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to 1278dce7927301bf3d004a40061dbd2c1e0846a8: | 9 | for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21: |
10 | 10 | ||
11 | iotests: Fix iotests 110 and 126 (2019-05-07 17:14:21 +0200) | 11 | coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block patches: | 14 | Pull request |
15 | - Fixes to qcow2's implementation of qemu-img check | 15 | |
16 | - Our SSH driver now supports bdrv_refresh_filename() | 16 | (Resent due to an email preparation mistake.) |
17 | - Miscellaneous fixes | ||
18 | 17 | ||
19 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
20 | Alberto Garcia (2): | ||
21 | block: Assert that drv->bdrv_child_perm is set in bdrv_child_perm() | ||
22 | commit: Use bdrv_append() in commit_start() | ||
23 | 19 | ||
24 | Andrey Shinkevich (1): | 20 | Paolo Bonzini (6): |
25 | qcow2: discard bitmap when removed | 21 | coroutine-sleep: use a stack-allocated timer |
22 | coroutine-sleep: disallow NULL QemuCoSleepState** argument | ||
23 | coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing | ||
24 | coroutine-sleep: move timer out of QemuCoSleepState | ||
25 | coroutine-sleep: replace QemuCoSleepState pointer with struct in the | ||
26 | API | ||
27 | coroutine-sleep: introduce qemu_co_sleep | ||
26 | 28 | ||
27 | Max Reitz (3): | 29 | Philippe Mathieu-Daudé (1): |
28 | block/ssh: Implement .bdrv_refresh_filename() | 30 | bitops.h: Improve find_xxx_bit() documentation |
29 | block/ssh: Implement .bdrv_dirname() | ||
30 | iotests: Fix iotests 110 and 126 | ||
31 | 31 | ||
32 | Vladimir Sementsov-Ogievskiy (5): | 32 | Zenghui Yu (1): |
33 | qcow2-refcount: fix check_oflag_copied | 33 | multi-process: Initialize variables declared with g_auto* |
34 | qcow2-refcount: avoid eating RAM | ||
35 | qcow2-refcount: check_refcounts_l2: reduce ignored overlaps | ||
36 | qcow2-refcount: check_refcounts_l2: don't count fixed cluster as | ||
37 | allocated | ||
38 | qcow2-refcount: don't mask corruptions under internal errors | ||
39 | 34 | ||
40 | block.c | 9 ++-- | 35 | include/qemu/bitops.h | 15 ++++++-- |
41 | block/commit.c | 11 +---- | 36 | include/qemu/coroutine.h | 27 ++++++++----- |
42 | block/qcow2-bitmap.c | 2 +- | 37 | block/block-copy.c | 10 ++--- |
43 | block/qcow2-refcount.c | 80 ++++++++++++++++++++++------------- | 38 | block/nbd.c | 14 +++---- |
44 | block/ssh.c | 73 +++++++++++++++++++++++++++++--- | 39 | hw/remote/memory.c | 5 +-- |
45 | tests/qemu-iotests/110 | 10 +++-- | 40 | hw/remote/proxy.c | 3 +- |
46 | tests/qemu-iotests/126 | 10 +++-- | 41 | util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------ |
47 | tests/qemu-iotests/138 | 12 +++--- | 42 | 7 files changed, 79 insertions(+), 70 deletions(-) |
48 | tests/qemu-iotests/138.out | 5 ++- | ||
49 | tests/qemu-iotests/207 | 10 ++--- | ||
50 | tests/qemu-iotests/207.out | 10 ++--- | ||
51 | tests/qemu-iotests/common.rc | 2 +- | ||
52 | tests/qemu-iotests/iotests.py | 2 +- | ||
53 | 13 files changed, 159 insertions(+), 77 deletions(-) | ||
54 | 43 | ||
55 | -- | 44 | -- |
56 | 2.20.1 | 45 | 2.31.1 |
57 | 46 | ||
58 | diff view generated by jsdifflib |
1 | A recent patch results in qemu-img reporting the backing file format of | 1 | From: Zenghui Yu <yuzenghui@huawei.com> |
---|---|---|---|
2 | vmdk images as vmdk. This broke iotests 110 and 126. | ||
3 | 2 | ||
4 | Fixes: 7502be838e2fb62cc00f9e55f632e0b88ead5e6e | 3 | Quote docs/devel/style.rst (section "Automatic memory deallocation"): |
5 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 4 | |
6 | Message-id: 20190415154129.31021-1-mreitz@redhat.com | 5 | * Variables declared with g_auto* MUST always be initialized, |
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 6 | otherwise the cleanup function will use uninitialized stack memory |
7 | |||
8 | Initialize @name properly to get rid of the compilation error (using | ||
9 | gcc-7.3.0 on CentOS): | ||
10 | |||
11 | ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize': | ||
12 | /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized] | ||
13 | g_free (*pp); | ||
14 | ^~~~~~~~~~~~ | ||
15 | ../hw/remote/proxy.c:350:30: note: 'name' was declared here | ||
16 | g_autofree char *name; | ||
17 | ^~~~ | ||
18 | |||
19 | Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> | ||
20 | Reviewed-by: Jagannathan Raman <jag.raman@oracle.com> | ||
21 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
22 | Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com> | ||
23 | Message-id: 20210312112143.1369-1-yuzenghui@huawei.com | ||
24 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
8 | --- | 25 | --- |
9 | tests/qemu-iotests/110 | 10 +++++++--- | 26 | hw/remote/memory.c | 5 ++--- |
10 | tests/qemu-iotests/126 | 10 +++++++--- | 27 | hw/remote/proxy.c | 3 +-- |
11 | 2 files changed, 14 insertions(+), 6 deletions(-) | 28 | 2 files changed, 3 insertions(+), 5 deletions(-) |
12 | 29 | ||
13 | diff --git a/tests/qemu-iotests/110 b/tests/qemu-iotests/110 | 30 | diff --git a/hw/remote/memory.c b/hw/remote/memory.c |
14 | index XXXXXXX..XXXXXXX 100755 | 31 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/tests/qemu-iotests/110 | 32 | --- a/hw/remote/memory.c |
16 | +++ b/tests/qemu-iotests/110 | 33 | +++ b/hw/remote/memory.c |
17 | @@ -XXX,XX +XXX,XX @@ TEST_IMG="$TEST_IMG.base" _make_test_img 64M | 34 | @@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp) |
18 | _make_test_img -b "$TEST_IMG_REL.base" 64M | 35 | |
19 | # qemu should be able to reconstruct the filename, so relative backing names | 36 | remote_sysmem_reset(); |
20 | # should work | 37 | |
21 | +# (We have to filter the backing file format because vmdk always | 38 | - for (region = 0; region < msg->num_fds; region++) { |
22 | +# reports it (as vmdk), whereas other image formats would do so only | 39 | - g_autofree char *name; |
23 | +# with the backing_fmt creation option, which neither vmdk nor qcow | 40 | + for (region = 0; region < msg->num_fds; region++, suffix++) { |
24 | +# support) | 41 | + g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix); |
25 | TEST_IMG="json:{'driver':'$IMGFMT','file':{'driver':'file','filename':'$TEST_IMG'}}" \ | 42 | subregion = g_new(MemoryRegion, 1); |
26 | - _img_info | _filter_img_info | 43 | - name = g_strdup_printf("remote-mem-%u", suffix++); |
27 | + _img_info | _filter_img_info | grep -v 'backing file format' | 44 | memory_region_init_ram_from_fd(subregion, NULL, |
28 | 45 | name, sysmem_info->sizes[region], | |
29 | echo | 46 | true, msg->fds[region], |
30 | echo '=== Non-reconstructable filename ===' | 47 | diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c |
31 | @@ -XXX,XX +XXX,XX @@ TEST_IMG="json:{ | 48 | index XXXXXXX..XXXXXXX 100644 |
49 | --- a/hw/remote/proxy.c | ||
50 | +++ b/hw/remote/proxy.c | ||
51 | @@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp) | ||
52 | PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY; | ||
53 | |||
54 | if (size) { | ||
55 | - g_autofree char *name; | ||
56 | + g_autofree char *name = g_strdup_printf("bar-region-%d", i); | ||
57 | pdev->region[i].dev = pdev; | ||
58 | pdev->region[i].present = true; | ||
59 | if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) { | ||
60 | pdev->region[i].memory = true; | ||
32 | } | 61 | } |
33 | ] | 62 | - name = g_strdup_printf("bar-region-%d", i); |
34 | } | 63 | memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev), |
35 | -}" _img_info | _filter_img_info | 64 | &proxy_mr_ops, &pdev->region[i], |
36 | +}" _img_info | _filter_img_info | grep -v 'backing file format' | 65 | name, size); |
37 | |||
38 | echo | ||
39 | echo '=== Backing name is always relative to the backed image ===' | ||
40 | @@ -XXX,XX +XXX,XX @@ TEST_IMG="json:{ | ||
41 | } | ||
42 | ] | ||
43 | } | ||
44 | -}" _img_info | _filter_img_info | ||
45 | +}" _img_info | _filter_img_info | grep -v 'backing file format' | ||
46 | |||
47 | |||
48 | # success, all done | ||
49 | diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126 | ||
50 | index XXXXXXX..XXXXXXX 100755 | ||
51 | --- a/tests/qemu-iotests/126 | ||
52 | +++ b/tests/qemu-iotests/126 | ||
53 | @@ -XXX,XX +XXX,XX @@ TOP_IMG="$TEST_DIR/image:top.$IMGFMT" | ||
54 | TEST_IMG=$BASE_IMG _make_test_img 64M | ||
55 | TEST_IMG=$TOP_IMG _make_test_img -b ./image:base.$IMGFMT | ||
56 | |||
57 | -# The default cluster size depends on the image format | ||
58 | -TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size' | ||
59 | +# (1) The default cluster size depends on the image format | ||
60 | +# (2) vmdk only supports vmdk backing files, so it always reports the | ||
61 | +# format of its backing file as such (but neither it nor qcow | ||
62 | +# support the backing_fmt creation option, so we cannot use that to | ||
63 | +# harmonize the output across all image formats this test supports) | ||
64 | +TEST_IMG=$TOP_IMG _img_info | grep -ve 'cluster_size' -e 'backing file format' | ||
65 | |||
66 | _rm_test_img "$BASE_IMG" | ||
67 | _rm_test_img "$TOP_IMG" | ||
68 | @@ -XXX,XX +XXX,XX @@ TOP_IMG="file:image:top.$IMGFMT" | ||
69 | TEST_IMG=$BASE_IMG _make_test_img 64M | ||
70 | TEST_IMG=$TOP_IMG _make_test_img -b "$BASE_IMG" | ||
71 | |||
72 | -TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size' | ||
73 | +TEST_IMG=$TOP_IMG _img_info | grep -ve 'cluster_size' -e 'backing file format' | ||
74 | |||
75 | _rm_test_img "$BASE_IMG" | ||
76 | _rm_test_img "image:top.$IMGFMT" | ||
77 | -- | 66 | -- |
78 | 2.20.1 | 67 | 2.31.1 |
79 | 68 | ||
80 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | This function combines bdrv_set_backing_hd() and bdrv_replace_node() | 3 | Document the following functions return the bitmap size |
4 | so we can use it to simplify the code a bit in commit_start(). | 4 | if no matching bit is found: |
5 | 5 | ||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | 6 | - find_first_bit |
7 | Message-id: 20190403143748.9790-1-berto@igalia.com | 7 | - find_next_bit |
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 8 | - find_last_bit |
9 | - find_first_zero_bit | ||
10 | - find_next_zero_bit | ||
11 | |||
12 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
13 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
15 | Message-id: 20210510200758.2623154-2-philmd@redhat.com | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | --- | 17 | --- |
10 | block/commit.c | 11 +---------- | 18 | include/qemu/bitops.h | 15 ++++++++++++--- |
11 | 1 file changed, 1 insertion(+), 10 deletions(-) | 19 | 1 file changed, 12 insertions(+), 3 deletions(-) |
12 | 20 | ||
13 | diff --git a/block/commit.c b/block/commit.c | 21 | diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h |
14 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/commit.c | 23 | --- a/include/qemu/bitops.h |
16 | +++ b/block/commit.c | 24 | +++ b/include/qemu/bitops.h |
17 | @@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs, | 25 | @@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr) |
18 | commit_top_bs->total_sectors = top->total_sectors; | 26 | * @addr: The address to start the search at |
19 | bdrv_set_aio_context(commit_top_bs, bdrv_get_aio_context(top)); | 27 | * @size: The maximum size to search |
20 | 28 | * | |
21 | - bdrv_set_backing_hd(commit_top_bs, top, &local_err); | 29 | - * Returns the bit number of the first set bit, or size. |
22 | + bdrv_append(commit_top_bs, top, &local_err); | 30 | + * Returns the bit number of the last set bit, |
23 | if (local_err) { | 31 | + * or @size if there is no set bit in the bitmap. |
24 | - bdrv_unref(commit_top_bs); | 32 | */ |
25 | - commit_top_bs = NULL; | 33 | unsigned long find_last_bit(const unsigned long *addr, |
26 | - error_propagate(errp, local_err); | 34 | unsigned long size); |
27 | - goto fail; | 35 | @@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr, |
28 | - } | 36 | * @addr: The address to base the search on |
29 | - bdrv_replace_node(top, commit_top_bs, &local_err); | 37 | * @offset: The bitnumber to start searching at |
30 | - if (local_err) { | 38 | * @size: The bitmap size in bits |
31 | - bdrv_unref(commit_top_bs); | 39 | + * |
32 | commit_top_bs = NULL; | 40 | + * Returns the bit number of the next set bit, |
33 | error_propagate(errp, local_err); | 41 | + * or @size if there are no further set bits in the bitmap. |
34 | goto fail; | 42 | */ |
35 | } | 43 | unsigned long find_next_bit(const unsigned long *addr, |
36 | 44 | unsigned long size, | |
37 | s->commit_top_bs = commit_top_bs; | 45 | @@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr, |
38 | - bdrv_unref(commit_top_bs); | 46 | * @addr: The address to base the search on |
39 | 47 | * @offset: The bitnumber to start searching at | |
40 | /* Block all nodes between top and base, because they will | 48 | * @size: The bitmap size in bits |
41 | * disappear from the chain after this operation. */ | 49 | + * |
50 | + * Returns the bit number of the next cleared bit, | ||
51 | + * or @size if there are no further clear bits in the bitmap. | ||
52 | */ | ||
53 | |||
54 | unsigned long find_next_zero_bit(const unsigned long *addr, | ||
55 | @@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr, | ||
56 | * @addr: The address to start the search at | ||
57 | * @size: The maximum size to search | ||
58 | * | ||
59 | - * Returns the bit number of the first set bit. | ||
60 | + * Returns the bit number of the first set bit, | ||
61 | + * or @size if there is no set bit in the bitmap. | ||
62 | */ | ||
63 | static inline unsigned long find_first_bit(const unsigned long *addr, | ||
64 | unsigned long size) | ||
65 | @@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr, | ||
66 | * @addr: The address to start the search at | ||
67 | * @size: The maximum size to search | ||
68 | * | ||
69 | - * Returns the bit number of the first cleared bit. | ||
70 | + * Returns the bit number of the first cleared bit, | ||
71 | + * or @size if there is no clear bit in the bitmap. | ||
72 | */ | ||
73 | static inline unsigned long find_first_zero_bit(const unsigned long *addr, | ||
74 | unsigned long size) | ||
42 | -- | 75 | -- |
43 | 2.20.1 | 76 | 2.31.1 |
44 | 77 | ||
45 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | There is no need to check for this because all block drivers that have | 3 | The lifetime of the timer is well-known (it cannot outlive |
4 | children implement bdrv_child_perm and all callers already ensure that | 4 | qemu_co_sleep_ns_wakeable, because it's deleted by the time the |
5 | bs->drv is set. | 5 | coroutine resumes), so it is not necessary to place it on the heap. |
6 | 6 | ||
7 | Furthermore, if this check would fail then the callers would end up | 7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
8 | with uninitialized values for nperm and nshared. | 8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
9 | 9 | Message-id: 20210517100548.28806-2-pbonzini@redhat.com | |
10 | This patch replaces the check with an assertion. | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
11 | |||
12 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
13 | Message-id: 20190404112953.4058-1-berto@igalia.com | ||
14 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
15 | --- | 11 | --- |
16 | block.c | 9 ++++----- | 12 | util/qemu-coroutine-sleep.c | 9 ++++----- |
17 | 1 file changed, 4 insertions(+), 5 deletions(-) | 13 | 1 file changed, 4 insertions(+), 5 deletions(-) |
18 | 14 | ||
19 | diff --git a/block.c b/block.c | 15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
20 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/block.c | 17 | --- a/util/qemu-coroutine-sleep.c |
22 | +++ b/block.c | 18 | +++ b/util/qemu-coroutine-sleep.c |
23 | @@ -XXX,XX +XXX,XX @@ static void bdrv_child_perm(BlockDriverState *bs, BlockDriverState *child_bs, | 19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; |
24 | uint64_t parent_perm, uint64_t parent_shared, | 20 | |
25 | uint64_t *nperm, uint64_t *nshared) | 21 | struct QemuCoSleepState { |
26 | { | 22 | Coroutine *co; |
27 | - if (bs->drv && bs->drv->bdrv_child_perm) { | 23 | - QEMUTimer *ts; |
28 | - bs->drv->bdrv_child_perm(bs, c, role, reopen_queue, | 24 | + QEMUTimer ts; |
29 | - parent_perm, parent_shared, | 25 | QemuCoSleepState **user_state_pointer; |
30 | - nperm, nshared); | 26 | }; |
31 | - } | 27 | |
32 | + assert(bs->drv && bs->drv->bdrv_child_perm); | 28 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) |
33 | + bs->drv->bdrv_child_perm(bs, c, role, reopen_queue, | 29 | if (sleep_state->user_state_pointer) { |
34 | + parent_perm, parent_shared, | 30 | *sleep_state->user_state_pointer = NULL; |
35 | + nperm, nshared); | 31 | } |
36 | /* TODO Take force_share from reopen_queue */ | 32 | - timer_del(sleep_state->ts); |
37 | if (child_bs && child_bs->force_share) { | 33 | + timer_del(&sleep_state->ts); |
38 | *nshared = BLK_PERM_ALL; | 34 | aio_co_wake(sleep_state->co); |
35 | } | ||
36 | |||
37 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
38 | AioContext *ctx = qemu_get_current_aio_context(); | ||
39 | QemuCoSleepState state = { | ||
40 | .co = qemu_coroutine_self(), | ||
41 | - .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state), | ||
42 | .user_state_pointer = sleep_state, | ||
43 | }; | ||
44 | |||
45 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
46 | abort(); | ||
47 | } | ||
48 | |||
49 | + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | ||
50 | if (sleep_state) { | ||
51 | *sleep_state = &state; | ||
52 | } | ||
53 | - timer_mod(state.ts, qemu_clock_get_ns(type) + ns); | ||
54 | + timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
55 | qemu_coroutine_yield(); | ||
56 | if (sleep_state) { | ||
57 | /* | ||
58 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
59 | */ | ||
60 | assert(*sleep_state == NULL); | ||
61 | } | ||
62 | - timer_free(state.ts); | ||
63 | } | ||
39 | -- | 64 | -- |
40 | 2.20.1 | 65 | 2.31.1 |
41 | 66 | ||
42 | diff view generated by jsdifflib |
1 | From: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Bitmap data may take a lot of disk space, so it's better to discard it | 3 | Simplify the code by removing conditionals. qemu_co_sleep_ns |
4 | always. | 4 | can simply point the argument to an on-stack temporary. |
5 | 5 | ||
6 | Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com> | ||
7 | Message-id: 1551346019-293202-1-git-send-email-andrey.shinkevich@virtuozzo.com | ||
8 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 6 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
9 | [mreitz: Use the commit message proposed by Vladimir] | 7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
10 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 8 | Message-id: 20210517100548.28806-3-pbonzini@redhat.com |
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | 10 | --- |
12 | block/qcow2-bitmap.c | 2 +- | 11 | include/qemu/coroutine.h | 5 +++-- |
13 | 1 file changed, 1 insertion(+), 1 deletion(-) | 12 | util/qemu-coroutine-sleep.c | 18 +++++------------- |
13 | 2 files changed, 8 insertions(+), 15 deletions(-) | ||
14 | 14 | ||
15 | diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c | 15 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
16 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/block/qcow2-bitmap.c | 17 | --- a/include/qemu/coroutine.h |
18 | +++ b/block/qcow2-bitmap.c | 18 | +++ b/include/qemu/coroutine.h |
19 | @@ -XXX,XX +XXX,XX @@ static void clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table, | 19 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState; |
20 | continue; | 20 | |
21 | } | 21 | /** |
22 | 22 | * Yield the coroutine for a given duration. During this yield, @sleep_state | |
23 | - qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_OTHER); | 23 | - * (if not NULL) is set to an opaque pointer, which may be used for |
24 | + qcow2_free_clusters(bs, addr, s->cluster_size, QCOW2_DISCARD_ALWAYS); | 24 | + * is set to an opaque pointer, which may be used for |
25 | bitmap_table[i] = 0; | 25 | * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the |
26 | * timer fires. Don't save the obtained value to other variables and don't call | ||
27 | * qemu_co_sleep_wake from another aio context. | ||
28 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
29 | QemuCoSleepState **sleep_state); | ||
30 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | ||
31 | { | ||
32 | - qemu_co_sleep_ns_wakeable(type, ns, NULL); | ||
33 | + QemuCoSleepState *unused = NULL; | ||
34 | + qemu_co_sleep_ns_wakeable(type, ns, &unused); | ||
35 | } | ||
36 | |||
37 | /** | ||
38 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/util/qemu-coroutine-sleep.c | ||
41 | +++ b/util/qemu-coroutine-sleep.c | ||
42 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
43 | qemu_co_sleep_ns__scheduled, NULL); | ||
44 | |||
45 | assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
46 | - if (sleep_state->user_state_pointer) { | ||
47 | - *sleep_state->user_state_pointer = NULL; | ||
48 | - } | ||
49 | + *sleep_state->user_state_pointer = NULL; | ||
50 | timer_del(&sleep_state->ts); | ||
51 | aio_co_wake(sleep_state->co); | ||
52 | } | ||
53 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
26 | } | 54 | } |
55 | |||
56 | aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | ||
57 | - if (sleep_state) { | ||
58 | - *sleep_state = &state; | ||
59 | - } | ||
60 | + *sleep_state = &state; | ||
61 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
62 | qemu_coroutine_yield(); | ||
63 | - if (sleep_state) { | ||
64 | - /* | ||
65 | - * Note that *sleep_state is cleared during qemu_co_sleep_wake | ||
66 | - * before resuming this coroutine. | ||
67 | - */ | ||
68 | - assert(*sleep_state == NULL); | ||
69 | - } | ||
70 | + | ||
71 | + /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
72 | + assert(*sleep_state == NULL); | ||
27 | } | 73 | } |
28 | -- | 74 | -- |
29 | 2.20.1 | 75 | 2.31.1 |
30 | 76 | ||
31 | diff view generated by jsdifflib |
1 | ssh_bdrv_dirname() is basically the generic bdrv_dirname(), except it | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | takes care not to silently chop off any query string (i.e., | ||
3 | host_key_check). | ||
4 | 2 | ||
5 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 3 | All callers of qemu_co_sleep_wake are checking whether they are passing |
6 | Tested-by: Richard W.M. Jones <rjones@redhat.com> | 4 | a NULL argument inside the pointer-to-pointer: do the check in |
7 | Message-id: 20190225190828.17726-3-mreitz@redhat.com | 5 | qemu_co_sleep_wake itself. |
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 6 | |
7 | As a side effect, qemu_co_sleep_wake can be called more than once and | ||
8 | it will only wake the coroutine once; after the first time, the argument | ||
9 | will be set to NULL via *sleep_state->user_state_pointer. However, this | ||
10 | would not be safe unless co_sleep_cb keeps using the QemuCoSleepState* | ||
11 | directly, so make it go through the pointer-to-pointer instead. | ||
12 | |||
13 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
14 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
15 | Message-id: 20210517100548.28806-4-pbonzini@redhat.com | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | --- | 17 | --- |
10 | block/ssh.c | 21 +++++++++++++++++++++ | 18 | block/block-copy.c | 4 +--- |
11 | 1 file changed, 21 insertions(+) | 19 | block/nbd.c | 8 ++------ |
20 | util/qemu-coroutine-sleep.c | 21 ++++++++++++--------- | ||
21 | 3 files changed, 15 insertions(+), 18 deletions(-) | ||
12 | 22 | ||
13 | diff --git a/block/ssh.c b/block/ssh.c | 23 | diff --git a/block/block-copy.c b/block/block-copy.c |
14 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/ssh.c | 25 | --- a/block/block-copy.c |
16 | +++ b/block/ssh.c | 26 | +++ b/block/block-copy.c |
17 | @@ -XXX,XX +XXX,XX @@ static void ssh_refresh_filename(BlockDriverState *bs) | 27 | @@ -XXX,XX +XXX,XX @@ out: |
28 | |||
29 | void block_copy_kick(BlockCopyCallState *call_state) | ||
30 | { | ||
31 | - if (call_state->sleep_state) { | ||
32 | - qemu_co_sleep_wake(call_state->sleep_state); | ||
33 | - } | ||
34 | + qemu_co_sleep_wake(call_state->sleep_state); | ||
35 | } | ||
36 | |||
37 | /* | ||
38 | diff --git a/block/nbd.c b/block/nbd.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/block/nbd.c | ||
41 | +++ b/block/nbd.c | ||
42 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) | ||
43 | BDRVNBDState *s = (BDRVNBDState *)bs->opaque; | ||
44 | |||
45 | s->drained = true; | ||
46 | - if (s->connection_co_sleep_ns_state) { | ||
47 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
48 | - } | ||
49 | + qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
50 | |||
51 | nbd_co_establish_connection_cancel(bs, false); | ||
52 | |||
53 | @@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs) | ||
54 | |||
55 | s->state = NBD_CLIENT_QUIT; | ||
56 | if (s->connection_co) { | ||
57 | - if (s->connection_co_sleep_ns_state) { | ||
58 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
59 | - } | ||
60 | + qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
61 | nbd_co_establish_connection_cancel(bs, true); | ||
18 | } | 62 | } |
63 | if (qemu_in_coroutine()) { | ||
64 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
65 | index XXXXXXX..XXXXXXX 100644 | ||
66 | --- a/util/qemu-coroutine-sleep.c | ||
67 | +++ b/util/qemu-coroutine-sleep.c | ||
68 | @@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState { | ||
69 | |||
70 | void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
71 | { | ||
72 | - /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
73 | - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
74 | - qemu_co_sleep_ns__scheduled, NULL); | ||
75 | + if (sleep_state) { | ||
76 | + /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
77 | + const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
78 | + qemu_co_sleep_ns__scheduled, NULL); | ||
79 | |||
80 | - assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
81 | - *sleep_state->user_state_pointer = NULL; | ||
82 | - timer_del(&sleep_state->ts); | ||
83 | - aio_co_wake(sleep_state->co); | ||
84 | + assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
85 | + *sleep_state->user_state_pointer = NULL; | ||
86 | + timer_del(&sleep_state->ts); | ||
87 | + aio_co_wake(sleep_state->co); | ||
88 | + } | ||
19 | } | 89 | } |
20 | 90 | ||
21 | +static char *ssh_bdrv_dirname(BlockDriverState *bs, Error **errp) | 91 | static void co_sleep_cb(void *opaque) |
22 | +{ | 92 | { |
23 | + if (qdict_haskey(bs->full_open_options, "host_key_check")) { | 93 | - qemu_co_sleep_wake(opaque); |
24 | + /* | 94 | + QemuCoSleepState **sleep_state = opaque; |
25 | + * We cannot generate a simple prefix if we would have to | 95 | + qemu_co_sleep_wake(*sleep_state); |
26 | + * append a query string. | 96 | } |
27 | + */ | 97 | |
28 | + error_setg(errp, | 98 | void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, |
29 | + "Cannot generate a base directory with host_key_check set"); | 99 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, |
30 | + return NULL; | 100 | abort(); |
31 | + } | 101 | } |
32 | + | 102 | |
33 | + if (bs->exact_filename[0] == '\0') { | 103 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); |
34 | + error_setg(errp, "Cannot generate a base directory for this ssh node"); | 104 | + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); |
35 | + return NULL; | 105 | *sleep_state = &state; |
36 | + } | 106 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); |
37 | + | 107 | qemu_coroutine_yield(); |
38 | + return path_combine(bs->exact_filename, ""); | ||
39 | +} | ||
40 | + | ||
41 | static const char *const ssh_strong_runtime_opts[] = { | ||
42 | "host", | ||
43 | "port", | ||
44 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_ssh = { | ||
45 | .bdrv_co_truncate = ssh_co_truncate, | ||
46 | .bdrv_co_flush_to_disk = ssh_co_flush, | ||
47 | .bdrv_refresh_filename = ssh_refresh_filename, | ||
48 | + .bdrv_dirname = ssh_bdrv_dirname, | ||
49 | .create_opts = &ssh_create_opts, | ||
50 | .strong_runtime_opts = ssh_strong_runtime_opts, | ||
51 | }; | ||
52 | -- | 108 | -- |
53 | 2.20.1 | 109 | 2.31.1 |
54 | 110 | ||
55 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Increase corruptions_fixed only after successful fix. | 3 | This simplification is enabled by the previous patch. Now aio_co_wake |
4 | will only be called once, therefore we do not care about a spurious | ||
5 | firing of the timer after a qemu_co_sleep_wake. | ||
4 | 6 | ||
5 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
6 | Reviewed-by: Max Reitz <mreitz@redhat.com> | 8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
7 | Message-id: 20190227131433.197063-2-vsementsov@virtuozzo.com | 9 | Message-id: 20210517100548.28806-5-pbonzini@redhat.com |
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
9 | --- | 11 | --- |
10 | block/qcow2-refcount.c | 8 ++++---- | 12 | util/qemu-coroutine-sleep.c | 8 ++++---- |
11 | 1 file changed, 4 insertions(+), 4 deletions(-) | 13 | 1 file changed, 4 insertions(+), 4 deletions(-) |
12 | 14 | ||
13 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | 15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
14 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/block/qcow2-refcount.c | 17 | --- a/util/qemu-coroutine-sleep.c |
16 | +++ b/block/qcow2-refcount.c | 18 | +++ b/util/qemu-coroutine-sleep.c |
17 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; |
18 | for (i = 0; i < s->l1_size; i++) { | 20 | |
19 | uint64_t l1_entry = s->l1_table[i]; | 21 | struct QemuCoSleepState { |
20 | uint64_t l2_offset = l1_entry & L1E_OFFSET_MASK; | 22 | Coroutine *co; |
21 | - bool l2_dirty = false; | 23 | - QEMUTimer ts; |
22 | + int l2_dirty = 0; | 24 | QemuCoSleepState **user_state_pointer; |
23 | 25 | }; | |
24 | if (!l2_offset) { | 26 | |
25 | continue; | 27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) |
26 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 28 | |
27 | l2_table[j] = cpu_to_be64(refcount == 1 | 29 | assert(scheduled == qemu_co_sleep_ns__scheduled); |
28 | ? l2_entry | QCOW_OFLAG_COPIED | 30 | *sleep_state->user_state_pointer = NULL; |
29 | : l2_entry & ~QCOW_OFLAG_COPIED); | 31 | - timer_del(&sleep_state->ts); |
30 | - l2_dirty = true; | 32 | aio_co_wake(sleep_state->co); |
31 | - res->corruptions_fixed++; | ||
32 | + l2_dirty++; | ||
33 | } else { | ||
34 | res->corruptions++; | ||
35 | } | ||
36 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | ||
37 | } | ||
38 | } | ||
39 | |||
40 | - if (l2_dirty) { | ||
41 | + if (l2_dirty > 0) { | ||
42 | ret = qcow2_pre_write_overlap_check(bs, QCOW2_OL_ACTIVE_L2, | ||
43 | l2_offset, s->cluster_size, | ||
44 | false); | ||
45 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | ||
46 | res->check_errors++; | ||
47 | goto fail; | ||
48 | } | ||
49 | + res->corruptions_fixed += l2_dirty; | ||
50 | } | ||
51 | } | 33 | } |
52 | 34 | } | |
35 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
36 | QemuCoSleepState **sleep_state) | ||
37 | { | ||
38 | AioContext *ctx = qemu_get_current_aio_context(); | ||
39 | + QEMUTimer ts; | ||
40 | QemuCoSleepState state = { | ||
41 | .co = qemu_coroutine_self(), | ||
42 | .user_state_pointer = sleep_state, | ||
43 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
44 | abort(); | ||
45 | } | ||
46 | |||
47 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
48 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
49 | *sleep_state = &state; | ||
50 | - timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
51 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
52 | qemu_coroutine_yield(); | ||
53 | + timer_del(&ts); | ||
54 | |||
55 | /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
56 | assert(*sleep_state == NULL); | ||
53 | -- | 57 | -- |
54 | 2.20.1 | 58 | 2.31.1 |
55 | 59 | ||
56 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | No reasons for not reporting found corruptions as corruptions in case | 3 | Right now, users of qemu_co_sleep_ns_wakeable are simply passing |
4 | of some internal errors, especially in case of just failed to fix l2 | 4 | a pointer to QemuCoSleepState by reference to the function. But |
5 | entry (and in this case, missed corruptions may influence comparing | 5 | QemuCoSleepState really is just a Coroutine*; making the |
6 | logic, when we calculate difference between corruptions fields of two | 6 | content of the struct public is just as efficient and lets us |
7 | results) | 7 | skip the user_state_pointer indirection. |
8 | 8 | ||
9 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 9 | Since the usage is changed, take the occasion to rename the |
10 | Message-id: 20190227131433.197063-6-vsementsov@virtuozzo.com | 10 | struct to QemuCoSleep. |
11 | Reviewed-by: Max Reitz <mreitz@redhat.com> | 11 | |
12 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
13 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
14 | Message-id: 20210517100548.28806-6-pbonzini@redhat.com | ||
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
13 | --- | 16 | --- |
14 | block/qcow2-refcount.c | 19 +++++++++---------- | 17 | include/qemu/coroutine.h | 23 +++++++++++---------- |
15 | 1 file changed, 9 insertions(+), 10 deletions(-) | 18 | block/block-copy.c | 8 ++++---- |
16 | 19 | block/nbd.c | 10 ++++----- | |
17 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | 20 | util/qemu-coroutine-sleep.c | 41 ++++++++++++++++--------------------- |
18 | index XXXXXXX..XXXXXXX 100644 | 21 | 4 files changed, 39 insertions(+), 43 deletions(-) |
19 | --- a/block/qcow2-refcount.c | 22 | |
20 | +++ b/block/qcow2-refcount.c | 23 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
21 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | 24 | index XXXXXXX..XXXXXXX 100644 |
22 | 25 | --- a/include/qemu/coroutine.h | |
23 | /* Correct offsets are cluster aligned */ | 26 | +++ b/include/qemu/coroutine.h |
24 | if (offset_into_cluster(s, offset)) { | 27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock); |
25 | + res->corruptions++; | 28 | */ |
29 | void qemu_co_rwlock_unlock(CoRwlock *lock); | ||
30 | |||
31 | -typedef struct QemuCoSleepState QemuCoSleepState; | ||
32 | +typedef struct QemuCoSleep { | ||
33 | + Coroutine *to_wake; | ||
34 | +} QemuCoSleep; | ||
35 | |||
36 | /** | ||
37 | - * Yield the coroutine for a given duration. During this yield, @sleep_state | ||
38 | - * is set to an opaque pointer, which may be used for | ||
39 | - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | ||
40 | - * timer fires. Don't save the obtained value to other variables and don't call | ||
41 | - * qemu_co_sleep_wake from another aio context. | ||
42 | + * Yield the coroutine for a given duration. Initializes @w so that, | ||
43 | + * during this yield, it can be passed to qemu_co_sleep_wake() to | ||
44 | + * terminate the sleep. | ||
45 | */ | ||
46 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
47 | - QemuCoSleepState **sleep_state); | ||
48 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
49 | + QEMUClockType type, int64_t ns); | ||
26 | + | 50 | + |
27 | if (qcow2_get_cluster_type(bs, l2_entry) == | 51 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) |
28 | QCOW2_CLUSTER_ZERO_ALLOC) | 52 | { |
29 | { | 53 | - QemuCoSleepState *unused = NULL; |
30 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | 54 | - qemu_co_sleep_ns_wakeable(type, ns, &unused); |
31 | /* Do not abort, continue checking the rest of this | 55 | + QemuCoSleep w = { 0 }; |
32 | * L2 table's entries */ | 56 | + qemu_co_sleep_ns_wakeable(&w, type, ns); |
33 | } else { | 57 | } |
34 | + res->corruptions--; | 58 | |
35 | res->corruptions_fixed++; | 59 | /** |
36 | /* Skip marking the cluster as used | 60 | @@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) |
37 | * (it is unused now) */ | 61 | * qemu_co_sleep_ns() and should be checked to be non-NULL before calling |
38 | continue; | 62 | * qemu_co_sleep_wake(). |
39 | } | 63 | */ |
40 | - } else { | 64 | -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state); |
41 | - res->corruptions++; | 65 | +void qemu_co_sleep_wake(QemuCoSleep *w); |
42 | } | 66 | |
43 | } else { | 67 | /** |
44 | fprintf(stderr, "ERROR offset=%" PRIx64 ": Data cluster is " | 68 | * Yield until a file descriptor becomes readable |
45 | "not properly aligned; L2 entry corrupted.\n", offset); | 69 | diff --git a/block/block-copy.c b/block/block-copy.c |
46 | - res->corruptions++; | 70 | index XXXXXXX..XXXXXXX 100644 |
71 | --- a/block/block-copy.c | ||
72 | +++ b/block/block-copy.c | ||
73 | @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState { | ||
74 | /* State */ | ||
75 | int ret; | ||
76 | bool finished; | ||
77 | - QemuCoSleepState *sleep_state; | ||
78 | + QemuCoSleep sleep; | ||
79 | bool cancelled; | ||
80 | |||
81 | /* OUT parameters */ | ||
82 | @@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) | ||
83 | if (ns > 0) { | ||
84 | block_copy_task_end(task, -EAGAIN); | ||
85 | g_free(task); | ||
86 | - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns, | ||
87 | - &call_state->sleep_state); | ||
88 | + qemu_co_sleep_ns_wakeable(&call_state->sleep, | ||
89 | + QEMU_CLOCK_REALTIME, ns); | ||
90 | continue; | ||
47 | } | 91 | } |
48 | } | 92 | } |
49 | 93 | @@ -XXX,XX +XXX,XX @@ out: | |
50 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 94 | |
51 | continue; | 95 | void block_copy_kick(BlockCopyCallState *call_state) |
52 | } | 96 | { |
53 | if ((refcount == 1) != ((l1_entry & QCOW_OFLAG_COPIED) != 0)) { | 97 | - qemu_co_sleep_wake(call_state->sleep_state); |
54 | + res->corruptions++; | 98 | + qemu_co_sleep_wake(&call_state->sleep); |
55 | fprintf(stderr, "%s OFLAG_COPIED L2 cluster: l1_index=%d " | 99 | } |
56 | "l1_entry=%" PRIx64 " refcount=%" PRIu64 "\n", | 100 | |
57 | repair ? "Repairing" : "ERROR", i, l1_entry, refcount); | 101 | /* |
58 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 102 | diff --git a/block/nbd.c b/block/nbd.c |
59 | res->check_errors++; | 103 | index XXXXXXX..XXXXXXX 100644 |
60 | goto fail; | 104 | --- a/block/nbd.c |
61 | } | 105 | +++ b/block/nbd.c |
62 | + res->corruptions--; | 106 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState { |
63 | res->corruptions_fixed++; | 107 | CoQueue free_sema; |
64 | - } else { | 108 | Coroutine *connection_co; |
65 | - res->corruptions++; | 109 | Coroutine *teardown_co; |
110 | - QemuCoSleepState *connection_co_sleep_ns_state; | ||
111 | + QemuCoSleep reconnect_sleep; | ||
112 | bool drained; | ||
113 | bool wait_drained_end; | ||
114 | int in_flight; | ||
115 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) | ||
116 | BDRVNBDState *s = (BDRVNBDState *)bs->opaque; | ||
117 | |||
118 | s->drained = true; | ||
119 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
120 | + qemu_co_sleep_wake(&s->reconnect_sleep); | ||
121 | |||
122 | nbd_co_establish_connection_cancel(bs, false); | ||
123 | |||
124 | @@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs) | ||
125 | |||
126 | s->state = NBD_CLIENT_QUIT; | ||
127 | if (s->connection_co) { | ||
128 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
129 | + qemu_co_sleep_wake(&s->reconnect_sleep); | ||
130 | nbd_co_establish_connection_cancel(bs, true); | ||
131 | } | ||
132 | if (qemu_in_coroutine()) { | ||
133 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) | ||
66 | } | 134 | } |
67 | } | 135 | bdrv_inc_in_flight(s->bs); |
68 | 136 | } else { | |
69 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 137 | - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout, |
70 | } | 138 | - &s->connection_co_sleep_ns_state); |
71 | } | 139 | + qemu_co_sleep_ns_wakeable(&s->reconnect_sleep, |
72 | if ((refcount == 1) != ((l2_entry & QCOW_OFLAG_COPIED) != 0)) { | 140 | + QEMU_CLOCK_REALTIME, timeout); |
73 | + res->corruptions++; | 141 | if (s->drained) { |
74 | fprintf(stderr, "%s OFLAG_COPIED data cluster: " | 142 | continue; |
75 | "l2_entry=%" PRIx64 " refcount=%" PRIu64 "\n", | ||
76 | repair ? "Repairing" : "ERROR", l2_entry, refcount); | ||
77 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | ||
78 | ? l2_entry | QCOW_OFLAG_COPIED | ||
79 | : l2_entry & ~QCOW_OFLAG_COPIED); | ||
80 | l2_dirty++; | ||
81 | - } else { | ||
82 | - res->corruptions++; | ||
83 | } | ||
84 | } | ||
85 | } | 143 | } |
86 | @@ -XXX,XX +XXX,XX @@ static int check_oflag_copied(BlockDriverState *bs, BdrvCheckResult *res, | 144 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
87 | res->check_errors++; | 145 | index XXXXXXX..XXXXXXX 100644 |
88 | goto fail; | 146 | --- a/util/qemu-coroutine-sleep.c |
89 | } | 147 | +++ b/util/qemu-coroutine-sleep.c |
90 | + res->corruptions -= l2_dirty; | 148 | @@ -XXX,XX +XXX,XX @@ |
91 | res->corruptions_fixed += l2_dirty; | 149 | |
92 | } | 150 | static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; |
151 | |||
152 | -struct QemuCoSleepState { | ||
153 | +void qemu_co_sleep_wake(QemuCoSleep *w) | ||
154 | +{ | ||
155 | Coroutine *co; | ||
156 | - QemuCoSleepState **user_state_pointer; | ||
157 | -}; | ||
158 | |||
159 | -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
160 | -{ | ||
161 | - if (sleep_state) { | ||
162 | + co = w->to_wake; | ||
163 | + w->to_wake = NULL; | ||
164 | + if (co) { | ||
165 | /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
166 | - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
167 | + const char *scheduled = qatomic_cmpxchg(&co->scheduled, | ||
168 | qemu_co_sleep_ns__scheduled, NULL); | ||
169 | |||
170 | assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
171 | - *sleep_state->user_state_pointer = NULL; | ||
172 | - aio_co_wake(sleep_state->co); | ||
173 | + aio_co_wake(co); | ||
93 | } | 174 | } |
94 | @@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, | 175 | } |
95 | } | 176 | |
96 | 177 | static void co_sleep_cb(void *opaque) | |
97 | if (cluster >= *nb_clusters) { | 178 | { |
98 | + res->corruptions++; | 179 | - QemuCoSleepState **sleep_state = opaque; |
99 | fprintf(stderr, "%s refcount block %" PRId64 " is outside image\n", | 180 | - qemu_co_sleep_wake(*sleep_state); |
100 | fix & BDRV_FIX_ERRORS ? "Repairing" : "ERROR", i); | 181 | + QemuCoSleep *w = opaque; |
101 | 182 | + qemu_co_sleep_wake(w); | |
102 | @@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, | 183 | } |
103 | goto resize_fail; | 184 | |
104 | } | 185 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, |
105 | 186 | - QemuCoSleepState **sleep_state) | |
106 | + res->corruptions--; | 187 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, |
107 | res->corruptions_fixed++; | 188 | + QEMUClockType type, int64_t ns) |
108 | ret = qcow2_inc_refcounts_imrt(bs, res, | 189 | { |
109 | refcount_table, nb_clusters, | 190 | + Coroutine *co = qemu_coroutine_self(); |
110 | @@ -XXX,XX +XXX,XX @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, | 191 | AioContext *ctx = qemu_get_current_aio_context(); |
111 | continue; | 192 | QEMUTimer ts; |
112 | 193 | - QemuCoSleepState state = { | |
113 | resize_fail: | 194 | - .co = qemu_coroutine_self(), |
114 | - res->corruptions++; | 195 | - .user_state_pointer = sleep_state, |
115 | *rebuild = true; | 196 | - }; |
116 | fprintf(stderr, "ERROR could not resize image: %s\n", | 197 | |
117 | strerror(-ret)); | 198 | - const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL, |
118 | - } else { | 199 | - qemu_co_sleep_ns__scheduled); |
119 | - res->corruptions++; | 200 | + const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, |
120 | } | 201 | + qemu_co_sleep_ns__scheduled); |
121 | continue; | 202 | if (scheduled) { |
122 | } | 203 | fprintf(stderr, |
204 | "%s: Co-routine was already scheduled in '%s'\n", | ||
205 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
206 | abort(); | ||
207 | } | ||
208 | |||
209 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
210 | - *sleep_state = &state; | ||
211 | + w->to_wake = co; | ||
212 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), | ||
213 | timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
214 | qemu_coroutine_yield(); | ||
215 | timer_del(&ts); | ||
216 | |||
217 | - /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
218 | - assert(*sleep_state == NULL); | ||
219 | + /* w->to_wake is cleared before resuming this coroutine. */ | ||
220 | + assert(w->to_wake == NULL); | ||
221 | } | ||
123 | -- | 222 | -- |
124 | 2.20.1 | 223 | 2.31.1 |
125 | 224 | ||
126 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | qcow2_inc_refcounts_imrt() (through realloc_refcount_array()) can eat | 3 | Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake. |
4 | an unpredictable amount of memory on corrupted table entries, which are | 4 | This makes the logic of qemu_co_sleep_ns_wakeable easy to understand. |
5 | referencing regions far beyond the end of file. | ||
6 | 5 | ||
7 | Prevent this, by skipping such regions from further processing. | 6 | In the future we will introduce an API that can work even if the |
7 | sleep and wake happen from different threads. For now, initializing | ||
8 | w->to_wake after timer_mod is fine because the timer can only fire in | ||
9 | the same AioContext. | ||
8 | 10 | ||
9 | Interesting that iotest 138 checks exactly the behavior which we fix | 11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
10 | here. So, change the test appropriately. | 12 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
13 | Message-id: 20210517100548.28806-7-pbonzini@redhat.com | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
15 | --- | ||
16 | include/qemu/coroutine.h | 5 +++++ | ||
17 | util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++------- | ||
18 | 2 files changed, 24 insertions(+), 7 deletions(-) | ||
11 | 19 | ||
12 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 20 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
13 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
14 | Message-id: 20190227131433.197063-3-vsementsov@virtuozzo.com | ||
15 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
16 | --- | ||
17 | block/qcow2-refcount.c | 19 +++++++++++++++++++ | ||
18 | tests/qemu-iotests/138 | 12 +++++------- | ||
19 | tests/qemu-iotests/138.out | 5 ++++- | ||
20 | 3 files changed, 28 insertions(+), 8 deletions(-) | ||
21 | |||
22 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | ||
23 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
24 | --- a/block/qcow2-refcount.c | 22 | --- a/include/qemu/coroutine.h |
25 | +++ b/block/qcow2-refcount.c | 23 | +++ b/include/qemu/coroutine.h |
26 | @@ -XXX,XX +XXX,XX @@ int qcow2_inc_refcounts_imrt(BlockDriverState *bs, BdrvCheckResult *res, | 24 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep { |
25 | void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
26 | QEMUClockType type, int64_t ns); | ||
27 | |||
28 | +/** | ||
29 | + * Yield the coroutine until the next call to qemu_co_sleep_wake. | ||
30 | + */ | ||
31 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w); | ||
32 | + | ||
33 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | ||
27 | { | 34 | { |
28 | BDRVQcow2State *s = bs->opaque; | 35 | QemuCoSleep w = { 0 }; |
29 | uint64_t start, last, cluster_offset, k, refcount; | 36 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
30 | + int64_t file_len; | 37 | index XXXXXXX..XXXXXXX 100644 |
31 | int ret; | 38 | --- a/util/qemu-coroutine-sleep.c |
32 | 39 | +++ b/util/qemu-coroutine-sleep.c | |
33 | if (size <= 0) { | 40 | @@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque) |
34 | return 0; | 41 | qemu_co_sleep_wake(w); |
42 | } | ||
43 | |||
44 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
45 | - QEMUClockType type, int64_t ns) | ||
46 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w) | ||
47 | { | ||
48 | Coroutine *co = qemu_coroutine_self(); | ||
49 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
50 | - QEMUTimer ts; | ||
51 | |||
52 | const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, | ||
53 | qemu_co_sleep_ns__scheduled); | ||
54 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
35 | } | 55 | } |
36 | 56 | ||
37 | + file_len = bdrv_getlength(bs->file->bs); | 57 | w->to_wake = co; |
38 | + if (file_len < 0) { | 58 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), |
39 | + return file_len; | 59 | - timer_mod(&ts, qemu_clock_get_ns(type) + ns); |
40 | + } | 60 | qemu_coroutine_yield(); |
61 | - timer_del(&ts); | ||
62 | |||
63 | /* w->to_wake is cleared before resuming this coroutine. */ | ||
64 | assert(w->to_wake == NULL); | ||
65 | } | ||
66 | + | ||
67 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
68 | + QEMUClockType type, int64_t ns) | ||
69 | +{ | ||
70 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
71 | + QEMUTimer ts; | ||
72 | + | ||
73 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w); | ||
74 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
41 | + | 75 | + |
42 | + /* | 76 | + /* |
43 | + * Last cluster of qcow2 image may be semi-allocated, so it may be OK to | 77 | + * The timer will fire in the current AiOContext, so the callback |
44 | + * reference some space after file end but it should be less than one | 78 | + * must happen after qemu_co_sleep yields and there is no race |
45 | + * cluster. | 79 | + * between timer_mod and qemu_co_sleep. |
46 | + */ | 80 | + */ |
47 | + if (offset + size - file_len >= s->cluster_size) { | 81 | + qemu_co_sleep(w); |
48 | + fprintf(stderr, "ERROR: counting reference for region exceeding the " | 82 | + timer_del(&ts); |
49 | + "end of the file by one cluster or more: offset 0x%" PRIx64 | 83 | +} |
50 | + " size 0x%" PRIx64 "\n", offset, size); | ||
51 | + res->corruptions++; | ||
52 | + return 0; | ||
53 | + } | ||
54 | + | ||
55 | start = start_of_cluster(s, offset); | ||
56 | last = start_of_cluster(s, offset + size - 1); | ||
57 | for(cluster_offset = start; cluster_offset <= last; | ||
58 | diff --git a/tests/qemu-iotests/138 b/tests/qemu-iotests/138 | ||
59 | index XXXXXXX..XXXXXXX 100755 | ||
60 | --- a/tests/qemu-iotests/138 | ||
61 | +++ b/tests/qemu-iotests/138 | ||
62 | @@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'write 0 512' "$TEST_IMG" | _filter_qemu_io | ||
63 | # Put the data cluster at a multiple of 2 TB, resulting in the image apparently | ||
64 | # having a multiple of 2^32 clusters | ||
65 | # (To be more specific: It is at 32 PB) | ||
66 | -poke_file "$TEST_IMG" 2048 "\x80\x80\x00\x00\x00\x00\x00\x00" | ||
67 | +poke_file "$TEST_IMG" $((2048 + 8)) "\x00\x80\x00\x00\x00\x00\x00\x00" | ||
68 | |||
69 | # An offset of 32 PB results in qemu-img check having to allocate an in-memory | ||
70 | -# refcount table of 128 TB (16 bit refcounts, 512 byte clusters). | ||
71 | -# This should be generally too much for any system and thus fail. | ||
72 | -# What this test is checking is that the qcow2 driver actually tries to allocate | ||
73 | -# such a large amount of memory (and is consequently aborting) instead of having | ||
74 | -# truncated the cluster count somewhere (which would result in much less memory | ||
75 | -# being allocated and then a segfault occurring). | ||
76 | +# refcount table of 128 TB (16 bit refcounts, 512 byte clusters), if qemu-img | ||
77 | +# don't check that referenced data cluster is far beyond the end of file. | ||
78 | +# But starting from 4.0, qemu-img does this check, and instead of "Cannot | ||
79 | +# allocate memory", we have an error showing that l2 entry is invalid. | ||
80 | _check_test_img | ||
81 | |||
82 | # success, all done | ||
83 | diff --git a/tests/qemu-iotests/138.out b/tests/qemu-iotests/138.out | ||
84 | index XXXXXXX..XXXXXXX 100644 | ||
85 | --- a/tests/qemu-iotests/138.out | ||
86 | +++ b/tests/qemu-iotests/138.out | ||
87 | @@ -XXX,XX +XXX,XX @@ QA output created by 138 | ||
88 | Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=512 | ||
89 | wrote 512/512 bytes at offset 0 | ||
90 | 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
91 | -qemu-img: Check failed: Cannot allocate memory | ||
92 | +ERROR: counting reference for region exceeding the end of the file by one cluster or more: offset 0x80000000000000 size 0x200 | ||
93 | + | ||
94 | +1 errors were found on the image. | ||
95 | +Data may be corrupted, or further writes to the image may corrupt it. | ||
96 | *** done | ||
97 | -- | 84 | -- |
98 | 2.20.1 | 85 | 2.31.1 |
99 | 86 | ||
100 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Reduce number of structures ignored in overlap check: when checking | ||
4 | active table ignore active tables, when checking inactive table ignore | ||
5 | inactive ones. | ||
6 | |||
7 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
9 | Message-id: 20190227131433.197063-4-vsementsov@virtuozzo.com | ||
10 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
11 | --- | ||
12 | block/qcow2-refcount.c | 16 +++++++++------- | ||
13 | 1 file changed, 9 insertions(+), 7 deletions(-) | ||
14 | |||
15 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/block/qcow2-refcount.c | ||
18 | +++ b/block/qcow2-refcount.c | ||
19 | @@ -XXX,XX +XXX,XX @@ enum { | ||
20 | static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | ||
21 | void **refcount_table, | ||
22 | int64_t *refcount_table_size, int64_t l2_offset, | ||
23 | - int flags, BdrvCheckMode fix) | ||
24 | + int flags, BdrvCheckMode fix, bool active) | ||
25 | { | ||
26 | BDRVQcow2State *s = bs->opaque; | ||
27 | uint64_t *l2_table, l2_entry; | ||
28 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | ||
29 | if (fix & BDRV_FIX_ERRORS) { | ||
30 | uint64_t l2e_offset = | ||
31 | l2_offset + (uint64_t)i * sizeof(uint64_t); | ||
32 | + int ign = active ? QCOW2_OL_ACTIVE_L2 : | ||
33 | + QCOW2_OL_INACTIVE_L2; | ||
34 | |||
35 | l2_entry = QCOW_OFLAG_ZERO; | ||
36 | l2_table[i] = cpu_to_be64(l2_entry); | ||
37 | - ret = qcow2_pre_write_overlap_check(bs, | ||
38 | - QCOW2_OL_ACTIVE_L2 | QCOW2_OL_INACTIVE_L2, | ||
39 | + ret = qcow2_pre_write_overlap_check(bs, ign, | ||
40 | l2e_offset, sizeof(uint64_t), false); | ||
41 | if (ret < 0) { | ||
42 | fprintf(stderr, "ERROR: Overlap check failed\n"); | ||
43 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l1(BlockDriverState *bs, | ||
44 | void **refcount_table, | ||
45 | int64_t *refcount_table_size, | ||
46 | int64_t l1_table_offset, int l1_size, | ||
47 | - int flags, BdrvCheckMode fix) | ||
48 | + int flags, BdrvCheckMode fix, bool active) | ||
49 | { | ||
50 | BDRVQcow2State *s = bs->opaque; | ||
51 | uint64_t *l1_table = NULL, l2_offset, l1_size2; | ||
52 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l1(BlockDriverState *bs, | ||
53 | /* Process and check L2 entries */ | ||
54 | ret = check_refcounts_l2(bs, res, refcount_table, | ||
55 | refcount_table_size, l2_offset, flags, | ||
56 | - fix); | ||
57 | + fix, active); | ||
58 | if (ret < 0) { | ||
59 | goto fail; | ||
60 | } | ||
61 | @@ -XXX,XX +XXX,XX @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, | ||
62 | /* current L1 table */ | ||
63 | ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, | ||
64 | s->l1_table_offset, s->l1_size, CHECK_FRAG_INFO, | ||
65 | - fix); | ||
66 | + fix, true); | ||
67 | if (ret < 0) { | ||
68 | return ret; | ||
69 | } | ||
70 | @@ -XXX,XX +XXX,XX @@ static int calculate_refcounts(BlockDriverState *bs, BdrvCheckResult *res, | ||
71 | continue; | ||
72 | } | ||
73 | ret = check_refcounts_l1(bs, res, refcount_table, nb_clusters, | ||
74 | - sn->l1_table_offset, sn->l1_size, 0, fix); | ||
75 | + sn->l1_table_offset, sn->l1_size, 0, fix, | ||
76 | + false); | ||
77 | if (ret < 0) { | ||
78 | return ret; | ||
79 | } | ||
80 | -- | ||
81 | 2.20.1 | ||
82 | |||
83 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Do not count a cluster which is fixed to be ZERO as allocated. | ||
4 | |||
5 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
6 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
7 | Message-id: 20190227131433.197063-5-vsementsov@virtuozzo.com | ||
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
9 | --- | ||
10 | block/qcow2-refcount.c | 18 +++++++++--------- | ||
11 | 1 file changed, 9 insertions(+), 9 deletions(-) | ||
12 | |||
13 | diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/qcow2-refcount.c | ||
16 | +++ b/block/qcow2-refcount.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | ||
18 | { | ||
19 | uint64_t offset = l2_entry & L2E_OFFSET_MASK; | ||
20 | |||
21 | - if (flags & CHECK_FRAG_INFO) { | ||
22 | - res->bfi.allocated_clusters++; | ||
23 | - if (next_contiguous_offset && | ||
24 | - offset != next_contiguous_offset) { | ||
25 | - res->bfi.fragmented_clusters++; | ||
26 | - } | ||
27 | - next_contiguous_offset = offset + s->cluster_size; | ||
28 | - } | ||
29 | - | ||
30 | /* Correct offsets are cluster aligned */ | ||
31 | if (offset_into_cluster(s, offset)) { | ||
32 | if (qcow2_get_cluster_type(bs, l2_entry) == | ||
33 | @@ -XXX,XX +XXX,XX @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, | ||
34 | } | ||
35 | } | ||
36 | |||
37 | + if (flags & CHECK_FRAG_INFO) { | ||
38 | + res->bfi.allocated_clusters++; | ||
39 | + if (next_contiguous_offset && | ||
40 | + offset != next_contiguous_offset) { | ||
41 | + res->bfi.fragmented_clusters++; | ||
42 | + } | ||
43 | + next_contiguous_offset = offset + s->cluster_size; | ||
44 | + } | ||
45 | + | ||
46 | /* Mark cluster as used */ | ||
47 | if (!has_data_file(bs)) { | ||
48 | ret = qcow2_inc_refcounts_imrt(bs, res, refcount_table, | ||
49 | -- | ||
50 | 2.20.1 | ||
51 | |||
52 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | This requires some changes to keep iotests 104 and 207 working. | ||
2 | 1 | ||
3 | qemu-img info in 104 will now return a filename including the user name | ||
4 | and the port, which need to be filtered by adjusting REMOTE_TEST_DIR in | ||
5 | common.rc. This additional information has to be marked optional, | ||
6 | however (which is simple as REMOTE_TEST_DIR is a regex), because | ||
7 | otherwise 197 and 215 would fail: They use it (indirectly) to filter | ||
8 | qemu-img create output which contains a backing filename they have | ||
9 | passed to it -- which probably does not contain a user name or port | ||
10 | number. | ||
11 | |||
12 | The problem in 207 is a nice one to have: qemu-img info used to return | ||
13 | json:{} filenames, but with this patch it returns nice plain ones. We | ||
14 | now need to adjust the filtering to hide the user name (and port number | ||
15 | while we are at it). The simplest way to do this is to include both in | ||
16 | iotests.remote_filename() so that bdrv_refresh_filename() will not | ||
17 | change it, and then iotests.img_info_log() will filter it correctly | ||
18 | automatically. | ||
19 | |||
20 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
21 | Tested-by: Richard W.M. Jones <rjones@redhat.com> | ||
22 | Message-id: 20190225190828.17726-2-mreitz@redhat.com | ||
23 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
24 | --- | ||
25 | block/ssh.c | 52 +++++++++++++++++++++++++++++++---- | ||
26 | tests/qemu-iotests/207 | 10 +++---- | ||
27 | tests/qemu-iotests/207.out | 10 +++---- | ||
28 | tests/qemu-iotests/common.rc | 2 +- | ||
29 | tests/qemu-iotests/iotests.py | 2 +- | ||
30 | 5 files changed, 59 insertions(+), 17 deletions(-) | ||
31 | |||
32 | diff --git a/block/ssh.c b/block/ssh.c | ||
33 | index XXXXXXX..XXXXXXX 100644 | ||
34 | --- a/block/ssh.c | ||
35 | +++ b/block/ssh.c | ||
36 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVSSHState { | ||
37 | |||
38 | /* Used to warn if 'flush' is not supported. */ | ||
39 | bool unsafe_flush_warning; | ||
40 | + | ||
41 | + /* | ||
42 | + * Store the user name for ssh_refresh_filename() because the | ||
43 | + * default depends on the system you are on -- therefore, when we | ||
44 | + * generate a filename, it should always contain the user name we | ||
45 | + * are actually using. | ||
46 | + */ | ||
47 | + char *user; | ||
48 | } BDRVSSHState; | ||
49 | |||
50 | static void ssh_state_init(BDRVSSHState *s) | ||
51 | @@ -XXX,XX +XXX,XX @@ static void ssh_state_init(BDRVSSHState *s) | ||
52 | |||
53 | static void ssh_state_free(BDRVSSHState *s) | ||
54 | { | ||
55 | + g_free(s->user); | ||
56 | + | ||
57 | if (s->sftp_handle) { | ||
58 | libssh2_sftp_close(s->sftp_handle); | ||
59 | } | ||
60 | @@ -XXX,XX +XXX,XX @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts, | ||
61 | int ssh_flags, int creat_mode, Error **errp) | ||
62 | { | ||
63 | int r, ret; | ||
64 | - const char *user; | ||
65 | long port = 0; | ||
66 | |||
67 | if (opts->has_user) { | ||
68 | - user = opts->user; | ||
69 | + s->user = g_strdup(opts->user); | ||
70 | } else { | ||
71 | - user = g_get_user_name(); | ||
72 | - if (!user) { | ||
73 | + s->user = g_strdup(g_get_user_name()); | ||
74 | + if (!s->user) { | ||
75 | error_setg_errno(errp, errno, "Can't get user name"); | ||
76 | ret = -errno; | ||
77 | goto err; | ||
78 | @@ -XXX,XX +XXX,XX @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts, | ||
79 | } | ||
80 | |||
81 | /* Authenticate. */ | ||
82 | - ret = authenticate(s, user, errp); | ||
83 | + ret = authenticate(s, s->user, errp); | ||
84 | if (ret < 0) { | ||
85 | goto err; | ||
86 | } | ||
87 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset, | ||
88 | return ssh_grow_file(s, offset, errp); | ||
89 | } | ||
90 | |||
91 | +static void ssh_refresh_filename(BlockDriverState *bs) | ||
92 | +{ | ||
93 | + BDRVSSHState *s = bs->opaque; | ||
94 | + const char *path, *host_key_check; | ||
95 | + int ret; | ||
96 | + | ||
97 | + /* | ||
98 | + * None of these options can be represented in a plain "host:port" | ||
99 | + * format, so if any was given, we have to abort. | ||
100 | + */ | ||
101 | + if (s->inet->has_ipv4 || s->inet->has_ipv6 || s->inet->has_to || | ||
102 | + s->inet->has_numeric) | ||
103 | + { | ||
104 | + return; | ||
105 | + } | ||
106 | + | ||
107 | + path = qdict_get_try_str(bs->full_open_options, "path"); | ||
108 | + assert(path); /* mandatory option */ | ||
109 | + | ||
110 | + host_key_check = qdict_get_try_str(bs->full_open_options, "host_key_check"); | ||
111 | + | ||
112 | + ret = snprintf(bs->exact_filename, sizeof(bs->exact_filename), | ||
113 | + "ssh://%s@%s:%s%s%s%s", | ||
114 | + s->user, s->inet->host, s->inet->port, path, | ||
115 | + host_key_check ? "?host_key_check=" : "", | ||
116 | + host_key_check ?: ""); | ||
117 | + if (ret >= sizeof(bs->exact_filename)) { | ||
118 | + /* An overflow makes the filename unusable, so do not report any */ | ||
119 | + bs->exact_filename[0] = '\0'; | ||
120 | + } | ||
121 | +} | ||
122 | + | ||
123 | static const char *const ssh_strong_runtime_opts[] = { | ||
124 | "host", | ||
125 | "port", | ||
126 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_ssh = { | ||
127 | .bdrv_getlength = ssh_getlength, | ||
128 | .bdrv_co_truncate = ssh_co_truncate, | ||
129 | .bdrv_co_flush_to_disk = ssh_co_flush, | ||
130 | + .bdrv_refresh_filename = ssh_refresh_filename, | ||
131 | .create_opts = &ssh_create_opts, | ||
132 | .strong_runtime_opts = ssh_strong_runtime_opts, | ||
133 | }; | ||
134 | diff --git a/tests/qemu-iotests/207 b/tests/qemu-iotests/207 | ||
135 | index XXXXXXX..XXXXXXX 100755 | ||
136 | --- a/tests/qemu-iotests/207 | ||
137 | +++ b/tests/qemu-iotests/207 | ||
138 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \ | ||
139 | 'size': 4194304 }) | ||
140 | vm.shutdown() | ||
141 | |||
142 | - iotests.img_info_log(remote_path, filter_path=disk_path) | ||
143 | + iotests.img_info_log(remote_path) | ||
144 | iotests.log("") | ||
145 | iotests.img_info_log(disk_path) | ||
146 | |||
147 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \ | ||
148 | 'size': 8388608 }) | ||
149 | vm.shutdown() | ||
150 | |||
151 | - iotests.img_info_log(remote_path, filter_path=disk_path) | ||
152 | + iotests.img_info_log(remote_path) | ||
153 | |||
154 | vm.launch() | ||
155 | blockdev_create(vm, { 'driver': 'ssh', | ||
156 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \ | ||
157 | 'size': 4194304 }) | ||
158 | vm.shutdown() | ||
159 | |||
160 | - iotests.img_info_log(remote_path, filter_path=disk_path) | ||
161 | + iotests.img_info_log(remote_path) | ||
162 | |||
163 | md5_key = subprocess.check_output( | ||
164 | 'ssh-keyscan -t rsa 127.0.0.1 2>/dev/null | grep -v "\\^#" | ' + | ||
165 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \ | ||
166 | 'size': 8388608 }) | ||
167 | vm.shutdown() | ||
168 | |||
169 | - iotests.img_info_log(remote_path, filter_path=disk_path) | ||
170 | + iotests.img_info_log(remote_path) | ||
171 | |||
172 | sha1_key = subprocess.check_output( | ||
173 | 'ssh-keyscan -t rsa 127.0.0.1 2>/dev/null | grep -v "\\^#" | ' + | ||
174 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('t.img') as disk_path, \ | ||
175 | 'size': 4194304 }) | ||
176 | vm.shutdown() | ||
177 | |||
178 | - iotests.img_info_log(remote_path, filter_path=disk_path) | ||
179 | + iotests.img_info_log(remote_path) | ||
180 | |||
181 | # | ||
182 | # Invalid path and user | ||
183 | diff --git a/tests/qemu-iotests/207.out b/tests/qemu-iotests/207.out | ||
184 | index XXXXXXX..XXXXXXX 100644 | ||
185 | --- a/tests/qemu-iotests/207.out | ||
186 | +++ b/tests/qemu-iotests/207.out | ||
187 | @@ -XXX,XX +XXX,XX @@ | ||
188 | {"execute": "job-dismiss", "arguments": {"id": "job0"}} | ||
189 | {"return": {}} | ||
190 | |||
191 | -image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}} | ||
192 | +image: TEST_IMG | ||
193 | file format: IMGFMT | ||
194 | virtual size: 4 MiB (4194304 bytes) | ||
195 | |||
196 | @@ -XXX,XX +XXX,XX @@ virtual size: 4 MiB (4194304 bytes) | ||
197 | {"execute": "job-dismiss", "arguments": {"id": "job0"}} | ||
198 | {"return": {}} | ||
199 | |||
200 | -image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}} | ||
201 | +image: TEST_IMG | ||
202 | file format: IMGFMT | ||
203 | virtual size: 8 MiB (8388608 bytes) | ||
204 | |||
205 | @@ -XXX,XX +XXX,XX @@ virtual size: 8 MiB (8388608 bytes) | ||
206 | {"execute": "job-dismiss", "arguments": {"id": "job0"}} | ||
207 | {"return": {}} | ||
208 | |||
209 | -image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}} | ||
210 | +image: TEST_IMG | ||
211 | file format: IMGFMT | ||
212 | virtual size: 4 MiB (4194304 bytes) | ||
213 | |||
214 | @@ -XXX,XX +XXX,XX @@ Job failed: remote host key does not match host_key_check 'wrong' | ||
215 | {"execute": "job-dismiss", "arguments": {"id": "job0"}} | ||
216 | {"return": {}} | ||
217 | |||
218 | -image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}} | ||
219 | +image: TEST_IMG | ||
220 | file format: IMGFMT | ||
221 | virtual size: 8 MiB (8388608 bytes) | ||
222 | |||
223 | @@ -XXX,XX +XXX,XX @@ Job failed: remote host key does not match host_key_check 'wrong' | ||
224 | {"execute": "job-dismiss", "arguments": {"id": "job0"}} | ||
225 | {"return": {}} | ||
226 | |||
227 | -image: json:{"driver": "IMGFMT", "file": {"server.host": "127.0.0.1", "server.port": "22", "driver": "ssh", "path": "TEST_IMG"}} | ||
228 | +image: TEST_IMG | ||
229 | file format: IMGFMT | ||
230 | virtual size: 4 MiB (4194304 bytes) | ||
231 | |||
232 | diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc | ||
233 | index XXXXXXX..XXXXXXX 100644 | ||
234 | --- a/tests/qemu-iotests/common.rc | ||
235 | +++ b/tests/qemu-iotests/common.rc | ||
236 | @@ -XXX,XX +XXX,XX @@ else | ||
237 | TEST_IMG="nbd:127.0.0.1:10810" | ||
238 | elif [ "$IMGPROTO" = "ssh" ]; then | ||
239 | TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT | ||
240 | - REMOTE_TEST_DIR="ssh://127.0.0.1$TEST_DIR" | ||
241 | + REMOTE_TEST_DIR="ssh://\\($USER@\\)\\?127.0.0.1\\(:[0-9]\\+\\)\\?$TEST_DIR" | ||
242 | TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE" | ||
243 | elif [ "$IMGPROTO" = "nfs" ]; then | ||
244 | TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT | ||
245 | diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py | ||
246 | index XXXXXXX..XXXXXXX 100644 | ||
247 | --- a/tests/qemu-iotests/iotests.py | ||
248 | +++ b/tests/qemu-iotests/iotests.py | ||
249 | @@ -XXX,XX +XXX,XX @@ def remote_filename(path): | ||
250 | if imgproto == 'file': | ||
251 | return path | ||
252 | elif imgproto == 'ssh': | ||
253 | - return "ssh://127.0.0.1%s" % (path) | ||
254 | + return "ssh://%s@127.0.0.1:22%s" % (os.environ.get('USER'), path) | ||
255 | else: | ||
256 | raise Exception("Protocol %s not supported" % (imgproto)) | ||
257 | |||
258 | -- | ||
259 | 2.20.1 | ||
260 | |||
261 | diff view generated by jsdifflib |