1 | The following changes since commit a395717cbd26e7593d3c3fe81faca121ec6d13e8: | 1 | The following changes since commit 9db3065c62a983286d06c207f4981408cf42184d: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/cody/tags/block-pull-request' into staging (2018-07-03 11:49:51 +0100) | 3 | Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.1-pull-request' into staging (2021-07-08 16:30:18 +0100) |
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 59738025a1674bb7e07713c3c93ff4fb9c5079f5: | 9 | for you to fetch changes up to e60edf69e2f64e818466019313517a2e6d6b63f4: |
10 | 10 | ||
11 | block: Add blklogwrites (2018-07-03 16:09:48 +0200) | 11 | block: Make blockdev-reopen stable API (2021-07-09 13:19:11 +0200) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block layer patches: | 14 | Block layer patches |
15 | 15 | ||
16 | - qcow2: Use worker threads for compression to improve performance of | 16 | - Make blockdev-reopen stable |
17 | 'qemu-img convert -W' and compressed backup jobs | 17 | - Remove deprecated qemu-img backing file without format |
18 | - blklogwrites: New filter driver to log write requests to an image in | 18 | - rbd: Convert to coroutines and add write zeroes support |
19 | the dm-log-writes format | 19 | - rbd: Updated MAINTAINERS |
20 | - export/fuse: Allow other users access to the export | ||
21 | - vhost-user: Fix backends without multiqueue support | ||
22 | - Fix drive-backup transaction endless drained section | ||
20 | 23 | ||
21 | ---------------------------------------------------------------- | 24 | ---------------------------------------------------------------- |
22 | Aapo Vienamo (1): | 25 | Alberto Garcia (4): |
23 | block: Add blklogwrites | 26 | block: Add bdrv_reopen_queue_free() |
27 | block: Support multiple reopening with x-blockdev-reopen | ||
28 | iotests: Test reopening multiple devices at the same time | ||
29 | block: Make blockdev-reopen stable API | ||
24 | 30 | ||
25 | Ari Sundholm (1): | 31 | Eric Blake (3): |
26 | block: Move two block permission constants to the relevant enum | 32 | qcow2: Prohibit backing file changes in 'qemu-img amend' |
33 | qemu-img: Require -F with -b backing image | ||
34 | qemu-img: Improve error for rebase without backing format | ||
27 | 35 | ||
28 | Vladimir Sementsov-Ogievskiy (3): | 36 | Heinrich Schuchardt (1): |
29 | qemu-img: allow compressed not-in-order writes | 37 | util/uri: do not check argument of uri_free() |
30 | qcow2: refactor data compression | ||
31 | qcow2: add compress threads | ||
32 | 38 | ||
33 | qapi/block-core.json | 33 ++++- | 39 | Ilya Dryomov (1): |
34 | block/qcow2.h | 3 + | 40 | MAINTAINERS: update block/rbd.c maintainer |
35 | include/block/block.h | 7 + | ||
36 | block.c | 6 - | ||
37 | block/blklogwrites.c | 392 ++++++++++++++++++++++++++++++++++++++++++++++++++ | ||
38 | block/qcow2.c | 136 ++++++++++++++---- | ||
39 | qemu-img.c | 5 - | ||
40 | MAINTAINERS | 6 + | ||
41 | block/Makefile.objs | 1 + | ||
42 | 9 files changed, 545 insertions(+), 44 deletions(-) | ||
43 | create mode 100644 block/blklogwrites.c | ||
44 | 41 | ||
42 | Kevin Wolf (3): | ||
43 | vhost-user: Fix backends without multiqueue support | ||
44 | qcow2: Fix dangling pointer after reopen for 'file' | ||
45 | block: Acquire AioContexts during bdrv_reopen_multiple() | ||
46 | |||
47 | Max Reitz (6): | ||
48 | export/fuse: Pass default_permissions for mount | ||
49 | export/fuse: Add allow-other option | ||
50 | export/fuse: Give SET_ATTR_SIZE its own branch | ||
51 | export/fuse: Let permissions be adjustable | ||
52 | iotests/308: Test +w on read-only FUSE exports | ||
53 | iotests/fuse-allow-other: Test allow-other | ||
54 | |||
55 | Or Ozeri (1): | ||
56 | block/rbd: Add support for rbd image encryption | ||
57 | |||
58 | Peter Lieven (8): | ||
59 | block/rbd: bump librbd requirement to luminous release | ||
60 | block/rbd: store object_size in BDRVRBDState | ||
61 | block/rbd: update s->image_size in qemu_rbd_getlength | ||
62 | block/rbd: migrate from aio to coroutines | ||
63 | block/rbd: add write zeroes support | ||
64 | block/rbd: drop qemu_rbd_refresh_limits | ||
65 | block/rbd: fix type of task->complete | ||
66 | MAINTAINERS: add block/rbd.c reviewer | ||
67 | |||
68 | Vladimir Sementsov-Ogievskiy (1): | ||
69 | blockdev: fix drive-backup transaction endless drained section | ||
70 | |||
71 | qapi/block-core.json | 134 +++- | ||
72 | qapi/block-export.json | 33 +- | ||
73 | docs/system/deprecated.rst | 32 - | ||
74 | docs/system/removed-features.rst | 31 + | ||
75 | include/block/block.h | 3 + | ||
76 | block.c | 108 +-- | ||
77 | block/export/fuse.c | 121 +++- | ||
78 | block/nfs.c | 4 +- | ||
79 | block/qcow2.c | 42 +- | ||
80 | block/rbd.c | 749 +++++++++++++-------- | ||
81 | block/replication.c | 7 + | ||
82 | block/ssh.c | 4 +- | ||
83 | blockdev.c | 77 ++- | ||
84 | hw/virtio/vhost-user.c | 3 + | ||
85 | qemu-img.c | 9 +- | ||
86 | qemu-io-cmds.c | 7 +- | ||
87 | util/uri.c | 22 +- | ||
88 | MAINTAINERS | 3 +- | ||
89 | meson.build | 7 +- | ||
90 | tests/qemu-iotests/040 | 4 +- | ||
91 | tests/qemu-iotests/041 | 6 +- | ||
92 | tests/qemu-iotests/061 | 3 + | ||
93 | tests/qemu-iotests/061.out | 3 +- | ||
94 | tests/qemu-iotests/082.out | 6 +- | ||
95 | tests/qemu-iotests/114 | 18 +- | ||
96 | tests/qemu-iotests/114.out | 11 +- | ||
97 | tests/qemu-iotests/155 | 9 +- | ||
98 | tests/qemu-iotests/165 | 4 +- | ||
99 | tests/qemu-iotests/245 | 78 ++- | ||
100 | tests/qemu-iotests/245.out | 4 +- | ||
101 | tests/qemu-iotests/248 | 4 +- | ||
102 | tests/qemu-iotests/248.out | 2 +- | ||
103 | tests/qemu-iotests/296 | 11 +- | ||
104 | tests/qemu-iotests/298 | 4 +- | ||
105 | tests/qemu-iotests/301 | 4 +- | ||
106 | tests/qemu-iotests/301.out | 16 +- | ||
107 | tests/qemu-iotests/308 | 20 +- | ||
108 | tests/qemu-iotests/308.out | 6 +- | ||
109 | tests/qemu-iotests/common.rc | 6 +- | ||
110 | tests/qemu-iotests/tests/fuse-allow-other | 168 +++++ | ||
111 | tests/qemu-iotests/tests/fuse-allow-other.out | 88 +++ | ||
112 | .../qemu-iotests/tests/remove-bitmap-from-backing | 22 +- | ||
113 | 42 files changed, 1350 insertions(+), 543 deletions(-) | ||
114 | create mode 100755 tests/qemu-iotests/tests/fuse-allow-other | ||
115 | create mode 100644 tests/qemu-iotests/tests/fuse-allow-other.out | ||
116 | |||
117 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Ilya Dryomov <idryomov@gmail.com> | ||
1 | 2 | ||
3 | Jason has moved on from working on RBD and Ceph. I'm taking over | ||
4 | his role upstream. | ||
5 | |||
6 | Signed-off-by: Ilya Dryomov <idryomov@gmail.com> | ||
7 | Message-Id: <20210519112513.19694-1-idryomov@gmail.com> | ||
8 | Acked-by: Stefano Garzarella <sgarzare@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | MAINTAINERS | 2 +- | ||
12 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
13 | |||
14 | diff --git a/MAINTAINERS b/MAINTAINERS | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/MAINTAINERS | ||
17 | +++ b/MAINTAINERS | ||
18 | @@ -XXX,XX +XXX,XX @@ S: Supported | ||
19 | F: block/vmdk.c | ||
20 | |||
21 | RBD | ||
22 | -M: Jason Dillaman <dillaman@redhat.com> | ||
23 | +M: Ilya Dryomov <idryomov@gmail.com> | ||
24 | L: qemu-block@nongnu.org | ||
25 | S: Supported | ||
26 | F: block/rbd.c | ||
27 | -- | ||
28 | 2.31.1 | ||
29 | |||
30 | diff view generated by jsdifflib |
1 | From: Aapo Vienamo <aapo@tuxera.com> | 1 | From: Or Ozeri <oro@il.ibm.com> |
---|---|---|---|
2 | 2 | ||
3 | Implements a block device write logging system, similar to Linux kernel | 3 | Starting from ceph Pacific, RBD has built-in support for image-level encryption. |
4 | device mapper dm-log-writes. The write operations that are performed | 4 | Currently supported formats are LUKS version 1 and 2. |
5 | on a block device are logged to a file or another block device. The | 5 | |
6 | write log format is identical to the dm-log-writes format. Currently, | 6 | There are 2 new relevant librbd APIs for controlling encryption, both expect an |
7 | log markers are not supported. | 7 | open image context: |
8 | 8 | ||
9 | This functionality can be used for crash consistency and fs consistency | 9 | rbd_encryption_format: formats an image (i.e. writes the LUKS header) |
10 | testing. By implementing it in qemu, tests utilizing write logs can be | 10 | rbd_encryption_load: loads encryptor/decryptor to the image IO stack |
11 | be used to test non-Linux drivers and older kernels. | 11 | |
12 | 12 | This commit extends the qemu rbd driver API to support the above. | |
13 | The driver accepts an optional parameter to set the sector size used | 13 | |
14 | for logging. This makes the driver require all requests to be aligned | 14 | Signed-off-by: Or Ozeri <oro@il.ibm.com> |
15 | to this sector size and also makes offsets and sizes of writes in the | 15 | Message-Id: <20210627114635.39326-1-oro@il.ibm.com> |
16 | log metadata to be expressed in terms of this value (the log format has | 16 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> |
17 | a granularity of one sector for offsets and sizes). This allows | ||
18 | accurate logging of writes to guest block devices that have unusual | ||
19 | sector sizes. | ||
20 | |||
21 | The implementation is based on the blkverify and blkdebug block | ||
22 | drivers. | ||
23 | |||
24 | Signed-off-by: Aapo Vienamo <aapo@tuxera.com> | ||
25 | Signed-off-by: Ari Sundholm <ari@tuxera.com> | ||
26 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
27 | --- | 18 | --- |
28 | qapi/block-core.json | 33 ++++- | 19 | qapi/block-core.json | 110 ++++++++++++- |
29 | block/blklogwrites.c | 392 +++++++++++++++++++++++++++++++++++++++++++++++++++ | 20 | block/rbd.c | 361 ++++++++++++++++++++++++++++++++++++++++++- |
30 | MAINTAINERS | 6 + | 21 | 2 files changed, 465 insertions(+), 6 deletions(-) |
31 | block/Makefile.objs | 1 + | ||
32 | 4 files changed, 426 insertions(+), 6 deletions(-) | ||
33 | create mode 100644 block/blklogwrites.c | ||
34 | 22 | ||
35 | diff --git a/qapi/block-core.json b/qapi/block-core.json | 23 | diff --git a/qapi/block-core.json b/qapi/block-core.json |
36 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
37 | --- a/qapi/block-core.json | 25 | --- a/qapi/block-core.json |
38 | +++ b/qapi/block-core.json | 26 | +++ b/qapi/block-core.json |
39 | @@ -XXX,XX +XXX,XX @@ | 27 | @@ -XXX,XX +XXX,XX @@ |
40 | # @throttle: Since 2.11 | 28 | 'extents': ['ImageInfo'] |
41 | # @nvme: Since 2.12 | 29 | } } |
42 | # @copy-on-read: Since 3.0 | 30 | |
43 | +# @blklogwrites: Since 3.0 | 31 | +## |
32 | +# @ImageInfoSpecificRbd: | ||
33 | +# | ||
34 | +# @encryption-format: Image encryption format | ||
35 | +# | ||
36 | +# Since: 6.1 | ||
37 | +## | ||
38 | +{ 'struct': 'ImageInfoSpecificRbd', | ||
39 | + 'data': { | ||
40 | + '*encryption-format': 'RbdImageEncryptionFormat' | ||
41 | + } } | ||
42 | + | ||
43 | ## | ||
44 | # @ImageInfoSpecific: | ||
44 | # | 45 | # |
45 | # Since: 2.9 | 46 | @@ -XXX,XX +XXX,XX @@ |
47 | # If we need to add block driver specific parameters for | ||
48 | # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS | ||
49 | # to define a ImageInfoSpecificLUKS | ||
50 | - 'luks': 'QCryptoBlockInfoLUKS' | ||
51 | + 'luks': 'QCryptoBlockInfoLUKS', | ||
52 | + 'rbd': 'ImageInfoSpecificRbd' | ||
53 | } } | ||
54 | |||
46 | ## | 55 | ## |
47 | { 'enum': 'BlockdevDriver', | 56 | @@ -XXX,XX +XXX,XX @@ |
48 | - 'data': [ 'blkdebug', 'blkverify', 'bochs', 'cloop', 'copy-on-read', | 57 | { 'enum': 'RbdAuthMode', |
49 | - 'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom', | 58 | 'data': [ 'cephx', 'none' ] } |
50 | - 'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs', | 59 | |
51 | - 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow', 'qcow2', 'qed', | 60 | +## |
52 | - 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', 'ssh', | 61 | +# @RbdImageEncryptionFormat: |
53 | - 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } | 62 | +# |
54 | + 'data': [ 'blkdebug', 'blklogwrites', 'blkverify', 'bochs', 'cloop', | 63 | +# Since: 6.1 |
55 | + 'copy-on-read', 'dmg', 'file', 'ftp', 'ftps', 'gluster', | 64 | +## |
56 | + 'host_cdrom', 'host_device', 'http', 'https', 'iscsi', 'luks', | 65 | +{ 'enum': 'RbdImageEncryptionFormat', |
57 | + 'nbd', 'nfs', 'null-aio', 'null-co', 'nvme', 'parallels', 'qcow', | 66 | + 'data': [ 'luks', 'luks2' ] } |
58 | + 'qcow2', 'qed', 'quorum', 'raw', 'rbd', 'replication', 'sheepdog', | 67 | + |
59 | + 'ssh', 'throttle', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat', 'vxhs' ] } | 68 | +## |
60 | 69 | +# @RbdEncryptionOptionsLUKSBase: | |
70 | +# | ||
71 | +# @key-secret: ID of a QCryptoSecret object providing a passphrase | ||
72 | +# for unlocking the encryption | ||
73 | +# | ||
74 | +# Since: 6.1 | ||
75 | +## | ||
76 | +{ 'struct': 'RbdEncryptionOptionsLUKSBase', | ||
77 | + 'data': { 'key-secret': 'str' } } | ||
78 | + | ||
79 | +## | ||
80 | +# @RbdEncryptionCreateOptionsLUKSBase: | ||
81 | +# | ||
82 | +# @cipher-alg: The encryption algorithm | ||
83 | +# | ||
84 | +# Since: 6.1 | ||
85 | +## | ||
86 | +{ 'struct': 'RbdEncryptionCreateOptionsLUKSBase', | ||
87 | + 'base': 'RbdEncryptionOptionsLUKSBase', | ||
88 | + 'data': { '*cipher-alg': 'QCryptoCipherAlgorithm' } } | ||
89 | + | ||
90 | +## | ||
91 | +# @RbdEncryptionOptionsLUKS: | ||
92 | +# | ||
93 | +# Since: 6.1 | ||
94 | +## | ||
95 | +{ 'struct': 'RbdEncryptionOptionsLUKS', | ||
96 | + 'base': 'RbdEncryptionOptionsLUKSBase', | ||
97 | + 'data': { } } | ||
98 | + | ||
99 | +## | ||
100 | +# @RbdEncryptionOptionsLUKS2: | ||
101 | +# | ||
102 | +# Since: 6.1 | ||
103 | +## | ||
104 | +{ 'struct': 'RbdEncryptionOptionsLUKS2', | ||
105 | + 'base': 'RbdEncryptionOptionsLUKSBase', | ||
106 | + 'data': { } } | ||
107 | + | ||
108 | +## | ||
109 | +# @RbdEncryptionCreateOptionsLUKS: | ||
110 | +# | ||
111 | +# Since: 6.1 | ||
112 | +## | ||
113 | +{ 'struct': 'RbdEncryptionCreateOptionsLUKS', | ||
114 | + 'base': 'RbdEncryptionCreateOptionsLUKSBase', | ||
115 | + 'data': { } } | ||
116 | + | ||
117 | +## | ||
118 | +# @RbdEncryptionCreateOptionsLUKS2: | ||
119 | +# | ||
120 | +# Since: 6.1 | ||
121 | +## | ||
122 | +{ 'struct': 'RbdEncryptionCreateOptionsLUKS2', | ||
123 | + 'base': 'RbdEncryptionCreateOptionsLUKSBase', | ||
124 | + 'data': { } } | ||
125 | + | ||
126 | +## | ||
127 | +# @RbdEncryptionOptions: | ||
128 | +# | ||
129 | +# Since: 6.1 | ||
130 | +## | ||
131 | +{ 'union': 'RbdEncryptionOptions', | ||
132 | + 'base': { 'format': 'RbdImageEncryptionFormat' }, | ||
133 | + 'discriminator': 'format', | ||
134 | + 'data': { 'luks': 'RbdEncryptionOptionsLUKS', | ||
135 | + 'luks2': 'RbdEncryptionOptionsLUKS2' } } | ||
136 | + | ||
137 | +## | ||
138 | +# @RbdEncryptionCreateOptions: | ||
139 | +# | ||
140 | +# Since: 6.1 | ||
141 | +## | ||
142 | +{ 'union': 'RbdEncryptionCreateOptions', | ||
143 | + 'base': { 'format': 'RbdImageEncryptionFormat' }, | ||
144 | + 'discriminator': 'format', | ||
145 | + 'data': { 'luks': 'RbdEncryptionCreateOptionsLUKS', | ||
146 | + 'luks2': 'RbdEncryptionCreateOptionsLUKS2' } } | ||
147 | + | ||
61 | ## | 148 | ## |
62 | # @BlockdevOptionsFile: | 149 | # @BlockdevOptionsRbd: |
150 | # | ||
63 | @@ -XXX,XX +XXX,XX @@ | 151 | @@ -XXX,XX +XXX,XX @@ |
64 | '*set-state': ['BlkdebugSetStateOptions'] } } | 152 | # |
65 | 153 | # @snapshot: Ceph snapshot name. | |
154 | # | ||
155 | +# @encrypt: Image encryption options. (Since 6.1) | ||
156 | +# | ||
157 | # @user: Ceph id name. | ||
158 | # | ||
159 | # @auth-client-required: Acceptable authentication modes. | ||
160 | @@ -XXX,XX +XXX,XX @@ | ||
161 | 'image': 'str', | ||
162 | '*conf': 'str', | ||
163 | '*snapshot': 'str', | ||
164 | + '*encrypt': 'RbdEncryptionOptions', | ||
165 | '*user': 'str', | ||
166 | '*auth-client-required': ['RbdAuthMode'], | ||
167 | '*key-secret': 'str', | ||
168 | @@ -XXX,XX +XXX,XX @@ | ||
169 | # point to a snapshot. | ||
170 | # @size: Size of the virtual disk in bytes | ||
171 | # @cluster-size: RBD object size | ||
172 | +# @encrypt: Image encryption options. (Since 6.1) | ||
173 | # | ||
174 | # Since: 2.12 | ||
66 | ## | 175 | ## |
67 | +# @BlockdevOptionsBlklogwrites: | 176 | { 'struct': 'BlockdevCreateOptionsRbd', |
68 | +# | 177 | 'data': { 'location': 'BlockdevOptionsRbd', |
69 | +# Driver specific block device options for blklogwrites. | 178 | 'size': 'size', |
70 | +# | 179 | - '*cluster-size' : 'size' } } |
71 | +# @file: block device | 180 | + '*cluster-size' : 'size', |
72 | +# | 181 | + '*encrypt' : 'RbdEncryptionCreateOptions' } } |
73 | +# @log: block device used to log writes to @file | 182 | |
74 | +# | 183 | ## |
75 | +# @log-sector-size: sector size used in logging writes to @file, determines | 184 | # @BlockdevVmdkSubformat: |
76 | +# granularity of offsets and sizes of writes (default: 512) | 185 | diff --git a/block/rbd.c b/block/rbd.c |
77 | +# | 186 | index XXXXXXX..XXXXXXX 100644 |
78 | +# Since: 3.0 | 187 | --- a/block/rbd.c |
79 | +## | 188 | +++ b/block/rbd.c |
80 | +{ 'struct': 'BlockdevOptionsBlklogwrites', | ||
81 | + 'data': { 'file': 'BlockdevRef', | ||
82 | + 'log': 'BlockdevRef', | ||
83 | + '*log-sector-size': 'uint32' } } | ||
84 | + | ||
85 | +## | ||
86 | # @BlockdevOptionsBlkverify: | ||
87 | # | ||
88 | # Driver specific block device options for blkverify. | ||
89 | @@ -XXX,XX +XXX,XX @@ | 189 | @@ -XXX,XX +XXX,XX @@ |
90 | 'discriminator': 'driver', | 190 | #define LIBRBD_USE_IOVEC 0 |
91 | 'data': { | 191 | #endif |
92 | 'blkdebug': 'BlockdevOptionsBlkdebug', | 192 | |
93 | + 'blklogwrites':'BlockdevOptionsBlklogwrites', | 193 | +#define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8 |
94 | 'blkverify': 'BlockdevOptionsBlkverify', | 194 | + |
95 | 'bochs': 'BlockdevOptionsGenericFormat', | 195 | +static const char rbd_luks_header_verification[ |
96 | 'cloop': 'BlockdevOptionsGenericFormat', | 196 | + RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = { |
97 | diff --git a/block/blklogwrites.c b/block/blklogwrites.c | 197 | + 'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 1 |
98 | new file mode 100644 | 198 | +}; |
99 | index XXXXXXX..XXXXXXX | 199 | + |
100 | --- /dev/null | 200 | +static const char rbd_luks2_header_verification[ |
101 | +++ b/block/blklogwrites.c | 201 | + RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = { |
102 | @@ -XXX,XX +XXX,XX @@ | 202 | + 'L', 'U', 'K', 'S', 0xBA, 0xBE, 0, 2 |
103 | +/* | 203 | +}; |
104 | + * Write logging blk driver based on blkverify and blkdebug. | 204 | + |
105 | + * | 205 | typedef enum { |
106 | + * Copyright (c) 2017 Tuomas Tynkkynen <tuomas@tuxera.com> | 206 | RBD_AIO_READ, |
107 | + * Copyright (c) 2018 Aapo Vienamo <aapo@tuxera.com> | 207 | RBD_AIO_WRITE, |
108 | + * Copyright (c) 2018 Ari Sundholm <ari@tuxera.com> | 208 | @@ -XXX,XX +XXX,XX @@ static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) |
109 | + * | 209 | } |
110 | + * This work is licensed under the terms of the GNU GPL, version 2 or later. | 210 | } |
111 | + * See the COPYING file in the top-level directory. | 211 | |
112 | + */ | 212 | +#ifdef LIBRBD_SUPPORTS_ENCRYPTION |
113 | + | 213 | +static int qemu_rbd_convert_luks_options( |
114 | +#include "qemu/osdep.h" | 214 | + RbdEncryptionOptionsLUKSBase *luks_opts, |
115 | +#include "qapi/error.h" | 215 | + char **passphrase, |
116 | +#include "qemu/sockets.h" /* for EINPROGRESS on Windows */ | 216 | + size_t *passphrase_len, |
117 | +#include "block/block_int.h" | 217 | + Error **errp) |
118 | +#include "qapi/qmp/qdict.h" | ||
119 | +#include "qapi/qmp/qstring.h" | ||
120 | +#include "qemu/cutils.h" | ||
121 | +#include "qemu/option.h" | ||
122 | + | ||
123 | +/* Disk format stuff - taken from Linux drivers/md/dm-log-writes.c */ | ||
124 | + | ||
125 | +#define LOG_FLUSH_FLAG (1 << 0) | ||
126 | +#define LOG_FUA_FLAG (1 << 1) | ||
127 | +#define LOG_DISCARD_FLAG (1 << 2) | ||
128 | +#define LOG_MARK_FLAG (1 << 3) | ||
129 | + | ||
130 | +#define WRITE_LOG_VERSION 1ULL | ||
131 | +#define WRITE_LOG_MAGIC 0x6a736677736872ULL | ||
132 | + | ||
133 | +/* All fields are little-endian. */ | ||
134 | +struct log_write_super { | ||
135 | + uint64_t magic; | ||
136 | + uint64_t version; | ||
137 | + uint64_t nr_entries; | ||
138 | + uint32_t sectorsize; | ||
139 | +} QEMU_PACKED; | ||
140 | + | ||
141 | +struct log_write_entry { | ||
142 | + uint64_t sector; | ||
143 | + uint64_t nr_sectors; | ||
144 | + uint64_t flags; | ||
145 | + uint64_t data_len; | ||
146 | +} QEMU_PACKED; | ||
147 | + | ||
148 | +/* End of disk format structures. */ | ||
149 | + | ||
150 | +typedef struct { | ||
151 | + BdrvChild *log_file; | ||
152 | + uint32_t sectorsize; | ||
153 | + uint32_t sectorbits; | ||
154 | + uint64_t cur_log_sector; | ||
155 | + uint64_t nr_entries; | ||
156 | +} BDRVBlkLogWritesState; | ||
157 | + | ||
158 | +static inline uint32_t blk_log_writes_log2(uint32_t value) | ||
159 | +{ | 218 | +{ |
160 | + assert(value > 0); | 219 | + return qcrypto_secret_lookup(luks_opts->key_secret, (uint8_t **)passphrase, |
161 | + return 31 - clz32(value); | 220 | + passphrase_len, errp); |
162 | +} | 221 | +} |
163 | + | 222 | + |
164 | +static int blk_log_writes_open(BlockDriverState *bs, QDict *options, int flags, | 223 | +static int qemu_rbd_convert_luks_create_options( |
165 | + Error **errp) | 224 | + RbdEncryptionCreateOptionsLUKSBase *luks_opts, |
225 | + rbd_encryption_algorithm_t *alg, | ||
226 | + char **passphrase, | ||
227 | + size_t *passphrase_len, | ||
228 | + Error **errp) | ||
166 | +{ | 229 | +{ |
167 | + BDRVBlkLogWritesState *s = bs->opaque; | 230 | + int r = 0; |
168 | + Error *local_err = NULL; | 231 | + |
169 | + int ret; | 232 | + r = qemu_rbd_convert_luks_options( |
170 | + int64_t log_sector_size = BDRV_SECTOR_SIZE; | 233 | + qapi_RbdEncryptionCreateOptionsLUKSBase_base(luks_opts), |
171 | + | 234 | + passphrase, passphrase_len, errp); |
172 | + /* Open the file */ | 235 | + if (r < 0) { |
173 | + bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file, false, | 236 | + return r; |
174 | + &local_err); | 237 | + } |
175 | + if (local_err) { | 238 | + |
239 | + if (luks_opts->has_cipher_alg) { | ||
240 | + switch (luks_opts->cipher_alg) { | ||
241 | + case QCRYPTO_CIPHER_ALG_AES_128: { | ||
242 | + *alg = RBD_ENCRYPTION_ALGORITHM_AES128; | ||
243 | + break; | ||
244 | + } | ||
245 | + case QCRYPTO_CIPHER_ALG_AES_256: { | ||
246 | + *alg = RBD_ENCRYPTION_ALGORITHM_AES256; | ||
247 | + break; | ||
248 | + } | ||
249 | + default: { | ||
250 | + r = -ENOTSUP; | ||
251 | + error_setg_errno(errp, -r, "unknown encryption algorithm: %u", | ||
252 | + luks_opts->cipher_alg); | ||
253 | + return r; | ||
254 | + } | ||
255 | + } | ||
256 | + } else { | ||
257 | + /* default alg */ | ||
258 | + *alg = RBD_ENCRYPTION_ALGORITHM_AES256; | ||
259 | + } | ||
260 | + | ||
261 | + return 0; | ||
262 | +} | ||
263 | + | ||
264 | +static int qemu_rbd_encryption_format(rbd_image_t image, | ||
265 | + RbdEncryptionCreateOptions *encrypt, | ||
266 | + Error **errp) | ||
267 | +{ | ||
268 | + int r = 0; | ||
269 | + g_autofree char *passphrase = NULL; | ||
270 | + size_t passphrase_len; | ||
271 | + rbd_encryption_format_t format; | ||
272 | + rbd_encryption_options_t opts; | ||
273 | + rbd_encryption_luks1_format_options_t luks_opts; | ||
274 | + rbd_encryption_luks2_format_options_t luks2_opts; | ||
275 | + size_t opts_size; | ||
276 | + uint64_t raw_size, effective_size; | ||
277 | + | ||
278 | + r = rbd_get_size(image, &raw_size); | ||
279 | + if (r < 0) { | ||
280 | + error_setg_errno(errp, -r, "cannot get raw image size"); | ||
281 | + return r; | ||
282 | + } | ||
283 | + | ||
284 | + switch (encrypt->format) { | ||
285 | + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: { | ||
286 | + memset(&luks_opts, 0, sizeof(luks_opts)); | ||
287 | + format = RBD_ENCRYPTION_FORMAT_LUKS1; | ||
288 | + opts = &luks_opts; | ||
289 | + opts_size = sizeof(luks_opts); | ||
290 | + r = qemu_rbd_convert_luks_create_options( | ||
291 | + qapi_RbdEncryptionCreateOptionsLUKS_base(&encrypt->u.luks), | ||
292 | + &luks_opts.alg, &passphrase, &passphrase_len, errp); | ||
293 | + if (r < 0) { | ||
294 | + return r; | ||
295 | + } | ||
296 | + luks_opts.passphrase = passphrase; | ||
297 | + luks_opts.passphrase_size = passphrase_len; | ||
298 | + break; | ||
299 | + } | ||
300 | + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: { | ||
301 | + memset(&luks2_opts, 0, sizeof(luks2_opts)); | ||
302 | + format = RBD_ENCRYPTION_FORMAT_LUKS2; | ||
303 | + opts = &luks2_opts; | ||
304 | + opts_size = sizeof(luks2_opts); | ||
305 | + r = qemu_rbd_convert_luks_create_options( | ||
306 | + qapi_RbdEncryptionCreateOptionsLUKS2_base( | ||
307 | + &encrypt->u.luks2), | ||
308 | + &luks2_opts.alg, &passphrase, &passphrase_len, errp); | ||
309 | + if (r < 0) { | ||
310 | + return r; | ||
311 | + } | ||
312 | + luks2_opts.passphrase = passphrase; | ||
313 | + luks2_opts.passphrase_size = passphrase_len; | ||
314 | + break; | ||
315 | + } | ||
316 | + default: { | ||
317 | + r = -ENOTSUP; | ||
318 | + error_setg_errno( | ||
319 | + errp, -r, "unknown image encryption format: %u", | ||
320 | + encrypt->format); | ||
321 | + return r; | ||
322 | + } | ||
323 | + } | ||
324 | + | ||
325 | + r = rbd_encryption_format(image, format, opts, opts_size); | ||
326 | + if (r < 0) { | ||
327 | + error_setg_errno(errp, -r, "encryption format fail"); | ||
328 | + return r; | ||
329 | + } | ||
330 | + | ||
331 | + r = rbd_get_size(image, &effective_size); | ||
332 | + if (r < 0) { | ||
333 | + error_setg_errno(errp, -r, "cannot get effective image size"); | ||
334 | + return r; | ||
335 | + } | ||
336 | + | ||
337 | + r = rbd_resize(image, raw_size + (raw_size - effective_size)); | ||
338 | + if (r < 0) { | ||
339 | + error_setg_errno(errp, -r, "cannot resize image after format"); | ||
340 | + return r; | ||
341 | + } | ||
342 | + | ||
343 | + return 0; | ||
344 | +} | ||
345 | + | ||
346 | +static int qemu_rbd_encryption_load(rbd_image_t image, | ||
347 | + RbdEncryptionOptions *encrypt, | ||
348 | + Error **errp) | ||
349 | +{ | ||
350 | + int r = 0; | ||
351 | + g_autofree char *passphrase = NULL; | ||
352 | + size_t passphrase_len; | ||
353 | + rbd_encryption_luks1_format_options_t luks_opts; | ||
354 | + rbd_encryption_luks2_format_options_t luks2_opts; | ||
355 | + rbd_encryption_format_t format; | ||
356 | + rbd_encryption_options_t opts; | ||
357 | + size_t opts_size; | ||
358 | + | ||
359 | + switch (encrypt->format) { | ||
360 | + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS: { | ||
361 | + memset(&luks_opts, 0, sizeof(luks_opts)); | ||
362 | + format = RBD_ENCRYPTION_FORMAT_LUKS1; | ||
363 | + opts = &luks_opts; | ||
364 | + opts_size = sizeof(luks_opts); | ||
365 | + r = qemu_rbd_convert_luks_options( | ||
366 | + qapi_RbdEncryptionOptionsLUKS_base(&encrypt->u.luks), | ||
367 | + &passphrase, &passphrase_len, errp); | ||
368 | + if (r < 0) { | ||
369 | + return r; | ||
370 | + } | ||
371 | + luks_opts.passphrase = passphrase; | ||
372 | + luks_opts.passphrase_size = passphrase_len; | ||
373 | + break; | ||
374 | + } | ||
375 | + case RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2: { | ||
376 | + memset(&luks2_opts, 0, sizeof(luks2_opts)); | ||
377 | + format = RBD_ENCRYPTION_FORMAT_LUKS2; | ||
378 | + opts = &luks2_opts; | ||
379 | + opts_size = sizeof(luks2_opts); | ||
380 | + r = qemu_rbd_convert_luks_options( | ||
381 | + qapi_RbdEncryptionOptionsLUKS2_base(&encrypt->u.luks2), | ||
382 | + &passphrase, &passphrase_len, errp); | ||
383 | + if (r < 0) { | ||
384 | + return r; | ||
385 | + } | ||
386 | + luks2_opts.passphrase = passphrase; | ||
387 | + luks2_opts.passphrase_size = passphrase_len; | ||
388 | + break; | ||
389 | + } | ||
390 | + default: { | ||
391 | + r = -ENOTSUP; | ||
392 | + error_setg_errno( | ||
393 | + errp, -r, "unknown image encryption format: %u", | ||
394 | + encrypt->format); | ||
395 | + return r; | ||
396 | + } | ||
397 | + } | ||
398 | + | ||
399 | + r = rbd_encryption_load(image, format, opts, opts_size); | ||
400 | + if (r < 0) { | ||
401 | + error_setg_errno(errp, -r, "encryption load fail"); | ||
402 | + return r; | ||
403 | + } | ||
404 | + | ||
405 | + return 0; | ||
406 | +} | ||
407 | +#endif | ||
408 | + | ||
409 | /* FIXME Deprecate and remove keypairs or make it available in QMP. */ | ||
410 | static int qemu_rbd_do_create(BlockdevCreateOptions *options, | ||
411 | const char *keypairs, const char *password_secret, | ||
412 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_do_create(BlockdevCreateOptions *options, | ||
413 | return -EINVAL; | ||
414 | } | ||
415 | |||
416 | +#ifndef LIBRBD_SUPPORTS_ENCRYPTION | ||
417 | + if (opts->has_encrypt) { | ||
418 | + error_setg(errp, "RBD library does not support image encryption"); | ||
419 | + return -ENOTSUP; | ||
420 | + } | ||
421 | +#endif | ||
422 | + | ||
423 | if (opts->has_cluster_size) { | ||
424 | int64_t objsize = opts->cluster_size; | ||
425 | if ((objsize - 1) & objsize) { /* not a power of 2? */ | ||
426 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_do_create(BlockdevCreateOptions *options, | ||
427 | goto out; | ||
428 | } | ||
429 | |||
430 | +#ifdef LIBRBD_SUPPORTS_ENCRYPTION | ||
431 | + if (opts->has_encrypt) { | ||
432 | + rbd_image_t image; | ||
433 | + | ||
434 | + ret = rbd_open(io_ctx, opts->location->image, &image, NULL); | ||
435 | + if (ret < 0) { | ||
436 | + error_setg_errno(errp, -ret, | ||
437 | + "error opening image '%s' for encryption format", | ||
438 | + opts->location->image); | ||
439 | + goto out; | ||
440 | + } | ||
441 | + | ||
442 | + ret = qemu_rbd_encryption_format(image, opts->encrypt, errp); | ||
443 | + rbd_close(image); | ||
444 | + if (ret < 0) { | ||
445 | + /* encryption format fail, try removing the image */ | ||
446 | + rbd_remove(io_ctx, opts->location->image); | ||
447 | + goto out; | ||
448 | + } | ||
449 | + } | ||
450 | +#endif | ||
451 | + | ||
452 | ret = 0; | ||
453 | out: | ||
454 | rados_ioctx_destroy(io_ctx); | ||
455 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_co_create(BlockdevCreateOptions *options, Error **errp) | ||
456 | return qemu_rbd_do_create(options, NULL, NULL, errp); | ||
457 | } | ||
458 | |||
459 | +static int qemu_rbd_extract_encryption_create_options( | ||
460 | + QemuOpts *opts, | ||
461 | + RbdEncryptionCreateOptions **spec, | ||
462 | + Error **errp) | ||
463 | +{ | ||
464 | + QDict *opts_qdict; | ||
465 | + QDict *encrypt_qdict; | ||
466 | + Visitor *v; | ||
467 | + int ret = 0; | ||
468 | + | ||
469 | + opts_qdict = qemu_opts_to_qdict(opts, NULL); | ||
470 | + qdict_extract_subqdict(opts_qdict, &encrypt_qdict, "encrypt."); | ||
471 | + qobject_unref(opts_qdict); | ||
472 | + if (!qdict_size(encrypt_qdict)) { | ||
473 | + *spec = NULL; | ||
474 | + goto exit; | ||
475 | + } | ||
476 | + | ||
477 | + /* Convert options into a QAPI object */ | ||
478 | + v = qobject_input_visitor_new_flat_confused(encrypt_qdict, errp); | ||
479 | + if (!v) { | ||
176 | + ret = -EINVAL; | 480 | + ret = -EINVAL; |
177 | + error_propagate(errp, local_err); | 481 | + goto exit; |
178 | + goto fail; | 482 | + } |
179 | + } | 483 | + |
180 | + | 484 | + visit_type_RbdEncryptionCreateOptions(v, NULL, spec, errp); |
181 | + if (qdict_haskey(options, "log-sector-size")) { | 485 | + visit_free(v); |
182 | + log_sector_size = qdict_get_int(options, "log-sector-size"); | 486 | + if (!*spec) { |
183 | + qdict_del(options, "log-sector-size"); | ||
184 | + } | ||
185 | + | ||
186 | + if (log_sector_size < 0 || log_sector_size >= (1ull << 32) || | ||
187 | + !is_power_of_2(log_sector_size)) | ||
188 | + { | ||
189 | + ret = -EINVAL; | 487 | + ret = -EINVAL; |
190 | + error_setg(errp, "Invalid log sector size %"PRId64, log_sector_size); | 488 | + goto exit; |
191 | + goto fail; | 489 | + } |
192 | + } | 490 | + |
193 | + | 491 | +exit: |
194 | + s->sectorsize = log_sector_size; | 492 | + qobject_unref(encrypt_qdict); |
195 | + s->sectorbits = blk_log_writes_log2(log_sector_size); | ||
196 | + s->cur_log_sector = 1; | ||
197 | + s->nr_entries = 0; | ||
198 | + | ||
199 | + /* Open the log file */ | ||
200 | + s->log_file = bdrv_open_child(NULL, options, "log", bs, &child_file, false, | ||
201 | + &local_err); | ||
202 | + if (local_err) { | ||
203 | + ret = -EINVAL; | ||
204 | + error_propagate(errp, local_err); | ||
205 | + goto fail; | ||
206 | + } | ||
207 | + | ||
208 | + ret = 0; | ||
209 | +fail: | ||
210 | + if (ret < 0) { | ||
211 | + bdrv_unref_child(bs, bs->file); | ||
212 | + bs->file = NULL; | ||
213 | + } | ||
214 | + return ret; | 493 | + return ret; |
215 | +} | 494 | +} |
216 | + | 495 | + |
217 | +static void blk_log_writes_close(BlockDriverState *bs) | 496 | static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv, |
497 | const char *filename, | ||
498 | QemuOpts *opts, | ||
499 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv, | ||
500 | BlockdevCreateOptions *create_options; | ||
501 | BlockdevCreateOptionsRbd *rbd_opts; | ||
502 | BlockdevOptionsRbd *loc; | ||
503 | + RbdEncryptionCreateOptions *encrypt = NULL; | ||
504 | Error *local_err = NULL; | ||
505 | const char *keypairs, *password_secret; | ||
506 | QDict *options = NULL; | ||
507 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qemu_rbd_co_create_opts(BlockDriver *drv, | ||
508 | goto exit; | ||
509 | } | ||
510 | |||
511 | + ret = qemu_rbd_extract_encryption_create_options(opts, &encrypt, errp); | ||
512 | + if (ret < 0) { | ||
513 | + goto exit; | ||
514 | + } | ||
515 | + rbd_opts->encrypt = encrypt; | ||
516 | + rbd_opts->has_encrypt = !!encrypt; | ||
517 | + | ||
518 | /* | ||
519 | * Caution: while qdict_get_try_str() is fine, getting non-string | ||
520 | * types would require more care. When @options come from -blockdev | ||
521 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
522 | goto failed_open; | ||
523 | } | ||
524 | |||
525 | + if (opts->has_encrypt) { | ||
526 | +#ifdef LIBRBD_SUPPORTS_ENCRYPTION | ||
527 | + r = qemu_rbd_encryption_load(s->image, opts->encrypt, errp); | ||
528 | + if (r < 0) { | ||
529 | + goto failed_post_open; | ||
530 | + } | ||
531 | +#else | ||
532 | + r = -ENOTSUP; | ||
533 | + error_setg(errp, "RBD library does not support image encryption"); | ||
534 | + goto failed_post_open; | ||
535 | +#endif | ||
536 | + } | ||
537 | + | ||
538 | r = rbd_get_size(s->image, &s->image_size); | ||
539 | if (r < 0) { | ||
540 | error_setg_errno(errp, -r, "error getting image size from %s", | ||
541 | s->image_name); | ||
542 | - rbd_close(s->image); | ||
543 | - goto failed_open; | ||
544 | + goto failed_post_open; | ||
545 | } | ||
546 | |||
547 | /* If we are using an rbd snapshot, we must be r/o, otherwise | ||
548 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
549 | if (s->snap != NULL) { | ||
550 | r = bdrv_apply_auto_read_only(bs, "rbd snapshots are read-only", errp); | ||
551 | if (r < 0) { | ||
552 | - rbd_close(s->image); | ||
553 | - goto failed_open; | ||
554 | + goto failed_post_open; | ||
555 | } | ||
556 | } | ||
557 | |||
558 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
559 | r = 0; | ||
560 | goto out; | ||
561 | |||
562 | +failed_post_open: | ||
563 | + rbd_close(s->image); | ||
564 | failed_open: | ||
565 | rados_ioctx_destroy(s->io_ctx); | ||
566 | g_free(s->snap); | ||
567 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) | ||
568 | return 0; | ||
569 | } | ||
570 | |||
571 | +static ImageInfoSpecific *qemu_rbd_get_specific_info(BlockDriverState *bs, | ||
572 | + Error **errp) | ||
218 | +{ | 573 | +{ |
219 | + BDRVBlkLogWritesState *s = bs->opaque; | 574 | + BDRVRBDState *s = bs->opaque; |
220 | + | 575 | + ImageInfoSpecific *spec_info; |
221 | + bdrv_unref_child(bs, s->log_file); | 576 | + char buf[RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN] = {0}; |
222 | + s->log_file = NULL; | 577 | + int r; |
578 | + | ||
579 | + if (s->image_size >= RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) { | ||
580 | + r = rbd_read(s->image, 0, | ||
581 | + RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN, buf); | ||
582 | + if (r < 0) { | ||
583 | + error_setg_errno(errp, -r, "cannot read image start for probe"); | ||
584 | + return NULL; | ||
585 | + } | ||
586 | + } | ||
587 | + | ||
588 | + spec_info = g_new(ImageInfoSpecific, 1); | ||
589 | + *spec_info = (ImageInfoSpecific){ | ||
590 | + .type = IMAGE_INFO_SPECIFIC_KIND_RBD, | ||
591 | + .u.rbd.data = g_new0(ImageInfoSpecificRbd, 1), | ||
592 | + }; | ||
593 | + | ||
594 | + if (memcmp(buf, rbd_luks_header_verification, | ||
595 | + RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) { | ||
596 | + spec_info->u.rbd.data->encryption_format = | ||
597 | + RBD_IMAGE_ENCRYPTION_FORMAT_LUKS; | ||
598 | + spec_info->u.rbd.data->has_encryption_format = true; | ||
599 | + } else if (memcmp(buf, rbd_luks2_header_verification, | ||
600 | + RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN) == 0) { | ||
601 | + spec_info->u.rbd.data->encryption_format = | ||
602 | + RBD_IMAGE_ENCRYPTION_FORMAT_LUKS2; | ||
603 | + spec_info->u.rbd.data->has_encryption_format = true; | ||
604 | + } else { | ||
605 | + spec_info->u.rbd.data->has_encryption_format = false; | ||
606 | + } | ||
607 | + | ||
608 | + return spec_info; | ||
223 | +} | 609 | +} |
224 | + | 610 | + |
225 | +static int64_t blk_log_writes_getlength(BlockDriverState *bs) | 611 | static int64_t qemu_rbd_getlength(BlockDriverState *bs) |
226 | +{ | 612 | { |
227 | + return bdrv_getlength(bs->file->bs); | 613 | BDRVRBDState *s = bs->opaque; |
228 | +} | 614 | @@ -XXX,XX +XXX,XX @@ static QemuOptsList qemu_rbd_create_opts = { |
229 | + | 615 | .type = QEMU_OPT_STRING, |
230 | +static void blk_log_writes_refresh_filename(BlockDriverState *bs, | 616 | .help = "ID of secret providing the password", |
231 | + QDict *options) | 617 | }, |
232 | +{ | 618 | + { |
233 | + BDRVBlkLogWritesState *s = bs->opaque; | 619 | + .name = "encrypt.format", |
234 | + | 620 | + .type = QEMU_OPT_STRING, |
235 | + /* bs->file->bs has already been refreshed */ | 621 | + .help = "Encrypt the image, format choices: 'luks', 'luks2'", |
236 | + bdrv_refresh_filename(s->log_file->bs); | ||
237 | + | ||
238 | + if (bs->file->bs->full_open_options | ||
239 | + && s->log_file->bs->full_open_options) | ||
240 | + { | ||
241 | + QDict *opts = qdict_new(); | ||
242 | + qdict_put_str(opts, "driver", "blklogwrites"); | ||
243 | + | ||
244 | + qobject_ref(bs->file->bs->full_open_options); | ||
245 | + qdict_put_obj(opts, "file", QOBJECT(bs->file->bs->full_open_options)); | ||
246 | + qobject_ref(s->log_file->bs->full_open_options); | ||
247 | + qdict_put_obj(opts, "log", | ||
248 | + QOBJECT(s->log_file->bs->full_open_options)); | ||
249 | + | ||
250 | + bs->full_open_options = opts; | ||
251 | + } | ||
252 | +} | ||
253 | + | ||
254 | +static void blk_log_writes_child_perm(BlockDriverState *bs, BdrvChild *c, | ||
255 | + const BdrvChildRole *role, | ||
256 | + BlockReopenQueue *ro_q, | ||
257 | + uint64_t perm, uint64_t shrd, | ||
258 | + uint64_t *nperm, uint64_t *nshrd) | ||
259 | +{ | ||
260 | + if (!c) { | ||
261 | + *nperm = perm & DEFAULT_PERM_PASSTHROUGH; | ||
262 | + *nshrd = (shrd & DEFAULT_PERM_PASSTHROUGH) | DEFAULT_PERM_UNCHANGED; | ||
263 | + return; | ||
264 | + } | ||
265 | + | ||
266 | + if (!strcmp(c->name, "log")) { | ||
267 | + bdrv_format_default_perms(bs, c, role, ro_q, perm, shrd, nperm, nshrd); | ||
268 | + } else { | ||
269 | + bdrv_filter_default_perms(bs, c, role, ro_q, perm, shrd, nperm, nshrd); | ||
270 | + } | ||
271 | +} | ||
272 | + | ||
273 | +static void blk_log_writes_refresh_limits(BlockDriverState *bs, Error **errp) | ||
274 | +{ | ||
275 | + BDRVBlkLogWritesState *s = bs->opaque; | ||
276 | + bs->bl.request_alignment = s->sectorsize; | ||
277 | +} | ||
278 | + | ||
279 | +static int coroutine_fn | ||
280 | +blk_log_writes_co_preadv(BlockDriverState *bs, uint64_t offset, uint64_t bytes, | ||
281 | + QEMUIOVector *qiov, int flags) | ||
282 | +{ | ||
283 | + return bdrv_co_preadv(bs->file, offset, bytes, qiov, flags); | ||
284 | +} | ||
285 | + | ||
286 | +typedef struct BlkLogWritesFileReq { | ||
287 | + BlockDriverState *bs; | ||
288 | + uint64_t offset; | ||
289 | + uint64_t bytes; | ||
290 | + int file_flags; | ||
291 | + QEMUIOVector *qiov; | ||
292 | + int (*func)(struct BlkLogWritesFileReq *r); | ||
293 | + int file_ret; | ||
294 | +} BlkLogWritesFileReq; | ||
295 | + | ||
296 | +typedef struct { | ||
297 | + BlockDriverState *bs; | ||
298 | + QEMUIOVector *qiov; | ||
299 | + struct log_write_entry entry; | ||
300 | + uint64_t zero_size; | ||
301 | + int log_ret; | ||
302 | +} BlkLogWritesLogReq; | ||
303 | + | ||
304 | +static void coroutine_fn blk_log_writes_co_do_log(BlkLogWritesLogReq *lr) | ||
305 | +{ | ||
306 | + BDRVBlkLogWritesState *s = lr->bs->opaque; | ||
307 | + uint64_t cur_log_offset = s->cur_log_sector << s->sectorbits; | ||
308 | + | ||
309 | + s->nr_entries++; | ||
310 | + s->cur_log_sector += | ||
311 | + ROUND_UP(lr->qiov->size, s->sectorsize) >> s->sectorbits; | ||
312 | + | ||
313 | + lr->log_ret = bdrv_co_pwritev(s->log_file, cur_log_offset, lr->qiov->size, | ||
314 | + lr->qiov, 0); | ||
315 | + | ||
316 | + /* Logging for the "write zeroes" operation */ | ||
317 | + if (lr->log_ret == 0 && lr->zero_size) { | ||
318 | + cur_log_offset = s->cur_log_sector << s->sectorbits; | ||
319 | + s->cur_log_sector += | ||
320 | + ROUND_UP(lr->zero_size, s->sectorsize) >> s->sectorbits; | ||
321 | + | ||
322 | + lr->log_ret = bdrv_co_pwrite_zeroes(s->log_file, cur_log_offset, | ||
323 | + lr->zero_size, 0); | ||
324 | + } | ||
325 | + | ||
326 | + /* Update super block on flush */ | ||
327 | + if (lr->log_ret == 0 && lr->entry.flags & LOG_FLUSH_FLAG) { | ||
328 | + struct log_write_super super = { | ||
329 | + .magic = cpu_to_le64(WRITE_LOG_MAGIC), | ||
330 | + .version = cpu_to_le64(WRITE_LOG_VERSION), | ||
331 | + .nr_entries = cpu_to_le64(s->nr_entries), | ||
332 | + .sectorsize = cpu_to_le32(s->sectorsize), | ||
333 | + }; | ||
334 | + void *zeroes = g_malloc0(s->sectorsize - sizeof(super)); | ||
335 | + QEMUIOVector qiov; | ||
336 | + | ||
337 | + qemu_iovec_init(&qiov, 2); | ||
338 | + qemu_iovec_add(&qiov, &super, sizeof(super)); | ||
339 | + qemu_iovec_add(&qiov, zeroes, s->sectorsize - sizeof(super)); | ||
340 | + | ||
341 | + lr->log_ret = | ||
342 | + bdrv_co_pwritev(s->log_file, 0, s->sectorsize, &qiov, 0); | ||
343 | + if (lr->log_ret == 0) { | ||
344 | + lr->log_ret = bdrv_co_flush(s->log_file->bs); | ||
345 | + } | ||
346 | + qemu_iovec_destroy(&qiov); | ||
347 | + g_free(zeroes); | ||
348 | + } | ||
349 | +} | ||
350 | + | ||
351 | +static void coroutine_fn blk_log_writes_co_do_file(BlkLogWritesFileReq *fr) | ||
352 | +{ | ||
353 | + fr->file_ret = fr->func(fr); | ||
354 | +} | ||
355 | + | ||
356 | +static int coroutine_fn | ||
357 | +blk_log_writes_co_log(BlockDriverState *bs, uint64_t offset, uint64_t bytes, | ||
358 | + QEMUIOVector *qiov, int flags, | ||
359 | + int (*file_func)(BlkLogWritesFileReq *r), | ||
360 | + uint64_t entry_flags, bool is_zero_write) | ||
361 | +{ | ||
362 | + QEMUIOVector log_qiov; | ||
363 | + size_t niov = qiov ? qiov->niov : 0; | ||
364 | + BDRVBlkLogWritesState *s = bs->opaque; | ||
365 | + BlkLogWritesFileReq fr = { | ||
366 | + .bs = bs, | ||
367 | + .offset = offset, | ||
368 | + .bytes = bytes, | ||
369 | + .file_flags = flags, | ||
370 | + .qiov = qiov, | ||
371 | + .func = file_func, | ||
372 | + }; | ||
373 | + BlkLogWritesLogReq lr = { | ||
374 | + .bs = bs, | ||
375 | + .qiov = &log_qiov, | ||
376 | + .entry = { | ||
377 | + .sector = cpu_to_le64(offset >> s->sectorbits), | ||
378 | + .nr_sectors = cpu_to_le64(bytes >> s->sectorbits), | ||
379 | + .flags = cpu_to_le64(entry_flags), | ||
380 | + .data_len = 0, | ||
381 | + }, | 622 | + }, |
382 | + .zero_size = is_zero_write ? bytes : 0, | 623 | + { |
383 | + }; | 624 | + .name = "encrypt.cipher-alg", |
384 | + void *zeroes = g_malloc0(s->sectorsize - sizeof(lr.entry)); | 625 | + .type = QEMU_OPT_STRING, |
385 | + | 626 | + .help = "Name of encryption cipher algorithm" |
386 | + assert((1 << s->sectorbits) == s->sectorsize); | 627 | + " (allowed values: aes-128, aes-256)", |
387 | + assert(bs->bl.request_alignment == s->sectorsize); | 628 | + }, |
388 | + assert(QEMU_IS_ALIGNED(offset, bs->bl.request_alignment)); | 629 | + { |
389 | + assert(QEMU_IS_ALIGNED(bytes, bs->bl.request_alignment)); | 630 | + .name = "encrypt.key-secret", |
390 | + | 631 | + .type = QEMU_OPT_STRING, |
391 | + qemu_iovec_init(&log_qiov, niov + 2); | 632 | + .help = "ID of secret providing LUKS passphrase", |
392 | + qemu_iovec_add(&log_qiov, &lr.entry, sizeof(lr.entry)); | 633 | + }, |
393 | + qemu_iovec_add(&log_qiov, zeroes, s->sectorsize - sizeof(lr.entry)); | 634 | { /* end of list */ } |
394 | + if (qiov) { | 635 | } |
395 | + qemu_iovec_concat(&log_qiov, qiov, 0, qiov->size); | 636 | }; |
396 | + } | 637 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { |
397 | + | 638 | .bdrv_co_create_opts = qemu_rbd_co_create_opts, |
398 | + blk_log_writes_co_do_file(&fr); | 639 | .bdrv_has_zero_init = bdrv_has_zero_init_1, |
399 | + blk_log_writes_co_do_log(&lr); | 640 | .bdrv_get_info = qemu_rbd_getinfo, |
400 | + | 641 | + .bdrv_get_specific_info = qemu_rbd_get_specific_info, |
401 | + qemu_iovec_destroy(&log_qiov); | 642 | .create_opts = &qemu_rbd_create_opts, |
402 | + g_free(zeroes); | 643 | .bdrv_getlength = qemu_rbd_getlength, |
403 | + | 644 | .bdrv_co_truncate = qemu_rbd_co_truncate, |
404 | + if (lr.log_ret < 0) { | ||
405 | + return lr.log_ret; | ||
406 | + } | ||
407 | + | ||
408 | + return fr.file_ret; | ||
409 | +} | ||
410 | + | ||
411 | +static int coroutine_fn | ||
412 | +blk_log_writes_co_do_file_pwritev(BlkLogWritesFileReq *fr) | ||
413 | +{ | ||
414 | + return bdrv_co_pwritev(fr->bs->file, fr->offset, fr->bytes, | ||
415 | + fr->qiov, fr->file_flags); | ||
416 | +} | ||
417 | + | ||
418 | +static int coroutine_fn | ||
419 | +blk_log_writes_co_do_file_pwrite_zeroes(BlkLogWritesFileReq *fr) | ||
420 | +{ | ||
421 | + return bdrv_co_pwrite_zeroes(fr->bs->file, fr->offset, fr->bytes, | ||
422 | + fr->file_flags); | ||
423 | +} | ||
424 | + | ||
425 | +static int coroutine_fn blk_log_writes_co_do_file_flush(BlkLogWritesFileReq *fr) | ||
426 | +{ | ||
427 | + return bdrv_co_flush(fr->bs->file->bs); | ||
428 | +} | ||
429 | + | ||
430 | +static int coroutine_fn | ||
431 | +blk_log_writes_co_do_file_pdiscard(BlkLogWritesFileReq *fr) | ||
432 | +{ | ||
433 | + return bdrv_co_pdiscard(fr->bs->file->bs, fr->offset, fr->bytes); | ||
434 | +} | ||
435 | + | ||
436 | +static int coroutine_fn | ||
437 | +blk_log_writes_co_pwritev(BlockDriverState *bs, uint64_t offset, uint64_t bytes, | ||
438 | + QEMUIOVector *qiov, int flags) | ||
439 | +{ | ||
440 | + return blk_log_writes_co_log(bs, offset, bytes, qiov, flags, | ||
441 | + blk_log_writes_co_do_file_pwritev, 0, false); | ||
442 | +} | ||
443 | + | ||
444 | +static int coroutine_fn | ||
445 | +blk_log_writes_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, | ||
446 | + BdrvRequestFlags flags) | ||
447 | +{ | ||
448 | + return blk_log_writes_co_log(bs, offset, bytes, NULL, flags, | ||
449 | + blk_log_writes_co_do_file_pwrite_zeroes, 0, | ||
450 | + true); | ||
451 | +} | ||
452 | + | ||
453 | +static int coroutine_fn blk_log_writes_co_flush_to_disk(BlockDriverState *bs) | ||
454 | +{ | ||
455 | + return blk_log_writes_co_log(bs, 0, 0, NULL, 0, | ||
456 | + blk_log_writes_co_do_file_flush, | ||
457 | + LOG_FLUSH_FLAG, false); | ||
458 | +} | ||
459 | + | ||
460 | +static int coroutine_fn | ||
461 | +blk_log_writes_co_pdiscard(BlockDriverState *bs, int64_t offset, int count) | ||
462 | +{ | ||
463 | + return blk_log_writes_co_log(bs, offset, count, NULL, 0, | ||
464 | + blk_log_writes_co_do_file_pdiscard, | ||
465 | + LOG_DISCARD_FLAG, false); | ||
466 | +} | ||
467 | + | ||
468 | +static BlockDriver bdrv_blk_log_writes = { | ||
469 | + .format_name = "blklogwrites", | ||
470 | + .instance_size = sizeof(BDRVBlkLogWritesState), | ||
471 | + | ||
472 | + .bdrv_open = blk_log_writes_open, | ||
473 | + .bdrv_close = blk_log_writes_close, | ||
474 | + .bdrv_getlength = blk_log_writes_getlength, | ||
475 | + .bdrv_refresh_filename = blk_log_writes_refresh_filename, | ||
476 | + .bdrv_child_perm = blk_log_writes_child_perm, | ||
477 | + .bdrv_refresh_limits = blk_log_writes_refresh_limits, | ||
478 | + | ||
479 | + .bdrv_co_preadv = blk_log_writes_co_preadv, | ||
480 | + .bdrv_co_pwritev = blk_log_writes_co_pwritev, | ||
481 | + .bdrv_co_pwrite_zeroes = blk_log_writes_co_pwrite_zeroes, | ||
482 | + .bdrv_co_flush_to_disk = blk_log_writes_co_flush_to_disk, | ||
483 | + .bdrv_co_pdiscard = blk_log_writes_co_pdiscard, | ||
484 | + .bdrv_co_block_status = bdrv_co_block_status_from_file, | ||
485 | + | ||
486 | + .is_filter = true, | ||
487 | +}; | ||
488 | + | ||
489 | +static void bdrv_blk_log_writes_init(void) | ||
490 | +{ | ||
491 | + bdrv_register(&bdrv_blk_log_writes); | ||
492 | +} | ||
493 | + | ||
494 | +block_init(bdrv_blk_log_writes_init); | ||
495 | diff --git a/MAINTAINERS b/MAINTAINERS | ||
496 | index XXXXXXX..XXXXXXX 100644 | ||
497 | --- a/MAINTAINERS | ||
498 | +++ b/MAINTAINERS | ||
499 | @@ -XXX,XX +XXX,XX @@ S: Supported | ||
500 | F: block/quorum.c | ||
501 | L: qemu-block@nongnu.org | ||
502 | |||
503 | +blklogwrites | ||
504 | +M: Ari Sundholm <ari@tuxera.com> | ||
505 | +L: qemu-block@nongnu.org | ||
506 | +S: Supported | ||
507 | +F: block/blklogwrites.c | ||
508 | + | ||
509 | blkverify | ||
510 | M: Stefan Hajnoczi <stefanha@redhat.com> | ||
511 | L: qemu-block@nongnu.org | ||
512 | diff --git a/block/Makefile.objs b/block/Makefile.objs | ||
513 | index XXXXXXX..XXXXXXX 100644 | ||
514 | --- a/block/Makefile.objs | ||
515 | +++ b/block/Makefile.objs | ||
516 | @@ -XXX,XX +XXX,XX @@ block-obj-y += qed-check.o | ||
517 | block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o | ||
518 | block-obj-y += quorum.o | ||
519 | block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o | ||
520 | +block-obj-y += blklogwrites.o | ||
521 | block-obj-y += block-backend.o snapshot.o qapi.o | ||
522 | block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o | ||
523 | block-obj-$(CONFIG_POSIX) += file-posix.o | ||
524 | -- | 645 | -- |
525 | 2.13.6 | 646 | 2.31.1 |
526 | 647 | ||
527 | 648 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Peter Lieven <pl@kamp.de> | |
2 | |||
3 | Ceph Luminous (version 12.2.z) is almost 4 years old at this point. | ||
4 | Bump the requirement to get rid of the ifdef'ry in the code. | ||
5 | Qemu 6.1 dropped the support for RHEL-7 which was the last supported | ||
6 | OS that required an older librbd. | ||
7 | |||
8 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
9 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
10 | Message-Id: <20210702172356.11574-2-idryomov@gmail.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | --- | ||
13 | block/rbd.c | 120 ++++------------------------------------------------ | ||
14 | meson.build | 7 ++- | ||
15 | 2 files changed, 13 insertions(+), 114 deletions(-) | ||
16 | |||
17 | diff --git a/block/rbd.c b/block/rbd.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/block/rbd.c | ||
20 | +++ b/block/rbd.c | ||
21 | @@ -XXX,XX +XXX,XX @@ | ||
22 | * leading "\". | ||
23 | */ | ||
24 | |||
25 | -/* rbd_aio_discard added in 0.1.2 */ | ||
26 | -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 2) | ||
27 | -#define LIBRBD_SUPPORTS_DISCARD | ||
28 | -#else | ||
29 | -#undef LIBRBD_SUPPORTS_DISCARD | ||
30 | -#endif | ||
31 | - | ||
32 | #define OBJ_MAX_SIZE (1UL << OBJ_DEFAULT_OBJ_ORDER) | ||
33 | |||
34 | #define RBD_MAX_SNAPS 100 | ||
35 | |||
36 | -/* The LIBRBD_SUPPORTS_IOVEC is defined in librbd.h */ | ||
37 | -#ifdef LIBRBD_SUPPORTS_IOVEC | ||
38 | -#define LIBRBD_USE_IOVEC 1 | ||
39 | -#else | ||
40 | -#define LIBRBD_USE_IOVEC 0 | ||
41 | -#endif | ||
42 | - | ||
43 | #define RBD_ENCRYPTION_LUKS_HEADER_VERIFICATION_LEN 8 | ||
44 | |||
45 | static const char rbd_luks_header_verification[ | ||
46 | @@ -XXX,XX +XXX,XX @@ typedef struct RBDAIOCB { | ||
47 | BlockAIOCB common; | ||
48 | int64_t ret; | ||
49 | QEMUIOVector *qiov; | ||
50 | - char *bounce; | ||
51 | RBDAIOCmd cmd; | ||
52 | int error; | ||
53 | struct BDRVRBDState *s; | ||
54 | @@ -XXX,XX +XXX,XX @@ typedef struct RADOSCB { | ||
55 | RBDAIOCB *acb; | ||
56 | struct BDRVRBDState *s; | ||
57 | int64_t size; | ||
58 | - char *buf; | ||
59 | int64_t ret; | ||
60 | } RADOSCB; | ||
61 | |||
62 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, | ||
63 | |||
64 | static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) | ||
65 | { | ||
66 | - if (LIBRBD_USE_IOVEC) { | ||
67 | - RBDAIOCB *acb = rcb->acb; | ||
68 | - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, | ||
69 | - acb->qiov->size - offs); | ||
70 | - } else { | ||
71 | - memset(rcb->buf + offs, 0, rcb->size - offs); | ||
72 | - } | ||
73 | + RBDAIOCB *acb = rcb->acb; | ||
74 | + iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, | ||
75 | + acb->qiov->size - offs); | ||
76 | } | ||
77 | |||
78 | #ifdef LIBRBD_SUPPORTS_ENCRYPTION | ||
79 | @@ -XXX,XX +XXX,XX @@ static void qemu_rbd_complete_aio(RADOSCB *rcb) | ||
80 | |||
81 | g_free(rcb); | ||
82 | |||
83 | - if (!LIBRBD_USE_IOVEC) { | ||
84 | - if (acb->cmd == RBD_AIO_READ) { | ||
85 | - qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size); | ||
86 | - } | ||
87 | - qemu_vfree(acb->bounce); | ||
88 | - } | ||
89 | - | ||
90 | acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); | ||
91 | |||
92 | qemu_aio_unref(acb); | ||
93 | @@ -XXX,XX +XXX,XX @@ static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) | ||
94 | rbd_finish_bh, rcb); | ||
95 | } | ||
96 | |||
97 | -static int rbd_aio_discard_wrapper(rbd_image_t image, | ||
98 | - uint64_t off, | ||
99 | - uint64_t len, | ||
100 | - rbd_completion_t comp) | ||
101 | -{ | ||
102 | -#ifdef LIBRBD_SUPPORTS_DISCARD | ||
103 | - return rbd_aio_discard(image, off, len, comp); | ||
104 | -#else | ||
105 | - return -ENOTSUP; | ||
106 | -#endif | ||
107 | -} | ||
108 | - | ||
109 | -static int rbd_aio_flush_wrapper(rbd_image_t image, | ||
110 | - rbd_completion_t comp) | ||
111 | -{ | ||
112 | -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH | ||
113 | - return rbd_aio_flush(image, comp); | ||
114 | -#else | ||
115 | - return -ENOTSUP; | ||
116 | -#endif | ||
117 | -} | ||
118 | - | ||
119 | static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
120 | int64_t off, | ||
121 | QEMUIOVector *qiov, | ||
122 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
123 | |||
124 | rcb = g_new(RADOSCB, 1); | ||
125 | |||
126 | - if (!LIBRBD_USE_IOVEC) { | ||
127 | - if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) { | ||
128 | - acb->bounce = NULL; | ||
129 | - } else { | ||
130 | - acb->bounce = qemu_try_blockalign(bs, qiov->size); | ||
131 | - if (acb->bounce == NULL) { | ||
132 | - goto failed; | ||
133 | - } | ||
134 | - } | ||
135 | - if (cmd == RBD_AIO_WRITE) { | ||
136 | - qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size); | ||
137 | - } | ||
138 | - rcb->buf = acb->bounce; | ||
139 | - } | ||
140 | - | ||
141 | acb->ret = 0; | ||
142 | acb->error = 0; | ||
143 | acb->s = s; | ||
144 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
145 | } | ||
146 | |||
147 | switch (cmd) { | ||
148 | - case RBD_AIO_WRITE: { | ||
149 | + case RBD_AIO_WRITE: | ||
150 | /* | ||
151 | * RBD APIs don't allow us to write more than actual size, so in order | ||
152 | * to support growing images, we resize the image before write | ||
153 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
154 | goto failed_completion; | ||
155 | } | ||
156 | } | ||
157 | -#ifdef LIBRBD_SUPPORTS_IOVEC | ||
158 | - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); | ||
159 | -#else | ||
160 | - r = rbd_aio_write(s->image, off, size, rcb->buf, c); | ||
161 | -#endif | ||
162 | + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); | ||
163 | break; | ||
164 | - } | ||
165 | case RBD_AIO_READ: | ||
166 | -#ifdef LIBRBD_SUPPORTS_IOVEC | ||
167 | - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); | ||
168 | -#else | ||
169 | - r = rbd_aio_read(s->image, off, size, rcb->buf, c); | ||
170 | -#endif | ||
171 | + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); | ||
172 | break; | ||
173 | case RBD_AIO_DISCARD: | ||
174 | - r = rbd_aio_discard_wrapper(s->image, off, size, c); | ||
175 | + r = rbd_aio_discard(s->image, off, size, c); | ||
176 | break; | ||
177 | case RBD_AIO_FLUSH: | ||
178 | - r = rbd_aio_flush_wrapper(s->image, c); | ||
179 | + r = rbd_aio_flush(s->image, c); | ||
180 | break; | ||
181 | default: | ||
182 | r = -EINVAL; | ||
183 | @@ -XXX,XX +XXX,XX @@ failed_completion: | ||
184 | rbd_aio_release(c); | ||
185 | failed: | ||
186 | g_free(rcb); | ||
187 | - if (!LIBRBD_USE_IOVEC) { | ||
188 | - qemu_vfree(acb->bounce); | ||
189 | - } | ||
190 | |||
191 | qemu_aio_unref(acb); | ||
192 | return NULL; | ||
193 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, | ||
194 | RBD_AIO_WRITE); | ||
195 | } | ||
196 | |||
197 | -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH | ||
198 | static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, | ||
199 | BlockCompletionFunc *cb, | ||
200 | void *opaque) | ||
201 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, | ||
202 | return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); | ||
203 | } | ||
204 | |||
205 | -#else | ||
206 | - | ||
207 | -static int qemu_rbd_co_flush(BlockDriverState *bs) | ||
208 | -{ | ||
209 | -#if LIBRBD_VERSION_CODE >= LIBRBD_VERSION(0, 1, 1) | ||
210 | - /* rbd_flush added in 0.1.1 */ | ||
211 | - BDRVRBDState *s = bs->opaque; | ||
212 | - return rbd_flush(s->image); | ||
213 | -#else | ||
214 | - return 0; | ||
215 | -#endif | ||
216 | -} | ||
217 | -#endif | ||
218 | - | ||
219 | static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) | ||
220 | { | ||
221 | BDRVRBDState *s = bs->opaque; | ||
222 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_snap_list(BlockDriverState *bs, | ||
223 | return snap_count; | ||
224 | } | ||
225 | |||
226 | -#ifdef LIBRBD_SUPPORTS_DISCARD | ||
227 | static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, | ||
228 | int64_t offset, | ||
229 | int bytes, | ||
230 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, | ||
231 | return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, | ||
232 | RBD_AIO_DISCARD); | ||
233 | } | ||
234 | -#endif | ||
235 | |||
236 | -#ifdef LIBRBD_SUPPORTS_INVALIDATE | ||
237 | static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, | ||
238 | Error **errp) | ||
239 | { | ||
240 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, | ||
241 | error_setg_errno(errp, -r, "Failed to invalidate the cache"); | ||
242 | } | ||
243 | } | ||
244 | -#endif | ||
245 | |||
246 | static QemuOptsList qemu_rbd_create_opts = { | ||
247 | .name = "rbd-create-opts", | ||
248 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { | ||
249 | .bdrv_aio_preadv = qemu_rbd_aio_preadv, | ||
250 | .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, | ||
251 | |||
252 | -#ifdef LIBRBD_SUPPORTS_AIO_FLUSH | ||
253 | .bdrv_aio_flush = qemu_rbd_aio_flush, | ||
254 | -#else | ||
255 | - .bdrv_co_flush_to_disk = qemu_rbd_co_flush, | ||
256 | -#endif | ||
257 | - | ||
258 | -#ifdef LIBRBD_SUPPORTS_DISCARD | ||
259 | .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, | ||
260 | -#endif | ||
261 | |||
262 | .bdrv_snapshot_create = qemu_rbd_snap_create, | ||
263 | .bdrv_snapshot_delete = qemu_rbd_snap_remove, | ||
264 | .bdrv_snapshot_list = qemu_rbd_snap_list, | ||
265 | .bdrv_snapshot_goto = qemu_rbd_snap_rollback, | ||
266 | -#ifdef LIBRBD_SUPPORTS_INVALIDATE | ||
267 | .bdrv_co_invalidate_cache = qemu_rbd_co_invalidate_cache, | ||
268 | -#endif | ||
269 | |||
270 | .strong_runtime_opts = qemu_rbd_strong_runtime_opts, | ||
271 | }; | ||
272 | diff --git a/meson.build b/meson.build | ||
273 | index XXXXXXX..XXXXXXX 100644 | ||
274 | --- a/meson.build | ||
275 | +++ b/meson.build | ||
276 | @@ -XXX,XX +XXX,XX @@ if not get_option('rbd').auto() or have_block | ||
277 | int main(void) { | ||
278 | rados_t cluster; | ||
279 | rados_create(&cluster, NULL); | ||
280 | + #if LIBRBD_VERSION_CODE < LIBRBD_VERSION(1, 12, 0) | ||
281 | + #error | ||
282 | + #endif | ||
283 | return 0; | ||
284 | }''', dependencies: [librbd, librados]) | ||
285 | rbd = declare_dependency(dependencies: [librbd, librados]) | ||
286 | elif get_option('rbd').enabled() | ||
287 | - error('could not link librados') | ||
288 | + error('librbd >= 1.12.0 required') | ||
289 | else | ||
290 | - warning('could not link librados, disabling') | ||
291 | + warning('librbd >= 1.12.0 not found, disabling') | ||
292 | endif | ||
293 | endif | ||
294 | endif | ||
295 | -- | ||
296 | 2.31.1 | ||
297 | |||
298 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
4 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
5 | Message-Id: <20210702172356.11574-3-idryomov@gmail.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | --- | ||
8 | block/rbd.c | 18 +++++++----------- | ||
9 | 1 file changed, 7 insertions(+), 11 deletions(-) | ||
10 | |||
11 | diff --git a/block/rbd.c b/block/rbd.c | ||
12 | index XXXXXXX..XXXXXXX 100644 | ||
13 | --- a/block/rbd.c | ||
14 | +++ b/block/rbd.c | ||
15 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVRBDState { | ||
16 | char *snap; | ||
17 | char *namespace; | ||
18 | uint64_t image_size; | ||
19 | + uint64_t object_size; | ||
20 | } BDRVRBDState; | ||
21 | |||
22 | static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx, | ||
23 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
24 | const QDictEntry *e; | ||
25 | Error *local_err = NULL; | ||
26 | char *keypairs, *secretid; | ||
27 | + rbd_image_info_t info; | ||
28 | int r; | ||
29 | |||
30 | keypairs = g_strdup(qdict_get_try_str(options, "=keyvalue-pairs")); | ||
31 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
32 | #endif | ||
33 | } | ||
34 | |||
35 | - r = rbd_get_size(s->image, &s->image_size); | ||
36 | + r = rbd_stat(s->image, &info, sizeof(info)); | ||
37 | if (r < 0) { | ||
38 | - error_setg_errno(errp, -r, "error getting image size from %s", | ||
39 | + error_setg_errno(errp, -r, "error getting image info from %s", | ||
40 | s->image_name); | ||
41 | goto failed_post_open; | ||
42 | } | ||
43 | + s->image_size = info.size; | ||
44 | + s->object_size = info.obj_size; | ||
45 | |||
46 | /* If we are using an rbd snapshot, we must be r/o, otherwise | ||
47 | * leave as-is */ | ||
48 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, | ||
49 | static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) | ||
50 | { | ||
51 | BDRVRBDState *s = bs->opaque; | ||
52 | - rbd_image_info_t info; | ||
53 | - int r; | ||
54 | - | ||
55 | - r = rbd_stat(s->image, &info, sizeof(info)); | ||
56 | - if (r < 0) { | ||
57 | - return r; | ||
58 | - } | ||
59 | - | ||
60 | - bdi->cluster_size = info.obj_size; | ||
61 | + bdi->cluster_size = s->object_size; | ||
62 | return 0; | ||
63 | } | ||
64 | |||
65 | -- | ||
66 | 2.31.1 | ||
67 | |||
68 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | While at it just call rbd_get_size and avoid rbd_image_info_t. | ||
4 | |||
5 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
6 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
7 | Message-Id: <20210702172356.11574-4-idryomov@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/rbd.c | 5 ++--- | ||
11 | 1 file changed, 2 insertions(+), 3 deletions(-) | ||
12 | |||
13 | diff --git a/block/rbd.c b/block/rbd.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/rbd.c | ||
16 | +++ b/block/rbd.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ImageInfoSpecific *qemu_rbd_get_specific_info(BlockDriverState *bs, | ||
18 | static int64_t qemu_rbd_getlength(BlockDriverState *bs) | ||
19 | { | ||
20 | BDRVRBDState *s = bs->opaque; | ||
21 | - rbd_image_info_t info; | ||
22 | int r; | ||
23 | |||
24 | - r = rbd_stat(s->image, &info, sizeof(info)); | ||
25 | + r = rbd_get_size(s->image, &s->image_size); | ||
26 | if (r < 0) { | ||
27 | return r; | ||
28 | } | ||
29 | |||
30 | - return info.size; | ||
31 | + return s->image_size; | ||
32 | } | ||
33 | |||
34 | static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs, | ||
35 | -- | ||
36 | 2.31.1 | ||
37 | |||
38 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Peter Lieven <pl@kamp.de> |
---|---|---|---|
2 | 2 | ||
3 | Do data compression in separate threads. This significantly improve | 3 | Signed-off-by: Peter Lieven <pl@kamp.de> |
4 | performance for qemu-img convert with -W (allow async writes) and -c | 4 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> |
5 | (compressed) options. | 5 | Message-Id: <20210702172356.11574-5-idryomov@gmail.com> |
6 | |||
7 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
9 | --- | 7 | --- |
10 | block/qcow2.h | 3 +++ | 8 | block/rbd.c | 252 +++++++++++++++++++--------------------------------- |
11 | block/qcow2.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++- | 9 | 1 file changed, 90 insertions(+), 162 deletions(-) |
12 | 2 files changed, 64 insertions(+), 1 deletion(-) | ||
13 | 10 | ||
14 | diff --git a/block/qcow2.h b/block/qcow2.h | 11 | diff --git a/block/rbd.c b/block/rbd.c |
15 | index XXXXXXX..XXXXXXX 100644 | 12 | index XXXXXXX..XXXXXXX 100644 |
16 | --- a/block/qcow2.h | 13 | --- a/block/rbd.c |
17 | +++ b/block/qcow2.h | 14 | +++ b/block/rbd.c |
18 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State { | 15 | @@ -XXX,XX +XXX,XX @@ typedef enum { |
19 | * override) */ | 16 | RBD_AIO_FLUSH |
20 | char *image_backing_file; | 17 | } RBDAIOCmd; |
21 | char *image_backing_format; | 18 | |
19 | -typedef struct RBDAIOCB { | ||
20 | - BlockAIOCB common; | ||
21 | - int64_t ret; | ||
22 | - QEMUIOVector *qiov; | ||
23 | - RBDAIOCmd cmd; | ||
24 | - int error; | ||
25 | - struct BDRVRBDState *s; | ||
26 | -} RBDAIOCB; | ||
27 | - | ||
28 | -typedef struct RADOSCB { | ||
29 | - RBDAIOCB *acb; | ||
30 | - struct BDRVRBDState *s; | ||
31 | - int64_t size; | ||
32 | - int64_t ret; | ||
33 | -} RADOSCB; | ||
34 | - | ||
35 | typedef struct BDRVRBDState { | ||
36 | rados_t cluster; | ||
37 | rados_ioctx_t io_ctx; | ||
38 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVRBDState { | ||
39 | uint64_t object_size; | ||
40 | } BDRVRBDState; | ||
41 | |||
42 | +typedef struct RBDTask { | ||
43 | + BlockDriverState *bs; | ||
44 | + Coroutine *co; | ||
45 | + bool complete; | ||
46 | + int64_t ret; | ||
47 | +} RBDTask; | ||
22 | + | 48 | + |
23 | + CoQueue compress_wait_queue; | 49 | static int qemu_rbd_connect(rados_t *cluster, rados_ioctx_t *io_ctx, |
24 | + int nb_compress_threads; | 50 | BlockdevOptionsRbd *opts, bool cache, |
25 | } BDRVQcow2State; | 51 | const char *keypairs, const char *secretid, |
26 | 52 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_set_keypairs(rados_t cluster, const char *keypairs_json, | |
27 | typedef struct Qcow2COWRegion { | 53 | return ret; |
28 | diff --git a/block/qcow2.c b/block/qcow2.c | 54 | } |
29 | index XXXXXXX..XXXXXXX 100644 | 55 | |
30 | --- a/block/qcow2.c | 56 | -static void qemu_rbd_memset(RADOSCB *rcb, int64_t offs) |
31 | +++ b/block/qcow2.c | 57 | -{ |
32 | @@ -XXX,XX +XXX,XX @@ | 58 | - RBDAIOCB *acb = rcb->acb; |
33 | #include "qapi/qobject-input-visitor.h" | 59 | - iov_memset(acb->qiov->iov, acb->qiov->niov, offs, 0, |
34 | #include "qapi/qapi-visit-block-core.h" | 60 | - acb->qiov->size - offs); |
35 | #include "crypto.h" | 61 | -} |
36 | +#include "block/thread-pool.h" | 62 | - |
63 | #ifdef LIBRBD_SUPPORTS_ENCRYPTION | ||
64 | static int qemu_rbd_convert_luks_options( | ||
65 | RbdEncryptionOptionsLUKSBase *luks_opts, | ||
66 | @@ -XXX,XX +XXX,XX @@ exit: | ||
67 | return ret; | ||
68 | } | ||
69 | |||
70 | -/* | ||
71 | - * This aio completion is being called from rbd_finish_bh() and runs in qemu | ||
72 | - * BH context. | ||
73 | - */ | ||
74 | -static void qemu_rbd_complete_aio(RADOSCB *rcb) | ||
75 | -{ | ||
76 | - RBDAIOCB *acb = rcb->acb; | ||
77 | - int64_t r; | ||
78 | - | ||
79 | - r = rcb->ret; | ||
80 | - | ||
81 | - if (acb->cmd != RBD_AIO_READ) { | ||
82 | - if (r < 0) { | ||
83 | - acb->ret = r; | ||
84 | - acb->error = 1; | ||
85 | - } else if (!acb->error) { | ||
86 | - acb->ret = rcb->size; | ||
87 | - } | ||
88 | - } else { | ||
89 | - if (r < 0) { | ||
90 | - qemu_rbd_memset(rcb, 0); | ||
91 | - acb->ret = r; | ||
92 | - acb->error = 1; | ||
93 | - } else if (r < rcb->size) { | ||
94 | - qemu_rbd_memset(rcb, r); | ||
95 | - if (!acb->error) { | ||
96 | - acb->ret = rcb->size; | ||
97 | - } | ||
98 | - } else if (!acb->error) { | ||
99 | - acb->ret = r; | ||
100 | - } | ||
101 | - } | ||
102 | - | ||
103 | - g_free(rcb); | ||
104 | - | ||
105 | - acb->common.cb(acb->common.opaque, (acb->ret > 0 ? 0 : acb->ret)); | ||
106 | - | ||
107 | - qemu_aio_unref(acb); | ||
108 | -} | ||
109 | - | ||
110 | static char *qemu_rbd_mon_host(BlockdevOptionsRbd *opts, Error **errp) | ||
111 | { | ||
112 | const char **vals; | ||
113 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_resize(BlockDriverState *bs, uint64_t size) | ||
114 | return 0; | ||
115 | } | ||
116 | |||
117 | -static const AIOCBInfo rbd_aiocb_info = { | ||
118 | - .aiocb_size = sizeof(RBDAIOCB), | ||
119 | -}; | ||
120 | - | ||
121 | -static void rbd_finish_bh(void *opaque) | ||
122 | +static void qemu_rbd_finish_bh(void *opaque) | ||
123 | { | ||
124 | - RADOSCB *rcb = opaque; | ||
125 | - qemu_rbd_complete_aio(rcb); | ||
126 | + RBDTask *task = opaque; | ||
127 | + task->complete = 1; | ||
128 | + aio_co_wake(task->co); | ||
129 | } | ||
37 | 130 | ||
38 | /* | 131 | /* |
39 | Differences with QCOW: | 132 | - * This is the callback function for rbd_aio_read and _write |
40 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, | 133 | + * This is the completion callback function for all rbd aio calls |
41 | qcow2_check_refcounts(bs, &result, 0); | 134 | + * started from qemu_rbd_start_co(). |
135 | * | ||
136 | * Note: this function is being called from a non qemu thread so | ||
137 | * we need to be careful about what we do here. Generally we only | ||
138 | * schedule a BH, and do the rest of the io completion handling | ||
139 | - * from rbd_finish_bh() which runs in a qemu context. | ||
140 | + * from qemu_rbd_finish_bh() which runs in a qemu context. | ||
141 | */ | ||
142 | -static void rbd_finish_aiocb(rbd_completion_t c, RADOSCB *rcb) | ||
143 | +static void qemu_rbd_completion_cb(rbd_completion_t c, RBDTask *task) | ||
144 | { | ||
145 | - RBDAIOCB *acb = rcb->acb; | ||
146 | - | ||
147 | - rcb->ret = rbd_aio_get_return_value(c); | ||
148 | + task->ret = rbd_aio_get_return_value(c); | ||
149 | rbd_aio_release(c); | ||
150 | - | ||
151 | - replay_bh_schedule_oneshot_event(bdrv_get_aio_context(acb->common.bs), | ||
152 | - rbd_finish_bh, rcb); | ||
153 | + aio_bh_schedule_oneshot(bdrv_get_aio_context(task->bs), | ||
154 | + qemu_rbd_finish_bh, task); | ||
155 | } | ||
156 | |||
157 | -static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
158 | - int64_t off, | ||
159 | - QEMUIOVector *qiov, | ||
160 | - int64_t size, | ||
161 | - BlockCompletionFunc *cb, | ||
162 | - void *opaque, | ||
163 | - RBDAIOCmd cmd) | ||
164 | +static int coroutine_fn qemu_rbd_start_co(BlockDriverState *bs, | ||
165 | + uint64_t offset, | ||
166 | + uint64_t bytes, | ||
167 | + QEMUIOVector *qiov, | ||
168 | + int flags, | ||
169 | + RBDAIOCmd cmd) | ||
170 | { | ||
171 | - RBDAIOCB *acb; | ||
172 | - RADOSCB *rcb = NULL; | ||
173 | + BDRVRBDState *s = bs->opaque; | ||
174 | + RBDTask task = { .bs = bs, .co = qemu_coroutine_self() }; | ||
175 | rbd_completion_t c; | ||
176 | int r; | ||
177 | |||
178 | - BDRVRBDState *s = bs->opaque; | ||
179 | - | ||
180 | - acb = qemu_aio_get(&rbd_aiocb_info, bs, cb, opaque); | ||
181 | - acb->cmd = cmd; | ||
182 | - acb->qiov = qiov; | ||
183 | - assert(!qiov || qiov->size == size); | ||
184 | - | ||
185 | - rcb = g_new(RADOSCB, 1); | ||
186 | + assert(!qiov || qiov->size == bytes); | ||
187 | |||
188 | - acb->ret = 0; | ||
189 | - acb->error = 0; | ||
190 | - acb->s = s; | ||
191 | - | ||
192 | - rcb->acb = acb; | ||
193 | - rcb->s = acb->s; | ||
194 | - rcb->size = size; | ||
195 | - r = rbd_aio_create_completion(rcb, (rbd_callback_t) rbd_finish_aiocb, &c); | ||
196 | + r = rbd_aio_create_completion(&task, | ||
197 | + (rbd_callback_t) qemu_rbd_completion_cb, &c); | ||
198 | if (r < 0) { | ||
199 | - goto failed; | ||
200 | + return r; | ||
42 | } | 201 | } |
43 | #endif | 202 | |
203 | switch (cmd) { | ||
204 | - case RBD_AIO_WRITE: | ||
205 | - /* | ||
206 | - * RBD APIs don't allow us to write more than actual size, so in order | ||
207 | - * to support growing images, we resize the image before write | ||
208 | - * operations that exceed the current size. | ||
209 | - */ | ||
210 | - if (off + size > s->image_size) { | ||
211 | - r = qemu_rbd_resize(bs, off + size); | ||
212 | - if (r < 0) { | ||
213 | - goto failed_completion; | ||
214 | - } | ||
215 | - } | ||
216 | - r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, off, c); | ||
217 | - break; | ||
218 | case RBD_AIO_READ: | ||
219 | - r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, off, c); | ||
220 | + r = rbd_aio_readv(s->image, qiov->iov, qiov->niov, offset, c); | ||
221 | + break; | ||
222 | + case RBD_AIO_WRITE: | ||
223 | + r = rbd_aio_writev(s->image, qiov->iov, qiov->niov, offset, c); | ||
224 | break; | ||
225 | case RBD_AIO_DISCARD: | ||
226 | - r = rbd_aio_discard(s->image, off, size, c); | ||
227 | + r = rbd_aio_discard(s->image, offset, bytes, c); | ||
228 | break; | ||
229 | case RBD_AIO_FLUSH: | ||
230 | r = rbd_aio_flush(s->image, c); | ||
231 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *rbd_start_aio(BlockDriverState *bs, | ||
232 | } | ||
233 | |||
234 | if (r < 0) { | ||
235 | - goto failed_completion; | ||
236 | + error_report("rbd request failed early: cmd %d offset %" PRIu64 | ||
237 | + " bytes %" PRIu64 " flags %d r %d (%s)", cmd, offset, | ||
238 | + bytes, flags, r, strerror(-r)); | ||
239 | + rbd_aio_release(c); | ||
240 | + return r; | ||
241 | } | ||
242 | - return &acb->common; | ||
243 | |||
244 | -failed_completion: | ||
245 | - rbd_aio_release(c); | ||
246 | -failed: | ||
247 | - g_free(rcb); | ||
248 | + while (!task.complete) { | ||
249 | + qemu_coroutine_yield(); | ||
250 | + } | ||
251 | |||
252 | - qemu_aio_unref(acb); | ||
253 | - return NULL; | ||
254 | + if (task.ret < 0) { | ||
255 | + error_report("rbd request failed: cmd %d offset %" PRIu64 " bytes %" | ||
256 | + PRIu64 " flags %d task.ret %" PRIi64 " (%s)", cmd, offset, | ||
257 | + bytes, flags, task.ret, strerror(-task.ret)); | ||
258 | + return task.ret; | ||
259 | + } | ||
44 | + | 260 | + |
45 | + qemu_co_queue_init(&s->compress_wait_queue); | 261 | + /* zero pad short reads */ |
46 | + | 262 | + if (cmd == RBD_AIO_READ && task.ret < qiov->size) { |
47 | return ret; | 263 | + qemu_iovec_memset(qiov, task.ret, 0, qiov->size - task.ret); |
48 | 264 | + } | |
49 | fail: | ||
50 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | ||
51 | return ret; | ||
52 | } | ||
53 | |||
54 | +#define MAX_COMPRESS_THREADS 4 | ||
55 | + | ||
56 | +typedef struct Qcow2CompressData { | ||
57 | + void *dest; | ||
58 | + const void *src; | ||
59 | + size_t size; | ||
60 | + ssize_t ret; | ||
61 | +} Qcow2CompressData; | ||
62 | + | ||
63 | +static int qcow2_compress_pool_func(void *opaque) | ||
64 | +{ | ||
65 | + Qcow2CompressData *data = opaque; | ||
66 | + | ||
67 | + data->ret = qcow2_compress(data->dest, data->src, data->size); | ||
68 | + | 265 | + |
69 | + return 0; | 266 | + return 0; |
70 | +} | 267 | +} |
71 | + | 268 | + |
72 | +static void qcow2_compress_complete(void *opaque, int ret) | 269 | +static int |
270 | +coroutine_fn qemu_rbd_co_preadv(BlockDriverState *bs, uint64_t offset, | ||
271 | + uint64_t bytes, QEMUIOVector *qiov, | ||
272 | + int flags) | ||
73 | +{ | 273 | +{ |
74 | + qemu_coroutine_enter(opaque); | 274 | + return qemu_rbd_start_co(bs, offset, bytes, qiov, flags, RBD_AIO_READ); |
75 | +} | 275 | } |
76 | + | 276 | |
77 | +/* See qcow2_compress definition for parameters description */ | 277 | -static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs, |
78 | +static ssize_t qcow2_co_compress(BlockDriverState *bs, | 278 | - uint64_t offset, uint64_t bytes, |
79 | + void *dest, const void *src, size_t size) | 279 | - QEMUIOVector *qiov, int flags, |
80 | +{ | 280 | - BlockCompletionFunc *cb, |
81 | + BDRVQcow2State *s = bs->opaque; | 281 | - void *opaque) |
82 | + BlockAIOCB *acb; | 282 | +static int |
83 | + ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | 283 | +coroutine_fn qemu_rbd_co_pwritev(BlockDriverState *bs, uint64_t offset, |
84 | + Qcow2CompressData arg = { | 284 | + uint64_t bytes, QEMUIOVector *qiov, |
85 | + .dest = dest, | 285 | + int flags) |
86 | + .src = src, | 286 | { |
87 | + .size = size, | 287 | - return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque, |
88 | + }; | 288 | - RBD_AIO_READ); |
89 | + | 289 | + BDRVRBDState *s = bs->opaque; |
90 | + while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) { | 290 | + /* |
91 | + qemu_co_queue_wait(&s->compress_wait_queue, NULL); | 291 | + * RBD APIs don't allow us to write more than actual size, so in order |
292 | + * to support growing images, we resize the image before write | ||
293 | + * operations that exceed the current size. | ||
294 | + */ | ||
295 | + if (offset + bytes > s->image_size) { | ||
296 | + int r = qemu_rbd_resize(bs, offset + bytes); | ||
297 | + if (r < 0) { | ||
298 | + return r; | ||
299 | + } | ||
92 | + } | 300 | + } |
93 | + | 301 | + return qemu_rbd_start_co(bs, offset, bytes, qiov, flags, RBD_AIO_WRITE); |
94 | + s->nb_compress_threads++; | 302 | } |
95 | + acb = thread_pool_submit_aio(pool, qcow2_compress_pool_func, &arg, | 303 | |
96 | + qcow2_compress_complete, | 304 | -static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs, |
97 | + qemu_coroutine_self()); | 305 | - uint64_t offset, uint64_t bytes, |
98 | + | 306 | - QEMUIOVector *qiov, int flags, |
99 | + if (!acb) { | 307 | - BlockCompletionFunc *cb, |
100 | + s->nb_compress_threads--; | 308 | - void *opaque) |
101 | + return -EINVAL; | 309 | +static int coroutine_fn qemu_rbd_co_flush(BlockDriverState *bs) |
102 | + } | 310 | { |
103 | + qemu_coroutine_yield(); | 311 | - return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque, |
104 | + s->nb_compress_threads--; | 312 | - RBD_AIO_WRITE); |
105 | + qemu_co_queue_next(&s->compress_wait_queue); | 313 | + return qemu_rbd_start_co(bs, 0, 0, NULL, 0, RBD_AIO_FLUSH); |
106 | + | 314 | } |
107 | + return arg.ret; | 315 | |
108 | +} | 316 | -static BlockAIOCB *qemu_rbd_aio_flush(BlockDriverState *bs, |
109 | + | 317 | - BlockCompletionFunc *cb, |
110 | /* XXX: put compressed sectors first, then all the cluster aligned | 318 | - void *opaque) |
111 | tables to avoid losing bytes in alignment */ | 319 | +static int coroutine_fn qemu_rbd_co_pdiscard(BlockDriverState *bs, |
112 | static coroutine_fn int | 320 | + int64_t offset, int count) |
113 | @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, | 321 | { |
114 | 322 | - return rbd_start_aio(bs, 0, NULL, 0, cb, opaque, RBD_AIO_FLUSH); | |
115 | out_buf = g_malloc(s->cluster_size); | 323 | + return qemu_rbd_start_co(bs, offset, count, NULL, 0, RBD_AIO_DISCARD); |
116 | 324 | } | |
117 | - out_len = qcow2_compress(out_buf, buf, s->cluster_size); | 325 | |
118 | + out_len = qcow2_co_compress(bs, out_buf, buf, s->cluster_size); | 326 | static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) |
119 | if (out_len == -2) { | 327 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_snap_list(BlockDriverState *bs, |
120 | ret = -EINVAL; | 328 | return snap_count; |
121 | goto fail; | 329 | } |
330 | |||
331 | -static BlockAIOCB *qemu_rbd_aio_pdiscard(BlockDriverState *bs, | ||
332 | - int64_t offset, | ||
333 | - int bytes, | ||
334 | - BlockCompletionFunc *cb, | ||
335 | - void *opaque) | ||
336 | -{ | ||
337 | - return rbd_start_aio(bs, offset, NULL, bytes, cb, opaque, | ||
338 | - RBD_AIO_DISCARD); | ||
339 | -} | ||
340 | - | ||
341 | static void coroutine_fn qemu_rbd_co_invalidate_cache(BlockDriverState *bs, | ||
342 | Error **errp) | ||
343 | { | ||
344 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { | ||
345 | .bdrv_co_truncate = qemu_rbd_co_truncate, | ||
346 | .protocol_name = "rbd", | ||
347 | |||
348 | - .bdrv_aio_preadv = qemu_rbd_aio_preadv, | ||
349 | - .bdrv_aio_pwritev = qemu_rbd_aio_pwritev, | ||
350 | - | ||
351 | - .bdrv_aio_flush = qemu_rbd_aio_flush, | ||
352 | - .bdrv_aio_pdiscard = qemu_rbd_aio_pdiscard, | ||
353 | + .bdrv_co_preadv = qemu_rbd_co_preadv, | ||
354 | + .bdrv_co_pwritev = qemu_rbd_co_pwritev, | ||
355 | + .bdrv_co_flush_to_disk = qemu_rbd_co_flush, | ||
356 | + .bdrv_co_pdiscard = qemu_rbd_co_pdiscard, | ||
357 | |||
358 | .bdrv_snapshot_create = qemu_rbd_snap_create, | ||
359 | .bdrv_snapshot_delete = qemu_rbd_snap_remove, | ||
122 | -- | 360 | -- |
123 | 2.13.6 | 361 | 2.31.1 |
124 | 362 | ||
125 | 363 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | This patch wittingly sets BDRV_REQ_NO_FALLBACK and silently ignores | ||
4 | BDRV_REQ_MAY_UNMAP for older librbd versions. | ||
5 | |||
6 | The rationale for this is as follows (citing Ilya Dryomov current RBD | ||
7 | maintainer): | ||
8 | |||
9 | ---8<--- | ||
10 | a) remove the BDRV_REQ_MAY_UNMAP check in qemu_rbd_co_pwrite_zeroes() | ||
11 | and as a consequence always unmap if librbd is too old | ||
12 | |||
13 | It's not clear what qemu's expectation is but in general Write | ||
14 | Zeroes is allowed to unmap. The only guarantee is that subsequent | ||
15 | reads return zeroes, everything else is a hint. This is how it is | ||
16 | specified in the kernel and in the NVMe spec. | ||
17 | |||
18 | In particular, block/nvme.c implements it as follows: | ||
19 | |||
20 | if (flags & BDRV_REQ_MAY_UNMAP) { | ||
21 | cdw12 |= (1 << 25); | ||
22 | } | ||
23 | |||
24 | This sets the Deallocate bit. But if it's not set, the device may | ||
25 | still deallocate: | ||
26 | |||
27 | """ | ||
28 | If the Deallocate bit (CDW12.DEAC) is set to '1' in a Write Zeroes | ||
29 | command, and the namespace supports clearing all bytes to 0h in the | ||
30 | values read (e.g., bits 2:0 in the DLFEAT field are set to 001b) | ||
31 | from a deallocated logical block and its metadata (excluding | ||
32 | protection information), then for each specified logical block, the | ||
33 | controller: | ||
34 | - should deallocate that logical block; | ||
35 | |||
36 | ... | ||
37 | |||
38 | If the Deallocate bit is cleared to '0' in a Write Zeroes command, | ||
39 | and the namespace supports clearing all bytes to 0h in the values | ||
40 | read (e.g., bits 2:0 in the DLFEAT field are set to 001b) from | ||
41 | a deallocated logical block and its metadata (excluding protection | ||
42 | information), then, for each specified logical block, the | ||
43 | controller: | ||
44 | - may deallocate that logical block; | ||
45 | """ | ||
46 | |||
47 | https://nvmexpress.org/wp-content/uploads/NVM-Express-NVM-Command-Set-Specification-2021.06.02-Ratified-1.pdf | ||
48 | |||
49 | b) set BDRV_REQ_NO_FALLBACK in supported_zero_flags | ||
50 | |||
51 | Again, it's not clear what qemu expects here, but without it we end | ||
52 | up in a ridiculous situation where specifying the "don't allow slow | ||
53 | fallback" switch immediately fails all efficient zeroing requests on | ||
54 | a device where Write Zeroes is always efficient: | ||
55 | |||
56 | $ qemu-io -c 'help write' | grep -- '-[zun]' | ||
57 | -n, -- with -z, don't allow slow fallback | ||
58 | -u, -- with -z, allow unmapping | ||
59 | -z, -- write zeroes using blk_co_pwrite_zeroes | ||
60 | |||
61 | $ qemu-io -f rbd -c 'write -z -u -n 0 1M' rbd:foo/bar | ||
62 | write failed: Operation not supported | ||
63 | --->8--- | ||
64 | |||
65 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
66 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
67 | Message-Id: <20210702172356.11574-6-idryomov@gmail.com> | ||
68 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
69 | --- | ||
70 | block/rbd.c | 32 +++++++++++++++++++++++++++++++- | ||
71 | 1 file changed, 31 insertions(+), 1 deletion(-) | ||
72 | |||
73 | diff --git a/block/rbd.c b/block/rbd.c | ||
74 | index XXXXXXX..XXXXXXX 100644 | ||
75 | --- a/block/rbd.c | ||
76 | +++ b/block/rbd.c | ||
77 | @@ -XXX,XX +XXX,XX @@ typedef enum { | ||
78 | RBD_AIO_READ, | ||
79 | RBD_AIO_WRITE, | ||
80 | RBD_AIO_DISCARD, | ||
81 | - RBD_AIO_FLUSH | ||
82 | + RBD_AIO_FLUSH, | ||
83 | + RBD_AIO_WRITE_ZEROES | ||
84 | } RBDAIOCmd; | ||
85 | |||
86 | typedef struct BDRVRBDState { | ||
87 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags, | ||
88 | } | ||
89 | } | ||
90 | |||
91 | +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES | ||
92 | + bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK; | ||
93 | +#endif | ||
94 | + | ||
95 | /* When extending regular files, we get zeros from the OS */ | ||
96 | bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; | ||
97 | |||
98 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qemu_rbd_start_co(BlockDriverState *bs, | ||
99 | case RBD_AIO_FLUSH: | ||
100 | r = rbd_aio_flush(s->image, c); | ||
101 | break; | ||
102 | +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES | ||
103 | + case RBD_AIO_WRITE_ZEROES: { | ||
104 | + int zero_flags = 0; | ||
105 | +#ifdef RBD_WRITE_ZEROES_FLAG_THICK_PROVISION | ||
106 | + if (!(flags & BDRV_REQ_MAY_UNMAP)) { | ||
107 | + zero_flags = RBD_WRITE_ZEROES_FLAG_THICK_PROVISION; | ||
108 | + } | ||
109 | +#endif | ||
110 | + r = rbd_aio_write_zeroes(s->image, offset, bytes, c, zero_flags, 0); | ||
111 | + break; | ||
112 | + } | ||
113 | +#endif | ||
114 | default: | ||
115 | r = -EINVAL; | ||
116 | } | ||
117 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qemu_rbd_co_pdiscard(BlockDriverState *bs, | ||
118 | return qemu_rbd_start_co(bs, offset, count, NULL, 0, RBD_AIO_DISCARD); | ||
119 | } | ||
120 | |||
121 | +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES | ||
122 | +static int | ||
123 | +coroutine_fn qemu_rbd_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset, | ||
124 | + int count, BdrvRequestFlags flags) | ||
125 | +{ | ||
126 | + return qemu_rbd_start_co(bs, offset, count, NULL, flags, | ||
127 | + RBD_AIO_WRITE_ZEROES); | ||
128 | +} | ||
129 | +#endif | ||
130 | + | ||
131 | static int qemu_rbd_getinfo(BlockDriverState *bs, BlockDriverInfo *bdi) | ||
132 | { | ||
133 | BDRVRBDState *s = bs->opaque; | ||
134 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { | ||
135 | .bdrv_co_pwritev = qemu_rbd_co_pwritev, | ||
136 | .bdrv_co_flush_to_disk = qemu_rbd_co_flush, | ||
137 | .bdrv_co_pdiscard = qemu_rbd_co_pdiscard, | ||
138 | +#ifdef LIBRBD_SUPPORTS_WRITE_ZEROES | ||
139 | + .bdrv_co_pwrite_zeroes = qemu_rbd_co_pwrite_zeroes, | ||
140 | +#endif | ||
141 | |||
142 | .bdrv_snapshot_create = qemu_rbd_snap_create, | ||
143 | .bdrv_snapshot_delete = qemu_rbd_snap_remove, | ||
144 | -- | ||
145 | 2.31.1 | ||
146 | |||
147 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | librbd supports 1 byte alignment for all aio operations. | ||
4 | |||
5 | Currently, there is no API call to query limits from the Ceph | ||
6 | ObjectStore backend. So drop the bdrv_refresh_limits completely | ||
7 | until there is such an API call. | ||
8 | |||
9 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
10 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
11 | Message-Id: <20210702172356.11574-7-idryomov@gmail.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | block/rbd.c | 9 --------- | ||
15 | 1 file changed, 9 deletions(-) | ||
16 | |||
17 | diff --git a/block/rbd.c b/block/rbd.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/block/rbd.c | ||
20 | +++ b/block/rbd.c | ||
21 | @@ -XXX,XX +XXX,XX @@ done: | ||
22 | return; | ||
23 | } | ||
24 | |||
25 | - | ||
26 | -static void qemu_rbd_refresh_limits(BlockDriverState *bs, Error **errp) | ||
27 | -{ | ||
28 | - /* XXX Does RBD support AIO on less than 512-byte alignment? */ | ||
29 | - bs->bl.request_alignment = 512; | ||
30 | -} | ||
31 | - | ||
32 | - | ||
33 | static int qemu_rbd_set_auth(rados_t cluster, BlockdevOptionsRbd *opts, | ||
34 | Error **errp) | ||
35 | { | ||
36 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_rbd = { | ||
37 | .format_name = "rbd", | ||
38 | .instance_size = sizeof(BDRVRBDState), | ||
39 | .bdrv_parse_filename = qemu_rbd_parse_filename, | ||
40 | - .bdrv_refresh_limits = qemu_rbd_refresh_limits, | ||
41 | .bdrv_file_open = qemu_rbd_open, | ||
42 | .bdrv_close = qemu_rbd_close, | ||
43 | .bdrv_reopen_prepare = qemu_rbd_reopen_prepare, | ||
44 | -- | ||
45 | 2.31.1 | ||
46 | |||
47 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Heinrich Schuchardt <xypron.glpk@gmx.de> | ||
1 | 2 | ||
3 | uri_free() checks if its argument is NULL in uri_clean() and g_free(). | ||
4 | There is no need to check the argument before the call. | ||
5 | |||
6 | Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> | ||
7 | Message-Id: <20210629063602.4239-1-xypron.glpk@gmx.de> | ||
8 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
9 | Reviewed-by: Richard W.M. Jones <rjones@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | block/nfs.c | 4 +--- | ||
13 | block/ssh.c | 4 +--- | ||
14 | util/uri.c | 22 ++++++---------------- | ||
15 | 3 files changed, 8 insertions(+), 22 deletions(-) | ||
16 | |||
17 | diff --git a/block/nfs.c b/block/nfs.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/block/nfs.c | ||
20 | +++ b/block/nfs.c | ||
21 | @@ -XXX,XX +XXX,XX @@ out: | ||
22 | if (qp) { | ||
23 | query_params_free(qp); | ||
24 | } | ||
25 | - if (uri) { | ||
26 | - uri_free(uri); | ||
27 | - } | ||
28 | + uri_free(uri); | ||
29 | return ret; | ||
30 | } | ||
31 | |||
32 | diff --git a/block/ssh.c b/block/ssh.c | ||
33 | index XXXXXXX..XXXXXXX 100644 | ||
34 | --- a/block/ssh.c | ||
35 | +++ b/block/ssh.c | ||
36 | @@ -XXX,XX +XXX,XX @@ static int parse_uri(const char *filename, QDict *options, Error **errp) | ||
37 | return 0; | ||
38 | |||
39 | err: | ||
40 | - if (uri) { | ||
41 | - uri_free(uri); | ||
42 | - } | ||
43 | + uri_free(uri); | ||
44 | return -EINVAL; | ||
45 | } | ||
46 | |||
47 | diff --git a/util/uri.c b/util/uri.c | ||
48 | index XXXXXXX..XXXXXXX 100644 | ||
49 | --- a/util/uri.c | ||
50 | +++ b/util/uri.c | ||
51 | @@ -XXX,XX +XXX,XX @@ static void uri_clean(URI *uri) | ||
52 | |||
53 | /** | ||
54 | * uri_free: | ||
55 | - * @uri: pointer to an URI | ||
56 | + * @uri: pointer to an URI, NULL is ignored | ||
57 | * | ||
58 | * Free up the URI struct | ||
59 | */ | ||
60 | @@ -XXX,XX +XXX,XX @@ step_7: | ||
61 | val = uri_to_string(res); | ||
62 | |||
63 | done: | ||
64 | - if (ref != NULL) { | ||
65 | - uri_free(ref); | ||
66 | - } | ||
67 | - if (bas != NULL) { | ||
68 | - uri_free(bas); | ||
69 | - } | ||
70 | - if (res != NULL) { | ||
71 | - uri_free(res); | ||
72 | - } | ||
73 | + uri_free(ref); | ||
74 | + uri_free(bas); | ||
75 | + uri_free(res); | ||
76 | return val; | ||
77 | } | ||
78 | |||
79 | @@ -XXX,XX +XXX,XX @@ done: | ||
80 | if (remove_path != 0) { | ||
81 | ref->path = NULL; | ||
82 | } | ||
83 | - if (ref != NULL) { | ||
84 | - uri_free(ref); | ||
85 | - } | ||
86 | - if (bas != NULL) { | ||
87 | - uri_free(bas); | ||
88 | - } | ||
89 | + uri_free(ref); | ||
90 | + uri_free(bas); | ||
91 | |||
92 | return val; | ||
93 | } | ||
94 | -- | ||
95 | 2.31.1 | ||
96 | |||
97 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
1 | 2 | ||
3 | We do not do any permission checks in fuse_open(), so let the kernel do | ||
4 | them. We already let fuse_getattr() report the proper UNIX permissions, | ||
5 | so this should work the way we want. | ||
6 | |||
7 | This causes a change in 308's reference output, because now opening a | ||
8 | non-writable export with O_RDWR fails already, instead of only actually | ||
9 | attempting to write to it. (That is an improvement.) | ||
10 | |||
11 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
12 | Message-Id: <20210625142317.271673-2-mreitz@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | block/export/fuse.c | 8 ++++++-- | ||
16 | tests/qemu-iotests/308 | 3 ++- | ||
17 | tests/qemu-iotests/308.out | 2 +- | ||
18 | 3 files changed, 9 insertions(+), 4 deletions(-) | ||
19 | |||
20 | diff --git a/block/export/fuse.c b/block/export/fuse.c | ||
21 | index XXXXXXX..XXXXXXX 100644 | ||
22 | --- a/block/export/fuse.c | ||
23 | +++ b/block/export/fuse.c | ||
24 | @@ -XXX,XX +XXX,XX @@ static int setup_fuse_export(FuseExport *exp, const char *mountpoint, | ||
25 | struct fuse_args fuse_args; | ||
26 | int ret; | ||
27 | |||
28 | - /* Needs to match what fuse_init() sets. Only max_read must be supplied. */ | ||
29 | - mount_opts = g_strdup_printf("max_read=%zu", FUSE_MAX_BOUNCE_BYTES); | ||
30 | + /* | ||
31 | + * max_read needs to match what fuse_init() sets. | ||
32 | + * max_write need not be supplied. | ||
33 | + */ | ||
34 | + mount_opts = g_strdup_printf("max_read=%zu,default_permissions", | ||
35 | + FUSE_MAX_BOUNCE_BYTES); | ||
36 | |||
37 | fuse_argv[0] = ""; /* Dummy program name */ | ||
38 | fuse_argv[1] = "-o"; | ||
39 | diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308 | ||
40 | index XXXXXXX..XXXXXXX 100755 | ||
41 | --- a/tests/qemu-iotests/308 | ||
42 | +++ b/tests/qemu-iotests/308 | ||
43 | @@ -XXX,XX +XXX,XX @@ echo '=== Writable export ===' | ||
44 | fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP', 'writable': true" | ||
45 | |||
46 | # Check that writing to the read-only export fails | ||
47 | -$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" | _filter_qemu_io | ||
48 | +$QEMU_IO -f raw -c 'write -P 42 1M 64k' "$TEST_IMG" 2>&1 \ | ||
49 | + | _filter_qemu_io | _filter_testdir | _filter_imgfmt | ||
50 | |||
51 | # But here it should work | ||
52 | $QEMU_IO -f raw -c 'write -P 42 1M 64k' "$EXT_MP" | _filter_qemu_io | ||
53 | diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out | ||
54 | index XXXXXXX..XXXXXXX 100644 | ||
55 | --- a/tests/qemu-iotests/308.out | ||
56 | +++ b/tests/qemu-iotests/308.out | ||
57 | @@ -XXX,XX +XXX,XX @@ virtual size: 0 B (0 bytes) | ||
58 | 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true | ||
59 | } } | ||
60 | {"return": {}} | ||
61 | -write failed: Permission denied | ||
62 | +qemu-io: can't open device TEST_DIR/t.IMGFMT: Could not open 'TEST_DIR/t.IMGFMT': Permission denied | ||
63 | wrote 65536/65536 bytes at offset 1048576 | ||
64 | 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
65 | wrote 65536/65536 bytes at offset 1048576 | ||
66 | -- | ||
67 | 2.31.1 | ||
68 | |||
69 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Max Reitz <mreitz@redhat.com> | |
2 | |||
3 | Without the allow_other mount option, no user (not even root) but the | ||
4 | one who started qemu/the storage daemon can access the export. Allow | ||
5 | users to configure the export such that such accesses are possible. | ||
6 | |||
7 | While allow_other is probably what users want, we cannot make it an | ||
8 | unconditional default, because passing it is only possible (for non-root | ||
9 | users) if the global fuse.conf configuration file allows it. Thus, the | ||
10 | default is an 'auto' mode, in which we first try with allow_other, and | ||
11 | then fall back to without. | ||
12 | |||
13 | FuseExport.allow_other reports whether allow_other was actually used as | ||
14 | a mount option or not. Currently, this information is not used, but a | ||
15 | future patch will let this field decide whether e.g. an export's UID and | ||
16 | GID can be changed through chmod. | ||
17 | |||
18 | One notable thing about 'auto' mode is that libfuse may print error | ||
19 | messages directly to stderr, and so may fusermount (which it executes). | ||
20 | Our export code cannot really filter or hide them. Therefore, if 'auto' | ||
21 | fails its first attempt and has to fall back, fusermount will print an | ||
22 | error message that mounting with allow_other failed. | ||
23 | |||
24 | This behavior necessitates a change to iotest 308, namely we need to | ||
25 | filter out this error message (because if the first attempt at mounting | ||
26 | with allow_other succeeds, there will be no such message). | ||
27 | |||
28 | Furthermore, common.rc's _make_test_img should use allow-other=off for | ||
29 | FUSE exports, because iotests generally do not need to access images | ||
30 | from other users, so allow-other=on or allow-other=auto have no | ||
31 | advantage. OTOH, allow-other=on will not work on systems where | ||
32 | user_allow_other is disabled, and with allow-other=auto, we get said | ||
33 | error message that we would need to filter out again. Just disabling | ||
34 | allow-other is simplest. | ||
35 | |||
36 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
37 | Message-Id: <20210625142317.271673-3-mreitz@redhat.com> | ||
38 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
39 | --- | ||
40 | qapi/block-export.json | 33 ++++++++++++++++++++++++++++++++- | ||
41 | block/export/fuse.c | 28 +++++++++++++++++++++++----- | ||
42 | tests/qemu-iotests/308 | 6 +++++- | ||
43 | tests/qemu-iotests/common.rc | 6 +++++- | ||
44 | 4 files changed, 65 insertions(+), 8 deletions(-) | ||
45 | |||
46 | diff --git a/qapi/block-export.json b/qapi/block-export.json | ||
47 | index XXXXXXX..XXXXXXX 100644 | ||
48 | --- a/qapi/block-export.json | ||
49 | +++ b/qapi/block-export.json | ||
50 | @@ -XXX,XX +XXX,XX @@ | ||
51 | '*logical-block-size': 'size', | ||
52 | '*num-queues': 'uint16'} } | ||
53 | |||
54 | +## | ||
55 | +# @FuseExportAllowOther: | ||
56 | +# | ||
57 | +# Possible allow_other modes for FUSE exports. | ||
58 | +# | ||
59 | +# @off: Do not pass allow_other as a mount option. | ||
60 | +# | ||
61 | +# @on: Pass allow_other as a mount option. | ||
62 | +# | ||
63 | +# @auto: Try mounting with allow_other first, and if that fails, retry | ||
64 | +# without allow_other. | ||
65 | +# | ||
66 | +# Since: 6.1 | ||
67 | +## | ||
68 | +{ 'enum': 'FuseExportAllowOther', | ||
69 | + 'data': ['off', 'on', 'auto'] } | ||
70 | + | ||
71 | ## | ||
72 | # @BlockExportOptionsFuse: | ||
73 | # | ||
74 | @@ -XXX,XX +XXX,XX @@ | ||
75 | # @growable: Whether writes beyond the EOF should grow the block node | ||
76 | # accordingly. (default: false) | ||
77 | # | ||
78 | +# @allow-other: If this is off, only qemu's user is allowed access to | ||
79 | +# this export. That cannot be changed even with chmod or | ||
80 | +# chown. | ||
81 | +# Enabling this option will allow other users access to | ||
82 | +# the export with the FUSE mount option "allow_other". | ||
83 | +# Note that using allow_other as a non-root user requires | ||
84 | +# user_allow_other to be enabled in the global fuse.conf | ||
85 | +# configuration file. | ||
86 | +# In auto mode (the default), the FUSE export driver will | ||
87 | +# first attempt to mount the export with allow_other, and | ||
88 | +# if that fails, try again without. | ||
89 | +# (since 6.1; default: auto) | ||
90 | +# | ||
91 | # Since: 6.0 | ||
92 | ## | ||
93 | { 'struct': 'BlockExportOptionsFuse', | ||
94 | 'data': { 'mountpoint': 'str', | ||
95 | - '*growable': 'bool' }, | ||
96 | + '*growable': 'bool', | ||
97 | + '*allow-other': 'FuseExportAllowOther' }, | ||
98 | 'if': 'defined(CONFIG_FUSE)' } | ||
99 | |||
100 | ## | ||
101 | diff --git a/block/export/fuse.c b/block/export/fuse.c | ||
102 | index XXXXXXX..XXXXXXX 100644 | ||
103 | --- a/block/export/fuse.c | ||
104 | +++ b/block/export/fuse.c | ||
105 | @@ -XXX,XX +XXX,XX @@ typedef struct FuseExport { | ||
106 | char *mountpoint; | ||
107 | bool writable; | ||
108 | bool growable; | ||
109 | + /* Whether allow_other was used as a mount option or not */ | ||
110 | + bool allow_other; | ||
111 | } FuseExport; | ||
112 | |||
113 | static GHashTable *exports; | ||
114 | @@ -XXX,XX +XXX,XX @@ static void fuse_export_delete(BlockExport *exp); | ||
115 | static void init_exports_table(void); | ||
116 | |||
117 | static int setup_fuse_export(FuseExport *exp, const char *mountpoint, | ||
118 | - Error **errp); | ||
119 | + bool allow_other, Error **errp); | ||
120 | static void read_from_fuse_export(void *opaque); | ||
121 | |||
122 | static bool is_regular_file(const char *path, Error **errp); | ||
123 | @@ -XXX,XX +XXX,XX @@ static int fuse_export_create(BlockExport *blk_exp, | ||
124 | exp->writable = blk_exp_args->writable; | ||
125 | exp->growable = args->growable; | ||
126 | |||
127 | - ret = setup_fuse_export(exp, args->mountpoint, errp); | ||
128 | + /* set default */ | ||
129 | + if (!args->has_allow_other) { | ||
130 | + args->allow_other = FUSE_EXPORT_ALLOW_OTHER_AUTO; | ||
131 | + } | ||
132 | + | ||
133 | + if (args->allow_other == FUSE_EXPORT_ALLOW_OTHER_AUTO) { | ||
134 | + /* Ignore errors on our first attempt */ | ||
135 | + ret = setup_fuse_export(exp, args->mountpoint, true, NULL); | ||
136 | + exp->allow_other = ret == 0; | ||
137 | + if (ret < 0) { | ||
138 | + ret = setup_fuse_export(exp, args->mountpoint, false, errp); | ||
139 | + } | ||
140 | + } else { | ||
141 | + exp->allow_other = args->allow_other == FUSE_EXPORT_ALLOW_OTHER_ON; | ||
142 | + ret = setup_fuse_export(exp, args->mountpoint, exp->allow_other, errp); | ||
143 | + } | ||
144 | if (ret < 0) { | ||
145 | goto fail; | ||
146 | } | ||
147 | @@ -XXX,XX +XXX,XX @@ static void init_exports_table(void) | ||
148 | * Create exp->fuse_session and mount it. | ||
149 | */ | ||
150 | static int setup_fuse_export(FuseExport *exp, const char *mountpoint, | ||
151 | - Error **errp) | ||
152 | + bool allow_other, Error **errp) | ||
153 | { | ||
154 | const char *fuse_argv[4]; | ||
155 | char *mount_opts; | ||
156 | @@ -XXX,XX +XXX,XX @@ static int setup_fuse_export(FuseExport *exp, const char *mountpoint, | ||
157 | * max_read needs to match what fuse_init() sets. | ||
158 | * max_write need not be supplied. | ||
159 | */ | ||
160 | - mount_opts = g_strdup_printf("max_read=%zu,default_permissions", | ||
161 | - FUSE_MAX_BOUNCE_BYTES); | ||
162 | + mount_opts = g_strdup_printf("max_read=%zu,default_permissions%s", | ||
163 | + FUSE_MAX_BOUNCE_BYTES, | ||
164 | + allow_other ? ",allow_other" : ""); | ||
165 | |||
166 | fuse_argv[0] = ""; /* Dummy program name */ | ||
167 | fuse_argv[1] = "-o"; | ||
168 | diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308 | ||
169 | index XXXXXXX..XXXXXXX 100755 | ||
170 | --- a/tests/qemu-iotests/308 | ||
171 | +++ b/tests/qemu-iotests/308 | ||
172 | @@ -XXX,XX +XXX,XX @@ _supported_os Linux # We need /dev/urandom | ||
173 | # $4: Node to export (defaults to 'node-format') | ||
174 | fuse_export_add() | ||
175 | { | ||
176 | + # The grep -v is a filter for errors when /etc/fuse.conf does not contain | ||
177 | + # user_allow_other. (The error is benign, but it is printed by fusermount | ||
178 | + # on the first mount attempt, so our export code cannot hide it.) | ||
179 | _send_qemu_cmd $QEMU_HANDLE \ | ||
180 | "{'execute': 'block-export-add', | ||
181 | 'arguments': { | ||
182 | @@ -XXX,XX +XXX,XX @@ fuse_export_add() | ||
183 | $2 | ||
184 | } }" \ | ||
185 | "${3:-return}" \ | ||
186 | - | _filter_imgfmt | ||
187 | + | _filter_imgfmt \ | ||
188 | + | grep -v 'option allow_other only allowed if' | ||
189 | } | ||
190 | |||
191 | # $1: Export ID | ||
192 | diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc | ||
193 | index XXXXXXX..XXXXXXX 100644 | ||
194 | --- a/tests/qemu-iotests/common.rc | ||
195 | +++ b/tests/qemu-iotests/common.rc | ||
196 | @@ -XXX,XX +XXX,XX @@ _make_test_img() | ||
197 | # Usually, users would export formatted nodes. But we present fuse as a | ||
198 | # protocol-level driver here, so we have to leave the format to the | ||
199 | # client. | ||
200 | + # Switch off allow-other, because in general we do not need it for | ||
201 | + # iotests. The default allow-other=auto has the downside of printing a | ||
202 | + # fusermount error on its first attempt if allow_other is not | ||
203 | + # permissible, which we would need to filter. | ||
204 | QSD_NEED_PID=y $QSD \ | ||
205 | --blockdev file,node-name=export-node,filename=$img_name,discard=unmap \ | ||
206 | - --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on \ | ||
207 | + --export fuse,id=fuse-export,node-name=export-node,mountpoint="$export_mp",writable=on,growable=on,allow-other=off \ | ||
208 | & | ||
209 | |||
210 | pidfile="$QEMU_TEST_DIR/qemu-storage-daemon.pid" | ||
211 | -- | ||
212 | 2.31.1 | ||
213 | |||
214 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
1 | 2 | ||
3 | In order to support changing other attributes than the file size in | ||
4 | fuse_setattr(), we have to give each its own independent branch. This | ||
5 | also applies to the only attribute we do support right now. | ||
6 | |||
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
8 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | Message-Id: <20210625142317.271673-4-mreitz@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | block/export/fuse.c | 20 +++++++++++--------- | ||
13 | 1 file changed, 11 insertions(+), 9 deletions(-) | ||
14 | |||
15 | diff --git a/block/export/fuse.c b/block/export/fuse.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/block/export/fuse.c | ||
18 | +++ b/block/export/fuse.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf, | ||
20 | FuseExport *exp = fuse_req_userdata(req); | ||
21 | int ret; | ||
22 | |||
23 | - if (!exp->writable) { | ||
24 | - fuse_reply_err(req, EACCES); | ||
25 | - return; | ||
26 | - } | ||
27 | - | ||
28 | if (to_set & ~FUSE_SET_ATTR_SIZE) { | ||
29 | fuse_reply_err(req, ENOTSUP); | ||
30 | return; | ||
31 | } | ||
32 | |||
33 | - ret = fuse_do_truncate(exp, statbuf->st_size, true, PREALLOC_MODE_OFF); | ||
34 | - if (ret < 0) { | ||
35 | - fuse_reply_err(req, -ret); | ||
36 | - return; | ||
37 | + if (to_set & FUSE_SET_ATTR_SIZE) { | ||
38 | + if (!exp->writable) { | ||
39 | + fuse_reply_err(req, EACCES); | ||
40 | + return; | ||
41 | + } | ||
42 | + | ||
43 | + ret = fuse_do_truncate(exp, statbuf->st_size, true, PREALLOC_MODE_OFF); | ||
44 | + if (ret < 0) { | ||
45 | + fuse_reply_err(req, -ret); | ||
46 | + return; | ||
47 | + } | ||
48 | } | ||
49 | |||
50 | fuse_getattr(req, inode, fi); | ||
51 | -- | ||
52 | 2.31.1 | ||
53 | |||
54 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
1 | 2 | ||
3 | Allow changing the file mode, UID, and GID through SETATTR. | ||
4 | |||
5 | Without allow_other, UID and GID are not allowed to be changed, because | ||
6 | it would not make sense. Also, changing group or others' permissions | ||
7 | is not allowed either. | ||
8 | |||
9 | For read-only exports, +w cannot be set. | ||
10 | |||
11 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
12 | Message-Id: <20210625142317.271673-5-mreitz@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | block/export/fuse.c | 73 ++++++++++++++++++++++++++++++++++++++------- | ||
16 | 1 file changed, 62 insertions(+), 11 deletions(-) | ||
17 | |||
18 | diff --git a/block/export/fuse.c b/block/export/fuse.c | ||
19 | index XXXXXXX..XXXXXXX 100644 | ||
20 | --- a/block/export/fuse.c | ||
21 | +++ b/block/export/fuse.c | ||
22 | @@ -XXX,XX +XXX,XX @@ typedef struct FuseExport { | ||
23 | bool growable; | ||
24 | /* Whether allow_other was used as a mount option or not */ | ||
25 | bool allow_other; | ||
26 | + | ||
27 | + mode_t st_mode; | ||
28 | + uid_t st_uid; | ||
29 | + gid_t st_gid; | ||
30 | } FuseExport; | ||
31 | |||
32 | static GHashTable *exports; | ||
33 | @@ -XXX,XX +XXX,XX @@ static int fuse_export_create(BlockExport *blk_exp, | ||
34 | args->allow_other = FUSE_EXPORT_ALLOW_OTHER_AUTO; | ||
35 | } | ||
36 | |||
37 | + exp->st_mode = S_IFREG | S_IRUSR; | ||
38 | + if (exp->writable) { | ||
39 | + exp->st_mode |= S_IWUSR; | ||
40 | + } | ||
41 | + exp->st_uid = getuid(); | ||
42 | + exp->st_gid = getgid(); | ||
43 | + | ||
44 | if (args->allow_other == FUSE_EXPORT_ALLOW_OTHER_AUTO) { | ||
45 | /* Ignore errors on our first attempt */ | ||
46 | ret = setup_fuse_export(exp, args->mountpoint, true, NULL); | ||
47 | @@ -XXX,XX +XXX,XX @@ static void fuse_getattr(fuse_req_t req, fuse_ino_t inode, | ||
48 | int64_t length, allocated_blocks; | ||
49 | time_t now = time(NULL); | ||
50 | FuseExport *exp = fuse_req_userdata(req); | ||
51 | - mode_t mode; | ||
52 | |||
53 | length = blk_getlength(exp->common.blk); | ||
54 | if (length < 0) { | ||
55 | @@ -XXX,XX +XXX,XX @@ static void fuse_getattr(fuse_req_t req, fuse_ino_t inode, | ||
56 | allocated_blocks = DIV_ROUND_UP(allocated_blocks, 512); | ||
57 | } | ||
58 | |||
59 | - mode = S_IFREG | S_IRUSR; | ||
60 | - if (exp->writable) { | ||
61 | - mode |= S_IWUSR; | ||
62 | - } | ||
63 | - | ||
64 | statbuf = (struct stat) { | ||
65 | .st_ino = inode, | ||
66 | - .st_mode = mode, | ||
67 | + .st_mode = exp->st_mode, | ||
68 | .st_nlink = 1, | ||
69 | - .st_uid = getuid(), | ||
70 | - .st_gid = getgid(), | ||
71 | + .st_uid = exp->st_uid, | ||
72 | + .st_gid = exp->st_gid, | ||
73 | .st_size = length, | ||
74 | .st_blksize = blk_bs(exp->common.blk)->bl.request_alignment, | ||
75 | .st_blocks = allocated_blocks, | ||
76 | @@ -XXX,XX +XXX,XX @@ static int fuse_do_truncate(const FuseExport *exp, int64_t size, | ||
77 | } | ||
78 | |||
79 | /** | ||
80 | - * Let clients set file attributes. Only resizing is supported. | ||
81 | + * Let clients set file attributes. Only resizing and changing | ||
82 | + * permissions (st_mode, st_uid, st_gid) is allowed. | ||
83 | + * Changing permissions is only allowed as far as it will actually | ||
84 | + * permit access: Read-only exports cannot be given +w, and exports | ||
85 | + * without allow_other cannot be given a different UID or GID, and | ||
86 | + * they cannot be given non-owner access. | ||
87 | */ | ||
88 | static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf, | ||
89 | int to_set, struct fuse_file_info *fi) | ||
90 | { | ||
91 | FuseExport *exp = fuse_req_userdata(req); | ||
92 | + int supported_attrs; | ||
93 | int ret; | ||
94 | |||
95 | - if (to_set & ~FUSE_SET_ATTR_SIZE) { | ||
96 | + supported_attrs = FUSE_SET_ATTR_SIZE | FUSE_SET_ATTR_MODE; | ||
97 | + if (exp->allow_other) { | ||
98 | + supported_attrs |= FUSE_SET_ATTR_UID | FUSE_SET_ATTR_GID; | ||
99 | + } | ||
100 | + | ||
101 | + if (to_set & ~supported_attrs) { | ||
102 | fuse_reply_err(req, ENOTSUP); | ||
103 | return; | ||
104 | } | ||
105 | |||
106 | + /* Do some argument checks first before committing to anything */ | ||
107 | + if (to_set & FUSE_SET_ATTR_MODE) { | ||
108 | + /* | ||
109 | + * Without allow_other, non-owners can never access the export, so do | ||
110 | + * not allow setting permissions for them | ||
111 | + */ | ||
112 | + if (!exp->allow_other && | ||
113 | + (statbuf->st_mode & (S_IRWXG | S_IRWXO)) != 0) | ||
114 | + { | ||
115 | + fuse_reply_err(req, EPERM); | ||
116 | + return; | ||
117 | + } | ||
118 | + | ||
119 | + /* +w for read-only exports makes no sense, disallow it */ | ||
120 | + if (!exp->writable && | ||
121 | + (statbuf->st_mode & (S_IWUSR | S_IWGRP | S_IWOTH)) != 0) | ||
122 | + { | ||
123 | + fuse_reply_err(req, EROFS); | ||
124 | + return; | ||
125 | + } | ||
126 | + } | ||
127 | + | ||
128 | if (to_set & FUSE_SET_ATTR_SIZE) { | ||
129 | if (!exp->writable) { | ||
130 | fuse_reply_err(req, EACCES); | ||
131 | @@ -XXX,XX +XXX,XX @@ static void fuse_setattr(fuse_req_t req, fuse_ino_t inode, struct stat *statbuf, | ||
132 | } | ||
133 | } | ||
134 | |||
135 | + if (to_set & FUSE_SET_ATTR_MODE) { | ||
136 | + /* Ignore FUSE-supplied file type, only change the mode */ | ||
137 | + exp->st_mode = (statbuf->st_mode & 07777) | S_IFREG; | ||
138 | + } | ||
139 | + | ||
140 | + if (to_set & FUSE_SET_ATTR_UID) { | ||
141 | + exp->st_uid = statbuf->st_uid; | ||
142 | + } | ||
143 | + | ||
144 | + if (to_set & FUSE_SET_ATTR_GID) { | ||
145 | + exp->st_gid = statbuf->st_gid; | ||
146 | + } | ||
147 | + | ||
148 | fuse_getattr(req, inode, fi); | ||
149 | } | ||
150 | |||
151 | -- | ||
152 | 2.31.1 | ||
153 | |||
154 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Max Reitz <mreitz@redhat.com> | ||
1 | 2 | ||
3 | Test that +w on read-only FUSE exports returns an EROFS error. u+x on | ||
4 | the other hand should work. (There is no special reason to choose u+x | ||
5 | here, it simply is like +w another flag that is not set by default.) | ||
6 | |||
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
8 | Message-Id: <20210625142317.271673-6-mreitz@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | tests/qemu-iotests/308 | 11 +++++++++++ | ||
12 | tests/qemu-iotests/308.out | 4 ++++ | ||
13 | 2 files changed, 15 insertions(+) | ||
14 | |||
15 | diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308 | ||
16 | index XXXXXXX..XXXXXXX 100755 | ||
17 | --- a/tests/qemu-iotests/308 | ||
18 | +++ b/tests/qemu-iotests/308 | ||
19 | @@ -XXX,XX +XXX,XX @@ fuse_export_add 'export-mp' "'mountpoint': '$EXT_MP'" | ||
20 | # Check that the export presents the same data as the original image | ||
21 | $QEMU_IMG compare -f raw -F $IMGFMT -U "$EXT_MP" "$TEST_IMG" | ||
22 | |||
23 | +# Some quick chmod tests | ||
24 | +stat -c 'Permissions pre-chmod: %a' "$EXT_MP" | ||
25 | + | ||
26 | +# Verify that we cannot set +w | ||
27 | +chmod u+w "$EXT_MP" 2>&1 | _filter_testdir | _filter_imgfmt | ||
28 | +stat -c 'Permissions post-+w: %a' "$EXT_MP" | ||
29 | + | ||
30 | +# But that we can set, say, +x (if we are so inclined) | ||
31 | +chmod u+x "$EXT_MP" 2>&1 | _filter_testdir | _filter_imgfmt | ||
32 | +stat -c 'Permissions post-+x: %a' "$EXT_MP" | ||
33 | + | ||
34 | echo | ||
35 | echo '=== Mount over existing file ===' | ||
36 | |||
37 | diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out | ||
38 | index XXXXXXX..XXXXXXX 100644 | ||
39 | --- a/tests/qemu-iotests/308.out | ||
40 | +++ b/tests/qemu-iotests/308.out | ||
41 | @@ -XXX,XX +XXX,XX @@ wrote 67108864/67108864 bytes at offset 0 | ||
42 | } } | ||
43 | {"return": {}} | ||
44 | Images are identical. | ||
45 | +Permissions pre-chmod: 400 | ||
46 | +chmod: changing permissions of 'TEST_DIR/t.IMGFMT.fuse': Read-only file system | ||
47 | +Permissions post-+w: 400 | ||
48 | +Permissions post-+x: 500 | ||
49 | |||
50 | === Mount over existing file === | ||
51 | {'execute': 'block-export-add', | ||
52 | -- | ||
53 | 2.31.1 | ||
54 | |||
55 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Max Reitz <mreitz@redhat.com> | |
2 | |||
3 | Signed-off-by: Max Reitz <mreitz@redhat.com> | ||
4 | Message-Id: <20210625142317.271673-7-mreitz@redhat.com> | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | --- | ||
7 | tests/qemu-iotests/tests/fuse-allow-other | 168 ++++++++++++++++++ | ||
8 | tests/qemu-iotests/tests/fuse-allow-other.out | 88 +++++++++ | ||
9 | 2 files changed, 256 insertions(+) | ||
10 | create mode 100755 tests/qemu-iotests/tests/fuse-allow-other | ||
11 | create mode 100644 tests/qemu-iotests/tests/fuse-allow-other.out | ||
12 | |||
13 | diff --git a/tests/qemu-iotests/tests/fuse-allow-other b/tests/qemu-iotests/tests/fuse-allow-other | ||
14 | new file mode 100755 | ||
15 | index XXXXXXX..XXXXXXX | ||
16 | --- /dev/null | ||
17 | +++ b/tests/qemu-iotests/tests/fuse-allow-other | ||
18 | @@ -XXX,XX +XXX,XX @@ | ||
19 | +#!/usr/bin/env bash | ||
20 | +# group: rw | ||
21 | +# | ||
22 | +# Test FUSE exports' allow-other option | ||
23 | +# | ||
24 | +# Copyright (C) 2021 Red Hat, Inc. | ||
25 | +# | ||
26 | +# This program is free software; you can redistribute it and/or modify | ||
27 | +# it under the terms of the GNU General Public License as published by | ||
28 | +# the Free Software Foundation; either version 2 of the License, or | ||
29 | +# (at your option) any later version. | ||
30 | +# | ||
31 | +# This program is distributed in the hope that it will be useful, | ||
32 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
33 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
34 | +# GNU General Public License for more details. | ||
35 | +# | ||
36 | +# You should have received a copy of the GNU General Public License | ||
37 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
38 | +# | ||
39 | + | ||
40 | +seq=$(basename "$0") | ||
41 | +echo "QA output created by $seq" | ||
42 | + | ||
43 | +status=1 # failure is the default! | ||
44 | + | ||
45 | +_cleanup() | ||
46 | +{ | ||
47 | + _cleanup_qemu | ||
48 | + _cleanup_test_img | ||
49 | + rm -f "$EXT_MP" | ||
50 | +} | ||
51 | +trap "_cleanup; exit \$status" 0 1 2 3 15 | ||
52 | + | ||
53 | +# get standard environment, filters and checks | ||
54 | +. ../common.rc | ||
55 | +. ../common.filter | ||
56 | +. ../common.qemu | ||
57 | + | ||
58 | +_supported_fmt generic | ||
59 | + | ||
60 | +_supported_proto file # We create the FUSE export manually | ||
61 | + | ||
62 | +sudo -n -u nobody true || \ | ||
63 | + _notrun 'Password-less sudo as nobody required to test allow_other' | ||
64 | + | ||
65 | +# $1: Export ID | ||
66 | +# $2: Options (beyond the node-name and ID) | ||
67 | +# $3: Expected return value (defaults to 'return') | ||
68 | +# $4: Node to export (defaults to 'node-format') | ||
69 | +fuse_export_add() | ||
70 | +{ | ||
71 | + allow_other_not_supported='option allow_other only allowed if' | ||
72 | + | ||
73 | + output=$( | ||
74 | + success_or_failure=yes _send_qemu_cmd $QEMU_HANDLE \ | ||
75 | + "{'execute': 'block-export-add', | ||
76 | + 'arguments': { | ||
77 | + 'type': 'fuse', | ||
78 | + 'id': '$1', | ||
79 | + 'node-name': '${4:-node-format}', | ||
80 | + $2 | ||
81 | + } }" \ | ||
82 | + "${3:-return}" \ | ||
83 | + "$allow_other_not_supported" \ | ||
84 | + | _filter_imgfmt | ||
85 | + ) | ||
86 | + | ||
87 | + if echo "$output" | grep -q "$allow_other_not_supported"; then | ||
88 | + # Shut down qemu gracefully so it can unmount the export | ||
89 | + _send_qemu_cmd $QEMU_HANDLE \ | ||
90 | + "{'execute': 'quit'}" \ | ||
91 | + 'return' | ||
92 | + | ||
93 | + wait=yes _cleanup_qemu | ||
94 | + | ||
95 | + _notrun "allow_other not supported" | ||
96 | + fi | ||
97 | + | ||
98 | + echo "$output" | ||
99 | +} | ||
100 | + | ||
101 | +EXT_MP="$TEST_DIR/fuse-export" | ||
102 | + | ||
103 | +_make_test_img 64k | ||
104 | +touch "$EXT_MP" | ||
105 | + | ||
106 | +echo | ||
107 | +echo '=== Test permissions ===' | ||
108 | + | ||
109 | +# $1: allow-other value ('on'/'off'/'auto') | ||
110 | +run_permission_test() | ||
111 | +{ | ||
112 | + _launch_qemu \ | ||
113 | + -blockdev \ | ||
114 | + "$IMGFMT,node-name=node-format,file.driver=file,file.filename=$TEST_IMG" | ||
115 | + | ||
116 | + _send_qemu_cmd $QEMU_HANDLE \ | ||
117 | + "{'execute': 'qmp_capabilities'}" \ | ||
118 | + 'return' | ||
119 | + | ||
120 | + fuse_export_add 'export' \ | ||
121 | + "'mountpoint': '$EXT_MP', | ||
122 | + 'allow-other': '$1'" | ||
123 | + | ||
124 | + # Should always work | ||
125 | + echo '(Removing all permissions)' | ||
126 | + chmod 000 "$EXT_MP" 2>&1 | _filter_testdir | _filter_imgfmt | ||
127 | + stat -c 'Permissions post-chmod: %a' "$EXT_MP" | ||
128 | + | ||
129 | + # Should always work | ||
130 | + echo '(Granting u+r)' | ||
131 | + chmod u+r "$EXT_MP" 2>&1 | _filter_testdir | _filter_imgfmt | ||
132 | + stat -c 'Permissions post-chmod: %a' "$EXT_MP" | ||
133 | + | ||
134 | + # Should only work with allow-other: Otherwise, no permissions can be | ||
135 | + # granted to the group or others | ||
136 | + echo '(Granting read permissions for everyone)' | ||
137 | + chmod 444 "$EXT_MP" 2>&1 | _filter_testdir | _filter_imgfmt | ||
138 | + stat -c 'Permissions post-chmod: %a' "$EXT_MP" | ||
139 | + | ||
140 | + echo 'Doing operations as nobody:' | ||
141 | + # Change to TEST_DIR, so nobody will not have to attempt a lookup | ||
142 | + pushd "$TEST_DIR" >/dev/null | ||
143 | + | ||
144 | + # This is already prevented by the permissions (without allow-other, FUSE | ||
145 | + # exports always have o-r), but test it anyway | ||
146 | + sudo -n -u nobody cat fuse-export >/dev/null | ||
147 | + | ||
148 | + # If the only problem were the lack of permissions, we should still be able | ||
149 | + # to stat the export as nobody; it should not work without allow-other, | ||
150 | + # though | ||
151 | + sudo -n -u nobody \ | ||
152 | + stat -c 'Permissions seen by nobody: %a' fuse-export 2>&1 \ | ||
153 | + | _filter_imgfmt | ||
154 | + | ||
155 | + # To prove the point, revoke read permissions for others and try again | ||
156 | + chmod o-r fuse-export 2>&1 | _filter_testdir | _filter_imgfmt | ||
157 | + | ||
158 | + # Should fail | ||
159 | + sudo -n -u nobody cat fuse-export >/dev/null | ||
160 | + # Should work with allow_other | ||
161 | + sudo -n -u nobody \ | ||
162 | + stat -c 'Permissions seen by nobody: %a' fuse-export 2>&1 \ | ||
163 | + | _filter_imgfmt | ||
164 | + | ||
165 | + popd >/dev/null | ||
166 | + | ||
167 | + _send_qemu_cmd $QEMU_HANDLE \ | ||
168 | + "{'execute': 'quit'}" \ | ||
169 | + 'return' | ||
170 | + | ||
171 | + wait=yes _cleanup_qemu | ||
172 | +} | ||
173 | + | ||
174 | +# 'auto' should behave exactly like 'on', because 'on' tests that | ||
175 | +# allow_other works (otherwise, this test is skipped) | ||
176 | +for ao in off on auto; do | ||
177 | + echo | ||
178 | + echo "--- allow-other=$ao ---" | ||
179 | + | ||
180 | + run_permission_test "$ao" | ||
181 | +done | ||
182 | + | ||
183 | +# success, all done | ||
184 | +echo "*** done" | ||
185 | +rm -f $seq.full | ||
186 | +status=0 | ||
187 | diff --git a/tests/qemu-iotests/tests/fuse-allow-other.out b/tests/qemu-iotests/tests/fuse-allow-other.out | ||
188 | new file mode 100644 | ||
189 | index XXXXXXX..XXXXXXX | ||
190 | --- /dev/null | ||
191 | +++ b/tests/qemu-iotests/tests/fuse-allow-other.out | ||
192 | @@ -XXX,XX +XXX,XX @@ | ||
193 | +QA output created by fuse-allow-other | ||
194 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536 | ||
195 | + | ||
196 | +=== Test permissions === | ||
197 | + | ||
198 | +--- allow-other=off --- | ||
199 | +{'execute': 'qmp_capabilities'} | ||
200 | +{"return": {}} | ||
201 | +{'execute': 'block-export-add', | ||
202 | + 'arguments': { | ||
203 | + 'type': 'fuse', | ||
204 | + 'id': 'export', | ||
205 | + 'node-name': 'node-format', | ||
206 | + 'mountpoint': 'TEST_DIR/fuse-export', | ||
207 | + 'allow-other': 'off' | ||
208 | + } } | ||
209 | +{"return": {}} | ||
210 | +(Removing all permissions) | ||
211 | +Permissions post-chmod: 0 | ||
212 | +(Granting u+r) | ||
213 | +Permissions post-chmod: 400 | ||
214 | +(Granting read permissions for everyone) | ||
215 | +chmod: changing permissions of 'TEST_DIR/fuse-export': Operation not permitted | ||
216 | +Permissions post-chmod: 400 | ||
217 | +Doing operations as nobody: | ||
218 | +cat: fuse-export: Permission denied | ||
219 | +stat: cannot statx 'fuse-export': Permission denied | ||
220 | +cat: fuse-export: Permission denied | ||
221 | +stat: cannot statx 'fuse-export': Permission denied | ||
222 | +{'execute': 'quit'} | ||
223 | +{"return": {}} | ||
224 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} | ||
225 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export"}} | ||
226 | + | ||
227 | +--- allow-other=on --- | ||
228 | +{'execute': 'qmp_capabilities'} | ||
229 | +{"return": {}} | ||
230 | +{'execute': 'block-export-add', | ||
231 | + 'arguments': { | ||
232 | + 'type': 'fuse', | ||
233 | + 'id': 'export', | ||
234 | + 'node-name': 'node-format', | ||
235 | + 'mountpoint': 'TEST_DIR/fuse-export', | ||
236 | + 'allow-other': 'on' | ||
237 | + } } | ||
238 | +{"return": {}} | ||
239 | +(Removing all permissions) | ||
240 | +Permissions post-chmod: 0 | ||
241 | +(Granting u+r) | ||
242 | +Permissions post-chmod: 400 | ||
243 | +(Granting read permissions for everyone) | ||
244 | +Permissions post-chmod: 444 | ||
245 | +Doing operations as nobody: | ||
246 | +Permissions seen by nobody: 444 | ||
247 | +cat: fuse-export: Permission denied | ||
248 | +Permissions seen by nobody: 440 | ||
249 | +{'execute': 'quit'} | ||
250 | +{"return": {}} | ||
251 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} | ||
252 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export"}} | ||
253 | + | ||
254 | +--- allow-other=auto --- | ||
255 | +{'execute': 'qmp_capabilities'} | ||
256 | +{"return": {}} | ||
257 | +{'execute': 'block-export-add', | ||
258 | + 'arguments': { | ||
259 | + 'type': 'fuse', | ||
260 | + 'id': 'export', | ||
261 | + 'node-name': 'node-format', | ||
262 | + 'mountpoint': 'TEST_DIR/fuse-export', | ||
263 | + 'allow-other': 'auto' | ||
264 | + } } | ||
265 | +{"return": {}} | ||
266 | +(Removing all permissions) | ||
267 | +Permissions post-chmod: 0 | ||
268 | +(Granting u+r) | ||
269 | +Permissions post-chmod: 400 | ||
270 | +(Granting read permissions for everyone) | ||
271 | +Permissions post-chmod: 444 | ||
272 | +Doing operations as nobody: | ||
273 | +Permissions seen by nobody: 444 | ||
274 | +cat: fuse-export: Permission denied | ||
275 | +Permissions seen by nobody: 440 | ||
276 | +{'execute': 'quit'} | ||
277 | +{"return": {}} | ||
278 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}} | ||
279 | +{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export"}} | ||
280 | +*** done | ||
281 | -- | ||
282 | 2.31.1 | ||
283 | |||
284 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | task->complete is a bool not an integer. | ||
4 | |||
5 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
6 | Message-Id: <20210707180449.32665-1-pl@kamp.de> | ||
7 | Reviewed-by: Ilya Dryomov <idryomov@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/rbd.c | 2 +- | ||
11 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
12 | |||
13 | diff --git a/block/rbd.c b/block/rbd.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/rbd.c | ||
16 | +++ b/block/rbd.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static int qemu_rbd_resize(BlockDriverState *bs, uint64_t size) | ||
18 | static void qemu_rbd_finish_bh(void *opaque) | ||
19 | { | ||
20 | RBDTask *task = opaque; | ||
21 | - task->complete = 1; | ||
22 | + task->complete = true; | ||
23 | aio_co_wake(task->co); | ||
24 | } | ||
25 | |||
26 | -- | ||
27 | 2.31.1 | ||
28 | |||
29 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Peter Lieven <pl@kamp.de> | ||
1 | 2 | ||
3 | adding myself as a designated reviewer. | ||
4 | |||
5 | Signed-off-by: Peter Lieven <pl@kamp.de> | ||
6 | Message-Id: <20210707180449.32665-2-pl@kamp.de> | ||
7 | Acked-by: Ilya Dryomov <idryomov@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | MAINTAINERS | 1 + | ||
11 | 1 file changed, 1 insertion(+) | ||
12 | |||
13 | diff --git a/MAINTAINERS b/MAINTAINERS | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/MAINTAINERS | ||
16 | +++ b/MAINTAINERS | ||
17 | @@ -XXX,XX +XXX,XX @@ F: block/vmdk.c | ||
18 | |||
19 | RBD | ||
20 | M: Ilya Dryomov <idryomov@gmail.com> | ||
21 | +R: Peter Lieven <pl@kamp.de> | ||
22 | L: qemu-block@nongnu.org | ||
23 | S: Supported | ||
24 | F: block/rbd.c | ||
25 | -- | ||
26 | 2.31.1 | ||
27 | |||
28 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | dev->max_queues was never initialised for backends that don't support | ||
2 | VHOST_USER_PROTOCOL_F_MQ, so it would use 0 as the maximum number of | ||
3 | queues to check against and consequently fail for any such backend. | ||
1 | 4 | ||
5 | Set it to 1 if the backend doesn't have multiqueue support. | ||
6 | |||
7 | Fixes: c90bd505a3e8210c23d69fecab9ee6f56ec4a161 | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | Message-Id: <20210705171429.29286-1-kwolf@redhat.com> | ||
10 | Reviewed-by: Cornelia Huck <cohuck@redhat.com> | ||
11 | Reviewed-by: Raphael Norwitz <raphael.norwitz@nutanix.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | hw/virtio/vhost-user.c | 3 +++ | ||
15 | 1 file changed, 3 insertions(+) | ||
16 | |||
17 | diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/hw/virtio/vhost-user.c | ||
20 | +++ b/hw/virtio/vhost-user.c | ||
21 | @@ -XXX,XX +XXX,XX @@ static int vhost_user_backend_init(struct vhost_dev *dev, void *opaque, | ||
22 | if (err < 0) { | ||
23 | return -EPROTO; | ||
24 | } | ||
25 | + } else { | ||
26 | + dev->max_queues = 1; | ||
27 | } | ||
28 | + | ||
29 | if (dev->num_queues && dev->max_queues < dev->num_queues) { | ||
30 | error_setg(errp, "The maximum number of queues supported by the " | ||
31 | "backend is %" PRIu64, dev->max_queues); | ||
32 | -- | ||
33 | 2.31.1 | ||
34 | |||
35 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
1 | 2 | ||
3 | drive_backup_prepare() does bdrv_drained_begin() in hope that | ||
4 | bdrv_drained_end() will be called in drive_backup_clean(). Still we | ||
5 | need to set state->bs for this to work. That's done too late: a lot of | ||
6 | failure paths in drive_backup_prepare() miss setting state->bs. Fix | ||
7 | that. | ||
8 | |||
9 | Fixes: 2288ccfac96281c316db942d10e3f921c1373064 | ||
10 | Fixes: https://gitlab.com/qemu-project/qemu/-/issues/399 | ||
11 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
12 | Message-Id: <20210608171852.250775-1-vsementsov@virtuozzo.com> | ||
13 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
15 | --- | ||
16 | blockdev.c | 3 +-- | ||
17 | 1 file changed, 1 insertion(+), 2 deletions(-) | ||
18 | |||
19 | diff --git a/blockdev.c b/blockdev.c | ||
20 | index XXXXXXX..XXXXXXX 100644 | ||
21 | --- a/blockdev.c | ||
22 | +++ b/blockdev.c | ||
23 | @@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp) | ||
24 | aio_context = bdrv_get_aio_context(bs); | ||
25 | aio_context_acquire(aio_context); | ||
26 | |||
27 | + state->bs = bs; | ||
28 | /* Paired with .clean() */ | ||
29 | bdrv_drained_begin(bs); | ||
30 | |||
31 | @@ -XXX,XX +XXX,XX @@ static void drive_backup_prepare(BlkActionState *common, Error **errp) | ||
32 | } | ||
33 | } | ||
34 | |||
35 | - state->bs = bs; | ||
36 | - | ||
37 | state->job = do_backup_common(qapi_DriveBackup_base(backup), | ||
38 | bs, target_bs, aio_context, | ||
39 | common->block_job_txn, errp); | ||
40 | -- | ||
41 | 2.31.1 | ||
42 | |||
43 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Eric Blake <eblake@redhat.com> | ||
1 | 2 | ||
3 | This was deprecated back in bc5ee6da7 (qcow2: Deprecate use of | ||
4 | qemu-img amend to change backing file), and no one in the meantime has | ||
5 | given any reasons why it should be supported. Time to make change | ||
6 | attempts a hard error (but for convenience, specifying the _same_ | ||
7 | backing chain is not forbidden). Update a couple of iotests to match. | ||
8 | |||
9 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
10 | Message-Id: <20210503213600.569128-2-eblake@redhat.com> | ||
11 | Reviewed-by: Connor Kuehl <ckuehl@redhat.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | docs/system/deprecated.rst | 12 ------------ | ||
15 | docs/system/removed-features.rst | 12 ++++++++++++ | ||
16 | block/qcow2.c | 13 ++++--------- | ||
17 | tests/qemu-iotests/061 | 3 +++ | ||
18 | tests/qemu-iotests/061.out | 3 ++- | ||
19 | tests/qemu-iotests/082.out | 6 ++++-- | ||
20 | 6 files changed, 25 insertions(+), 24 deletions(-) | ||
21 | |||
22 | diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/docs/system/deprecated.rst | ||
25 | +++ b/docs/system/deprecated.rst | ||
26 | @@ -XXX,XX +XXX,XX @@ this CPU is also deprecated. | ||
27 | Related binaries | ||
28 | ---------------- | ||
29 | |||
30 | -qemu-img amend to adjust backing file (since 5.1) | ||
31 | -''''''''''''''''''''''''''''''''''''''''''''''''' | ||
32 | - | ||
33 | -The use of ``qemu-img amend`` to modify the name or format of a qcow2 | ||
34 | -backing image is deprecated; this functionality was never fully | ||
35 | -documented or tested, and interferes with other amend operations that | ||
36 | -need access to the original backing image (such as deciding whether a | ||
37 | -v3 zero cluster may be left unallocated when converting to a v2 | ||
38 | -image). Rather, any changes to the backing chain should be performed | ||
39 | -with ``qemu-img rebase -u`` either before or after the remaining | ||
40 | -changes being performed by amend, as appropriate. | ||
41 | - | ||
42 | qemu-img backing file without format (since 5.1) | ||
43 | '''''''''''''''''''''''''''''''''''''''''''''''' | ||
44 | |||
45 | diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst | ||
46 | index XXXXXXX..XXXXXXX 100644 | ||
47 | --- a/docs/system/removed-features.rst | ||
48 | +++ b/docs/system/removed-features.rst | ||
49 | @@ -XXX,XX +XXX,XX @@ topologies described with -smp include all possible cpus, i.e. | ||
50 | The ``enforce-config-section`` property was replaced by the | ||
51 | ``-global migration.send-configuration={on|off}`` option. | ||
52 | |||
53 | +qemu-img amend to adjust backing file (removed in 6.1) | ||
54 | +'''''''''''''''''''''''''''''''''''''''''''''''''''''' | ||
55 | + | ||
56 | +The use of ``qemu-img amend`` to modify the name or format of a qcow2 | ||
57 | +backing image was never fully documented or tested, and interferes | ||
58 | +with other amend operations that need access to the original backing | ||
59 | +image (such as deciding whether a v3 zero cluster may be left | ||
60 | +unallocated when converting to a v2 image). Any changes to the | ||
61 | +backing chain should be performed with ``qemu-img rebase -u`` either | ||
62 | +before or after the remaining changes being performed by amend, as | ||
63 | +appropriate. | ||
64 | + | ||
65 | Block devices | ||
66 | ------------- | ||
67 | |||
68 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
69 | index XXXXXXX..XXXXXXX 100644 | ||
70 | --- a/block/qcow2.c | ||
71 | +++ b/block/qcow2.c | ||
72 | @@ -XXX,XX +XXX,XX @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, | ||
73 | if (backing_file || backing_format) { | ||
74 | if (g_strcmp0(backing_file, s->image_backing_file) || | ||
75 | g_strcmp0(backing_format, s->image_backing_format)) { | ||
76 | - warn_report("Deprecated use of amend to alter the backing file; " | ||
77 | - "use qemu-img rebase instead"); | ||
78 | - } | ||
79 | - ret = qcow2_change_backing_file(bs, | ||
80 | - backing_file ?: s->image_backing_file, | ||
81 | - backing_format ?: s->image_backing_format); | ||
82 | - if (ret < 0) { | ||
83 | - error_setg_errno(errp, -ret, "Failed to change the backing file"); | ||
84 | - return ret; | ||
85 | + error_setg(errp, "Cannot amend the backing file"); | ||
86 | + error_append_hint(errp, | ||
87 | + "You can use 'qemu-img rebase' instead.\n"); | ||
88 | + return -EINVAL; | ||
89 | } | ||
90 | } | ||
91 | |||
92 | diff --git a/tests/qemu-iotests/061 b/tests/qemu-iotests/061 | ||
93 | index XXXXXXX..XXXXXXX 100755 | ||
94 | --- a/tests/qemu-iotests/061 | ||
95 | +++ b/tests/qemu-iotests/061 | ||
96 | @@ -XXX,XX +XXX,XX @@ _make_test_img -o "compat=1.1" 64M | ||
97 | TEST_IMG="$TEST_IMG.base" _make_test_img -o "compat=1.1" 64M | ||
98 | $QEMU_IO -c "write -P 0x2a 0 128k" "$TEST_IMG.base" | _filter_qemu_io | ||
99 | $QEMU_IO -c "read -P 0 0 128k" "$TEST_IMG" | _filter_qemu_io | ||
100 | +$QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" \ | ||
101 | + "$TEST_IMG" && echo "unexpected pass" | ||
102 | +$QEMU_IMG rebase -u -b "$TEST_IMG.base" -F qcow2 "$TEST_IMG" | ||
103 | $QEMU_IMG amend -o "backing_file=$TEST_IMG.base,backing_fmt=qcow2" "$TEST_IMG" | ||
104 | $QEMU_IO -c "read -P 0x2a 0 128k" "$TEST_IMG" | _filter_qemu_io | ||
105 | _check_test_img | ||
106 | diff --git a/tests/qemu-iotests/061.out b/tests/qemu-iotests/061.out | ||
107 | index XXXXXXX..XXXXXXX 100644 | ||
108 | --- a/tests/qemu-iotests/061.out | ||
109 | +++ b/tests/qemu-iotests/061.out | ||
110 | @@ -XXX,XX +XXX,XX @@ wrote 131072/131072 bytes at offset 0 | ||
111 | 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
112 | read 131072/131072 bytes at offset 0 | ||
113 | 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
114 | -qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead | ||
115 | +qemu-img: Cannot amend the backing file | ||
116 | +You can use 'qemu-img rebase' instead. | ||
117 | read 131072/131072 bytes at offset 0 | ||
118 | 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
119 | No errors were found on the image. | ||
120 | diff --git a/tests/qemu-iotests/082.out b/tests/qemu-iotests/082.out | ||
121 | index XXXXXXX..XXXXXXX 100644 | ||
122 | --- a/tests/qemu-iotests/082.out | ||
123 | +++ b/tests/qemu-iotests/082.out | ||
124 | @@ -XXX,XX +XXX,XX @@ Amend options for 'qcow2': | ||
125 | size=<size> - Virtual disk size | ||
126 | |||
127 | Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,help TEST_DIR/t.qcow2 | ||
128 | -qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead | ||
129 | +qemu-img: Cannot amend the backing file | ||
130 | +You can use 'qemu-img rebase' instead. | ||
131 | |||
132 | Testing: rebase -u -b -f qcow2 TEST_DIR/t.qcow2 | ||
133 | |||
134 | Testing: amend -f qcow2 -o backing_file=TEST_DIR/t.qcow2,,? TEST_DIR/t.qcow2 | ||
135 | -qemu-img: warning: Deprecated use of amend to alter the backing file; use qemu-img rebase instead | ||
136 | +qemu-img: Cannot amend the backing file | ||
137 | +You can use 'qemu-img rebase' instead. | ||
138 | |||
139 | Testing: rebase -u -b -f qcow2 TEST_DIR/t.qcow2 | ||
140 | |||
141 | -- | ||
142 | 2.31.1 | ||
143 | |||
144 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Eric Blake <eblake@redhat.com> | |
2 | |||
3 | Back in commit d9f059aa6c (qemu-img: Deprecate use of -b without -F), | ||
4 | we deprecated the ability to create a file with a backing image that | ||
5 | requires qemu to perform format probing. Qemu can still probe older | ||
6 | files for backwards compatibility, but it is time to finish off the | ||
7 | ability to create such images, due to the potential security risk they | ||
8 | present. Update a couple of iotests affected by the change. | ||
9 | |||
10 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
11 | Message-Id: <20210503213600.569128-3-eblake@redhat.com> | ||
12 | Reviewed-by: Connor Kuehl <ckuehl@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | docs/system/deprecated.rst | 20 ----------------- | ||
16 | docs/system/removed-features.rst | 19 ++++++++++++++++ | ||
17 | block.c | 37 ++++++++++---------------------- | ||
18 | qemu-img.c | 6 ++++-- | ||
19 | tests/qemu-iotests/040 | 4 ++-- | ||
20 | tests/qemu-iotests/041 | 6 ++++-- | ||
21 | tests/qemu-iotests/114 | 18 ++++++++-------- | ||
22 | tests/qemu-iotests/114.out | 11 ++++------ | ||
23 | tests/qemu-iotests/301 | 4 +--- | ||
24 | tests/qemu-iotests/301.out | 16 ++------------ | ||
25 | 10 files changed, 56 insertions(+), 85 deletions(-) | ||
26 | |||
27 | diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst | ||
28 | index XXXXXXX..XXXXXXX 100644 | ||
29 | --- a/docs/system/deprecated.rst | ||
30 | +++ b/docs/system/deprecated.rst | ||
31 | @@ -XXX,XX +XXX,XX @@ this CPU is also deprecated. | ||
32 | Related binaries | ||
33 | ---------------- | ||
34 | |||
35 | -qemu-img backing file without format (since 5.1) | ||
36 | -'''''''''''''''''''''''''''''''''''''''''''''''' | ||
37 | - | ||
38 | -The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img | ||
39 | -convert`` to create or modify an image that depends on a backing file | ||
40 | -now recommends that an explicit backing format be provided. This is | ||
41 | -for safety: if QEMU probes a different format than what you thought, | ||
42 | -the data presented to the guest will be corrupt; similarly, presenting | ||
43 | -a raw image to a guest allows a potential security exploit if a future | ||
44 | -probe sees a non-raw image based on guest writes. | ||
45 | - | ||
46 | -To avoid the warning message, or even future refusal to create an | ||
47 | -unsafe image, you must pass ``-o backing_fmt=`` (or the shorthand | ||
48 | -``-F`` during create) to specify the intended backing format. You may | ||
49 | -use ``qemu-img rebase -u`` to retroactively add a backing format to an | ||
50 | -existing image. However, be aware that there are already potential | ||
51 | -security risks to blindly using ``qemu-img info`` to probe the format | ||
52 | -of an untrusted backing image, when deciding what format to add into | ||
53 | -an existing image. | ||
54 | - | ||
55 | Backwards compatibility | ||
56 | ----------------------- | ||
57 | |||
58 | diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst | ||
59 | index XXXXXXX..XXXXXXX 100644 | ||
60 | --- a/docs/system/removed-features.rst | ||
61 | +++ b/docs/system/removed-features.rst | ||
62 | @@ -XXX,XX +XXX,XX @@ backing chain should be performed with ``qemu-img rebase -u`` either | ||
63 | before or after the remaining changes being performed by amend, as | ||
64 | appropriate. | ||
65 | |||
66 | +qemu-img backing file without format (removed in 6.1) | ||
67 | +''''''''''''''''''''''''''''''''''''''''''''''''''''' | ||
68 | + | ||
69 | +The use of ``qemu-img create``, ``qemu-img rebase``, or ``qemu-img | ||
70 | +convert`` to create or modify an image that depends on a backing file | ||
71 | +now requires that an explicit backing format be provided. This is | ||
72 | +for safety: if QEMU probes a different format than what you thought, | ||
73 | +the data presented to the guest will be corrupt; similarly, presenting | ||
74 | +a raw image to a guest allows a potential security exploit if a future | ||
75 | +probe sees a non-raw image based on guest writes. | ||
76 | + | ||
77 | +To avoid creating unsafe backing chains, you must pass ``-o | ||
78 | +backing_fmt=`` (or the shorthand ``-F`` during create) to specify the | ||
79 | +intended backing format. You may use ``qemu-img rebase -u`` to | ||
80 | +retroactively add a backing format to an existing image. However, be | ||
81 | +aware that there are already potential security risks to blindly using | ||
82 | +``qemu-img info`` to probe the format of an untrusted backing image, | ||
83 | +when deciding what format to add into an existing image. | ||
84 | + | ||
85 | Block devices | ||
86 | ------------- | ||
87 | |||
88 | diff --git a/block.c b/block.c | ||
89 | index XXXXXXX..XXXXXXX 100644 | ||
90 | --- a/block.c | ||
91 | +++ b/block.c | ||
92 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_check(BlockDriverState *bs, | ||
93 | * -ENOTSUP - format driver doesn't support changing the backing file | ||
94 | */ | ||
95 | int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, | ||
96 | - const char *backing_fmt, bool warn) | ||
97 | + const char *backing_fmt, bool require) | ||
98 | { | ||
99 | BlockDriver *drv = bs->drv; | ||
100 | int ret; | ||
101 | @@ -XXX,XX +XXX,XX @@ int bdrv_change_backing_file(BlockDriverState *bs, const char *backing_file, | ||
102 | return -EINVAL; | ||
103 | } | ||
104 | |||
105 | - if (warn && backing_file && !backing_fmt) { | ||
106 | - warn_report("Deprecated use of backing file without explicit " | ||
107 | - "backing format, use of this image requires " | ||
108 | - "potentially unsafe format probing"); | ||
109 | + if (require && backing_file && !backing_fmt) { | ||
110 | + return -EINVAL; | ||
111 | } | ||
112 | |||
113 | if (drv->bdrv_change_backing_file != NULL) { | ||
114 | @@ -XXX,XX +XXX,XX @@ void bdrv_img_create(const char *filename, const char *fmt, | ||
115 | goto out; | ||
116 | } else { | ||
117 | if (!backing_fmt) { | ||
118 | - warn_report("Deprecated use of backing file without explicit " | ||
119 | - "backing format (detected format of %s)", | ||
120 | - bs->drv->format_name); | ||
121 | - if (bs->drv != &bdrv_raw) { | ||
122 | - /* | ||
123 | - * A probe of raw deserves the most attention: | ||
124 | - * leaving the backing format out of the image | ||
125 | - * will ensure bs->probed is set (ensuring we | ||
126 | - * don't accidentally commit into the backing | ||
127 | - * file), and allow more spots to warn the users | ||
128 | - * to fix their toolchain when opening this image | ||
129 | - * later. For other images, we can safely record | ||
130 | - * the format that we probed. | ||
131 | - */ | ||
132 | - backing_fmt = bs->drv->format_name; | ||
133 | - qemu_opt_set(opts, BLOCK_OPT_BACKING_FMT, backing_fmt, | ||
134 | - NULL); | ||
135 | - } | ||
136 | + error_setg(&local_err, | ||
137 | + "Backing file specified without backing format"); | ||
138 | + error_append_hint(&local_err, "Detected format of %s.", | ||
139 | + bs->drv->format_name); | ||
140 | + goto out; | ||
141 | } | ||
142 | if (size == -1) { | ||
143 | /* Opened BS, have no size */ | ||
144 | @@ -XXX,XX +XXX,XX @@ void bdrv_img_create(const char *filename, const char *fmt, | ||
145 | } | ||
146 | /* (backing_file && !(flags & BDRV_O_NO_BACKING)) */ | ||
147 | } else if (backing_file && !backing_fmt) { | ||
148 | - warn_report("Deprecated use of unopened backing file without " | ||
149 | - "explicit backing format, use of this image requires " | ||
150 | - "potentially unsafe format probing"); | ||
151 | + error_setg(&local_err, | ||
152 | + "Backing file specified without backing format"); | ||
153 | + goto out; | ||
154 | } | ||
155 | |||
156 | if (size == -1) { | ||
157 | diff --git a/qemu-img.c b/qemu-img.c | ||
158 | index XXXXXXX..XXXXXXX 100644 | ||
159 | --- a/qemu-img.c | ||
160 | +++ b/qemu-img.c | ||
161 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | ||
162 | |||
163 | if (out_baseimg_param) { | ||
164 | if (!qemu_opt_get(opts, BLOCK_OPT_BACKING_FMT)) { | ||
165 | - warn_report("Deprecated use of backing file without explicit " | ||
166 | - "backing format"); | ||
167 | + error_report("Use of backing file requires explicit " | ||
168 | + "backing format"); | ||
169 | + ret = -1; | ||
170 | + goto out; | ||
171 | } | ||
172 | } | ||
173 | |||
174 | diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 | ||
175 | index XXXXXXX..XXXXXXX 100755 | ||
176 | --- a/tests/qemu-iotests/040 | ||
177 | +++ b/tests/qemu-iotests/040 | ||
178 | @@ -XXX,XX +XXX,XX @@ class TestCommitWithOverriddenBacking(iotests.QMPTestCase): | ||
179 | def setUp(self): | ||
180 | qemu_img('create', '-f', iotests.imgfmt, self.img_base_a, '1M') | ||
181 | qemu_img('create', '-f', iotests.imgfmt, self.img_base_b, '1M') | ||
182 | - qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, \ | ||
183 | - self.img_top) | ||
184 | + qemu_img('create', '-f', iotests.imgfmt, '-b', self.img_base_a, | ||
185 | + '-F', iotests.imgfmt, self.img_top) | ||
186 | |||
187 | self.vm = iotests.VM() | ||
188 | self.vm.launch() | ||
189 | diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 | ||
190 | index XXXXXXX..XXXXXXX 100755 | ||
191 | --- a/tests/qemu-iotests/041 | ||
192 | +++ b/tests/qemu-iotests/041 | ||
193 | @@ -XXX,XX +XXX,XX @@ class TestReplaces(iotests.QMPTestCase): | ||
194 | class TestFilters(iotests.QMPTestCase): | ||
195 | def setUp(self): | ||
196 | qemu_img('create', '-f', iotests.imgfmt, backing_img, '1M') | ||
197 | - qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, test_img) | ||
198 | - qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, target_img) | ||
199 | + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, | ||
200 | + '-F', iotests.imgfmt, test_img) | ||
201 | + qemu_img('create', '-f', iotests.imgfmt, '-b', backing_img, | ||
202 | + '-F', iotests.imgfmt, target_img) | ||
203 | |||
204 | qemu_io('-c', 'write -P 1 0 512k', backing_img) | ||
205 | qemu_io('-c', 'write -P 2 512k 512k', test_img) | ||
206 | diff --git a/tests/qemu-iotests/114 b/tests/qemu-iotests/114 | ||
207 | index XXXXXXX..XXXXXXX 100755 | ||
208 | --- a/tests/qemu-iotests/114 | ||
209 | +++ b/tests/qemu-iotests/114 | ||
210 | @@ -XXX,XX +XXX,XX @@ _supported_os Linux | ||
211 | # qcow2.py does not work too well with external data files | ||
212 | _unsupported_imgopts data_file | ||
213 | |||
214 | -# Intentionally specify backing file without backing format; demonstrate | ||
215 | -# the difference in warning messages when backing file could be probed. | ||
216 | -# Note that only a non-raw probe result will affect the resulting image. | ||
217 | +# Older qemu-img could set up backing file without backing format; modern | ||
218 | +# qemu can't but we can use qcow2.py to simulate older files. | ||
219 | truncate -s $((64 * 1024 * 1024)) "$TEST_IMG.orig" | ||
220 | -_make_test_img -b "$TEST_IMG.orig" 64M | ||
221 | +_make_test_img -b "$TEST_IMG.orig" -F raw 64M | ||
222 | +$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0xE2792ACA | ||
223 | |||
224 | TEST_IMG="$TEST_IMG.base" _make_test_img 64M | ||
225 | $QEMU_IMG convert -O qcow2 -B "$TEST_IMG.orig" "$TEST_IMG.orig" "$TEST_IMG" | ||
226 | -_make_test_img -b "$TEST_IMG.base" 64M | ||
227 | -_make_test_img -u -b "$TEST_IMG.base" 64M | ||
228 | +_make_test_img -b "$TEST_IMG.base" -F $IMGFMT 64M | ||
229 | +_make_test_img -u -b "$TEST_IMG.base" -F $IMGFMT 64M | ||
230 | |||
231 | # Set an invalid backing file format | ||
232 | $PYTHON qcow2.py "$TEST_IMG" add-header-ext 0xE2792ACA "foo" | ||
233 | @@ -XXX,XX +XXX,XX @@ _img_info | ||
234 | $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_testdir | ||
235 | $QEMU_IO -c "open -o backing.driver=$IMGFMT $TEST_IMG" -c "read 0 4k" | _filter_qemu_io | ||
236 | |||
237 | -# Rebase the image, to show that omitting backing format triggers a warning, | ||
238 | -# but probing now lets us use the backing file. | ||
239 | -$QEMU_IMG rebase -u -b "$TEST_IMG.base" "$TEST_IMG" | ||
240 | +# Rebase the image, to show that backing format is required. | ||
241 | +($QEMU_IMG rebase -u -b "$TEST_IMG.base" "$TEST_IMG" 2>&1 && echo "unexpected pass") | _filter_testdir | ||
242 | +$QEMU_IMG rebase -u -b "$TEST_IMG.base" -F $IMGFMT "$TEST_IMG" | ||
243 | $QEMU_IO -c "open $TEST_IMG" -c "read 0 4k" 2>&1 | _filter_qemu_io | _filter_testdir | ||
244 | |||
245 | # success, all done | ||
246 | diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out | ||
247 | index XXXXXXX..XXXXXXX 100644 | ||
248 | --- a/tests/qemu-iotests/114.out | ||
249 | +++ b/tests/qemu-iotests/114.out | ||
250 | @@ -XXX,XX +XXX,XX @@ | ||
251 | QA output created by 114 | ||
252 | -qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) | ||
253 | -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig | ||
254 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.orig backing_fmt=raw | ||
255 | Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 | ||
256 | -qemu-img: warning: Deprecated use of backing file without explicit backing format | ||
257 | -qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of IMGFMT) | ||
258 | +qemu-img: Use of backing file requires explicit backing format | ||
259 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT | ||
260 | Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT | ||
261 | -qemu-img: warning: Deprecated use of unopened backing file without explicit backing format, use of this image requires potentially unsafe format probing | ||
262 | -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file=TEST_DIR/t.IMGFMT.base | ||
263 | image: TEST_DIR/t.IMGFMT | ||
264 | file format: IMGFMT | ||
265 | virtual size: 64 MiB (67108864 bytes) | ||
266 | @@ -XXX,XX +XXX,XX @@ qemu-io: can't open device TEST_DIR/t.qcow2: Could not open backing file: Unknow | ||
267 | no file open, try 'help open' | ||
268 | read 4096/4096 bytes at offset 0 | ||
269 | 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
270 | -qemu-img: warning: Deprecated use of backing file without explicit backing format, use of this image requires potentially unsafe format probing | ||
271 | +qemu-img: Could not change the backing file to 'TEST_DIR/t.qcow2.base': Invalid argument | ||
272 | read 4096/4096 bytes at offset 0 | ||
273 | 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
274 | *** done | ||
275 | diff --git a/tests/qemu-iotests/301 b/tests/qemu-iotests/301 | ||
276 | index XXXXXXX..XXXXXXX 100755 | ||
277 | --- a/tests/qemu-iotests/301 | ||
278 | +++ b/tests/qemu-iotests/301 | ||
279 | @@ -XXX,XX +XXX,XX @@ | ||
280 | # | ||
281 | # Test qcow backing file warnings | ||
282 | # | ||
283 | -# Copyright (C) 2020 Red Hat, Inc. | ||
284 | +# Copyright (C) 2020-2021 Red Hat, Inc. | ||
285 | # | ||
286 | # This program is free software; you can redistribute it and/or modify | ||
287 | # it under the terms of the GNU General Public License as published by | ||
288 | @@ -XXX,XX +XXX,XX @@ echo "== qcow backed by qcow ==" | ||
289 | |||
290 | TEST_IMG="$TEST_IMG.base" _make_test_img $size | ||
291 | _make_test_img -b "$TEST_IMG.base" $size | ||
292 | -_img_info | ||
293 | _make_test_img -b "$TEST_IMG.base" -F $IMGFMT $size | ||
294 | _img_info | ||
295 | |||
296 | @@ -XXX,XX +XXX,XX @@ echo "== qcow backed by raw ==" | ||
297 | rm "$TEST_IMG.base" | ||
298 | truncate --size=$size "$TEST_IMG.base" | ||
299 | _make_test_img -b "$TEST_IMG.base" $size | ||
300 | -_img_info | ||
301 | _make_test_img -b "$TEST_IMG.base" -F raw $size | ||
302 | _img_info | ||
303 | |||
304 | diff --git a/tests/qemu-iotests/301.out b/tests/qemu-iotests/301.out | ||
305 | index XXXXXXX..XXXXXXX 100644 | ||
306 | --- a/tests/qemu-iotests/301.out | ||
307 | +++ b/tests/qemu-iotests/301.out | ||
308 | @@ -XXX,XX +XXX,XX @@ QA output created by 301 | ||
309 | |||
310 | == qcow backed by qcow == | ||
311 | Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=33554432 | ||
312 | -qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of IMGFMT) | ||
313 | -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT | ||
314 | -image: TEST_DIR/t.IMGFMT | ||
315 | -file format: IMGFMT | ||
316 | -virtual size: 32 MiB (33554432 bytes) | ||
317 | -cluster_size: 512 | ||
318 | -backing file: TEST_DIR/t.IMGFMT.base | ||
319 | +qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format | ||
320 | Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=IMGFMT | ||
321 | image: TEST_DIR/t.IMGFMT | ||
322 | file format: IMGFMT | ||
323 | @@ -XXX,XX +XXX,XX @@ cluster_size: 512 | ||
324 | backing file: TEST_DIR/t.IMGFMT.base | ||
325 | |||
326 | == qcow backed by raw == | ||
327 | -qemu-img: warning: Deprecated use of backing file without explicit backing format (detected format of raw) | ||
328 | -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base | ||
329 | -image: TEST_DIR/t.IMGFMT | ||
330 | -file format: IMGFMT | ||
331 | -virtual size: 32 MiB (33554432 bytes) | ||
332 | -cluster_size: 512 | ||
333 | -backing file: TEST_DIR/t.IMGFMT.base | ||
334 | +qemu-img: TEST_DIR/t.IMGFMT: Backing file specified without backing format | ||
335 | Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=33554432 backing_file=TEST_DIR/t.IMGFMT.base backing_fmt=raw | ||
336 | image: TEST_DIR/t.IMGFMT | ||
337 | file format: IMGFMT | ||
338 | -- | ||
339 | 2.31.1 | ||
340 | |||
341 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Eric Blake <eblake@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | No reason to forbid them, and they are needed to improve performance | 3 | When removeing support for qemu-img being able to create backing |
4 | with compress-threads in further patches. | 4 | chains without embedded backing formats, we caused a poor error |
5 | message as caught by iotest 114. Improve the situation to inform the | ||
6 | user what went wrong. | ||
5 | 7 | ||
6 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 8 | Suggested-by: Kevin Wolf <kwolf@redhat.com> |
9 | Signed-off-by: Eric Blake <eblake@redhat.com> | ||
10 | Message-Id: <20210708155228.2666172-1-eblake@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 12 | --- |
9 | qemu-img.c | 5 ----- | 13 | qemu-img.c | 3 +++ |
10 | 1 file changed, 5 deletions(-) | 14 | tests/qemu-iotests/114.out | 2 +- |
15 | 2 files changed, 4 insertions(+), 1 deletion(-) | ||
11 | 16 | ||
12 | diff --git a/qemu-img.c b/qemu-img.c | 17 | diff --git a/qemu-img.c b/qemu-img.c |
13 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/qemu-img.c | 19 | --- a/qemu-img.c |
15 | +++ b/qemu-img.c | 20 | +++ b/qemu-img.c |
16 | @@ -XXX,XX +XXX,XX @@ static int img_convert(int argc, char **argv) | 21 | @@ -XXX,XX +XXX,XX @@ static int img_rebase(int argc, char **argv) |
17 | goto fail_getopt; | 22 | if (ret == -ENOSPC) { |
18 | } | 23 | error_report("Could not change the backing file to '%s': No " |
19 | 24 | "space left in the file header", out_baseimg); | |
20 | - if (!s.wr_in_order && s.compressed) { | 25 | + } else if (ret == -EINVAL && out_baseimg && !out_basefmt) { |
21 | - error_report("Out of order write and compress are mutually exclusive"); | 26 | + error_report("Could not change the backing file to '%s': backing " |
22 | - goto fail_getopt; | 27 | + "format must be specified", out_baseimg); |
23 | - } | 28 | } else if (ret < 0) { |
24 | - | 29 | error_report("Could not change the backing file to '%s': %s", |
25 | if (tgt_image_opts && !skip_create) { | 30 | out_baseimg, strerror(-ret)); |
26 | error_report("--target-image-opts requires use of -n flag"); | 31 | diff --git a/tests/qemu-iotests/114.out b/tests/qemu-iotests/114.out |
27 | goto fail_getopt; | 32 | index XXXXXXX..XXXXXXX 100644 |
33 | --- a/tests/qemu-iotests/114.out | ||
34 | +++ b/tests/qemu-iotests/114.out | ||
35 | @@ -XXX,XX +XXX,XX @@ qemu-io: can't open device TEST_DIR/t.qcow2: Could not open backing file: Unknow | ||
36 | no file open, try 'help open' | ||
37 | read 4096/4096 bytes at offset 0 | ||
38 | 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
39 | -qemu-img: Could not change the backing file to 'TEST_DIR/t.qcow2.base': Invalid argument | ||
40 | +qemu-img: Could not change the backing file to 'TEST_DIR/t.qcow2.base': backing format must be specified | ||
41 | read 4096/4096 bytes at offset 0 | ||
42 | 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
43 | *** done | ||
28 | -- | 44 | -- |
29 | 2.13.6 | 45 | 2.31.1 |
30 | 46 | ||
31 | 47 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | Without an external data file, s->data_file is a second pointer with the |
---|---|---|---|
2 | same value as bs->file. When changing bs->file to a different BdrvChild | ||
3 | and freeing the old BdrvChild, s->data_file must also be updated, | ||
4 | otherwise it points to freed memory and causes crashes. | ||
2 | 5 | ||
3 | Make a separate function for compression to be parallelized later. | 6 | This problem was caught by iotests case 245. |
4 | - use .avail_out field instead of .next_out to calculate size of | ||
5 | compressed data. It looks more natural and it allows to keep dest to | ||
6 | be void pointer | ||
7 | - set avail_out to be at least one byte less than input, to be sure | ||
8 | avoid inefficient compression earlier | ||
9 | 7 | ||
10 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 8 | Fixes: df2b7086f169239ebad5d150efa29c9bb6d4f820 |
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Message-Id: <20210708114709.206487-2-kwolf@redhat.com> | ||
11 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
12 | --- | 14 | --- |
13 | block/qcow2.c | 76 ++++++++++++++++++++++++++++++++++++++--------------------- | 15 | block/qcow2.c | 29 +++++++++++++++++++++++++++++ |
14 | 1 file changed, 49 insertions(+), 27 deletions(-) | 16 | 1 file changed, 29 insertions(+) |
15 | 17 | ||
16 | diff --git a/block/qcow2.c b/block/qcow2.c | 18 | diff --git a/block/qcow2.c b/block/qcow2.c |
17 | index XXXXXXX..XXXXXXX 100644 | 19 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/block/qcow2.c | 20 | --- a/block/qcow2.c |
19 | +++ b/block/qcow2.c | 21 | +++ b/block/qcow2.c |
20 | @@ -XXX,XX +XXX,XX @@ | 22 | @@ -XXX,XX +XXX,XX @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp) |
21 | */ | 23 | static int qcow2_reopen_prepare(BDRVReopenState *state, |
22 | 24 | BlockReopenQueue *queue, Error **errp) | |
23 | #include "qemu/osdep.h" | 25 | { |
24 | + | 26 | + BDRVQcow2State *s = state->bs->opaque; |
25 | +#define ZLIB_CONST | 27 | Qcow2ReopenState *r; |
26 | +#include <zlib.h> | 28 | int ret; |
27 | + | 29 | |
28 | #include "block/block_int.h" | 30 | @@ -XXX,XX +XXX,XX @@ static int qcow2_reopen_prepare(BDRVReopenState *state, |
29 | #include "block/qdict.h" | 31 | } |
30 | #include "sysemu/block-backend.h" | 32 | } |
31 | #include "qemu/module.h" | 33 | |
32 | -#include <zlib.h> | 34 | + /* |
33 | #include "qcow2.h" | 35 | + * Without an external data file, s->data_file points to the same BdrvChild |
34 | #include "qemu/error-report.h" | 36 | + * as bs->file. It needs to be resynced after reopen because bs->file may |
35 | #include "qapi/error.h" | 37 | + * be changed. We can't use it in the meantime. |
36 | @@ -XXX,XX +XXX,XX @@ fail: | 38 | + */ |
37 | return ret; | 39 | + if (!has_data_file(state->bs)) { |
38 | } | 40 | + assert(s->data_file == state->bs->file); |
39 | 41 | + s->data_file = NULL; | |
40 | +/* | ||
41 | + * qcow2_compress() | ||
42 | + * | ||
43 | + * @dest - destination buffer, at least of @size-1 bytes | ||
44 | + * @src - source buffer, @size bytes | ||
45 | + * | ||
46 | + * Returns: compressed size on success | ||
47 | + * -1 if compression is inefficient | ||
48 | + * -2 on any other error | ||
49 | + */ | ||
50 | +static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | ||
51 | +{ | ||
52 | + ssize_t ret; | ||
53 | + z_stream strm; | ||
54 | + | ||
55 | + /* best compression, small window, no zlib header */ | ||
56 | + memset(&strm, 0, sizeof(strm)); | ||
57 | + ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED, | ||
58 | + -12, 9, Z_DEFAULT_STRATEGY); | ||
59 | + if (ret != 0) { | ||
60 | + return -2; | ||
61 | + } | 42 | + } |
62 | + | 43 | + |
63 | + strm.avail_in = size; | 44 | return 0; |
64 | + strm.next_in = src; | 45 | |
65 | + strm.avail_out = size - 1; | 46 | fail: |
66 | + strm.next_out = dest; | 47 | @@ -XXX,XX +XXX,XX @@ fail: |
48 | |||
49 | static void qcow2_reopen_commit(BDRVReopenState *state) | ||
50 | { | ||
51 | + BDRVQcow2State *s = state->bs->opaque; | ||
67 | + | 52 | + |
68 | + ret = deflate(&strm, Z_FINISH); | 53 | qcow2_update_options_commit(state->bs, state->opaque); |
69 | + if (ret == Z_STREAM_END) { | 54 | + if (!s->data_file) { |
70 | + ret = size - 1 - strm.avail_out; | 55 | + /* |
71 | + } else { | 56 | + * If we don't have an external data file, s->data_file was cleared by |
72 | + ret = (ret == Z_OK ? -1 : -2); | 57 | + * qcow2_reopen_prepare() and needs to be updated. |
58 | + */ | ||
59 | + s->data_file = state->bs->file; | ||
73 | + } | 60 | + } |
61 | g_free(state->opaque); | ||
62 | } | ||
63 | |||
64 | @@ -XXX,XX +XXX,XX @@ static void qcow2_reopen_commit_post(BDRVReopenState *state) | ||
65 | |||
66 | static void qcow2_reopen_abort(BDRVReopenState *state) | ||
67 | { | ||
68 | + BDRVQcow2State *s = state->bs->opaque; | ||
74 | + | 69 | + |
75 | + deflateEnd(&strm); | 70 | + if (!s->data_file) { |
76 | + | 71 | + /* |
77 | + return ret; | 72 | + * If we don't have an external data file, s->data_file was cleared by |
78 | +} | 73 | + * qcow2_reopen_prepare() and needs to be restored. |
79 | + | 74 | + */ |
80 | /* XXX: put compressed sectors first, then all the cluster aligned | 75 | + s->data_file = state->bs->file; |
81 | tables to avoid losing bytes in alignment */ | 76 | + } |
82 | static coroutine_fn int | 77 | qcow2_update_options_abort(state->bs, state->opaque); |
83 | @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, | 78 | g_free(state->opaque); |
84 | BDRVQcow2State *s = bs->opaque; | 79 | } |
85 | QEMUIOVector hd_qiov; | ||
86 | struct iovec iov; | ||
87 | - z_stream strm; | ||
88 | - int ret, out_len; | ||
89 | + int ret; | ||
90 | + size_t out_len; | ||
91 | uint8_t *buf, *out_buf; | ||
92 | int64_t cluster_offset; | ||
93 | |||
94 | @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, | ||
95 | |||
96 | out_buf = g_malloc(s->cluster_size); | ||
97 | |||
98 | - /* best compression, small window, no zlib header */ | ||
99 | - memset(&strm, 0, sizeof(strm)); | ||
100 | - ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, | ||
101 | - Z_DEFLATED, -12, | ||
102 | - 9, Z_DEFAULT_STRATEGY); | ||
103 | - if (ret != 0) { | ||
104 | + out_len = qcow2_compress(out_buf, buf, s->cluster_size); | ||
105 | + if (out_len == -2) { | ||
106 | ret = -EINVAL; | ||
107 | goto fail; | ||
108 | - } | ||
109 | - | ||
110 | - strm.avail_in = s->cluster_size; | ||
111 | - strm.next_in = (uint8_t *)buf; | ||
112 | - strm.avail_out = s->cluster_size; | ||
113 | - strm.next_out = out_buf; | ||
114 | - | ||
115 | - ret = deflate(&strm, Z_FINISH); | ||
116 | - if (ret != Z_STREAM_END && ret != Z_OK) { | ||
117 | - deflateEnd(&strm); | ||
118 | - ret = -EINVAL; | ||
119 | - goto fail; | ||
120 | - } | ||
121 | - out_len = strm.next_out - out_buf; | ||
122 | - | ||
123 | - deflateEnd(&strm); | ||
124 | - | ||
125 | - if (ret != Z_STREAM_END || out_len >= s->cluster_size) { | ||
126 | + } else if (out_len == -1) { | ||
127 | /* could not compress: write normal cluster */ | ||
128 | ret = qcow2_co_pwritev(bs, offset, bytes, qiov, 0); | ||
129 | if (ret < 0) { | ||
130 | -- | 80 | -- |
131 | 2.13.6 | 81 | 2.31.1 |
132 | 82 | ||
133 | 83 | diff view generated by jsdifflib |
1 | From: Ari Sundholm <ari@tuxera.com> | 1 | From: Alberto Garcia <berto@igalia.com> |
---|---|---|---|
2 | 2 | ||
3 | This allows using the two constants outside of block.c, which will | 3 | Move the code to free a BlockReopenQueue to a separate function. |
4 | happen in a subsequent patch. | 4 | It will be used in a subsequent patch. |
5 | 5 | ||
6 | Signed-off-by: Ari Sundholm <ari@tuxera.com> | 6 | [ kwolf: Also free explicit_options and options, and explicitly |
7 | qobject_ref() the value when it continues to be used. This makes | ||
8 | future memory leaks less likely. ] | ||
9 | |||
10 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
13 | Message-Id: <20210708114709.206487-3-kwolf@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 14 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 15 | --- |
9 | include/block/block.h | 7 +++++++ | 16 | include/block/block.h | 1 + |
10 | block.c | 6 ------ | 17 | block.c | 22 ++++++++++++++++------ |
11 | 2 files changed, 7 insertions(+), 6 deletions(-) | 18 | 2 files changed, 17 insertions(+), 6 deletions(-) |
12 | 19 | ||
13 | diff --git a/include/block/block.h b/include/block/block.h | 20 | diff --git a/include/block/block.h b/include/block/block.h |
14 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/include/block/block.h | 22 | --- a/include/block/block.h |
16 | +++ b/include/block/block.h | 23 | +++ b/include/block/block.h |
17 | @@ -XXX,XX +XXX,XX @@ enum { | 24 | @@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name, |
18 | BLK_PERM_GRAPH_MOD = 0x10, | 25 | BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, |
19 | 26 | BlockDriverState *bs, QDict *options, | |
20 | BLK_PERM_ALL = 0x1f, | 27 | bool keep_old_opts); |
21 | + | 28 | +void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue); |
22 | + DEFAULT_PERM_PASSTHROUGH = BLK_PERM_CONSISTENT_READ | 29 | int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp); |
23 | + | BLK_PERM_WRITE | 30 | int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, |
24 | + | BLK_PERM_WRITE_UNCHANGED | 31 | Error **errp); |
25 | + | BLK_PERM_RESIZE, | ||
26 | + | ||
27 | + DEFAULT_PERM_UNCHANGED = BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH, | ||
28 | }; | ||
29 | |||
30 | char *bdrv_perm_names(uint64_t perm); | ||
31 | diff --git a/block.c b/block.c | 32 | diff --git a/block.c b/block.c |
32 | index XXXXXXX..XXXXXXX 100644 | 33 | index XXXXXXX..XXXXXXX 100644 |
33 | --- a/block.c | 34 | --- a/block.c |
34 | +++ b/block.c | 35 | +++ b/block.c |
35 | @@ -XXX,XX +XXX,XX @@ int bdrv_child_try_set_perm(BdrvChild *c, uint64_t perm, uint64_t shared, | 36 | @@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, |
36 | return 0; | 37 | NULL, 0, keep_old_opts); |
37 | } | 38 | } |
38 | 39 | ||
39 | -#define DEFAULT_PERM_PASSTHROUGH (BLK_PERM_CONSISTENT_READ \ | 40 | +void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue) |
40 | - | BLK_PERM_WRITE \ | 41 | +{ |
41 | - | BLK_PERM_WRITE_UNCHANGED \ | 42 | + if (bs_queue) { |
42 | - | BLK_PERM_RESIZE) | 43 | + BlockReopenQueueEntry *bs_entry, *next; |
43 | -#define DEFAULT_PERM_UNCHANGED (BLK_PERM_ALL & ~DEFAULT_PERM_PASSTHROUGH) | 44 | + QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { |
44 | - | 45 | + qobject_unref(bs_entry->state.explicit_options); |
45 | void bdrv_filter_default_perms(BlockDriverState *bs, BdrvChild *c, | 46 | + qobject_unref(bs_entry->state.options); |
46 | const BdrvChildRole *role, | 47 | + g_free(bs_entry); |
47 | BlockReopenQueue *reopen_queue, | 48 | + } |
49 | + g_free(bs_queue); | ||
50 | + } | ||
51 | +} | ||
52 | + | ||
53 | /* | ||
54 | * Reopen multiple BlockDriverStates atomically & transactionally. | ||
55 | * | ||
56 | @@ -XXX,XX +XXX,XX @@ abort: | ||
57 | if (bs_entry->prepared) { | ||
58 | bdrv_reopen_abort(&bs_entry->state); | ||
59 | } | ||
60 | - qobject_unref(bs_entry->state.explicit_options); | ||
61 | - qobject_unref(bs_entry->state.options); | ||
62 | } | ||
63 | |||
64 | cleanup: | ||
65 | - QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { | ||
66 | - g_free(bs_entry); | ||
67 | - } | ||
68 | - g_free(bs_queue); | ||
69 | + bdrv_reopen_queue_free(bs_queue); | ||
70 | |||
71 | return ret; | ||
72 | } | ||
73 | @@ -XXX,XX +XXX,XX @@ static void bdrv_reopen_commit(BDRVReopenState *reopen_state) | ||
74 | /* set BDS specific flags now */ | ||
75 | qobject_unref(bs->explicit_options); | ||
76 | qobject_unref(bs->options); | ||
77 | + qobject_ref(reopen_state->explicit_options); | ||
78 | + qobject_ref(reopen_state->options); | ||
79 | |||
80 | bs->explicit_options = reopen_state->explicit_options; | ||
81 | bs->options = reopen_state->options; | ||
48 | -- | 82 | -- |
49 | 2.13.6 | 83 | 2.31.1 |
50 | 84 | ||
51 | 85 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | As the BlockReopenQueue can contain nodes in multiple AioContexts, only | |
2 | one of which may be locked when AIO_WAIT_WHILE() can be called, we can't | ||
3 | let the caller lock the right contexts. Instead, individually lock the | ||
4 | AioContext of a single node when iterating the queue. | ||
5 | |||
6 | Reintroduce bdrv_reopen() as a wrapper for reopening a single node that | ||
7 | drains the node and temporarily drops the AioContext lock for | ||
8 | bdrv_reopen_multiple(). | ||
9 | |||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
12 | Message-Id: <20210708114709.206487-4-kwolf@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | include/block/block.h | 2 ++ | ||
16 | block.c | 49 ++++++++++++++++++++++++++++++++++++------- | ||
17 | block/replication.c | 7 +++++++ | ||
18 | blockdev.c | 5 +++++ | ||
19 | qemu-io-cmds.c | 7 +------ | ||
20 | 5 files changed, 57 insertions(+), 13 deletions(-) | ||
21 | |||
22 | diff --git a/include/block/block.h b/include/block/block.h | ||
23 | index XXXXXXX..XXXXXXX 100644 | ||
24 | --- a/include/block/block.h | ||
25 | +++ b/include/block/block.h | ||
26 | @@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
27 | bool keep_old_opts); | ||
28 | void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue); | ||
29 | int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp); | ||
30 | +int bdrv_reopen(BlockDriverState *bs, QDict *opts, bool keep_old_opts, | ||
31 | + Error **errp); | ||
32 | int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
33 | Error **errp); | ||
34 | int bdrv_pwrite_zeroes(BdrvChild *child, int64_t offset, | ||
35 | diff --git a/block.c b/block.c | ||
36 | index XXXXXXX..XXXXXXX 100644 | ||
37 | --- a/block.c | ||
38 | +++ b/block.c | ||
39 | @@ -XXX,XX +XXX,XX @@ void bdrv_reopen_queue_free(BlockReopenQueue *bs_queue) | ||
40 | * | ||
41 | * All affected nodes must be drained between bdrv_reopen_queue() and | ||
42 | * bdrv_reopen_multiple(). | ||
43 | + * | ||
44 | + * To be called from the main thread, with all other AioContexts unlocked. | ||
45 | */ | ||
46 | int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) | ||
47 | { | ||
48 | int ret = -1; | ||
49 | BlockReopenQueueEntry *bs_entry, *next; | ||
50 | + AioContext *ctx; | ||
51 | Transaction *tran = tran_new(); | ||
52 | g_autoptr(GHashTable) found = NULL; | ||
53 | g_autoptr(GSList) refresh_list = NULL; | ||
54 | |||
55 | + assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | ||
56 | assert(bs_queue != NULL); | ||
57 | |||
58 | QTAILQ_FOREACH(bs_entry, bs_queue, entry) { | ||
59 | + ctx = bdrv_get_aio_context(bs_entry->state.bs); | ||
60 | + aio_context_acquire(ctx); | ||
61 | ret = bdrv_flush(bs_entry->state.bs); | ||
62 | + aio_context_release(ctx); | ||
63 | if (ret < 0) { | ||
64 | error_setg_errno(errp, -ret, "Error flushing drive"); | ||
65 | goto abort; | ||
66 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) | ||
67 | |||
68 | QTAILQ_FOREACH(bs_entry, bs_queue, entry) { | ||
69 | assert(bs_entry->state.bs->quiesce_counter > 0); | ||
70 | + ctx = bdrv_get_aio_context(bs_entry->state.bs); | ||
71 | + aio_context_acquire(ctx); | ||
72 | ret = bdrv_reopen_prepare(&bs_entry->state, bs_queue, tran, errp); | ||
73 | + aio_context_release(ctx); | ||
74 | if (ret < 0) { | ||
75 | goto abort; | ||
76 | } | ||
77 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) | ||
78 | * to first element. | ||
79 | */ | ||
80 | QTAILQ_FOREACH_REVERSE(bs_entry, bs_queue, entry) { | ||
81 | + ctx = bdrv_get_aio_context(bs_entry->state.bs); | ||
82 | + aio_context_acquire(ctx); | ||
83 | bdrv_reopen_commit(&bs_entry->state); | ||
84 | + aio_context_release(ctx); | ||
85 | } | ||
86 | |||
87 | tran_commit(tran); | ||
88 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_multiple(BlockReopenQueue *bs_queue, Error **errp) | ||
89 | BlockDriverState *bs = bs_entry->state.bs; | ||
90 | |||
91 | if (bs->drv->bdrv_reopen_commit_post) { | ||
92 | + ctx = bdrv_get_aio_context(bs); | ||
93 | + aio_context_acquire(ctx); | ||
94 | bs->drv->bdrv_reopen_commit_post(&bs_entry->state); | ||
95 | + aio_context_release(ctx); | ||
96 | } | ||
97 | } | ||
98 | |||
99 | @@ -XXX,XX +XXX,XX @@ abort: | ||
100 | tran_abort(tran); | ||
101 | QTAILQ_FOREACH_SAFE(bs_entry, bs_queue, entry, next) { | ||
102 | if (bs_entry->prepared) { | ||
103 | + ctx = bdrv_get_aio_context(bs_entry->state.bs); | ||
104 | + aio_context_acquire(ctx); | ||
105 | bdrv_reopen_abort(&bs_entry->state); | ||
106 | + aio_context_release(ctx); | ||
107 | } | ||
108 | } | ||
109 | |||
110 | @@ -XXX,XX +XXX,XX @@ cleanup: | ||
111 | return ret; | ||
112 | } | ||
113 | |||
114 | -int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
115 | - Error **errp) | ||
116 | +int bdrv_reopen(BlockDriverState *bs, QDict *opts, bool keep_old_opts, | ||
117 | + Error **errp) | ||
118 | { | ||
119 | - int ret; | ||
120 | + AioContext *ctx = bdrv_get_aio_context(bs); | ||
121 | BlockReopenQueue *queue; | ||
122 | - QDict *opts = qdict_new(); | ||
123 | - | ||
124 | - qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only); | ||
125 | + int ret; | ||
126 | |||
127 | bdrv_subtree_drained_begin(bs); | ||
128 | - queue = bdrv_reopen_queue(NULL, bs, opts, true); | ||
129 | + if (ctx != qemu_get_aio_context()) { | ||
130 | + aio_context_release(ctx); | ||
131 | + } | ||
132 | + | ||
133 | + queue = bdrv_reopen_queue(NULL, bs, opts, keep_old_opts); | ||
134 | ret = bdrv_reopen_multiple(queue, errp); | ||
135 | + | ||
136 | + if (ctx != qemu_get_aio_context()) { | ||
137 | + aio_context_acquire(ctx); | ||
138 | + } | ||
139 | bdrv_subtree_drained_end(bs); | ||
140 | |||
141 | return ret; | ||
142 | } | ||
143 | |||
144 | +int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
145 | + Error **errp) | ||
146 | +{ | ||
147 | + QDict *opts = qdict_new(); | ||
148 | + | ||
149 | + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only); | ||
150 | + | ||
151 | + return bdrv_reopen(bs, opts, true, errp); | ||
152 | +} | ||
153 | + | ||
154 | /* | ||
155 | * Take a BDRVReopenState and check if the value of 'backing' in the | ||
156 | * reopen_state->options QDict is valid or not. | ||
157 | diff --git a/block/replication.c b/block/replication.c | ||
158 | index XXXXXXX..XXXXXXX 100644 | ||
159 | --- a/block/replication.c | ||
160 | +++ b/block/replication.c | ||
161 | @@ -XXX,XX +XXX,XX @@ static void reopen_backing_file(BlockDriverState *bs, bool writable, | ||
162 | } | ||
163 | |||
164 | if (reopen_queue) { | ||
165 | + AioContext *ctx = bdrv_get_aio_context(bs); | ||
166 | + if (ctx != qemu_get_aio_context()) { | ||
167 | + aio_context_release(ctx); | ||
168 | + } | ||
169 | bdrv_reopen_multiple(reopen_queue, errp); | ||
170 | + if (ctx != qemu_get_aio_context()) { | ||
171 | + aio_context_acquire(ctx); | ||
172 | + } | ||
173 | } | ||
174 | |||
175 | bdrv_subtree_drained_end(s->hidden_disk->bs); | ||
176 | diff --git a/blockdev.c b/blockdev.c | ||
177 | index XXXXXXX..XXXXXXX 100644 | ||
178 | --- a/blockdev.c | ||
179 | +++ b/blockdev.c | ||
180 | @@ -XXX,XX +XXX,XX @@ void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp) | ||
181 | ctx = bdrv_get_aio_context(bs); | ||
182 | aio_context_acquire(ctx); | ||
183 | bdrv_subtree_drained_begin(bs); | ||
184 | + aio_context_release(ctx); | ||
185 | + | ||
186 | queue = bdrv_reopen_queue(NULL, bs, qdict, false); | ||
187 | bdrv_reopen_multiple(queue, errp); | ||
188 | + | ||
189 | + ctx = bdrv_get_aio_context(bs); | ||
190 | + aio_context_acquire(ctx); | ||
191 | bdrv_subtree_drained_end(bs); | ||
192 | aio_context_release(ctx); | ||
193 | |||
194 | diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c | ||
195 | index XXXXXXX..XXXXXXX 100644 | ||
196 | --- a/qemu-io-cmds.c | ||
197 | +++ b/qemu-io-cmds.c | ||
198 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
199 | bool writethrough = !blk_enable_write_cache(blk); | ||
200 | bool has_rw_option = false; | ||
201 | bool has_cache_option = false; | ||
202 | - | ||
203 | - BlockReopenQueue *brq; | ||
204 | Error *local_err = NULL; | ||
205 | |||
206 | while ((c = getopt(argc, argv, "c:o:rw")) != -1) { | ||
207 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
208 | qdict_put_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, flags & BDRV_O_NO_FLUSH); | ||
209 | } | ||
210 | |||
211 | - bdrv_subtree_drained_begin(bs); | ||
212 | - brq = bdrv_reopen_queue(NULL, bs, opts, true); | ||
213 | - bdrv_reopen_multiple(brq, &local_err); | ||
214 | - bdrv_subtree_drained_end(bs); | ||
215 | + bdrv_reopen(bs, opts, true, &local_err); | ||
216 | |||
217 | if (local_err) { | ||
218 | error_report_err(local_err); | ||
219 | -- | ||
220 | 2.31.1 | ||
221 | |||
222 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Alberto Garcia <berto@igalia.com> | |
2 | |||
3 | [ kwolf: Fixed AioContext locking ] | ||
4 | |||
5 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Message-Id: <20210708114709.206487-5-kwolf@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | qapi/block-core.json | 18 +++-- | ||
12 | blockdev.c | 81 ++++++++++--------- | ||
13 | tests/qemu-iotests/155 | 9 ++- | ||
14 | tests/qemu-iotests/165 | 4 +- | ||
15 | tests/qemu-iotests/245 | 27 ++++--- | ||
16 | tests/qemu-iotests/248 | 2 +- | ||
17 | tests/qemu-iotests/248.out | 2 +- | ||
18 | tests/qemu-iotests/296 | 9 ++- | ||
19 | tests/qemu-iotests/298 | 4 +- | ||
20 | .../tests/remove-bitmap-from-backing | 18 +++-- | ||
21 | 10 files changed, 99 insertions(+), 75 deletions(-) | ||
22 | |||
23 | diff --git a/qapi/block-core.json b/qapi/block-core.json | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/qapi/block-core.json | ||
26 | +++ b/qapi/block-core.json | ||
27 | @@ -XXX,XX +XXX,XX @@ | ||
28 | ## | ||
29 | # @x-blockdev-reopen: | ||
30 | # | ||
31 | -# Reopens a block device using the given set of options. Any option | ||
32 | -# not specified will be reset to its default value regardless of its | ||
33 | -# previous status. If an option cannot be changed or a particular | ||
34 | +# Reopens one or more block devices using the given set of options. | ||
35 | +# Any option not specified will be reset to its default value regardless | ||
36 | +# of its previous status. If an option cannot be changed or a particular | ||
37 | # driver does not support reopening then the command will return an | ||
38 | -# error. | ||
39 | +# error. All devices in the list are reopened in one transaction, so | ||
40 | +# if one of them fails then the whole transaction is cancelled. | ||
41 | # | ||
42 | -# The top-level @node-name option (from BlockdevOptions) must be | ||
43 | +# The command receives a list of block devices to reopen. For each one | ||
44 | +# of them, the top-level @node-name option (from BlockdevOptions) must be | ||
45 | # specified and is used to select the block device to be reopened. | ||
46 | # Other @node-name options must be either omitted or set to the | ||
47 | # current name of the appropriate node. This command won't change any | ||
48 | @@ -XXX,XX +XXX,XX @@ | ||
49 | # | ||
50 | # 4) NULL: the current child (if any) is detached. | ||
51 | # | ||
52 | -# Options (1) and (2) are supported in all cases, but at the moment | ||
53 | -# only @backing allows replacing or detaching an existing child. | ||
54 | +# Options (1) and (2) are supported in all cases. Option (3) is | ||
55 | +# supported for @file and @backing, and option (4) for @backing only. | ||
56 | # | ||
57 | # Unlike with blockdev-add, the @backing option must always be present | ||
58 | # unless the node being reopened does not have a backing file and its | ||
59 | @@ -XXX,XX +XXX,XX @@ | ||
60 | # Since: 4.0 | ||
61 | ## | ||
62 | { 'command': 'x-blockdev-reopen', | ||
63 | - 'data': 'BlockdevOptions', 'boxed': true } | ||
64 | + 'data': { 'options': ['BlockdevOptions'] } } | ||
65 | |||
66 | ## | ||
67 | # @blockdev-del: | ||
68 | diff --git a/blockdev.c b/blockdev.c | ||
69 | index XXXXXXX..XXXXXXX 100644 | ||
70 | --- a/blockdev.c | ||
71 | +++ b/blockdev.c | ||
72 | @@ -XXX,XX +XXX,XX @@ fail: | ||
73 | visit_free(v); | ||
74 | } | ||
75 | |||
76 | -void qmp_x_blockdev_reopen(BlockdevOptions *options, Error **errp) | ||
77 | -{ | ||
78 | - BlockDriverState *bs; | ||
79 | - AioContext *ctx; | ||
80 | - QObject *obj; | ||
81 | - Visitor *v = qobject_output_visitor_new(&obj); | ||
82 | - BlockReopenQueue *queue; | ||
83 | - QDict *qdict; | ||
84 | +void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp) | ||
85 | +{ | ||
86 | + BlockReopenQueue *queue = NULL; | ||
87 | + GSList *drained = NULL; | ||
88 | + | ||
89 | + /* Add each one of the BDS that we want to reopen to the queue */ | ||
90 | + for (; reopen_list != NULL; reopen_list = reopen_list->next) { | ||
91 | + BlockdevOptions *options = reopen_list->value; | ||
92 | + BlockDriverState *bs; | ||
93 | + AioContext *ctx; | ||
94 | + QObject *obj; | ||
95 | + Visitor *v; | ||
96 | + QDict *qdict; | ||
97 | + | ||
98 | + /* Check for the selected node name */ | ||
99 | + if (!options->has_node_name) { | ||
100 | + error_setg(errp, "node-name not specified"); | ||
101 | + goto fail; | ||
102 | + } | ||
103 | |||
104 | - /* Check for the selected node name */ | ||
105 | - if (!options->has_node_name) { | ||
106 | - error_setg(errp, "node-name not specified"); | ||
107 | - goto fail; | ||
108 | - } | ||
109 | + bs = bdrv_find_node(options->node_name); | ||
110 | + if (!bs) { | ||
111 | + error_setg(errp, "Failed to find node with node-name='%s'", | ||
112 | + options->node_name); | ||
113 | + goto fail; | ||
114 | + } | ||
115 | |||
116 | - bs = bdrv_find_node(options->node_name); | ||
117 | - if (!bs) { | ||
118 | - error_setg(errp, "Failed to find node with node-name='%s'", | ||
119 | - options->node_name); | ||
120 | - goto fail; | ||
121 | - } | ||
122 | + /* Put all options in a QDict and flatten it */ | ||
123 | + v = qobject_output_visitor_new(&obj); | ||
124 | + visit_type_BlockdevOptions(v, NULL, &options, &error_abort); | ||
125 | + visit_complete(v, &obj); | ||
126 | + visit_free(v); | ||
127 | |||
128 | - /* Put all options in a QDict and flatten it */ | ||
129 | - visit_type_BlockdevOptions(v, NULL, &options, &error_abort); | ||
130 | - visit_complete(v, &obj); | ||
131 | - qdict = qobject_to(QDict, obj); | ||
132 | + qdict = qobject_to(QDict, obj); | ||
133 | |||
134 | - qdict_flatten(qdict); | ||
135 | + qdict_flatten(qdict); | ||
136 | |||
137 | - /* Perform the reopen operation */ | ||
138 | - ctx = bdrv_get_aio_context(bs); | ||
139 | - aio_context_acquire(ctx); | ||
140 | - bdrv_subtree_drained_begin(bs); | ||
141 | - aio_context_release(ctx); | ||
142 | + ctx = bdrv_get_aio_context(bs); | ||
143 | + aio_context_acquire(ctx); | ||
144 | |||
145 | - queue = bdrv_reopen_queue(NULL, bs, qdict, false); | ||
146 | - bdrv_reopen_multiple(queue, errp); | ||
147 | + bdrv_subtree_drained_begin(bs); | ||
148 | + queue = bdrv_reopen_queue(queue, bs, qdict, false); | ||
149 | + drained = g_slist_prepend(drained, bs); | ||
150 | |||
151 | - ctx = bdrv_get_aio_context(bs); | ||
152 | - aio_context_acquire(ctx); | ||
153 | - bdrv_subtree_drained_end(bs); | ||
154 | - aio_context_release(ctx); | ||
155 | + aio_context_release(ctx); | ||
156 | + } | ||
157 | + | ||
158 | + /* Perform the reopen operation */ | ||
159 | + bdrv_reopen_multiple(queue, errp); | ||
160 | + queue = NULL; | ||
161 | |||
162 | fail: | ||
163 | - visit_free(v); | ||
164 | + bdrv_reopen_queue_free(queue); | ||
165 | + g_slist_free_full(drained, (GDestroyNotify) bdrv_subtree_drained_end); | ||
166 | } | ||
167 | |||
168 | void qmp_blockdev_del(const char *node_name, Error **errp) | ||
169 | diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155 | ||
170 | index XXXXXXX..XXXXXXX 100755 | ||
171 | --- a/tests/qemu-iotests/155 | ||
172 | +++ b/tests/qemu-iotests/155 | ||
173 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorReopen(MirrorBaseClass): | ||
174 | result = self.vm.qmp('blockdev-add', node_name="backing", | ||
175 | driver="null-co") | ||
176 | self.assert_qmp(result, 'return', {}) | ||
177 | - result = self.vm.qmp('x-blockdev-reopen', node_name="target", | ||
178 | - driver=iotests.imgfmt, file="target-file", | ||
179 | - backing="backing") | ||
180 | + result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
181 | + 'node-name': "target", | ||
182 | + 'driver': iotests.imgfmt, | ||
183 | + 'file': "target-file", | ||
184 | + 'backing': "backing" | ||
185 | + }]) | ||
186 | self.assert_qmp(result, 'return', {}) | ||
187 | |||
188 | class TestBlockdevMirrorReopenIothread(TestBlockdevMirrorReopen): | ||
189 | diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165 | ||
190 | index XXXXXXX..XXXXXXX 100755 | ||
191 | --- a/tests/qemu-iotests/165 | ||
192 | +++ b/tests/qemu-iotests/165 | ||
193 | @@ -XXX,XX +XXX,XX @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): | ||
194 | assert sha256_1 == self.getSha256() | ||
195 | |||
196 | # Reopen to RW | ||
197 | - result = self.vm.qmp('x-blockdev-reopen', **{ | ||
198 | + result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
199 | 'node-name': 'node0', | ||
200 | 'driver': iotests.imgfmt, | ||
201 | 'file': { | ||
202 | @@ -XXX,XX +XXX,XX @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): | ||
203 | 'filename': disk | ||
204 | }, | ||
205 | 'read-only': False | ||
206 | - }) | ||
207 | + }]) | ||
208 | self.assert_qmp(result, 'return', {}) | ||
209 | |||
210 | # Check that bitmap is reopened to RW and we can write to it. | ||
211 | diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 | ||
212 | index XXXXXXX..XXXXXXX 100755 | ||
213 | --- a/tests/qemu-iotests/245 | ||
214 | +++ b/tests/qemu-iotests/245 | ||
215 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
216 | "Expected output of %d qemu-io commands, found %d" % | ||
217 | (found, self.total_io_cmds)) | ||
218 | |||
219 | - # Run x-blockdev-reopen with 'opts' but applying 'newopts' | ||
220 | - # on top of it. The original 'opts' dict is unmodified | ||
221 | + # Run x-blockdev-reopen on a list of block devices | ||
222 | + def reopenMultiple(self, opts, errmsg = None): | ||
223 | + result = self.vm.qmp('x-blockdev-reopen', conv_keys=False, options=opts) | ||
224 | + if errmsg: | ||
225 | + self.assert_qmp(result, 'error/class', 'GenericError') | ||
226 | + self.assert_qmp(result, 'error/desc', errmsg) | ||
227 | + else: | ||
228 | + self.assert_qmp(result, 'return', {}) | ||
229 | + | ||
230 | + # Run x-blockdev-reopen on a single block device (specified by | ||
231 | + # 'opts') but applying 'newopts' on top of it. The original 'opts' | ||
232 | + # dict is unmodified | ||
233 | def reopen(self, opts, newopts = {}, errmsg = None): | ||
234 | opts = copy.deepcopy(opts) | ||
235 | |||
236 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
237 | subdict = opts[prefix] | ||
238 | subdict[key] = value | ||
239 | |||
240 | - result = self.vm.qmp('x-blockdev-reopen', conv_keys = False, **opts) | ||
241 | - if errmsg: | ||
242 | - self.assert_qmp(result, 'error/class', 'GenericError') | ||
243 | - self.assert_qmp(result, 'error/desc', errmsg) | ||
244 | - else: | ||
245 | - self.assert_qmp(result, 'return', {}) | ||
246 | + self.reopenMultiple([ opts ], errmsg) | ||
247 | |||
248 | |||
249 | # Run query-named-block-nodes and return the specified entry | ||
250 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
251 | # We cannot change any of these | ||
252 | self.reopen(opts, {'node-name': 'not-found'}, "Failed to find node with node-name='not-found'") | ||
253 | self.reopen(opts, {'node-name': ''}, "Failed to find node with node-name=''") | ||
254 | - self.reopen(opts, {'node-name': None}, "Invalid parameter type for 'node-name', expected: string") | ||
255 | + self.reopen(opts, {'node-name': None}, "Invalid parameter type for 'options[0].node-name', expected: string") | ||
256 | self.reopen(opts, {'driver': 'raw'}, "Cannot change the option 'driver'") | ||
257 | self.reopen(opts, {'driver': ''}, "Invalid parameter ''") | ||
258 | - self.reopen(opts, {'driver': None}, "Invalid parameter type for 'driver', expected: string") | ||
259 | + self.reopen(opts, {'driver': None}, "Invalid parameter type for 'options[0].driver', expected: string") | ||
260 | self.reopen(opts, {'file': 'not-found'}, "Cannot find device='' nor node-name='not-found'") | ||
261 | self.reopen(opts, {'file': ''}, "Cannot find device='' nor node-name=''") | ||
262 | self.reopen(opts, {'file': None}, "Invalid parameter type for 'file', expected: BlockdevRef") | ||
263 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
264 | self.reopen(opts, {'file.filename': hd_path[1]}, "Cannot change the option 'filename'") | ||
265 | self.reopen(opts, {'file.aio': 'native'}, "Cannot change the option 'aio'") | ||
266 | self.reopen(opts, {'file.locking': 'off'}, "Cannot change the option 'locking'") | ||
267 | - self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'file.filename', expected: string") | ||
268 | + self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'options[0].file.filename', expected: string") | ||
269 | |||
270 | # node-name is optional in BlockdevOptions, but x-blockdev-reopen needs it | ||
271 | del opts['node-name'] | ||
272 | diff --git a/tests/qemu-iotests/248 b/tests/qemu-iotests/248 | ||
273 | index XXXXXXX..XXXXXXX 100755 | ||
274 | --- a/tests/qemu-iotests/248 | ||
275 | +++ b/tests/qemu-iotests/248 | ||
276 | @@ -XXX,XX +XXX,XX @@ vm.get_qmp_events() | ||
277 | |||
278 | del blockdev_opts['file']['size'] | ||
279 | vm.qmp_log('x-blockdev-reopen', filters=[filter_qmp_testfiles], | ||
280 | - **blockdev_opts) | ||
281 | + options = [ blockdev_opts ]) | ||
282 | |||
283 | vm.qmp_log('block-job-resume', device='drive0') | ||
284 | vm.event_wait('JOB_STATUS_CHANGE', timeout=1.0, | ||
285 | diff --git a/tests/qemu-iotests/248.out b/tests/qemu-iotests/248.out | ||
286 | index XXXXXXX..XXXXXXX 100644 | ||
287 | --- a/tests/qemu-iotests/248.out | ||
288 | +++ b/tests/qemu-iotests/248.out | ||
289 | @@ -XXX,XX +XXX,XX @@ | ||
290 | {"return": {}} | ||
291 | {"execute": "blockdev-mirror", "arguments": {"device": "drive0", "on-target-error": "enospc", "sync": "full", "target": "target"}} | ||
292 | {"return": {}} | ||
293 | -{"execute": "x-blockdev-reopen", "arguments": {"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}} | ||
294 | +{"execute": "x-blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}} | ||
295 | {"return": {}} | ||
296 | {"execute": "block-job-resume", "arguments": {"device": "drive0"}} | ||
297 | {"return": {}} | ||
298 | diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296 | ||
299 | index XXXXXXX..XXXXXXX 100755 | ||
300 | --- a/tests/qemu-iotests/296 | ||
301 | +++ b/tests/qemu-iotests/296 | ||
302 | @@ -XXX,XX +XXX,XX @@ class EncryptionSetupTestCase(iotests.QMPTestCase): | ||
303 | |||
304 | command = 'x-blockdev-reopen' if reOpen else 'blockdev-add' | ||
305 | |||
306 | - result = vm.qmp(command, ** | ||
307 | - { | ||
308 | + opts = { | ||
309 | 'driver': iotests.imgfmt, | ||
310 | 'node-name': id, | ||
311 | 'read-only': readOnly, | ||
312 | @@ -XXX,XX +XXX,XX @@ class EncryptionSetupTestCase(iotests.QMPTestCase): | ||
313 | 'filename': test_img, | ||
314 | } | ||
315 | } | ||
316 | - ) | ||
317 | + | ||
318 | + if reOpen: | ||
319 | + result = vm.qmp(command, options=[opts]) | ||
320 | + else: | ||
321 | + result = vm.qmp(command, **opts) | ||
322 | self.assert_qmp(result, 'return', {}) | ||
323 | |||
324 | |||
325 | diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298 | ||
326 | index XXXXXXX..XXXXXXX 100755 | ||
327 | --- a/tests/qemu-iotests/298 | ||
328 | +++ b/tests/qemu-iotests/298 | ||
329 | @@ -XXX,XX +XXX,XX @@ class TestPreallocateFilter(TestPreallocateBase): | ||
330 | self.check_big() | ||
331 | |||
332 | def test_reopen_opts(self): | ||
333 | - result = self.vm.qmp('x-blockdev-reopen', **{ | ||
334 | + result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
335 | 'node-name': 'disk', | ||
336 | 'driver': iotests.imgfmt, | ||
337 | 'file': { | ||
338 | @@ -XXX,XX +XXX,XX @@ class TestPreallocateFilter(TestPreallocateBase): | ||
339 | 'filename': disk | ||
340 | } | ||
341 | } | ||
342 | - }) | ||
343 | + }]) | ||
344 | self.assert_qmp(result, 'return', {}) | ||
345 | |||
346 | self.vm.hmp_qemu_io('drive0', 'write 0 1M') | ||
347 | diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing b/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
348 | index XXXXXXX..XXXXXXX 100755 | ||
349 | --- a/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
350 | +++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
351 | @@ -XXX,XX +XXX,XX @@ log('Trying to remove persistent bitmap from r-o base node, should fail:') | ||
352 | vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0') | ||
353 | |||
354 | new_base_opts = { | ||
355 | - 'node-name': 'base', | ||
356 | - 'driver': 'qcow2', | ||
357 | - 'file': { | ||
358 | - 'driver': 'file', | ||
359 | - 'filename': base | ||
360 | - }, | ||
361 | - 'read-only': False | ||
362 | + 'options': [{ | ||
363 | + 'node-name': 'base', | ||
364 | + 'driver': 'qcow2', | ||
365 | + 'file': { | ||
366 | + 'driver': 'file', | ||
367 | + 'filename': base | ||
368 | + }, | ||
369 | + 'read-only': False | ||
370 | + }] | ||
371 | } | ||
372 | |||
373 | # Don't want to bother with filtering qmp_log for reopen command | ||
374 | @@ -XXX,XX +XXX,XX @@ if result != {'return': {}}: | ||
375 | log('Remove persistent bitmap from base node reopened to RW:') | ||
376 | vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0') | ||
377 | |||
378 | -new_base_opts['read-only'] = True | ||
379 | +new_base_opts['options'][0]['read-only'] = True | ||
380 | result = vm.qmp('x-blockdev-reopen', **new_base_opts) | ||
381 | if result != {'return': {}}: | ||
382 | log('Failed to reopen: ' + str(result)) | ||
383 | -- | ||
384 | 2.31.1 | ||
385 | |||
386 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
1 | 2 | ||
3 | This test swaps the images used by two active block devices. | ||
4 | |||
5 | This is now possible thanks to the new ability to run | ||
6 | x-blockdev-reopen on multiple devices at the same time. | ||
7 | |||
8 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
11 | Message-Id: <20210708114709.206487-6-kwolf@redhat.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | tests/qemu-iotests/245 | 47 ++++++++++++++++++++++++++++++++++++++ | ||
15 | tests/qemu-iotests/245.out | 4 ++-- | ||
16 | 2 files changed, 49 insertions(+), 2 deletions(-) | ||
17 | |||
18 | diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 | ||
19 | index XXXXXXX..XXXXXXX 100755 | ||
20 | --- a/tests/qemu-iotests/245 | ||
21 | +++ b/tests/qemu-iotests/245 | ||
22 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
23 | '-c', 'read -P 0x40 0x40008 1', | ||
24 | '-c', 'read -P 0x80 0x40010 1', hd_path[0]) | ||
25 | |||
26 | + # Swap the disk images of two active block devices | ||
27 | + def test_swap_files(self): | ||
28 | + # Add hd0 and hd2 (none of them with backing files) | ||
29 | + opts0 = hd_opts(0) | ||
30 | + result = self.vm.qmp('blockdev-add', conv_keys = False, **opts0) | ||
31 | + self.assert_qmp(result, 'return', {}) | ||
32 | + | ||
33 | + opts2 = hd_opts(2) | ||
34 | + result = self.vm.qmp('blockdev-add', conv_keys = False, **opts2) | ||
35 | + self.assert_qmp(result, 'return', {}) | ||
36 | + | ||
37 | + # Write different data to both block devices | ||
38 | + self.run_qemu_io("hd0", "write -P 0xa0 0 1k") | ||
39 | + self.run_qemu_io("hd2", "write -P 0xa2 0 1k") | ||
40 | + | ||
41 | + # Check that the data reads correctly | ||
42 | + self.run_qemu_io("hd0", "read -P 0xa0 0 1k") | ||
43 | + self.run_qemu_io("hd2", "read -P 0xa2 0 1k") | ||
44 | + | ||
45 | + # It's not possible to make a block device use an image that | ||
46 | + # is already being used by the other device. | ||
47 | + self.reopen(opts0, {'file': 'hd2-file'}, | ||
48 | + "Permission conflict on node 'hd2-file': permissions " | ||
49 | + "'write, resize' are both required by node 'hd2' (uses " | ||
50 | + "node 'hd2-file' as 'file' child) and unshared by node " | ||
51 | + "'hd0' (uses node 'hd2-file' as 'file' child).") | ||
52 | + self.reopen(opts2, {'file': 'hd0-file'}, | ||
53 | + "Permission conflict on node 'hd0-file': permissions " | ||
54 | + "'write, resize' are both required by node 'hd0' (uses " | ||
55 | + "node 'hd0-file' as 'file' child) and unshared by node " | ||
56 | + "'hd2' (uses node 'hd0-file' as 'file' child).") | ||
57 | + | ||
58 | + # But we can swap the images if we reopen both devices at the | ||
59 | + # same time | ||
60 | + opts0['file'] = 'hd2-file' | ||
61 | + opts2['file'] = 'hd0-file' | ||
62 | + self.reopenMultiple([opts0, opts2]) | ||
63 | + self.run_qemu_io("hd0", "read -P 0xa2 0 1k") | ||
64 | + self.run_qemu_io("hd2", "read -P 0xa0 0 1k") | ||
65 | + | ||
66 | + # And we can of course come back to the original state | ||
67 | + opts0['file'] = 'hd0-file' | ||
68 | + opts2['file'] = 'hd2-file' | ||
69 | + self.reopenMultiple([opts0, opts2]) | ||
70 | + self.run_qemu_io("hd0", "read -P 0xa0 0 1k") | ||
71 | + self.run_qemu_io("hd2", "read -P 0xa2 0 1k") | ||
72 | + | ||
73 | # Misc reopen tests with different block drivers | ||
74 | @iotests.skip_if_unsupported(['quorum', 'throttle']) | ||
75 | def test_misc_drivers(self): | ||
76 | diff --git a/tests/qemu-iotests/245.out b/tests/qemu-iotests/245.out | ||
77 | index XXXXXXX..XXXXXXX 100644 | ||
78 | --- a/tests/qemu-iotests/245.out | ||
79 | +++ b/tests/qemu-iotests/245.out | ||
80 | @@ -XXX,XX +XXX,XX @@ read 1/1 bytes at offset 262152 | ||
81 | read 1/1 bytes at offset 262160 | ||
82 | 1 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
83 | |||
84 | -.............. | ||
85 | +............... | ||
86 | ---------------------------------------------------------------------- | ||
87 | -Ran 24 tests | ||
88 | +Ran 25 tests | ||
89 | |||
90 | OK | ||
91 | -- | ||
92 | 2.31.1 | ||
93 | |||
94 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Alberto Garcia <berto@igalia.com> | |
2 | |||
3 | This patch drops the 'x-' prefix from x-blockdev-reopen. | ||
4 | |||
5 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
6 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Message-Id: <20210708114709.206487-7-kwolf@redhat.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | qapi/block-core.json | 6 +++--- | ||
12 | blockdev.c | 2 +- | ||
13 | tests/qemu-iotests/155 | 2 +- | ||
14 | tests/qemu-iotests/165 | 2 +- | ||
15 | tests/qemu-iotests/245 | 10 +++++----- | ||
16 | tests/qemu-iotests/248 | 2 +- | ||
17 | tests/qemu-iotests/248.out | 2 +- | ||
18 | tests/qemu-iotests/296 | 2 +- | ||
19 | tests/qemu-iotests/298 | 2 +- | ||
20 | tests/qemu-iotests/tests/remove-bitmap-from-backing | 4 ++-- | ||
21 | 10 files changed, 17 insertions(+), 17 deletions(-) | ||
22 | |||
23 | diff --git a/qapi/block-core.json b/qapi/block-core.json | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/qapi/block-core.json | ||
26 | +++ b/qapi/block-core.json | ||
27 | @@ -XXX,XX +XXX,XX @@ | ||
28 | { 'command': 'blockdev-add', 'data': 'BlockdevOptions', 'boxed': true } | ||
29 | |||
30 | ## | ||
31 | -# @x-blockdev-reopen: | ||
32 | +# @blockdev-reopen: | ||
33 | # | ||
34 | # Reopens one or more block devices using the given set of options. | ||
35 | # Any option not specified will be reset to its default value regardless | ||
36 | @@ -XXX,XX +XXX,XX @@ | ||
37 | # image does not have a default backing file name as part of its | ||
38 | # metadata. | ||
39 | # | ||
40 | -# Since: 4.0 | ||
41 | +# Since: 6.1 | ||
42 | ## | ||
43 | -{ 'command': 'x-blockdev-reopen', | ||
44 | +{ 'command': 'blockdev-reopen', | ||
45 | 'data': { 'options': ['BlockdevOptions'] } } | ||
46 | |||
47 | ## | ||
48 | diff --git a/blockdev.c b/blockdev.c | ||
49 | index XXXXXXX..XXXXXXX 100644 | ||
50 | --- a/blockdev.c | ||
51 | +++ b/blockdev.c | ||
52 | @@ -XXX,XX +XXX,XX @@ fail: | ||
53 | visit_free(v); | ||
54 | } | ||
55 | |||
56 | -void qmp_x_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp) | ||
57 | +void qmp_blockdev_reopen(BlockdevOptionsList *reopen_list, Error **errp) | ||
58 | { | ||
59 | BlockReopenQueue *queue = NULL; | ||
60 | GSList *drained = NULL; | ||
61 | diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155 | ||
62 | index XXXXXXX..XXXXXXX 100755 | ||
63 | --- a/tests/qemu-iotests/155 | ||
64 | +++ b/tests/qemu-iotests/155 | ||
65 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevMirrorReopen(MirrorBaseClass): | ||
66 | result = self.vm.qmp('blockdev-add', node_name="backing", | ||
67 | driver="null-co") | ||
68 | self.assert_qmp(result, 'return', {}) | ||
69 | - result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
70 | + result = self.vm.qmp('blockdev-reopen', options=[{ | ||
71 | 'node-name': "target", | ||
72 | 'driver': iotests.imgfmt, | ||
73 | 'file': "target-file", | ||
74 | diff --git a/tests/qemu-iotests/165 b/tests/qemu-iotests/165 | ||
75 | index XXXXXXX..XXXXXXX 100755 | ||
76 | --- a/tests/qemu-iotests/165 | ||
77 | +++ b/tests/qemu-iotests/165 | ||
78 | @@ -XXX,XX +XXX,XX @@ class TestPersistentDirtyBitmap(iotests.QMPTestCase): | ||
79 | assert sha256_1 == self.getSha256() | ||
80 | |||
81 | # Reopen to RW | ||
82 | - result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
83 | + result = self.vm.qmp('blockdev-reopen', options=[{ | ||
84 | 'node-name': 'node0', | ||
85 | 'driver': iotests.imgfmt, | ||
86 | 'file': { | ||
87 | diff --git a/tests/qemu-iotests/245 b/tests/qemu-iotests/245 | ||
88 | index XXXXXXX..XXXXXXX 100755 | ||
89 | --- a/tests/qemu-iotests/245 | ||
90 | +++ b/tests/qemu-iotests/245 | ||
91 | @@ -XXX,XX +XXX,XX @@ | ||
92 | #!/usr/bin/env python3 | ||
93 | # group: rw | ||
94 | # | ||
95 | -# Test cases for the QMP 'x-blockdev-reopen' command | ||
96 | +# Test cases for the QMP 'blockdev-reopen' command | ||
97 | # | ||
98 | # Copyright (C) 2018-2019 Igalia, S.L. | ||
99 | # Author: Alberto Garcia <berto@igalia.com> | ||
100 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
101 | "Expected output of %d qemu-io commands, found %d" % | ||
102 | (found, self.total_io_cmds)) | ||
103 | |||
104 | - # Run x-blockdev-reopen on a list of block devices | ||
105 | + # Run blockdev-reopen on a list of block devices | ||
106 | def reopenMultiple(self, opts, errmsg = None): | ||
107 | - result = self.vm.qmp('x-blockdev-reopen', conv_keys=False, options=opts) | ||
108 | + result = self.vm.qmp('blockdev-reopen', conv_keys=False, options=opts) | ||
109 | if errmsg: | ||
110 | self.assert_qmp(result, 'error/class', 'GenericError') | ||
111 | self.assert_qmp(result, 'error/desc', errmsg) | ||
112 | else: | ||
113 | self.assert_qmp(result, 'return', {}) | ||
114 | |||
115 | - # Run x-blockdev-reopen on a single block device (specified by | ||
116 | + # Run blockdev-reopen on a single block device (specified by | ||
117 | # 'opts') but applying 'newopts' on top of it. The original 'opts' | ||
118 | # dict is unmodified | ||
119 | def reopen(self, opts, newopts = {}, errmsg = None): | ||
120 | @@ -XXX,XX +XXX,XX @@ class TestBlockdevReopen(iotests.QMPTestCase): | ||
121 | self.reopen(opts, {'file.locking': 'off'}, "Cannot change the option 'locking'") | ||
122 | self.reopen(opts, {'file.filename': None}, "Invalid parameter type for 'options[0].file.filename', expected: string") | ||
123 | |||
124 | - # node-name is optional in BlockdevOptions, but x-blockdev-reopen needs it | ||
125 | + # node-name is optional in BlockdevOptions, but blockdev-reopen needs it | ||
126 | del opts['node-name'] | ||
127 | self.reopen(opts, {}, "node-name not specified") | ||
128 | |||
129 | diff --git a/tests/qemu-iotests/248 b/tests/qemu-iotests/248 | ||
130 | index XXXXXXX..XXXXXXX 100755 | ||
131 | --- a/tests/qemu-iotests/248 | ||
132 | +++ b/tests/qemu-iotests/248 | ||
133 | @@ -XXX,XX +XXX,XX @@ vm.event_wait('JOB_STATUS_CHANGE', timeout=3.0, | ||
134 | vm.get_qmp_events() | ||
135 | |||
136 | del blockdev_opts['file']['size'] | ||
137 | -vm.qmp_log('x-blockdev-reopen', filters=[filter_qmp_testfiles], | ||
138 | +vm.qmp_log('blockdev-reopen', filters=[filter_qmp_testfiles], | ||
139 | options = [ blockdev_opts ]) | ||
140 | |||
141 | vm.qmp_log('block-job-resume', device='drive0') | ||
142 | diff --git a/tests/qemu-iotests/248.out b/tests/qemu-iotests/248.out | ||
143 | index XXXXXXX..XXXXXXX 100644 | ||
144 | --- a/tests/qemu-iotests/248.out | ||
145 | +++ b/tests/qemu-iotests/248.out | ||
146 | @@ -XXX,XX +XXX,XX @@ | ||
147 | {"return": {}} | ||
148 | {"execute": "blockdev-mirror", "arguments": {"device": "drive0", "on-target-error": "enospc", "sync": "full", "target": "target"}} | ||
149 | {"return": {}} | ||
150 | -{"execute": "x-blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}} | ||
151 | +{"execute": "blockdev-reopen", "arguments": {"options": [{"driver": "qcow2", "file": {"driver": "raw", "file": {"driver": "file", "filename": "TEST_DIR/PID-target"}}, "node-name": "target"}]}} | ||
152 | {"return": {}} | ||
153 | {"execute": "block-job-resume", "arguments": {"device": "drive0"}} | ||
154 | {"return": {}} | ||
155 | diff --git a/tests/qemu-iotests/296 b/tests/qemu-iotests/296 | ||
156 | index XXXXXXX..XXXXXXX 100755 | ||
157 | --- a/tests/qemu-iotests/296 | ||
158 | +++ b/tests/qemu-iotests/296 | ||
159 | @@ -XXX,XX +XXX,XX @@ class EncryptionSetupTestCase(iotests.QMPTestCase): | ||
160 | def openImageQmp(self, vm, id, file, secret, | ||
161 | readOnly = False, reOpen = False): | ||
162 | |||
163 | - command = 'x-blockdev-reopen' if reOpen else 'blockdev-add' | ||
164 | + command = 'blockdev-reopen' if reOpen else 'blockdev-add' | ||
165 | |||
166 | opts = { | ||
167 | 'driver': iotests.imgfmt, | ||
168 | diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298 | ||
169 | index XXXXXXX..XXXXXXX 100755 | ||
170 | --- a/tests/qemu-iotests/298 | ||
171 | +++ b/tests/qemu-iotests/298 | ||
172 | @@ -XXX,XX +XXX,XX @@ class TestPreallocateFilter(TestPreallocateBase): | ||
173 | self.check_big() | ||
174 | |||
175 | def test_reopen_opts(self): | ||
176 | - result = self.vm.qmp('x-blockdev-reopen', options=[{ | ||
177 | + result = self.vm.qmp('blockdev-reopen', options=[{ | ||
178 | 'node-name': 'disk', | ||
179 | 'driver': iotests.imgfmt, | ||
180 | 'file': { | ||
181 | diff --git a/tests/qemu-iotests/tests/remove-bitmap-from-backing b/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
182 | index XXXXXXX..XXXXXXX 100755 | ||
183 | --- a/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
184 | +++ b/tests/qemu-iotests/tests/remove-bitmap-from-backing | ||
185 | @@ -XXX,XX +XXX,XX @@ new_base_opts = { | ||
186 | } | ||
187 | |||
188 | # Don't want to bother with filtering qmp_log for reopen command | ||
189 | -result = vm.qmp('x-blockdev-reopen', **new_base_opts) | ||
190 | +result = vm.qmp('blockdev-reopen', **new_base_opts) | ||
191 | if result != {'return': {}}: | ||
192 | log('Failed to reopen: ' + str(result)) | ||
193 | |||
194 | @@ -XXX,XX +XXX,XX @@ log('Remove persistent bitmap from base node reopened to RW:') | ||
195 | vm.qmp_log('block-dirty-bitmap-remove', node='base', name='bitmap0') | ||
196 | |||
197 | new_base_opts['options'][0]['read-only'] = True | ||
198 | -result = vm.qmp('x-blockdev-reopen', **new_base_opts) | ||
199 | +result = vm.qmp('blockdev-reopen', **new_base_opts) | ||
200 | if result != {'return': {}}: | ||
201 | log('Failed to reopen: ' + str(result)) | ||
202 | |||
203 | -- | ||
204 | 2.31.1 | ||
205 | |||
206 | diff view generated by jsdifflib |