1 | The following changes since commit 9964e96dc9999cf7f7c936ee854a795415d19b60: | 1 | The following changes since commit 0280396a33c7210c4df5306afeab63411a41535a: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'jasowang/tags/net-pull-request' into staging (2017-05-23 15:01:31 +0100) | 3 | Merge remote-tracking branch 'remotes/stsquad/tags/pull-testing-gdbstub-150221-1' into staging (2021-02-15 10:13:13 +0000) |
4 | 4 | ||
5 | are available in the git repository at: | 5 | are available in the Git repository at: |
6 | |||
7 | 6 | ||
8 | git://repo.or.cz/qemu/kevin.git tags/for-upstream | 7 | git://repo.or.cz/qemu/kevin.git tags/for-upstream |
9 | 8 | ||
10 | for you to fetch changes up to 42a48128417b3bfade93d1a4721348cc480e9e50: | 9 | for you to fetch changes up to b248e61652e20c3353af4b0ccb90f17d76f4db21: |
11 | 10 | ||
12 | Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block (2017-05-29 16:34:27 +0200) | 11 | monitor/qmp: Stop processing requests when shutdown is requested (2021-02-15 15:10:14 +0100) |
13 | 12 | ||
14 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block layer patches: | ||
15 | 15 | ||
16 | Block layer patches | 16 | - qemu-storage-daemon: Enable object-add |
17 | - blockjob: Fix crash with IOthread when block commit after snapshot | ||
18 | - monitor: Shutdown fixes | ||
19 | - xen-block: fix reporting of discard feature | ||
20 | - qcow2: Remove half-initialised image file after failed image creation | ||
21 | - ahci: Fix DMA direction | ||
22 | - iotests fixes | ||
17 | 23 | ||
18 | ---------------------------------------------------------------- | 24 | ---------------------------------------------------------------- |
19 | Alberto Garcia (2): | 25 | Alexander Bulekov (1): |
20 | stream: fix crash in stream_start() when block_job_create() fails | 26 | hw/ide/ahci: map cmd_fis as DMA_DIRECTION_TO_DEVICE |
21 | qcow2: remove extra local_error variable | ||
22 | |||
23 | Daniel P. Berrange (4): | ||
24 | qemu-img: add support for --object with 'dd' command | ||
25 | qemu-img: fix --image-opts usage with dd command | ||
26 | qemu-img: introduce --target-image-opts for 'convert' command | ||
27 | qemu-img: copy *key-secret opts when opening newly created files | ||
28 | |||
29 | Eric Blake (1): | ||
30 | block: Tweak error message related to qemu-img amend | ||
31 | |||
32 | Fam Zheng (3): | ||
33 | iotests: 147: Don't test inet6 if not available | ||
34 | qemu-img: Fix documentation of convert | ||
35 | qemu-img: Fix leakage of options on error | ||
36 | 27 | ||
37 | Kevin Wolf (3): | 28 | Kevin Wolf (3): |
38 | qemu-iotests: Test streaming with missing job ID | 29 | qemu-storage-daemon: Enable object-add |
39 | mirror: Drop permissions on s->target on completion | 30 | monitor: Fix assertion failure on shutdown |
40 | Merge remote-tracking branch 'mreitz/tags/pull-block-2017-05-29-v3' into queue-block | 31 | monitor/qmp: Stop processing requests when shutdown is requested |
41 | 32 | ||
42 | Max Reitz (2): | 33 | Max Reitz (1): |
43 | block: Fix backing paths for filenames with colons | 34 | iotests: Consistent $IMGOPTS boundary matching |
44 | block/file-*: *_parse_filename() and colons | ||
45 | 35 | ||
46 | Stephen Bates (1): | 36 | Maxim Levitsky (3): |
47 | nvme: Add support for Controller Memory Buffers | 37 | crypto: luks: Fix tiny memory leak |
38 | block: add bdrv_co_delete_file_noerr | ||
39 | block: qcow2: remove the created file on initialization error | ||
48 | 40 | ||
49 | block.c | 50 +++++++++++++-- | 41 | Michael Qiu (1): |
50 | block/file-posix.c | 17 +----- | 42 | blockjob: Fix crash with IOthread when block commit after snapshot |
51 | block/file-win32.c | 12 +--- | ||
52 | block/mirror.c | 7 ++- | ||
53 | block/qcow2-cluster.c | 3 +- | ||
54 | block/qcow2.c | 5 +- | ||
55 | block/stream.c | 2 +- | ||
56 | hw/block/nvme.c | 75 +++++++++++++++++++++-- | ||
57 | hw/block/nvme.h | 73 ++++++++++++++++++++++ | ||
58 | include/block/block_int.h | 3 + | ||
59 | qemu-img-cmds.hx | 4 +- | ||
60 | qemu-img.c | 148 +++++++++++++++++++++++++++++++++++---------- | ||
61 | qemu-img.texi | 12 +++- | ||
62 | tests/qemu-iotests/030 | 4 ++ | ||
63 | tests/qemu-iotests/030.out | 4 +- | ||
64 | tests/qemu-iotests/060.out | 2 +- | ||
65 | tests/qemu-iotests/147 | 7 +++ | ||
66 | 17 files changed, 351 insertions(+), 77 deletions(-) | ||
67 | 43 | ||
44 | Roger Pau Monné (1): | ||
45 | xen-block: fix reporting of discard feature | ||
46 | |||
47 | Thomas Huth (1): | ||
48 | tests/qemu-iotests: Remove test 259 from the "auto" group | ||
49 | |||
50 | include/block/block.h | 1 + | ||
51 | block.c | 22 ++++++++++++++++++++++ | ||
52 | block/crypto.c | 13 ++----------- | ||
53 | block/qcow2.c | 8 +++++--- | ||
54 | blockjob.c | 8 ++++++-- | ||
55 | hw/block/xen-block.c | 1 + | ||
56 | hw/ide/ahci.c | 12 ++++++------ | ||
57 | monitor/monitor.c | 25 +++++++++++++++---------- | ||
58 | monitor/qmp.c | 5 +++++ | ||
59 | storage-daemon/qemu-storage-daemon.c | 2 ++ | ||
60 | tests/qemu-iotests/259 | 2 +- | ||
61 | tests/qemu-iotests/common.rc | 4 +++- | ||
62 | 12 files changed, 69 insertions(+), 34 deletions(-) | ||
63 | |||
64 | diff view generated by jsdifflib |
1 | From: Eric Blake <eblake@redhat.com> | 1 | As we don't have a fully QAPIfied version of object-add yet and it still |
---|---|---|---|
2 | has 'gen': false in the schema, it needs to be registered explicitly in | ||
3 | init_qmp_commands() to be available for users. | ||
2 | 4 | ||
3 | When converting a 1.1 image down to 0.10, qemu-iotests 060 forces | 5 | Fixes: 2af282ec51a27116d0402cab237b8970800f870c |
4 | a contrived failure where allocating a cluster used to replace a | 6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
5 | zero cluster reads unaligned data. Since it is a zero cluster | 7 | Message-Id: <20210204072137.19663-1-kwolf@redhat.com> |
6 | rather than a data cluster being converted, changing the error | 8 | Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> |
7 | message to match our earlier change in 'qcow2: Make distinction | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | between zero cluster types obvious' is worthwhile. | 10 | --- |
11 | storage-daemon/qemu-storage-daemon.c | 2 ++ | ||
12 | 1 file changed, 2 insertions(+) | ||
9 | 13 | ||
10 | Suggested-by: Max Reitz <mreitz@redhat.com> | 14 | diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c |
11 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
12 | Message-id: 20170508171302.17805-1-eblake@redhat.com | ||
13 | [mreitz: Commit message fixes] | ||
14 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
15 | --- | ||
16 | block/qcow2-cluster.c | 3 ++- | ||
17 | tests/qemu-iotests/060.out | 2 +- | ||
18 | 2 files changed, 3 insertions(+), 2 deletions(-) | ||
19 | |||
20 | diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c | ||
21 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/block/qcow2-cluster.c | 16 | --- a/storage-daemon/qemu-storage-daemon.c |
23 | +++ b/block/qcow2-cluster.c | 17 | +++ b/storage-daemon/qemu-storage-daemon.c |
24 | @@ -XXX,XX +XXX,XX @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table, | 18 | @@ -XXX,XX +XXX,XX @@ static void init_qmp_commands(void) |
25 | } | 19 | qmp_init_marshal(&qmp_commands); |
26 | 20 | qmp_register_command(&qmp_commands, "query-qmp-schema", | |
27 | if (offset_into_cluster(s, offset)) { | 21 | qmp_query_qmp_schema, QCO_ALLOW_PRECONFIG); |
28 | - qcow2_signal_corruption(bs, true, -1, -1, "Data cluster offset " | 22 | + qmp_register_command(&qmp_commands, "object-add", qmp_object_add, |
29 | + qcow2_signal_corruption(bs, true, -1, -1, | 23 | + QCO_NO_OPTIONS); |
30 | + "Cluster allocation offset " | 24 | |
31 | "%#" PRIx64 " unaligned (L2 offset: %#" | 25 | QTAILQ_INIT(&qmp_cap_negotiation_commands); |
32 | PRIx64 ", L2 index: %#x)", offset, | 26 | qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities", |
33 | l2_offset, j); | ||
34 | diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out | ||
35 | index XXXXXXX..XXXXXXX 100644 | ||
36 | --- a/tests/qemu-iotests/060.out | ||
37 | +++ b/tests/qemu-iotests/060.out | ||
38 | @@ -XXX,XX +XXX,XX @@ read failed: Input/output error | ||
39 | Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 | ||
40 | wrote 65536/65536 bytes at offset 0 | ||
41 | 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
42 | -qcow2: Marking image as corrupt: Data cluster offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed | ||
43 | +qcow2: Marking image as corrupt: Cluster allocation offset 0x52a00 unaligned (L2 offset: 0x40000, L2 index: 0); further corruption events will be suppressed | ||
44 | qemu-img: Error while amending options: Input/output error | ||
45 | |||
46 | === Testing unaligned reftable entry === | ||
47 | -- | 27 | -- |
48 | 1.8.3.1 | 28 | 2.29.2 |
49 | 29 | ||
50 | 30 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Max Reitz <mreitz@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | path_combine() naturally tries to preserve a protocol prefix. However, | 3 | To disallow certain refcount_bits values, some _unsupported_imgopts |
4 | it recognizes such a prefix by scanning for the first colon; which is | 4 | invocations look like "refcount_bits=1[^0-9]", i.e. they match an |
5 | different from what path_has_protocol() does: There only is a protocol | 5 | integer boundary with [^0-9]. This expression does not match the end of |
6 | prefix if there is a colon before the first slash. | 6 | the string, though, so it breaks down when refcount_bits is the last |
7 | option (which it tends to be after the rewrite of the check script in | ||
8 | Python). | ||
7 | 9 | ||
8 | A protocol prefix that is not recognized by path_has_protocol() is none, | 10 | Those invocations could use \b or \> instead, but those are not |
9 | and should thus not be taken as one. | 11 | portable. They could use something like \([^0-9]\|$\), but that would |
12 | be cumbersome. To make it simple and keep the existing invocations | ||
13 | working, just let _unsupported_imgopts match the regex against $IMGOPTS | ||
14 | plus a trailing space. | ||
10 | 15 | ||
11 | Case in point, before this patch: | 16 | Suggested-by: Eric Blake <eblake@redhat.com> |
12 | $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 | 17 | Signed-off-by: Max Reitz <mreitz@redhat.com> |
13 | qemu-img: ./top:image.qcow2: Could not open './top:backing.qcow2': | 18 | Message-Id: <20210210095128.22732-1-mreitz@redhat.com> |
14 | No such file or directory | 19 | Reviewed-by: Eric Blake <eblake@redhat.com> |
20 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
21 | --- | ||
22 | tests/qemu-iotests/common.rc | 4 +++- | ||
23 | 1 file changed, 3 insertions(+), 1 deletion(-) | ||
15 | 24 | ||
16 | Afterwards: | 25 | diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc |
17 | $ ./qemu-img create -f qcow2 -b backing.qcow2 ./top:image.qcow2 | ||
18 | qemu-img: ./top:image.qcow2: Could not open './backing.qcow2': | ||
19 | No such file or directory | ||
20 | |||
21 | Reported-by: yangyang <yangyang@redhat.com> | ||
22 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
23 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
24 | Message-id: 20170522195217.12991-2-mreitz@redhat.com | ||
25 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
26 | --- | ||
27 | block.c | 15 ++++++++++----- | ||
28 | 1 file changed, 10 insertions(+), 5 deletions(-) | ||
29 | |||
30 | diff --git a/block.c b/block.c | ||
31 | index XXXXXXX..XXXXXXX 100644 | 26 | index XXXXXXX..XXXXXXX 100644 |
32 | --- a/block.c | 27 | --- a/tests/qemu-iotests/common.rc |
33 | +++ b/block.c | 28 | +++ b/tests/qemu-iotests/common.rc |
34 | @@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size, | 29 | @@ -XXX,XX +XXX,XX @@ _unsupported_imgopts() |
35 | if (path_is_absolute(filename)) { | 30 | { |
36 | pstrcpy(dest, dest_size, filename); | 31 | for bad_opt |
37 | } else { | 32 | do |
38 | - p = strchr(base_path, ':'); | 33 | - if echo "$IMGOPTS" | grep -q 2>/dev/null "$bad_opt" |
39 | - if (p) | 34 | + # Add a space so tests can match for whitespace that marks the |
40 | - p++; | 35 | + # end of an option (\b or \> are not portable) |
41 | - else | 36 | + if echo "$IMGOPTS " | grep -q 2>/dev/null "$bad_opt" |
42 | - p = base_path; | 37 | then |
43 | + const char *protocol_stripped = NULL; | 38 | _notrun "not suitable for image option: $bad_opt" |
44 | + | 39 | fi |
45 | + if (path_has_protocol(base_path)) { | ||
46 | + protocol_stripped = strchr(base_path, ':'); | ||
47 | + if (protocol_stripped) { | ||
48 | + protocol_stripped++; | ||
49 | + } | ||
50 | + } | ||
51 | + p = protocol_stripped ?: base_path; | ||
52 | + | ||
53 | p1 = strrchr(base_path, '/'); | ||
54 | #ifdef _WIN32 | ||
55 | { | ||
56 | -- | 40 | -- |
57 | 1.8.3.1 | 41 | 2.29.2 |
58 | 42 | ||
59 | 43 | diff view generated by jsdifflib |
1 | From: Fam Zheng <famz@redhat.com> | 1 | From: Michael Qiu <qiudayu@huayun.com> |
---|---|---|---|
2 | 2 | ||
3 | This is the case in our docker tests, as we use --net=none there. Skip | 3 | Currently, if guest has workloads, IO thread will acquire aio_context |
4 | this method. | 4 | lock before do io_submit, it leads to segmentfault when do block commit |
5 | after snapshot. Just like below: | ||
5 | 6 | ||
6 | Signed-off-by: Fam Zheng <famz@redhat.com> | 7 | Program received signal SIGSEGV, Segmentation fault. |
8 | |||
9 | [Switching to Thread 0x7f7c7d91f700 (LWP 99907)] | ||
10 | 0x00005576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437 | ||
11 | 1437 ../block/mirror.c: No such file or directory. | ||
12 | (gdb) p s->job | ||
13 | $17 = (MirrorBlockJob *) 0x0 | ||
14 | (gdb) p s->stop | ||
15 | $18 = false | ||
16 | |||
17 | Call trace of IO thread: | ||
18 | 0 0x00005576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437 | ||
19 | 1 0x00005576d0f7f3ab in bdrv_driver_pwritev at ../block/io.c:1174 | ||
20 | 2 0x00005576d0f8139d in bdrv_aligned_pwritev at ../block/io.c:1988 | ||
21 | 3 0x00005576d0f81b65 in bdrv_co_pwritev_part at ../block/io.c:2156 | ||
22 | 4 0x00005576d0f8e6b7 in blk_do_pwritev_part at ../block/block-backend.c:1260 | ||
23 | 5 0x00005576d0f8e84d in blk_aio_write_entry at ../block/block-backend.c:1476 | ||
24 | ... | ||
25 | |||
26 | Switch to qemu main thread: | ||
27 | 0 0x00007f903be704ed in __lll_lock_wait at | ||
28 | /lib/../lib64/libpthread.so.0 | ||
29 | 1 0x00007f903be6bde6 in _L_lock_941 at /lib/../lib64/libpthread.so.0 | ||
30 | 2 0x00007f903be6bcdf in pthread_mutex_lock at | ||
31 | /lib/../lib64/libpthread.so.0 | ||
32 | 3 0x0000564b21456889 in qemu_mutex_lock_impl at | ||
33 | ../util/qemu-thread-posix.c:79 | ||
34 | 4 0x0000564b213af8a5 in block_job_add_bdrv at ../blockjob.c:224 | ||
35 | 5 0x0000564b213b00ad in block_job_create at ../blockjob.c:440 | ||
36 | 6 0x0000564b21357c0a in mirror_start_job at ../block/mirror.c:1622 | ||
37 | 7 0x0000564b2135a9af in commit_active_start at ../block/mirror.c:1867 | ||
38 | 8 0x0000564b2133d132 in qmp_block_commit at ../blockdev.c:2768 | ||
39 | 9 0x0000564b2141fef3 in qmp_marshal_block_commit at | ||
40 | qapi/qapi-commands-block-core.c:346 | ||
41 | 10 0x0000564b214503c9 in do_qmp_dispatch_bh at | ||
42 | ../qapi/qmp-dispatch.c:110 | ||
43 | 11 0x0000564b21451996 in aio_bh_poll at ../util/async.c:164 | ||
44 | 12 0x0000564b2146018e in aio_dispatch at ../util/aio-posix.c:381 | ||
45 | 13 0x0000564b2145187e in aio_ctx_dispatch at ../util/async.c:306 | ||
46 | 14 0x00007f9040239049 in g_main_context_dispatch at | ||
47 | /lib/../lib64/libglib-2.0.so.0 | ||
48 | 15 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:232 | ||
49 | 16 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:255 | ||
50 | 17 0x0000564b21447368 in main_loop_wait at ../util/main-loop.c:531 | ||
51 | 18 0x0000564b212304e1 in qemu_main_loop at ../softmmu/runstate.c:721 | ||
52 | 19 0x0000564b20f7975e in main at ../softmmu/main.c:50 | ||
53 | |||
54 | In IO thread when do bdrv_mirror_top_pwritev, the job is NULL, and stop field | ||
55 | is false, this means the MirrorBDSOpaque "s" object has not been initialized | ||
56 | yet, and this object is initialized by block_job_create(), but the initialize | ||
57 | process is stuck in acquiring the lock. | ||
58 | |||
59 | In this situation, IO thread come to bdrv_mirror_top_pwritev(),which means that | ||
60 | mirror-top node is already inserted into block graph, but its bs->opaque->job | ||
61 | is not initialized. | ||
62 | |||
63 | The root cause is that qemu main thread do release/acquire when hold the lock, | ||
64 | at the same time, IO thread get the lock after release stage, and the crash | ||
65 | occured. | ||
66 | |||
67 | Actually, in this situation, job->job.aio_context will not equal to | ||
68 | qemu_get_aio_context(), and will be the same as bs->aio_context, | ||
69 | thus, no need to release the lock, becasue bdrv_root_attach_child() | ||
70 | will not change the context. | ||
71 | |||
72 | This patch fix this issue. | ||
73 | |||
74 | Fixes: 132ada80 "block: Adjust AioContexts when attaching nodes" | ||
75 | |||
76 | Signed-off-by: Michael Qiu <qiudayu@huayun.com> | ||
77 | Message-Id: <20210203024059.52683-1-08005325@163.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 78 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 79 | --- |
9 | tests/qemu-iotests/147 | 7 +++++++ | 80 | blockjob.c | 8 ++++++-- |
10 | 1 file changed, 7 insertions(+) | 81 | 1 file changed, 6 insertions(+), 2 deletions(-) |
11 | 82 | ||
12 | diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147 | 83 | diff --git a/blockjob.c b/blockjob.c |
13 | index XXXXXXX..XXXXXXX 100755 | 84 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/tests/qemu-iotests/147 | 85 | --- a/blockjob.c |
15 | +++ b/tests/qemu-iotests/147 | 86 | +++ b/blockjob.c |
16 | @@ -XXX,XX +XXX,XX @@ class BuiltinNBD(NBDBlockdevAddBase): | 87 | @@ -XXX,XX +XXX,XX @@ int block_job_add_bdrv(BlockJob *job, const char *name, BlockDriverState *bs, |
17 | self._server_down() | 88 | uint64_t perm, uint64_t shared_perm, Error **errp) |
18 | 89 | { | |
19 | def test_inet6(self): | 90 | BdrvChild *c; |
20 | + try: | 91 | + bool need_context_ops; |
21 | + socket.getaddrinfo("::0", "0", socket.AF_INET6, | 92 | |
22 | + socket.SOCK_STREAM, socket.IPPROTO_TCP, | 93 | bdrv_ref(bs); |
23 | + socket.AI_ADDRCONFIG | socket.AI_CANONNAME) | 94 | - if (job->job.aio_context != qemu_get_aio_context()) { |
24 | + except socket.gaierror: | 95 | + |
25 | + # IPv6 not available, skip | 96 | + need_context_ops = bdrv_get_aio_context(bs) != job->job.aio_context; |
26 | + return | 97 | + |
27 | address = { 'type': 'inet', | 98 | + if (need_context_ops && job->job.aio_context != qemu_get_aio_context()) { |
28 | 'data': { | 99 | aio_context_release(job->job.aio_context); |
29 | 'host': '::1', | 100 | } |
101 | c = bdrv_root_attach_child(bs, name, &child_job, 0, | ||
102 | job->job.aio_context, perm, shared_perm, job, | ||
103 | errp); | ||
104 | - if (job->job.aio_context != qemu_get_aio_context()) { | ||
105 | + if (need_context_ops && job->job.aio_context != qemu_get_aio_context()) { | ||
106 | aio_context_acquire(job->job.aio_context); | ||
107 | } | ||
108 | if (c == NULL) { | ||
30 | -- | 109 | -- |
31 | 1.8.3.1 | 110 | 2.29.2 |
32 | 111 | ||
33 | 112 | diff view generated by jsdifflib |
1 | From: "Daniel P. Berrange" <berrange@redhat.com> | 1 | From: Alexander Bulekov <alxndr@bu.edu> |
---|---|---|---|
2 | 2 | ||
3 | The --image-opts flag can only be used to affect the parsing | 3 | cmd_fis is mapped as DMA_DIRECTION_FROM_DEVICE, however, it is read |
4 | of the source image. The target image has to be specified in | 4 | from, and not written to anywhere. Fix the DMA_DIRECTION and mark |
5 | the traditional style regardless, since it needs to be passed | 5 | cmd_fis as read-only in the code. |
6 | to the bdrv_create() API which does not support the new style | ||
7 | opts. | ||
8 | 6 | ||
9 | Reviewed-by: Fam Zheng <famz@redhat.com> | 7 | Signed-off-by: Alexander Bulekov <alxndr@bu.edu> |
10 | Reviewed-by: Max Reitz <mreitz@redhat.com> | 8 | Message-Id: <20210119164051.89268-1-alxndr@bu.edu> |
11 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | Message-id: 20170515164712.6643-3-berrange@redhat.com | ||
13 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
14 | --- | 10 | --- |
15 | qemu-img.c | 9 +++++++-- | 11 | hw/ide/ahci.c | 12 ++++++------ |
16 | 1 file changed, 7 insertions(+), 2 deletions(-) | 12 | 1 file changed, 6 insertions(+), 6 deletions(-) |
17 | 13 | ||
18 | diff --git a/qemu-img.c b/qemu-img.c | 14 | diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c |
19 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/qemu-img.c | 16 | --- a/hw/ide/ahci.c |
21 | +++ b/qemu-img.c | 17 | +++ b/hw/ide/ahci.c |
22 | @@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv) | 18 | @@ -XXX,XX +XXX,XX @@ static void ahci_reset_port(AHCIState *s, int port) |
23 | goto out; | 19 | } |
20 | |||
21 | /* Buffer pretty output based on a raw FIS structure. */ | ||
22 | -static char *ahci_pretty_buffer_fis(uint8_t *fis, int cmd_len) | ||
23 | +static char *ahci_pretty_buffer_fis(const uint8_t *fis, int cmd_len) | ||
24 | { | ||
25 | int i; | ||
26 | GString *s = g_string_new("FIS:"); | ||
27 | @@ -XXX,XX +XXX,XX @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) | ||
28 | } | ||
29 | |||
30 | |||
31 | -static void process_ncq_command(AHCIState *s, int port, uint8_t *cmd_fis, | ||
32 | +static void process_ncq_command(AHCIState *s, int port, const uint8_t *cmd_fis, | ||
33 | uint8_t slot) | ||
34 | { | ||
35 | AHCIDevice *ad = &s->dev[port]; | ||
36 | - NCQFrame *ncq_fis = (NCQFrame*)cmd_fis; | ||
37 | + const NCQFrame *ncq_fis = (NCQFrame *)cmd_fis; | ||
38 | uint8_t tag = ncq_fis->tag >> 3; | ||
39 | NCQTransferState *ncq_tfs = &ad->ncq_tfs[tag]; | ||
40 | size_t size; | ||
41 | @@ -XXX,XX +XXX,XX @@ static AHCICmdHdr *get_cmd_header(AHCIState *s, uint8_t port, uint8_t slot) | ||
42 | } | ||
43 | |||
44 | static void handle_reg_h2d_fis(AHCIState *s, int port, | ||
45 | - uint8_t slot, uint8_t *cmd_fis) | ||
46 | + uint8_t slot, const uint8_t *cmd_fis) | ||
47 | { | ||
48 | IDEState *ide_state = &s->dev[port].port.ifs[0]; | ||
49 | AHCICmdHdr *cmd = get_cmd_header(s, port, slot); | ||
50 | @@ -XXX,XX +XXX,XX @@ static int handle_cmd(AHCIState *s, int port, uint8_t slot) | ||
51 | tbl_addr = le64_to_cpu(cmd->tbl_addr); | ||
52 | cmd_len = 0x80; | ||
53 | cmd_fis = dma_memory_map(s->as, tbl_addr, &cmd_len, | ||
54 | - DMA_DIRECTION_FROM_DEVICE); | ||
55 | + DMA_DIRECTION_TO_DEVICE); | ||
56 | if (!cmd_fis) { | ||
57 | trace_handle_cmd_badfis(s, port); | ||
58 | return -1; | ||
59 | @@ -XXX,XX +XXX,XX @@ static int handle_cmd(AHCIState *s, int port, uint8_t slot) | ||
24 | } | 60 | } |
25 | 61 | ||
26 | - blk2 = img_open(image_opts, out.filename, out_fmt, BDRV_O_RDWR, | 62 | out: |
27 | - false, false, false); | 63 | - dma_memory_unmap(s->as, cmd_fis, cmd_len, DMA_DIRECTION_FROM_DEVICE, |
28 | + /* TODO, we can't honour --image-opts for the target, | 64 | + dma_memory_unmap(s->as, cmd_fis, cmd_len, DMA_DIRECTION_TO_DEVICE, |
29 | + * since it needs to be given in a format compatible | 65 | cmd_len); |
30 | + * with the bdrv_create() call above which does not | 66 | |
31 | + * support image-opts style. | 67 | if (s->dev[port].port.ifs[0].status & (BUSY_STAT|DRQ_STAT)) { |
32 | + */ | ||
33 | + blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR, | ||
34 | + false, false, false); | ||
35 | |||
36 | if (!blk2) { | ||
37 | ret = -1; | ||
38 | -- | 68 | -- |
39 | 1.8.3.1 | 69 | 2.29.2 |
40 | 70 | ||
41 | 71 | diff view generated by jsdifflib |
1 | From: Fam Zheng <famz@redhat.com> | 1 | From: Roger Pau Monne <roger.pau@citrix.com> |
---|---|---|---|
2 | 2 | ||
3 | Reported by Coverity. | 3 | Linux blkfront expects both "discard-granularity" and |
4 | "discard-alignment" present on xenbus in order to properly enable the | ||
5 | feature, not exposing "discard-alignment" left some Linux blkfront | ||
6 | versions with a broken discard setup. This has also been addressed in | ||
7 | Linux with: | ||
4 | 8 | ||
5 | Signed-off-by: Fam Zheng <famz@redhat.com> | 9 | https://lore.kernel.org/lkml/20210118151528.81668-1-roger.pau@citrix.com/T/#u |
6 | Message-id: 20170515141014.25793-1-famz@redhat.com | 10 | |
7 | Reviewed-by: Eric Blake <eblake@redhat.com> | 11 | Fix QEMU to report a "discard-alignment" of 0, in order for it to work |
8 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 12 | with older Linux frontends. |
13 | |||
14 | Reported-by: Arthur Borsboom <arthurborsboom@gmail.com> | ||
15 | Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> | ||
16 | Message-Id: <20210118153330.82324-1-roger.pau@citrix.com> | ||
17 | Reviewed-by: Paul Durrant <paul@xen.org> | ||
18 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | 19 | --- |
10 | qemu-img.c | 1 + | 20 | hw/block/xen-block.c | 1 + |
11 | 1 file changed, 1 insertion(+) | 21 | 1 file changed, 1 insertion(+) |
12 | 22 | ||
13 | diff --git a/qemu-img.c b/qemu-img.c | 23 | diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c |
14 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/qemu-img.c | 25 | --- a/hw/block/xen-block.c |
16 | +++ b/qemu-img.c | 26 | +++ b/hw/block/xen-block.c |
17 | @@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_opts(const char *optstr, | 27 | @@ -XXX,XX +XXX,XX @@ static void xen_block_realize(XenDevice *xendev, Error **errp) |
18 | if (qdict_haskey(options, BDRV_OPT_FORCE_SHARE) | 28 | xen_device_backend_printf(xendev, "feature-discard", "%u", 1); |
19 | && !qdict_get_bool(options, BDRV_OPT_FORCE_SHARE)) { | 29 | xen_device_backend_printf(xendev, "discard-granularity", "%u", |
20 | error_report("--force-share/-U conflicts with image options"); | 30 | conf->discard_granularity); |
21 | + QDECREF(options); | 31 | + xen_device_backend_printf(xendev, "discard-alignment", "%u", 0); |
22 | return NULL; | 32 | } |
23 | } | 33 | |
24 | qdict_put(options, BDRV_OPT_FORCE_SHARE, qbool_from_bool(true)); | 34 | xen_device_backend_printf(xendev, "feature-flush-cache", "%u", 1); |
25 | -- | 35 | -- |
26 | 1.8.3.1 | 36 | 2.29.2 |
27 | 37 | ||
28 | 38 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Thomas Huth <thuth@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The code that tries to reopen a BlockDriverState in stream_start() | 3 | Tests in the "auto" group should support qcow2 so that they can |
4 | when the creation of a new block job fails crashes because it attempts | 4 | be run during "make check-block". Test 259 only supports "raw", so |
5 | to dereference a pointer that is known to be NULL. | 5 | it currently always gets skipped when running "make check-block". |
6 | Let's skip this unnecessary step and remove it from the auto group. | ||
6 | 7 | ||
7 | This is a regression introduced in a170a91fd3eab6155da39e740381867e, | 8 | Signed-off-by: Thomas Huth <thuth@redhat.com> |
8 | likely because the code was copied from stream_complete(). | 9 | Message-Id: <20210215103835.1129145-1-thuth@redhat.com> |
9 | |||
10 | Cc: qemu-stable@nongnu.org | ||
11 | Reported-by: Kashyap Chamarthy <kchamart@redhat.com> | ||
12 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
13 | Tested-by: Kashyap Chamarthy <kchamart@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
15 | --- | 11 | --- |
16 | block/stream.c | 2 +- | 12 | tests/qemu-iotests/259 | 2 +- |
17 | 1 file changed, 1 insertion(+), 1 deletion(-) | 13 | 1 file changed, 1 insertion(+), 1 deletion(-) |
18 | 14 | ||
19 | diff --git a/block/stream.c b/block/stream.c | 15 | diff --git a/tests/qemu-iotests/259 b/tests/qemu-iotests/259 |
20 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100755 |
21 | --- a/block/stream.c | 17 | --- a/tests/qemu-iotests/259 |
22 | +++ b/block/stream.c | 18 | +++ b/tests/qemu-iotests/259 |
23 | @@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs, | 19 | @@ -XXX,XX +XXX,XX @@ |
24 | 20 | #!/usr/bin/env bash | |
25 | fail: | 21 | -# group: rw auto quick |
26 | if (orig_bs_flags != bdrv_get_flags(bs)) { | 22 | +# group: rw quick |
27 | - bdrv_reopen(bs, s->bs_flags, NULL); | 23 | # |
28 | + bdrv_reopen(bs, orig_bs_flags, NULL); | 24 | # Test generic image creation fallback (by using NBD) |
29 | } | 25 | # |
30 | } | ||
31 | -- | 26 | -- |
32 | 1.8.3.1 | 27 | 2.29.2 |
33 | 28 | ||
34 | 29 | diff view generated by jsdifflib |
1 | From: "Daniel P. Berrange" <berrange@redhat.com> | 1 | From: Maxim Levitsky <mlevitsk@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The '--image-opts' flag indicates whether the source filename | 3 | When the underlying block device doesn't support the |
4 | includes options. The target filename has to remain in the | 4 | bdrv_co_delete_file interface, an 'Error' object was leaked. |
5 | plain filename format though, since it needs to be passed to | ||
6 | bdrv_create(). When using --skip-create though, it would be | ||
7 | possible to use image-opts syntax. This adds --target-image-opts | ||
8 | to indicate that the target filename includes options. Currently | ||
9 | this mandates use of the --skip-create flag too. | ||
10 | 5 | ||
11 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | 6 | Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> |
12 | Message-id: 20170515164712.6643-4-berrange@redhat.com | 7 | Reviewed-by: Alberto Garcia <berto@igalia.com> |
13 | Reviewed-by: Max Reitz <mreitz@redhat.com> | 8 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
14 | Reviewed-by: Eric Blake <eblake@redhat.com> | 9 | Message-Id: <20201217170904.946013-2-mlevitsk@redhat.com> |
15 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
16 | --- | 11 | --- |
17 | qemu-img-cmds.hx | 4 +-- | 12 | block/crypto.c | 2 ++ |
18 | qemu-img.c | 84 ++++++++++++++++++++++++++++++++++++++------------------ | 13 | 1 file changed, 2 insertions(+) |
19 | qemu-img.texi | 12 ++++++-- | ||
20 | 3 files changed, 69 insertions(+), 31 deletions(-) | ||
21 | 14 | ||
22 | diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx | 15 | diff --git a/block/crypto.c b/block/crypto.c |
23 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
24 | --- a/qemu-img-cmds.hx | 17 | --- a/block/crypto.c |
25 | +++ b/qemu-img-cmds.hx | 18 | +++ b/block/crypto.c |
26 | @@ -XXX,XX +XXX,XX @@ STEXI | 19 | @@ -XXX,XX +XXX,XX @@ fail: |
27 | ETEXI | 20 | */ |
28 | 21 | if ((r_del < 0) && (r_del != -ENOTSUP)) { | |
29 | DEF("convert", img_convert, | 22 | error_report_err(local_delete_err); |
30 | - "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename") | 23 | + } else { |
31 | + "convert [--object objectdef] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename") | 24 | + error_free(local_delete_err); |
32 | STEXI | ||
33 | -@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename} | ||
34 | +@item convert [--object @var{objectdef}] [--image-opts] [--target-image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename} | ||
35 | ETEXI | ||
36 | |||
37 | DEF("dd", img_dd, | ||
38 | diff --git a/qemu-img.c b/qemu-img.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/qemu-img.c | ||
41 | +++ b/qemu-img.c | ||
42 | @@ -XXX,XX +XXX,XX @@ enum { | ||
43 | OPTION_PATTERN = 260, | ||
44 | OPTION_FLUSH_INTERVAL = 261, | ||
45 | OPTION_NO_DRAIN = 262, | ||
46 | + OPTION_TARGET_IMAGE_OPTS = 263, | ||
47 | }; | ||
48 | |||
49 | typedef enum OutputFormat { | ||
50 | @@ -XXX,XX +XXX,XX @@ static int convert_do_copy(ImgConvertState *s) | ||
51 | static int img_convert(int argc, char **argv) | ||
52 | { | ||
53 | int c, bs_i, flags, src_flags = 0; | ||
54 | - const char *fmt = NULL, *out_fmt = "raw", *cache = "unsafe", | ||
55 | + const char *fmt = NULL, *out_fmt = NULL, *cache = "unsafe", | ||
56 | *src_cache = BDRV_DEFAULT_CACHE, *out_baseimg = NULL, | ||
57 | *out_filename, *out_baseimg_param, *snapshot_name = NULL; | ||
58 | - BlockDriver *drv, *proto_drv; | ||
59 | + BlockDriver *drv = NULL, *proto_drv = NULL; | ||
60 | BlockDriverInfo bdi; | ||
61 | BlockDriverState *out_bs; | ||
62 | QemuOpts *opts = NULL, *sn_opts = NULL; | ||
63 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
64 | char *options = NULL; | ||
65 | Error *local_err = NULL; | ||
66 | bool writethrough, src_writethrough, quiet = false, image_opts = false, | ||
67 | - skip_create = false, progress = false; | ||
68 | + skip_create = false, progress = false, tgt_image_opts = false; | ||
69 | int64_t ret = -EINVAL; | ||
70 | bool force_share = false; | ||
71 | |||
72 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
73 | {"object", required_argument, 0, OPTION_OBJECT}, | ||
74 | {"image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, | ||
75 | {"force-share", no_argument, 0, 'U'}, | ||
76 | + {"target-image-opts", no_argument, 0, OPTION_TARGET_IMAGE_OPTS}, | ||
77 | {0, 0, 0, 0} | ||
78 | }; | ||
79 | c = getopt_long(argc, argv, ":hf:O:B:ce6o:s:l:S:pt:T:qnm:WU", | ||
80 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
81 | case OPTION_IMAGE_OPTS: | ||
82 | image_opts = true; | ||
83 | break; | ||
84 | + case OPTION_TARGET_IMAGE_OPTS: | ||
85 | + tgt_image_opts = true; | ||
86 | + break; | ||
87 | } | 25 | } |
88 | } | 26 | } |
89 | 27 | ||
90 | + if (!out_fmt && !tgt_image_opts) { | ||
91 | + out_fmt = "raw"; | ||
92 | + } | ||
93 | + | ||
94 | if (qemu_opts_foreach(&qemu_object_opts, | ||
95 | user_creatable_add_opts_foreach, | ||
96 | NULL, NULL)) { | ||
97 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
98 | goto fail_getopt; | ||
99 | } | ||
100 | |||
101 | + if (tgt_image_opts && !skip_create) { | ||
102 | + error_report("--target-image-opts requires use of -n flag"); | ||
103 | + goto fail_getopt; | ||
104 | + } | ||
105 | + | ||
106 | s.src_num = argc - optind - 1; | ||
107 | out_filename = s.src_num >= 1 ? argv[argc - 1] : NULL; | ||
108 | |||
109 | if (options && has_help_option(options)) { | ||
110 | - ret = print_block_option_help(out_filename, out_fmt); | ||
111 | - goto fail_getopt; | ||
112 | + if (out_fmt) { | ||
113 | + ret = print_block_option_help(out_filename, out_fmt); | ||
114 | + goto fail_getopt; | ||
115 | + } else { | ||
116 | + error_report("Option help requires a format be specified"); | ||
117 | + goto fail_getopt; | ||
118 | + } | ||
119 | } | ||
120 | |||
121 | if (s.src_num < 1) { | ||
122 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
123 | goto out; | ||
124 | } | ||
125 | |||
126 | - /* Find driver and parse its options */ | ||
127 | - drv = bdrv_find_format(out_fmt); | ||
128 | - if (!drv) { | ||
129 | - error_report("Unknown file format '%s'", out_fmt); | ||
130 | - ret = -1; | ||
131 | - goto out; | ||
132 | - } | ||
133 | + if (!skip_create) { | ||
134 | + /* Find driver and parse its options */ | ||
135 | + drv = bdrv_find_format(out_fmt); | ||
136 | + if (!drv) { | ||
137 | + error_report("Unknown file format '%s'", out_fmt); | ||
138 | + ret = -1; | ||
139 | + goto out; | ||
140 | + } | ||
141 | |||
142 | - proto_drv = bdrv_find_protocol(out_filename, true, &local_err); | ||
143 | - if (!proto_drv) { | ||
144 | - error_report_err(local_err); | ||
145 | - ret = -1; | ||
146 | - goto out; | ||
147 | - } | ||
148 | + proto_drv = bdrv_find_protocol(out_filename, true, &local_err); | ||
149 | + if (!proto_drv) { | ||
150 | + error_report_err(local_err); | ||
151 | + ret = -1; | ||
152 | + goto out; | ||
153 | + } | ||
154 | |||
155 | - if (!skip_create) { | ||
156 | if (!drv->create_opts) { | ||
157 | error_report("Format driver '%s' does not support image creation", | ||
158 | drv->format_name); | ||
159 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
160 | const char *preallocation = | ||
161 | qemu_opt_get(opts, BLOCK_OPT_PREALLOC); | ||
162 | |||
163 | - if (!drv->bdrv_co_pwritev_compressed) { | ||
164 | + if (drv && !drv->bdrv_co_pwritev_compressed) { | ||
165 | error_report("Compression not supported for this file format"); | ||
166 | ret = -1; | ||
167 | goto out; | ||
168 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
169 | goto out; | ||
170 | } | ||
171 | |||
172 | - /* XXX we should allow --image-opts to trigger use of | ||
173 | - * img_open() here, but then we have trouble with | ||
174 | - * the bdrv_create() call which takes different params. | ||
175 | - * Not critical right now, so fix can wait... | ||
176 | - */ | ||
177 | - s.target = img_open_file(out_filename, out_fmt, flags, writethrough, quiet, | ||
178 | - false); | ||
179 | + if (skip_create) { | ||
180 | + s.target = img_open(tgt_image_opts, out_filename, out_fmt, | ||
181 | + flags, writethrough, quiet, false); | ||
182 | + } else { | ||
183 | + /* TODO ultimately we should allow --target-image-opts | ||
184 | + * to be used even when -n is not given. | ||
185 | + * That has to wait for bdrv_create to be improved | ||
186 | + * to allow filenames in option syntax | ||
187 | + */ | ||
188 | + s.target = img_open_file(out_filename, out_fmt, flags, | ||
189 | + writethrough, quiet, false); | ||
190 | + } | ||
191 | if (!s.target) { | ||
192 | ret = -1; | ||
193 | goto out; | ||
194 | } | ||
195 | out_bs = blk_bs(s.target); | ||
196 | |||
197 | + if (s.compressed && !out_bs->drv->bdrv_co_pwritev_compressed) { | ||
198 | + error_report("Compression not supported for this file format"); | ||
199 | + ret = -1; | ||
200 | + goto out; | ||
201 | + } | ||
202 | + | ||
203 | /* increase bufsectors from the default 4096 (2M) if opt_transfer | ||
204 | * or discard_alignment of the out_bs is greater. Limit to 32768 (16MB) | ||
205 | * as maximum. */ | ||
206 | diff --git a/qemu-img.texi b/qemu-img.texi | ||
207 | index XXXXXXX..XXXXXXX 100644 | ||
208 | --- a/qemu-img.texi | ||
209 | +++ b/qemu-img.texi | ||
210 | @@ -XXX,XX +XXX,XX @@ keys. | ||
211 | |||
212 | @item --image-opts | ||
213 | |||
214 | -Indicates that the @var{filename} parameter is to be interpreted as a | ||
215 | +Indicates that the source @var{filename} parameter is to be interpreted as a | ||
216 | full option string, not a plain filename. This parameter is mutually | ||
217 | -exclusive with the @var{-f} and @var{-F} parameters. | ||
218 | +exclusive with the @var{-f} parameter. | ||
219 | + | ||
220 | +@item --target-image-opts | ||
221 | + | ||
222 | +Indicates that the @var{output_filename} parameter(s) are to be interpreted as | ||
223 | +a full option string, not a plain filename. This parameter is mutually | ||
224 | +exclusive with the @var{-O} parameters. It is currently required to also use | ||
225 | +the @var{-n} parameter to skip image creation. This restriction may be relaxed | ||
226 | +in a future release. | ||
227 | |||
228 | @item fmt | ||
229 | is the disk image format. It is guessed automatically in most cases. See below | ||
230 | -- | 28 | -- |
231 | 1.8.3.1 | 29 | 2.29.2 |
232 | 30 | ||
233 | 31 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Maxim Levitsky <mlevitsk@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The file drivers' *_parse_filename() implementations just strip the | 3 | This function wraps bdrv_co_delete_file for the common case of removing a file, |
4 | optional protocol prefix off the filename. However, for e.g. | 4 | which was just created by format driver, on an error condition. |
5 | "file:foo:bar", this would lead to "foo:bar" being stored as the BDS's | ||
6 | filename which looks like it should be managed using the "foo" protocol. | ||
7 | This is especially troublesome if you then try to resolve a backing | ||
8 | filename based on "foo:bar". | ||
9 | 5 | ||
10 | This issue can only occur if the stripped part is a relative filename | 6 | It hides the -ENOTSUPP error, and reports all other errors otherwise. |
11 | ("file:/foo:bar" will be shortened to "/foo:bar" and having a slash | ||
12 | before the first colon means that "/foo" is not recognized as a protocol | ||
13 | part). Therefore, we can easily fix it by prepending "./" to such | ||
14 | filenames. | ||
15 | 7 | ||
16 | Before this patch: | 8 | Use it in luks driver |
17 | $ ./qemu-img create -f qcow2 backing.qcow2 64M | ||
18 | Formatting 'backing.qcow2', fmt=qcow2 size=67108864 encryption=off | ||
19 | cluster_size=65536 lazy_refcounts=off refcount_bits=16 | ||
20 | $ ./qemu-img create -f qcow2 -b backing.qcow2 file:top:image.qcow2 | ||
21 | Formatting 'file:top:image.qcow2', fmt=qcow2 size=67108864 | ||
22 | backing_file=backing.qcow2 encryption=off cluster_size=65536 | ||
23 | lazy_refcounts=off refcount_bits=16 | ||
24 | $ ./qemu-io file:top:image.qcow2 | ||
25 | can't open device file:top:image.qcow2: Could not open backing file: | ||
26 | Unknown protocol 'top' | ||
27 | 9 | ||
28 | After this patch: | 10 | Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> |
29 | $ ./qemu-io file:top:image.qcow2 | 11 | Reviewed-by: Alberto Garcia <berto@igalia.com> |
30 | [no error] | 12 | Message-Id: <20201217170904.946013-3-mlevitsk@redhat.com> |
13 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | include/block/block.h | 1 + | ||
17 | block.c | 22 ++++++++++++++++++++++ | ||
18 | block/crypto.c | 15 ++------------- | ||
19 | 3 files changed, 25 insertions(+), 13 deletions(-) | ||
31 | 20 | ||
32 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 21 | diff --git a/include/block/block.h b/include/block/block.h |
33 | Message-id: 20170522195217.12991-3-mreitz@redhat.com | 22 | index XXXXXXX..XXXXXXX 100644 |
34 | Reviewed-by: Eric Blake <eblake@redhat.com> | 23 | --- a/include/block/block.h |
35 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 24 | +++ b/include/block/block.h |
36 | --- | 25 | @@ -XXX,XX +XXX,XX @@ int bdrv_freeze_backing_chain(BlockDriverState *bs, BlockDriverState *base, |
37 | block.c | 35 +++++++++++++++++++++++++++++++++++ | 26 | Error **errp); |
38 | block/file-posix.c | 17 +++-------------- | 27 | void bdrv_unfreeze_backing_chain(BlockDriverState *bs, BlockDriverState *base); |
39 | block/file-win32.c | 12 ++---------- | 28 | int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp); |
40 | include/block/block_int.h | 3 +++ | 29 | +void coroutine_fn bdrv_co_delete_file_noerr(BlockDriverState *bs); |
41 | 4 files changed, 43 insertions(+), 24 deletions(-) | 30 | |
42 | 31 | ||
32 | typedef struct BdrvCheckResult { | ||
43 | diff --git a/block.c b/block.c | 33 | diff --git a/block.c b/block.c |
44 | index XXXXXXX..XXXXXXX 100644 | 34 | index XXXXXXX..XXXXXXX 100644 |
45 | --- a/block.c | 35 | --- a/block.c |
46 | +++ b/block.c | 36 | +++ b/block.c |
47 | @@ -XXX,XX +XXX,XX @@ void path_combine(char *dest, int dest_size, | 37 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_delete_file(BlockDriverState *bs, Error **errp) |
48 | } | 38 | return ret; |
49 | } | 39 | } |
50 | 40 | ||
51 | +/* | 41 | +void coroutine_fn bdrv_co_delete_file_noerr(BlockDriverState *bs) |
52 | + * Helper function for bdrv_parse_filename() implementations to remove optional | ||
53 | + * protocol prefixes (especially "file:") from a filename and for putting the | ||
54 | + * stripped filename into the options QDict if there is such a prefix. | ||
55 | + */ | ||
56 | +void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, | ||
57 | + QDict *options) | ||
58 | +{ | 42 | +{ |
59 | + if (strstart(filename, prefix, &filename)) { | 43 | + Error *local_err = NULL; |
60 | + /* Stripping the explicit protocol prefix may result in a protocol | 44 | + int ret; |
61 | + * prefix being (wrongly) detected (if the filename contains a colon) */ | ||
62 | + if (path_has_protocol(filename)) { | ||
63 | + QString *fat_filename; | ||
64 | + | 45 | + |
65 | + /* This means there is some colon before the first slash; therefore, | 46 | + if (!bs) { |
66 | + * this cannot be an absolute path */ | 47 | + return; |
67 | + assert(!path_is_absolute(filename)); | 48 | + } |
68 | + | 49 | + |
69 | + /* And we can thus fix the protocol detection issue by prefixing it | 50 | + ret = bdrv_co_delete_file(bs, &local_err); |
70 | + * by "./" */ | 51 | + /* |
71 | + fat_filename = qstring_from_str("./"); | 52 | + * ENOTSUP will happen if the block driver doesn't support |
72 | + qstring_append(fat_filename, filename); | 53 | + * the 'bdrv_co_delete_file' interface. This is a predictable |
73 | + | 54 | + * scenario and shouldn't be reported back to the user. |
74 | + assert(!path_has_protocol(qstring_get_str(fat_filename))); | 55 | + */ |
75 | + | 56 | + if (ret == -ENOTSUP) { |
76 | + qdict_put(options, "filename", fat_filename); | 57 | + error_free(local_err); |
77 | + } else { | 58 | + } else if (ret < 0) { |
78 | + /* If no protocol prefix was detected, we can use the shortened | 59 | + error_report_err(local_err); |
79 | + * filename as-is */ | ||
80 | + qdict_put_str(options, "filename", filename); | ||
81 | + } | ||
82 | + } | 60 | + } |
83 | +} | 61 | +} |
84 | + | 62 | + |
85 | + | 63 | /** |
86 | /* Returns whether the image file is opened as read-only. Note that this can | 64 | * Try to get @bs's logical and physical block size. |
87 | * return false and writing to the image file is still not possible because the | 65 | * On success, store them in @bsz struct and return 0. |
88 | * image is inactivated. */ | 66 | diff --git a/block/crypto.c b/block/crypto.c |
89 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
90 | index XXXXXXX..XXXXXXX 100644 | 67 | index XXXXXXX..XXXXXXX 100644 |
91 | --- a/block/file-posix.c | 68 | --- a/block/crypto.c |
92 | +++ b/block/file-posix.c | 69 | +++ b/block/crypto.c |
93 | @@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int bdrv_flags, int *open_flags) | 70 | @@ -XXX,XX +XXX,XX @@ fail: |
94 | static void raw_parse_filename(const char *filename, QDict *options, | 71 | * If an error occurred, delete 'filename'. Even if the file existed |
95 | Error **errp) | 72 | * beforehand, it has been truncated and corrupted in the process. |
96 | { | 73 | */ |
97 | - /* The filename does not have to be prefixed by the protocol name, since | 74 | - if (ret && bs) { |
98 | - * "file" is the default protocol; therefore, the return value of this | 75 | - Error *local_delete_err = NULL; |
99 | - * function call can be ignored. */ | 76 | - int r_del = bdrv_co_delete_file(bs, &local_delete_err); |
100 | - strstart(filename, "file:", &filename); | 77 | - /* |
101 | - | 78 | - * ENOTSUP will happen if the block driver doesn't support |
102 | - qdict_put_str(options, "filename", filename); | 79 | - * the 'bdrv_co_delete_file' interface. This is a predictable |
103 | + bdrv_parse_filename_strip_prefix(filename, "file:", options); | 80 | - * scenario and shouldn't be reported back to the user. |
104 | } | 81 | - */ |
105 | 82 | - if ((r_del < 0) && (r_del != -ENOTSUP)) { | |
106 | static QemuOptsList raw_runtime_opts = { | 83 | - error_report_err(local_delete_err); |
107 | @@ -XXX,XX +XXX,XX @@ static int check_hdev_writable(BDRVRawState *s) | 84 | - } else { |
108 | static void hdev_parse_filename(const char *filename, QDict *options, | 85 | - error_free(local_delete_err); |
109 | Error **errp) | 86 | - } |
110 | { | 87 | + if (ret) { |
111 | - /* The prefix is optional, just as for "file". */ | 88 | + bdrv_co_delete_file_noerr(bs); |
112 | - strstart(filename, "host_device:", &filename); | 89 | } |
113 | - | 90 | |
114 | - qdict_put_str(options, "filename", filename); | 91 | bdrv_unref(bs); |
115 | + bdrv_parse_filename_strip_prefix(filename, "host_device:", options); | ||
116 | } | ||
117 | |||
118 | static bool hdev_is_sg(BlockDriverState *bs) | ||
119 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = { | ||
120 | static void cdrom_parse_filename(const char *filename, QDict *options, | ||
121 | Error **errp) | ||
122 | { | ||
123 | - /* The prefix is optional, just as for "file". */ | ||
124 | - strstart(filename, "host_cdrom:", &filename); | ||
125 | - | ||
126 | - qdict_put_str(options, "filename", filename); | ||
127 | + bdrv_parse_filename_strip_prefix(filename, "host_cdrom:", options); | ||
128 | } | ||
129 | #endif | ||
130 | |||
131 | diff --git a/block/file-win32.c b/block/file-win32.c | ||
132 | index XXXXXXX..XXXXXXX 100644 | ||
133 | --- a/block/file-win32.c | ||
134 | +++ b/block/file-win32.c | ||
135 | @@ -XXX,XX +XXX,XX @@ static void raw_parse_flags(int flags, bool use_aio, int *access_flags, | ||
136 | static void raw_parse_filename(const char *filename, QDict *options, | ||
137 | Error **errp) | ||
138 | { | ||
139 | - /* The filename does not have to be prefixed by the protocol name, since | ||
140 | - * "file" is the default protocol; therefore, the return value of this | ||
141 | - * function call can be ignored. */ | ||
142 | - strstart(filename, "file:", &filename); | ||
143 | - | ||
144 | - qdict_put_str(options, "filename", filename); | ||
145 | + bdrv_parse_filename_strip_prefix(filename, "file:", options); | ||
146 | } | ||
147 | |||
148 | static QemuOptsList raw_runtime_opts = { | ||
149 | @@ -XXX,XX +XXX,XX @@ static int hdev_probe_device(const char *filename) | ||
150 | static void hdev_parse_filename(const char *filename, QDict *options, | ||
151 | Error **errp) | ||
152 | { | ||
153 | - /* The prefix is optional, just as for "file". */ | ||
154 | - strstart(filename, "host_device:", &filename); | ||
155 | - | ||
156 | - qdict_put_str(options, "filename", filename); | ||
157 | + bdrv_parse_filename_strip_prefix(filename, "host_device:", options); | ||
158 | } | ||
159 | |||
160 | static int hdev_open(BlockDriverState *bs, QDict *options, int flags, | ||
161 | diff --git a/include/block/block_int.h b/include/block/block_int.h | ||
162 | index XXXXXXX..XXXXXXX 100644 | ||
163 | --- a/include/block/block_int.h | ||
164 | +++ b/include/block/block_int.h | ||
165 | @@ -XXX,XX +XXX,XX @@ int get_tmp_filename(char *filename, int size); | ||
166 | BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, | ||
167 | const char *filename); | ||
168 | |||
169 | +void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, | ||
170 | + QDict *options); | ||
171 | + | ||
172 | |||
173 | /** | ||
174 | * bdrv_add_before_write_notifier: | ||
175 | -- | 92 | -- |
176 | 1.8.3.1 | 93 | 2.29.2 |
177 | 94 | ||
178 | 95 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Maxim Levitsky <mlevitsk@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Commit d7086422b1c1e75e320519cfe26176db6ec97a37 added a local_err | 3 | If the qcow initialization fails, we should remove the file if it was |
4 | variable global to the qcow2_amend_options() function, so there's no | 4 | already created, to avoid leaving stale files around. |
5 | need to have this other one. | ||
6 | 5 | ||
7 | Signed-off-by: Alberto Garcia <berto@igalia.com> | 6 | We already do this for luks raw images. |
8 | Message-id: 20170511150337.21470-1-berto@igalia.com | 7 | |
9 | Reviewed-by: Eric Blake <eblake@redhat.com> | 8 | Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> |
10 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 9 | Reviewed-by: Alberto Garcia <berto@igalia.com> |
10 | Message-Id: <20201217170904.946013-4-mlevitsk@redhat.com> | ||
11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | 13 | --- |
12 | block/qcow2.c | 5 ++--- | 14 | block/qcow2.c | 8 +++++--- |
13 | 1 file changed, 2 insertions(+), 3 deletions(-) | 15 | 1 file changed, 5 insertions(+), 3 deletions(-) |
14 | 16 | ||
15 | diff --git a/block/qcow2.c b/block/qcow2.c | 17 | diff --git a/block/qcow2.c b/block/qcow2.c |
16 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/block/qcow2.c | 19 | --- a/block/qcow2.c |
18 | +++ b/block/qcow2.c | 20 | +++ b/block/qcow2.c |
19 | @@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, | 21 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_co_create_opts(BlockDriver *drv, |
20 | 22 | ||
21 | if (s->refcount_bits != refcount_bits) { | 23 | /* Create the qcow2 image (format layer) */ |
22 | int refcount_order = ctz32(refcount_bits); | 24 | ret = qcow2_co_create(create_options, errp); |
23 | - Error *local_error = NULL; | 25 | +finish: |
24 | 26 | if (ret < 0) { | |
25 | if (new_version < 3 && refcount_bits != 16) { | 27 | - goto finish; |
26 | error_report("Different refcount widths than 16 bits require " | 28 | + bdrv_co_delete_file_noerr(bs); |
27 | @@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, | 29 | + bdrv_co_delete_file_noerr(data_bs); |
28 | helper_cb_info.current_operation = QCOW2_CHANGING_REFCOUNT_ORDER; | 30 | + } else { |
29 | ret = qcow2_change_refcount_order(bs, refcount_order, | 31 | + ret = 0; |
30 | &qcow2_amend_helper_cb, | ||
31 | - &helper_cb_info, &local_error); | ||
32 | + &helper_cb_info, &local_err); | ||
33 | if (ret < 0) { | ||
34 | - error_report_err(local_error); | ||
35 | + error_report_err(local_err); | ||
36 | return ret; | ||
37 | } | ||
38 | } | 32 | } |
33 | |||
34 | - ret = 0; | ||
35 | -finish: | ||
36 | qobject_unref(qdict); | ||
37 | bdrv_unref(bs); | ||
38 | bdrv_unref(data_bs); | ||
39 | -- | 39 | -- |
40 | 1.8.3.1 | 40 | 2.29.2 |
41 | 41 | ||
42 | 42 | diff view generated by jsdifflib |
1 | From: Stephen Bates <sbates@raithlin.com> | 1 | Commit 357bda95 already tried to fix the order in monitor_cleanup() by |
---|---|---|---|
2 | moving shutdown of the dispatcher coroutine further to the start. | ||
3 | However, it didn't go far enough: | ||
2 | 4 | ||
3 | Implement NVMe Controller Memory Buffers (CMBs) which were added in | 5 | iothread_stop() makes sure that all pending work (bottom halves) in the |
4 | version 1.2 of the NVMe Specification. This patch adds an optional | 6 | AioContext of the monitor iothread is completed. iothread_destroy() |
5 | argument (cmb_size_mb) which indicates the size of the CMB (in | 7 | depends on this and fails an assertion if there is still a pending BH. |
6 | MB). Currently only the Submission Queue Support (SQS) is enabled | ||
7 | which aligns with the current Linux driver for NVMe. | ||
8 | 8 | ||
9 | Signed-off-by: Stephen Bates <sbates@raithlin.com> | 9 | While the dispatcher coroutine is running, it will try to resume the |
10 | Acked-by: Keith Busch <keith.busch@intel.com> | 10 | monitor after taking a request out of the queue, which involves a BH. |
11 | The dispatcher is run until it terminates in the AIO_WAIT_WHILE() loop. | ||
12 | However, adding new BHs between iothread_stop() and iothread_destroy() | ||
13 | is forbidden. | ||
14 | |||
15 | Fix this by stopping the dispatcher first before shutting down the other | ||
16 | parts of the monitor. This means we can now receive requests that aren't | ||
17 | handled any more when QEMU is shutting down, but this is unlikely to be | ||
18 | a problem for QMP clients. | ||
19 | |||
20 | Fixes: 357bda9590784ff75803d52de43150d4107ed98e | ||
21 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
22 | Message-Id: <20210212172028.288825-2-kwolf@redhat.com> | ||
23 | Tested-by: Markus Armbruster <armbru@redhat.com> | ||
24 | Reviewed-by: Markus Armbruster <armbru@redhat.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 25 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | --- | 26 | --- |
13 | hw/block/nvme.c | 75 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--- | 27 | monitor/monitor.c | 25 +++++++++++++++---------- |
14 | hw/block/nvme.h | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ | 28 | 1 file changed, 15 insertions(+), 10 deletions(-) |
15 | 2 files changed, 144 insertions(+), 4 deletions(-) | ||
16 | 29 | ||
17 | diff --git a/hw/block/nvme.c b/hw/block/nvme.c | 30 | diff --git a/monitor/monitor.c b/monitor/monitor.c |
18 | index XXXXXXX..XXXXXXX 100644 | 31 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/hw/block/nvme.c | 32 | --- a/monitor/monitor.c |
20 | +++ b/hw/block/nvme.c | 33 | +++ b/monitor/monitor.c |
21 | @@ -XXX,XX +XXX,XX @@ | 34 | @@ -XXX,XX +XXX,XX @@ void monitor_data_destroy(Monitor *mon) |
22 | */ | 35 | |
23 | 36 | void monitor_cleanup(void) | |
24 | /** | ||
25 | - * Reference Specs: http://www.nvmexpress.org, 1.1, 1.0e | ||
26 | + * Reference Specs: http://www.nvmexpress.org, 1.2, 1.1, 1.0e | ||
27 | * | ||
28 | * http://www.nvmexpress.org/resources/ | ||
29 | */ | ||
30 | @@ -XXX,XX +XXX,XX @@ | ||
31 | /** | ||
32 | * Usage: add options: | ||
33 | * -drive file=<file>,if=none,id=<drive_id> | ||
34 | - * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]> | ||
35 | + * -device nvme,drive=<drive_id>,serial=<serial>,id=<id[optional]>, \ | ||
36 | + * cmb_size_mb=<cmb_size_mb[optional]> | ||
37 | + * | ||
38 | + * Note cmb_size_mb denotes size of CMB in MB. CMB is assumed to be at | ||
39 | + * offset 0 in BAR2 and supports SQS only for now. | ||
40 | */ | ||
41 | |||
42 | #include "qemu/osdep.h" | ||
43 | @@ -XXX,XX +XXX,XX @@ | ||
44 | |||
45 | static void nvme_process_sq(void *opaque); | ||
46 | |||
47 | +static void nvme_addr_read(NvmeCtrl *n, hwaddr addr, void *buf, int size) | ||
48 | +{ | ||
49 | + if (n->cmbsz && addr >= n->ctrl_mem.addr && | ||
50 | + addr < (n->ctrl_mem.addr + int128_get64(n->ctrl_mem.size))) { | ||
51 | + memcpy(buf, (void *)&n->cmbuf[addr - n->ctrl_mem.addr], size); | ||
52 | + } else { | ||
53 | + pci_dma_read(&n->parent_obj, addr, buf, size); | ||
54 | + } | ||
55 | +} | ||
56 | + | ||
57 | static int nvme_check_sqid(NvmeCtrl *n, uint16_t sqid) | ||
58 | { | 37 | { |
59 | return sqid < n->num_queues && n->sq[sqid] != NULL ? 0 : -1; | 38 | - /* |
60 | @@ -XXX,XX +XXX,XX @@ static void nvme_process_sq(void *opaque) | 39 | - * We need to explicitly stop the I/O thread (but not destroy it), |
61 | 40 | - * clean up the monitor resources, then destroy the I/O thread since | |
62 | while (!(nvme_sq_empty(sq) || QTAILQ_EMPTY(&sq->req_list))) { | 41 | - * we need to unregister from chardev below in |
63 | addr = sq->dma_addr + sq->head * n->sqe_size; | 42 | - * monitor_data_destroy(), and chardev is not thread-safe yet |
64 | - pci_dma_read(&n->parent_obj, addr, (void *)&cmd, sizeof(cmd)); | 43 | - */ |
65 | + nvme_addr_read(n, addr, (void *)&cmd, sizeof(cmd)); | 44 | - if (mon_iothread) { |
66 | nvme_inc_sq_head(sq); | 45 | - iothread_stop(mon_iothread); |
67 | 46 | - } | |
68 | req = QTAILQ_FIRST(&sq->req_list); | 47 | - |
69 | @@ -XXX,XX +XXX,XX @@ static const MemoryRegionOps nvme_mmio_ops = { | 48 | /* |
70 | }, | 49 | * The dispatcher needs to stop before destroying the monitor and |
71 | }; | 50 | * the I/O thread. |
72 | 51 | @@ -XXX,XX +XXX,XX @@ void monitor_cleanup(void) | |
73 | +static void nvme_cmb_write(void *opaque, hwaddr addr, uint64_t data, | 52 | * eventually terminates. qemu_aio_context is automatically |
74 | + unsigned size) | 53 | * polled by calling AIO_WAIT_WHILE on it, but we must poll |
75 | +{ | 54 | * iohandler_ctx manually. |
76 | + NvmeCtrl *n = (NvmeCtrl *)opaque; | 55 | + * |
77 | + memcpy(&n->cmbuf[addr], &data, size); | 56 | + * Letting the iothread continue while shutting down the dispatcher |
78 | +} | 57 | + * means that new requests may still be coming in. This is okay, |
79 | + | 58 | + * we'll just leave them in the queue without sending a response |
80 | +static uint64_t nvme_cmb_read(void *opaque, hwaddr addr, unsigned size) | 59 | + * and monitor_data_destroy() will free them. |
81 | +{ | 60 | */ |
82 | + uint64_t val; | 61 | qmp_dispatcher_co_shutdown = true; |
83 | + NvmeCtrl *n = (NvmeCtrl *)opaque; | 62 | if (!qatomic_xchg(&qmp_dispatcher_co_busy, true)) { |
84 | + | 63 | @@ -XXX,XX +XXX,XX @@ void monitor_cleanup(void) |
85 | + memcpy(&val, &n->cmbuf[addr], size); | 64 | (aio_poll(iohandler_get_aio_context(), false), |
86 | + return val; | 65 | qatomic_mb_read(&qmp_dispatcher_co_busy))); |
87 | +} | 66 | |
88 | + | 67 | + /* |
89 | +static const MemoryRegionOps nvme_cmb_ops = { | 68 | + * We need to explicitly stop the I/O thread (but not destroy it), |
90 | + .read = nvme_cmb_read, | 69 | + * clean up the monitor resources, then destroy the I/O thread since |
91 | + .write = nvme_cmb_write, | 70 | + * we need to unregister from chardev below in |
92 | + .endianness = DEVICE_LITTLE_ENDIAN, | 71 | + * monitor_data_destroy(), and chardev is not thread-safe yet |
93 | + .impl = { | 72 | + */ |
94 | + .min_access_size = 2, | 73 | + if (mon_iothread) { |
95 | + .max_access_size = 8, | 74 | + iothread_stop(mon_iothread); |
96 | + }, | ||
97 | +}; | ||
98 | + | ||
99 | static int nvme_init(PCIDevice *pci_dev) | ||
100 | { | ||
101 | NvmeCtrl *n = NVME(pci_dev); | ||
102 | @@ -XXX,XX +XXX,XX @@ static int nvme_init(PCIDevice *pci_dev) | ||
103 | NVME_CAP_SET_CSS(n->bar.cap, 1); | ||
104 | NVME_CAP_SET_MPSMAX(n->bar.cap, 4); | ||
105 | |||
106 | - n->bar.vs = 0x00010100; | ||
107 | + n->bar.vs = 0x00010200; | ||
108 | n->bar.intmc = n->bar.intms = 0; | ||
109 | |||
110 | + if (n->cmb_size_mb) { | ||
111 | + | ||
112 | + NVME_CMBLOC_SET_BIR(n->bar.cmbloc, 2); | ||
113 | + NVME_CMBLOC_SET_OFST(n->bar.cmbloc, 0); | ||
114 | + | ||
115 | + NVME_CMBSZ_SET_SQS(n->bar.cmbsz, 1); | ||
116 | + NVME_CMBSZ_SET_CQS(n->bar.cmbsz, 0); | ||
117 | + NVME_CMBSZ_SET_LISTS(n->bar.cmbsz, 0); | ||
118 | + NVME_CMBSZ_SET_RDS(n->bar.cmbsz, 0); | ||
119 | + NVME_CMBSZ_SET_WDS(n->bar.cmbsz, 0); | ||
120 | + NVME_CMBSZ_SET_SZU(n->bar.cmbsz, 2); /* MBs */ | ||
121 | + NVME_CMBSZ_SET_SZ(n->bar.cmbsz, n->cmb_size_mb); | ||
122 | + | ||
123 | + n->cmbuf = g_malloc0(NVME_CMBSZ_GETSIZE(n->bar.cmbsz)); | ||
124 | + memory_region_init_io(&n->ctrl_mem, OBJECT(n), &nvme_cmb_ops, n, | ||
125 | + "nvme-cmb", NVME_CMBSZ_GETSIZE(n->bar.cmbsz)); | ||
126 | + pci_register_bar(&n->parent_obj, NVME_CMBLOC_BIR(n->bar.cmbloc), | ||
127 | + PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64 | | ||
128 | + PCI_BASE_ADDRESS_MEM_PREFETCH, &n->ctrl_mem); | ||
129 | + | ||
130 | + } | 75 | + } |
131 | + | 76 | + |
132 | for (i = 0; i < n->num_namespaces; i++) { | 77 | /* Flush output buffers and destroy monitors */ |
133 | NvmeNamespace *ns = &n->namespaces[i]; | 78 | qemu_mutex_lock(&monitor_lock); |
134 | NvmeIdNs *id_ns = &ns->id_ns; | 79 | monitor_destroyed = true; |
135 | @@ -XXX,XX +XXX,XX @@ static void nvme_exit(PCIDevice *pci_dev) | ||
136 | g_free(n->namespaces); | ||
137 | g_free(n->cq); | ||
138 | g_free(n->sq); | ||
139 | + if (n->cmbsz) { | ||
140 | + memory_region_unref(&n->ctrl_mem); | ||
141 | + } | ||
142 | + | ||
143 | msix_uninit_exclusive_bar(pci_dev); | ||
144 | } | ||
145 | |||
146 | static Property nvme_props[] = { | ||
147 | DEFINE_BLOCK_PROPERTIES(NvmeCtrl, conf), | ||
148 | DEFINE_PROP_STRING("serial", NvmeCtrl, serial), | ||
149 | + DEFINE_PROP_UINT32("cmb_size_mb", NvmeCtrl, cmb_size_mb, 0), | ||
150 | DEFINE_PROP_END_OF_LIST(), | ||
151 | }; | ||
152 | |||
153 | diff --git a/hw/block/nvme.h b/hw/block/nvme.h | ||
154 | index XXXXXXX..XXXXXXX 100644 | ||
155 | --- a/hw/block/nvme.h | ||
156 | +++ b/hw/block/nvme.h | ||
157 | @@ -XXX,XX +XXX,XX @@ typedef struct NvmeBar { | ||
158 | uint32_t aqa; | ||
159 | uint64_t asq; | ||
160 | uint64_t acq; | ||
161 | + uint32_t cmbloc; | ||
162 | + uint32_t cmbsz; | ||
163 | } NvmeBar; | ||
164 | |||
165 | enum NvmeCapShift { | ||
166 | @@ -XXX,XX +XXX,XX @@ enum NvmeAqaMask { | ||
167 | #define NVME_AQA_ASQS(aqa) ((aqa >> AQA_ASQS_SHIFT) & AQA_ASQS_MASK) | ||
168 | #define NVME_AQA_ACQS(aqa) ((aqa >> AQA_ACQS_SHIFT) & AQA_ACQS_MASK) | ||
169 | |||
170 | +enum NvmeCmblocShift { | ||
171 | + CMBLOC_BIR_SHIFT = 0, | ||
172 | + CMBLOC_OFST_SHIFT = 12, | ||
173 | +}; | ||
174 | + | ||
175 | +enum NvmeCmblocMask { | ||
176 | + CMBLOC_BIR_MASK = 0x7, | ||
177 | + CMBLOC_OFST_MASK = 0xfffff, | ||
178 | +}; | ||
179 | + | ||
180 | +#define NVME_CMBLOC_BIR(cmbloc) ((cmbloc >> CMBLOC_BIR_SHIFT) & \ | ||
181 | + CMBLOC_BIR_MASK) | ||
182 | +#define NVME_CMBLOC_OFST(cmbloc)((cmbloc >> CMBLOC_OFST_SHIFT) & \ | ||
183 | + CMBLOC_OFST_MASK) | ||
184 | + | ||
185 | +#define NVME_CMBLOC_SET_BIR(cmbloc, val) \ | ||
186 | + (cmbloc |= (uint64_t)(val & CMBLOC_BIR_MASK) << CMBLOC_BIR_SHIFT) | ||
187 | +#define NVME_CMBLOC_SET_OFST(cmbloc, val) \ | ||
188 | + (cmbloc |= (uint64_t)(val & CMBLOC_OFST_MASK) << CMBLOC_OFST_SHIFT) | ||
189 | + | ||
190 | +enum NvmeCmbszShift { | ||
191 | + CMBSZ_SQS_SHIFT = 0, | ||
192 | + CMBSZ_CQS_SHIFT = 1, | ||
193 | + CMBSZ_LISTS_SHIFT = 2, | ||
194 | + CMBSZ_RDS_SHIFT = 3, | ||
195 | + CMBSZ_WDS_SHIFT = 4, | ||
196 | + CMBSZ_SZU_SHIFT = 8, | ||
197 | + CMBSZ_SZ_SHIFT = 12, | ||
198 | +}; | ||
199 | + | ||
200 | +enum NvmeCmbszMask { | ||
201 | + CMBSZ_SQS_MASK = 0x1, | ||
202 | + CMBSZ_CQS_MASK = 0x1, | ||
203 | + CMBSZ_LISTS_MASK = 0x1, | ||
204 | + CMBSZ_RDS_MASK = 0x1, | ||
205 | + CMBSZ_WDS_MASK = 0x1, | ||
206 | + CMBSZ_SZU_MASK = 0xf, | ||
207 | + CMBSZ_SZ_MASK = 0xfffff, | ||
208 | +}; | ||
209 | + | ||
210 | +#define NVME_CMBSZ_SQS(cmbsz) ((cmbsz >> CMBSZ_SQS_SHIFT) & CMBSZ_SQS_MASK) | ||
211 | +#define NVME_CMBSZ_CQS(cmbsz) ((cmbsz >> CMBSZ_CQS_SHIFT) & CMBSZ_CQS_MASK) | ||
212 | +#define NVME_CMBSZ_LISTS(cmbsz)((cmbsz >> CMBSZ_LISTS_SHIFT) & CMBSZ_LISTS_MASK) | ||
213 | +#define NVME_CMBSZ_RDS(cmbsz) ((cmbsz >> CMBSZ_RDS_SHIFT) & CMBSZ_RDS_MASK) | ||
214 | +#define NVME_CMBSZ_WDS(cmbsz) ((cmbsz >> CMBSZ_WDS_SHIFT) & CMBSZ_WDS_MASK) | ||
215 | +#define NVME_CMBSZ_SZU(cmbsz) ((cmbsz >> CMBSZ_SZU_SHIFT) & CMBSZ_SZU_MASK) | ||
216 | +#define NVME_CMBSZ_SZ(cmbsz) ((cmbsz >> CMBSZ_SZ_SHIFT) & CMBSZ_SZ_MASK) | ||
217 | + | ||
218 | +#define NVME_CMBSZ_SET_SQS(cmbsz, val) \ | ||
219 | + (cmbsz |= (uint64_t)(val & CMBSZ_SQS_MASK) << CMBSZ_SQS_SHIFT) | ||
220 | +#define NVME_CMBSZ_SET_CQS(cmbsz, val) \ | ||
221 | + (cmbsz |= (uint64_t)(val & CMBSZ_CQS_MASK) << CMBSZ_CQS_SHIFT) | ||
222 | +#define NVME_CMBSZ_SET_LISTS(cmbsz, val) \ | ||
223 | + (cmbsz |= (uint64_t)(val & CMBSZ_LISTS_MASK) << CMBSZ_LISTS_SHIFT) | ||
224 | +#define NVME_CMBSZ_SET_RDS(cmbsz, val) \ | ||
225 | + (cmbsz |= (uint64_t)(val & CMBSZ_RDS_MASK) << CMBSZ_RDS_SHIFT) | ||
226 | +#define NVME_CMBSZ_SET_WDS(cmbsz, val) \ | ||
227 | + (cmbsz |= (uint64_t)(val & CMBSZ_WDS_MASK) << CMBSZ_WDS_SHIFT) | ||
228 | +#define NVME_CMBSZ_SET_SZU(cmbsz, val) \ | ||
229 | + (cmbsz |= (uint64_t)(val & CMBSZ_SZU_MASK) << CMBSZ_SZU_SHIFT) | ||
230 | +#define NVME_CMBSZ_SET_SZ(cmbsz, val) \ | ||
231 | + (cmbsz |= (uint64_t)(val & CMBSZ_SZ_MASK) << CMBSZ_SZ_SHIFT) | ||
232 | + | ||
233 | +#define NVME_CMBSZ_GETSIZE(cmbsz) \ | ||
234 | + (NVME_CMBSZ_SZ(cmbsz) * (1 << (12 + 4 * NVME_CMBSZ_SZU(cmbsz)))) | ||
235 | + | ||
236 | typedef struct NvmeCmd { | ||
237 | uint8_t opcode; | ||
238 | uint8_t fuse; | ||
239 | @@ -XXX,XX +XXX,XX @@ typedef struct NvmeNamespace { | ||
240 | typedef struct NvmeCtrl { | ||
241 | PCIDevice parent_obj; | ||
242 | MemoryRegion iomem; | ||
243 | + MemoryRegion ctrl_mem; | ||
244 | NvmeBar bar; | ||
245 | BlockConf conf; | ||
246 | |||
247 | @@ -XXX,XX +XXX,XX @@ typedef struct NvmeCtrl { | ||
248 | uint32_t num_queues; | ||
249 | uint32_t max_q_ents; | ||
250 | uint64_t ns_size; | ||
251 | + uint32_t cmb_size_mb; | ||
252 | + uint32_t cmbsz; | ||
253 | + uint32_t cmbloc; | ||
254 | + uint8_t *cmbuf; | ||
255 | |||
256 | char *serial; | ||
257 | NvmeNamespace *namespaces; | ||
258 | -- | 80 | -- |
259 | 1.8.3.1 | 81 | 2.29.2 |
260 | 82 | ||
261 | 83 | diff view generated by jsdifflib |
1 | This adds a small test for the image streaming error path for failing | 1 | Before this patch, monitor_qmp_dispatcher_co() used to check whether |
---|---|---|---|
2 | block_job_create(), which would have found the null pointer dereference | 2 | shutdown is requested only when it would have to wait for new requests. |
3 | in commit a170a91f. | 3 | If there were still some queued requests, it would try to execute all of |
4 | them before shutting down. | ||
5 | |||
6 | This can be surprising when the queued QMP commands take long or hang | ||
7 | because Ctrl-C may not actually exit QEMU as soon as possible. | ||
8 | |||
9 | Change monitor_qmp_dispatcher_co() so that it additionally checks | ||
10 | whether shutdown is request before it gets a new request from the queue. | ||
4 | 11 | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
6 | Reviewed-by: Alberto Garcia <berto@igalia.com> | 13 | Message-Id: <20210212172028.288825-3-kwolf@redhat.com> |
7 | Reviewed-by: Kashyap Chamarthy <kchamart@redhat.com> | 14 | Tested-by: Markus Armbruster <armbru@redhat.com> |
8 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 15 | Reviewed-by: Markus Armbruster <armbru@redhat.com> |
9 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 16 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
10 | --- | 17 | --- |
11 | tests/qemu-iotests/030 | 4 ++++ | 18 | monitor/qmp.c | 5 +++++ |
12 | tests/qemu-iotests/030.out | 4 ++-- | 19 | 1 file changed, 5 insertions(+) |
13 | 2 files changed, 6 insertions(+), 2 deletions(-) | ||
14 | 20 | ||
15 | diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030 | 21 | diff --git a/monitor/qmp.c b/monitor/qmp.c |
16 | index XXXXXXX..XXXXXXX 100755 | 22 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/tests/qemu-iotests/030 | 23 | --- a/monitor/qmp.c |
18 | +++ b/tests/qemu-iotests/030 | 24 | +++ b/monitor/qmp.c |
19 | @@ -XXX,XX +XXX,XX @@ class TestSingleDrive(iotests.QMPTestCase): | 25 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn monitor_qmp_dispatcher_co(void *data) |
20 | result = self.vm.qmp('block-stream', device='nonexistent') | 26 | */ |
21 | self.assert_qmp(result, 'error/class', 'GenericError') | 27 | qatomic_mb_set(&qmp_dispatcher_co_busy, false); |
22 | 28 | ||
23 | + def test_job_id_missing(self): | 29 | + /* On shutdown, don't take any more requests from the queue */ |
24 | + result = self.vm.qmp('block-stream', device='mid') | 30 | + if (qmp_dispatcher_co_shutdown) { |
25 | + self.assert_qmp(result, 'error/class', 'GenericError') | 31 | + return; |
32 | + } | ||
26 | + | 33 | + |
27 | 34 | while (!(req_obj = monitor_qmp_requests_pop_any_with_lock())) { | |
28 | class TestParallelOps(iotests.QMPTestCase): | 35 | /* |
29 | num_ops = 4 # Number of parallel block-stream operations | 36 | * No more requests to process. Wait to be reentered from |
30 | diff --git a/tests/qemu-iotests/030.out b/tests/qemu-iotests/030.out | ||
31 | index XXXXXXX..XXXXXXX 100644 | ||
32 | --- a/tests/qemu-iotests/030.out | ||
33 | +++ b/tests/qemu-iotests/030.out | ||
34 | @@ -XXX,XX +XXX,XX @@ | ||
35 | -...................... | ||
36 | +....................... | ||
37 | ---------------------------------------------------------------------- | ||
38 | -Ran 22 tests | ||
39 | +Ran 23 tests | ||
40 | |||
41 | OK | ||
42 | -- | 37 | -- |
43 | 1.8.3.1 | 38 | 2.29.2 |
44 | 39 | ||
45 | 40 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | This fixes an assertion failure that was triggered by qemu-iotests 129 | ||
2 | on some CI host, while the same test case didn't seem to fail on other | ||
3 | hosts. | ||
4 | 1 | ||
5 | Essentially the problem is that the blk_unref(s->target) in | ||
6 | mirror_exit() doesn't necessarily mean that the BlockBackend goes away | ||
7 | immediately. It is possible that the job completion was triggered nested | ||
8 | in mirror_drain(), which looks like this: | ||
9 | |||
10 | BlockBackend *target = s->target; | ||
11 | blk_ref(target); | ||
12 | blk_drain(target); | ||
13 | blk_unref(target); | ||
14 | |||
15 | In this case, the write permissions for s->target are retained until | ||
16 | after blk_drain(), which makes removing mirror_top_bs fail for the | ||
17 | active commit case (can't have a writable backing file in the chain | ||
18 | without the filter driver). | ||
19 | |||
20 | Explicitly dropping the permissions first means that the additional | ||
21 | reference doesn't hurt and the job can complete successfully even if | ||
22 | called from the nested blk_drain(). | ||
23 | |||
24 | Cc: qemu-stable@nongnu.org | ||
25 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
26 | Acked-by: Paolo Bonzini <pbonzini@redhat.com> | ||
27 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
28 | --- | ||
29 | block/mirror.c | 7 ++++++- | ||
30 | 1 file changed, 6 insertions(+), 1 deletion(-) | ||
31 | |||
32 | diff --git a/block/mirror.c b/block/mirror.c | ||
33 | index XXXXXXX..XXXXXXX 100644 | ||
34 | --- a/block/mirror.c | ||
35 | +++ b/block/mirror.c | ||
36 | @@ -XXX,XX +XXX,XX @@ static void mirror_exit(BlockJob *job, void *opaque) | ||
37 | |||
38 | /* Remove target parent that still uses BLK_PERM_WRITE/RESIZE before | ||
39 | * inserting target_bs at s->to_replace, where we might not be able to get | ||
40 | - * these permissions. */ | ||
41 | + * these permissions. | ||
42 | + * | ||
43 | + * Note that blk_unref() alone doesn't necessarily drop permissions because | ||
44 | + * we might be running nested inside mirror_drain(), which takes an extra | ||
45 | + * reference, so use an explicit blk_set_perm() first. */ | ||
46 | + blk_set_perm(s->target, 0, BLK_PERM_ALL, &error_abort); | ||
47 | blk_unref(s->target); | ||
48 | s->target = NULL; | ||
49 | |||
50 | -- | ||
51 | 1.8.3.1 | ||
52 | |||
53 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Fam Zheng <famz@redhat.com> | ||
2 | 1 | ||
3 | It got lost in commit a8d16f9ca "qemu-img: Update documentation for -U". | ||
4 | |||
5 | Reported-by: Max Reitz <mreitz@redhat.com> | ||
6 | Signed-off-by: Fam Zheng <famz@redhat.com> | ||
7 | Message-id: 20170515103551.31313-1-famz@redhat.com | ||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
10 | --- | ||
11 | qemu-img-cmds.hx | 4 ++-- | ||
12 | 1 file changed, 2 insertions(+), 2 deletions(-) | ||
13 | |||
14 | diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/qemu-img-cmds.hx | ||
17 | +++ b/qemu-img-cmds.hx | ||
18 | @@ -XXX,XX +XXX,XX @@ STEXI | ||
19 | ETEXI | ||
20 | |||
21 | DEF("convert", img_convert, | ||
22 | - "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename") | ||
23 | + "convert [--object objectdef] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f fmt] [-t cache] [-T src_cache] [-O output_fmt] [-B backing_file] [-o options] [-s snapshot_id_or_name] [-l snapshot_param] [-S sparse_size] [-m num_coroutines] [-W] filename [filename2 [...]] output_filename") | ||
24 | STEXI | ||
25 | -@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename} | ||
26 | +@item convert [--object @var{objectdef}] [--image-opts] [-U] [-c] [-p] [-q] [-n] [-f @var{fmt}] [-t @var{cache}] [-T @var{src_cache}] [-O @var{output_fmt}] [-B @var{backing_file}] [-o @var{options}] [-s @var{snapshot_id_or_name}] [-l @var{snapshot_param}] [-S @var{sparse_size}] [-m @var{num_coroutines}] [-W] @var{filename} [@var{filename2} [...]] @var{output_filename} | ||
27 | ETEXI | ||
28 | |||
29 | DEF("dd", img_dd, | ||
30 | -- | ||
31 | 1.8.3.1 | ||
32 | |||
33 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
2 | 1 | ||
3 | The qemu-img dd command added --image-opts support, but missed | ||
4 | the corresponding --object support. This prevented passing | ||
5 | secrets (eg auth passwords) needed by certain disk images. | ||
6 | |||
7 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
8 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
9 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
10 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
11 | Message-id: 20170515164712.6643-2-berrange@redhat.com | ||
12 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
13 | --- | ||
14 | qemu-img.c | 18 ++++++++++++++++++ | ||
15 | 1 file changed, 18 insertions(+) | ||
16 | |||
17 | diff --git a/qemu-img.c b/qemu-img.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/qemu-img.c | ||
20 | +++ b/qemu-img.c | ||
21 | @@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv) | ||
22 | }; | ||
23 | const struct option long_options[] = { | ||
24 | { "help", no_argument, 0, 'h'}, | ||
25 | + { "object", required_argument, 0, OPTION_OBJECT}, | ||
26 | { "image-opts", no_argument, 0, OPTION_IMAGE_OPTS}, | ||
27 | { "force-share", no_argument, 0, 'U'}, | ||
28 | { 0, 0, 0, 0 } | ||
29 | @@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv) | ||
30 | case 'U': | ||
31 | force_share = true; | ||
32 | break; | ||
33 | + case OPTION_OBJECT: { | ||
34 | + QemuOpts *opts; | ||
35 | + opts = qemu_opts_parse_noisily(&qemu_object_opts, | ||
36 | + optarg, true); | ||
37 | + if (!opts) { | ||
38 | + ret = -1; | ||
39 | + goto out; | ||
40 | + } | ||
41 | + } break; | ||
42 | case OPTION_IMAGE_OPTS: | ||
43 | image_opts = true; | ||
44 | break; | ||
45 | @@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv) | ||
46 | ret = -1; | ||
47 | goto out; | ||
48 | } | ||
49 | + | ||
50 | + if (qemu_opts_foreach(&qemu_object_opts, | ||
51 | + user_creatable_add_opts_foreach, | ||
52 | + NULL, NULL)) { | ||
53 | + ret = -1; | ||
54 | + goto out; | ||
55 | + } | ||
56 | + | ||
57 | blk1 = img_open(image_opts, in.filename, fmt, 0, false, false, | ||
58 | force_share); | ||
59 | |||
60 | -- | ||
61 | 1.8.3.1 | ||
62 | |||
63 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: "Daniel P. Berrange" <berrange@redhat.com> | ||
2 | 1 | ||
3 | The qemu-img dd/convert commands will create an image file and | ||
4 | then try to open it. Historically it has been possible to open | ||
5 | new files without passing any options. With encrypted files | ||
6 | though, the *key-secret options are mandatory, so we need to | ||
7 | provide those options when opening the newly created file. | ||
8 | |||
9 | Signed-off-by: Daniel P. Berrange <berrange@redhat.com> | ||
10 | Message-id: 20170515164712.6643-5-berrange@redhat.com | ||
11 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
12 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
13 | --- | ||
14 | qemu-img.c | 42 +++++++++++++++++++++++++++++++++++++----- | ||
15 | 1 file changed, 37 insertions(+), 5 deletions(-) | ||
16 | |||
17 | diff --git a/qemu-img.c b/qemu-img.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/qemu-img.c | ||
20 | +++ b/qemu-img.c | ||
21 | @@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_opts(const char *optstr, | ||
22 | } | ||
23 | |||
24 | static BlockBackend *img_open_file(const char *filename, | ||
25 | + QDict *options, | ||
26 | const char *fmt, int flags, | ||
27 | bool writethrough, bool quiet, | ||
28 | bool force_share) | ||
29 | { | ||
30 | BlockBackend *blk; | ||
31 | Error *local_err = NULL; | ||
32 | - QDict *options = qdict_new(); | ||
33 | |||
34 | + if (!options) { | ||
35 | + options = qdict_new(); | ||
36 | + } | ||
37 | if (fmt) { | ||
38 | qdict_put_str(options, "driver", fmt); | ||
39 | } | ||
40 | @@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open_file(const char *filename, | ||
41 | } | ||
42 | |||
43 | |||
44 | +static int img_add_key_secrets(void *opaque, | ||
45 | + const char *name, const char *value, | ||
46 | + Error **errp) | ||
47 | +{ | ||
48 | + QDict *options = opaque; | ||
49 | + | ||
50 | + if (g_str_has_suffix(name, "key-secret")) { | ||
51 | + qdict_put(options, name, qstring_from_str(value)); | ||
52 | + } | ||
53 | + | ||
54 | + return 0; | ||
55 | +} | ||
56 | + | ||
57 | +static BlockBackend *img_open_new_file(const char *filename, | ||
58 | + QemuOpts *create_opts, | ||
59 | + const char *fmt, int flags, | ||
60 | + bool writethrough, bool quiet, | ||
61 | + bool force_share) | ||
62 | +{ | ||
63 | + QDict *options = NULL; | ||
64 | + | ||
65 | + options = qdict_new(); | ||
66 | + qemu_opt_foreach(create_opts, img_add_key_secrets, options, &error_abort); | ||
67 | + | ||
68 | + return img_open_file(filename, options, fmt, flags, writethrough, quiet, | ||
69 | + force_share); | ||
70 | +} | ||
71 | + | ||
72 | + | ||
73 | static BlockBackend *img_open(bool image_opts, | ||
74 | const char *filename, | ||
75 | const char *fmt, int flags, bool writethrough, | ||
76 | @@ -XXX,XX +XXX,XX @@ static BlockBackend *img_open(bool image_opts, | ||
77 | blk = img_open_opts(filename, opts, flags, writethrough, quiet, | ||
78 | force_share); | ||
79 | } else { | ||
80 | - blk = img_open_file(filename, fmt, flags, writethrough, quiet, | ||
81 | + blk = img_open_file(filename, NULL, fmt, flags, writethrough, quiet, | ||
82 | force_share); | ||
83 | } | ||
84 | return blk; | ||
85 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
86 | * That has to wait for bdrv_create to be improved | ||
87 | * to allow filenames in option syntax | ||
88 | */ | ||
89 | - s.target = img_open_file(out_filename, out_fmt, flags, | ||
90 | - writethrough, quiet, false); | ||
91 | + s.target = img_open_new_file(out_filename, opts, out_fmt, | ||
92 | + flags, writethrough, quiet, false); | ||
93 | } | ||
94 | if (!s.target) { | ||
95 | ret = -1; | ||
96 | @@ -XXX,XX +XXX,XX @@ static int img_dd(int argc, char **argv) | ||
97 | * with the bdrv_create() call above which does not | ||
98 | * support image-opts style. | ||
99 | */ | ||
100 | - blk2 = img_open_file(out.filename, out_fmt, BDRV_O_RDWR, | ||
101 | + blk2 = img_open_file(out.filename, NULL, out_fmt, BDRV_O_RDWR, | ||
102 | false, false, false); | ||
103 | |||
104 | if (!blk2) { | ||
105 | -- | ||
106 | 1.8.3.1 | ||
107 | |||
108 | diff view generated by jsdifflib |