1
The following changes since commit 105b07f1ba462ec48b27e5cb74ddf81c6a79364c:
1
The following changes since commit 281f327487c9c9b1599f93c589a408bbf4a651b8:
2
2
3
Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20200127' into staging (2020-01-27 13:02:36 +0000)
3
Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.12-pull-request' into staging (2017-12-22 00:11:36 +0000)
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
git://repo.or.cz/qemu/kevin.git tags/for-upstream
8
8
9
for you to fetch changes up to 5fbf1d56c24018772e900a40a0955175ff82f35c:
9
for you to fetch changes up to 1a63a907507fbbcfaee3f622907ec244b7eabda8:
10
10
11
iscsi: Don't access non-existent scsi_lba_status_descriptor (2020-01-27 17:19:53 +0100)
11
block: Keep nodes drained between reopen_queue/multiple (2017-12-22 15:05:32 +0100)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Block layer patches:
14
Block layer patches
15
16
- iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)
17
- AioContext fixes in QMP commands for backup and bitmaps
18
- iotests fixes
19
15
20
----------------------------------------------------------------
16
----------------------------------------------------------------
21
Eiichi Tsukata (1):
17
Doug Gale (1):
22
block/backup: fix memory leak in bdrv_backup_top_append()
18
nvme: Add tracing
23
19
24
Felipe Franciosi (1):
20
Edgar Kaziakhmedov (1):
25
iscsi: Cap block count from GET LBA STATUS (CVE-2020-1711)
21
qcow2: get rid of qcow2_backing_read1 routine
26
22
27
Kevin Wolf (1):
23
Fam Zheng (2):
28
iscsi: Don't access non-existent scsi_lba_status_descriptor
24
block: Open backing image in force share mode for size probe
25
block: Remove unused bdrv_requests_pending
29
26
30
Max Reitz (1):
27
John Snow (1):
31
iotests.py: Let wait_migration wait even more
28
iotests: fix 197 for vpc
32
29
33
Sergio Lopez (8):
30
Kevin Wolf (27):
34
blockdev: fix coding style issues in drive_backup_prepare
31
block: Formats don't need CONSISTENT_READ with NO_IO
35
blockdev: unify qmp_drive_backup and drive-backup transaction paths
32
block: Make bdrv_drain_invoke() recursive
36
blockdev: unify qmp_blockdev_backup and blockdev-backup transaction paths
33
block: Call .drain_begin only once in bdrv_drain_all_begin()
37
blockdev: honor bdrv_try_set_aio_context() context requirements
34
test-bdrv-drain: Test BlockDriver callbacks for drain
38
block/backup-top: Don't acquire context while dropping top
35
block: bdrv_drain_recurse(): Remove unused begin parameter
39
blockdev: Acquire AioContext on dirty bitmap functions
36
block: Don't wait for requests in bdrv_drain*_end()
40
blockdev: Return bs to the proper context on snapshot abort
37
block: Unify order in drain functions
41
iotests: Test handling of AioContexts with some blockdev actions
38
block: Don't acquire AioContext in hmp_qemu_io()
39
block: Document that x-blockdev-change breaks quorum children list
40
block: Assert drain_all is only called from main AioContext
41
block: Make bdrv_drain() driver callbacks non-recursive
42
test-bdrv-drain: Test callback for bdrv_drain
43
test-bdrv-drain: Test bs->quiesce_counter
44
blockjob: Pause job on draining any job BDS
45
test-bdrv-drain: Test drain vs. block jobs
46
block: Don't block_job_pause_all() in bdrv_drain_all()
47
block: Nested drain_end must still call callbacks
48
test-bdrv-drain: Test nested drain sections
49
block: Don't notify parents in drain call chain
50
block: Add bdrv_subtree_drained_begin/end()
51
test-bdrv-drain: Tests for bdrv_subtree_drain
52
test-bdrv-drain: Test behaviour in coroutine context
53
test-bdrv-drain: Recursive draining with multiple parents
54
block: Allow graph changes in subtree drained section
55
test-bdrv-drain: Test graph changes in drained section
56
commit: Simplify reopen of base
57
block: Keep nodes drained between reopen_queue/multiple
42
58
43
Thomas Huth (1):
59
Thomas Huth (3):
44
iotests: Add more "skip_if_unsupported" statements to the python tests
60
block: Remove the obsolete -drive boot=on|off parameter
61
block: Remove the deprecated -hdachs option
62
block: Mention -drive cyls/heads/secs/trans/serial/addr in deprecation chapter
45
63
46
block/backup-top.c | 7 +-
64
qapi/block-core.json | 4 +
47
block/backup.c | 3 +
65
block/qcow2.h | 3 -
48
block/iscsi.c | 7 +-
66
include/block/block.h | 15 +-
49
blockdev.c | 393 +++++++++++++++++++++++-------------------
67
include/block/block_int.h | 6 +-
50
tests/qemu-iotests/iotests.py | 6 +-
68
block.c | 75 ++++-
51
tests/qemu-iotests/030 | 4 +-
69
block/commit.c | 8 +-
52
tests/qemu-iotests/040 | 2 +
70
block/io.c | 164 +++++++---
53
tests/qemu-iotests/041 | 39 +----
71
block/qcow2.c | 51 +--
54
tests/qemu-iotests/141.out | 2 +
72
block/replication.c | 6 +
55
tests/qemu-iotests/185.out | 2 +
73
blockdev.c | 11 -
56
tests/qemu-iotests/219 | 7 +-
74
blockjob.c | 22 +-
57
tests/qemu-iotests/219.out | 8 +
75
hmp.c | 6 -
58
tests/qemu-iotests/234 | 8 +-
76
hw/block/nvme.c | 349 +++++++++++++++++----
59
tests/qemu-iotests/245 | 2 +
77
qemu-io-cmds.c | 3 +
60
tests/qemu-iotests/262 | 4 +-
78
tests/test-bdrv-drain.c | 651 +++++++++++++++++++++++++++++++++++++++
61
tests/qemu-iotests/280 | 2 +-
79
vl.c | 86 +-----
62
tests/qemu-iotests/281 | 247 ++++++++++++++++++++++++++
80
hw/block/trace-events | 93 ++++++
63
tests/qemu-iotests/281.out | 5 +
81
qemu-doc.texi | 29 +-
64
tests/qemu-iotests/group | 1 +
82
qemu-options.hx | 19 +-
65
19 files changed, 510 insertions(+), 239 deletions(-)
83
tests/Makefile.include | 2 +
66
create mode 100755 tests/qemu-iotests/281
84
tests/qemu-iotests/197 | 4 +
67
create mode 100644 tests/qemu-iotests/281.out
85
tests/qemu-iotests/common.filter | 3 +-
86
22 files changed, 1294 insertions(+), 316 deletions(-)
87
create mode 100644 tests/test-bdrv-drain.c
68
88
69
diff view generated by jsdifflib
New patch
1
Commit 1f4ad7d fixed 'qemu-img info' for raw images that are currently
2
in use as a mirror target. It is not enough for image formats, though,
3
as these still unconditionally request BLK_PERM_CONSISTENT_READ.
1
4
5
As this permission is geared towards whether the guest-visible data is
6
consistent, and has no impact on whether the metadata is sane, and
7
'qemu-img info' does not read guest-visible data (except for the raw
8
format), it makes sense to not require BLK_PERM_CONSISTENT_READ if there
9
is not going to be any guest I/O performed, regardless of image format.
10
11
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
12
---
13
block.c | 6 +++++-
14
1 file changed, 5 insertions(+), 1 deletion(-)
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 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
21
assert(role == &child_backing || role == &child_file);
22
23
if (!backing) {
24
+ int flags = bdrv_reopen_get_flags(reopen_queue, bs);
25
+
26
/* Apart from the modifications below, the same permissions are
27
* forwarded and left alone as for filters */
28
bdrv_filter_default_perms(bs, c, role, reopen_queue, perm, shared,
29
@@ -XXX,XX +XXX,XX @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
30
31
/* bs->file always needs to be consistent because of the metadata. We
32
* can never allow other users to resize or write to it. */
33
- perm |= BLK_PERM_CONSISTENT_READ;
34
+ if (!(flags & BDRV_O_NO_IO)) {
35
+ perm |= BLK_PERM_CONSISTENT_READ;
36
+ }
37
shared &= ~(BLK_PERM_WRITE | BLK_PERM_RESIZE);
38
} else {
39
/* We want consistent read from backing files if the parent needs it.
40
--
41
2.13.6
42
43
diff view generated by jsdifflib
New patch
1
From: John Snow <jsnow@redhat.com>
1
2
3
VPC has some difficulty creating geometries of particular size.
4
However, we can indeed force it to use a literal one, so let's
5
do that for the sake of test 197, which is testing some specific
6
offsets.
7
8
Signed-off-by: John Snow <jsnow@redhat.com>
9
Reviewed-by: Eric Blake <eblake@redhat.com>
10
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
11
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
12
Reviewed-by: Lukáš Doktor <ldoktor@redhat.com>
13
---
14
tests/qemu-iotests/197 | 4 ++++
15
tests/qemu-iotests/common.filter | 3 ++-
16
2 files changed, 6 insertions(+), 1 deletion(-)
17
18
diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
19
index XXXXXXX..XXXXXXX 100755
20
--- a/tests/qemu-iotests/197
21
+++ b/tests/qemu-iotests/197
22
@@ -XXX,XX +XXX,XX @@ echo '=== Copy-on-read ==='
23
echo
24
25
# Prep the images
26
+# VPC rounds image sizes to a specific geometry, force a specific size.
27
+if [ "$IMGFMT" = "vpc" ]; then
28
+ IMGOPTS=$(_optstr_add "$IMGOPTS" "force_size")
29
+fi
30
_make_test_img 4G
31
$QEMU_IO -c "write -P 55 3G 1k" "$TEST_IMG" | _filter_qemu_io
32
IMGPROTO=file IMGFMT=qcow2 IMGOPTS= TEST_IMG_FILE="$TEST_WRAP" \
33
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
34
index XXXXXXX..XXXXXXX 100644
35
--- a/tests/qemu-iotests/common.filter
36
+++ b/tests/qemu-iotests/common.filter
37
@@ -XXX,XX +XXX,XX @@ _filter_img_create()
38
-e "s# log_size=[0-9]\\+##g" \
39
-e "s# refcount_bits=[0-9]\\+##g" \
40
-e "s# key-secret=[a-zA-Z0-9]\\+##g" \
41
- -e "s# iter-time=[0-9]\\+##g"
42
+ -e "s# iter-time=[0-9]\\+##g" \
43
+ -e "s# force_size=\\(on\\|off\\)##g"
44
}
45
46
_filter_img_info()
47
--
48
2.13.6
49
50
diff view generated by jsdifflib
New patch
1
This change separates bdrv_drain_invoke(), which calls the BlockDriver
2
drain callbacks, from bdrv_drain_recurse(). Instead, the function
3
performs its own recursion now.
1
4
5
One reason for this is that bdrv_drain_recurse() can be called multiple
6
times by bdrv_drain_all_begin(), but the callbacks may only be called
7
once. The separation is necessary to fix this bug.
8
9
The other reason is that we intend to go to a model where we call all
10
driver callbacks first, and only then start polling. This is not fully
11
achieved yet with this patch, as bdrv_drain_invoke() contains a
12
BDRV_POLL_WHILE() loop for the block driver callbacks, which can still
13
call callbacks for any unrelated event. It's a step in this direction
14
anyway.
15
16
Cc: qemu-stable@nongnu.org
17
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
18
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
19
---
20
block/io.c | 14 +++++++++++---
21
1 file changed, 11 insertions(+), 3 deletions(-)
22
23
diff --git a/block/io.c b/block/io.c
24
index XXXXXXX..XXXXXXX 100644
25
--- a/block/io.c
26
+++ b/block/io.c
27
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
28
bdrv_wakeup(bs);
29
}
30
31
+/* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */
32
static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
33
{
34
+ BdrvChild *child, *tmp;
35
BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
36
37
if (!bs->drv || (begin && !bs->drv->bdrv_co_drain_begin) ||
38
@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
39
data.co = qemu_coroutine_create(bdrv_drain_invoke_entry, &data);
40
bdrv_coroutine_enter(bs, data.co);
41
BDRV_POLL_WHILE(bs, !data.done);
42
+
43
+ QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
44
+ bdrv_drain_invoke(child->bs, begin);
45
+ }
46
}
47
48
static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
49
@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
50
BdrvChild *child, *tmp;
51
bool waited;
52
53
- /* Ensure any pending metadata writes are submitted to bs->file. */
54
- bdrv_drain_invoke(bs, begin);
55
-
56
/* Wait for drained requests to finish */
57
waited = BDRV_POLL_WHILE(bs, atomic_read(&bs->in_flight) > 0);
58
59
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
60
bdrv_parent_drained_begin(bs);
61
}
62
63
+ bdrv_drain_invoke(bs, true);
64
bdrv_drain_recurse(bs, true);
65
}
66
67
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
68
}
69
70
bdrv_parent_drained_end(bs);
71
+ bdrv_drain_invoke(bs, false);
72
bdrv_drain_recurse(bs, false);
73
aio_enable_external(bdrv_get_aio_context(bs));
74
}
75
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
76
aio_context_acquire(aio_context);
77
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
78
if (aio_context == bdrv_get_aio_context(bs)) {
79
+ /* FIXME Calling this multiple times is wrong */
80
+ bdrv_drain_invoke(bs, true);
81
waited |= bdrv_drain_recurse(bs, true);
82
}
83
}
84
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
85
aio_context_acquire(aio_context);
86
aio_enable_external(aio_context);
87
bdrv_parent_drained_end(bs);
88
+ bdrv_drain_invoke(bs, false);
89
bdrv_drain_recurse(bs, false);
90
aio_context_release(aio_context);
91
}
92
--
93
2.13.6
94
95
diff view generated by jsdifflib
New patch
1
bdrv_drain_all_begin() used to call the .bdrv_co_drain_begin() driver
2
callback inside its polling loop. This means that how many times it got
3
called for each node depended on long it had to poll the event loop.
1
4
5
This is obviously not right and results in nodes that stay drained even
6
after bdrv_drain_all_end(), which calls .bdrv_co_drain_begin() once per
7
node.
8
9
Fix bdrv_drain_all_begin() to call the callback only once, too.
10
11
Cc: qemu-stable@nongnu.org
12
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
13
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
14
---
15
block/io.c | 3 +--
16
1 file changed, 1 insertion(+), 2 deletions(-)
17
18
diff --git a/block/io.c b/block/io.c
19
index XXXXXXX..XXXXXXX 100644
20
--- a/block/io.c
21
+++ b/block/io.c
22
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
23
aio_context_acquire(aio_context);
24
bdrv_parent_drained_begin(bs);
25
aio_disable_external(aio_context);
26
+ bdrv_drain_invoke(bs, true);
27
aio_context_release(aio_context);
28
29
if (!g_slist_find(aio_ctxs, aio_context)) {
30
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
31
aio_context_acquire(aio_context);
32
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
33
if (aio_context == bdrv_get_aio_context(bs)) {
34
- /* FIXME Calling this multiple times is wrong */
35
- bdrv_drain_invoke(bs, true);
36
waited |= bdrv_drain_recurse(bs, true);
37
}
38
}
39
--
40
2.13.6
41
42
diff view generated by jsdifflib
New patch
1
This adds a test case that the BlockDriver callbacks for drain are
2
called in bdrv_drained_all_begin/end(), and that both of them are called
3
exactly once.
1
4
5
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
6
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
7
Reviewed-by: Eric Blake <eblake@redhat.com>
8
---
9
tests/test-bdrv-drain.c | 137 ++++++++++++++++++++++++++++++++++++++++++++++++
10
tests/Makefile.include | 2 +
11
2 files changed, 139 insertions(+)
12
create mode 100644 tests/test-bdrv-drain.c
13
14
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
15
new file mode 100644
16
index XXXXXXX..XXXXXXX
17
--- /dev/null
18
+++ b/tests/test-bdrv-drain.c
19
@@ -XXX,XX +XXX,XX @@
20
+/*
21
+ * Block node draining tests
22
+ *
23
+ * Copyright (c) 2017 Kevin Wolf <kwolf@redhat.com>
24
+ *
25
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
26
+ * of this software and associated documentation files (the "Software"), to deal
27
+ * in the Software without restriction, including without limitation the rights
28
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
29
+ * copies of the Software, and to permit persons to whom the Software is
30
+ * furnished to do so, subject to the following conditions:
31
+ *
32
+ * The above copyright notice and this permission notice shall be included in
33
+ * all copies or substantial portions of the Software.
34
+ *
35
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
36
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
37
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
38
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
39
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
40
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
41
+ * THE SOFTWARE.
42
+ */
43
+
44
+#include "qemu/osdep.h"
45
+#include "block/block.h"
46
+#include "sysemu/block-backend.h"
47
+#include "qapi/error.h"
48
+
49
+typedef struct BDRVTestState {
50
+ int drain_count;
51
+} BDRVTestState;
52
+
53
+static void coroutine_fn bdrv_test_co_drain_begin(BlockDriverState *bs)
54
+{
55
+ BDRVTestState *s = bs->opaque;
56
+ s->drain_count++;
57
+}
58
+
59
+static void coroutine_fn bdrv_test_co_drain_end(BlockDriverState *bs)
60
+{
61
+ BDRVTestState *s = bs->opaque;
62
+ s->drain_count--;
63
+}
64
+
65
+static void bdrv_test_close(BlockDriverState *bs)
66
+{
67
+ BDRVTestState *s = bs->opaque;
68
+ g_assert_cmpint(s->drain_count, >, 0);
69
+}
70
+
71
+static int coroutine_fn bdrv_test_co_preadv(BlockDriverState *bs,
72
+ uint64_t offset, uint64_t bytes,
73
+ QEMUIOVector *qiov, int flags)
74
+{
75
+ /* We want this request to stay until the polling loop in drain waits for
76
+ * it to complete. We need to sleep a while as bdrv_drain_invoke() comes
77
+ * first and polls its result, too, but it shouldn't accidentally complete
78
+ * this request yet. */
79
+ qemu_co_sleep_ns(QEMU_CLOCK_REALTIME, 100000);
80
+
81
+ return 0;
82
+}
83
+
84
+static BlockDriver bdrv_test = {
85
+ .format_name = "test",
86
+ .instance_size = sizeof(BDRVTestState),
87
+
88
+ .bdrv_close = bdrv_test_close,
89
+ .bdrv_co_preadv = bdrv_test_co_preadv,
90
+
91
+ .bdrv_co_drain_begin = bdrv_test_co_drain_begin,
92
+ .bdrv_co_drain_end = bdrv_test_co_drain_end,
93
+};
94
+
95
+static void aio_ret_cb(void *opaque, int ret)
96
+{
97
+ int *aio_ret = opaque;
98
+ *aio_ret = ret;
99
+}
100
+
101
+static void test_drv_cb_drain_all(void)
102
+{
103
+ BlockBackend *blk;
104
+ BlockDriverState *bs;
105
+ BDRVTestState *s;
106
+ BlockAIOCB *acb;
107
+ int aio_ret;
108
+
109
+ QEMUIOVector qiov;
110
+ struct iovec iov = {
111
+ .iov_base = NULL,
112
+ .iov_len = 0,
113
+ };
114
+ qemu_iovec_init_external(&qiov, &iov, 1);
115
+
116
+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
117
+ bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
118
+ &error_abort);
119
+ s = bs->opaque;
120
+ blk_insert_bs(blk, bs, &error_abort);
121
+
122
+ /* Simple bdrv_drain_all_begin/end pair, check that CBs are called */
123
+ g_assert_cmpint(s->drain_count, ==, 0);
124
+ bdrv_drain_all_begin();
125
+ g_assert_cmpint(s->drain_count, ==, 1);
126
+ bdrv_drain_all_end();
127
+ g_assert_cmpint(s->drain_count, ==, 0);
128
+
129
+ /* Now do the same while a request is pending */
130
+ aio_ret = -EINPROGRESS;
131
+ acb = blk_aio_preadv(blk, 0, &qiov, 0, aio_ret_cb, &aio_ret);
132
+ g_assert(acb != NULL);
133
+ g_assert_cmpint(aio_ret, ==, -EINPROGRESS);
134
+
135
+ g_assert_cmpint(s->drain_count, ==, 0);
136
+ bdrv_drain_all_begin();
137
+ g_assert_cmpint(aio_ret, ==, 0);
138
+ g_assert_cmpint(s->drain_count, ==, 1);
139
+ bdrv_drain_all_end();
140
+ g_assert_cmpint(s->drain_count, ==, 0);
141
+
142
+ bdrv_unref(bs);
143
+ blk_unref(blk);
144
+}
145
+
146
+int main(int argc, char **argv)
147
+{
148
+ bdrv_init();
149
+ qemu_init_main_loop(&error_abort);
150
+
151
+ g_test_init(&argc, &argv, NULL);
152
+
153
+ g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_all);
154
+
155
+ return g_test_run();
156
+}
157
diff --git a/tests/Makefile.include b/tests/Makefile.include
158
index XXXXXXX..XXXXXXX 100644
159
--- a/tests/Makefile.include
160
+++ b/tests/Makefile.include
161
@@ -XXX,XX +XXX,XX @@ gcov-files-test-thread-pool-y = thread-pool.c
162
gcov-files-test-hbitmap-y = util/hbitmap.c
163
check-unit-y += tests/test-hbitmap$(EXESUF)
164
gcov-files-test-hbitmap-y = blockjob.c
165
+check-unit-y += tests/test-bdrv-drain$(EXESUF)
166
check-unit-y += tests/test-blockjob$(EXESUF)
167
check-unit-y += tests/test-blockjob-txn$(EXESUF)
168
check-unit-y += tests/test-x86-cpuid$(EXESUF)
169
@@ -XXX,XX +XXX,XX @@ tests/test-coroutine$(EXESUF): tests/test-coroutine.o $(test-block-obj-y)
170
tests/test-aio$(EXESUF): tests/test-aio.o $(test-block-obj-y)
171
tests/test-aio-multithread$(EXESUF): tests/test-aio-multithread.o $(test-block-obj-y)
172
tests/test-throttle$(EXESUF): tests/test-throttle.o $(test-block-obj-y)
173
+tests/test-bdrv-drain$(EXESUF): tests/test-bdrv-drain.o $(test-block-obj-y) $(test-util-obj-y)
174
tests/test-blockjob$(EXESUF): tests/test-blockjob.o $(test-block-obj-y) $(test-util-obj-y)
175
tests/test-blockjob-txn$(EXESUF): tests/test-blockjob-txn.o $(test-block-obj-y) $(test-util-obj-y)
176
tests/test-thread-pool$(EXESUF): tests/test-thread-pool.o $(test-block-obj-y)
177
--
178
2.13.6
179
180
diff view generated by jsdifflib
New patch
1
Now that the bdrv_drain_invoke() calls are pulled up to the callers of
2
bdrv_drain_recurse(), the 'begin' parameter isn't needed any more.
1
3
4
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
5
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
6
---
7
block/io.c | 12 ++++++------
8
1 file changed, 6 insertions(+), 6 deletions(-)
9
10
diff --git a/block/io.c b/block/io.c
11
index XXXXXXX..XXXXXXX 100644
12
--- a/block/io.c
13
+++ b/block/io.c
14
@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
15
}
16
}
17
18
-static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
19
+static bool bdrv_drain_recurse(BlockDriverState *bs)
20
{
21
BdrvChild *child, *tmp;
22
bool waited;
23
@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs, bool begin)
24
*/
25
bdrv_ref(bs);
26
}
27
- waited |= bdrv_drain_recurse(bs, begin);
28
+ waited |= bdrv_drain_recurse(bs);
29
if (in_main_loop) {
30
bdrv_unref(bs);
31
}
32
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
33
}
34
35
bdrv_drain_invoke(bs, true);
36
- bdrv_drain_recurse(bs, true);
37
+ bdrv_drain_recurse(bs);
38
}
39
40
void bdrv_drained_end(BlockDriverState *bs)
41
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
42
43
bdrv_parent_drained_end(bs);
44
bdrv_drain_invoke(bs, false);
45
- bdrv_drain_recurse(bs, false);
46
+ bdrv_drain_recurse(bs);
47
aio_enable_external(bdrv_get_aio_context(bs));
48
}
49
50
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
51
aio_context_acquire(aio_context);
52
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
53
if (aio_context == bdrv_get_aio_context(bs)) {
54
- waited |= bdrv_drain_recurse(bs, true);
55
+ waited |= bdrv_drain_recurse(bs);
56
}
57
}
58
aio_context_release(aio_context);
59
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
60
aio_enable_external(aio_context);
61
bdrv_parent_drained_end(bs);
62
bdrv_drain_invoke(bs, false);
63
- bdrv_drain_recurse(bs, false);
64
+ bdrv_drain_recurse(bs);
65
aio_context_release(aio_context);
66
}
67
68
--
69
2.13.6
70
71
diff view generated by jsdifflib
New patch
1
The device is drained, so there is no point in waiting for requests at
2
the end of the drained section. Remove the bdrv_drain_recurse() calls
3
there.
1
4
5
The bdrv_drain_recurse() calls were introduced in commit 481cad48e5e
6
in order to call the .bdrv_co_drain_end() driver callback. This is now
7
done by a separate bdrv_drain_invoke() call.
8
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
11
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
12
---
13
block/io.c | 2 --
14
1 file changed, 2 deletions(-)
15
16
diff --git a/block/io.c b/block/io.c
17
index XXXXXXX..XXXXXXX 100644
18
--- a/block/io.c
19
+++ b/block/io.c
20
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
21
22
bdrv_parent_drained_end(bs);
23
bdrv_drain_invoke(bs, false);
24
- bdrv_drain_recurse(bs);
25
aio_enable_external(bdrv_get_aio_context(bs));
26
}
27
28
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
29
aio_enable_external(aio_context);
30
bdrv_parent_drained_end(bs);
31
bdrv_drain_invoke(bs, false);
32
- bdrv_drain_recurse(bs);
33
aio_context_release(aio_context);
34
}
35
36
--
37
2.13.6
38
39
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
Drain requests are propagated to child nodes, parent nodes and directly
2
to the AioContext. The order in which this happened was different
3
between all combinations of drain/drain_all and begin/end.
2
4
3
bdrv_try_set_aio_context() requires that the old context is held, and
5
The correct order is to keep children only drained when their parents
4
the new context is not held. Fix all the occurrences where it's not
6
are also drained. This means that at the start of a drained section, the
5
done this way.
7
AioContext needs to be drained first, the parents second and only then
8
the children. The correct order for the end of a drained section is the
9
opposite.
6
10
7
Suggested-by: Max Reitz <mreitz@redhat.com>
11
This patch changes the three other functions to follow the example of
8
Signed-off-by: Sergio Lopez <slp@redhat.com>
12
bdrv_drained_begin(), which is the only one that got it right.
13
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
14
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
15
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
10
---
16
---
11
blockdev.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++-------
17
block/io.c | 12 ++++++++----
12
1 file changed, 60 insertions(+), 8 deletions(-)
18
1 file changed, 8 insertions(+), 4 deletions(-)
13
19
14
diff --git a/blockdev.c b/blockdev.c
20
diff --git a/block/io.c b/block/io.c
15
index XXXXXXX..XXXXXXX 100644
21
index XXXXXXX..XXXXXXX 100644
16
--- a/blockdev.c
22
--- a/block/io.c
17
+++ b/blockdev.c
23
+++ b/block/io.c
18
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
24
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
19
DO_UPCAST(ExternalSnapshotState, common, common);
20
TransactionAction *action = common->action;
21
AioContext *aio_context;
22
+ AioContext *old_context;
23
int ret;
24
25
/* 'blockdev-snapshot' and 'blockdev-snapshot-sync' have similar
26
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_prepare(BlkActionState *common,
27
goto out;
28
}
29
30
+ /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
31
+ old_context = bdrv_get_aio_context(state->new_bs);
32
+ aio_context_release(aio_context);
33
+ aio_context_acquire(old_context);
34
+
35
ret = bdrv_try_set_aio_context(state->new_bs, aio_context, errp);
36
+
37
+ aio_context_release(old_context);
38
+ aio_context_acquire(aio_context);
39
+
40
if (ret < 0) {
41
goto out;
42
}
43
@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
44
BlockDriverState *target_bs;
45
BlockDriverState *source = NULL;
46
AioContext *aio_context;
47
+ AioContext *old_context;
48
QDict *options;
49
Error *local_err = NULL;
50
int flags;
51
int64_t size;
52
bool set_backing_hd = false;
53
+ int ret;
54
55
assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
56
backup = common->action->u.drive_backup.data;
57
@@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp)
58
goto out;
59
}
60
61
+ /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
62
+ old_context = bdrv_get_aio_context(target_bs);
63
+ aio_context_release(aio_context);
64
+ aio_context_acquire(old_context);
65
+
66
+ ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
67
+ if (ret < 0) {
68
+ bdrv_unref(target_bs);
69
+ aio_context_release(old_context);
70
+ return;
71
+ }
72
+
73
+ aio_context_release(old_context);
74
+ aio_context_acquire(aio_context);
75
+
76
if (set_backing_hd) {
77
bdrv_set_backing_hd(target_bs, source, &local_err);
78
if (local_err) {
79
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
80
BlockDriverState *bs;
81
BlockDriverState *target_bs;
82
AioContext *aio_context;
83
+ AioContext *old_context;
84
+ int ret;
85
86
assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
87
backup = common->action->u.blockdev_backup.data;
88
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
89
return;
25
return;
90
}
26
}
91
27
92
+ /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
28
+ /* Stop things in parent-to-child order */
93
aio_context = bdrv_get_aio_context(bs);
29
if (atomic_fetch_inc(&bs->quiesce_counter) == 0) {
94
+ old_context = bdrv_get_aio_context(target_bs);
30
aio_disable_external(bdrv_get_aio_context(bs));
95
+ aio_context_acquire(old_context);
31
bdrv_parent_drained_begin(bs);
96
+
32
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
97
+ ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
33
return;
98
+ if (ret < 0) {
99
+ aio_context_release(old_context);
100
+ return;
101
+ }
102
+
103
+ aio_context_release(old_context);
104
aio_context_acquire(aio_context);
105
state->bs = bs;
106
107
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_backup_common(BackupCommon *backup,
108
BlockJob *job = NULL;
109
BdrvDirtyBitmap *bmap = NULL;
110
int job_flags = JOB_DEFAULT;
111
- int ret;
112
113
if (!backup->has_speed) {
114
backup->speed = 0;
115
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_backup_common(BackupCommon *backup,
116
backup->compress = false;
117
}
34
}
118
35
119
- ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
36
- bdrv_parent_drained_end(bs);
120
- if (ret < 0) {
37
+ /* Re-enable things in child-to-parent order */
121
- return NULL;
38
bdrv_drain_invoke(bs, false);
122
- }
39
+ bdrv_parent_drained_end(bs);
123
-
40
aio_enable_external(bdrv_get_aio_context(bs));
124
if ((backup->sync == MIRROR_SYNC_MODE_BITMAP) ||
41
}
125
(backup->sync == MIRROR_SYNC_MODE_INCREMENTAL)) {
42
126
/* done before desugaring 'incremental' to print the right message */
43
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
127
@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
44
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
128
BlockDriverState *bs;
45
AioContext *aio_context = bdrv_get_aio_context(bs);
129
BlockDriverState *source, *target_bs;
46
130
AioContext *aio_context;
47
+ /* Stop things in parent-to-child order */
131
+ AioContext *old_context;
48
aio_context_acquire(aio_context);
132
BlockMirrorBackingMode backing_mode;
49
- bdrv_parent_drained_begin(bs);
133
Error *local_err = NULL;
50
aio_disable_external(aio_context);
134
QDict *options = NULL;
51
+ bdrv_parent_drained_begin(bs);
135
@@ -XXX,XX +XXX,XX @@ void qmp_drive_mirror(DriveMirror *arg, Error **errp)
52
bdrv_drain_invoke(bs, true);
136
(arg->mode == NEW_IMAGE_MODE_EXISTING ||
53
aio_context_release(aio_context);
137
!bdrv_has_zero_init(target_bs)));
54
138
55
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
139
+
56
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
140
+ /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
57
AioContext *aio_context = bdrv_get_aio_context(bs);
141
+ old_context = bdrv_get_aio_context(target_bs);
58
142
+ aio_context_release(aio_context);
59
+ /* Re-enable things in child-to-parent order */
143
+ aio_context_acquire(old_context);
60
aio_context_acquire(aio_context);
144
+
61
- aio_enable_external(aio_context);
145
ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
62
- bdrv_parent_drained_end(bs);
146
if (ret < 0) {
63
bdrv_drain_invoke(bs, false);
147
bdrv_unref(target_bs);
64
+ bdrv_parent_drained_end(bs);
148
- goto out;
65
+ aio_enable_external(aio_context);
149
+ aio_context_release(old_context);
66
aio_context_release(aio_context);
150
+ return;
151
}
67
}
152
68
153
+ aio_context_release(old_context);
154
+ aio_context_acquire(aio_context);
155
+
156
blockdev_mirror_common(arg->has_job_id ? arg->job_id : NULL, bs, target_bs,
157
arg->has_replaces, arg->replaces, arg->sync,
158
backing_mode, zero_target,
159
@@ -XXX,XX +XXX,XX @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id,
160
BlockDriverState *bs;
161
BlockDriverState *target_bs;
162
AioContext *aio_context;
163
+ AioContext *old_context;
164
BlockMirrorBackingMode backing_mode = MIRROR_LEAVE_BACKING_CHAIN;
165
Error *local_err = NULL;
166
bool zero_target;
167
@@ -XXX,XX +XXX,XX @@ void qmp_blockdev_mirror(bool has_job_id, const char *job_id,
168
169
zero_target = (sync == MIRROR_SYNC_MODE_FULL);
170
171
+ /* Honor bdrv_try_set_aio_context() context acquisition requirements. */
172
+ old_context = bdrv_get_aio_context(target_bs);
173
aio_context = bdrv_get_aio_context(bs);
174
- aio_context_acquire(aio_context);
175
+ aio_context_acquire(old_context);
176
177
ret = bdrv_try_set_aio_context(target_bs, aio_context, errp);
178
+
179
+ aio_context_release(old_context);
180
+ aio_context_acquire(aio_context);
181
+
182
if (ret < 0) {
183
goto out;
184
}
185
--
69
--
186
2.20.1
70
2.13.6
187
71
188
72
diff view generated by jsdifflib
New patch
1
Commit 15afd94a047 added code to acquire and release the AioContext in
2
qemuio_command(). This means that the lock is taken twice now in the
3
call path from hmp_qemu_io(). This causes BDRV_POLL_WHILE() to hang for
4
any requests issued to nodes in a non-mainloop AioContext.
1
5
6
Dropping the first locking from hmp_qemu_io() fixes the problem.
7
8
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
10
---
11
hmp.c | 6 ------
12
1 file changed, 6 deletions(-)
13
14
diff --git a/hmp.c b/hmp.c
15
index XXXXXXX..XXXXXXX 100644
16
--- a/hmp.c
17
+++ b/hmp.c
18
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
19
{
20
BlockBackend *blk;
21
BlockBackend *local_blk = NULL;
22
- AioContext *aio_context;
23
const char* device = qdict_get_str(qdict, "device");
24
const char* command = qdict_get_str(qdict, "command");
25
Error *err = NULL;
26
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
27
}
28
}
29
30
- aio_context = blk_get_aio_context(blk);
31
- aio_context_acquire(aio_context);
32
-
33
/*
34
* Notably absent: Proper permission management. This is sad, but it seems
35
* almost impossible to achieve without changing the semantics and thereby
36
@@ -XXX,XX +XXX,XX @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
37
*/
38
qemuio_command(blk, command);
39
40
- aio_context_release(aio_context);
41
-
42
fail:
43
blk_unref(local_blk);
44
hmp_handle_error(mon, &err);
45
--
46
2.13.6
47
48
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
From: Edgar Kaziakhmedov <edgar.kaziakhmedov@virtuozzo.com>
2
2
3
Issuing a blockdev-backup from qmp_blockdev_backup takes a slightly
3
Since bdrv_co_preadv does all neccessary checks including
4
different path than when it's issued from a transaction. In the code,
4
reading after the end of the backing file, avoid duplication
5
this is manifested as some redundancy between do_blockdev_backup() and
5
of verification before bdrv_co_preadv call.
6
blockdev_backup_prepare().
7
6
8
This change unifies both paths, merging do_blockdev_backup() and
7
Signed-off-by: Edgar Kaziakhmedov <edgar.kaziakhmedov@virtuozzo.com>
9
blockdev_backup_prepare(), and changing qmp_blockdev_backup() to
8
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
10
create a transaction instead of calling do_backup_common() direcly.
9
Reviewed-by: Eric Blake <eblake@redhat.com>
11
12
As a side-effect, now qmp_blockdev_backup() is executed inside a
13
drained section, as it happens when creating a blockdev-backup
14
transaction. This change is visible from the user's perspective, as
15
the job gets paused and immediately resumed before starting the actual
16
work.
17
18
Signed-off-by: Sergio Lopez <slp@redhat.com>
19
Reviewed-by: Max Reitz <mreitz@redhat.com>
20
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
21
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
22
---
11
---
23
blockdev.c | 60 ++++++++++++------------------------------------------
12
block/qcow2.h | 3 ---
24
1 file changed, 13 insertions(+), 47 deletions(-)
13
block/qcow2.c | 51 ++++++++-------------------------------------------
14
2 files changed, 8 insertions(+), 46 deletions(-)
25
15
26
diff --git a/blockdev.c b/blockdev.c
16
diff --git a/block/qcow2.h b/block/qcow2.h
27
index XXXXXXX..XXXXXXX 100644
17
index XXXXXXX..XXXXXXX 100644
28
--- a/blockdev.c
18
--- a/block/qcow2.h
29
+++ b/blockdev.c
19
+++ b/block/qcow2.h
30
@@ -XXX,XX +XXX,XX @@ typedef struct BlockdevBackupState {
20
@@ -XXX,XX +XXX,XX @@ uint32_t offset_to_reftable_index(BDRVQcow2State *s, uint64_t offset)
31
BlockJob *job;
21
}
32
} BlockdevBackupState;
22
33
23
/* qcow2.c functions */
34
-static BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
24
-int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
35
- Error **errp);
25
- int64_t sector_num, int nb_sectors);
36
-
26
-
37
static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
27
int64_t qcow2_refcount_metadata_size(int64_t clusters, size_t cluster_size,
38
{
28
int refcount_order, bool generous_increase,
39
BlockdevBackupState *state = DO_UPCAST(BlockdevBackupState, common, common);
29
uint64_t *refblock_count);
40
BlockdevBackup *backup;
30
diff --git a/block/qcow2.c b/block/qcow2.c
41
- BlockDriverState *bs, *target;
31
index XXXXXXX..XXXXXXX 100644
42
+ BlockDriverState *bs;
32
--- a/block/qcow2.c
43
+ BlockDriverState *target_bs;
33
+++ b/block/qcow2.c
44
AioContext *aio_context;
34
@@ -XXX,XX +XXX,XX @@ static int64_t coroutine_fn qcow2_co_get_block_status(BlockDriverState *bs,
45
- Error *local_err = NULL;
35
return status;
46
47
assert(common->action->type == TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP);
48
backup = common->action->u.blockdev_backup.data;
49
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
50
return;
51
}
52
53
- target = bdrv_lookup_bs(backup->target, backup->target, errp);
54
- if (!target) {
55
+ target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
56
+ if (!target_bs) {
57
return;
58
}
59
60
@@ -XXX,XX +XXX,XX @@ static void blockdev_backup_prepare(BlkActionState *common, Error **errp)
61
/* Paired with .clean() */
62
bdrv_drained_begin(state->bs);
63
64
- state->job = do_blockdev_backup(backup, common->block_job_txn, &local_err);
65
- if (local_err) {
66
- error_propagate(errp, local_err);
67
- goto out;
68
- }
69
+ state->job = do_backup_common(qapi_BlockdevBackup_base(backup),
70
+ bs, target_bs, aio_context,
71
+ common->block_job_txn, errp);
72
73
-out:
74
aio_context_release(aio_context);
75
}
36
}
76
37
77
@@ -XXX,XX +XXX,XX @@ XDbgBlockGraph *qmp_x_debug_query_block_graph(Error **errp)
38
-/* handle reading after the end of the backing file */
78
return bdrv_get_xdbg_block_graph(errp);
39
-int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
79
}
40
- int64_t offset, int bytes)
80
41
-{
81
-BlockJob *do_blockdev_backup(BlockdevBackup *backup, JobTxn *txn,
42
- uint64_t bs_size = bs->total_sectors * BDRV_SECTOR_SIZE;
82
- Error **errp)
43
- int n1;
83
+void qmp_blockdev_backup(BlockdevBackup *backup, Error **errp)
84
{
85
- BlockDriverState *bs;
86
- BlockDriverState *target_bs;
87
- AioContext *aio_context;
88
- BlockJob *job;
89
-
44
-
90
- bs = bdrv_lookup_bs(backup->device, backup->device, errp);
45
- if ((offset + bytes) <= bs_size) {
91
- if (!bs) {
46
- return bytes;
92
- return NULL;
93
- }
47
- }
94
-
48
-
95
- target_bs = bdrv_lookup_bs(backup->target, backup->target, errp);
49
- if (offset >= bs_size) {
96
- if (!target_bs) {
50
- n1 = 0;
97
- return NULL;
51
- } else {
52
- n1 = bs_size - offset;
98
- }
53
- }
99
-
54
-
100
- aio_context = bdrv_get_aio_context(bs);
55
- qemu_iovec_memset(qiov, n1, 0, bytes - n1);
101
- aio_context_acquire(aio_context);
102
-
56
-
103
- job = do_backup_common(qapi_BlockdevBackup_base(backup),
57
- return n1;
104
- bs, target_bs, aio_context, txn, errp);
105
-
106
- aio_context_release(aio_context);
107
- return job;
108
-}
58
-}
109
-
59
-
110
-void qmp_blockdev_backup(BlockdevBackup *arg, Error **errp)
60
static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
111
-{
61
uint64_t bytes, QEMUIOVector *qiov,
112
- BlockJob *job;
62
int flags)
113
- job = do_blockdev_backup(arg, NULL, errp);
63
{
114
- if (job) {
64
BDRVQcow2State *s = bs->opaque;
115
- job_start(&job->job);
65
- int offset_in_cluster, n1;
116
- }
66
+ int offset_in_cluster;
117
+ TransactionAction action = {
67
int ret;
118
+ .type = TRANSACTION_ACTION_KIND_BLOCKDEV_BACKUP,
68
unsigned int cur_bytes; /* number of bytes in current iteration */
119
+ .u.blockdev_backup.data = backup,
69
uint64_t cluster_offset = 0;
120
+ };
70
@@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset,
121
+ blockdev_do_action(&action, errp);
71
case QCOW2_CLUSTER_UNALLOCATED:
122
}
72
123
73
if (bs->backing) {
124
/* Parameter check and block job starting for drive mirroring.
74
- /* read from the base image */
75
- n1 = qcow2_backing_read1(bs->backing->bs, &hd_qiov,
76
- offset, cur_bytes);
77
- if (n1 > 0) {
78
- QEMUIOVector local_qiov;
79
-
80
- qemu_iovec_init(&local_qiov, hd_qiov.niov);
81
- qemu_iovec_concat(&local_qiov, &hd_qiov, 0, n1);
82
-
83
- BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
84
- qemu_co_mutex_unlock(&s->lock);
85
- ret = bdrv_co_preadv(bs->backing, offset, n1,
86
- &local_qiov, 0);
87
- qemu_co_mutex_lock(&s->lock);
88
-
89
- qemu_iovec_destroy(&local_qiov);
90
-
91
- if (ret < 0) {
92
- goto fail;
93
- }
94
+ BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
95
+ qemu_co_mutex_unlock(&s->lock);
96
+ ret = bdrv_co_preadv(bs->backing, offset, cur_bytes,
97
+ &hd_qiov, 0);
98
+ qemu_co_mutex_lock(&s->lock);
99
+ if (ret < 0) {
100
+ goto fail;
101
}
102
} else {
103
/* Note: in this case, no need to wait */
125
--
104
--
126
2.20.1
105
2.13.6
127
106
128
107
diff view generated by jsdifflib
New patch
1
Removing a quorum child node with x-blockdev-change results in a quorum
2
driver state that cannot be recreated with create options because it
3
would require a list with gaps. This causes trouble in at least
4
.bdrv_refresh_filename().
1
5
6
Document this problem so that we won't accidentally mark the command
7
stable without having addressed it.
8
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
Reviewed-by: Alberto Garcia <berto@igalia.com>
11
---
12
qapi/block-core.json | 4 ++++
13
1 file changed, 4 insertions(+)
14
15
diff --git a/qapi/block-core.json b/qapi/block-core.json
16
index XXXXXXX..XXXXXXX 100644
17
--- a/qapi/block-core.json
18
+++ b/qapi/block-core.json
19
@@ -XXX,XX +XXX,XX @@
20
# does not support all kinds of operations, all kinds of children, nor
21
# all block drivers.
22
#
23
+# FIXME Removing children from a quorum node means introducing gaps in the
24
+# child indices. This cannot be represented in the 'children' list of
25
+# BlockdevOptionsQuorum, as returned by .bdrv_refresh_filename().
26
+#
27
# Warning: The data in a new quorum child MUST be consistent with that of
28
# the rest of the array.
29
#
30
--
31
2.13.6
32
33
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
From: Doug Gale <doug16k@gmail.com>
2
2
3
Dirty map addition and removal functions are not acquiring to BDS
3
Add trace output for commands, errors, and undefined behavior.
4
AioContext, while they may call to code that expects it to be
4
Add guest error log output for undefined behavior.
5
acquired.
5
Report invalid undefined accesses to MMIO.
6
Annotate unlikely error checks with unlikely.
6
7
7
This may trigger a crash with a stack trace like this one:
8
Signed-off-by: Doug Gale <doug16k@gmail.com>
8
9
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
9
#0 0x00007f0ef146370f in __GI_raise (sig=sig@entry=6)
10
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
10
at ../sysdeps/unix/sysv/linux/raise.c:50
11
#1 0x00007f0ef144db25 in __GI_abort () at abort.c:79
12
#2 0x0000565022294dce in error_exit
13
(err=<optimized out>, msg=msg@entry=0x56502243a730 <__func__.16350> "qemu_mutex_unlock_impl") at util/qemu-thread-posix.c:36
14
#3 0x00005650222950ba in qemu_mutex_unlock_impl
15
(mutex=mutex@entry=0x5650244b0240, file=file@entry=0x565022439adf "util/async.c", line=line@entry=526) at util/qemu-thread-posix.c:108
16
#4 0x0000565022290029 in aio_context_release
17
(ctx=ctx@entry=0x5650244b01e0) at util/async.c:526
18
#5 0x000056502221cd08 in bdrv_can_store_new_dirty_bitmap
19
(bs=bs@entry=0x5650244dc820, name=name@entry=0x56502481d360 "bitmap1", granularity=granularity@entry=65536, errp=errp@entry=0x7fff22831718)
20
at block/dirty-bitmap.c:542
21
#6 0x000056502206ae53 in qmp_block_dirty_bitmap_add
22
(errp=0x7fff22831718, disabled=false, has_disabled=<optimized out>, persistent=<optimized out>, has_persistent=true, granularity=65536, has_granularity=<optimized out>, name=0x56502481d360 "bitmap1", node=<optimized out>) at blockdev.c:2894
23
#7 0x000056502206ae53 in qmp_block_dirty_bitmap_add
24
(node=<optimized out>, name=0x56502481d360 "bitmap1", has_granularity=<optimized out>, granularity=<optimized out>, has_persistent=true, persistent=<optimized out>, has_disabled=false, disabled=false, errp=0x7fff22831718) at blockdev.c:2856
25
#8 0x00005650221847a3 in qmp_marshal_block_dirty_bitmap_add
26
(args=<optimized out>, ret=<optimized out>, errp=0x7fff22831798)
27
at qapi/qapi-commands-block-core.c:651
28
#9 0x0000565022247e6c in do_qmp_dispatch
29
(errp=0x7fff22831790, allow_oob=<optimized out>, request=<optimized out>, cmds=0x565022b32d60 <qmp_commands>) at qapi/qmp-dispatch.c:132
30
#10 0x0000565022247e6c in qmp_dispatch
31
(cmds=0x565022b32d60 <qmp_commands>, request=<optimized out>, allow_oob=<optimized out>) at qapi/qmp-dispatch.c:175
32
#11 0x0000565022166061 in monitor_qmp_dispatch
33
(mon=0x56502450faa0, req=<optimized out>) at monitor/qmp.c:145
34
#12 0x00005650221666fa in monitor_qmp_bh_dispatcher
35
(data=<optimized out>) at monitor/qmp.c:234
36
#13 0x000056502228f866 in aio_bh_call (bh=0x56502440eae0)
37
at util/async.c:117
38
#14 0x000056502228f866 in aio_bh_poll (ctx=ctx@entry=0x56502440d7a0)
39
at util/async.c:117
40
#15 0x0000565022292c54 in aio_dispatch (ctx=0x56502440d7a0)
41
at util/aio-posix.c:459
42
#16 0x000056502228f742 in aio_ctx_dispatch
43
(source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:260
44
#17 0x00007f0ef5ce667d in g_main_dispatch (context=0x56502449aa40)
45
at gmain.c:3176
46
#18 0x00007f0ef5ce667d in g_main_context_dispatch
47
(context=context@entry=0x56502449aa40) at gmain.c:3829
48
#19 0x0000565022291d08 in glib_pollfds_poll () at util/main-loop.c:219
49
#20 0x0000565022291d08 in os_host_main_loop_wait
50
(timeout=<optimized out>) at util/main-loop.c:242
51
#21 0x0000565022291d08 in main_loop_wait (nonblocking=<optimized out>)
52
at util/main-loop.c:518
53
#22 0x00005650220743c1 in main_loop () at vl.c:1828
54
#23 0x0000565021f20a72 in main
55
(argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
56
at vl.c:4504
57
58
Fix this by acquiring the AioContext at qmp_block_dirty_bitmap_add()
59
and qmp_block_dirty_bitmap_add().
60
61
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1782175
62
Signed-off-by: Sergio Lopez <slp@redhat.com>
63
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
11
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
64
---
12
---
65
blockdev.c | 22 ++++++++++++++++++----
13
hw/block/nvme.c | 349 ++++++++++++++++++++++++++++++++++++++++++--------
66
1 file changed, 18 insertions(+), 4 deletions(-)
14
hw/block/trace-events | 93 ++++++++++++++
15
2 files changed, 390 insertions(+), 52 deletions(-)
67
16
68
diff --git a/blockdev.c b/blockdev.c
17
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
69
index XXXXXXX..XXXXXXX 100644
18
index XXXXXXX..XXXXXXX 100644
70
--- a/blockdev.c
19
--- a/hw/block/nvme.c
71
+++ b/blockdev.c
20
+++ b/hw/block/nvme.c
72
@@ -XXX,XX +XXX,XX @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
21
@@ -XXX,XX +XXX,XX @@
22
#include "qapi/visitor.h"
23
#include "sysemu/block-backend.h"
24
25
+#include "qemu/log.h"
26
+#include "trace.h"
27
#include "nvme.h"
28
29
+#define NVME_GUEST_ERR(trace, fmt, ...) \
30
+ do { \
31
+ (trace_##trace)(__VA_ARGS__); \
32
+ qemu_log_mask(LOG_GUEST_ERROR, #trace \
33
+ " in %s: " fmt "\n", __func__, ## __VA_ARGS__); \
34
+ } while (0)
35
+
36
static void nvme_process_sq(void *opaque);
37
38
static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size)
39
@@ -XXX,XX +XXX,XX @@ static void nvme_isr_notify(NvmeCtrl *n, NvmeCQueue *cq)
73
{
40
{
74
BlockDriverState *bs;
41
if (cq->irq_enabled) {
75
BdrvDirtyBitmap *bitmap;
42
if (msix_enabled(&(n->parent_obj))) {
76
+ AioContext *aio_context;
43
+ trace_nvme_irq_msix(cq->vector);
77
44
msix_notify(&(n->parent_obj), cq->vector);
78
if (!name || name[0] == '\0') {
45
} else {
79
error_setg(errp, "Bitmap name cannot be empty");
46
+ trace_nvme_irq_pin();
80
@@ -XXX,XX +XXX,XX @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
47
pci_irq_pulse(&n->parent_obj);
48
}
49
+ } else {
50
+ trace_nvme_irq_masked();
51
}
52
}
53
54
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
55
trans_len = MIN(len, trans_len);
56
int num_prps = (len >> n->page_bits) + 1;
57
58
- if (!prp1) {
59
+ if (unlikely(!prp1)) {
60
+ trace_nvme_err_invalid_prp();
61
return NVME_INVALID_FIELD | NVME_DNR;
62
} else if (n->cmbsz && prp1 >= n->ctrl_mem.addr &&
63
prp1 < n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size)) {
64
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
65
}
66
len -= trans_len;
67
if (len) {
68
- if (!prp2) {
69
+ if (unlikely(!prp2)) {
70
+ trace_nvme_err_invalid_prp2_missing();
71
goto unmap;
72
}
73
if (len > n->page_size) {
74
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
75
uint64_t prp_ent = le64_to_cpu(prp_list[i]);
76
77
if (i == n->max_prp_ents - 1 && len > n->page_size) {
78
- if (!prp_ent || prp_ent & (n->page_size - 1)) {
79
+ if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
80
+ trace_nvme_err_invalid_prplist_ent(prp_ent);
81
goto unmap;
82
}
83
84
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
85
prp_ent = le64_to_cpu(prp_list[i]);
86
}
87
88
- if (!prp_ent || prp_ent & (n->page_size - 1)) {
89
+ if (unlikely(!prp_ent || prp_ent & (n->page_size - 1))) {
90
+ trace_nvme_err_invalid_prplist_ent(prp_ent);
91
goto unmap;
92
}
93
94
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_map_prp(QEMUSGList *qsg, QEMUIOVector *iov, uint64_t prp1,
95
i++;
96
}
97
} else {
98
- if (prp2 & (n->page_size - 1)) {
99
+ if (unlikely(prp2 & (n->page_size - 1))) {
100
+ trace_nvme_err_invalid_prp2_align(prp2);
101
goto unmap;
102
}
103
if (qsg->nsg) {
104
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_dma_read_prp(NvmeCtrl *n, uint8_t *ptr, uint32_t len,
105
QEMUIOVector iov;
106
uint16_t status = NVME_SUCCESS;
107
108
+ trace_nvme_dma_read(prp1, prp2);
109
+
110
if (nvme_map_prp(&qsg, &iov, prp1, prp2, len, n)) {
111
return NVME_INVALID_FIELD | NVME_DNR;
112
}
113
if (qsg.nsg > 0) {
114
- if (dma_buf_read(ptr, len, &qsg)) {
115
+ if (unlikely(dma_buf_read(ptr, len, &qsg))) {
116
+ trace_nvme_err_invalid_dma();
117
status = NVME_INVALID_FIELD | NVME_DNR;
118
}
119
qemu_sglist_destroy(&qsg);
120
} else {
121
- if (qemu_iovec_to_buf(&iov, 0, ptr, len) != len) {
122
+ if (unlikely(qemu_iovec_to_buf(&iov, 0, ptr, len) != len)) {
123
+ trace_nvme_err_invalid_dma();
124
status = NVME_INVALID_FIELD | NVME_DNR;
125
}
126
qemu_iovec_destroy(&iov);
127
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_write_zeros(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
128
uint64_t aio_slba = slba << (data_shift - BDRV_SECTOR_BITS);
129
uint32_t aio_nlb = nlb << (data_shift - BDRV_SECTOR_BITS);
130
131
- if (slba + nlb > ns->id_ns.nsze) {
132
+ if (unlikely(slba + nlb > ns->id_ns.nsze)) {
133
+ trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
134
return NVME_LBA_RANGE | NVME_DNR;
135
}
136
137
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd,
138
int is_write = rw->opcode == NVME_CMD_WRITE ? 1 : 0;
139
enum BlockAcctType acct = is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ;
140
141
- if ((slba + nlb) > ns->id_ns.nsze) {
142
+ trace_nvme_rw(is_write ? "write" : "read", nlb, data_size, slba);
143
+
144
+ if (unlikely((slba + nlb) > ns->id_ns.nsze)) {
145
block_acct_invalid(blk_get_stats(n->conf.blk), acct);
146
+ trace_nvme_err_invalid_lba_range(slba, nlb, ns->id_ns.nsze);
147
return NVME_LBA_RANGE | NVME_DNR;
148
}
149
150
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
151
NvmeNamespace *ns;
152
uint32_t nsid = le32_to_cpu(cmd->nsid);
153
154
- if (nsid == 0 || nsid > n->num_namespaces) {
155
+ if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
156
+ trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
157
return NVME_INVALID_NSID | NVME_DNR;
158
}
159
160
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_io_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
161
case NVME_CMD_READ:
162
return nvme_rw(n, ns, cmd, req);
163
default:
164
+ trace_nvme_err_invalid_opc(cmd->opcode);
165
return NVME_INVALID_OPCODE | NVME_DNR;
166
}
167
}
168
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_del_sq(NvmeCtrl *n, NvmeCmd *cmd)
169
NvmeCQueue *cq;
170
uint16_t qid = le16_to_cpu(c->qid);
171
172
- if (!qid || nvme_check_sqid(n, qid)) {
173
+ if (unlikely(!qid || nvme_check_sqid(n, qid))) {
174
+ trace_nvme_err_invalid_del_sq(qid);
175
return NVME_INVALID_QID | NVME_DNR;
176
}
177
178
+ trace_nvme_del_sq(qid);
179
+
180
sq = n->sq[qid];
181
while (!QTAILQ_EMPTY(&sq->out_req_list)) {
182
req = QTAILQ_FIRST(&sq->out_req_list);
183
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_create_sq(NvmeCtrl *n, NvmeCmd *cmd)
184
uint16_t qflags = le16_to_cpu(c->sq_flags);
185
uint64_t prp1 = le64_to_cpu(c->prp1);
186
187
- if (!cqid || nvme_check_cqid(n, cqid)) {
188
+ trace_nvme_create_sq(prp1, sqid, cqid, qsize, qflags);
189
+
190
+ if (unlikely(!cqid || nvme_check_cqid(n, cqid))) {
191
+ trace_nvme_err_invalid_create_sq_cqid(cqid);
192
return NVME_INVALID_CQID | NVME_DNR;
193
}
194
- if (!sqid || !nvme_check_sqid(n, sqid)) {
195
+ if (unlikely(!sqid || !nvme_check_sqid(n, sqid))) {
196
+ trace_nvme_err_invalid_create_sq_sqid(sqid);
197
return NVME_INVALID_QID | NVME_DNR;
198
}
199
- if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
200
+ if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
201
+ trace_nvme_err_invalid_create_sq_size(qsize);
202
return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
203
}
204
- if (!prp1 || prp1 & (n->page_size - 1)) {
205
+ if (unlikely(!prp1 || prp1 & (n->page_size - 1))) {
206
+ trace_nvme_err_invalid_create_sq_addr(prp1);
207
return NVME_INVALID_FIELD | NVME_DNR;
208
}
209
- if (!(NVME_SQ_FLAGS_PC(qflags))) {
210
+ if (unlikely(!(NVME_SQ_FLAGS_PC(qflags)))) {
211
+ trace_nvme_err_invalid_create_sq_qflags(NVME_SQ_FLAGS_PC(qflags));
212
return NVME_INVALID_FIELD | NVME_DNR;
213
}
214
sq = g_malloc0(sizeof(*sq));
215
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_del_cq(NvmeCtrl *n, NvmeCmd *cmd)
216
NvmeCQueue *cq;
217
uint16_t qid = le16_to_cpu(c->qid);
218
219
- if (!qid || nvme_check_cqid(n, qid)) {
220
+ if (unlikely(!qid || nvme_check_cqid(n, qid))) {
221
+ trace_nvme_err_invalid_del_cq_cqid(qid);
222
return NVME_INVALID_CQID | NVME_DNR;
223
}
224
225
cq = n->cq[qid];
226
- if (!QTAILQ_EMPTY(&cq->sq_list)) {
227
+ if (unlikely(!QTAILQ_EMPTY(&cq->sq_list))) {
228
+ trace_nvme_err_invalid_del_cq_notempty(qid);
229
return NVME_INVALID_QUEUE_DEL;
230
}
231
+ trace_nvme_del_cq(qid);
232
nvme_free_cq(cq, n);
233
return NVME_SUCCESS;
234
}
235
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_create_cq(NvmeCtrl *n, NvmeCmd *cmd)
236
uint16_t qflags = le16_to_cpu(c->cq_flags);
237
uint64_t prp1 = le64_to_cpu(c->prp1);
238
239
- if (!cqid || !nvme_check_cqid(n, cqid)) {
240
+ trace_nvme_create_cq(prp1, cqid, vector, qsize, qflags,
241
+ NVME_CQ_FLAGS_IEN(qflags) != 0);
242
+
243
+ if (unlikely(!cqid || !nvme_check_cqid(n, cqid))) {
244
+ trace_nvme_err_invalid_create_cq_cqid(cqid);
245
return NVME_INVALID_CQID | NVME_DNR;
246
}
247
- if (!qsize || qsize > NVME_CAP_MQES(n->bar.cap)) {
248
+ if (unlikely(!qsize || qsize > NVME_CAP_MQES(n->bar.cap))) {
249
+ trace_nvme_err_invalid_create_cq_size(qsize);
250
return NVME_MAX_QSIZE_EXCEEDED | NVME_DNR;
251
}
252
- if (!prp1) {
253
+ if (unlikely(!prp1)) {
254
+ trace_nvme_err_invalid_create_cq_addr(prp1);
255
return NVME_INVALID_FIELD | NVME_DNR;
256
}
257
- if (vector > n->num_queues) {
258
+ if (unlikely(vector > n->num_queues)) {
259
+ trace_nvme_err_invalid_create_cq_vector(vector);
260
return NVME_INVALID_IRQ_VECTOR | NVME_DNR;
261
}
262
- if (!(NVME_CQ_FLAGS_PC(qflags))) {
263
+ if (unlikely(!(NVME_CQ_FLAGS_PC(qflags)))) {
264
+ trace_nvme_err_invalid_create_cq_qflags(NVME_CQ_FLAGS_PC(qflags));
265
return NVME_INVALID_FIELD | NVME_DNR;
266
}
267
268
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_identify_ctrl(NvmeCtrl *n, NvmeIdentify *c)
269
uint64_t prp1 = le64_to_cpu(c->prp1);
270
uint64_t prp2 = le64_to_cpu(c->prp2);
271
272
+ trace_nvme_identify_ctrl();
273
+
274
return nvme_dma_read_prp(n, (uint8_t *)&n->id_ctrl, sizeof(n->id_ctrl),
275
prp1, prp2);
276
}
277
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_identify_ns(NvmeCtrl *n, NvmeIdentify *c)
278
uint64_t prp1 = le64_to_cpu(c->prp1);
279
uint64_t prp2 = le64_to_cpu(c->prp2);
280
281
- if (nsid == 0 || nsid > n->num_namespaces) {
282
+ trace_nvme_identify_ns(nsid);
283
+
284
+ if (unlikely(nsid == 0 || nsid > n->num_namespaces)) {
285
+ trace_nvme_err_invalid_ns(nsid, n->num_namespaces);
286
return NVME_INVALID_NSID | NVME_DNR;
287
}
288
289
ns = &n->namespaces[nsid - 1];
290
+
291
return nvme_dma_read_prp(n, (uint8_t *)&ns->id_ns, sizeof(ns->id_ns),
292
prp1, prp2);
293
}
294
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_identify_nslist(NvmeCtrl *n, NvmeIdentify *c)
295
uint16_t ret;
296
int i, j = 0;
297
298
+ trace_nvme_identify_nslist(min_nsid);
299
+
300
list = g_malloc0(data_len);
301
for (i = 0; i < n->num_namespaces; i++) {
302
if (i < min_nsid) {
303
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_identify(NvmeCtrl *n, NvmeCmd *cmd)
304
case 0x02:
305
return nvme_identify_nslist(n, c);
306
default:
307
+ trace_nvme_err_invalid_identify_cns(le32_to_cpu(c->cns));
308
return NVME_INVALID_FIELD | NVME_DNR;
309
}
310
}
311
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_get_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
312
switch (dw10) {
313
case NVME_VOLATILE_WRITE_CACHE:
314
result = blk_enable_write_cache(n->conf.blk);
315
+ trace_nvme_getfeat_vwcache(result ? "enabled" : "disabled");
316
break;
317
case NVME_NUMBER_OF_QUEUES:
318
result = cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
319
+ trace_nvme_getfeat_numq(result);
320
break;
321
default:
322
+ trace_nvme_err_invalid_getfeat(dw10);
323
return NVME_INVALID_FIELD | NVME_DNR;
324
}
325
326
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_set_feature(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
327
blk_set_enable_write_cache(n->conf.blk, dw11 & 1);
328
break;
329
case NVME_NUMBER_OF_QUEUES:
330
+ trace_nvme_setfeat_numq((dw11 & 0xFFFF) + 1,
331
+ ((dw11 >> 16) & 0xFFFF) + 1,
332
+ n->num_queues - 1, n->num_queues - 1);
333
req->cqe.result =
334
cpu_to_le32((n->num_queues - 2) | ((n->num_queues - 2) << 16));
335
break;
336
default:
337
+ trace_nvme_err_invalid_setfeat(dw10);
338
return NVME_INVALID_FIELD | NVME_DNR;
339
}
340
return NVME_SUCCESS;
341
@@ -XXX,XX +XXX,XX @@ static uint16_t nvme_admin_cmd(NvmeCtrl *n, NvmeCmd *cmd, NvmeRequest *req)
342
case NVME_ADM_CMD_GET_FEATURES:
343
return nvme_get_feature(n, cmd, req);
344
default:
345
+ trace_nvme_err_invalid_admin_opc(cmd->opcode);
346
return NVME_INVALID_OPCODE | NVME_DNR;
347
}
348
}
349
@@ -XXX,XX +XXX,XX @@ static int nvme_start_ctrl(NvmeCtrl *n)
350
uint32_t page_bits = NVME_CC_MPS(n->bar.cc) + 12;
351
uint32_t page_size = 1 << page_bits;
352
353
- if (n->cq[0] || n->sq[0] || !n->bar.asq || !n->bar.acq ||
354
- n->bar.asq & (page_size - 1) || n->bar.acq & (page_size - 1) ||
355
- NVME_CC_MPS(n->bar.cc) < NVME_CAP_MPSMIN(n->bar.cap) ||
356
- NVME_CC_MPS(n->bar.cc) > NVME_CAP_MPSMAX(n->bar.cap) ||
357
- NVME_CC_IOCQES(n->bar.cc) < NVME_CTRL_CQES_MIN(n->id_ctrl.cqes) ||
358
- NVME_CC_IOCQES(n->bar.cc) > NVME_CTRL_CQES_MAX(n->id_ctrl.cqes) ||
359
- NVME_CC_IOSQES(n->bar.cc) < NVME_CTRL_SQES_MIN(n->id_ctrl.sqes) ||
360
- NVME_CC_IOSQES(n->bar.cc) > NVME_CTRL_SQES_MAX(n->id_ctrl.sqes) ||
361
- !NVME_AQA_ASQS(n->bar.aqa) || !NVME_AQA_ACQS(n->bar.aqa)) {
362
+ if (unlikely(n->cq[0])) {
363
+ trace_nvme_err_startfail_cq();
364
+ return -1;
365
+ }
366
+ if (unlikely(n->sq[0])) {
367
+ trace_nvme_err_startfail_sq();
368
+ return -1;
369
+ }
370
+ if (unlikely(!n->bar.asq)) {
371
+ trace_nvme_err_startfail_nbarasq();
372
+ return -1;
373
+ }
374
+ if (unlikely(!n->bar.acq)) {
375
+ trace_nvme_err_startfail_nbaracq();
376
+ return -1;
377
+ }
378
+ if (unlikely(n->bar.asq & (page_size - 1))) {
379
+ trace_nvme_err_startfail_asq_misaligned(n->bar.asq);
380
+ return -1;
381
+ }
382
+ if (unlikely(n->bar.acq & (page_size - 1))) {
383
+ trace_nvme_err_startfail_acq_misaligned(n->bar.acq);
384
+ return -1;
385
+ }
386
+ if (unlikely(NVME_CC_MPS(n->bar.cc) <
387
+ NVME_CAP_MPSMIN(n->bar.cap))) {
388
+ trace_nvme_err_startfail_page_too_small(
389
+ NVME_CC_MPS(n->bar.cc),
390
+ NVME_CAP_MPSMIN(n->bar.cap));
391
+ return -1;
392
+ }
393
+ if (unlikely(NVME_CC_MPS(n->bar.cc) >
394
+ NVME_CAP_MPSMAX(n->bar.cap))) {
395
+ trace_nvme_err_startfail_page_too_large(
396
+ NVME_CC_MPS(n->bar.cc),
397
+ NVME_CAP_MPSMAX(n->bar.cap));
398
+ return -1;
399
+ }
400
+ if (unlikely(NVME_CC_IOCQES(n->bar.cc) <
401
+ NVME_CTRL_CQES_MIN(n->id_ctrl.cqes))) {
402
+ trace_nvme_err_startfail_cqent_too_small(
403
+ NVME_CC_IOCQES(n->bar.cc),
404
+ NVME_CTRL_CQES_MIN(n->bar.cap));
405
+ return -1;
406
+ }
407
+ if (unlikely(NVME_CC_IOCQES(n->bar.cc) >
408
+ NVME_CTRL_CQES_MAX(n->id_ctrl.cqes))) {
409
+ trace_nvme_err_startfail_cqent_too_large(
410
+ NVME_CC_IOCQES(n->bar.cc),
411
+ NVME_CTRL_CQES_MAX(n->bar.cap));
412
+ return -1;
413
+ }
414
+ if (unlikely(NVME_CC_IOSQES(n->bar.cc) <
415
+ NVME_CTRL_SQES_MIN(n->id_ctrl.sqes))) {
416
+ trace_nvme_err_startfail_sqent_too_small(
417
+ NVME_CC_IOSQES(n->bar.cc),
418
+ NVME_CTRL_SQES_MIN(n->bar.cap));
419
+ return -1;
420
+ }
421
+ if (unlikely(NVME_CC_IOSQES(n->bar.cc) >
422
+ NVME_CTRL_SQES_MAX(n->id_ctrl.sqes))) {
423
+ trace_nvme_err_startfail_sqent_too_large(
424
+ NVME_CC_IOSQES(n->bar.cc),
425
+ NVME_CTRL_SQES_MAX(n->bar.cap));
426
+ return -1;
427
+ }
428
+ if (unlikely(!NVME_AQA_ASQS(n->bar.aqa))) {
429
+ trace_nvme_err_startfail_asqent_sz_zero();
430
+ return -1;
431
+ }
432
+ if (unlikely(!NVME_AQA_ACQS(n->bar.aqa))) {
433
+ trace_nvme_err_startfail_acqent_sz_zero();
434
return -1;
435
}
436
437
@@ -XXX,XX +XXX,XX @@ static int nvme_start_ctrl(NvmeCtrl *n)
438
static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
439
unsigned size)
440
{
441
+ if (unlikely(offset & (sizeof(uint32_t) - 1))) {
442
+ NVME_GUEST_ERR(nvme_ub_mmiowr_misaligned32,
443
+ "MMIO write not 32-bit aligned,"
444
+ " offset=0x%"PRIx64"", offset);
445
+ /* should be ignored, fall through for now */
446
+ }
447
+
448
+ if (unlikely(size < sizeof(uint32_t))) {
449
+ NVME_GUEST_ERR(nvme_ub_mmiowr_toosmall,
450
+ "MMIO write smaller than 32-bits,"
451
+ " offset=0x%"PRIx64", size=%u",
452
+ offset, size);
453
+ /* should be ignored, fall through for now */
454
+ }
455
+
456
switch (offset) {
457
- case 0xc:
458
+ case 0xc: /* INTMS */
459
+ if (unlikely(msix_enabled(&(n->parent_obj)))) {
460
+ NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
461
+ "undefined access to interrupt mask set"
462
+ " when MSI-X is enabled");
463
+ /* should be ignored, fall through for now */
464
+ }
465
n->bar.intms |= data & 0xffffffff;
466
n->bar.intmc = n->bar.intms;
467
+ trace_nvme_mmio_intm_set(data & 0xffffffff,
468
+ n->bar.intmc);
469
break;
470
- case 0x10:
471
+ case 0x10: /* INTMC */
472
+ if (unlikely(msix_enabled(&(n->parent_obj)))) {
473
+ NVME_GUEST_ERR(nvme_ub_mmiowr_intmask_with_msix,
474
+ "undefined access to interrupt mask clr"
475
+ " when MSI-X is enabled");
476
+ /* should be ignored, fall through for now */
477
+ }
478
n->bar.intms &= ~(data & 0xffffffff);
479
n->bar.intmc = n->bar.intms;
480
+ trace_nvme_mmio_intm_clr(data & 0xffffffff,
481
+ n->bar.intmc);
482
break;
483
- case 0x14:
484
+ case 0x14: /* CC */
485
+ trace_nvme_mmio_cfg(data & 0xffffffff);
486
/* Windows first sends data, then sends enable bit */
487
if (!NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc) &&
488
!NVME_CC_SHN(data) && !NVME_CC_SHN(n->bar.cc))
489
@@ -XXX,XX +XXX,XX @@ static void nvme_write_bar(NvmeCtrl *n, hwaddr offset, uint64_t data,
490
491
if (NVME_CC_EN(data) && !NVME_CC_EN(n->bar.cc)) {
492
n->bar.cc = data;
493
- if (nvme_start_ctrl(n)) {
494
+ if (unlikely(nvme_start_ctrl(n))) {
495
+ trace_nvme_err_startfail();
496
n->bar.csts = NVME_CSTS_FAILED;
497
} else {
498
+ trace_nvme_mmio_start_success();
499
n->bar.csts = NVME_CSTS_READY;
500
}
501
} else if (!NVME_CC_EN(data) && NVME_CC_EN(n->bar.cc)) {
502
+ trace_nvme_mmio_stopped();
503
nvme_clear_ctrl(n);
504
n->bar.csts &= ~NVME_CSTS_READY;
505
}
506
if (NVME_CC_SHN(data) && !(NVME_CC_SHN(n->bar.cc))) {
507
- nvme_clear_ctrl(n);
508
- n->bar.cc = data;
509
- n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
510
+ trace_nvme_mmio_shutdown_set();
511
+ nvme_clear_ctrl(n);
512
+ n->bar.cc = data;
513
+ n->bar.csts |= NVME_CSTS_SHST_COMPLETE;
514
} else if (!NVME_CC_SHN(data) && NVME_CC_SHN(n->bar.cc)) {
515
- n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
516
- n->bar.cc = data;
517
+ trace_nvme_mmio_shutdown_cleared();
518
+ n->bar.csts &= ~NVME_CSTS_SHST_COMPLETE;
519
+ n->bar.cc = data;
520
+ }
521
+ break;
522
+ case 0x1C: /* CSTS */
523
+ if (data & (1 << 4)) {
524
+ NVME_GUEST_ERR(nvme_ub_mmiowr_ssreset_w1c_unsupported,
525
+ "attempted to W1C CSTS.NSSRO"
526
+ " but CAP.NSSRS is zero (not supported)");
527
+ } else if (data != 0) {
528
+ NVME_GUEST_ERR(nvme_ub_mmiowr_ro_csts,
529
+ "attempted to set a read only bit"
530
+ " of controller status");
531
+ }
532
+ break;
533
+ case 0x20: /* NSSR */
534
+ if (data == 0x4E564D65) {
535
+ trace_nvme_ub_mmiowr_ssreset_unsupported();
536
+ } else {
537
+ /* The spec says that writes of other values have no effect */
538
+ return;
539
}
540
break;
541
- case 0x24:
542
+ case 0x24: /* AQA */
543
n->bar.aqa = data & 0xffffffff;
544
+ trace_nvme_mmio_aqattr(data & 0xffffffff);
545
break;
546
- case 0x28:
547
+ case 0x28: /* ASQ */
548
n->bar.asq = data;
549
+ trace_nvme_mmio_asqaddr(data);
550
break;
551
- case 0x2c:
552
+ case 0x2c: /* ASQ hi */
553
n->bar.asq |= data << 32;
554
+ trace_nvme_mmio_asqaddr_hi(data, n->bar.asq);
555
break;
556
- case 0x30:
557
+ case 0x30: /* ACQ */
558
+ trace_nvme_mmio_acqaddr(data);
559
n->bar.acq = data;
560
break;
561
- case 0x34:
562
+ case 0x34: /* ACQ hi */
563
n->bar.acq |= data << 32;
564
+ trace_nvme_mmio_acqaddr_hi(data, n->bar.acq);
565
break;
566
+ case 0x38: /* CMBLOC */
567
+ NVME_GUEST_ERR(nvme_ub_mmiowr_cmbloc_reserved,
568
+ "invalid write to reserved CMBLOC"
569
+ " when CMBSZ is zero, ignored");
570
+ return;
571
+ case 0x3C: /* CMBSZ */
572
+ NVME_GUEST_ERR(nvme_ub_mmiowr_cmbsz_readonly,
573
+ "invalid write to read only CMBSZ, ignored");
574
+ return;
575
default:
576
+ NVME_GUEST_ERR(nvme_ub_mmiowr_invalid,
577
+ "invalid MMIO write,"
578
+ " offset=0x%"PRIx64", data=%"PRIx64"",
579
+ offset, data);
580
break;
581
}
582
}
583
@@ -XXX,XX +XXX,XX @@ static uint64_t nvme_mmio_read(void *opaque, hwaddr addr, unsigned size)
584
uint8_t *ptr = (uint8_t *)&n->bar;
585
uint64_t val = 0;
586
587
+ if (unlikely(addr & (sizeof(uint32_t) - 1))) {
588
+ NVME_GUEST_ERR(nvme_ub_mmiord_misaligned32,
589
+ "MMIO read not 32-bit aligned,"
590
+ " offset=0x%"PRIx64"", addr);
591
+ /* should RAZ, fall through for now */
592
+ } else if (unlikely(size < sizeof(uint32_t))) {
593
+ NVME_GUEST_ERR(nvme_ub_mmiord_toosmall,
594
+ "MMIO read smaller than 32-bits,"
595
+ " offset=0x%"PRIx64"", addr);
596
+ /* should RAZ, fall through for now */
597
+ }
598
+
599
if (addr < sizeof(n->bar)) {
600
memcpy(&val, ptr + addr, size);
601
+ } else {
602
+ NVME_GUEST_ERR(nvme_ub_mmiord_invalid_ofs,
603
+ "MMIO read beyond last register,"
604
+ " offset=0x%"PRIx64", returning 0", addr);
605
}
606
+
607
return val;
608
}
609
610
@@ -XXX,XX +XXX,XX @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
611
{
612
uint32_t qid;
613
614
- if (addr & ((1 << 2) - 1)) {
615
+ if (unlikely(addr & ((1 << 2) - 1))) {
616
+ NVME_GUEST_ERR(nvme_ub_db_wr_misaligned,
617
+ "doorbell write not 32-bit aligned,"
618
+ " offset=0x%"PRIx64", ignoring", addr);
81
return;
619
return;
82
}
620
}
83
621
84
+ aio_context = bdrv_get_aio_context(bs);
622
if (((addr - 0x1000) >> 2) & 1) {
85
+ aio_context_acquire(aio_context);
623
+ /* Completion queue doorbell write */
86
+
624
+
87
if (has_granularity) {
625
uint16_t new_head = val & 0xffff;
88
if (granularity < 512 || !is_power_of_2(granularity)) {
626
int start_sqs;
89
error_setg(errp, "Granularity must be power of 2 "
627
NvmeCQueue *cq;
90
"and at least 512");
628
91
- return;
629
qid = (addr - (0x1000 + (1 << 2))) >> 3;
92
+ goto out;
630
- if (nvme_check_cqid(n, qid)) {
631
+ if (unlikely(nvme_check_cqid(n, qid))) {
632
+ NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cq,
633
+ "completion queue doorbell write"
634
+ " for nonexistent queue,"
635
+ " sqid=%"PRIu32", ignoring", qid);
636
return;
637
}
638
639
cq = n->cq[qid];
640
- if (new_head >= cq->size) {
641
+ if (unlikely(new_head >= cq->size)) {
642
+ NVME_GUEST_ERR(nvme_ub_db_wr_invalid_cqhead,
643
+ "completion queue doorbell write value"
644
+ " beyond queue size, sqid=%"PRIu32","
645
+ " new_head=%"PRIu16", ignoring",
646
+ qid, new_head);
647
return;
648
}
649
650
@@ -XXX,XX +XXX,XX @@ static void nvme_process_db(NvmeCtrl *n, hwaddr addr, int val)
651
nvme_isr_notify(n, cq);
93
}
652
}
94
} else {
653
} else {
95
/* Default to cluster size, if available: */
654
+ /* Submission queue doorbell write */
96
@@ -XXX,XX +XXX,XX @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
655
+
97
if (persistent &&
656
uint16_t new_tail = val & 0xffff;
98
!bdrv_can_store_new_dirty_bitmap(bs, name, granularity, errp))
657
NvmeSQueue *sq;
99
{
658
100
- return;
659
qid = (addr - 0x1000) >> 3;
101
+ goto out;
660
- if (nvme_check_sqid(n, qid)) {
102
}
661
+ if (unlikely(nvme_check_sqid(n, qid))) {
103
662
+ NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sq,
104
bitmap = bdrv_create_dirty_bitmap(bs, granularity, name, errp);
663
+ "submission queue doorbell write"
105
if (bitmap == NULL) {
664
+ " for nonexistent queue,"
106
- return;
665
+ " sqid=%"PRIu32", ignoring", qid);
107
+ goto out;
666
return;
108
}
667
}
109
668
110
if (disabled) {
669
sq = n->sq[qid];
111
@@ -XXX,XX +XXX,XX @@ void qmp_block_dirty_bitmap_add(const char *node, const char *name,
670
- if (new_tail >= sq->size) {
112
}
671
+ if (unlikely(new_tail >= sq->size)) {
113
672
+ NVME_GUEST_ERR(nvme_ub_db_wr_invalid_sqtail,
114
bdrv_dirty_bitmap_set_persistence(bitmap, persistent);
673
+ "submission queue doorbell write value"
115
+
674
+ " beyond queue size, sqid=%"PRIu32","
116
+out:
675
+ " new_tail=%"PRIu16", ignoring",
117
+ aio_context_release(aio_context);
676
+ qid, new_tail);
118
}
677
return;
119
678
}
120
static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
679
121
@@ -XXX,XX +XXX,XX @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
680
diff --git a/hw/block/trace-events b/hw/block/trace-events
122
{
681
index XXXXXXX..XXXXXXX 100644
123
BlockDriverState *bs;
682
--- a/hw/block/trace-events
124
BdrvDirtyBitmap *bitmap;
683
+++ b/hw/block/trace-events
125
+ AioContext *aio_context;
684
@@ -XXX,XX +XXX,XX @@ virtio_blk_submit_multireq(void *vdev, void *mrb, int start, int num_reqs, uint6
126
685
hd_geometry_lchs_guess(void *blk, int cyls, int heads, int secs) "blk %p LCHS %d %d %d"
127
bitmap = block_dirty_bitmap_lookup(node, name, &bs, errp);
686
hd_geometry_guess(void *blk, uint32_t cyls, uint32_t heads, uint32_t secs, int trans) "blk %p CHS %u %u %u trans %d"
128
if (!bitmap || !bs) {
687
129
return NULL;
688
+# hw/block/nvme.c
130
}
689
+# nvme traces for successful events
131
690
+nvme_irq_msix(uint32_t vector) "raising MSI-X IRQ vector %u"
132
+ aio_context = bdrv_get_aio_context(bs);
691
+nvme_irq_pin(void) "pulsing IRQ pin"
133
+ aio_context_acquire(aio_context);
692
+nvme_irq_masked(void) "IRQ is masked"
134
+
693
+nvme_dma_read(uint64_t prp1, uint64_t prp2) "DMA read, prp1=0x%"PRIx64" prp2=0x%"PRIx64""
135
if (bdrv_dirty_bitmap_check(bitmap, BDRV_BITMAP_BUSY | BDRV_BITMAP_RO,
694
+nvme_rw(char const *verb, uint32_t blk_count, uint64_t byte_count, uint64_t lba) "%s %"PRIu32" blocks (%"PRIu64" bytes) from LBA %"PRIu64""
136
errp)) {
695
+nvme_create_sq(uint64_t addr, uint16_t sqid, uint16_t cqid, uint16_t qsize, uint16_t qflags) "create submission queue, addr=0x%"PRIx64", sqid=%"PRIu16", cqid=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16""
137
+ aio_context_release(aio_context);
696
+nvme_create_cq(uint64_t addr, uint16_t cqid, uint16_t vector, uint16_t size, uint16_t qflags, int ien) "create completion queue, addr=0x%"PRIx64", cqid=%"PRIu16", vector=%"PRIu16", qsize=%"PRIu16", qflags=%"PRIu16", ien=%d"
138
return NULL;
697
+nvme_del_sq(uint16_t qid) "deleting submission queue sqid=%"PRIu16""
139
}
698
+nvme_del_cq(uint16_t cqid) "deleted completion queue, sqid=%"PRIu16""
140
699
+nvme_identify_ctrl(void) "identify controller"
141
if (bdrv_dirty_bitmap_get_persistence(bitmap) &&
700
+nvme_identify_ns(uint16_t ns) "identify namespace, nsid=%"PRIu16""
142
bdrv_remove_persistent_dirty_bitmap(bs, name, errp) < 0)
701
+nvme_identify_nslist(uint16_t ns) "identify namespace list, nsid=%"PRIu16""
143
{
702
+nvme_getfeat_vwcache(char const* result) "get feature volatile write cache, result=%s"
144
- return NULL;
703
+nvme_getfeat_numq(int result) "get feature number of queues, result=%d"
145
+ aio_context_release(aio_context);
704
+nvme_setfeat_numq(int reqcq, int reqsq, int gotcq, int gotsq) "requested cq_count=%d sq_count=%d, responding with cq_count=%d sq_count=%d"
146
+ return NULL;
705
+nvme_mmio_intm_set(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask set, data=0x%"PRIx64", new_mask=0x%"PRIx64""
147
}
706
+nvme_mmio_intm_clr(uint64_t data, uint64_t new_mask) "wrote MMIO, interrupt mask clr, data=0x%"PRIx64", new_mask=0x%"PRIx64""
148
707
+nvme_mmio_cfg(uint64_t data) "wrote MMIO, config controller config=0x%"PRIx64""
149
if (release) {
708
+nvme_mmio_aqattr(uint64_t data) "wrote MMIO, admin queue attributes=0x%"PRIx64""
150
@@ -XXX,XX +XXX,XX @@ static BdrvDirtyBitmap *do_block_dirty_bitmap_remove(
709
+nvme_mmio_asqaddr(uint64_t data) "wrote MMIO, admin submission queue address=0x%"PRIx64""
151
*bitmap_bs = bs;
710
+nvme_mmio_acqaddr(uint64_t data) "wrote MMIO, admin completion queue address=0x%"PRIx64""
152
}
711
+nvme_mmio_asqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin submission queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
153
712
+nvme_mmio_acqaddr_hi(uint64_t data, uint64_t new_addr) "wrote MMIO, admin completion queue high half=0x%"PRIx64", new_address=0x%"PRIx64""
154
+ aio_context_release(aio_context);
713
+nvme_mmio_start_success(void) "setting controller enable bit succeeded"
155
return release ? NULL : bitmap;
714
+nvme_mmio_stopped(void) "cleared controller enable bit"
156
}
715
+nvme_mmio_shutdown_set(void) "shutdown bit set"
157
716
+nvme_mmio_shutdown_cleared(void) "shutdown bit cleared"
717
+
718
+# nvme traces for error conditions
719
+nvme_err_invalid_dma(void) "PRP/SGL is too small for transfer size"
720
+nvme_err_invalid_prplist_ent(uint64_t prplist) "PRP list entry is null or not page aligned: 0x%"PRIx64""
721
+nvme_err_invalid_prp2_align(uint64_t prp2) "PRP2 is not page aligned: 0x%"PRIx64""
722
+nvme_err_invalid_prp2_missing(void) "PRP2 is null and more data to be transferred"
723
+nvme_err_invalid_field(void) "invalid field"
724
+nvme_err_invalid_prp(void) "invalid PRP"
725
+nvme_err_invalid_sgl(void) "invalid SGL"
726
+nvme_err_invalid_ns(uint32_t ns, uint32_t limit) "invalid namespace %u not within 1-%u"
727
+nvme_err_invalid_opc(uint8_t opc) "invalid opcode 0x%"PRIx8""
728
+nvme_err_invalid_admin_opc(uint8_t opc) "invalid admin opcode 0x%"PRIx8""
729
+nvme_err_invalid_lba_range(uint64_t start, uint64_t len, uint64_t limit) "Invalid LBA start=%"PRIu64" len=%"PRIu64" limit=%"PRIu64""
730
+nvme_err_invalid_del_sq(uint16_t qid) "invalid submission queue deletion, sid=%"PRIu16""
731
+nvme_err_invalid_create_sq_cqid(uint16_t cqid) "failed creating submission queue, invalid cqid=%"PRIu16""
732
+nvme_err_invalid_create_sq_sqid(uint16_t sqid) "failed creating submission queue, invalid sqid=%"PRIu16""
733
+nvme_err_invalid_create_sq_size(uint16_t qsize) "failed creating submission queue, invalid qsize=%"PRIu16""
734
+nvme_err_invalid_create_sq_addr(uint64_t addr) "failed creating submission queue, addr=0x%"PRIx64""
735
+nvme_err_invalid_create_sq_qflags(uint16_t qflags) "failed creating submission queue, qflags=%"PRIu16""
736
+nvme_err_invalid_del_cq_cqid(uint16_t cqid) "failed deleting completion queue, cqid=%"PRIu16""
737
+nvme_err_invalid_del_cq_notempty(uint16_t cqid) "failed deleting completion queue, it is not empty, cqid=%"PRIu16""
738
+nvme_err_invalid_create_cq_cqid(uint16_t cqid) "failed creating completion queue, cqid=%"PRIu16""
739
+nvme_err_invalid_create_cq_size(uint16_t size) "failed creating completion queue, size=%"PRIu16""
740
+nvme_err_invalid_create_cq_addr(uint64_t addr) "failed creating completion queue, addr=0x%"PRIx64""
741
+nvme_err_invalid_create_cq_vector(uint16_t vector) "failed creating completion queue, vector=%"PRIu16""
742
+nvme_err_invalid_create_cq_qflags(uint16_t qflags) "failed creating completion queue, qflags=%"PRIu16""
743
+nvme_err_invalid_identify_cns(uint16_t cns) "identify, invalid cns=0x%"PRIx16""
744
+nvme_err_invalid_getfeat(int dw10) "invalid get features, dw10=0x%"PRIx32""
745
+nvme_err_invalid_setfeat(uint32_t dw10) "invalid set features, dw10=0x%"PRIx32""
746
+nvme_err_startfail_cq(void) "nvme_start_ctrl failed because there are non-admin completion queues"
747
+nvme_err_startfail_sq(void) "nvme_start_ctrl failed because there are non-admin submission queues"
748
+nvme_err_startfail_nbarasq(void) "nvme_start_ctrl failed because the admin submission queue address is null"
749
+nvme_err_startfail_nbaracq(void) "nvme_start_ctrl failed because the admin completion queue address is null"
750
+nvme_err_startfail_asq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin submission queue address is misaligned: 0x%"PRIx64""
751
+nvme_err_startfail_acq_misaligned(uint64_t addr) "nvme_start_ctrl failed because the admin completion queue address is misaligned: 0x%"PRIx64""
752
+nvme_err_startfail_page_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too small: log2size=%u, min=%u"
753
+nvme_err_startfail_page_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the page size is too large: log2size=%u, max=%u"
754
+nvme_err_startfail_cqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too small: log2size=%u, min=%u"
755
+nvme_err_startfail_cqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the completion queue entry size is too large: log2size=%u, max=%u"
756
+nvme_err_startfail_sqent_too_small(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too small: log2size=%u, min=%u"
757
+nvme_err_startfail_sqent_too_large(uint8_t log2ps, uint8_t maxlog2ps) "nvme_start_ctrl failed because the submission queue entry size is too large: log2size=%u, max=%u"
758
+nvme_err_startfail_asqent_sz_zero(void) "nvme_start_ctrl failed because the admin submission queue size is zero"
759
+nvme_err_startfail_acqent_sz_zero(void) "nvme_start_ctrl failed because the admin completion queue size is zero"
760
+nvme_err_startfail(void) "setting controller enable bit failed"
761
+
762
+# Traces for undefined behavior
763
+nvme_ub_mmiowr_misaligned32(uint64_t offset) "MMIO write not 32-bit aligned, offset=0x%"PRIx64""
764
+nvme_ub_mmiowr_toosmall(uint64_t offset, unsigned size) "MMIO write smaller than 32 bits, offset=0x%"PRIx64", size=%u"
765
+nvme_ub_mmiowr_intmask_with_msix(void) "undefined access to interrupt mask set when MSI-X is enabled"
766
+nvme_ub_mmiowr_ro_csts(void) "attempted to set a read only bit of controller status"
767
+nvme_ub_mmiowr_ssreset_w1c_unsupported(void) "attempted to W1C CSTS.NSSRO but CAP.NSSRS is zero (not supported)"
768
+nvme_ub_mmiowr_ssreset_unsupported(void) "attempted NVM subsystem reset but CAP.NSSRS is zero (not supported)"
769
+nvme_ub_mmiowr_cmbloc_reserved(void) "invalid write to reserved CMBLOC when CMBSZ is zero, ignored"
770
+nvme_ub_mmiowr_cmbsz_readonly(void) "invalid write to read only CMBSZ, ignored"
771
+nvme_ub_mmiowr_invalid(uint64_t offset, uint64_t data) "invalid MMIO write, offset=0x%"PRIx64", data=0x%"PRIx64""
772
+nvme_ub_mmiord_misaligned32(uint64_t offset) "MMIO read not 32-bit aligned, offset=0x%"PRIx64""
773
+nvme_ub_mmiord_toosmall(uint64_t offset) "MMIO read smaller than 32-bits, offset=0x%"PRIx64""
774
+nvme_ub_mmiord_invalid_ofs(uint64_t offset) "MMIO read beyond last register, offset=0x%"PRIx64", returning 0"
775
+nvme_ub_db_wr_misaligned(uint64_t offset) "doorbell write not 32-bit aligned, offset=0x%"PRIx64", ignoring"
776
+nvme_ub_db_wr_invalid_cq(uint32_t qid) "completion queue doorbell write for nonexistent queue, cqid=%"PRIu32", ignoring"
777
+nvme_ub_db_wr_invalid_cqhead(uint32_t qid, uint16_t new_head) "completion queue doorbell write value beyond queue size, cqid=%"PRIu32", new_head=%"PRIu16", ignoring"
778
+nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for nonexistent queue, sqid=%"PRIu32", ignoring"
779
+nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
780
+
781
# hw/block/xen_disk.c
782
xen_disk_alloc(char *name) "%s"
783
xen_disk_init(char *name) "%s"
158
--
784
--
159
2.20.1
785
2.13.6
160
786
161
787
diff view generated by jsdifflib
New patch
1
From: Fam Zheng <famz@redhat.com>
1
2
3
Management tools create overlays of running guests with qemu-img:
4
5
$ qemu-img create -b /image/in/use.qcow2 -f qcow2 /overlay/image.qcow2
6
7
but this doesn't work anymore due to image locking:
8
9
qemu-img: /overlay/image.qcow2: Failed to get shared "write" lock
10
Is another process using the image?
11
Could not open backing image to determine size.
12
Use the force share option to allow this use case again.
13
14
Cc: qemu-stable@nongnu.org
15
Signed-off-by: Fam Zheng <famz@redhat.com>
16
Reviewed-by: Eric Blake <eblake@redhat.com>
17
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
18
---
19
block.c | 3 ++-
20
1 file changed, 2 insertions(+), 1 deletion(-)
21
22
diff --git a/block.c b/block.c
23
index XXXXXXX..XXXXXXX 100644
24
--- a/block.c
25
+++ b/block.c
26
@@ -XXX,XX +XXX,XX @@ void bdrv_img_create(const char *filename, const char *fmt,
27
back_flags = flags;
28
back_flags &= ~(BDRV_O_RDWR | BDRV_O_SNAPSHOT | BDRV_O_NO_BACKING);
29
30
+ backing_options = qdict_new();
31
if (backing_fmt) {
32
- backing_options = qdict_new();
33
qdict_put_str(backing_options, "driver", backing_fmt);
34
}
35
+ qdict_put_bool(backing_options, BDRV_OPT_FORCE_SHARE, true);
36
37
bs = bdrv_open(full_backing, NULL, backing_options, back_flags,
38
&local_err);
39
--
40
2.13.6
41
42
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
From: Thomas Huth <thuth@redhat.com>
2
2
3
Fix a couple of minor coding style issues in drive_backup_prepare.
3
It's not working anymore since QEMU v1.3.0 - time to remove it now.
4
4
5
Signed-off-by: Sergio Lopez <slp@redhat.com>
5
Signed-off-by: Thomas Huth <thuth@redhat.com>
6
Reviewed-by: Max Reitz <mreitz@redhat.com>
6
Reviewed-by: John Snow <jsnow@redhat.com>
7
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
7
Reviewed-by: Markus Armbruster <armbru@redhat.com>
8
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
8
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9
---
9
---
10
blockdev.c | 8 +++++---
10
blockdev.c | 11 -----------
11
1 file changed, 5 insertions(+), 3 deletions(-)
11
qemu-doc.texi | 6 ------
12
2 files changed, 17 deletions(-)
12
13
13
diff --git a/blockdev.c b/blockdev.c
14
diff --git a/blockdev.c b/blockdev.c
14
index XXXXXXX..XXXXXXX 100644
15
index XXXXXXX..XXXXXXX 100644
15
--- a/blockdev.c
16
--- a/blockdev.c
16
+++ b/blockdev.c
17
+++ b/blockdev.c
17
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
18
@@ -XXX,XX +XXX,XX @@ QemuOptsList qemu_legacy_drive_opts = {
18
19
.type = QEMU_OPT_STRING,
19
if (!backup->has_format) {
20
.help = "chs translation (auto, lba, none)",
20
backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
21
},{
21
- NULL : (char*) bs->drv->format_name;
22
- .name = "boot",
22
+ NULL : (char *) bs->drv->format_name;
23
- .type = QEMU_OPT_BOOL,
24
- .help = "(deprecated, ignored)",
25
- },{
26
.name = "addr",
27
.type = QEMU_OPT_STRING,
28
.help = "pci address (virtio only)",
29
@@ -XXX,XX +XXX,XX @@ DriveInfo *drive_new(QemuOpts *all_opts, BlockInterfaceType block_default_type)
30
goto fail;
23
}
31
}
24
32
25
/* Early check to avoid creating target */
33
- /* Deprecated option boot=[on|off] */
26
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
34
- if (qemu_opt_get(legacy_opts, "boot") != NULL) {
27
35
- fprintf(stderr, "qemu-kvm: boot=on|off is deprecated and will be "
28
flags = bs->open_flags | BDRV_O_RDWR;
36
- "ignored. Future versions will reject this parameter. Please "
29
37
- "update your scripts.\n");
30
- /* See if we have a backing HD we can use to create our new image
38
- }
31
- * on top of. */
39
-
32
+ /*
40
/* Other deprecated options */
33
+ * See if we have a backing HD we can use to create our new image
41
if (!qtest_enabled()) {
34
+ * on top of.
42
for (i = 0; i < ARRAY_SIZE(deprecated); i++) {
35
+ */
43
diff --git a/qemu-doc.texi b/qemu-doc.texi
36
if (backup->sync == MIRROR_SYNC_MODE_TOP) {
44
index XXXXXXX..XXXXXXX 100644
37
source = backing_bs(bs);
45
--- a/qemu-doc.texi
38
if (!source) {
46
+++ b/qemu-doc.texi
47
@@ -XXX,XX +XXX,XX @@ deprecated.
48
49
@section System emulator command line arguments
50
51
-@subsection -drive boot=on|off (since 1.3.0)
52
-
53
-The ``boot=on|off'' option to the ``-drive'' argument is
54
-ignored. Applications should use the ``bootindex=N'' parameter
55
-to set an absolute ordering between devices instead.
56
-
57
@subsection -tdf (since 1.3.0)
58
59
The ``-tdf'' argument is ignored. The behaviour implemented
39
--
60
--
40
2.20.1
61
2.13.6
41
62
42
63
diff view generated by jsdifflib
New patch
1
1
From: Thomas Huth <thuth@redhat.com>
2
3
It's been marked as deprecated since QEMU v2.10.0, and so far nobody
4
complained that we should keep it, so let's remove this legacy option
5
now to simplify the code quite a bit.
6
7
Signed-off-by: Thomas Huth <thuth@redhat.com>
8
Reviewed-by: John Snow <jsnow@redhat.com>
9
Reviewed-by: Markus Armbruster <armbru@redhat.com>
10
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
11
---
12
vl.c | 86 ++-------------------------------------------------------
13
qemu-doc.texi | 8 ------
14
qemu-options.hx | 19 ++-----------
15
3 files changed, 4 insertions(+), 109 deletions(-)
16
17
diff --git a/vl.c b/vl.c
18
index XXXXXXX..XXXXXXX 100644
19
--- a/vl.c
20
+++ b/vl.c
21
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
22
const char *boot_order = NULL;
23
const char *boot_once = NULL;
24
DisplayState *ds;
25
- int cyls, heads, secs, translation;
26
QemuOpts *opts, *machine_opts;
27
- QemuOpts *hda_opts = NULL, *icount_opts = NULL, *accel_opts = NULL;
28
+ QemuOpts *icount_opts = NULL, *accel_opts = NULL;
29
QemuOptsList *olist;
30
int optind;
31
const char *optarg;
32
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
33
34
cpu_model = NULL;
35
snapshot = 0;
36
- cyls = heads = secs = 0;
37
- translation = BIOS_ATA_TRANSLATION_AUTO;
38
39
nb_nics = 0;
40
41
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
42
if (optind >= argc)
43
break;
44
if (argv[optind][0] != '-') {
45
- hda_opts = drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
46
+ drive_add(IF_DEFAULT, 0, argv[optind++], HD_OPTS);
47
} else {
48
const QEMUOption *popt;
49
50
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
51
cpu_model = optarg;
52
break;
53
case QEMU_OPTION_hda:
54
- {
55
- char buf[256];
56
- if (cyls == 0)
57
- snprintf(buf, sizeof(buf), "%s", HD_OPTS);
58
- else
59
- snprintf(buf, sizeof(buf),
60
- "%s,cyls=%d,heads=%d,secs=%d%s",
61
- HD_OPTS , cyls, heads, secs,
62
- translation == BIOS_ATA_TRANSLATION_LBA ?
63
- ",trans=lba" :
64
- translation == BIOS_ATA_TRANSLATION_NONE ?
65
- ",trans=none" : "");
66
- drive_add(IF_DEFAULT, 0, optarg, buf);
67
- break;
68
- }
69
case QEMU_OPTION_hdb:
70
case QEMU_OPTION_hdc:
71
case QEMU_OPTION_hdd:
72
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv, char **envp)
73
case QEMU_OPTION_snapshot:
74
snapshot = 1;
75
break;
76
- case QEMU_OPTION_hdachs:
77
- {
78
- const char *p;
79
- p = optarg;
80
- cyls = strtol(p, (char **)&p, 0);
81
- if (cyls < 1 || cyls > 16383)
82
- goto chs_fail;
83
- if (*p != ',')
84
- goto chs_fail;
85
- p++;
86
- heads = strtol(p, (char **)&p, 0);
87
- if (heads < 1 || heads > 16)
88
- goto chs_fail;
89
- if (*p != ',')
90
- goto chs_fail;
91
- p++;
92
- secs = strtol(p, (char **)&p, 0);
93
- if (secs < 1 || secs > 63)
94
- goto chs_fail;
95
- if (*p == ',') {
96
- p++;
97
- if (!strcmp(p, "large")) {
98
- translation = BIOS_ATA_TRANSLATION_LARGE;
99
- } else if (!strcmp(p, "rechs")) {
100
- translation = BIOS_ATA_TRANSLATION_RECHS;
101
- } else if (!strcmp(p, "none")) {
102
- translation = BIOS_ATA_TRANSLATION_NONE;
103
- } else if (!strcmp(p, "lba")) {
104
- translation = BIOS_ATA_TRANSLATION_LBA;
105
- } else if (!strcmp(p, "auto")) {
106
- translation = BIOS_ATA_TRANSLATION_AUTO;
107
- } else {
108
- goto chs_fail;
109
- }
110
- } else if (*p != '\0') {
111
- chs_fail:
112
- error_report("invalid physical CHS format");
113
- exit(1);
114
- }
115
- if (hda_opts != NULL) {
116
- qemu_opt_set_number(hda_opts, "cyls", cyls,
117
- &error_abort);
118
- qemu_opt_set_number(hda_opts, "heads", heads,
119
- &error_abort);
120
- qemu_opt_set_number(hda_opts, "secs", secs,
121
- &error_abort);
122
- if (translation == BIOS_ATA_TRANSLATION_LARGE) {
123
- qemu_opt_set(hda_opts, "trans", "large",
124
- &error_abort);
125
- } else if (translation == BIOS_ATA_TRANSLATION_RECHS) {
126
- qemu_opt_set(hda_opts, "trans", "rechs",
127
- &error_abort);
128
- } else if (translation == BIOS_ATA_TRANSLATION_LBA) {
129
- qemu_opt_set(hda_opts, "trans", "lba",
130
- &error_abort);
131
- } else if (translation == BIOS_ATA_TRANSLATION_NONE) {
132
- qemu_opt_set(hda_opts, "trans", "none",
133
- &error_abort);
134
- }
135
- }
136
- }
137
- error_report("'-hdachs' is deprecated, please use '-device"
138
- " ide-hd,cyls=c,heads=h,secs=s,...' instead");
139
- break;
140
case QEMU_OPTION_numa:
141
opts = qemu_opts_parse_noisily(qemu_find_opts("numa"),
142
optarg, true);
143
diff --git a/qemu-doc.texi b/qemu-doc.texi
144
index XXXXXXX..XXXXXXX 100644
145
--- a/qemu-doc.texi
146
+++ b/qemu-doc.texi
147
@@ -XXX,XX +XXX,XX @@ The ``--net dump'' argument is now replaced with the
148
``-object filter-dump'' argument which works in combination
149
with the modern ``-netdev`` backends instead.
150
151
-@subsection -hdachs (since 2.10.0)
152
-
153
-The ``-hdachs'' argument is now a synonym for setting
154
-the ``cyls'', ``heads'', ``secs'', and ``trans'' properties
155
-on the ``ide-hd'' device using the ``-device'' argument.
156
-The new syntax allows different settings to be provided
157
-per disk.
158
-
159
@subsection -usbdevice (since 2.10.0)
160
161
The ``-usbdevice DEV'' argument is now a synonym for setting
162
diff --git a/qemu-options.hx b/qemu-options.hx
163
index XXXXXXX..XXXXXXX 100644
164
--- a/qemu-options.hx
165
+++ b/qemu-options.hx
166
@@ -XXX,XX +XXX,XX @@ of available connectors of a given interface type.
167
@item media=@var{media}
168
This option defines the type of the media: disk or cdrom.
169
@item cyls=@var{c},heads=@var{h},secs=@var{s}[,trans=@var{t}]
170
-These options have the same definition as they have in @option{-hdachs}.
171
-These parameters are deprecated, use the corresponding parameters
172
+Force disk physical geometry and the optional BIOS translation (trans=none or
173
+lba). These parameters are deprecated, use the corresponding parameters
174
of @code{-device} instead.
175
@item snapshot=@var{snapshot}
176
@var{snapshot} is "on" or "off" and controls snapshot mode for the given drive
177
@@ -XXX,XX +XXX,XX @@ the raw disk image you use is not written back. You can however force
178
the write back by pressing @key{C-a s} (@pxref{disk_images}).
179
ETEXI
180
181
-DEF("hdachs", HAS_ARG, QEMU_OPTION_hdachs, \
182
- "-hdachs c,h,s[,t]\n" \
183
- " force hard disk 0 physical geometry and the optional BIOS\n" \
184
- " translation (t=none or lba) (usually QEMU can guess them)\n",
185
- QEMU_ARCH_ALL)
186
-STEXI
187
-@item -hdachs @var{c},@var{h},@var{s},[,@var{t}]
188
-@findex -hdachs
189
-Force hard disk 0 physical geometry (1 <= @var{c} <= 16383, 1 <=
190
-@var{h} <= 16, 1 <= @var{s} <= 63) and optionally force the BIOS
191
-translation mode (@var{t}=none, lba or auto). Usually QEMU can guess
192
-all those parameters. This option is deprecated, please use
193
-@code{-device ide-hd,cyls=c,heads=h,secs=s,...} instead.
194
-ETEXI
195
-
196
DEF("fsdev", HAS_ARG, QEMU_OPTION_fsdev,
197
"-fsdev fsdriver,id=id[,path=path,][security_model={mapped-xattr|mapped-file|passthrough|none}]\n"
198
" [,writeout=immediate][,readonly][,socket=socket|sock_fd=sock_fd][,fmode=fmode][,dmode=dmode]\n"
199
--
200
2.13.6
201
202
diff view generated by jsdifflib
1
From: Thomas Huth <thuth@redhat.com>
1
From: Thomas Huth <thuth@redhat.com>
2
2
3
The python code already contains a possibility to skip tests if the
3
Looks like we forgot to announce the deprecation of these options in
4
corresponding driver is not available in the qemu binary - use it
4
the corresponding chapter of the qemu-doc text, so let's do that now.
5
in more spots to avoid that the tests are failing if the driver has
6
been disabled.
7
8
While we're at it, we can now also remove some of the old checks that
9
were using iotests.supports_quorum() - and which were apparently not
10
working as expected since the tests aborted instead of being skipped
11
when "quorum" was missing in the QEMU binary.
12
5
13
Signed-off-by: Thomas Huth <thuth@redhat.com>
6
Signed-off-by: Thomas Huth <thuth@redhat.com>
7
Reviewed-by: John Snow <jsnow@redhat.com>
8
Reviewed-by: Markus Armbruster <armbru@redhat.com>
14
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
15
---
10
---
16
tests/qemu-iotests/030 | 4 +---
11
qemu-doc.texi | 15 +++++++++++++++
17
tests/qemu-iotests/040 | 2 ++
12
1 file changed, 15 insertions(+)
18
tests/qemu-iotests/041 | 39 +++------------------------------------
19
tests/qemu-iotests/245 | 2 ++
20
4 files changed, 8 insertions(+), 39 deletions(-)
21
13
22
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
14
diff --git a/qemu-doc.texi b/qemu-doc.texi
23
index XXXXXXX..XXXXXXX 100755
24
--- a/tests/qemu-iotests/030
25
+++ b/tests/qemu-iotests/030
26
@@ -XXX,XX +XXX,XX @@ class TestQuorum(iotests.QMPTestCase):
27
children = []
28
backing = []
29
30
+ @iotests.skip_if_unsupported(['quorum'])
31
def setUp(self):
32
opts = ['driver=quorum', 'vote-threshold=2']
33
34
@@ -XXX,XX +XXX,XX @@ class TestQuorum(iotests.QMPTestCase):
35
os.remove(img)
36
37
def test_stream_quorum(self):
38
- if not iotests.supports_quorum():
39
- return
40
-
41
self.assertNotEqual(qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.children[0]),
42
qemu_io('-f', iotests.imgfmt, '-rU', '-c', 'map', self.backing[0]),
43
'image file map matches backing file before streaming')
44
diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040
45
index XXXXXXX..XXXXXXX 100755
46
--- a/tests/qemu-iotests/040
47
+++ b/tests/qemu-iotests/040
48
@@ -XXX,XX +XXX,XX @@ class TestSingleDrive(ImageCommitTestCase):
49
self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xab 0 524288', backing_img).find("verification failed"))
50
self.assertEqual(-1, qemu_io('-f', 'raw', '-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed"))
51
52
+ @iotests.skip_if_unsupported(['throttle'])
53
def test_commit_with_filter_and_quit(self):
54
result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
55
self.assert_qmp(result, 'return', {})
56
@@ -XXX,XX +XXX,XX @@ class TestSingleDrive(ImageCommitTestCase):
57
self.has_quit = True
58
59
# Same as above, but this time we add the filter after starting the job
60
+ @iotests.skip_if_unsupported(['throttle'])
61
def test_commit_plus_filter_and_quit(self):
62
result = self.vm.qmp('object-add', qom_type='throttle-group', id='tg')
63
self.assert_qmp(result, 'return', {})
64
diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
65
index XXXXXXX..XXXXXXX 100755
66
--- a/tests/qemu-iotests/041
67
+++ b/tests/qemu-iotests/041
68
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
69
image_len = 1 * 1024 * 1024 # MB
70
IMAGES = [ quorum_img1, quorum_img2, quorum_img3 ]
71
72
+ @iotests.skip_if_unsupported(['quorum'])
73
def setUp(self):
74
self.vm = iotests.VM()
75
76
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
77
#assemble the quorum block device from the individual files
78
args = { "driver": "quorum", "node-name": "quorum0",
79
"vote-threshold": 2, "children": [ "img0", "img1", "img2" ] }
80
- if iotests.supports_quorum():
81
- result = self.vm.qmp("blockdev-add", **args)
82
- self.assert_qmp(result, 'return', {})
83
+ result = self.vm.qmp("blockdev-add", **args)
84
+ self.assert_qmp(result, 'return', {})
85
86
87
def tearDown(self):
88
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
89
pass
90
91
def test_complete(self):
92
- if not iotests.supports_quorum():
93
- return
94
-
95
self.assert_no_active_block_jobs()
96
97
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
98
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
99
'target image does not match source after mirroring')
100
101
def test_cancel(self):
102
- if not iotests.supports_quorum():
103
- return
104
-
105
self.assert_no_active_block_jobs()
106
107
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
108
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
109
self.vm.shutdown()
110
111
def test_cancel_after_ready(self):
112
- if not iotests.supports_quorum():
113
- return
114
-
115
self.assert_no_active_block_jobs()
116
117
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
118
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
119
'target image does not match source after mirroring')
120
121
def test_pause(self):
122
- if not iotests.supports_quorum():
123
- return
124
-
125
self.assert_no_active_block_jobs()
126
127
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
128
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
129
'target image does not match source after mirroring')
130
131
def test_medium_not_found(self):
132
- if not iotests.supports_quorum():
133
- return
134
-
135
if iotests.qemu_default_machine != 'pc':
136
return
137
138
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
139
self.assert_qmp(result, 'error/class', 'GenericError')
140
141
def test_image_not_found(self):
142
- if not iotests.supports_quorum():
143
- return
144
-
145
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
146
sync='full', node_name='repair0', replaces='img1',
147
mode='existing', target=quorum_repair_img,
148
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
149
self.assert_qmp(result, 'error/class', 'GenericError')
150
151
def test_device_not_found(self):
152
- if not iotests.supports_quorum():
153
- return
154
-
155
result = self.vm.qmp('drive-mirror', job_id='job0',
156
device='nonexistent', sync='full',
157
node_name='repair0',
158
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
159
self.assert_qmp(result, 'error/class', 'GenericError')
160
161
def test_wrong_sync_mode(self):
162
- if not iotests.supports_quorum():
163
- return
164
-
165
result = self.vm.qmp('drive-mirror', device='quorum0', job_id='job0',
166
node_name='repair0',
167
replaces='img1',
168
@@ -XXX,XX +XXX,XX @@ class TestRepairQuorum(iotests.QMPTestCase):
169
self.assert_qmp(result, 'error/class', 'GenericError')
170
171
def test_no_node_name(self):
172
- if not iotests.supports_quorum():
173
- return
174
-
175
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
176
sync='full', replaces='img1',
177
target=quorum_repair_img, format=iotests.imgfmt)
178
self.assert_qmp(result, 'error/class', 'GenericError')
179
180
def test_nonexistent_replaces(self):
181
- if not iotests.supports_quorum():
182
- return
183
-
184
result = self.vm.qmp('drive-mirror', job_id='job0', device='quorum0',
185
sync='full', node_name='repair0', replaces='img77',
186
target=quorum_repair_img, format=iotests.imgfmt)
187
self.assert_qmp(result, 'error/class', 'GenericError')
188
189
def test_after_a_quorum_snapshot(self):
190
- if not iotests.supports_quorum():
191
- return
192
-
193
result = self.vm.qmp('blockdev-snapshot-sync', node_name='img1',
194
snapshot_file=quorum_snapshot_file,
195
snapshot_node_name="snap1");
196
diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245
197
index XXXXXXX..XXXXXXX 100644
15
index XXXXXXX..XXXXXXX 100644
198
--- a/tests/qemu-iotests/245
16
--- a/qemu-doc.texi
199
+++ b/tests/qemu-iotests/245
17
+++ b/qemu-doc.texi
200
@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):
18
@@ -XXX,XX +XXX,XX @@ longer be directly supported in QEMU.
201
# This test verifies that we can't change the children of a block
19
The ``-drive if=scsi'' argument is replaced by the the
202
# device during a reopen operation in a way that would create
20
``-device BUS-TYPE'' argument combined with ``-drive if=none''.
203
# cycles in the node graph
21
204
+ @iotests.skip_if_unsupported(['blkverify'])
22
+@subsection -drive cyls=...,heads=...,secs=...,trans=... (since 2.10.0)
205
def test_graph_cycles(self):
23
+
206
opts = []
24
+The drive geometry arguments are replaced by the the geometry arguments
207
25
+that can be specified with the ``-device'' parameter.
208
@@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase):
26
+
209
self.assert_qmp(result, 'return', {})
27
+@subsection -drive serial=... (since 2.10.0)
210
28
+
211
# Misc reopen tests with different block drivers
29
+The drive serial argument is replaced by the the serial argument
212
+ @iotests.skip_if_unsupported(['quorum', 'throttle'])
30
+that can be specified with the ``-device'' parameter.
213
def test_misc_drivers(self):
31
+
214
####################
32
+@subsection -drive addr=... (since 2.10.0)
215
###### quorum ######
33
+
34
+The drive addr argument is replaced by the the addr argument
35
+that can be specified with the ``-device'' parameter.
36
+
37
@subsection -net dump (since 2.10.0)
38
39
The ``--net dump'' argument is now replaced with the
216
--
40
--
217
2.20.1
41
2.13.6
218
42
219
43
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
From: Fam Zheng <famz@redhat.com>
2
2
3
Issuing a drive-backup from qmp_drive_backup takes a slightly
3
Signed-off-by: Fam Zheng <famz@redhat.com>
4
different path than when it's issued from a transaction. In the code,
5
this is manifested as some redundancy between do_drive_backup() and
6
drive_backup_prepare().
7
8
This change unifies both paths, merging do_drive_backup() and
9
drive_backup_prepare(), and changing qmp_drive_backup() to create a
10
transaction instead of calling do_backup_common() direcly.
11
12
As a side-effect, now qmp_drive_backup() is executed inside a drained
13
section, as it happens when creating a drive-backup transaction. This
14
change is visible from the user's perspective, as the job gets paused
15
and immediately resumed before starting the actual work.
16
17
Also fix tests 141, 185 and 219 to cope with the extra
18
JOB_STATUS_CHANGE lines.
19
20
Signed-off-by: Sergio Lopez <slp@redhat.com>
21
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
22
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
4
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
23
---
5
---
24
blockdev.c | 224 +++++++++++++++++--------------------
6
include/block/block_int.h | 1 -
25
tests/qemu-iotests/141.out | 2 +
7
block/io.c | 18 ------------------
26
tests/qemu-iotests/185.out | 2 +
8
2 files changed, 19 deletions(-)
27
tests/qemu-iotests/219 | 7 +-
28
tests/qemu-iotests/219.out | 8 ++
29
5 files changed, 117 insertions(+), 126 deletions(-)
30
9
31
diff --git a/blockdev.c b/blockdev.c
10
diff --git a/include/block/block_int.h b/include/block/block_int.h
32
index XXXXXXX..XXXXXXX 100644
11
index XXXXXXX..XXXXXXX 100644
33
--- a/blockdev.c
12
--- a/include/block/block_int.h
34
+++ b/blockdev.c
13
+++ b/include/block/block_int.h
35
@@ -XXX,XX +XXX,XX @@ typedef struct DriveBackupState {
14
@@ -XXX,XX +XXX,XX @@ bool blk_dev_is_tray_open(BlockBackend *blk);
36
BlockJob *job;
15
bool blk_dev_is_medium_locked(BlockBackend *blk);
37
} DriveBackupState;
16
38
17
void bdrv_set_dirty(BlockDriverState *bs, int64_t offset, int64_t bytes);
39
-static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
18
-bool bdrv_requests_pending(BlockDriverState *bs);
40
- Error **errp);
19
41
+static BlockJob *do_backup_common(BackupCommon *backup,
20
void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap **out);
42
+ BlockDriverState *bs,
21
void bdrv_undo_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap, HBitmap *in);
43
+ BlockDriverState *target_bs,
22
diff --git a/block/io.c b/block/io.c
44
+ AioContext *aio_context,
23
index XXXXXXX..XXXXXXX 100644
45
+ JobTxn *txn, Error **errp);
24
--- a/block/io.c
46
25
+++ b/block/io.c
47
static void drive_backup_prepare(BlkActionState *common, Error **errp)
26
@@ -XXX,XX +XXX,XX @@ void bdrv_disable_copy_on_read(BlockDriverState *bs)
48
{
27
assert(old >= 1);
49
DriveBackupState *state = DO_UPCAST(DriveBackupState, common, common);
50
- BlockDriverState *bs;
51
DriveBackup *backup;
52
+ BlockDriverState *bs;
53
+ BlockDriverState *target_bs;
54
+ BlockDriverState *source = NULL;
55
AioContext *aio_context;
56
+ QDict *options;
57
Error *local_err = NULL;
58
+ int flags;
59
+ int64_t size;
60
+ bool set_backing_hd = false;
61
62
assert(common->action->type == TRANSACTION_ACTION_KIND_DRIVE_BACKUP);
63
backup = common->action->u.drive_backup.data;
64
65
+ if (!backup->has_mode) {
66
+ backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
67
+ }
68
+
69
bs = bdrv_lookup_bs(backup->device, backup->device, errp);
70
if (!bs) {
71
return;
72
}
73
74
+ if (!bs->drv) {
75
+ error_setg(errp, "Device has no medium");
76
+ return;
77
+ }
78
+
79
aio_context = bdrv_get_aio_context(bs);
80
aio_context_acquire(aio_context);
81
82
/* Paired with .clean() */
83
bdrv_drained_begin(bs);
84
85
- state->bs = bs;
86
+ if (!backup->has_format) {
87
+ backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
88
+ NULL : (char *) bs->drv->format_name;
89
+ }
90
+
91
+ /* Early check to avoid creating target */
92
+ if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
93
+ goto out;
94
+ }
95
+
96
+ flags = bs->open_flags | BDRV_O_RDWR;
97
+
98
+ /*
99
+ * See if we have a backing HD we can use to create our new image
100
+ * on top of.
101
+ */
102
+ if (backup->sync == MIRROR_SYNC_MODE_TOP) {
103
+ source = backing_bs(bs);
104
+ if (!source) {
105
+ backup->sync = MIRROR_SYNC_MODE_FULL;
106
+ }
107
+ }
108
+ if (backup->sync == MIRROR_SYNC_MODE_NONE) {
109
+ source = bs;
110
+ flags |= BDRV_O_NO_BACKING;
111
+ set_backing_hd = true;
112
+ }
113
+
114
+ size = bdrv_getlength(bs);
115
+ if (size < 0) {
116
+ error_setg_errno(errp, -size, "bdrv_getlength failed");
117
+ goto out;
118
+ }
119
+
120
+ if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
121
+ assert(backup->format);
122
+ if (source) {
123
+ bdrv_refresh_filename(source);
124
+ bdrv_img_create(backup->target, backup->format, source->filename,
125
+ source->drv->format_name, NULL,
126
+ size, flags, false, &local_err);
127
+ } else {
128
+ bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
129
+ size, flags, false, &local_err);
130
+ }
131
+ }
132
133
- state->job = do_drive_backup(backup, common->block_job_txn, &local_err);
134
if (local_err) {
135
error_propagate(errp, local_err);
136
goto out;
137
}
138
139
+ options = qdict_new();
140
+ qdict_put_str(options, "discard", "unmap");
141
+ qdict_put_str(options, "detect-zeroes", "unmap");
142
+ if (backup->format) {
143
+ qdict_put_str(options, "driver", backup->format);
144
+ }
145
+
146
+ target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
147
+ if (!target_bs) {
148
+ goto out;
149
+ }
150
+
151
+ if (set_backing_hd) {
152
+ bdrv_set_backing_hd(target_bs, source, &local_err);
153
+ if (local_err) {
154
+ goto unref;
155
+ }
156
+ }
157
+
158
+ state->bs = bs;
159
+
160
+ state->job = do_backup_common(qapi_DriveBackup_base(backup),
161
+ bs, target_bs, aio_context,
162
+ common->block_job_txn, errp);
163
+
164
+unref:
165
+ bdrv_unref(target_bs);
166
out:
167
aio_context_release(aio_context);
168
}
28
}
169
@@ -XXX,XX +XXX,XX @@ static BlockJob *do_backup_common(BackupCommon *backup,
29
170
return job;
30
-/* Check if any requests are in-flight (including throttled requests) */
171
}
31
-bool bdrv_requests_pending(BlockDriverState *bs)
172
173
-static BlockJob *do_drive_backup(DriveBackup *backup, JobTxn *txn,
174
- Error **errp)
175
-{
32
-{
176
- BlockDriverState *bs;
33
- BdrvChild *child;
177
- BlockDriverState *target_bs;
178
- BlockDriverState *source = NULL;
179
- BlockJob *job = NULL;
180
- AioContext *aio_context;
181
- QDict *options;
182
- Error *local_err = NULL;
183
- int flags;
184
- int64_t size;
185
- bool set_backing_hd = false;
186
-
34
-
187
- if (!backup->has_mode) {
35
- if (atomic_read(&bs->in_flight)) {
188
- backup->mode = NEW_IMAGE_MODE_ABSOLUTE_PATHS;
36
- return true;
189
- }
37
- }
190
-
38
-
191
- bs = bdrv_lookup_bs(backup->device, backup->device, errp);
39
- QLIST_FOREACH(child, &bs->children, next) {
192
- if (!bs) {
40
- if (bdrv_requests_pending(child->bs)) {
193
- return NULL;
41
- return true;
194
- }
195
-
196
- if (!bs->drv) {
197
- error_setg(errp, "Device has no medium");
198
- return NULL;
199
- }
200
-
201
- aio_context = bdrv_get_aio_context(bs);
202
- aio_context_acquire(aio_context);
203
-
204
- if (!backup->has_format) {
205
- backup->format = backup->mode == NEW_IMAGE_MODE_EXISTING ?
206
- NULL : (char *) bs->drv->format_name;
207
- }
208
-
209
- /* Early check to avoid creating target */
210
- if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_BACKUP_SOURCE, errp)) {
211
- goto out;
212
- }
213
-
214
- flags = bs->open_flags | BDRV_O_RDWR;
215
-
216
- /*
217
- * See if we have a backing HD we can use to create our new image
218
- * on top of.
219
- */
220
- if (backup->sync == MIRROR_SYNC_MODE_TOP) {
221
- source = backing_bs(bs);
222
- if (!source) {
223
- backup->sync = MIRROR_SYNC_MODE_FULL;
224
- }
225
- }
226
- if (backup->sync == MIRROR_SYNC_MODE_NONE) {
227
- source = bs;
228
- flags |= BDRV_O_NO_BACKING;
229
- set_backing_hd = true;
230
- }
231
-
232
- size = bdrv_getlength(bs);
233
- if (size < 0) {
234
- error_setg_errno(errp, -size, "bdrv_getlength failed");
235
- goto out;
236
- }
237
-
238
- if (backup->mode != NEW_IMAGE_MODE_EXISTING) {
239
- assert(backup->format);
240
- if (source) {
241
- bdrv_refresh_filename(source);
242
- bdrv_img_create(backup->target, backup->format, source->filename,
243
- source->drv->format_name, NULL,
244
- size, flags, false, &local_err);
245
- } else {
246
- bdrv_img_create(backup->target, backup->format, NULL, NULL, NULL,
247
- size, flags, false, &local_err);
248
- }
42
- }
249
- }
43
- }
250
-
44
-
251
- if (local_err) {
45
- return false;
252
- error_propagate(errp, local_err);
253
- goto out;
254
- }
255
-
256
- options = qdict_new();
257
- qdict_put_str(options, "discard", "unmap");
258
- qdict_put_str(options, "detect-zeroes", "unmap");
259
- if (backup->format) {
260
- qdict_put_str(options, "driver", backup->format);
261
- }
262
-
263
- target_bs = bdrv_open(backup->target, NULL, options, flags, errp);
264
- if (!target_bs) {
265
- goto out;
266
- }
267
-
268
- if (set_backing_hd) {
269
- bdrv_set_backing_hd(target_bs, source, &local_err);
270
- if (local_err) {
271
- goto unref;
272
- }
273
- }
274
-
275
- job = do_backup_common(qapi_DriveBackup_base(backup),
276
- bs, target_bs, aio_context, txn, errp);
277
-
278
-unref:
279
- bdrv_unref(target_bs);
280
-out:
281
- aio_context_release(aio_context);
282
- return job;
283
-}
46
-}
284
-
47
-
285
-void qmp_drive_backup(DriveBackup *arg, Error **errp)
48
typedef struct {
286
+void qmp_drive_backup(DriveBackup *backup, Error **errp)
49
Coroutine *co;
287
{
50
BlockDriverState *bs;
288
-
289
- BlockJob *job;
290
- job = do_drive_backup(arg, NULL, errp);
291
- if (job) {
292
- job_start(&job->job);
293
- }
294
+ TransactionAction action = {
295
+ .type = TRANSACTION_ACTION_KIND_DRIVE_BACKUP,
296
+ .u.drive_backup.data = backup,
297
+ };
298
+ blockdev_do_action(&action, errp);
299
}
300
301
BlockDeviceInfoList *qmp_query_named_block_nodes(Error **errp)
302
diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
303
index XXXXXXX..XXXXXXX 100644
304
--- a/tests/qemu-iotests/141.out
305
+++ b/tests/qemu-iotests/141.out
306
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/m.
307
Formatting 'TEST_DIR/o.IMGFMT', fmt=IMGFMT size=1048576 backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
308
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "job0"}}
309
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
310
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "job0"}}
311
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "job0"}}
312
{'execute': 'blockdev-del', 'arguments': {'node-name': 'drv0'}}
313
{"error": {"class": "GenericError", "desc": "Node 'drv0' is busy: node is used as backing hd of 'NODE_NAME'"}}
314
{'execute': 'block-job-cancel', 'arguments': {'device': 'job0'}}
315
diff --git a/tests/qemu-iotests/185.out b/tests/qemu-iotests/185.out
316
index XXXXXXX..XXXXXXX 100644
317
--- a/tests/qemu-iotests/185.out
318
+++ b/tests/qemu-iotests/185.out
319
@@ -XXX,XX +XXX,XX @@ Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 l
320
Formatting 'TEST_DIR/t.qcow2.copy', fmt=qcow2 size=67108864 cluster_size=65536 lazy_refcounts=off refcount_bits=16
321
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "created", "id": "disk"}}
322
{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "disk"}}
323
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "paused", "id": "disk"}}
324
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "JOB_STATUS_CHANGE", "data": {"status": "running", "id": "disk"}}
325
{"return": {}}
326
{ 'execute': 'quit' }
327
{"return": {}}
328
diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
329
index XXXXXXX..XXXXXXX 100755
330
--- a/tests/qemu-iotests/219
331
+++ b/tests/qemu-iotests/219
332
@@ -XXX,XX +XXX,XX @@ def test_pause_resume(vm):
333
# logged immediately
334
iotests.log(vm.qmp('query-jobs'))
335
336
-def test_job_lifecycle(vm, job, job_args, has_ready=False):
337
+def test_job_lifecycle(vm, job, job_args, has_ready=False, is_mirror=False):
338
global img_size
339
340
iotests.log('')
341
@@ -XXX,XX +XXX,XX @@ def test_job_lifecycle(vm, job, job_args, has_ready=False):
342
iotests.log('Waiting for PENDING state...')
343
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
344
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
345
+ if is_mirror:
346
+ iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
347
+ iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
348
349
if not job_args.get('auto-finalize', True):
350
# PENDING state:
351
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('disk.img') as disk_path, \
352
353
for auto_finalize in [True, False]:
354
for auto_dismiss in [True, False]:
355
- test_job_lifecycle(vm, 'drive-backup', job_args={
356
+ test_job_lifecycle(vm, 'drive-backup', is_mirror=True, job_args={
357
'device': 'drive0-node',
358
'target': copy_path,
359
'sync': 'full',
360
diff --git a/tests/qemu-iotests/219.out b/tests/qemu-iotests/219.out
361
index XXXXXXX..XXXXXXX 100644
362
--- a/tests/qemu-iotests/219.out
363
+++ b/tests/qemu-iotests/219.out
364
@@ -XXX,XX +XXX,XX @@ Pause/resume in RUNNING
365
{"return": {}}
366
367
Waiting for PENDING state...
368
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
369
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
370
{"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
371
{"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
372
{"data": {"id": "job0", "status": "concluded"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
373
@@ -XXX,XX +XXX,XX @@ Pause/resume in RUNNING
374
{"return": {}}
375
376
Waiting for PENDING state...
377
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
378
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
379
{"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
380
{"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
381
{"data": {"id": "job0", "status": "concluded"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
382
@@ -XXX,XX +XXX,XX @@ Pause/resume in RUNNING
383
{"return": {}}
384
385
Waiting for PENDING state...
386
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
387
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
388
{"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
389
{"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
390
{"return": [{"current-progress": 4194304, "id": "job0", "status": "pending", "total-progress": 4194304, "type": "backup"}]}
391
@@ -XXX,XX +XXX,XX @@ Pause/resume in RUNNING
392
{"return": {}}
393
394
Waiting for PENDING state...
395
+{"data": {"id": "job0", "status": "paused"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
396
+{"data": {"id": "job0", "status": "running"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
397
{"data": {"id": "job0", "status": "waiting"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
398
{"data": {"id": "job0", "status": "pending"}, "event": "JOB_STATUS_CHANGE", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}}
399
{"return": [{"current-progress": 4194304, "id": "job0", "status": "pending", "total-progress": 4194304, "type": "backup"}]}
400
--
51
--
401
2.20.1
52
2.13.6
402
53
403
54
diff view generated by jsdifflib
New patch
1
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
2
Reviewed-by: Fam Zheng <famz@redhat.com>
3
---
4
block/io.c | 6 ++++++
5
1 file changed, 6 insertions(+)
1
6
7
diff --git a/block/io.c b/block/io.c
8
index XXXXXXX..XXXXXXX 100644
9
--- a/block/io.c
10
+++ b/block/io.c
11
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
12
BdrvNextIterator it;
13
GSList *aio_ctxs = NULL, *ctx;
14
15
+ /* BDRV_POLL_WHILE() for a node can only be called from its own I/O thread
16
+ * or the main loop AioContext. We potentially use BDRV_POLL_WHILE() on
17
+ * nodes in several different AioContexts, so make sure we're in the main
18
+ * context. */
19
+ assert(qemu_get_current_aio_context() == qemu_get_aio_context());
20
+
21
block_job_pause_all();
22
23
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
24
--
25
2.13.6
26
27
diff view generated by jsdifflib
New patch
1
bdrv_drained_begin() doesn't increase bs->quiesce_counter recursively
2
and also doesn't notify other parent nodes of children, which both means
3
that the child nodes are not actually drained, and bdrv_drained_begin()
4
is providing useful functionality only on a single node.
1
5
6
To keep things consistent, we also shouldn't call the block driver
7
callbacks recursively.
8
9
A proper recursive drain version that provides an actually working
10
drained section for child nodes will be introduced later.
11
12
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
13
Reviewed-by: Fam Zheng <famz@redhat.com>
14
---
15
block/io.c | 16 +++++++++-------
16
1 file changed, 9 insertions(+), 7 deletions(-)
17
18
diff --git a/block/io.c b/block/io.c
19
index XXXXXXX..XXXXXXX 100644
20
--- a/block/io.c
21
+++ b/block/io.c
22
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
23
}
24
25
/* Recursively call BlockDriver.bdrv_co_drain_begin/end callbacks */
26
-static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
27
+static void bdrv_drain_invoke(BlockDriverState *bs, bool begin, bool recursive)
28
{
29
BdrvChild *child, *tmp;
30
BdrvCoDrainData data = { .bs = bs, .done = false, .begin = begin};
31
@@ -XXX,XX +XXX,XX @@ static void bdrv_drain_invoke(BlockDriverState *bs, bool begin)
32
bdrv_coroutine_enter(bs, data.co);
33
BDRV_POLL_WHILE(bs, !data.done);
34
35
- QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
36
- bdrv_drain_invoke(child->bs, begin);
37
+ if (recursive) {
38
+ QLIST_FOREACH_SAFE(child, &bs->children, next, tmp) {
39
+ bdrv_drain_invoke(child->bs, begin, true);
40
+ }
41
}
42
}
43
44
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
45
bdrv_parent_drained_begin(bs);
46
}
47
48
- bdrv_drain_invoke(bs, true);
49
+ bdrv_drain_invoke(bs, true, false);
50
bdrv_drain_recurse(bs);
51
}
52
53
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
54
}
55
56
/* Re-enable things in child-to-parent order */
57
- bdrv_drain_invoke(bs, false);
58
+ bdrv_drain_invoke(bs, false, false);
59
bdrv_parent_drained_end(bs);
60
aio_enable_external(bdrv_get_aio_context(bs));
61
}
62
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
63
aio_context_acquire(aio_context);
64
aio_disable_external(aio_context);
65
bdrv_parent_drained_begin(bs);
66
- bdrv_drain_invoke(bs, true);
67
+ bdrv_drain_invoke(bs, true, true);
68
aio_context_release(aio_context);
69
70
if (!g_slist_find(aio_ctxs, aio_context)) {
71
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
72
73
/* Re-enable things in child-to-parent order */
74
aio_context_acquire(aio_context);
75
- bdrv_drain_invoke(bs, false);
76
+ bdrv_drain_invoke(bs, false, true);
77
bdrv_parent_drained_end(bs);
78
aio_enable_external(aio_context);
79
aio_context_release(aio_context);
80
--
81
2.13.6
82
83
diff view generated by jsdifflib
New patch
1
The existing test is for bdrv_drain_all_begin/end() only. Generalise the
2
test case so that it can be run for the other variants as well. At the
3
moment this is only bdrv_drain_begin/end(), but in a while, we'll add
4
another one.
1
5
6
Also, add a backing file to the test node to test whether the operations
7
work recursively.
8
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
---
11
tests/test-bdrv-drain.c | 69 ++++++++++++++++++++++++++++++++++++++++++++-----
12
1 file changed, 62 insertions(+), 7 deletions(-)
13
14
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
15
index XXXXXXX..XXXXXXX 100644
16
--- a/tests/test-bdrv-drain.c
17
+++ b/tests/test-bdrv-drain.c
18
@@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_test = {
19
20
.bdrv_co_drain_begin = bdrv_test_co_drain_begin,
21
.bdrv_co_drain_end = bdrv_test_co_drain_end,
22
+
23
+ .bdrv_child_perm = bdrv_format_default_perms,
24
};
25
26
static void aio_ret_cb(void *opaque, int ret)
27
@@ -XXX,XX +XXX,XX @@ static void aio_ret_cb(void *opaque, int ret)
28
*aio_ret = ret;
29
}
30
31
-static void test_drv_cb_drain_all(void)
32
+enum drain_type {
33
+ BDRV_DRAIN_ALL,
34
+ BDRV_DRAIN,
35
+};
36
+
37
+static void do_drain_begin(enum drain_type drain_type, BlockDriverState *bs)
38
+{
39
+ switch (drain_type) {
40
+ case BDRV_DRAIN_ALL: bdrv_drain_all_begin(); break;
41
+ case BDRV_DRAIN: bdrv_drained_begin(bs); break;
42
+ default: g_assert_not_reached();
43
+ }
44
+}
45
+
46
+static void do_drain_end(enum drain_type drain_type, BlockDriverState *bs)
47
+{
48
+ switch (drain_type) {
49
+ case BDRV_DRAIN_ALL: bdrv_drain_all_end(); break;
50
+ case BDRV_DRAIN: bdrv_drained_end(bs); break;
51
+ default: g_assert_not_reached();
52
+ }
53
+}
54
+
55
+static void test_drv_cb_common(enum drain_type drain_type, bool recursive)
56
{
57
BlockBackend *blk;
58
- BlockDriverState *bs;
59
- BDRVTestState *s;
60
+ BlockDriverState *bs, *backing;
61
+ BDRVTestState *s, *backing_s;
62
BlockAIOCB *acb;
63
int aio_ret;
64
65
@@ -XXX,XX +XXX,XX @@ static void test_drv_cb_drain_all(void)
66
s = bs->opaque;
67
blk_insert_bs(blk, bs, &error_abort);
68
69
+ backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
70
+ backing_s = backing->opaque;
71
+ bdrv_set_backing_hd(bs, backing, &error_abort);
72
+
73
/* Simple bdrv_drain_all_begin/end pair, check that CBs are called */
74
g_assert_cmpint(s->drain_count, ==, 0);
75
- bdrv_drain_all_begin();
76
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
77
+
78
+ do_drain_begin(drain_type, bs);
79
+
80
g_assert_cmpint(s->drain_count, ==, 1);
81
- bdrv_drain_all_end();
82
+ g_assert_cmpint(backing_s->drain_count, ==, !!recursive);
83
+
84
+ do_drain_end(drain_type, bs);
85
+
86
g_assert_cmpint(s->drain_count, ==, 0);
87
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
88
89
/* Now do the same while a request is pending */
90
aio_ret = -EINPROGRESS;
91
@@ -XXX,XX +XXX,XX @@ static void test_drv_cb_drain_all(void)
92
g_assert_cmpint(aio_ret, ==, -EINPROGRESS);
93
94
g_assert_cmpint(s->drain_count, ==, 0);
95
- bdrv_drain_all_begin();
96
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
97
+
98
+ do_drain_begin(drain_type, bs);
99
+
100
g_assert_cmpint(aio_ret, ==, 0);
101
g_assert_cmpint(s->drain_count, ==, 1);
102
- bdrv_drain_all_end();
103
+ g_assert_cmpint(backing_s->drain_count, ==, !!recursive);
104
+
105
+ do_drain_end(drain_type, bs);
106
+
107
g_assert_cmpint(s->drain_count, ==, 0);
108
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
109
110
+ bdrv_unref(backing);
111
bdrv_unref(bs);
112
blk_unref(blk);
113
}
114
115
+static void test_drv_cb_drain_all(void)
116
+{
117
+ test_drv_cb_common(BDRV_DRAIN_ALL, true);
118
+}
119
+
120
+static void test_drv_cb_drain(void)
121
+{
122
+ test_drv_cb_common(BDRV_DRAIN, false);
123
+}
124
+
125
int main(int argc, char **argv)
126
{
127
bdrv_init();
128
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
129
g_test_init(&argc, &argv, NULL);
130
131
g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_all);
132
+ g_test_add_func("/bdrv-drain/driver-cb/drain", test_drv_cb_drain);
133
134
return g_test_run();
135
}
136
--
137
2.13.6
138
139
diff view generated by jsdifflib
New patch
1
This is currently only working correctly for bdrv_drain(), not for
2
bdrv_drain_all(). Leave a comment for the drain_all case, we'll address
3
it later.
1
4
5
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
6
---
7
tests/test-bdrv-drain.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
8
1 file changed, 45 insertions(+)
9
10
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
11
index XXXXXXX..XXXXXXX 100644
12
--- a/tests/test-bdrv-drain.c
13
+++ b/tests/test-bdrv-drain.c
14
@@ -XXX,XX +XXX,XX @@ static void test_drv_cb_drain(void)
15
test_drv_cb_common(BDRV_DRAIN, false);
16
}
17
18
+static void test_quiesce_common(enum drain_type drain_type, bool recursive)
19
+{
20
+ BlockBackend *blk;
21
+ BlockDriverState *bs, *backing;
22
+
23
+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
24
+ bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
25
+ &error_abort);
26
+ blk_insert_bs(blk, bs, &error_abort);
27
+
28
+ backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
29
+ bdrv_set_backing_hd(bs, backing, &error_abort);
30
+
31
+ g_assert_cmpint(bs->quiesce_counter, ==, 0);
32
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
33
+
34
+ do_drain_begin(drain_type, bs);
35
+
36
+ g_assert_cmpint(bs->quiesce_counter, ==, 1);
37
+ g_assert_cmpint(backing->quiesce_counter, ==, !!recursive);
38
+
39
+ do_drain_end(drain_type, bs);
40
+
41
+ g_assert_cmpint(bs->quiesce_counter, ==, 0);
42
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
43
+
44
+ bdrv_unref(backing);
45
+ bdrv_unref(bs);
46
+ blk_unref(blk);
47
+}
48
+
49
+static void test_quiesce_drain_all(void)
50
+{
51
+ // XXX drain_all doesn't quiesce
52
+ //test_quiesce_common(BDRV_DRAIN_ALL, true);
53
+}
54
+
55
+static void test_quiesce_drain(void)
56
+{
57
+ test_quiesce_common(BDRV_DRAIN, false);
58
+}
59
+
60
int main(int argc, char **argv)
61
{
62
bdrv_init();
63
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
64
g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_all);
65
g_test_add_func("/bdrv-drain/driver-cb/drain", test_drv_cb_drain);
66
67
+ g_test_add_func("/bdrv-drain/quiesce/drain_all", test_quiesce_drain_all);
68
+ g_test_add_func("/bdrv-drain/quiesce/drain", test_quiesce_drain);
69
+
70
return g_test_run();
71
}
72
--
73
2.13.6
74
75
diff view generated by jsdifflib
New patch
1
Block jobs already paused themselves when their main BlockBackend
2
entered a drained section. This is not good enough: We also want to
3
pause a block job and may not submit new requests if, for example, the
4
mirror target node should be drained.
1
5
6
This implements .drained_begin/end callbacks in child_job in order to
7
consider all block nodes related to the job, and removes the
8
BlockBackend callbacks which are unnecessary now because the root of the
9
job main BlockBackend is always referenced with a child_job, too.
10
11
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
12
---
13
blockjob.c | 22 +++++++++-------------
14
1 file changed, 9 insertions(+), 13 deletions(-)
15
16
diff --git a/blockjob.c b/blockjob.c
17
index XXXXXXX..XXXXXXX 100644
18
--- a/blockjob.c
19
+++ b/blockjob.c
20
@@ -XXX,XX +XXX,XX @@ static char *child_job_get_parent_desc(BdrvChild *c)
21
job->id);
22
}
23
24
-static const BdrvChildRole child_job = {
25
- .get_parent_desc = child_job_get_parent_desc,
26
- .stay_at_node = true,
27
-};
28
-
29
-static void block_job_drained_begin(void *opaque)
30
+static void child_job_drained_begin(BdrvChild *c)
31
{
32
- BlockJob *job = opaque;
33
+ BlockJob *job = c->opaque;
34
block_job_pause(job);
35
}
36
37
-static void block_job_drained_end(void *opaque)
38
+static void child_job_drained_end(BdrvChild *c)
39
{
40
- BlockJob *job = opaque;
41
+ BlockJob *job = c->opaque;
42
block_job_resume(job);
43
}
44
45
-static const BlockDevOps block_job_dev_ops = {
46
- .drained_begin = block_job_drained_begin,
47
- .drained_end = block_job_drained_end,
48
+static const BdrvChildRole child_job = {
49
+ .get_parent_desc = child_job_get_parent_desc,
50
+ .drained_begin = child_job_drained_begin,
51
+ .drained_end = child_job_drained_end,
52
+ .stay_at_node = true,
53
};
54
55
void block_job_remove_all_bdrv(BlockJob *job)
56
@@ -XXX,XX +XXX,XX @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
57
block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort);
58
bs->job = job;
59
60
- blk_set_dev_ops(blk, &block_job_dev_ops, job);
61
bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
62
63
QLIST_INSERT_HEAD(&block_jobs, job, job_list);
64
--
65
2.13.6
66
67
diff view generated by jsdifflib
New patch
1
Block jobs must be paused if any of the involved nodes are drained.
1
2
3
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
4
---
5
tests/test-bdrv-drain.c | 121 ++++++++++++++++++++++++++++++++++++++++++++++++
6
1 file changed, 121 insertions(+)
7
8
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
9
index XXXXXXX..XXXXXXX 100644
10
--- a/tests/test-bdrv-drain.c
11
+++ b/tests/test-bdrv-drain.c
12
@@ -XXX,XX +XXX,XX @@
13
14
#include "qemu/osdep.h"
15
#include "block/block.h"
16
+#include "block/blockjob_int.h"
17
#include "sysemu/block-backend.h"
18
#include "qapi/error.h"
19
20
@@ -XXX,XX +XXX,XX @@ static void test_quiesce_drain(void)
21
test_quiesce_common(BDRV_DRAIN, false);
22
}
23
24
+
25
+typedef struct TestBlockJob {
26
+ BlockJob common;
27
+ bool should_complete;
28
+} TestBlockJob;
29
+
30
+static void test_job_completed(BlockJob *job, void *opaque)
31
+{
32
+ block_job_completed(job, 0);
33
+}
34
+
35
+static void coroutine_fn test_job_start(void *opaque)
36
+{
37
+ TestBlockJob *s = opaque;
38
+
39
+ while (!s->should_complete) {
40
+ block_job_sleep_ns(&s->common, 100000);
41
+ }
42
+
43
+ block_job_defer_to_main_loop(&s->common, test_job_completed, NULL);
44
+}
45
+
46
+static void test_job_complete(BlockJob *job, Error **errp)
47
+{
48
+ TestBlockJob *s = container_of(job, TestBlockJob, common);
49
+ s->should_complete = true;
50
+}
51
+
52
+BlockJobDriver test_job_driver = {
53
+ .instance_size = sizeof(TestBlockJob),
54
+ .start = test_job_start,
55
+ .complete = test_job_complete,
56
+};
57
+
58
+static void test_blockjob_common(enum drain_type drain_type)
59
+{
60
+ BlockBackend *blk_src, *blk_target;
61
+ BlockDriverState *src, *target;
62
+ BlockJob *job;
63
+ int ret;
64
+
65
+ src = bdrv_new_open_driver(&bdrv_test, "source", BDRV_O_RDWR,
66
+ &error_abort);
67
+ blk_src = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
68
+ blk_insert_bs(blk_src, src, &error_abort);
69
+
70
+ target = bdrv_new_open_driver(&bdrv_test, "target", BDRV_O_RDWR,
71
+ &error_abort);
72
+ blk_target = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
73
+ blk_insert_bs(blk_target, target, &error_abort);
74
+
75
+ job = block_job_create("job0", &test_job_driver, src, 0, BLK_PERM_ALL, 0,
76
+ 0, NULL, NULL, &error_abort);
77
+ block_job_add_bdrv(job, "target", target, 0, BLK_PERM_ALL, &error_abort);
78
+ block_job_start(job);
79
+
80
+ g_assert_cmpint(job->pause_count, ==, 0);
81
+ g_assert_false(job->paused);
82
+ g_assert_false(job->busy); /* We're in block_job_sleep_ns() */
83
+
84
+ do_drain_begin(drain_type, src);
85
+
86
+ if (drain_type == BDRV_DRAIN_ALL) {
87
+ /* bdrv_drain_all() drains both src and target, and involves an
88
+ * additional block_job_pause_all() */
89
+ g_assert_cmpint(job->pause_count, ==, 3);
90
+ } else {
91
+ g_assert_cmpint(job->pause_count, ==, 1);
92
+ }
93
+ /* XXX We don't wait until the job is actually paused. Is this okay? */
94
+ /* g_assert_true(job->paused); */
95
+ g_assert_false(job->busy); /* The job is paused */
96
+
97
+ do_drain_end(drain_type, src);
98
+
99
+ g_assert_cmpint(job->pause_count, ==, 0);
100
+ g_assert_false(job->paused);
101
+ g_assert_false(job->busy); /* We're in block_job_sleep_ns() */
102
+
103
+ do_drain_begin(drain_type, target);
104
+
105
+ if (drain_type == BDRV_DRAIN_ALL) {
106
+ /* bdrv_drain_all() drains both src and target, and involves an
107
+ * additional block_job_pause_all() */
108
+ g_assert_cmpint(job->pause_count, ==, 3);
109
+ } else {
110
+ g_assert_cmpint(job->pause_count, ==, 1);
111
+ }
112
+ /* XXX We don't wait until the job is actually paused. Is this okay? */
113
+ /* g_assert_true(job->paused); */
114
+ g_assert_false(job->busy); /* The job is paused */
115
+
116
+ do_drain_end(drain_type, target);
117
+
118
+ g_assert_cmpint(job->pause_count, ==, 0);
119
+ g_assert_false(job->paused);
120
+ g_assert_false(job->busy); /* We're in block_job_sleep_ns() */
121
+
122
+ ret = block_job_complete_sync(job, &error_abort);
123
+ g_assert_cmpint(ret, ==, 0);
124
+
125
+ blk_unref(blk_src);
126
+ blk_unref(blk_target);
127
+ bdrv_unref(src);
128
+ bdrv_unref(target);
129
+}
130
+
131
+static void test_blockjob_drain_all(void)
132
+{
133
+ test_blockjob_common(BDRV_DRAIN_ALL);
134
+}
135
+
136
+static void test_blockjob_drain(void)
137
+{
138
+ test_blockjob_common(BDRV_DRAIN);
139
+}
140
+
141
int main(int argc, char **argv)
142
{
143
bdrv_init();
144
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
145
g_test_add_func("/bdrv-drain/quiesce/drain_all", test_quiesce_drain_all);
146
g_test_add_func("/bdrv-drain/quiesce/drain", test_quiesce_drain);
147
148
+ g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
149
+ g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain);
150
+
151
return g_test_run();
152
}
153
--
154
2.13.6
155
156
diff view generated by jsdifflib
1
In iscsi_co_block_status(), we may have received num_descriptors == 0
1
Block jobs are already paused using the BdrvChildRole drain callbacks,
2
from the iscsi server. Therefore, we can't unconditionally access
2
so we don't need an additional block_job_pause_all() call.
3
lbas->descriptors[0]. Add the missing check.
4
3
5
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
4
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
6
Reviewed-by: Felipe Franciosi <felipe@nutanix.com>
7
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
8
Reviewed-by: John Snow <jsnow@redhat.com>
9
Reviewed-by: Peter Lieven <pl@kamp.de>
10
---
5
---
11
block/iscsi.c | 2 +-
6
block/io.c | 4 ----
12
1 file changed, 1 insertion(+), 1 deletion(-)
7
tests/test-bdrv-drain.c | 10 ++++------
8
2 files changed, 4 insertions(+), 10 deletions(-)
13
9
14
diff --git a/block/iscsi.c b/block/iscsi.c
10
diff --git a/block/io.c b/block/io.c
15
index XXXXXXX..XXXXXXX 100644
11
index XXXXXXX..XXXXXXX 100644
16
--- a/block/iscsi.c
12
--- a/block/io.c
17
+++ b/block/iscsi.c
13
+++ b/block/io.c
18
@@ -XXX,XX +XXX,XX @@ retry:
14
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
15
* context. */
16
assert(qemu_get_current_aio_context() == qemu_get_aio_context());
17
18
- block_job_pause_all();
19
-
20
for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
21
AioContext *aio_context = bdrv_get_aio_context(bs);
22
23
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
24
aio_enable_external(aio_context);
25
aio_context_release(aio_context);
19
}
26
}
20
27
-
21
lbas = scsi_datain_unmarshall(iTask.task);
28
- block_job_resume_all();
22
- if (lbas == NULL) {
29
}
23
+ if (lbas == NULL || lbas->num_descriptors == 0) {
30
24
ret = -EIO;
31
void bdrv_drain_all(void)
25
goto out_unlock;
32
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
33
index XXXXXXX..XXXXXXX 100644
34
--- a/tests/test-bdrv-drain.c
35
+++ b/tests/test-bdrv-drain.c
36
@@ -XXX,XX +XXX,XX @@ static void test_blockjob_common(enum drain_type drain_type)
37
do_drain_begin(drain_type, src);
38
39
if (drain_type == BDRV_DRAIN_ALL) {
40
- /* bdrv_drain_all() drains both src and target, and involves an
41
- * additional block_job_pause_all() */
42
- g_assert_cmpint(job->pause_count, ==, 3);
43
+ /* bdrv_drain_all() drains both src and target */
44
+ g_assert_cmpint(job->pause_count, ==, 2);
45
} else {
46
g_assert_cmpint(job->pause_count, ==, 1);
47
}
48
@@ -XXX,XX +XXX,XX @@ static void test_blockjob_common(enum drain_type drain_type)
49
do_drain_begin(drain_type, target);
50
51
if (drain_type == BDRV_DRAIN_ALL) {
52
- /* bdrv_drain_all() drains both src and target, and involves an
53
- * additional block_job_pause_all() */
54
- g_assert_cmpint(job->pause_count, ==, 3);
55
+ /* bdrv_drain_all() drains both src and target */
56
+ g_assert_cmpint(job->pause_count, ==, 2);
57
} else {
58
g_assert_cmpint(job->pause_count, ==, 1);
26
}
59
}
27
--
60
--
28
2.20.1
61
2.13.6
29
62
30
63
diff view generated by jsdifflib
New patch
1
bdrv_do_drained_begin() restricts the call of parent callbacks and
2
aio_disable_external() to the outermost drain section, but the block
3
driver callbacks are always called. bdrv_do_drained_end() must match
4
this behaviour, otherwise nodes stay drained even if begin/end calls
5
were balanced.
1
6
7
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
8
---
9
block/io.c | 12 +++++++-----
10
1 file changed, 7 insertions(+), 5 deletions(-)
11
12
diff --git a/block/io.c b/block/io.c
13
index XXXXXXX..XXXXXXX 100644
14
--- a/block/io.c
15
+++ b/block/io.c
16
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs)
17
18
void bdrv_drained_end(BlockDriverState *bs)
19
{
20
+ int old_quiesce_counter;
21
+
22
if (qemu_in_coroutine()) {
23
bdrv_co_yield_to_drain(bs, false);
24
return;
25
}
26
assert(bs->quiesce_counter > 0);
27
- if (atomic_fetch_dec(&bs->quiesce_counter) > 1) {
28
- return;
29
- }
30
+ old_quiesce_counter = atomic_fetch_dec(&bs->quiesce_counter);
31
32
/* Re-enable things in child-to-parent order */
33
bdrv_drain_invoke(bs, false, false);
34
- bdrv_parent_drained_end(bs);
35
- aio_enable_external(bdrv_get_aio_context(bs));
36
+ if (old_quiesce_counter == 1) {
37
+ bdrv_parent_drained_end(bs);
38
+ aio_enable_external(bdrv_get_aio_context(bs));
39
+ }
40
}
41
42
/*
43
--
44
2.13.6
45
46
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
2
3
external_snapshot_abort() calls to bdrv_set_backing_hd(), which
4
returns state->old_bs to the main AioContext, as it's intended to be
5
used then the BDS is going to be released. As that's not the case when
6
aborting an external snapshot, return it to the AioContext it was
7
before the call.
8
9
This issue can be triggered by issuing a transaction with two actions,
10
a proper blockdev-snapshot-sync and a bogus one, so the second will
11
trigger a transaction abort. This results in a crash with an stack
12
trace like this one:
13
14
#0 0x00007fa1048b28df in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
15
#1 0x00007fa10489ccf5 in __GI_abort () at abort.c:79
16
#2 0x00007fa10489cbc9 in __assert_fail_base
17
(fmt=0x7fa104a03300 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs)", file=0x557224014d30 "block.c", line=2240, function=<optimized out>) at assert.c:92
18
#3 0x00007fa1048aae96 in __GI___assert_fail
19
(assertion=assertion@entry=0x5572240b44d8 "bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs)", file=file@entry=0x557224014d30 "block.c", line=line@entry=2240, function=function@entry=0x5572240b5d60 <__PRETTY_FUNCTION__.31620> "bdrv_replace_child_noperm") at assert.c:101
20
#4 0x0000557223e631f8 in bdrv_replace_child_noperm (child=0x557225b9c980, new_bs=new_bs@entry=0x557225c42e40) at block.c:2240
21
#5 0x0000557223e68be7 in bdrv_replace_node (from=0x557226951a60, to=0x557225c42e40, errp=0x5572247d6138 <error_abort>) at block.c:4196
22
#6 0x0000557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at blockdev.c:1731
23
#7 0x0000557223d069c4 in external_snapshot_abort (common=0x557225d7e170) at blockdev.c:1717
24
#8 0x0000557223d09013 in qmp_transaction (dev_list=<optimized out>, has_props=<optimized out>, props=0x557225cc7d70, errp=errp@entry=0x7ffe704c0c98) at blockdev.c:2360
25
#9 0x0000557223e32085 in qmp_marshal_transaction (args=<optimized out>, ret=<optimized out>, errp=0x7ffe704c0d08) at qapi/qapi-commands-transaction.c:44
26
#10 0x0000557223ee798c in do_qmp_dispatch (errp=0x7ffe704c0d00, allow_oob=<optimized out>, request=<optimized out>, cmds=0x5572247d3cc0 <qmp_commands>) at qapi/qmp-dispatch.c:132
27
#11 0x0000557223ee798c in qmp_dispatch (cmds=0x5572247d3cc0 <qmp_commands>, request=<optimized out>, allow_oob=<optimized out>) at qapi/qmp-dispatch.c:175
28
#12 0x0000557223e06141 in monitor_qmp_dispatch (mon=0x557225c69ff0, req=<optimized out>) at monitor/qmp.c:120
29
#13 0x0000557223e0678a in monitor_qmp_bh_dispatcher (data=<optimized out>) at monitor/qmp.c:209
30
#14 0x0000557223f2f366 in aio_bh_call (bh=0x557225b9dc60) at util/async.c:117
31
#15 0x0000557223f2f366 in aio_bh_poll (ctx=ctx@entry=0x557225b9c840) at util/async.c:117
32
#16 0x0000557223f32754 in aio_dispatch (ctx=0x557225b9c840) at util/aio-posix.c:459
33
#17 0x0000557223f2f242 in aio_ctx_dispatch (source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:260
34
#18 0x00007fa10913467d in g_main_dispatch (context=0x557225c28e80) at gmain.c:3176
35
#19 0x00007fa10913467d in g_main_context_dispatch (context=context@entry=0x557225c28e80) at gmain.c:3829
36
#20 0x0000557223f31808 in glib_pollfds_poll () at util/main-loop.c:219
37
#21 0x0000557223f31808 in os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:242
38
#22 0x0000557223f31808 in main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:518
39
#23 0x0000557223d13201 in main_loop () at vl.c:1828
40
#24 0x0000557223bbfb82 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>) at vl.c:4504
41
42
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1779036
43
Signed-off-by: Sergio Lopez <slp@redhat.com>
44
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
1
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
45
---
2
---
46
blockdev.c | 21 +++++++++++++++++++++
3
tests/test-bdrv-drain.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
47
1 file changed, 21 insertions(+)
4
1 file changed, 57 insertions(+)
48
5
49
diff --git a/blockdev.c b/blockdev.c
6
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
50
index XXXXXXX..XXXXXXX 100644
7
index XXXXXXX..XXXXXXX 100644
51
--- a/blockdev.c
8
--- a/tests/test-bdrv-drain.c
52
+++ b/blockdev.c
9
+++ b/tests/test-bdrv-drain.c
53
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common)
10
@@ -XXX,XX +XXX,XX @@ static void aio_ret_cb(void *opaque, int ret)
54
if (state->new_bs) {
11
enum drain_type {
55
if (state->overlay_appended) {
12
BDRV_DRAIN_ALL,
56
AioContext *aio_context;
13
BDRV_DRAIN,
57
+ AioContext *tmp_context;
14
+ DRAIN_TYPE_MAX,
58
+ int ret;
15
};
59
16
60
aio_context = bdrv_get_aio_context(state->old_bs);
17
static void do_drain_begin(enum drain_type drain_type, BlockDriverState *bs)
61
aio_context_acquire(aio_context);
18
@@ -XXX,XX +XXX,XX @@ static void test_quiesce_drain(void)
62
@@ -XXX,XX +XXX,XX @@ static void external_snapshot_abort(BlkActionState *common)
19
test_quiesce_common(BDRV_DRAIN, false);
63
bdrv_ref(state->old_bs); /* we can't let bdrv_set_backind_hd()
20
}
64
close state->old_bs; we need it */
21
65
bdrv_set_backing_hd(state->new_bs, NULL, &error_abort);
22
+static void test_nested(void)
23
+{
24
+ BlockBackend *blk;
25
+ BlockDriverState *bs, *backing;
26
+ BDRVTestState *s, *backing_s;
27
+ enum drain_type outer, inner;
66
+
28
+
67
+ /*
29
+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
68
+ * The call to bdrv_set_backing_hd() above returns state->old_bs to
30
+ bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
69
+ * the main AioContext. As we're still going to be using it, return
31
+ &error_abort);
70
+ * it to the AioContext it was before.
32
+ s = bs->opaque;
71
+ */
33
+ blk_insert_bs(blk, bs, &error_abort);
72
+ tmp_context = bdrv_get_aio_context(state->old_bs);
73
+ if (aio_context != tmp_context) {
74
+ aio_context_release(aio_context);
75
+ aio_context_acquire(tmp_context);
76
+
34
+
77
+ ret = bdrv_try_set_aio_context(state->old_bs,
35
+ backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
78
+ aio_context, NULL);
36
+ backing_s = backing->opaque;
79
+ assert(ret == 0);
37
+ bdrv_set_backing_hd(bs, backing, &error_abort);
80
+
38
+
81
+ aio_context_release(tmp_context);
39
+ for (outer = 0; outer < DRAIN_TYPE_MAX; outer++) {
82
+ aio_context_acquire(aio_context);
40
+ for (inner = 0; inner < DRAIN_TYPE_MAX; inner++) {
83
+ }
41
+ /* XXX bdrv_drain_all() doesn't increase the quiesce_counter */
42
+ int bs_quiesce = (outer != BDRV_DRAIN_ALL) +
43
+ (inner != BDRV_DRAIN_ALL);
44
+ int backing_quiesce = 0;
45
+ int backing_cb_cnt = (outer != BDRV_DRAIN) +
46
+ (inner != BDRV_DRAIN);
84
+
47
+
85
bdrv_replace_node(state->new_bs, state->old_bs, &error_abort);
48
+ g_assert_cmpint(bs->quiesce_counter, ==, 0);
86
bdrv_unref(state->old_bs); /* bdrv_replace_node() ref'ed old_bs */
49
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
50
+ g_assert_cmpint(s->drain_count, ==, 0);
51
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
52
+
53
+ do_drain_begin(outer, bs);
54
+ do_drain_begin(inner, bs);
55
+
56
+ g_assert_cmpint(bs->quiesce_counter, ==, bs_quiesce);
57
+ g_assert_cmpint(backing->quiesce_counter, ==, backing_quiesce);
58
+ g_assert_cmpint(s->drain_count, ==, 2);
59
+ g_assert_cmpint(backing_s->drain_count, ==, backing_cb_cnt);
60
+
61
+ do_drain_end(inner, bs);
62
+ do_drain_end(outer, bs);
63
+
64
+ g_assert_cmpint(bs->quiesce_counter, ==, 0);
65
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
66
+ g_assert_cmpint(s->drain_count, ==, 0);
67
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
68
+ }
69
+ }
70
+
71
+ bdrv_unref(backing);
72
+ bdrv_unref(bs);
73
+ blk_unref(blk);
74
+}
75
+
76
77
typedef struct TestBlockJob {
78
BlockJob common;
79
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
80
g_test_add_func("/bdrv-drain/quiesce/drain_all", test_quiesce_drain_all);
81
g_test_add_func("/bdrv-drain/quiesce/drain", test_quiesce_drain);
82
83
+ g_test_add_func("/bdrv-drain/nested", test_nested);
84
+
85
g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
86
g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain);
87
87
88
--
88
--
89
2.20.1
89
2.13.6
90
90
91
91
diff view generated by jsdifflib
New patch
1
1
This is in preparation for subtree drains, i.e. drained sections that
2
affect not only a single node, but recursively all child nodes, too.
3
4
Calling the parent callbacks for drain is pointless when we just came
5
from that parent node recursively and leads to multiple increases of
6
bs->quiesce_counter in a single drain call. Don't do it.
7
8
In order for this to work correctly, the parent callback must be called
9
for every bdrv_drain_begin/end() call, not only for the outermost one:
10
11
If we have a node N with two parents A and B, recursive draining of A
12
should cause the quiesce_counter of B to increase because its child N is
13
drained independently of B. If now B is recursively drained, too, A must
14
increase its quiesce_counter because N is drained independently of A
15
only now, even if N is going from quiesce_counter 1 to 2.
16
17
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
18
---
19
include/block/block.h | 4 ++--
20
block.c | 13 +++++++++----
21
block/io.c | 47 ++++++++++++++++++++++++++++++++++-------------
22
3 files changed, 45 insertions(+), 19 deletions(-)
23
24
diff --git a/include/block/block.h b/include/block/block.h
25
index XXXXXXX..XXXXXXX 100644
26
--- a/include/block/block.h
27
+++ b/include/block/block.h
28
@@ -XXX,XX +XXX,XX @@ void bdrv_io_unplug(BlockDriverState *bs);
29
* Begin a quiesced section of all users of @bs. This is part of
30
* bdrv_drained_begin.
31
*/
32
-void bdrv_parent_drained_begin(BlockDriverState *bs);
33
+void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore);
34
35
/**
36
* bdrv_parent_drained_end:
37
@@ -XXX,XX +XXX,XX @@ void bdrv_parent_drained_begin(BlockDriverState *bs);
38
* End a quiesced section of all users of @bs. This is part of
39
* bdrv_drained_end.
40
*/
41
-void bdrv_parent_drained_end(BlockDriverState *bs);
42
+void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore);
43
44
/**
45
* bdrv_drained_begin:
46
diff --git a/block.c b/block.c
47
index XXXXXXX..XXXXXXX 100644
48
--- a/block.c
49
+++ b/block.c
50
@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child_noperm(BdrvChild *child,
51
BlockDriverState *new_bs)
52
{
53
BlockDriverState *old_bs = child->bs;
54
+ int i;
55
56
if (old_bs && new_bs) {
57
assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
58
}
59
if (old_bs) {
60
if (old_bs->quiesce_counter && child->role->drained_end) {
61
- child->role->drained_end(child);
62
+ for (i = 0; i < old_bs->quiesce_counter; i++) {
63
+ child->role->drained_end(child);
64
+ }
65
}
66
if (child->role->detach) {
67
child->role->detach(child);
68
@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child_noperm(BdrvChild *child,
69
if (new_bs) {
70
QLIST_INSERT_HEAD(&new_bs->parents, child, next_parent);
71
if (new_bs->quiesce_counter && child->role->drained_begin) {
72
- child->role->drained_begin(child);
73
+ for (i = 0; i < new_bs->quiesce_counter; i++) {
74
+ child->role->drained_begin(child);
75
+ }
76
}
77
78
if (child->role->attach) {
79
@@ -XXX,XX +XXX,XX @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
80
AioContext *ctx = bdrv_get_aio_context(bs);
81
82
aio_disable_external(ctx);
83
- bdrv_parent_drained_begin(bs);
84
+ bdrv_parent_drained_begin(bs, NULL);
85
bdrv_drain(bs); /* ensure there are no in-flight requests */
86
87
while (aio_poll(ctx, false)) {
88
@@ -XXX,XX +XXX,XX @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
89
*/
90
aio_context_acquire(new_context);
91
bdrv_attach_aio_context(bs, new_context);
92
- bdrv_parent_drained_end(bs);
93
+ bdrv_parent_drained_end(bs, NULL);
94
aio_enable_external(ctx);
95
aio_context_release(new_context);
96
}
97
diff --git a/block/io.c b/block/io.c
98
index XXXXXXX..XXXXXXX 100644
99
--- a/block/io.c
100
+++ b/block/io.c
101
@@ -XXX,XX +XXX,XX @@
102
static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
103
int64_t offset, int bytes, BdrvRequestFlags flags);
104
105
-void bdrv_parent_drained_begin(BlockDriverState *bs)
106
+void bdrv_parent_drained_begin(BlockDriverState *bs, BdrvChild *ignore)
107
{
108
BdrvChild *c, *next;
109
110
QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) {
111
+ if (c == ignore) {
112
+ continue;
113
+ }
114
if (c->role->drained_begin) {
115
c->role->drained_begin(c);
116
}
117
}
118
}
119
120
-void bdrv_parent_drained_end(BlockDriverState *bs)
121
+void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore)
122
{
123
BdrvChild *c, *next;
124
125
QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) {
126
+ if (c == ignore) {
127
+ continue;
128
+ }
129
if (c->role->drained_end) {
130
c->role->drained_end(c);
131
}
132
@@ -XXX,XX +XXX,XX @@ typedef struct {
133
BlockDriverState *bs;
134
bool done;
135
bool begin;
136
+ BdrvChild *parent;
137
} BdrvCoDrainData;
138
139
static void coroutine_fn bdrv_drain_invoke_entry(void *opaque)
140
@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs)
141
return waited;
142
}
143
144
+static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent);
145
+static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent);
146
+
147
static void bdrv_co_drain_bh_cb(void *opaque)
148
{
149
BdrvCoDrainData *data = opaque;
150
@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
151
152
bdrv_dec_in_flight(bs);
153
if (data->begin) {
154
- bdrv_drained_begin(bs);
155
+ bdrv_do_drained_begin(bs, data->parent);
156
} else {
157
- bdrv_drained_end(bs);
158
+ bdrv_do_drained_end(bs, data->parent);
159
}
160
161
data->done = true;
162
@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
163
}
164
165
static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
166
- bool begin)
167
+ bool begin, BdrvChild *parent)
168
{
169
BdrvCoDrainData data;
170
171
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
172
.bs = bs,
173
.done = false,
174
.begin = begin,
175
+ .parent = parent,
176
};
177
bdrv_inc_in_flight(bs);
178
aio_bh_schedule_oneshot(bdrv_get_aio_context(bs),
179
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
180
assert(data.done);
181
}
182
183
-void bdrv_drained_begin(BlockDriverState *bs)
184
+static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent)
185
{
186
if (qemu_in_coroutine()) {
187
- bdrv_co_yield_to_drain(bs, true);
188
+ bdrv_co_yield_to_drain(bs, true, parent);
189
return;
190
}
191
192
/* Stop things in parent-to-child order */
193
if (atomic_fetch_inc(&bs->quiesce_counter) == 0) {
194
aio_disable_external(bdrv_get_aio_context(bs));
195
- bdrv_parent_drained_begin(bs);
196
}
197
198
+ bdrv_parent_drained_begin(bs, parent);
199
bdrv_drain_invoke(bs, true, false);
200
bdrv_drain_recurse(bs);
201
}
202
203
-void bdrv_drained_end(BlockDriverState *bs)
204
+void bdrv_drained_begin(BlockDriverState *bs)
205
+{
206
+ bdrv_do_drained_begin(bs, NULL);
207
+}
208
+
209
+static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
210
{
211
int old_quiesce_counter;
212
213
if (qemu_in_coroutine()) {
214
- bdrv_co_yield_to_drain(bs, false);
215
+ bdrv_co_yield_to_drain(bs, false, parent);
216
return;
217
}
218
assert(bs->quiesce_counter > 0);
219
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_end(BlockDriverState *bs)
220
221
/* Re-enable things in child-to-parent order */
222
bdrv_drain_invoke(bs, false, false);
223
+ bdrv_parent_drained_end(bs, parent);
224
if (old_quiesce_counter == 1) {
225
- bdrv_parent_drained_end(bs);
226
aio_enable_external(bdrv_get_aio_context(bs));
227
}
228
}
229
230
+void bdrv_drained_end(BlockDriverState *bs)
231
+{
232
+ bdrv_do_drained_end(bs, NULL);
233
+}
234
+
235
/*
236
* Wait for pending requests to complete on a single BlockDriverState subtree,
237
* and suspend block driver's internal I/O until next request arrives.
238
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_begin(void)
239
/* Stop things in parent-to-child order */
240
aio_context_acquire(aio_context);
241
aio_disable_external(aio_context);
242
- bdrv_parent_drained_begin(bs);
243
+ bdrv_parent_drained_begin(bs, NULL);
244
bdrv_drain_invoke(bs, true, true);
245
aio_context_release(aio_context);
246
247
@@ -XXX,XX +XXX,XX @@ void bdrv_drain_all_end(void)
248
/* Re-enable things in child-to-parent order */
249
aio_context_acquire(aio_context);
250
bdrv_drain_invoke(bs, false, true);
251
- bdrv_parent_drained_end(bs);
252
+ bdrv_parent_drained_end(bs, NULL);
253
aio_enable_external(aio_context);
254
aio_context_release(aio_context);
255
}
256
--
257
2.13.6
258
259
diff view generated by jsdifflib
1
From: Felipe Franciosi <felipe@nutanix.com>
1
bdrv_drained_begin() waits for the completion of requests in the whole
2
subtree, but it only actually keeps its immediate bs parameter quiesced
3
until bdrv_drained_end().
2
4
3
When querying an iSCSI server for the provisioning status of blocks (via
5
Add a version that keeps the whole subtree drained. As of this commit,
4
GET LBA STATUS), Qemu only validates that the response descriptor zero's
6
graph changes cannot be allowed during a subtree drained section, but
5
LBA matches the one requested. Given the SCSI spec allows servers to
7
this will be fixed soon.
6
respond with the status of blocks beyond the end of the LUN, Qemu may
7
have its heap corrupted by clearing/setting too many bits at the end of
8
its allocmap for the LUN.
9
8
10
A malicious guest in control of the iSCSI server could carefully program
11
Qemu's heap (by selectively setting the bitmap) and then smash it.
12
13
This limits the number of bits that iscsi_co_block_status() will try to
14
update in the allocmap so it can't overflow the bitmap.
15
16
Fixes: CVE-2020-1711
17
Cc: qemu-stable@nongnu.org
18
Signed-off-by: Felipe Franciosi <felipe@nutanix.com>
19
Signed-off-by: Peter Turschmid <peter.turschm@nutanix.com>
20
Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
21
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
22
---
10
---
23
block/iscsi.c | 5 +++--
11
include/block/block.h | 13 +++++++++++++
24
1 file changed, 3 insertions(+), 2 deletions(-)
12
block/io.c | 54 ++++++++++++++++++++++++++++++++++++++++-----------
13
2 files changed, 56 insertions(+), 11 deletions(-)
25
14
26
diff --git a/block/iscsi.c b/block/iscsi.c
15
diff --git a/include/block/block.h b/include/block/block.h
27
index XXXXXXX..XXXXXXX 100644
16
index XXXXXXX..XXXXXXX 100644
28
--- a/block/iscsi.c
17
--- a/include/block/block.h
29
+++ b/block/iscsi.c
18
+++ b/include/block/block.h
30
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_block_status(BlockDriverState *bs,
19
@@ -XXX,XX +XXX,XX @@ void bdrv_parent_drained_end(BlockDriverState *bs, BdrvChild *ignore);
31
struct scsi_get_lba_status *lbas = NULL;
20
void bdrv_drained_begin(BlockDriverState *bs);
32
struct scsi_lba_status_descriptor *lbasd = NULL;
21
33
struct IscsiTask iTask;
22
/**
34
- uint64_t lba;
23
+ * Like bdrv_drained_begin, but recursively begins a quiesced section for
35
+ uint64_t lba, max_bytes;
24
+ * exclusive access to all child nodes as well.
36
int ret;
25
+ *
37
26
+ * Graph changes are not allowed during a subtree drain section.
38
iscsi_co_init_iscsitask(iscsilun, &iTask);
27
+ */
39
@@ -XXX,XX +XXX,XX @@ static int coroutine_fn iscsi_co_block_status(BlockDriverState *bs,
28
+void bdrv_subtree_drained_begin(BlockDriverState *bs);
29
+
30
+/**
31
* bdrv_drained_end:
32
*
33
* End a quiescent section started by bdrv_drained_begin().
34
*/
35
void bdrv_drained_end(BlockDriverState *bs);
36
37
+/**
38
+ * End a quiescent section started by bdrv_subtree_drained_begin().
39
+ */
40
+void bdrv_subtree_drained_end(BlockDriverState *bs);
41
+
42
void bdrv_add_child(BlockDriverState *parent, BlockDriverState *child,
43
Error **errp);
44
void bdrv_del_child(BlockDriverState *parent, BdrvChild *child, Error **errp);
45
diff --git a/block/io.c b/block/io.c
46
index XXXXXXX..XXXXXXX 100644
47
--- a/block/io.c
48
+++ b/block/io.c
49
@@ -XXX,XX +XXX,XX @@ typedef struct {
50
BlockDriverState *bs;
51
bool done;
52
bool begin;
53
+ bool recursive;
54
BdrvChild *parent;
55
} BdrvCoDrainData;
56
57
@@ -XXX,XX +XXX,XX @@ static bool bdrv_drain_recurse(BlockDriverState *bs)
58
return waited;
59
}
60
61
-static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent);
62
-static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent);
63
+static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
64
+ BdrvChild *parent);
65
+static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
66
+ BdrvChild *parent);
67
68
static void bdrv_co_drain_bh_cb(void *opaque)
69
{
70
@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
71
72
bdrv_dec_in_flight(bs);
73
if (data->begin) {
74
- bdrv_do_drained_begin(bs, data->parent);
75
+ bdrv_do_drained_begin(bs, data->recursive, data->parent);
76
} else {
77
- bdrv_do_drained_end(bs, data->parent);
78
+ bdrv_do_drained_end(bs, data->recursive, data->parent);
40
}
79
}
41
80
42
lba = offset / iscsilun->block_size;
81
data->done = true;
43
+ max_bytes = (iscsilun->num_blocks - lba) * iscsilun->block_size;
82
@@ -XXX,XX +XXX,XX @@ static void bdrv_co_drain_bh_cb(void *opaque)
44
83
}
45
qemu_mutex_lock(&iscsilun->mutex);
84
46
retry:
85
static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
47
@@ -XXX,XX +XXX,XX @@ retry:
86
- bool begin, BdrvChild *parent)
48
goto out_unlock;
87
+ bool begin, bool recursive,
88
+ BdrvChild *parent)
89
{
90
BdrvCoDrainData data;
91
92
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
93
.bs = bs,
94
.done = false,
95
.begin = begin,
96
+ .recursive = recursive,
97
.parent = parent,
98
};
99
bdrv_inc_in_flight(bs);
100
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
101
assert(data.done);
102
}
103
104
-static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent)
105
+static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
106
+ BdrvChild *parent)
107
{
108
+ BdrvChild *child, *next;
109
+
110
if (qemu_in_coroutine()) {
111
- bdrv_co_yield_to_drain(bs, true, parent);
112
+ bdrv_co_yield_to_drain(bs, true, recursive, parent);
113
return;
49
}
114
}
50
115
51
- *pnum = (int64_t) lbasd->num_blocks * iscsilun->block_size;
116
@@ -XXX,XX +XXX,XX @@ static void bdrv_do_drained_begin(BlockDriverState *bs, BdrvChild *parent)
52
+ *pnum = MIN((int64_t) lbasd->num_blocks * iscsilun->block_size, max_bytes);
117
bdrv_parent_drained_begin(bs, parent);
53
118
bdrv_drain_invoke(bs, true, false);
54
if (lbasd->provisioning == SCSI_PROVISIONING_TYPE_DEALLOCATED ||
119
bdrv_drain_recurse(bs);
55
lbasd->provisioning == SCSI_PROVISIONING_TYPE_ANCHORED) {
120
+
121
+ if (recursive) {
122
+ QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
123
+ bdrv_do_drained_begin(child->bs, true, child);
124
+ }
125
+ }
126
}
127
128
void bdrv_drained_begin(BlockDriverState *bs)
129
{
130
- bdrv_do_drained_begin(bs, NULL);
131
+ bdrv_do_drained_begin(bs, false, NULL);
132
+}
133
+
134
+void bdrv_subtree_drained_begin(BlockDriverState *bs)
135
+{
136
+ bdrv_do_drained_begin(bs, true, NULL);
137
}
138
139
-static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
140
+static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
141
+ BdrvChild *parent)
142
{
143
+ BdrvChild *child, *next;
144
int old_quiesce_counter;
145
146
if (qemu_in_coroutine()) {
147
- bdrv_co_yield_to_drain(bs, false, parent);
148
+ bdrv_co_yield_to_drain(bs, false, recursive, parent);
149
return;
150
}
151
assert(bs->quiesce_counter > 0);
152
@@ -XXX,XX +XXX,XX @@ static void bdrv_do_drained_end(BlockDriverState *bs, BdrvChild *parent)
153
if (old_quiesce_counter == 1) {
154
aio_enable_external(bdrv_get_aio_context(bs));
155
}
156
+
157
+ if (recursive) {
158
+ QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
159
+ bdrv_do_drained_end(child->bs, true, child);
160
+ }
161
+ }
162
}
163
164
void bdrv_drained_end(BlockDriverState *bs)
165
{
166
- bdrv_do_drained_end(bs, NULL);
167
+ bdrv_do_drained_end(bs, false, NULL);
168
+}
169
+
170
+void bdrv_subtree_drained_end(BlockDriverState *bs)
171
+{
172
+ bdrv_do_drained_end(bs, true, NULL);
173
}
174
175
/*
56
--
176
--
57
2.20.1
177
2.13.6
58
178
59
179
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
Add a subtree drain version to the existing test cases.
2
2
3
All paths that lead to bdrv_backup_top_drop(), except for the call
4
from backup_clean(), imply that the BDS AioContext has already been
5
acquired, so doing it there too can potentially lead to QEMU hanging
6
on AIO_WAIT_WHILE().
7
8
An easy way to trigger this situation is by issuing a two actions
9
transaction, with a proper and a bogus blockdev-backup, so the second
10
one will trigger a rollback. This will trigger a hang with an stack
11
trace like this one:
12
13
#0 0x00007fb680c75016 in __GI_ppoll (fds=0x55e74580f7c0, nfds=1, timeout=<optimized out>,
14
timeout@entry=0x0, sigmask=sigmask@entry=0x0) at ../sysdeps/unix/sysv/linux/ppoll.c:39
15
#1 0x000055e743386e09 in ppoll (__ss=0x0, __timeout=0x0, __nfds=<optimized out>, __fds=<optimized out>)
16
at /usr/include/bits/poll2.h:77
17
#2 0x000055e743386e09 in qemu_poll_ns
18
(fds=<optimized out>, nfds=<optimized out>, timeout=<optimized out>) at util/qemu-timer.c:336
19
#3 0x000055e743388dc4 in aio_poll (ctx=0x55e7458925d0, blocking=blocking@entry=true)
20
at util/aio-posix.c:669
21
#4 0x000055e743305dea in bdrv_flush (bs=bs@entry=0x55e74593c0d0) at block/io.c:2878
22
#5 0x000055e7432be58e in bdrv_close (bs=0x55e74593c0d0) at block.c:4017
23
#6 0x000055e7432be58e in bdrv_delete (bs=<optimized out>) at block.c:4262
24
#7 0x000055e7432be58e in bdrv_unref (bs=bs@entry=0x55e74593c0d0) at block.c:5644
25
#8 0x000055e743316b9b in bdrv_backup_top_drop (bs=bs@entry=0x55e74593c0d0) at block/backup-top.c:273
26
#9 0x000055e74331461f in backup_job_create
27
(job_id=0x0, bs=bs@entry=0x55e7458d5820, target=target@entry=0x55e74589f640, speed=0, sync_mode=MIRROR_SYNC_MODE_FULL, sync_bitmap=sync_bitmap@entry=0x0, bitmap_mode=BITMAP_SYNC_MODE_ON_SUCCESS, compress=false, filter_node_name=0x0, on_source_error=BLOCKDEV_ON_ERROR_REPORT, on_target_error=BLOCKDEV_ON_ERROR_REPORT, creation_flags=0, cb=0x0, opaque=0x0, txn=0x0, errp=0x7ffddfd1efb0) at block/backup.c:478
28
#10 0x000055e74315bc52 in do_backup_common
29
(backup=backup@entry=0x55e746c066d0, bs=bs@entry=0x55e7458d5820, target_bs=target_bs@entry=0x55e74589f640, aio_context=aio_context@entry=0x55e7458a91e0, txn=txn@entry=0x0, errp=errp@entry=0x7ffddfd1efb0)
30
at blockdev.c:3580
31
#11 0x000055e74315c37c in do_blockdev_backup
32
(backup=backup@entry=0x55e746c066d0, txn=0x0, errp=errp@entry=0x7ffddfd1efb0)
33
at /usr/src/debug/qemu-kvm-4.2.0-2.module+el8.2.0+5135+ed3b2489.x86_64/./qapi/qapi-types-block-core.h:1492
34
#12 0x000055e74315c449 in blockdev_backup_prepare (common=0x55e746a8de90, errp=0x7ffddfd1f018)
35
at blockdev.c:1885
36
#13 0x000055e743160152 in qmp_transaction
37
(dev_list=<optimized out>, has_props=<optimized out>, props=0x55e7467fe2c0, errp=errp@entry=0x7ffddfd1f088) at blockdev.c:2340
38
#14 0x000055e743287ff5 in qmp_marshal_transaction
39
(args=<optimized out>, ret=<optimized out>, errp=0x7ffddfd1f0f8)
40
at qapi/qapi-commands-transaction.c:44
41
#15 0x000055e74333de6c in do_qmp_dispatch
42
(errp=0x7ffddfd1f0f0, allow_oob=<optimized out>, request=<optimized out>, cmds=0x55e743c28d60 <qmp_commands>) at qapi/qmp-dispatch.c:132
43
#16 0x000055e74333de6c in qmp_dispatch
44
(cmds=0x55e743c28d60 <qmp_commands>, request=<optimized out>, allow_oob=<optimized out>)
45
at qapi/qmp-dispatch.c:175
46
#17 0x000055e74325c061 in monitor_qmp_dispatch (mon=0x55e745908030, req=<optimized out>)
47
at monitor/qmp.c:145
48
#18 0x000055e74325c6fa in monitor_qmp_bh_dispatcher (data=<optimized out>) at monitor/qmp.c:234
49
#19 0x000055e743385866 in aio_bh_call (bh=0x55e745807ae0) at util/async.c:117
50
#20 0x000055e743385866 in aio_bh_poll (ctx=ctx@entry=0x55e7458067a0) at util/async.c:117
51
#21 0x000055e743388c54 in aio_dispatch (ctx=0x55e7458067a0) at util/aio-posix.c:459
52
#22 0x000055e743385742 in aio_ctx_dispatch
53
(source=<optimized out>, callback=<optimized out>, user_data=<optimized out>) at util/async.c:260
54
#23 0x00007fb68543e67d in g_main_dispatch (context=0x55e745893a40) at gmain.c:3176
55
#24 0x00007fb68543e67d in g_main_context_dispatch (context=context@entry=0x55e745893a40) at gmain.c:3829
56
#25 0x000055e743387d08 in glib_pollfds_poll () at util/main-loop.c:219
57
#26 0x000055e743387d08 in os_host_main_loop_wait (timeout=<optimized out>) at util/main-loop.c:242
58
#27 0x000055e743387d08 in main_loop_wait (nonblocking=<optimized out>) at util/main-loop.c:518
59
#28 0x000055e74316a3c1 in main_loop () at vl.c:1828
60
#29 0x000055e743016a72 in main (argc=<optimized out>, argv=<optimized out>, envp=<optimized out>)
61
at vl.c:4504
62
63
Fix this by not acquiring the AioContext there, and ensuring all paths
64
leading to it have it already acquired (backup_clean()).
65
66
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1782111
67
Signed-off-by: Sergio Lopez <slp@redhat.com>
68
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
3
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
69
---
4
---
70
block/backup-top.c | 5 -----
5
tests/test-bdrv-drain.c | 27 ++++++++++++++++++++++++++-
71
block/backup.c | 3 +++
6
1 file changed, 26 insertions(+), 1 deletion(-)
72
2 files changed, 3 insertions(+), 5 deletions(-)
73
7
74
diff --git a/block/backup-top.c b/block/backup-top.c
8
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
75
index XXXXXXX..XXXXXXX 100644
9
index XXXXXXX..XXXXXXX 100644
76
--- a/block/backup-top.c
10
--- a/tests/test-bdrv-drain.c
77
+++ b/block/backup-top.c
11
+++ b/tests/test-bdrv-drain.c
78
@@ -XXX,XX +XXX,XX @@ append_failed:
12
@@ -XXX,XX +XXX,XX @@ static void aio_ret_cb(void *opaque, int ret)
79
void bdrv_backup_top_drop(BlockDriverState *bs)
13
enum drain_type {
14
BDRV_DRAIN_ALL,
15
BDRV_DRAIN,
16
+ BDRV_SUBTREE_DRAIN,
17
DRAIN_TYPE_MAX,
18
};
19
20
@@ -XXX,XX +XXX,XX @@ static void do_drain_begin(enum drain_type drain_type, BlockDriverState *bs)
21
switch (drain_type) {
22
case BDRV_DRAIN_ALL: bdrv_drain_all_begin(); break;
23
case BDRV_DRAIN: bdrv_drained_begin(bs); break;
24
+ case BDRV_SUBTREE_DRAIN: bdrv_subtree_drained_begin(bs); break;
25
default: g_assert_not_reached();
26
}
27
}
28
@@ -XXX,XX +XXX,XX @@ static void do_drain_end(enum drain_type drain_type, BlockDriverState *bs)
29
switch (drain_type) {
30
case BDRV_DRAIN_ALL: bdrv_drain_all_end(); break;
31
case BDRV_DRAIN: bdrv_drained_end(bs); break;
32
+ case BDRV_SUBTREE_DRAIN: bdrv_subtree_drained_end(bs); break;
33
default: g_assert_not_reached();
34
}
35
}
36
@@ -XXX,XX +XXX,XX @@ static void test_drv_cb_drain(void)
37
test_drv_cb_common(BDRV_DRAIN, false);
38
}
39
40
+static void test_drv_cb_drain_subtree(void)
41
+{
42
+ test_drv_cb_common(BDRV_SUBTREE_DRAIN, true);
43
+}
44
+
45
static void test_quiesce_common(enum drain_type drain_type, bool recursive)
80
{
46
{
81
BDRVBackupTopState *s = bs->opaque;
47
BlockBackend *blk;
82
- AioContext *aio_context = bdrv_get_aio_context(bs);
48
@@ -XXX,XX +XXX,XX @@ static void test_quiesce_drain(void)
83
-
49
test_quiesce_common(BDRV_DRAIN, false);
84
- aio_context_acquire(aio_context);
85
86
bdrv_drained_begin(bs);
87
88
@@ -XXX,XX +XXX,XX @@ void bdrv_backup_top_drop(BlockDriverState *bs)
89
bdrv_drained_end(bs);
90
91
bdrv_unref(bs);
92
-
93
- aio_context_release(aio_context);
94
}
50
}
95
diff --git a/block/backup.c b/block/backup.c
51
96
index XXXXXXX..XXXXXXX 100644
52
+static void test_quiesce_drain_subtree(void)
97
--- a/block/backup.c
53
+{
98
+++ b/block/backup.c
54
+ test_quiesce_common(BDRV_SUBTREE_DRAIN, true);
99
@@ -XXX,XX +XXX,XX @@ static void backup_abort(Job *job)
55
+}
100
static void backup_clean(Job *job)
56
+
57
static void test_nested(void)
101
{
58
{
102
BackupBlockJob *s = container_of(job, BackupBlockJob, common.job);
59
BlockBackend *blk;
103
+ AioContext *aio_context = bdrv_get_aio_context(s->backup_top);
60
@@ -XXX,XX +XXX,XX @@ static void test_nested(void)
104
61
/* XXX bdrv_drain_all() doesn't increase the quiesce_counter */
105
+ aio_context_acquire(aio_context);
62
int bs_quiesce = (outer != BDRV_DRAIN_ALL) +
106
bdrv_backup_top_drop(s->backup_top);
63
(inner != BDRV_DRAIN_ALL);
107
+ aio_context_release(aio_context);
64
- int backing_quiesce = 0;
65
+ int backing_quiesce = (outer == BDRV_SUBTREE_DRAIN) +
66
+ (inner == BDRV_SUBTREE_DRAIN);
67
int backing_cb_cnt = (outer != BDRV_DRAIN) +
68
(inner != BDRV_DRAIN);
69
70
@@ -XXX,XX +XXX,XX @@ static void test_blockjob_drain(void)
71
test_blockjob_common(BDRV_DRAIN);
108
}
72
}
109
73
110
void backup_do_checkpoint(BlockJob *job, Error **errp)
74
+static void test_blockjob_drain_subtree(void)
75
+{
76
+ test_blockjob_common(BDRV_SUBTREE_DRAIN);
77
+}
78
+
79
int main(int argc, char **argv)
80
{
81
bdrv_init();
82
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
83
84
g_test_add_func("/bdrv-drain/driver-cb/drain_all", test_drv_cb_drain_all);
85
g_test_add_func("/bdrv-drain/driver-cb/drain", test_drv_cb_drain);
86
+ g_test_add_func("/bdrv-drain/driver-cb/drain_subtree",
87
+ test_drv_cb_drain_subtree);
88
89
g_test_add_func("/bdrv-drain/quiesce/drain_all", test_quiesce_drain_all);
90
g_test_add_func("/bdrv-drain/quiesce/drain", test_quiesce_drain);
91
+ g_test_add_func("/bdrv-drain/quiesce/drain_subtree",
92
+ test_quiesce_drain_subtree);
93
94
g_test_add_func("/bdrv-drain/nested", test_nested);
95
96
g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
97
g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain);
98
+ g_test_add_func("/bdrv-drain/blockjob/drain_subtree",
99
+ test_blockjob_drain_subtree);
100
101
return g_test_run();
102
}
111
--
103
--
112
2.20.1
104
2.13.6
113
105
114
106
diff view generated by jsdifflib
1
From: Sergio Lopez <slp@redhat.com>
1
If bdrv_do_drained_begin/end() are called in coroutine context, they
2
first use a BH to get out of the coroutine context. Call some existing
3
tests again from a coroutine to cover this code path.
2
4
3
Includes the following tests:
4
5
- Adding a dirty bitmap.
6
* RHBZ: 1782175
7
8
- Starting a drive-mirror to an NBD-backed target.
9
* RHBZ: 1746217, 1773517
10
11
- Aborting an external snapshot transaction.
12
* RHBZ: 1779036
13
14
- Aborting a blockdev backup transaction.
15
* RHBZ: 1782111
16
17
For each one of them, a VM with a number of disks running in an
18
IOThread AioContext is used.
19
20
Signed-off-by: Sergio Lopez <slp@redhat.com>
21
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
5
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
22
---
6
---
23
tests/qemu-iotests/281 | 247 +++++++++++++++++++++++++++++++++++++
7
tests/test-bdrv-drain.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++
24
tests/qemu-iotests/281.out | 5 +
8
1 file changed, 59 insertions(+)
25
tests/qemu-iotests/group | 1 +
26
3 files changed, 253 insertions(+)
27
create mode 100755 tests/qemu-iotests/281
28
create mode 100644 tests/qemu-iotests/281.out
29
9
30
diff --git a/tests/qemu-iotests/281 b/tests/qemu-iotests/281
10
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
31
new file mode 100755
11
index XXXXXXX..XXXXXXX 100644
32
index XXXXXXX..XXXXXXX
12
--- a/tests/test-bdrv-drain.c
33
--- /dev/null
13
+++ b/tests/test-bdrv-drain.c
34
+++ b/tests/qemu-iotests/281
14
@@ -XXX,XX +XXX,XX @@ static void aio_ret_cb(void *opaque, int ret)
35
@@ -XXX,XX +XXX,XX @@
15
*aio_ret = ret;
36
+#!/usr/bin/env python
16
}
37
+#
17
38
+# Test cases for blockdev + IOThread interactions
18
+typedef struct CallInCoroutineData {
39
+#
19
+ void (*entry)(void);
40
+# Copyright (C) 2019 Red Hat, Inc.
20
+ bool done;
41
+#
21
+} CallInCoroutineData;
42
+# This program is free software; you can redistribute it and/or modify
43
+# it under the terms of the GNU General Public License as published by
44
+# the Free Software Foundation; either version 2 of the License, or
45
+# (at your option) any later version.
46
+#
47
+# This program is distributed in the hope that it will be useful,
48
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
49
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
50
+# GNU General Public License for more details.
51
+#
52
+# You should have received a copy of the GNU General Public License
53
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
54
+#
55
+
22
+
56
+import os
23
+static coroutine_fn void call_in_coroutine_entry(void *opaque)
57
+import iotests
24
+{
58
+from iotests import qemu_img
25
+ CallInCoroutineData *data = opaque;
59
+
26
+
60
+image_len = 64 * 1024 * 1024
27
+ data->entry();
28
+ data->done = true;
29
+}
61
+
30
+
62
+# Test for RHBZ#1782175
31
+static void call_in_coroutine(void (*entry)(void))
63
+class TestDirtyBitmapIOThread(iotests.QMPTestCase):
32
+{
64
+ drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
33
+ Coroutine *co;
65
+ images = { 'drive0': drive0_img }
34
+ CallInCoroutineData data = {
35
+ .entry = entry,
36
+ .done = false,
37
+ };
66
+
38
+
67
+ def setUp(self):
39
+ co = qemu_coroutine_create(call_in_coroutine_entry, &data);
68
+ for name in self.images:
40
+ qemu_coroutine_enter(co);
69
+ qemu_img('create', '-f', iotests.imgfmt,
41
+ while (!data.done) {
70
+ self.images[name], str(image_len))
42
+ aio_poll(qemu_get_aio_context(), true);
43
+ }
44
+}
71
+
45
+
72
+ self.vm = iotests.VM()
46
enum drain_type {
73
+ self.vm.add_object('iothread,id=iothread0')
47
BDRV_DRAIN_ALL,
48
BDRV_DRAIN,
49
@@ -XXX,XX +XXX,XX @@ static void test_drv_cb_drain_subtree(void)
50
test_drv_cb_common(BDRV_SUBTREE_DRAIN, true);
51
}
52
53
+static void test_drv_cb_co_drain(void)
54
+{
55
+ call_in_coroutine(test_drv_cb_drain);
56
+}
74
+
57
+
75
+ for name in self.images:
58
+static void test_drv_cb_co_drain_subtree(void)
76
+ self.vm.add_blockdev('driver=file,filename=%s,node-name=file_%s'
59
+{
77
+ % (self.images[name], name))
60
+ call_in_coroutine(test_drv_cb_drain_subtree);
78
+ self.vm.add_blockdev('driver=qcow2,file=file_%s,node-name=%s'
61
+}
79
+ % (name, name))
80
+
62
+
81
+ self.vm.launch()
63
static void test_quiesce_common(enum drain_type drain_type, bool recursive)
82
+ self.vm.qmp('x-blockdev-set-iothread',
64
{
83
+ node_name='drive0', iothread='iothread0',
65
BlockBackend *blk;
84
+ force=True)
66
@@ -XXX,XX +XXX,XX @@ static void test_quiesce_drain_subtree(void)
67
test_quiesce_common(BDRV_SUBTREE_DRAIN, true);
68
}
69
70
+static void test_quiesce_co_drain(void)
71
+{
72
+ call_in_coroutine(test_quiesce_drain);
73
+}
85
+
74
+
86
+ def tearDown(self):
75
+static void test_quiesce_co_drain_subtree(void)
87
+ self.vm.shutdown()
76
+{
88
+ for name in self.images:
77
+ call_in_coroutine(test_quiesce_drain_subtree);
89
+ os.remove(self.images[name])
78
+}
90
+
79
+
91
+ def test_add_dirty_bitmap(self):
80
static void test_nested(void)
92
+ result = self.vm.qmp(
81
{
93
+ 'block-dirty-bitmap-add',
82
BlockBackend *blk;
94
+ node='drive0',
83
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
95
+ name='bitmap1',
84
g_test_add_func("/bdrv-drain/driver-cb/drain_subtree",
96
+ persistent=True,
85
test_drv_cb_drain_subtree);
97
+ )
86
98
+
87
+ // XXX bdrv_drain_all() doesn't work in coroutine context
99
+ self.assert_qmp(result, 'return', {})
88
+ g_test_add_func("/bdrv-drain/driver-cb/co/drain", test_drv_cb_co_drain);
89
+ g_test_add_func("/bdrv-drain/driver-cb/co/drain_subtree",
90
+ test_drv_cb_co_drain_subtree);
100
+
91
+
101
+
92
+
102
+# Test for RHBZ#1746217 & RHBZ#1773517
93
g_test_add_func("/bdrv-drain/quiesce/drain_all", test_quiesce_drain_all);
103
+class TestNBDMirrorIOThread(iotests.QMPTestCase):
94
g_test_add_func("/bdrv-drain/quiesce/drain", test_quiesce_drain);
104
+ nbd_sock = os.path.join(iotests.sock_dir, 'nbd.sock')
95
g_test_add_func("/bdrv-drain/quiesce/drain_subtree",
105
+ drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
96
test_quiesce_drain_subtree);
106
+ mirror_img = os.path.join(iotests.test_dir, 'mirror.img')
97
107
+ images = { 'drive0': drive0_img, 'mirror': mirror_img }
98
+ // XXX bdrv_drain_all() doesn't work in coroutine context
99
+ g_test_add_func("/bdrv-drain/quiesce/co/drain", test_quiesce_co_drain);
100
+ g_test_add_func("/bdrv-drain/quiesce/co/drain_subtree",
101
+ test_quiesce_co_drain_subtree);
108
+
102
+
109
+ def setUp(self):
103
g_test_add_func("/bdrv-drain/nested", test_nested);
110
+ for name in self.images:
104
111
+ qemu_img('create', '-f', iotests.imgfmt,
105
g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
112
+ self.images[name], str(image_len))
113
+
114
+ self.vm_src = iotests.VM(path_suffix='src')
115
+ self.vm_src.add_object('iothread,id=iothread0')
116
+ self.vm_src.add_blockdev('driver=file,filename=%s,node-name=file0'
117
+ % (self.drive0_img))
118
+ self.vm_src.add_blockdev('driver=qcow2,file=file0,node-name=drive0')
119
+ self.vm_src.launch()
120
+ self.vm_src.qmp('x-blockdev-set-iothread',
121
+ node_name='drive0', iothread='iothread0',
122
+ force=True)
123
+
124
+ self.vm_tgt = iotests.VM(path_suffix='tgt')
125
+ self.vm_tgt.add_object('iothread,id=iothread0')
126
+ self.vm_tgt.add_blockdev('driver=file,filename=%s,node-name=file0'
127
+ % (self.mirror_img))
128
+ self.vm_tgt.add_blockdev('driver=qcow2,file=file0,node-name=drive0')
129
+ self.vm_tgt.launch()
130
+ self.vm_tgt.qmp('x-blockdev-set-iothread',
131
+ node_name='drive0', iothread='iothread0',
132
+ force=True)
133
+
134
+ def tearDown(self):
135
+ self.vm_src.shutdown()
136
+ self.vm_tgt.shutdown()
137
+ for name in self.images:
138
+ os.remove(self.images[name])
139
+
140
+ def test_nbd_mirror(self):
141
+ result = self.vm_tgt.qmp(
142
+ 'nbd-server-start',
143
+ addr={
144
+ 'type': 'unix',
145
+ 'data': { 'path': self.nbd_sock }
146
+ }
147
+ )
148
+ self.assert_qmp(result, 'return', {})
149
+
150
+ result = self.vm_tgt.qmp(
151
+ 'nbd-server-add',
152
+ device='drive0',
153
+ writable=True
154
+ )
155
+ self.assert_qmp(result, 'return', {})
156
+
157
+ result = self.vm_src.qmp(
158
+ 'drive-mirror',
159
+ device='drive0',
160
+ target='nbd+unix:///drive0?socket=' + self.nbd_sock,
161
+ sync='full',
162
+ mode='existing',
163
+ speed=64*1024*1024,
164
+ job_id='j1'
165
+ )
166
+ self.assert_qmp(result, 'return', {})
167
+
168
+ self.vm_src.event_wait(name="BLOCK_JOB_READY")
169
+
170
+
171
+# Test for RHBZ#1779036
172
+class TestExternalSnapshotAbort(iotests.QMPTestCase):
173
+ drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
174
+ snapshot_img = os.path.join(iotests.test_dir, 'snapshot.img')
175
+ images = { 'drive0': drive0_img, 'snapshot': snapshot_img }
176
+
177
+ def setUp(self):
178
+ for name in self.images:
179
+ qemu_img('create', '-f', iotests.imgfmt,
180
+ self.images[name], str(image_len))
181
+
182
+ self.vm = iotests.VM()
183
+ self.vm.add_object('iothread,id=iothread0')
184
+ self.vm.add_blockdev('driver=file,filename=%s,node-name=file0'
185
+ % (self.drive0_img))
186
+ self.vm.add_blockdev('driver=qcow2,file=file0,node-name=drive0')
187
+ self.vm.launch()
188
+ self.vm.qmp('x-blockdev-set-iothread',
189
+ node_name='drive0', iothread='iothread0',
190
+ force=True)
191
+
192
+ def tearDown(self):
193
+ self.vm.shutdown()
194
+ for name in self.images:
195
+ os.remove(self.images[name])
196
+
197
+ def test_external_snapshot_abort(self):
198
+ # Use a two actions transaction with a bogus values on the second
199
+ # one to trigger an abort of the transaction.
200
+ result = self.vm.qmp('transaction', actions=[
201
+ {
202
+ 'type': 'blockdev-snapshot-sync',
203
+ 'data': { 'node-name': 'drive0',
204
+ 'snapshot-file': self.snapshot_img,
205
+ 'snapshot-node-name': 'snap1',
206
+ 'mode': 'absolute-paths',
207
+ 'format': 'qcow2' }
208
+ },
209
+ {
210
+ 'type': 'blockdev-snapshot-sync',
211
+ 'data': { 'node-name': 'drive0',
212
+ 'snapshot-file': '/fakesnapshot',
213
+ 'snapshot-node-name': 'snap2',
214
+ 'mode': 'absolute-paths',
215
+ 'format': 'qcow2' }
216
+ },
217
+ ])
218
+
219
+ # Crashes on failure, we expect this error.
220
+ self.assert_qmp(result, 'error/class', 'GenericError')
221
+
222
+
223
+# Test for RHBZ#1782111
224
+class TestBlockdevBackupAbort(iotests.QMPTestCase):
225
+ drive0_img = os.path.join(iotests.test_dir, 'drive0.img')
226
+ drive1_img = os.path.join(iotests.test_dir, 'drive1.img')
227
+ snap0_img = os.path.join(iotests.test_dir, 'snap0.img')
228
+ snap1_img = os.path.join(iotests.test_dir, 'snap1.img')
229
+ images = { 'drive0': drive0_img,
230
+ 'drive1': drive1_img,
231
+ 'snap0': snap0_img,
232
+ 'snap1': snap1_img }
233
+
234
+ def setUp(self):
235
+ for name in self.images:
236
+ qemu_img('create', '-f', iotests.imgfmt,
237
+ self.images[name], str(image_len))
238
+
239
+ self.vm = iotests.VM()
240
+ self.vm.add_object('iothread,id=iothread0')
241
+ self.vm.add_device('virtio-scsi,iothread=iothread0')
242
+
243
+ for name in self.images:
244
+ self.vm.add_blockdev('driver=file,filename=%s,node-name=file_%s'
245
+ % (self.images[name], name))
246
+ self.vm.add_blockdev('driver=qcow2,file=file_%s,node-name=%s'
247
+ % (name, name))
248
+
249
+ self.vm.add_device('scsi-hd,drive=drive0')
250
+ self.vm.add_device('scsi-hd,drive=drive1')
251
+ self.vm.launch()
252
+
253
+ def tearDown(self):
254
+ self.vm.shutdown()
255
+ for name in self.images:
256
+ os.remove(self.images[name])
257
+
258
+ def test_blockdev_backup_abort(self):
259
+ # Use a two actions transaction with a bogus values on the second
260
+ # one to trigger an abort of the transaction.
261
+ result = self.vm.qmp('transaction', actions=[
262
+ {
263
+ 'type': 'blockdev-backup',
264
+ 'data': { 'device': 'drive0',
265
+ 'target': 'snap0',
266
+ 'sync': 'full',
267
+ 'job-id': 'j1' }
268
+ },
269
+ {
270
+ 'type': 'blockdev-backup',
271
+ 'data': { 'device': 'drive1',
272
+ 'target': 'snap1',
273
+ 'sync': 'full' }
274
+ },
275
+ ])
276
+
277
+ # Hangs on failure, we expect this error.
278
+ self.assert_qmp(result, 'error/class', 'GenericError')
279
+
280
+if __name__ == '__main__':
281
+ iotests.main(supported_fmts=['qcow2'],
282
+ supported_protocols=['file'])
283
diff --git a/tests/qemu-iotests/281.out b/tests/qemu-iotests/281.out
284
new file mode 100644
285
index XXXXXXX..XXXXXXX
286
--- /dev/null
287
+++ b/tests/qemu-iotests/281.out
288
@@ -XXX,XX +XXX,XX @@
289
+....
290
+----------------------------------------------------------------------
291
+Ran 4 tests
292
+
293
+OK
294
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
295
index XXXXXXX..XXXXXXX 100644
296
--- a/tests/qemu-iotests/group
297
+++ b/tests/qemu-iotests/group
298
@@ -XXX,XX +XXX,XX @@
299
277 rw quick
300
279 rw backing quick
301
280 rw migration quick
302
+281 rw quick
303
--
106
--
304
2.20.1
107
2.13.6
305
108
306
109
diff view generated by jsdifflib
1
From: Eiichi Tsukata <devel@etsukata.com>
1
Test that drain sections are correctly propagated through the graph.
2
2
3
bdrv_open_driver() allocates bs->opaque according to drv->instance_size.
4
There is no need to allocate it and overwrite opaque in
5
bdrv_backup_top_append().
6
7
Reproducer:
8
9
$ QTEST_QEMU_BINARY=./x86_64-softmmu/qemu-system-x86_64 valgrind -q --leak-check=full tests/test-replication -p /replication/secondary/start
10
==29792== 24 bytes in 1 blocks are definitely lost in loss record 52 of 226
11
==29792== at 0x483AB1A: calloc (vg_replace_malloc.c:762)
12
==29792== by 0x4B07CE0: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6000.7)
13
==29792== by 0x12BAB9: bdrv_open_driver (block.c:1289)
14
==29792== by 0x12BEA9: bdrv_new_open_driver (block.c:1359)
15
==29792== by 0x1D15CB: bdrv_backup_top_append (backup-top.c:190)
16
==29792== by 0x1CC11A: backup_job_create (backup.c:439)
17
==29792== by 0x1CD542: replication_start (replication.c:544)
18
==29792== by 0x1401B9: replication_start_all (replication.c:52)
19
==29792== by 0x128B50: test_secondary_start (test-replication.c:427)
20
...
21
22
Fixes: 7df7868b9640 ("block: introduce backup-top filter driver")
23
Signed-off-by: Eiichi Tsukata <devel@etsukata.com>
24
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
25
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
3
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
26
---
4
---
27
block/backup-top.c | 2 +-
5
tests/test-bdrv-drain.c | 74 +++++++++++++++++++++++++++++++++++++++++++++++++
28
1 file changed, 1 insertion(+), 1 deletion(-)
6
1 file changed, 74 insertions(+)
29
7
30
diff --git a/block/backup-top.c b/block/backup-top.c
8
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
31
index XXXXXXX..XXXXXXX 100644
9
index XXXXXXX..XXXXXXX 100644
32
--- a/block/backup-top.c
10
--- a/tests/test-bdrv-drain.c
33
+++ b/block/backup-top.c
11
+++ b/tests/test-bdrv-drain.c
34
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_backup_top_append(BlockDriverState *source,
12
@@ -XXX,XX +XXX,XX @@ static void test_nested(void)
35
}
13
blk_unref(blk);
36
14
}
37
top->total_sectors = source->total_sectors;
15
38
- top->opaque = state = g_new0(BDRVBackupTopState, 1);
16
+static void test_multiparent(void)
39
+ state = top->opaque;
17
+{
40
18
+ BlockBackend *blk_a, *blk_b;
41
bdrv_ref(target);
19
+ BlockDriverState *bs_a, *bs_b, *backing;
42
state->target = bdrv_attach_child(top, target, "target", &child_file, errp);
20
+ BDRVTestState *a_s, *b_s, *backing_s;
21
+
22
+ blk_a = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
23
+ bs_a = bdrv_new_open_driver(&bdrv_test, "test-node-a", BDRV_O_RDWR,
24
+ &error_abort);
25
+ a_s = bs_a->opaque;
26
+ blk_insert_bs(blk_a, bs_a, &error_abort);
27
+
28
+ blk_b = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
29
+ bs_b = bdrv_new_open_driver(&bdrv_test, "test-node-b", BDRV_O_RDWR,
30
+ &error_abort);
31
+ b_s = bs_b->opaque;
32
+ blk_insert_bs(blk_b, bs_b, &error_abort);
33
+
34
+ backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
35
+ backing_s = backing->opaque;
36
+ bdrv_set_backing_hd(bs_a, backing, &error_abort);
37
+ bdrv_set_backing_hd(bs_b, backing, &error_abort);
38
+
39
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 0);
40
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 0);
41
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
42
+ g_assert_cmpint(a_s->drain_count, ==, 0);
43
+ g_assert_cmpint(b_s->drain_count, ==, 0);
44
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
45
+
46
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_a);
47
+
48
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 1);
49
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 1);
50
+ g_assert_cmpint(backing->quiesce_counter, ==, 1);
51
+ g_assert_cmpint(a_s->drain_count, ==, 1);
52
+ g_assert_cmpint(b_s->drain_count, ==, 1);
53
+ g_assert_cmpint(backing_s->drain_count, ==, 1);
54
+
55
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_b);
56
+
57
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 2);
58
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 2);
59
+ g_assert_cmpint(backing->quiesce_counter, ==, 2);
60
+ g_assert_cmpint(a_s->drain_count, ==, 2);
61
+ g_assert_cmpint(b_s->drain_count, ==, 2);
62
+ g_assert_cmpint(backing_s->drain_count, ==, 2);
63
+
64
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_b);
65
+
66
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 1);
67
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 1);
68
+ g_assert_cmpint(backing->quiesce_counter, ==, 1);
69
+ g_assert_cmpint(a_s->drain_count, ==, 1);
70
+ g_assert_cmpint(b_s->drain_count, ==, 1);
71
+ g_assert_cmpint(backing_s->drain_count, ==, 1);
72
+
73
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_a);
74
+
75
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 0);
76
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 0);
77
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
78
+ g_assert_cmpint(a_s->drain_count, ==, 0);
79
+ g_assert_cmpint(b_s->drain_count, ==, 0);
80
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
81
+
82
+ bdrv_unref(backing);
83
+ bdrv_unref(bs_a);
84
+ bdrv_unref(bs_b);
85
+ blk_unref(blk_a);
86
+ blk_unref(blk_b);
87
+}
88
+
89
90
typedef struct TestBlockJob {
91
BlockJob common;
92
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
93
test_quiesce_co_drain_subtree);
94
95
g_test_add_func("/bdrv-drain/nested", test_nested);
96
+ g_test_add_func("/bdrv-drain/multiparent", test_multiparent);
97
98
g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
99
g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain);
43
--
100
--
44
2.20.1
101
2.13.6
45
102
46
103
diff view generated by jsdifflib
New patch
1
1
We need to remember how many of the drain sections in which a node is
2
were recursive (i.e. subtree drain rather than node drain), so that they
3
can be correctly applied when children are added or removed during the
4
drained section.
5
6
With this change, it is safe to modify the graph even inside a
7
bdrv_subtree_drained_begin/end() section.
8
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
---
11
include/block/block.h | 2 --
12
include/block/block_int.h | 5 +++++
13
block.c | 32 +++++++++++++++++++++++++++++---
14
block/io.c | 28 ++++++++++++++++++++++++----
15
4 files changed, 58 insertions(+), 9 deletions(-)
16
17
diff --git a/include/block/block.h b/include/block/block.h
18
index XXXXXXX..XXXXXXX 100644
19
--- a/include/block/block.h
20
+++ b/include/block/block.h
21
@@ -XXX,XX +XXX,XX @@ void bdrv_drained_begin(BlockDriverState *bs);
22
/**
23
* Like bdrv_drained_begin, but recursively begins a quiesced section for
24
* exclusive access to all child nodes as well.
25
- *
26
- * Graph changes are not allowed during a subtree drain section.
27
*/
28
void bdrv_subtree_drained_begin(BlockDriverState *bs);
29
30
diff --git a/include/block/block_int.h b/include/block/block_int.h
31
index XXXXXXX..XXXXXXX 100644
32
--- a/include/block/block_int.h
33
+++ b/include/block/block_int.h
34
@@ -XXX,XX +XXX,XX @@ struct BlockDriverState {
35
36
/* Accessed with atomic ops. */
37
int quiesce_counter;
38
+ int recursive_quiesce_counter;
39
+
40
unsigned int write_gen; /* Current data generation */
41
42
/* Protected by reqs_lock. */
43
@@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pwritev(BdrvChild *child,
44
int64_t offset, unsigned int bytes, QEMUIOVector *qiov,
45
BdrvRequestFlags flags);
46
47
+void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent);
48
+void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_parent);
49
+
50
int get_tmp_filename(char *filename, int size);
51
BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size,
52
const char *filename);
53
diff --git a/block.c b/block.c
54
index XXXXXXX..XXXXXXX 100644
55
--- a/block.c
56
+++ b/block.c
57
@@ -XXX,XX +XXX,XX @@ static void bdrv_child_cb_drained_end(BdrvChild *child)
58
bdrv_drained_end(bs);
59
}
60
61
+static void bdrv_child_cb_attach(BdrvChild *child)
62
+{
63
+ BlockDriverState *bs = child->opaque;
64
+ bdrv_apply_subtree_drain(child, bs);
65
+}
66
+
67
+static void bdrv_child_cb_detach(BdrvChild *child)
68
+{
69
+ BlockDriverState *bs = child->opaque;
70
+ bdrv_unapply_subtree_drain(child, bs);
71
+}
72
+
73
static int bdrv_child_cb_inactivate(BdrvChild *child)
74
{
75
BlockDriverState *bs = child->opaque;
76
@@ -XXX,XX +XXX,XX @@ const BdrvChildRole child_file = {
77
.inherit_options = bdrv_inherited_options,
78
.drained_begin = bdrv_child_cb_drained_begin,
79
.drained_end = bdrv_child_cb_drained_end,
80
+ .attach = bdrv_child_cb_attach,
81
+ .detach = bdrv_child_cb_detach,
82
.inactivate = bdrv_child_cb_inactivate,
83
};
84
85
@@ -XXX,XX +XXX,XX @@ const BdrvChildRole child_format = {
86
.inherit_options = bdrv_inherited_fmt_options,
87
.drained_begin = bdrv_child_cb_drained_begin,
88
.drained_end = bdrv_child_cb_drained_end,
89
+ .attach = bdrv_child_cb_attach,
90
+ .detach = bdrv_child_cb_detach,
91
.inactivate = bdrv_child_cb_inactivate,
92
};
93
94
@@ -XXX,XX +XXX,XX @@ static void bdrv_backing_attach(BdrvChild *c)
95
parent->backing_blocker);
96
bdrv_op_unblock(backing_hd, BLOCK_OP_TYPE_BACKUP_TARGET,
97
parent->backing_blocker);
98
+
99
+ bdrv_child_cb_attach(c);
100
}
101
102
static void bdrv_backing_detach(BdrvChild *c)
103
@@ -XXX,XX +XXX,XX @@ static void bdrv_backing_detach(BdrvChild *c)
104
bdrv_op_unblock_all(c->bs, parent->backing_blocker);
105
error_free(parent->backing_blocker);
106
parent->backing_blocker = NULL;
107
+
108
+ bdrv_child_cb_detach(c);
109
}
110
111
/*
112
@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child_noperm(BdrvChild *child,
113
assert(bdrv_get_aio_context(old_bs) == bdrv_get_aio_context(new_bs));
114
}
115
if (old_bs) {
116
+ /* Detach first so that the recursive drain sections coming from @child
117
+ * are already gone and we only end the drain sections that came from
118
+ * elsewhere. */
119
+ if (child->role->detach) {
120
+ child->role->detach(child);
121
+ }
122
if (old_bs->quiesce_counter && child->role->drained_end) {
123
for (i = 0; i < old_bs->quiesce_counter; i++) {
124
child->role->drained_end(child);
125
}
126
}
127
- if (child->role->detach) {
128
- child->role->detach(child);
129
- }
130
QLIST_REMOVE(child, next_parent);
131
}
132
133
@@ -XXX,XX +XXX,XX @@ static void bdrv_replace_child_noperm(BdrvChild *child,
134
}
135
}
136
137
+ /* Attach only after starting new drained sections, so that recursive
138
+ * drain sections coming from @child don't get an extra .drained_begin
139
+ * callback. */
140
if (child->role->attach) {
141
child->role->attach(child);
142
}
143
diff --git a/block/io.c b/block/io.c
144
index XXXXXXX..XXXXXXX 100644
145
--- a/block/io.c
146
+++ b/block/io.c
147
@@ -XXX,XX +XXX,XX @@ static void coroutine_fn bdrv_co_yield_to_drain(BlockDriverState *bs,
148
assert(data.done);
149
}
150
151
-static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
152
- BdrvChild *parent)
153
+void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
154
+ BdrvChild *parent)
155
{
156
BdrvChild *child, *next;
157
158
@@ -XXX,XX +XXX,XX @@ static void bdrv_do_drained_begin(BlockDriverState *bs, bool recursive,
159
bdrv_drain_recurse(bs);
160
161
if (recursive) {
162
+ bs->recursive_quiesce_counter++;
163
QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
164
bdrv_do_drained_begin(child->bs, true, child);
165
}
166
@@ -XXX,XX +XXX,XX @@ void bdrv_subtree_drained_begin(BlockDriverState *bs)
167
bdrv_do_drained_begin(bs, true, NULL);
168
}
169
170
-static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
171
- BdrvChild *parent)
172
+void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
173
+ BdrvChild *parent)
174
{
175
BdrvChild *child, *next;
176
int old_quiesce_counter;
177
@@ -XXX,XX +XXX,XX @@ static void bdrv_do_drained_end(BlockDriverState *bs, bool recursive,
178
}
179
180
if (recursive) {
181
+ bs->recursive_quiesce_counter--;
182
QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
183
bdrv_do_drained_end(child->bs, true, child);
184
}
185
@@ -XXX,XX +XXX,XX @@ void bdrv_subtree_drained_end(BlockDriverState *bs)
186
bdrv_do_drained_end(bs, true, NULL);
187
}
188
189
+void bdrv_apply_subtree_drain(BdrvChild *child, BlockDriverState *new_parent)
190
+{
191
+ int i;
192
+
193
+ for (i = 0; i < new_parent->recursive_quiesce_counter; i++) {
194
+ bdrv_do_drained_begin(child->bs, true, child);
195
+ }
196
+}
197
+
198
+void bdrv_unapply_subtree_drain(BdrvChild *child, BlockDriverState *old_parent)
199
+{
200
+ int i;
201
+
202
+ for (i = 0; i < old_parent->recursive_quiesce_counter; i++) {
203
+ bdrv_do_drained_end(child->bs, true, child);
204
+ }
205
+}
206
+
207
/*
208
* Wait for pending requests to complete on a single BlockDriverState subtree,
209
* and suspend block driver's internal I/O until next request arrives.
210
--
211
2.13.6
212
213
diff view generated by jsdifflib
1
From: Max Reitz <mreitz@redhat.com>
2
3
The "migration completed" event may be sent (on the source, to be
4
specific) before the migration is actually completed, so the VM runstate
5
will still be "finish-migrate" instead of "postmigrate". So ask the
6
users of VM.wait_migration() to specify the final runstate they desire
7
and then poll the VM until it has reached that state. (This should be
8
over very quickly, so busy polling is fine.)
9
10
Without this patch, I see intermittent failures in the new iotest 280
11
under high system load. I have not yet seen such failures with other
12
iotests that use VM.wait_migration() and query-status afterwards, but
13
maybe they just occur even more rarely, or it is because they also wait
14
on the destination VM to be running.
15
16
Signed-off-by: Max Reitz <mreitz@redhat.com>
17
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
1
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
18
---
2
---
19
tests/qemu-iotests/iotests.py | 6 +++++-
3
tests/test-bdrv-drain.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++++
20
tests/qemu-iotests/234 | 8 ++++----
4
1 file changed, 80 insertions(+)
21
tests/qemu-iotests/262 | 4 ++--
22
tests/qemu-iotests/280 | 2 +-
23
4 files changed, 12 insertions(+), 8 deletions(-)
24
5
25
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
6
diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c
26
index XXXXXXX..XXXXXXX 100644
7
index XXXXXXX..XXXXXXX 100644
27
--- a/tests/qemu-iotests/iotests.py
8
--- a/tests/test-bdrv-drain.c
28
+++ b/tests/qemu-iotests/iotests.py
9
+++ b/tests/test-bdrv-drain.c
29
@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):
10
@@ -XXX,XX +XXX,XX @@ static void test_multiparent(void)
30
}
11
blk_unref(blk_b);
31
]))
12
}
32
13
33
- def wait_migration(self):
14
+static void test_graph_change(void)
34
+ def wait_migration(self, expect_runstate):
15
+{
35
while True:
16
+ BlockBackend *blk_a, *blk_b;
36
event = self.event_wait('MIGRATION')
17
+ BlockDriverState *bs_a, *bs_b, *backing;
37
log(event, filters=[filter_qmp_event])
18
+ BDRVTestState *a_s, *b_s, *backing_s;
38
if event['data']['status'] == 'completed':
19
+
39
break
20
+ blk_a = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
40
+ # The event may occur in finish-migrate, so wait for the expected
21
+ bs_a = bdrv_new_open_driver(&bdrv_test, "test-node-a", BDRV_O_RDWR,
41
+ # post-migration runstate
22
+ &error_abort);
42
+ while self.qmp('query-status')['return']['status'] != expect_runstate:
23
+ a_s = bs_a->opaque;
43
+ pass
24
+ blk_insert_bs(blk_a, bs_a, &error_abort);
44
25
+
45
def node_info(self, node_name):
26
+ blk_b = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
46
nodes = self.qmp('query-named-block-nodes')
27
+ bs_b = bdrv_new_open_driver(&bdrv_test, "test-node-b", BDRV_O_RDWR,
47
diff --git a/tests/qemu-iotests/234 b/tests/qemu-iotests/234
28
+ &error_abort);
48
index XXXXXXX..XXXXXXX 100755
29
+ b_s = bs_b->opaque;
49
--- a/tests/qemu-iotests/234
30
+ blk_insert_bs(blk_b, bs_b, &error_abort);
50
+++ b/tests/qemu-iotests/234
31
+
51
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
32
+ backing = bdrv_new_open_driver(&bdrv_test, "backing", 0, &error_abort);
52
iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo_a)))
33
+ backing_s = backing->opaque;
53
with iotests.Timeout(3, 'Migration does not complete'):
34
+ bdrv_set_backing_hd(bs_a, backing, &error_abort);
54
# Wait for the source first (which includes setup=setup)
35
+
55
- vm_a.wait_migration()
36
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 0);
56
+ vm_a.wait_migration('postmigrate')
37
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 0);
57
# Wait for the destination second (which does not)
38
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
58
- vm_b.wait_migration()
39
+ g_assert_cmpint(a_s->drain_count, ==, 0);
59
+ vm_b.wait_migration('running')
40
+ g_assert_cmpint(b_s->drain_count, ==, 0);
60
41
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
61
iotests.log(vm_a.qmp('query-migrate')['return']['status'])
42
+
62
iotests.log(vm_b.qmp('query-migrate')['return']['status'])
43
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_a);
63
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
44
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_a);
64
iotests.log(vm_b.qmp('migrate', uri='exec:cat >%s' % (fifo_b)))
45
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_a);
65
with iotests.Timeout(3, 'Migration does not complete'):
46
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_b);
66
# Wait for the source first (which includes setup=setup)
47
+ do_drain_begin(BDRV_SUBTREE_DRAIN, bs_b);
67
- vm_b.wait_migration()
48
+
68
+ vm_b.wait_migration('postmigrate')
49
+ bdrv_set_backing_hd(bs_b, backing, &error_abort);
69
# Wait for the destination second (which does not)
50
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 5);
70
- vm_a.wait_migration()
51
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 5);
71
+ vm_a.wait_migration('running')
52
+ g_assert_cmpint(backing->quiesce_counter, ==, 5);
72
53
+ g_assert_cmpint(a_s->drain_count, ==, 5);
73
iotests.log(vm_a.qmp('query-migrate')['return']['status'])
54
+ g_assert_cmpint(b_s->drain_count, ==, 5);
74
iotests.log(vm_b.qmp('query-migrate')['return']['status'])
55
+ g_assert_cmpint(backing_s->drain_count, ==, 5);
75
diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262
56
+
76
index XXXXXXX..XXXXXXX 100755
57
+ bdrv_set_backing_hd(bs_b, NULL, &error_abort);
77
--- a/tests/qemu-iotests/262
58
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 3);
78
+++ b/tests/qemu-iotests/262
59
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 2);
79
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \
60
+ g_assert_cmpint(backing->quiesce_counter, ==, 3);
80
iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo)))
61
+ g_assert_cmpint(a_s->drain_count, ==, 3);
81
with iotests.Timeout(3, 'Migration does not complete'):
62
+ g_assert_cmpint(b_s->drain_count, ==, 2);
82
# Wait for the source first (which includes setup=setup)
63
+ g_assert_cmpint(backing_s->drain_count, ==, 3);
83
- vm_a.wait_migration()
64
+
84
+ vm_a.wait_migration('postmigrate')
65
+ bdrv_set_backing_hd(bs_b, backing, &error_abort);
85
# Wait for the destination second (which does not)
66
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 5);
86
- vm_b.wait_migration()
67
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 5);
87
+ vm_b.wait_migration('running')
68
+ g_assert_cmpint(backing->quiesce_counter, ==, 5);
88
69
+ g_assert_cmpint(a_s->drain_count, ==, 5);
89
iotests.log(vm_a.qmp('query-migrate')['return']['status'])
70
+ g_assert_cmpint(b_s->drain_count, ==, 5);
90
iotests.log(vm_b.qmp('query-migrate')['return']['status'])
71
+ g_assert_cmpint(backing_s->drain_count, ==, 5);
91
diff --git a/tests/qemu-iotests/280 b/tests/qemu-iotests/280
72
+
92
index XXXXXXX..XXXXXXX 100755
73
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_b);
93
--- a/tests/qemu-iotests/280
74
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_b);
94
+++ b/tests/qemu-iotests/280
75
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_a);
95
@@ -XXX,XX +XXX,XX @@ with iotests.FilePath('base') as base_path , \
76
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_a);
96
vm.qmp_log('migrate', uri='exec:cat > /dev/null')
77
+ do_drain_end(BDRV_SUBTREE_DRAIN, bs_a);
97
78
+
98
with iotests.Timeout(3, 'Migration does not complete'):
79
+ g_assert_cmpint(bs_a->quiesce_counter, ==, 0);
99
- vm.wait_migration()
80
+ g_assert_cmpint(bs_b->quiesce_counter, ==, 0);
100
+ vm.wait_migration('postmigrate')
81
+ g_assert_cmpint(backing->quiesce_counter, ==, 0);
101
82
+ g_assert_cmpint(a_s->drain_count, ==, 0);
102
iotests.log('\nVM is now stopped:')
83
+ g_assert_cmpint(b_s->drain_count, ==, 0);
103
iotests.log(vm.qmp('query-migrate')['return']['status'])
84
+ g_assert_cmpint(backing_s->drain_count, ==, 0);
85
+
86
+ bdrv_unref(backing);
87
+ bdrv_unref(bs_a);
88
+ bdrv_unref(bs_b);
89
+ blk_unref(blk_a);
90
+ blk_unref(blk_b);
91
+}
92
+
93
94
typedef struct TestBlockJob {
95
BlockJob common;
96
@@ -XXX,XX +XXX,XX @@ int main(int argc, char **argv)
97
98
g_test_add_func("/bdrv-drain/nested", test_nested);
99
g_test_add_func("/bdrv-drain/multiparent", test_multiparent);
100
+ g_test_add_func("/bdrv-drain/graph-change", test_graph_change);
101
102
g_test_add_func("/bdrv-drain/blockjob/drain_all", test_blockjob_drain_all);
103
g_test_add_func("/bdrv-drain/blockjob/drain", test_blockjob_drain);
104
--
104
--
105
2.20.1
105
2.13.6
106
106
107
107
diff view generated by jsdifflib
New patch
1
Since commit bde70715, base is the only node that is reopened in
2
commit_start(). This means that the code, which still involves an
3
explicit BlockReopenQueue, can now be simplified by using bdrv_reopen().
1
4
5
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
6
Reviewed-by: Fam Zheng <famz@redhat.com>
7
---
8
block/commit.c | 8 +-------
9
1 file changed, 1 insertion(+), 7 deletions(-)
10
11
diff --git a/block/commit.c b/block/commit.c
12
index XXXXXXX..XXXXXXX 100644
13
--- a/block/commit.c
14
+++ b/block/commit.c
15
@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,
16
const char *filter_node_name, Error **errp)
17
{
18
CommitBlockJob *s;
19
- BlockReopenQueue *reopen_queue = NULL;
20
int orig_base_flags;
21
BlockDriverState *iter;
22
BlockDriverState *commit_top_bs = NULL;
23
@@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs,
24
/* convert base to r/w, if necessary */
25
orig_base_flags = bdrv_get_flags(base);
26
if (!(orig_base_flags & BDRV_O_RDWR)) {
27
- reopen_queue = bdrv_reopen_queue(reopen_queue, base, NULL,
28
- orig_base_flags | BDRV_O_RDWR);
29
- }
30
-
31
- if (reopen_queue) {
32
- bdrv_reopen_multiple(bdrv_get_aio_context(bs), reopen_queue, &local_err);
33
+ bdrv_reopen(base, orig_base_flags | BDRV_O_RDWR, &local_err);
34
if (local_err != NULL) {
35
error_propagate(errp, local_err);
36
goto fail;
37
--
38
2.13.6
39
40
diff view generated by jsdifflib
New patch
1
The bdrv_reopen*() implementation doesn't like it if the graph is
2
changed between queuing nodes for reopen and actually reopening them
3
(one of the reasons is that queuing can be recursive).
1
4
5
So instead of draining the device only in bdrv_reopen_multiple(),
6
require that callers already drained all affected nodes, and assert this
7
in bdrv_reopen_queue().
8
9
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
10
Reviewed-by: Fam Zheng <famz@redhat.com>
11
---
12
block.c | 23 ++++++++++++++++-------
13
block/replication.c | 6 ++++++
14
qemu-io-cmds.c | 3 +++
15
3 files changed, 25 insertions(+), 7 deletions(-)
16
17
diff --git a/block.c b/block.c
18
index XXXXXXX..XXXXXXX 100644
19
--- a/block.c
20
+++ b/block.c
21
@@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_open(const char *filename, const char *reference,
22
* returns a pointer to bs_queue, which is either the newly allocated
23
* bs_queue, or the existing bs_queue being used.
24
*
25
+ * bs must be drained between bdrv_reopen_queue() and bdrv_reopen_multiple().
26
*/
27
static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
28
BlockDriverState *bs,
29
@@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue,
30
BdrvChild *child;
31
QDict *old_options, *explicit_options;
32
33
+ /* Make sure that the caller remembered to use a drained section. This is
34
+ * important to avoid graph changes between the recursive queuing here and
35
+ * bdrv_reopen_multiple(). */
36
+ assert(bs->quiesce_counter > 0);
37
+
38
if (bs_queue == NULL) {
39
bs_queue = g_new0(BlockReopenQueue, 1);
40
QSIMPLEQ_INIT(bs_queue);
41
@@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue,
42
* If all devices prepare successfully, then the changes are committed
43
* to all devices.
44
*
45
+ * All affected nodes must be drained between bdrv_reopen_queue() and
46
+ * bdrv_reopen_multiple().
47
*/
48
int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **errp)
49
{
50
@@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **er
51
52
assert(bs_queue != NULL);
53
54
- aio_context_release(ctx);
55
- bdrv_drain_all_begin();
56
- aio_context_acquire(ctx);
57
-
58
QSIMPLEQ_FOREACH(bs_entry, bs_queue, entry) {
59
+ assert(bs_entry->state.bs->quiesce_counter > 0);
60
if (bdrv_reopen_prepare(&bs_entry->state, bs_queue, &local_err)) {
61
error_propagate(errp, local_err);
62
goto cleanup;
63
@@ -XXX,XX +XXX,XX @@ cleanup:
64
}
65
g_free(bs_queue);
66
67
- bdrv_drain_all_end();
68
-
69
return ret;
70
}
71
72
@@ -XXX,XX +XXX,XX @@ int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp)
73
{
74
int ret = -1;
75
Error *local_err = NULL;
76
- BlockReopenQueue *queue = bdrv_reopen_queue(NULL, bs, NULL, bdrv_flags);
77
+ BlockReopenQueue *queue;
78
79
+ bdrv_subtree_drained_begin(bs);
80
+
81
+ queue = bdrv_reopen_queue(NULL, bs, NULL, bdrv_flags);
82
ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, &local_err);
83
if (local_err != NULL) {
84
error_propagate(errp, local_err);
85
}
86
+
87
+ bdrv_subtree_drained_end(bs);
88
+
89
return ret;
90
}
91
92
diff --git a/block/replication.c b/block/replication.c
93
index XXXXXXX..XXXXXXX 100644
94
--- a/block/replication.c
95
+++ b/block/replication.c
96
@@ -XXX,XX +XXX,XX @@ static void reopen_backing_file(BlockDriverState *bs, bool writable,
97
new_secondary_flags = s->orig_secondary_flags;
98
}
99
100
+ bdrv_subtree_drained_begin(s->hidden_disk->bs);
101
+ bdrv_subtree_drained_begin(s->secondary_disk->bs);
102
+
103
if (orig_hidden_flags != new_hidden_flags) {
104
reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, NULL,
105
new_hidden_flags);
106
@@ -XXX,XX +XXX,XX @@ static void reopen_backing_file(BlockDriverState *bs, bool writable,
107
reopen_queue, &local_err);
108
error_propagate(errp, local_err);
109
}
110
+
111
+ bdrv_subtree_drained_end(s->hidden_disk->bs);
112
+ bdrv_subtree_drained_end(s->secondary_disk->bs);
113
}
114
115
static void backup_job_cleanup(BlockDriverState *bs)
116
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
117
index XXXXXXX..XXXXXXX 100644
118
--- a/qemu-io-cmds.c
119
+++ b/qemu-io-cmds.c
120
@@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv)
121
opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL;
122
qemu_opts_reset(&reopen_opts);
123
124
+ bdrv_subtree_drained_begin(bs);
125
brq = bdrv_reopen_queue(NULL, bs, opts, flags);
126
bdrv_reopen_multiple(bdrv_get_aio_context(bs), brq, &local_err);
127
+ bdrv_subtree_drained_end(bs);
128
+
129
if (local_err) {
130
error_report_err(local_err);
131
} else {
132
--
133
2.13.6
134
135
diff view generated by jsdifflib