1 | The following changes since commit bb9bf94b3e8926553290bc9a7cb84315af422086: | 1 | The following changes since commit 019fbfa4bcd2d3a835c241295e22ab2b5b56129b: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/ehabkost/tags/machine-next-pull-request' into staging (2018-12-11 19:18:58 +0000) | 3 | Merge tag 'pull-misc-2025-04-24' of https://repo.or.cz/qemu/armbru into staging (2025-04-24 13:44:57 -0400) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://repo.or.cz/qemu/kevin.git tags/for-upstream | 7 | https://repo.or.cz/qemu/kevin.git tags/for-upstream |
8 | 8 | ||
9 | for you to fetch changes up to 14a8882c0d6660684a0ec7e90cc75d4f38b07f0d: | 9 | for you to fetch changes up to 2b689db0bedd24eda8b491cb1fcfb015dfec5a31: |
10 | 10 | ||
11 | iotests: make 235 work on s390 (and others) (2018-12-12 12:22:31 +0100) | 11 | qemu-img: improve queue depth validation in img_bench (2025-04-25 18:09:04 +0200) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Block layer patches: | 14 | Block layer patches |
15 | 15 | ||
16 | - qcow2: Decompression worker threads | 16 | - Discard alignment fixes |
17 | - dmg: lzfse compression support | 17 | - Remove unused callback .bdrv_aio_pdiscard() |
18 | - file-posix: Simplify delegation to worker thread | 18 | - qemu-img bench: Input validation fix |
19 | - Don't pass flags to bdrv_reopen_queue() | ||
20 | - iotests: make 235 work on s390 (and others) | ||
21 | 19 | ||
22 | ---------------------------------------------------------------- | 20 | ---------------------------------------------------------------- |
23 | Alberto Garcia (15): | 21 | Denis Rastyogin (1): |
24 | block: Add bdrv_reopen_set_read_only() | 22 | qemu-img: improve queue depth validation in img_bench |
25 | block: Use bdrv_reopen_set_read_only() in bdrv_backing_update_filename() | ||
26 | block: Use bdrv_reopen_set_read_only() in commit_start/complete() | ||
27 | block: Use bdrv_reopen_set_read_only() in bdrv_commit() | ||
28 | block: Use bdrv_reopen_set_read_only() in stream_start/complete() | ||
29 | block: Use bdrv_reopen_set_read_only() in qmp_change_backing_file() | ||
30 | block: Use bdrv_reopen_set_read_only() in external_snapshot_commit() | ||
31 | block: Use bdrv_reopen_set_read_only() in the mirror driver | ||
32 | block: Drop bdrv_reopen() | ||
33 | qemu-io: Put flag changes in the options QDict in reopen_f() | ||
34 | block: Clean up reopen_backing_file() in block/replication.c | ||
35 | block: Remove flags parameter from bdrv_reopen_queue() | ||
36 | block: Stop passing flags to bdrv_reopen_queue_child() | ||
37 | block: Remove assertions from update_flags_from_options() | ||
38 | block: Assert that flags are up-to-date in bdrv_reopen_prepare() | ||
39 | 23 | ||
40 | Christian Borntraeger (1): | 24 | Stefan Hajnoczi (2): |
41 | iotests: make 235 work on s390 (and others) | 25 | file-posix: probe discard alignment on Linux block devices |
26 | block/io: skip head/tail requests on EINVAL | ||
42 | 27 | ||
43 | Julio Faracco (4): | 28 | Sunny Zhu (1): |
44 | block: adding lzfse decompressing support as a module. | 29 | block: Remove unused callback function *bdrv_aio_pdiscard |
45 | configure: adding support to lzfse library. | ||
46 | dmg: including dmg-lzfse module inside dmg block driver. | ||
47 | dmg: exchanging hardcoded dmg UDIF block types to enum. | ||
48 | 30 | ||
49 | Kevin Wolf (12): | 31 | include/block/block_int-common.h | 4 --- |
50 | file-posix: Reorganise RawPosixAIOData | 32 | block/file-posix.c | 67 +++++++++++++++++++++++++++++++++++++++- |
51 | file-posix: Factor out raw_thread_pool_submit() | 33 | block/io.c | 35 +++++++-------------- |
52 | file-posix: Avoid aio_worker() for QEMU_AIO_TRUNCATE | 34 | qemu-img.c | 2 +- |
53 | file-posix: Avoid aio_worker() for QEMU_AIO_COPY_RANGE | 35 | 4 files changed, 79 insertions(+), 29 deletions(-) |
54 | file-posix: Avoid aio_worker() for QEMU_AIO_WRITE_ZEROES | ||
55 | file-posix: Avoid aio_worker() for QEMU_AIO_DISCARD | ||
56 | file-posix: Avoid aio_worker() for QEMU_AIO_FLUSH | ||
57 | file-posix: Move read/write operation logic out of aio_worker() | ||
58 | file-posix: Avoid aio_worker() for QEMU_AIO_READ/WRITE | ||
59 | file-posix: Remove paio_submit_co() | ||
60 | file-posix: Switch to .bdrv_co_ioctl | ||
61 | file-posix: Avoid aio_worker() for QEMU_AIO_IOCTL | ||
62 | |||
63 | Vladimir Sementsov-Ogievskiy (9): | ||
64 | block/replication: drop extra synchronization | ||
65 | block/backup: drop unused synchronization interface | ||
66 | qcow2: use Z_OK instead of 0 for deflateInit2 return code check | ||
67 | qcow2: make more generic interface for qcow2_compress | ||
68 | qcow2: move decompression from qcow2-cluster.c to qcow2.c | ||
69 | qcow2: refactor decompress_buffer | ||
70 | qcow2: use byte-based read in qcow2_decompress_cluster | ||
71 | qcow2: aio support for compressed cluster read | ||
72 | qcow2: do decompression in threads | ||
73 | |||
74 | configure | 31 ++++ | ||
75 | block/dmg.h | 3 + | ||
76 | block/qcow2.h | 4 - | ||
77 | include/block/block.h | 6 +- | ||
78 | include/block/block_backup.h | 13 -- | ||
79 | include/scsi/pr-manager.h | 8 +- | ||
80 | block.c | 89 +++++----- | ||
81 | block/backup.c | 38 +---- | ||
82 | block/commit.c | 23 +-- | ||
83 | block/dmg-lzfse.c | 49 ++++++ | ||
84 | block/dmg.c | 65 ++++++-- | ||
85 | block/file-posix.c | 380 ++++++++++++++++++++++--------------------- | ||
86 | block/mirror.c | 19 ++- | ||
87 | block/qcow2-cluster.c | 70 -------- | ||
88 | block/qcow2.c | 170 ++++++++++++++++--- | ||
89 | block/replication.c | 67 +++----- | ||
90 | block/stream.c | 20 +-- | ||
91 | blockdev.c | 11 +- | ||
92 | qemu-io-cmds.c | 29 +++- | ||
93 | scsi/pr-manager.c | 21 +-- | ||
94 | block/Makefile.objs | 2 + | ||
95 | scsi/trace-events | 2 +- | ||
96 | tests/qemu-iotests/133 | 18 ++ | ||
97 | tests/qemu-iotests/133.out | 15 ++ | ||
98 | tests/qemu-iotests/235 | 4 +- | ||
99 | 25 files changed, 658 insertions(+), 499 deletions(-) | ||
100 | create mode 100644 block/dmg-lzfse.c | ||
101 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Julio Faracco <jcfaracco@gmail.com> | ||
2 | 1 | ||
3 | QEMU dmg support includes zlib and bzip2, but it does not contains lzfse | ||
4 | support. This commit adds the source file to extend compression support | ||
5 | for new DMGs. | ||
6 | |||
7 | Signed-off-by: Julio Faracco <jcfaracco@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/dmg-lzfse.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++ | ||
11 | 1 file changed, 49 insertions(+) | ||
12 | create mode 100644 block/dmg-lzfse.c | ||
13 | |||
14 | diff --git a/block/dmg-lzfse.c b/block/dmg-lzfse.c | ||
15 | new file mode 100644 | ||
16 | index XXXXXXX..XXXXXXX | ||
17 | --- /dev/null | ||
18 | +++ b/block/dmg-lzfse.c | ||
19 | @@ -XXX,XX +XXX,XX @@ | ||
20 | +/* | ||
21 | + * DMG lzfse uncompression | ||
22 | + * | ||
23 | + * Copyright (c) 2018 Julio Cesar Faracco | ||
24 | + * | ||
25 | + * Permission is hereby granted, free of charge, to any person obtaining a copy | ||
26 | + * of this software and associated documentation files (the "Software"), to deal | ||
27 | + * in the Software without restriction, including without limitation the rights | ||
28 | + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell | ||
29 | + * copies of the Software, and to permit persons to whom the Software is | ||
30 | + * furnished to do so, subject to the following conditions: | ||
31 | + * | ||
32 | + * The above copyright notice and this permission notice shall be included in | ||
33 | + * all copies or substantial portions of the Software. | ||
34 | + * | ||
35 | + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR | ||
36 | + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, | ||
37 | + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL | ||
38 | + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER | ||
39 | + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, | ||
40 | + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN | ||
41 | + * THE SOFTWARE. | ||
42 | + */ | ||
43 | +#include "qemu/osdep.h" | ||
44 | +#include "qemu-common.h" | ||
45 | +#include "dmg.h" | ||
46 | +#include <lzfse.h> | ||
47 | + | ||
48 | +static int dmg_uncompress_lzfse_do(char *next_in, unsigned int avail_in, | ||
49 | + char *next_out, unsigned int avail_out) | ||
50 | +{ | ||
51 | + size_t out_size = lzfse_decode_buffer((uint8_t *) next_out, avail_out, | ||
52 | + (uint8_t *) next_in, avail_in, | ||
53 | + NULL); | ||
54 | + | ||
55 | + /* We need to decode the single chunk only. */ | ||
56 | + /* So, out_size == avail_out is not an error here. */ | ||
57 | + if (out_size > 0) { | ||
58 | + return out_size; | ||
59 | + } | ||
60 | + return -1; | ||
61 | +} | ||
62 | + | ||
63 | +__attribute__((constructor)) | ||
64 | +static void dmg_lzfse_init(void) | ||
65 | +{ | ||
66 | + assert(!dmg_uncompress_lzfse); | ||
67 | + dmg_uncompress_lzfse = dmg_uncompress_lzfse_do; | ||
68 | +} | ||
69 | -- | ||
70 | 2.19.2 | ||
71 | |||
72 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Julio Faracco <jcfaracco@gmail.com> | ||
2 | 1 | ||
3 | This commit includes the support to lzfse opensource library. With this | ||
4 | library dmg block driver can decompress images with this type of | ||
5 | compression inside. | ||
6 | |||
7 | Signed-off-by: Julio Faracco <jcfaracco@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | configure | 31 +++++++++++++++++++++++++++++++ | ||
11 | block/Makefile.objs | 2 ++ | ||
12 | 2 files changed, 33 insertions(+) | ||
13 | |||
14 | diff --git a/configure b/configure | ||
15 | index XXXXXXX..XXXXXXX 100755 | ||
16 | --- a/configure | ||
17 | +++ b/configure | ||
18 | @@ -XXX,XX +XXX,XX @@ capstone="" | ||
19 | lzo="" | ||
20 | snappy="" | ||
21 | bzip2="" | ||
22 | +lzfse="" | ||
23 | guest_agent="" | ||
24 | guest_agent_with_vss="no" | ||
25 | guest_agent_ntddscsi="no" | ||
26 | @@ -XXX,XX +XXX,XX @@ for opt do | ||
27 | ;; | ||
28 | --enable-bzip2) bzip2="yes" | ||
29 | ;; | ||
30 | + --enable-lzfse) lzfse="yes" | ||
31 | + ;; | ||
32 | + --disable-lzfse) lzfse="no" | ||
33 | + ;; | ||
34 | --enable-guest-agent) guest_agent="yes" | ||
35 | ;; | ||
36 | --disable-guest-agent) guest_agent="no" | ||
37 | @@ -XXX,XX +XXX,XX @@ disabled with --disable-FEATURE, default is enabled if available: | ||
38 | snappy support of snappy compression library | ||
39 | bzip2 support of bzip2 compression library | ||
40 | (for reading bzip2-compressed dmg images) | ||
41 | + lzfse support of lzfse compression library | ||
42 | + (for reading lzfse-compressed dmg images) | ||
43 | seccomp seccomp support | ||
44 | coroutine-pool coroutine freelist (better performance) | ||
45 | glusterfs GlusterFS backend | ||
46 | @@ -XXX,XX +XXX,XX @@ EOF | ||
47 | fi | ||
48 | fi | ||
49 | |||
50 | +########################################## | ||
51 | +# lzfse check | ||
52 | + | ||
53 | +if test "$lzfse" != "no" ; then | ||
54 | + cat > $TMPC << EOF | ||
55 | +#include <lzfse.h> | ||
56 | +int main(void) { lzfse_decode_scratch_size(); return 0; } | ||
57 | +EOF | ||
58 | + if compile_prog "" "-llzfse" ; then | ||
59 | + lzfse="yes" | ||
60 | + else | ||
61 | + if test "$lzfse" = "yes"; then | ||
62 | + feature_not_found "lzfse" "Install lzfse devel" | ||
63 | + fi | ||
64 | + lzfse="no" | ||
65 | + fi | ||
66 | +fi | ||
67 | + | ||
68 | ########################################## | ||
69 | # libseccomp check | ||
70 | |||
71 | @@ -XXX,XX +XXX,XX @@ echo "Live block migration $live_block_migration" | ||
72 | echo "lzo support $lzo" | ||
73 | echo "snappy support $snappy" | ||
74 | echo "bzip2 support $bzip2" | ||
75 | +echo "lzfse support $lzfse" | ||
76 | echo "NUMA host support $numa" | ||
77 | echo "libxml2 $libxml2" | ||
78 | echo "tcmalloc support $tcmalloc" | ||
79 | @@ -XXX,XX +XXX,XX @@ if test "$bzip2" = "yes" ; then | ||
80 | echo "BZIP2_LIBS=-lbz2" >> $config_host_mak | ||
81 | fi | ||
82 | |||
83 | +if test "$lzfse" = "yes" ; then | ||
84 | + echo "CONFIG_LZFSE=y" >> $config_host_mak | ||
85 | + echo "LZFSE_LIBS=-llzfse" >> $config_host_mak | ||
86 | +fi | ||
87 | + | ||
88 | if test "$libiscsi" = "yes" ; then | ||
89 | echo "CONFIG_LIBISCSI=m" >> $config_host_mak | ||
90 | echo "LIBISCSI_CFLAGS=$libiscsi_cflags" >> $config_host_mak | ||
91 | diff --git a/block/Makefile.objs b/block/Makefile.objs | ||
92 | index XXXXXXX..XXXXXXX 100644 | ||
93 | --- a/block/Makefile.objs | ||
94 | +++ b/block/Makefile.objs | ||
95 | @@ -XXX,XX +XXX,XX @@ ssh.o-libs := $(LIBSSH2_LIBS) | ||
96 | block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o | ||
97 | block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y) | ||
98 | dmg-bz2.o-libs := $(BZIP2_LIBS) | ||
99 | +block-obj-$(if $(CONFIG_LZFSE),m,n) += dmg-lzfse.o | ||
100 | +dmg-lzfse.o-libs := $(LZFSE_LIBS) | ||
101 | qcow.o-libs := -lz | ||
102 | linux-aio.o-libs := -laio | ||
103 | parallels.o-cflags := $(LIBXML2_CFLAGS) | ||
104 | -- | ||
105 | 2.19.2 | ||
106 | |||
107 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Julio Faracco <jcfaracco@gmail.com> | ||
2 | 1 | ||
3 | This commit includes the support to new module dmg-lzfse into dmg block | ||
4 | driver. It includes the support for block type ULFO (0x80000007). | ||
5 | |||
6 | Signed-off-by: Julio Faracco <jcfaracco@gmail.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | --- | ||
9 | block/dmg.h | 3 +++ | ||
10 | block/dmg.c | 28 ++++++++++++++++++++++++++++ | ||
11 | 2 files changed, 31 insertions(+) | ||
12 | |||
13 | diff --git a/block/dmg.h b/block/dmg.h | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/dmg.h | ||
16 | +++ b/block/dmg.h | ||
17 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVDMGState { | ||
18 | extern int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in, | ||
19 | char *next_out, unsigned int avail_out); | ||
20 | |||
21 | +extern int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in, | ||
22 | + char *next_out, unsigned int avail_out); | ||
23 | + | ||
24 | #endif | ||
25 | diff --git a/block/dmg.c b/block/dmg.c | ||
26 | index XXXXXXX..XXXXXXX 100644 | ||
27 | --- a/block/dmg.c | ||
28 | +++ b/block/dmg.c | ||
29 | @@ -XXX,XX +XXX,XX @@ | ||
30 | int (*dmg_uncompress_bz2)(char *next_in, unsigned int avail_in, | ||
31 | char *next_out, unsigned int avail_out); | ||
32 | |||
33 | +int (*dmg_uncompress_lzfse)(char *next_in, unsigned int avail_in, | ||
34 | + char *next_out, unsigned int avail_out); | ||
35 | + | ||
36 | enum { | ||
37 | /* Limit chunk sizes to prevent unreasonable amounts of memory being used | ||
38 | * or truncating when converting to 32-bit types | ||
39 | @@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, | ||
40 | switch (s->types[chunk]) { | ||
41 | case 0x80000005: /* zlib compressed */ | ||
42 | case 0x80000006: /* bzip2 compressed */ | ||
43 | + case 0x80000007: /* lzfse compressed */ | ||
44 | compressed_size = s->lengths[chunk]; | ||
45 | uncompressed_sectors = s->sectorcounts[chunk]; | ||
46 | break; | ||
47 | @@ -XXX,XX +XXX,XX @@ static bool dmg_is_known_block_type(uint32_t entry_type) | ||
48 | return true; | ||
49 | case 0x80000006: /* bzip2 */ | ||
50 | return !!dmg_uncompress_bz2; | ||
51 | + case 0x80000007: /* lzfse */ | ||
52 | + return !!dmg_uncompress_lzfse; | ||
53 | default: | ||
54 | return false; | ||
55 | } | ||
56 | @@ -XXX,XX +XXX,XX @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags, | ||
57 | } | ||
58 | |||
59 | block_module_load_one("dmg-bz2"); | ||
60 | + block_module_load_one("dmg-lzfse"); | ||
61 | |||
62 | s->n_chunks = 0; | ||
63 | s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL; | ||
64 | @@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num) | ||
65 | return ret; | ||
66 | } | ||
67 | break; | ||
68 | + case 0x80000007: | ||
69 | + if (!dmg_uncompress_lzfse) { | ||
70 | + break; | ||
71 | + } | ||
72 | + /* we need to buffer, because only the chunk as whole can be | ||
73 | + * inflated. */ | ||
74 | + ret = bdrv_pread(bs->file, s->offsets[chunk], | ||
75 | + s->compressed_chunk, s->lengths[chunk]); | ||
76 | + if (ret != s->lengths[chunk]) { | ||
77 | + return -1; | ||
78 | + } | ||
79 | + | ||
80 | + ret = dmg_uncompress_lzfse((char *)s->compressed_chunk, | ||
81 | + (unsigned int) s->lengths[chunk], | ||
82 | + (char *)s->uncompressed_chunk, | ||
83 | + (unsigned int) | ||
84 | + (512 * s->sectorcounts[chunk])); | ||
85 | + if (ret < 0) { | ||
86 | + return ret; | ||
87 | + } | ||
88 | + break; | ||
89 | case 1: /* copy */ | ||
90 | ret = bdrv_pread(bs->file, s->offsets[chunk], | ||
91 | s->uncompressed_chunk, s->lengths[chunk]); | ||
92 | -- | ||
93 | 2.19.2 | ||
94 | |||
95 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Julio Faracco <jcfaracco@gmail.com> | ||
2 | 1 | ||
3 | This change is better to understand what kind of block type is being | ||
4 | handled by the code. Using a syntax similar to the DMG documentation is | ||
5 | easier than tracking all hex values assigned to a block type. | ||
6 | |||
7 | Signed-off-by: Julio Faracco <jcfaracco@gmail.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/dmg.c | 43 ++++++++++++++++++++++++++++--------------- | ||
11 | 1 file changed, 28 insertions(+), 15 deletions(-) | ||
12 | |||
13 | diff --git a/block/dmg.c b/block/dmg.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/dmg.c | ||
16 | +++ b/block/dmg.c | ||
17 | @@ -XXX,XX +XXX,XX @@ enum { | ||
18 | DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512, | ||
19 | }; | ||
20 | |||
21 | +enum { | ||
22 | + /* DMG Block Type */ | ||
23 | + UDZE = 0, /* Zeroes */ | ||
24 | + UDRW, /* RAW type */ | ||
25 | + UDIG, /* Ignore */ | ||
26 | + UDCO = 0x80000004, | ||
27 | + UDZO, | ||
28 | + UDBZ, | ||
29 | + ULFO, | ||
30 | + UDCM = 0x7ffffffe, /* Comments */ | ||
31 | + UDLE /* Last Entry */ | ||
32 | +}; | ||
33 | + | ||
34 | static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename) | ||
35 | { | ||
36 | int len; | ||
37 | @@ -XXX,XX +XXX,XX @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk, | ||
38 | uint32_t uncompressed_sectors = 0; | ||
39 | |||
40 | switch (s->types[chunk]) { | ||
41 | - case 0x80000005: /* zlib compressed */ | ||
42 | - case 0x80000006: /* bzip2 compressed */ | ||
43 | - case 0x80000007: /* lzfse compressed */ | ||
44 | + case UDZO: /* zlib compressed */ | ||
45 | + case UDBZ: /* bzip2 compressed */ | ||
46 | + case ULFO: /* lzfse compressed */ | ||
47 | compressed_size = s->lengths[chunk]; | ||
48 | uncompressed_sectors = s->sectorcounts[chunk]; | ||
49 | break; | ||
50 | - case 1: /* copy */ | ||
51 | + case UDRW: /* copy */ | ||
52 | uncompressed_sectors = DIV_ROUND_UP(s->lengths[chunk], 512); | ||
53 | break; | ||
54 | - case 2: /* zero */ | ||
55 | + case UDIG: /* zero */ | ||
56 | /* as the all-zeroes block may be large, it is treated specially: the | ||
57 | * sector is not copied from a large buffer, a simple memset is used | ||
58 | * instead. Therefore uncompressed_sectors does not need to be set. */ | ||
59 | @@ -XXX,XX +XXX,XX @@ typedef struct DmgHeaderState { | ||
60 | static bool dmg_is_known_block_type(uint32_t entry_type) | ||
61 | { | ||
62 | switch (entry_type) { | ||
63 | - case 0x00000001: /* uncompressed */ | ||
64 | - case 0x00000002: /* zeroes */ | ||
65 | - case 0x80000005: /* zlib */ | ||
66 | + case UDRW: /* uncompressed */ | ||
67 | + case UDIG: /* zeroes */ | ||
68 | + case UDZO: /* zlib */ | ||
69 | return true; | ||
70 | - case 0x80000006: /* bzip2 */ | ||
71 | + case UDBZ: /* bzip2 */ | ||
72 | return !!dmg_uncompress_bz2; | ||
73 | - case 0x80000007: /* lzfse */ | ||
74 | + case ULFO: /* lzfse */ | ||
75 | return !!dmg_uncompress_lzfse; | ||
76 | default: | ||
77 | return false; | ||
78 | @@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num) | ||
79 | |||
80 | s->current_chunk = s->n_chunks; | ||
81 | switch (s->types[chunk]) { /* block entry type */ | ||
82 | - case 0x80000005: { /* zlib compressed */ | ||
83 | + case UDZO: { /* zlib compressed */ | ||
84 | /* we need to buffer, because only the chunk as whole can be | ||
85 | * inflated. */ | ||
86 | ret = bdrv_pread(bs->file, s->offsets[chunk], | ||
87 | @@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num) | ||
88 | return -1; | ||
89 | } | ||
90 | break; } | ||
91 | - case 0x80000006: /* bzip2 compressed */ | ||
92 | + case UDBZ: /* bzip2 compressed */ | ||
93 | if (!dmg_uncompress_bz2) { | ||
94 | break; | ||
95 | } | ||
96 | @@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num) | ||
97 | return ret; | ||
98 | } | ||
99 | break; | ||
100 | - case 0x80000007: | ||
101 | + case ULFO: | ||
102 | if (!dmg_uncompress_lzfse) { | ||
103 | break; | ||
104 | } | ||
105 | @@ -XXX,XX +XXX,XX @@ static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num) | ||
106 | return ret; | ||
107 | } | ||
108 | break; | ||
109 | - case 1: /* copy */ | ||
110 | + case UDRW: /* copy */ | ||
111 | ret = bdrv_pread(bs->file, s->offsets[chunk], | ||
112 | s->uncompressed_chunk, s->lengths[chunk]); | ||
113 | if (ret != s->lengths[chunk]) { | ||
114 | return -1; | ||
115 | } | ||
116 | break; | ||
117 | - case 2: /* zero */ | ||
118 | + case UDIG: /* zero */ | ||
119 | /* see dmg_read, it is treated specially. No buffer needs to be | ||
120 | * pre-filled, the zeroes can be set directly. */ | ||
121 | break; | ||
122 | -- | ||
123 | 2.19.2 | ||
124 | |||
125 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | After commit f8d59dfb40 | ||
4 | "block/backup: fix fleecing scheme: use serialized writes" fleecing | ||
5 | (specifically reading from backup target, when backup source is in | ||
6 | backing chain of backup target) is safe, because all backup-job writes | ||
7 | to target are serialized. Therefore we don't need additional | ||
8 | synchronization for these reads. | ||
9 | |||
10 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | --- | ||
13 | block/replication.c | 24 +----------------------- | ||
14 | 1 file changed, 1 insertion(+), 23 deletions(-) | ||
15 | |||
16 | diff --git a/block/replication.c b/block/replication.c | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/block/replication.c | ||
19 | +++ b/block/replication.c | ||
20 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs, | ||
21 | QEMUIOVector *qiov) | ||
22 | { | ||
23 | BDRVReplicationState *s = bs->opaque; | ||
24 | - BdrvChild *child = s->secondary_disk; | ||
25 | - BlockJob *job = NULL; | ||
26 | - CowRequest req; | ||
27 | int ret; | ||
28 | |||
29 | if (s->mode == REPLICATION_MODE_PRIMARY) { | ||
30 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int replication_co_readv(BlockDriverState *bs, | ||
31 | return ret; | ||
32 | } | ||
33 | |||
34 | - if (child && child->bs) { | ||
35 | - job = child->bs->job; | ||
36 | - } | ||
37 | - | ||
38 | - if (job) { | ||
39 | - uint64_t remaining_bytes = remaining_sectors * BDRV_SECTOR_SIZE; | ||
40 | - | ||
41 | - backup_wait_for_overlapping_requests(child->bs->job, | ||
42 | - sector_num * BDRV_SECTOR_SIZE, | ||
43 | - remaining_bytes); | ||
44 | - backup_cow_request_begin(&req, child->bs->job, | ||
45 | - sector_num * BDRV_SECTOR_SIZE, | ||
46 | - remaining_bytes); | ||
47 | - ret = bdrv_co_preadv(bs->file, sector_num * BDRV_SECTOR_SIZE, | ||
48 | - remaining_bytes, qiov, 0); | ||
49 | - backup_cow_request_end(&req); | ||
50 | - goto out; | ||
51 | - } | ||
52 | - | ||
53 | ret = bdrv_co_preadv(bs->file, sector_num * BDRV_SECTOR_SIZE, | ||
54 | remaining_sectors * BDRV_SECTOR_SIZE, qiov, 0); | ||
55 | -out: | ||
56 | + | ||
57 | return replication_return_value(s, ret); | ||
58 | } | ||
59 | |||
60 | -- | ||
61 | 2.19.2 | ||
62 | |||
63 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
4 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
5 | --- | ||
6 | include/block/block_backup.h | 13 ------------ | ||
7 | block/backup.c | 38 +++++++----------------------------- | ||
8 | 2 files changed, 7 insertions(+), 44 deletions(-) | ||
9 | |||
10 | diff --git a/include/block/block_backup.h b/include/block/block_backup.h | ||
11 | index XXXXXXX..XXXXXXX 100644 | ||
12 | --- a/include/block/block_backup.h | ||
13 | +++ b/include/block/block_backup.h | ||
14 | @@ -XXX,XX +XXX,XX @@ | ||
15 | |||
16 | #include "block/block_int.h" | ||
17 | |||
18 | -typedef struct CowRequest { | ||
19 | - int64_t start_byte; | ||
20 | - int64_t end_byte; | ||
21 | - QLIST_ENTRY(CowRequest) list; | ||
22 | - CoQueue wait_queue; /* coroutines blocked on this request */ | ||
23 | -} CowRequest; | ||
24 | - | ||
25 | -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, | ||
26 | - uint64_t bytes); | ||
27 | -void backup_cow_request_begin(CowRequest *req, BlockJob *job, | ||
28 | - int64_t offset, uint64_t bytes); | ||
29 | -void backup_cow_request_end(CowRequest *req); | ||
30 | - | ||
31 | void backup_do_checkpoint(BlockJob *job, Error **errp); | ||
32 | |||
33 | #endif | ||
34 | diff --git a/block/backup.c b/block/backup.c | ||
35 | index XXXXXXX..XXXXXXX 100644 | ||
36 | --- a/block/backup.c | ||
37 | +++ b/block/backup.c | ||
38 | @@ -XXX,XX +XXX,XX @@ | ||
39 | |||
40 | #define BACKUP_CLUSTER_SIZE_DEFAULT (1 << 16) | ||
41 | |||
42 | +typedef struct CowRequest { | ||
43 | + int64_t start_byte; | ||
44 | + int64_t end_byte; | ||
45 | + QLIST_ENTRY(CowRequest) list; | ||
46 | + CoQueue wait_queue; /* coroutines blocked on this request */ | ||
47 | +} CowRequest; | ||
48 | + | ||
49 | typedef struct BackupBlockJob { | ||
50 | BlockJob common; | ||
51 | BlockBackend *target; | ||
52 | @@ -XXX,XX +XXX,XX @@ void backup_do_checkpoint(BlockJob *job, Error **errp) | ||
53 | hbitmap_set(backup_job->copy_bitmap, 0, len); | ||
54 | } | ||
55 | |||
56 | -void backup_wait_for_overlapping_requests(BlockJob *job, int64_t offset, | ||
57 | - uint64_t bytes) | ||
58 | -{ | ||
59 | - BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); | ||
60 | - int64_t start, end; | ||
61 | - | ||
62 | - assert(block_job_driver(job) == &backup_job_driver); | ||
63 | - | ||
64 | - start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); | ||
65 | - end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); | ||
66 | - wait_for_overlapping_requests(backup_job, start, end); | ||
67 | -} | ||
68 | - | ||
69 | -void backup_cow_request_begin(CowRequest *req, BlockJob *job, | ||
70 | - int64_t offset, uint64_t bytes) | ||
71 | -{ | ||
72 | - BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common); | ||
73 | - int64_t start, end; | ||
74 | - | ||
75 | - assert(block_job_driver(job) == &backup_job_driver); | ||
76 | - | ||
77 | - start = QEMU_ALIGN_DOWN(offset, backup_job->cluster_size); | ||
78 | - end = QEMU_ALIGN_UP(offset + bytes, backup_job->cluster_size); | ||
79 | - cow_request_begin(req, backup_job, start, end); | ||
80 | -} | ||
81 | - | ||
82 | -void backup_cow_request_end(CowRequest *req) | ||
83 | -{ | ||
84 | - cow_request_end(req); | ||
85 | -} | ||
86 | - | ||
87 | static void backup_drain(BlockJob *job) | ||
88 | { | ||
89 | BackupBlockJob *s = container_of(job, BackupBlockJob, common); | ||
90 | -- | ||
91 | 2.19.2 | ||
92 | |||
93 | diff view generated by jsdifflib |
1 | aio_worker() doesn't add anything interesting, it's only a useless | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | indirection. Call the handler function directly instead. | ||
3 | 2 | ||
4 | As we know that this handler function is only called from coroutine | 3 | Populate the pdiscard_alignment block limit so the block layer is able |
5 | context and the coroutine stays around until the worker thread finishes, | 4 | align discard requests correctly. |
6 | we can keep RawPosixAIOData on the stack. | ||
7 | 5 | ||
8 | This was the last user of aio_worker(), so the function goes away now. | 6 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
9 | 7 | Message-ID: <20250417150528.76470-2-stefanha@redhat.com> | |
8 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
11 | --- | 10 | --- |
12 | block/file-posix.c | 55 +++++++++++++--------------------------------- | 11 | block/file-posix.c | 67 +++++++++++++++++++++++++++++++++++++++++++++- |
13 | 1 file changed, 15 insertions(+), 40 deletions(-) | 12 | 1 file changed, 66 insertions(+), 1 deletion(-) |
14 | 13 | ||
15 | diff --git a/block/file-posix.c b/block/file-posix.c | 14 | diff --git a/block/file-posix.c b/block/file-posix.c |
16 | index XXXXXXX..XXXXXXX 100644 | 15 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/block/file-posix.c | 16 | --- a/block/file-posix.c |
18 | +++ b/block/file-posix.c | 17 | +++ b/block/file-posix.c |
19 | @@ -XXX,XX +XXX,XX @@ static int hdev_probe_geometry(BlockDriverState *bs, HDGeometry *geo) | 18 | @@ -XXX,XX +XXX,XX @@ static int get_sysfs_zoned_model(struct stat *st, BlockZoneModel *zoned) |
20 | } | 19 | } |
20 | #endif /* defined(CONFIG_BLKZONED) */ | ||
21 | |||
22 | +#ifdef CONFIG_LINUX | ||
23 | /* | ||
24 | * Get a sysfs attribute value as a long integer. | ||
25 | */ | ||
26 | -#ifdef CONFIG_LINUX | ||
27 | static long get_sysfs_long_val(struct stat *st, const char *attribute) | ||
28 | { | ||
29 | g_autofree char *str = NULL; | ||
30 | @@ -XXX,XX +XXX,XX @@ static long get_sysfs_long_val(struct stat *st, const char *attribute) | ||
31 | } | ||
32 | return ret; | ||
33 | } | ||
34 | + | ||
35 | +/* | ||
36 | + * Get a sysfs attribute value as a uint32_t. | ||
37 | + */ | ||
38 | +static int get_sysfs_u32_val(struct stat *st, const char *attribute, | ||
39 | + uint32_t *u32) | ||
40 | +{ | ||
41 | + g_autofree char *str = NULL; | ||
42 | + const char *end; | ||
43 | + unsigned int val; | ||
44 | + int ret; | ||
45 | + | ||
46 | + ret = get_sysfs_str_val(st, attribute, &str); | ||
47 | + if (ret < 0) { | ||
48 | + return ret; | ||
49 | + } | ||
50 | + | ||
51 | + /* The file is ended with '\n', pass 'end' to accept that. */ | ||
52 | + ret = qemu_strtoui(str, &end, 10, &val); | ||
53 | + if (ret == 0 && end && *end == '\0') { | ||
54 | + *u32 = val; | ||
55 | + } | ||
56 | + return ret; | ||
57 | +} | ||
21 | #endif | 58 | #endif |
22 | 59 | ||
23 | -static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) | 60 | static int hdev_get_max_segments(int fd, struct stat *st) |
24 | +static int handle_aiocb_ioctl(void *opaque) | 61 | @@ -XXX,XX +XXX,XX @@ static int hdev_get_max_segments(int fd, struct stat *st) |
25 | { | 62 | #endif |
26 | + RawPosixAIOData *aiocb = opaque; | ||
27 | int ret; | ||
28 | |||
29 | ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf); | ||
30 | @@ -XXX,XX +XXX,XX @@ out: | ||
31 | return result; | ||
32 | } | 63 | } |
33 | 64 | ||
34 | -static int aio_worker(void *arg) | 65 | +/* |
35 | -{ | 66 | + * Fills in *dalign with the discard alignment and returns 0 on success, |
36 | - RawPosixAIOData *aiocb = arg; | 67 | + * -errno otherwise. |
37 | - ssize_t ret = 0; | 68 | + */ |
38 | - | 69 | +static int hdev_get_pdiscard_alignment(struct stat *st, uint32_t *dalign) |
39 | - switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) { | 70 | +{ |
40 | - case QEMU_AIO_IOCTL: | 71 | +#ifdef CONFIG_LINUX |
41 | - ret = handle_aiocb_ioctl(aiocb); | 72 | + /* |
42 | - break; | 73 | + * Note that Linux "discard_granularity" is QEMU "discard_alignment". Linux |
43 | - case QEMU_AIO_READ: | 74 | + * "discard_alignment" is something else. |
44 | - case QEMU_AIO_WRITE: | 75 | + */ |
45 | - case QEMU_AIO_FLUSH: | 76 | + return get_sysfs_u32_val(st, "discard_granularity", dalign); |
46 | - case QEMU_AIO_DISCARD: | 77 | +#else |
47 | - case QEMU_AIO_WRITE_ZEROES: | 78 | + return -ENOTSUP; |
48 | - case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD: | 79 | +#endif |
49 | - case QEMU_AIO_COPY_RANGE: | 80 | +} |
50 | - case QEMU_AIO_TRUNCATE: | 81 | + |
51 | - g_assert_not_reached(); | 82 | #if defined(CONFIG_BLKZONED) |
52 | - default: | 83 | /* |
53 | - error_report("invalid aio request (0x%x)", aiocb->aio_type); | 84 | * If the reset_all flag is true, then the wps of zone whose state is |
54 | - ret = -EINVAL; | 85 | @@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) |
55 | - break; | ||
56 | - } | ||
57 | - | ||
58 | - g_free(aiocb); | ||
59 | - return ret; | ||
60 | -} | ||
61 | - | ||
62 | static int coroutine_fn raw_thread_pool_submit(BlockDriverState *bs, | ||
63 | ThreadPoolFunc func, void *arg) | ||
64 | { | ||
65 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn | ||
66 | hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) | ||
67 | { | ||
68 | BDRVRawState *s = bs->opaque; | ||
69 | - RawPosixAIOData *acb; | ||
70 | - ThreadPool *pool; | ||
71 | + RawPosixAIOData acb; | ||
72 | int ret; | ||
73 | |||
74 | ret = fd_open(bs); | ||
75 | @@ -XXX,XX +XXX,XX @@ hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) | ||
76 | } | 86 | } |
77 | } | 87 | } |
78 | 88 | ||
79 | - acb = g_new(RawPosixAIOData, 1); | 89 | + if (S_ISBLK(st.st_mode)) { |
80 | - acb->bs = bs; | 90 | + uint32_t dalign = 0; |
81 | - acb->aio_type = QEMU_AIO_IOCTL; | 91 | + int ret; |
82 | - acb->aio_fildes = s->fd; | ||
83 | - acb->aio_offset = 0; | ||
84 | - acb->ioctl.buf = buf; | ||
85 | - acb->ioctl.cmd = req; | ||
86 | - pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
87 | - return thread_pool_submit_co(pool, aio_worker, acb); | ||
88 | + acb = (RawPosixAIOData) { | ||
89 | + .bs = bs, | ||
90 | + .aio_type = QEMU_AIO_IOCTL, | ||
91 | + .aio_fildes = s->fd, | ||
92 | + .aio_offset = 0, | ||
93 | + .ioctl = { | ||
94 | + .buf = buf, | ||
95 | + .cmd = req, | ||
96 | + }, | ||
97 | + }; | ||
98 | + | 92 | + |
99 | + return raw_thread_pool_submit(bs, handle_aiocb_ioctl, &acb); | 93 | + ret = hdev_get_pdiscard_alignment(&st, &dalign); |
94 | + if (ret == 0) { | ||
95 | + uint32_t ralign = bs->bl.request_alignment; | ||
96 | + | ||
97 | + /* Probably never happens, but handle it just in case */ | ||
98 | + if (dalign < ralign && (ralign % dalign == 0)) { | ||
99 | + dalign = ralign; | ||
100 | + } | ||
101 | + | ||
102 | + /* The block layer requires a multiple of request_alignment */ | ||
103 | + if (dalign % ralign != 0) { | ||
104 | + error_setg(errp, "Invalid pdiscard_alignment limit %u is not a " | ||
105 | + "multiple of request_alignment %u", dalign, ralign); | ||
106 | + return; | ||
107 | + } | ||
108 | + | ||
109 | + bs->bl.pdiscard_alignment = dalign; | ||
110 | + } | ||
111 | + } | ||
112 | + | ||
113 | raw_refresh_zoned_limits(bs, &st, errp); | ||
100 | } | 114 | } |
101 | #endif /* linux */ | ||
102 | 115 | ||
103 | -- | 116 | -- |
104 | 2.19.2 | 117 | 2.49.0 |
105 | |||
106 | diff view generated by jsdifflib |
1 | From: Christian Borntraeger <borntraeger@de.ibm.com> | 1 | From: Stefan Hajnoczi <stefanha@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | "-machine pc" will not work all architectures. Lets fall back to the | 3 | When guests send misaligned discard requests, the block layer breaks |
4 | default machine by not specifying it. | 4 | them up into a misaligned head, an aligned main body, and a misaligned |
5 | tail. | ||
5 | 6 | ||
6 | In addition we also need to specify -no-shutdown on s390 as qemu will | 7 | The file-posix block driver on Linux returns -EINVAL on misaligned |
7 | exit otherwise. | 8 | discard requests. This causes bdrv_co_pdiscard() to fail and guests |
9 | configured with werror=stop will pause. | ||
8 | 10 | ||
9 | Cc: qemu-stable@nongnu.org | 11 | Add a special case for misaligned head/tail requests. Simply continue |
10 | Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> | 12 | when EINVAL is encountered so that the aligned main body of the request |
11 | Reviewed-by: Eric Blake <eblake@redhat.com> | 13 | can be completed and the guest is not paused. This is the best we can do |
14 | when guest discard limits do not match the host discard limits. | ||
15 | |||
16 | Fixes: https://issues.redhat.com/browse/RHEL-86032 | ||
17 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
18 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
19 | Message-ID: <20250417150528.76470-3-stefanha@redhat.com> | ||
20 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 21 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
13 | --- | 22 | --- |
14 | tests/qemu-iotests/235 | 4 +++- | 23 | block/io.c | 15 ++++++++++----- |
15 | 1 file changed, 3 insertions(+), 1 deletion(-) | 24 | 1 file changed, 10 insertions(+), 5 deletions(-) |
16 | 25 | ||
17 | diff --git a/tests/qemu-iotests/235 b/tests/qemu-iotests/235 | 26 | diff --git a/block/io.c b/block/io.c |
18 | index XXXXXXX..XXXXXXX 100755 | 27 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/tests/qemu-iotests/235 | 28 | --- a/block/io.c |
20 | +++ b/tests/qemu-iotests/235 | 29 | +++ b/block/io.c |
21 | @@ -XXX,XX +XXX,XX @@ qemu_img_create('-f', iotests.imgfmt, '-o', 'preallocation=metadata', disk, | 30 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, |
22 | str(size)) | 31 | /* Invalidate the cached block-status data range if this discard overlaps */ |
23 | 32 | bdrv_bsc_invalidate_range(bs, offset, bytes); | |
24 | vm = QEMUMachine(iotests.qemu_prog) | 33 | |
25 | -vm.add_args('-machine', 'pc,accel=kvm') | 34 | - /* Discard is advisory, but some devices track and coalesce |
26 | +vm.add_args('-machine', 'accel=kvm') | 35 | + /* |
27 | +if iotests.qemu_default_machine == 's390-ccw-virtio': | 36 | + * Discard is advisory, but some devices track and coalesce |
28 | + vm.add_args('-no-shutdown') | 37 | * unaligned requests, so we must pass everything down rather than |
29 | vm.add_args('-drive', 'id=src,file=' + disk) | 38 | - * round here. Still, most devices will just silently ignore |
30 | vm.launch() | 39 | - * unaligned requests (by returning -ENOTSUP), so we must fragment |
31 | 40 | - * the request accordingly. */ | |
41 | + * round here. Still, most devices reject unaligned requests with | ||
42 | + * -EINVAL or -ENOTSUP, so we must fragment the request accordingly. | ||
43 | + */ | ||
44 | align = MAX(bs->bl.pdiscard_alignment, bs->bl.request_alignment); | ||
45 | assert(align % bs->bl.request_alignment == 0); | ||
46 | head = offset % align; | ||
47 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, | ||
48 | } | ||
49 | } | ||
50 | if (ret && ret != -ENOTSUP) { | ||
51 | - goto out; | ||
52 | + if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { | ||
53 | + /* Silently skip rejected unaligned head/tail requests */ | ||
54 | + } else { | ||
55 | + goto out; /* bail out */ | ||
56 | + } | ||
57 | } | ||
58 | |||
59 | offset += num; | ||
32 | -- | 60 | -- |
33 | 2.19.2 | 61 | 2.49.0 |
34 | |||
35 | diff view generated by jsdifflib |
1 | From: Alberto Garcia <berto@igalia.com> | 1 | From: Sunny Zhu <sunnyzhyy@qq.com> |
---|---|---|---|
2 | 2 | ||
3 | This function takes four options (cache.direct, cache.no-flush, | 3 | The bytes type in *bdrv_aio_pdiscard should be int64_t rather than int. |
4 | read-only and auto-read-only) from a QemuOpts object and updates the | ||
5 | flags accordingly. | ||
6 | 4 | ||
7 | If any of those options is not set (because it was missing from the | 5 | There are no drivers implementing the *bdrv_aio_pdiscard() callback, |
8 | original QDict or because it had an invalid value) then the function | 6 | it appears to be an unused function. Therefore, we'll simply remove it |
9 | aborts with a failed assertion: | 7 | instead of fixing it. |
10 | 8 | ||
11 | $ qemu-io -c 'reopen -o read-only=foo' hd.qcow2 | 9 | Additionally, coroutine-based callbacks are preferred. If someone needs |
12 | block.c:1126: update_flags_from_options: Assertion `qemu_opt_find(opts, BDRV_OPT_CACHE_DIRECT)' failed. | 10 | to implement bdrv_aio_pdiscard, a coroutine-based version would be |
13 | Aborted | 11 | straightforward to implement. |
14 | 12 | ||
15 | This assertion is unnecessary, and it forces any caller of | 13 | Signed-off-by: Sunny Zhu <sunnyzhyy@qq.com> |
16 | bdrv_reopen() to pass all the aforementioned four options. This may | 14 | Message-ID: <tencent_7140D2E54157D98CF3D9E64B1A007A1A7906@qq.com> |
17 | have made sense in order to remove ambiguity when bdrv_reopen() was | 15 | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> |
18 | taking both flags and options, but that's not the case anymore. | 16 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> |
19 | |||
20 | It's also unnecessary if we want to validate the option values, | ||
21 | because bdrv_reopen_prepare() already takes care of that, as we can | ||
22 | see if we remove the assertions: | ||
23 | |||
24 | $ qemu-io -c 'reopen -o read-only=foo' hd.qcow2 | ||
25 | Parameter 'read-only' expects 'on' or 'off' | ||
26 | |||
27 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
28 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
29 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 17 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
30 | --- | 18 | --- |
31 | block.c | 4 ---- | 19 | include/block/block_int-common.h | 4 ---- |
32 | tests/qemu-iotests/133 | 9 +++++++++ | 20 | block/io.c | 22 +++------------------- |
33 | tests/qemu-iotests/133.out | 7 +++++++ | 21 | 2 files changed, 3 insertions(+), 23 deletions(-) |
34 | 3 files changed, 16 insertions(+), 4 deletions(-) | ||
35 | 22 | ||
36 | diff --git a/block.c b/block.c | 23 | diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h |
37 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
38 | --- a/block.c | 25 | --- a/include/block/block_int-common.h |
39 | +++ b/block.c | 26 | +++ b/include/block/block_int-common.h |
40 | @@ -XXX,XX +XXX,XX @@ static void update_flags_from_options(int *flags, QemuOpts *opts) | 27 | @@ -XXX,XX +XXX,XX @@ struct BlockDriver { |
41 | { | 28 | BlockAIOCB * GRAPH_RDLOCK_PTR (*bdrv_aio_flush)( |
42 | *flags &= ~(BDRV_O_CACHE_MASK | BDRV_O_RDWR | BDRV_O_AUTO_RDONLY); | 29 | BlockDriverState *bs, BlockCompletionFunc *cb, void *opaque); |
43 | 30 | ||
44 | - assert(qemu_opt_find(opts, BDRV_OPT_CACHE_NO_FLUSH)); | 31 | - BlockAIOCB * GRAPH_RDLOCK_PTR (*bdrv_aio_pdiscard)( |
45 | if (qemu_opt_get_bool_del(opts, BDRV_OPT_CACHE_NO_FLUSH, false)) { | 32 | - BlockDriverState *bs, int64_t offset, int bytes, |
46 | *flags |= BDRV_O_NO_FLUSH; | 33 | - BlockCompletionFunc *cb, void *opaque); |
34 | - | ||
35 | int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_readv)(BlockDriverState *bs, | ||
36 | int64_t sector_num, int nb_sectors, QEMUIOVector *qiov); | ||
37 | |||
38 | diff --git a/block/io.c b/block/io.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/block/io.c | ||
41 | +++ b/block/io.c | ||
42 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, | ||
43 | return 0; | ||
47 | } | 44 | } |
48 | 45 | ||
49 | - assert(qemu_opt_find(opts, BDRV_OPT_CACHE_DIRECT)); | 46 | - if (!bs->drv->bdrv_co_pdiscard && !bs->drv->bdrv_aio_pdiscard) { |
50 | if (qemu_opt_get_bool_del(opts, BDRV_OPT_CACHE_DIRECT, false)) { | 47 | + if (!bs->drv->bdrv_co_pdiscard) { |
51 | *flags |= BDRV_O_NOCACHE; | 48 | return 0; |
52 | } | 49 | } |
53 | 50 | ||
54 | - assert(qemu_opt_find(opts, BDRV_OPT_READ_ONLY)); | 51 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn bdrv_co_pdiscard(BdrvChild *child, int64_t offset, |
55 | if (!qemu_opt_get_bool_del(opts, BDRV_OPT_READ_ONLY, false)) { | 52 | ret = -ENOMEDIUM; |
56 | *flags |= BDRV_O_RDWR; | 53 | goto out; |
57 | } | 54 | } |
58 | 55 | - if (bs->drv->bdrv_co_pdiscard) { | |
59 | - assert(qemu_opt_find(opts, BDRV_OPT_AUTO_READ_ONLY)); | 56 | - ret = bs->drv->bdrv_co_pdiscard(bs, offset, num); |
60 | if (qemu_opt_get_bool_del(opts, BDRV_OPT_AUTO_READ_ONLY, false)) { | 57 | - } else { |
61 | *flags |= BDRV_O_AUTO_RDONLY; | 58 | - BlockAIOCB *acb; |
62 | } | 59 | - CoroutineIOCompletion co = { |
63 | diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133 | 60 | - .coroutine = qemu_coroutine_self(), |
64 | index XXXXXXX..XXXXXXX 100755 | 61 | - }; |
65 | --- a/tests/qemu-iotests/133 | 62 | - |
66 | +++ b/tests/qemu-iotests/133 | 63 | - acb = bs->drv->bdrv_aio_pdiscard(bs, offset, num, |
67 | @@ -XXX,XX +XXX,XX @@ $QEMU_IO -c 'reopen -w -o read-only=on' $TEST_IMG | 64 | - bdrv_co_io_em_complete, &co); |
68 | $QEMU_IO -c 'reopen -c none -o cache.direct=on' $TEST_IMG | 65 | - if (acb == NULL) { |
69 | $QEMU_IO -c 'reopen -c writeback -o cache.direct=on' $TEST_IMG | 66 | - ret = -EIO; |
70 | $QEMU_IO -c 'reopen -c directsync -o cache.no-flush=on' $TEST_IMG | 67 | - goto out; |
68 | - } else { | ||
69 | - qemu_coroutine_yield(); | ||
70 | - ret = co.ret; | ||
71 | - } | ||
72 | - } | ||
71 | + | 73 | + |
72 | +echo | 74 | + ret = bs->drv->bdrv_co_pdiscard(bs, offset, num); |
73 | +echo "=== Check that invalid options are handled correctly ===" | 75 | if (ret && ret != -ENOTSUP) { |
74 | +echo | 76 | if (ret == -EINVAL && (offset % align != 0 || num % align != 0)) { |
75 | + | 77 | /* Silently skip rejected unaligned head/tail requests */ |
76 | +$QEMU_IO -c 'reopen -o read-only=foo' $TEST_IMG | ||
77 | +$QEMU_IO -c 'reopen -o cache.no-flush=bar' $TEST_IMG | ||
78 | +$QEMU_IO -c 'reopen -o cache.direct=baz' $TEST_IMG | ||
79 | +$QEMU_IO -c 'reopen -o auto-read-only=qux' $TEST_IMG | ||
80 | # success, all done | ||
81 | echo "*** done" | ||
82 | rm -f $seq.full | ||
83 | diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out | ||
84 | index XXXXXXX..XXXXXXX 100644 | ||
85 | --- a/tests/qemu-iotests/133.out | ||
86 | +++ b/tests/qemu-iotests/133.out | ||
87 | @@ -XXX,XX +XXX,XX @@ Cannot set both -r/-w and 'read-only' | ||
88 | Cannot set both -c and the cache options | ||
89 | Cannot set both -c and the cache options | ||
90 | Cannot set both -c and the cache options | ||
91 | + | ||
92 | +=== Check that invalid options are handled correctly === | ||
93 | + | ||
94 | +Parameter 'read-only' expects 'on' or 'off' | ||
95 | +Parameter 'cache.no-flush' expects 'on' or 'off' | ||
96 | +Parameter 'cache.direct' expects 'on' or 'off' | ||
97 | +Parameter 'auto-read-only' expects 'on' or 'off' | ||
98 | *** done | ||
99 | -- | 78 | -- |
100 | 2.19.2 | 79 | 2.49.0 |
101 | 80 | ||
102 | 81 | diff view generated by jsdifflib |
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 1 | From: Denis Rastyogin <gerben@altlinux.org> |
---|---|---|---|
2 | 2 | ||
3 | Use appropriate macro, corresponding to deflateInit2 spec. | 3 | This error was discovered by fuzzing qemu-img. |
4 | 4 | ||
5 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 5 | Currently, running `qemu-img bench -d 0` in img_bench is allowed, |
6 | Reviewed-by: Alberto Garcia <berto@igalia.com> | 6 | which is a pointless operation and causes qemu-img to hang. |
7 | |||
8 | Signed-off-by: Denis Rastyogin <gerben@altlinux.org> | ||
9 | Message-ID: <20250327162423.25154-5-gerben@altlinux.org> | ||
10 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | 11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> |
8 | --- | 12 | --- |
9 | block/qcow2.c | 2 +- | 13 | qemu-img.c | 2 +- |
10 | 1 file changed, 1 insertion(+), 1 deletion(-) | 14 | 1 file changed, 1 insertion(+), 1 deletion(-) |
11 | 15 | ||
12 | diff --git a/block/qcow2.c b/block/qcow2.c | 16 | diff --git a/qemu-img.c b/qemu-img.c |
13 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/block/qcow2.c | 18 | --- a/qemu-img.c |
15 | +++ b/block/qcow2.c | 19 | +++ b/qemu-img.c |
16 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | 20 | @@ -XXX,XX +XXX,XX @@ static int img_bench(int argc, char **argv) |
17 | memset(&strm, 0, sizeof(strm)); | 21 | { |
18 | ret = deflateInit2(&strm, Z_DEFAULT_COMPRESSION, Z_DEFLATED, | 22 | unsigned long res; |
19 | -12, 9, Z_DEFAULT_STRATEGY); | 23 | |
20 | - if (ret != 0) { | 24 | - if (qemu_strtoul(optarg, NULL, 0, &res) < 0 || res > INT_MAX) { |
21 | + if (ret != Z_OK) { | 25 | + if (qemu_strtoul(optarg, NULL, 0, &res) <= 0 || res > INT_MAX) { |
22 | return -2; | 26 | error_report("Invalid queue depth specified"); |
23 | } | 27 | return 1; |
24 | 28 | } | |
25 | -- | 29 | -- |
26 | 2.19.2 | 30 | 2.49.0 |
27 | |||
28 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Give explicit size both for source and destination buffers, to make it | ||
4 | similar with decompression path and than cleanly reuse parameter | ||
5 | structure for decompression threads. | ||
6 | |||
7 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | block/qcow2.c | 30 ++++++++++++++++++------------ | ||
12 | 1 file changed, 18 insertions(+), 12 deletions(-) | ||
13 | |||
14 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/block/qcow2.c | ||
17 | +++ b/block/qcow2.c | ||
18 | @@ -XXX,XX +XXX,XX @@ fail: | ||
19 | /* | ||
20 | * qcow2_compress() | ||
21 | * | ||
22 | - * @dest - destination buffer, at least of @size-1 bytes | ||
23 | - * @src - source buffer, @size bytes | ||
24 | + * @dest - destination buffer, @dest_size bytes | ||
25 | + * @src - source buffer, @src_size bytes | ||
26 | * | ||
27 | * Returns: compressed size on success | ||
28 | - * -1 if compression is inefficient | ||
29 | + * -1 destination buffer is not enough to store compressed data | ||
30 | * -2 on any other error | ||
31 | */ | ||
32 | -static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | ||
33 | +static ssize_t qcow2_compress(void *dest, size_t dest_size, | ||
34 | + const void *src, size_t src_size) | ||
35 | { | ||
36 | ssize_t ret; | ||
37 | z_stream strm; | ||
38 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | ||
39 | |||
40 | /* strm.next_in is not const in old zlib versions, such as those used on | ||
41 | * OpenBSD/NetBSD, so cast the const away */ | ||
42 | - strm.avail_in = size; | ||
43 | + strm.avail_in = src_size; | ||
44 | strm.next_in = (void *) src; | ||
45 | - strm.avail_out = size - 1; | ||
46 | + strm.avail_out = dest_size; | ||
47 | strm.next_out = dest; | ||
48 | |||
49 | ret = deflate(&strm, Z_FINISH); | ||
50 | if (ret == Z_STREAM_END) { | ||
51 | - ret = size - 1 - strm.avail_out; | ||
52 | + ret = dest_size - strm.avail_out; | ||
53 | } else { | ||
54 | ret = (ret == Z_OK ? -1 : -2); | ||
55 | } | ||
56 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, const void *src, size_t size) | ||
57 | |||
58 | typedef struct Qcow2CompressData { | ||
59 | void *dest; | ||
60 | + size_t dest_size; | ||
61 | const void *src; | ||
62 | - size_t size; | ||
63 | + size_t src_size; | ||
64 | ssize_t ret; | ||
65 | } Qcow2CompressData; | ||
66 | |||
67 | @@ -XXX,XX +XXX,XX @@ static int qcow2_compress_pool_func(void *opaque) | ||
68 | { | ||
69 | Qcow2CompressData *data = opaque; | ||
70 | |||
71 | - data->ret = qcow2_compress(data->dest, data->src, data->size); | ||
72 | + data->ret = qcow2_compress(data->dest, data->dest_size, | ||
73 | + data->src, data->src_size); | ||
74 | |||
75 | return 0; | ||
76 | } | ||
77 | @@ -XXX,XX +XXX,XX @@ static void qcow2_compress_complete(void *opaque, int ret) | ||
78 | |||
79 | /* See qcow2_compress definition for parameters description */ | ||
80 | static ssize_t qcow2_co_compress(BlockDriverState *bs, | ||
81 | - void *dest, const void *src, size_t size) | ||
82 | + void *dest, size_t dest_size, | ||
83 | + const void *src, size_t src_size) | ||
84 | { | ||
85 | BDRVQcow2State *s = bs->opaque; | ||
86 | BlockAIOCB *acb; | ||
87 | ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
88 | Qcow2CompressData arg = { | ||
89 | .dest = dest, | ||
90 | + .dest_size = dest_size, | ||
91 | .src = src, | ||
92 | - .size = size, | ||
93 | + .src_size = src_size, | ||
94 | }; | ||
95 | |||
96 | while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) { | ||
97 | @@ -XXX,XX +XXX,XX @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, | ||
98 | |||
99 | out_buf = g_malloc(s->cluster_size); | ||
100 | |||
101 | - out_len = qcow2_co_compress(bs, out_buf, buf, s->cluster_size); | ||
102 | + out_len = qcow2_co_compress(bs, out_buf, s->cluster_size - 1, | ||
103 | + buf, s->cluster_size); | ||
104 | if (out_len == -2) { | ||
105 | ret = -EINVAL; | ||
106 | goto fail; | ||
107 | -- | ||
108 | 2.19.2 | ||
109 | |||
110 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Compression is done in threads in qcow2.c. We want to do decompression | ||
4 | in the same way, so, firstly, move it to the same file. | ||
5 | |||
6 | The only change is braces around if-body in decompress_buffer, to | ||
7 | satisfy checkpatch. | ||
8 | |||
9 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
10 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
11 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
12 | --- | ||
13 | block/qcow2-cluster.c | 70 ------------------------------------------ | ||
14 | block/qcow2.c | 71 +++++++++++++++++++++++++++++++++++++++++++ | ||
15 | 2 files changed, 71 insertions(+), 70 deletions(-) | ||
16 | |||
17 | diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/block/qcow2-cluster.c | ||
20 | +++ b/block/qcow2-cluster.c | ||
21 | @@ -XXX,XX +XXX,XX @@ again: | ||
22 | return 0; | ||
23 | } | ||
24 | |||
25 | -static int decompress_buffer(uint8_t *out_buf, int out_buf_size, | ||
26 | - const uint8_t *buf, int buf_size) | ||
27 | -{ | ||
28 | - z_stream strm1, *strm = &strm1; | ||
29 | - int ret, out_len; | ||
30 | - | ||
31 | - memset(strm, 0, sizeof(*strm)); | ||
32 | - | ||
33 | - strm->next_in = (uint8_t *)buf; | ||
34 | - strm->avail_in = buf_size; | ||
35 | - strm->next_out = out_buf; | ||
36 | - strm->avail_out = out_buf_size; | ||
37 | - | ||
38 | - ret = inflateInit2(strm, -12); | ||
39 | - if (ret != Z_OK) | ||
40 | - return -1; | ||
41 | - ret = inflate(strm, Z_FINISH); | ||
42 | - out_len = strm->next_out - out_buf; | ||
43 | - if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || | ||
44 | - out_len != out_buf_size) { | ||
45 | - inflateEnd(strm); | ||
46 | - return -1; | ||
47 | - } | ||
48 | - inflateEnd(strm); | ||
49 | - return 0; | ||
50 | -} | ||
51 | - | ||
52 | -int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
53 | -{ | ||
54 | - BDRVQcow2State *s = bs->opaque; | ||
55 | - int ret, csize, nb_csectors, sector_offset; | ||
56 | - uint64_t coffset; | ||
57 | - | ||
58 | - coffset = cluster_offset & s->cluster_offset_mask; | ||
59 | - if (s->cluster_cache_offset != coffset) { | ||
60 | - nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1; | ||
61 | - sector_offset = coffset & 511; | ||
62 | - csize = nb_csectors * 512 - sector_offset; | ||
63 | - | ||
64 | - /* Allocate buffers on first decompress operation, most images are | ||
65 | - * uncompressed and the memory overhead can be avoided. The buffers | ||
66 | - * are freed in .bdrv_close(). | ||
67 | - */ | ||
68 | - if (!s->cluster_data) { | ||
69 | - /* one more sector for decompressed data alignment */ | ||
70 | - s->cluster_data = qemu_try_blockalign(bs->file->bs, | ||
71 | - QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512); | ||
72 | - if (!s->cluster_data) { | ||
73 | - return -ENOMEM; | ||
74 | - } | ||
75 | - } | ||
76 | - if (!s->cluster_cache) { | ||
77 | - s->cluster_cache = g_malloc(s->cluster_size); | ||
78 | - } | ||
79 | - | ||
80 | - BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); | ||
81 | - ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data, | ||
82 | - nb_csectors); | ||
83 | - if (ret < 0) { | ||
84 | - return ret; | ||
85 | - } | ||
86 | - if (decompress_buffer(s->cluster_cache, s->cluster_size, | ||
87 | - s->cluster_data + sector_offset, csize) < 0) { | ||
88 | - return -EIO; | ||
89 | - } | ||
90 | - s->cluster_cache_offset = coffset; | ||
91 | - } | ||
92 | - return 0; | ||
93 | -} | ||
94 | - | ||
95 | /* | ||
96 | * This discards as many clusters of nb_clusters as possible at once (i.e. | ||
97 | * all clusters in the same L2 slice) and returns the number of discarded | ||
98 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
99 | index XXXXXXX..XXXXXXX 100644 | ||
100 | --- a/block/qcow2.c | ||
101 | +++ b/block/qcow2.c | ||
102 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, size_t dest_size, | ||
103 | return ret; | ||
104 | } | ||
105 | |||
106 | +static int decompress_buffer(uint8_t *out_buf, int out_buf_size, | ||
107 | + const uint8_t *buf, int buf_size) | ||
108 | +{ | ||
109 | + z_stream strm1, *strm = &strm1; | ||
110 | + int ret, out_len; | ||
111 | + | ||
112 | + memset(strm, 0, sizeof(*strm)); | ||
113 | + | ||
114 | + strm->next_in = (uint8_t *)buf; | ||
115 | + strm->avail_in = buf_size; | ||
116 | + strm->next_out = out_buf; | ||
117 | + strm->avail_out = out_buf_size; | ||
118 | + | ||
119 | + ret = inflateInit2(strm, -12); | ||
120 | + if (ret != Z_OK) { | ||
121 | + return -1; | ||
122 | + } | ||
123 | + ret = inflate(strm, Z_FINISH); | ||
124 | + out_len = strm->next_out - out_buf; | ||
125 | + if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || | ||
126 | + out_len != out_buf_size) { | ||
127 | + inflateEnd(strm); | ||
128 | + return -1; | ||
129 | + } | ||
130 | + inflateEnd(strm); | ||
131 | + return 0; | ||
132 | +} | ||
133 | + | ||
134 | #define MAX_COMPRESS_THREADS 4 | ||
135 | |||
136 | typedef struct Qcow2CompressData { | ||
137 | @@ -XXX,XX +XXX,XX @@ fail: | ||
138 | return ret; | ||
139 | } | ||
140 | |||
141 | +int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
142 | +{ | ||
143 | + BDRVQcow2State *s = bs->opaque; | ||
144 | + int ret, csize, nb_csectors, sector_offset; | ||
145 | + uint64_t coffset; | ||
146 | + | ||
147 | + coffset = cluster_offset & s->cluster_offset_mask; | ||
148 | + if (s->cluster_cache_offset != coffset) { | ||
149 | + nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1; | ||
150 | + sector_offset = coffset & 511; | ||
151 | + csize = nb_csectors * 512 - sector_offset; | ||
152 | + | ||
153 | + /* Allocate buffers on first decompress operation, most images are | ||
154 | + * uncompressed and the memory overhead can be avoided. The buffers | ||
155 | + * are freed in .bdrv_close(). | ||
156 | + */ | ||
157 | + if (!s->cluster_data) { | ||
158 | + /* one more sector for decompressed data alignment */ | ||
159 | + s->cluster_data = qemu_try_blockalign(bs->file->bs, | ||
160 | + QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512); | ||
161 | + if (!s->cluster_data) { | ||
162 | + return -ENOMEM; | ||
163 | + } | ||
164 | + } | ||
165 | + if (!s->cluster_cache) { | ||
166 | + s->cluster_cache = g_malloc(s->cluster_size); | ||
167 | + } | ||
168 | + | ||
169 | + BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); | ||
170 | + ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data, | ||
171 | + nb_csectors); | ||
172 | + if (ret < 0) { | ||
173 | + return ret; | ||
174 | + } | ||
175 | + if (decompress_buffer(s->cluster_cache, s->cluster_size, | ||
176 | + s->cluster_data + sector_offset, csize) < 0) { | ||
177 | + return -EIO; | ||
178 | + } | ||
179 | + s->cluster_cache_offset = coffset; | ||
180 | + } | ||
181 | + return 0; | ||
182 | +} | ||
183 | + | ||
184 | static int make_completely_empty(BlockDriverState *bs) | ||
185 | { | ||
186 | BDRVQcow2State *s = bs->opaque; | ||
187 | -- | ||
188 | 2.19.2 | ||
189 | |||
190 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | - make it look more like a pair of qcow2_compress - rename the function | ||
4 | and its parameters | ||
5 | - drop extra out_len variable, check filling of output buffer by strm | ||
6 | structure itself | ||
7 | - fix code style | ||
8 | - add some documentation | ||
9 | |||
10 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
11 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
12 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
13 | --- | ||
14 | block/qcow2.c | 56 ++++++++++++++++++++++++++++++++------------------- | ||
15 | 1 file changed, 35 insertions(+), 21 deletions(-) | ||
16 | |||
17 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
18 | index XXXXXXX..XXXXXXX 100644 | ||
19 | --- a/block/qcow2.c | ||
20 | +++ b/block/qcow2.c | ||
21 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_compress(void *dest, size_t dest_size, | ||
22 | return ret; | ||
23 | } | ||
24 | |||
25 | -static int decompress_buffer(uint8_t *out_buf, int out_buf_size, | ||
26 | - const uint8_t *buf, int buf_size) | ||
27 | +/* | ||
28 | + * qcow2_decompress() | ||
29 | + * | ||
30 | + * Decompress some data (not more than @src_size bytes) to produce exactly | ||
31 | + * @dest_size bytes. | ||
32 | + * | ||
33 | + * @dest - destination buffer, @dest_size bytes | ||
34 | + * @src - source buffer, @src_size bytes | ||
35 | + * | ||
36 | + * Returns: 0 on success | ||
37 | + * -1 on fail | ||
38 | + */ | ||
39 | +static ssize_t qcow2_decompress(void *dest, size_t dest_size, | ||
40 | + const void *src, size_t src_size) | ||
41 | { | ||
42 | - z_stream strm1, *strm = &strm1; | ||
43 | - int ret, out_len; | ||
44 | - | ||
45 | - memset(strm, 0, sizeof(*strm)); | ||
46 | + int ret = 0; | ||
47 | + z_stream strm; | ||
48 | |||
49 | - strm->next_in = (uint8_t *)buf; | ||
50 | - strm->avail_in = buf_size; | ||
51 | - strm->next_out = out_buf; | ||
52 | - strm->avail_out = out_buf_size; | ||
53 | + memset(&strm, 0, sizeof(strm)); | ||
54 | + strm.avail_in = src_size; | ||
55 | + strm.next_in = (void *) src; | ||
56 | + strm.avail_out = dest_size; | ||
57 | + strm.next_out = dest; | ||
58 | |||
59 | - ret = inflateInit2(strm, -12); | ||
60 | + ret = inflateInit2(&strm, -12); | ||
61 | if (ret != Z_OK) { | ||
62 | return -1; | ||
63 | } | ||
64 | - ret = inflate(strm, Z_FINISH); | ||
65 | - out_len = strm->next_out - out_buf; | ||
66 | - if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || | ||
67 | - out_len != out_buf_size) { | ||
68 | - inflateEnd(strm); | ||
69 | - return -1; | ||
70 | + | ||
71 | + ret = inflate(&strm, Z_FINISH); | ||
72 | + if ((ret != Z_STREAM_END && ret != Z_BUF_ERROR) || strm.avail_out != 0) { | ||
73 | + /* We approve Z_BUF_ERROR because we need @dest buffer to be filled, but | ||
74 | + * @src buffer may be processed partly (because in qcow2 we know size of | ||
75 | + * compressed data with precision of one sector) */ | ||
76 | + ret = -1; | ||
77 | } | ||
78 | - inflateEnd(strm); | ||
79 | - return 0; | ||
80 | + | ||
81 | + inflateEnd(&strm); | ||
82 | + | ||
83 | + return ret; | ||
84 | } | ||
85 | |||
86 | #define MAX_COMPRESS_THREADS 4 | ||
87 | @@ -XXX,XX +XXX,XX @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
88 | if (ret < 0) { | ||
89 | return ret; | ||
90 | } | ||
91 | - if (decompress_buffer(s->cluster_cache, s->cluster_size, | ||
92 | - s->cluster_data + sector_offset, csize) < 0) { | ||
93 | + if (qcow2_decompress(s->cluster_cache, s->cluster_size, | ||
94 | + s->cluster_data + sector_offset, csize) < 0) { | ||
95 | return -EIO; | ||
96 | } | ||
97 | s->cluster_cache_offset = coffset; | ||
98 | -- | ||
99 | 2.19.2 | ||
100 | |||
101 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | We are gradually moving away from sector-based interfaces, towards | ||
4 | byte-based. Get rid of it here too. | ||
5 | |||
6 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | --- | ||
9 | block/qcow2.c | 19 ++++++++++++------- | ||
10 | 1 file changed, 12 insertions(+), 7 deletions(-) | ||
11 | |||
12 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
13 | index XXXXXXX..XXXXXXX 100644 | ||
14 | --- a/block/qcow2.c | ||
15 | +++ b/block/qcow2.c | ||
16 | @@ -XXX,XX +XXX,XX @@ fail: | ||
17 | return ret; | ||
18 | } | ||
19 | |||
20 | -int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
21 | +int coroutine_fn | ||
22 | +qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
23 | { | ||
24 | BDRVQcow2State *s = bs->opaque; | ||
25 | - int ret, csize, nb_csectors, sector_offset; | ||
26 | + int ret, csize, nb_csectors; | ||
27 | uint64_t coffset; | ||
28 | + struct iovec iov; | ||
29 | + QEMUIOVector local_qiov; | ||
30 | |||
31 | coffset = cluster_offset & s->cluster_offset_mask; | ||
32 | if (s->cluster_cache_offset != coffset) { | ||
33 | nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1; | ||
34 | - sector_offset = coffset & 511; | ||
35 | - csize = nb_csectors * 512 - sector_offset; | ||
36 | + csize = nb_csectors * 512 - (coffset & 511); | ||
37 | |||
38 | /* Allocate buffers on first decompress operation, most images are | ||
39 | * uncompressed and the memory overhead can be avoided. The buffers | ||
40 | @@ -XXX,XX +XXX,XX @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
41 | s->cluster_cache = g_malloc(s->cluster_size); | ||
42 | } | ||
43 | |||
44 | + iov.iov_base = s->cluster_data; | ||
45 | + iov.iov_len = csize; | ||
46 | + qemu_iovec_init_external(&local_qiov, &iov, 1); | ||
47 | + | ||
48 | BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); | ||
49 | - ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data, | ||
50 | - nb_csectors); | ||
51 | + ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov, 0); | ||
52 | if (ret < 0) { | ||
53 | return ret; | ||
54 | } | ||
55 | if (qcow2_decompress(s->cluster_cache, s->cluster_size, | ||
56 | - s->cluster_data + sector_offset, csize) < 0) { | ||
57 | + s->cluster_data, csize) < 0) { | ||
58 | return -EIO; | ||
59 | } | ||
60 | s->cluster_cache_offset = coffset; | ||
61 | -- | ||
62 | 2.19.2 | ||
63 | |||
64 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Allocate buffers locally and release qcow2 lock. Than, reads inside | ||
4 | qcow2_co_preadv_compressed may be done in parallel, however all | ||
5 | decompression is still done synchronously. Let's improve it in the | ||
6 | following commit. | ||
7 | |||
8 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
9 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | block/qcow2.h | 4 --- | ||
13 | block/qcow2.c | 96 ++++++++++++++++++++++++++------------------------- | ||
14 | 2 files changed, 49 insertions(+), 51 deletions(-) | ||
15 | |||
16 | diff --git a/block/qcow2.h b/block/qcow2.h | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/block/qcow2.h | ||
19 | +++ b/block/qcow2.h | ||
20 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVQcow2State { | ||
21 | QEMUTimer *cache_clean_timer; | ||
22 | unsigned cache_clean_interval; | ||
23 | |||
24 | - uint8_t *cluster_cache; | ||
25 | - uint8_t *cluster_data; | ||
26 | - uint64_t cluster_cache_offset; | ||
27 | QLIST_HEAD(QCowClusterAlloc, QCowL2Meta) cluster_allocs; | ||
28 | |||
29 | uint64_t *refcount_table; | ||
30 | @@ -XXX,XX +XXX,XX @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, | ||
31 | bool exact_size); | ||
32 | int qcow2_shrink_l1_table(BlockDriverState *bs, uint64_t max_size); | ||
33 | int qcow2_write_l1_entry(BlockDriverState *bs, int l1_index); | ||
34 | -int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset); | ||
35 | int qcow2_encrypt_sectors(BDRVQcow2State *s, int64_t sector_num, | ||
36 | uint8_t *buf, int nb_sectors, bool enc, Error **errp); | ||
37 | |||
38 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/block/qcow2.c | ||
41 | +++ b/block/qcow2.c | ||
42 | @@ -XXX,XX +XXX,XX @@ typedef struct { | ||
43 | #define QCOW2_EXT_MAGIC_CRYPTO_HEADER 0x0537be77 | ||
44 | #define QCOW2_EXT_MAGIC_BITMAPS 0x23852875 | ||
45 | |||
46 | +static int coroutine_fn | ||
47 | +qcow2_co_preadv_compressed(BlockDriverState *bs, | ||
48 | + uint64_t file_cluster_offset, | ||
49 | + uint64_t offset, | ||
50 | + uint64_t bytes, | ||
51 | + QEMUIOVector *qiov); | ||
52 | + | ||
53 | static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename) | ||
54 | { | ||
55 | const QCowHeader *cow_header = (const void *)buf; | ||
56 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options, | ||
57 | goto fail; | ||
58 | } | ||
59 | |||
60 | - s->cluster_cache_offset = -1; | ||
61 | s->flags = flags; | ||
62 | |||
63 | ret = qcow2_refcount_init(bs); | ||
64 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_preadv(BlockDriverState *bs, uint64_t offset, | ||
65 | break; | ||
66 | |||
67 | case QCOW2_CLUSTER_COMPRESSED: | ||
68 | - /* add AIO support for compressed blocks ? */ | ||
69 | - ret = qcow2_decompress_cluster(bs, cluster_offset); | ||
70 | + qemu_co_mutex_unlock(&s->lock); | ||
71 | + ret = qcow2_co_preadv_compressed(bs, cluster_offset, | ||
72 | + offset, cur_bytes, | ||
73 | + &hd_qiov); | ||
74 | + qemu_co_mutex_lock(&s->lock); | ||
75 | if (ret < 0) { | ||
76 | goto fail; | ||
77 | } | ||
78 | |||
79 | - qemu_iovec_from_buf(&hd_qiov, 0, | ||
80 | - s->cluster_cache + offset_in_cluster, | ||
81 | - cur_bytes); | ||
82 | break; | ||
83 | |||
84 | case QCOW2_CLUSTER_NORMAL: | ||
85 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int qcow2_co_pwritev(BlockDriverState *bs, uint64_t offset, | ||
86 | |||
87 | qemu_iovec_init(&hd_qiov, qiov->niov); | ||
88 | |||
89 | - s->cluster_cache_offset = -1; /* disable compressed cache */ | ||
90 | - | ||
91 | qemu_co_mutex_lock(&s->lock); | ||
92 | |||
93 | while (bytes != 0) { | ||
94 | @@ -XXX,XX +XXX,XX @@ static void qcow2_close(BlockDriverState *bs) | ||
95 | g_free(s->image_backing_file); | ||
96 | g_free(s->image_backing_format); | ||
97 | |||
98 | - g_free(s->cluster_cache); | ||
99 | - qemu_vfree(s->cluster_data); | ||
100 | qcow2_refcount_close(bs); | ||
101 | qcow2_free_snapshots(bs); | ||
102 | } | ||
103 | @@ -XXX,XX +XXX,XX @@ qcow2_co_copy_range_to(BlockDriverState *bs, | ||
104 | QCowL2Meta *l2meta = NULL; | ||
105 | |||
106 | assert(!bs->encrypted); | ||
107 | - s->cluster_cache_offset = -1; /* disable compressed cache */ | ||
108 | |||
109 | qemu_co_mutex_lock(&s->lock); | ||
110 | |||
111 | @@ -XXX,XX +XXX,XX @@ fail: | ||
112 | return ret; | ||
113 | } | ||
114 | |||
115 | -int coroutine_fn | ||
116 | -qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset) | ||
117 | +static int coroutine_fn | ||
118 | +qcow2_co_preadv_compressed(BlockDriverState *bs, | ||
119 | + uint64_t file_cluster_offset, | ||
120 | + uint64_t offset, | ||
121 | + uint64_t bytes, | ||
122 | + QEMUIOVector *qiov) | ||
123 | { | ||
124 | BDRVQcow2State *s = bs->opaque; | ||
125 | - int ret, csize, nb_csectors; | ||
126 | + int ret = 0, csize, nb_csectors; | ||
127 | uint64_t coffset; | ||
128 | + uint8_t *buf, *out_buf; | ||
129 | struct iovec iov; | ||
130 | QEMUIOVector local_qiov; | ||
131 | + int offset_in_cluster = offset_into_cluster(s, offset); | ||
132 | |||
133 | - coffset = cluster_offset & s->cluster_offset_mask; | ||
134 | - if (s->cluster_cache_offset != coffset) { | ||
135 | - nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1; | ||
136 | - csize = nb_csectors * 512 - (coffset & 511); | ||
137 | + coffset = file_cluster_offset & s->cluster_offset_mask; | ||
138 | + nb_csectors = ((file_cluster_offset >> s->csize_shift) & s->csize_mask) + 1; | ||
139 | + csize = nb_csectors * 512 - (coffset & 511); | ||
140 | |||
141 | - /* Allocate buffers on first decompress operation, most images are | ||
142 | - * uncompressed and the memory overhead can be avoided. The buffers | ||
143 | - * are freed in .bdrv_close(). | ||
144 | - */ | ||
145 | - if (!s->cluster_data) { | ||
146 | - /* one more sector for decompressed data alignment */ | ||
147 | - s->cluster_data = qemu_try_blockalign(bs->file->bs, | ||
148 | - QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size + 512); | ||
149 | - if (!s->cluster_data) { | ||
150 | - return -ENOMEM; | ||
151 | - } | ||
152 | - } | ||
153 | - if (!s->cluster_cache) { | ||
154 | - s->cluster_cache = g_malloc(s->cluster_size); | ||
155 | - } | ||
156 | + buf = g_try_malloc(csize); | ||
157 | + if (!buf) { | ||
158 | + return -ENOMEM; | ||
159 | + } | ||
160 | + iov.iov_base = buf; | ||
161 | + iov.iov_len = csize; | ||
162 | + qemu_iovec_init_external(&local_qiov, &iov, 1); | ||
163 | |||
164 | - iov.iov_base = s->cluster_data; | ||
165 | - iov.iov_len = csize; | ||
166 | - qemu_iovec_init_external(&local_qiov, &iov, 1); | ||
167 | + out_buf = qemu_blockalign(bs, s->cluster_size); | ||
168 | |||
169 | - BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); | ||
170 | - ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov, 0); | ||
171 | - if (ret < 0) { | ||
172 | - return ret; | ||
173 | - } | ||
174 | - if (qcow2_decompress(s->cluster_cache, s->cluster_size, | ||
175 | - s->cluster_data, csize) < 0) { | ||
176 | - return -EIO; | ||
177 | - } | ||
178 | - s->cluster_cache_offset = coffset; | ||
179 | + BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED); | ||
180 | + ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov, 0); | ||
181 | + if (ret < 0) { | ||
182 | + goto fail; | ||
183 | } | ||
184 | - return 0; | ||
185 | + | ||
186 | + if (qcow2_decompress(out_buf, s->cluster_size, buf, csize) < 0) { | ||
187 | + ret = -EIO; | ||
188 | + goto fail; | ||
189 | + } | ||
190 | + | ||
191 | + qemu_iovec_from_buf(qiov, 0, out_buf + offset_in_cluster, bytes); | ||
192 | + | ||
193 | +fail: | ||
194 | + qemu_vfree(out_buf); | ||
195 | + g_free(buf); | ||
196 | + | ||
197 | + return ret; | ||
198 | } | ||
199 | |||
200 | static int make_completely_empty(BlockDriverState *bs) | ||
201 | -- | ||
202 | 2.19.2 | ||
203 | |||
204 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Do decompression in threads, like it is already done for compression. | ||
4 | This improves asynchronous compressed reads performance. | ||
5 | |||
6 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
7 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/qcow2.c | 34 +++++++++++++++++++++++++++------- | ||
11 | 1 file changed, 27 insertions(+), 7 deletions(-) | ||
12 | |||
13 | diff --git a/block/qcow2.c b/block/qcow2.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/qcow2.c | ||
16 | +++ b/block/qcow2.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_decompress(void *dest, size_t dest_size, | ||
18 | |||
19 | #define MAX_COMPRESS_THREADS 4 | ||
20 | |||
21 | +typedef ssize_t (*Qcow2CompressFunc)(void *dest, size_t dest_size, | ||
22 | + const void *src, size_t src_size); | ||
23 | typedef struct Qcow2CompressData { | ||
24 | void *dest; | ||
25 | size_t dest_size; | ||
26 | const void *src; | ||
27 | size_t src_size; | ||
28 | ssize_t ret; | ||
29 | + | ||
30 | + Qcow2CompressFunc func; | ||
31 | } Qcow2CompressData; | ||
32 | |||
33 | static int qcow2_compress_pool_func(void *opaque) | ||
34 | { | ||
35 | Qcow2CompressData *data = opaque; | ||
36 | |||
37 | - data->ret = qcow2_compress(data->dest, data->dest_size, | ||
38 | - data->src, data->src_size); | ||
39 | + data->ret = data->func(data->dest, data->dest_size, | ||
40 | + data->src, data->src_size); | ||
41 | |||
42 | return 0; | ||
43 | } | ||
44 | @@ -XXX,XX +XXX,XX @@ static void qcow2_compress_complete(void *opaque, int ret) | ||
45 | qemu_coroutine_enter(opaque); | ||
46 | } | ||
47 | |||
48 | -/* See qcow2_compress definition for parameters description */ | ||
49 | -static ssize_t qcow2_co_compress(BlockDriverState *bs, | ||
50 | - void *dest, size_t dest_size, | ||
51 | - const void *src, size_t src_size) | ||
52 | +static ssize_t coroutine_fn | ||
53 | +qcow2_co_do_compress(BlockDriverState *bs, void *dest, size_t dest_size, | ||
54 | + const void *src, size_t src_size, Qcow2CompressFunc func) | ||
55 | { | ||
56 | BDRVQcow2State *s = bs->opaque; | ||
57 | BlockAIOCB *acb; | ||
58 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_co_compress(BlockDriverState *bs, | ||
59 | .dest_size = dest_size, | ||
60 | .src = src, | ||
61 | .src_size = src_size, | ||
62 | + .func = func, | ||
63 | }; | ||
64 | |||
65 | while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) { | ||
66 | @@ -XXX,XX +XXX,XX @@ static ssize_t qcow2_co_compress(BlockDriverState *bs, | ||
67 | return arg.ret; | ||
68 | } | ||
69 | |||
70 | +static ssize_t coroutine_fn | ||
71 | +qcow2_co_compress(BlockDriverState *bs, void *dest, size_t dest_size, | ||
72 | + const void *src, size_t src_size) | ||
73 | +{ | ||
74 | + return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, | ||
75 | + qcow2_compress); | ||
76 | +} | ||
77 | + | ||
78 | +static ssize_t coroutine_fn | ||
79 | +qcow2_co_decompress(BlockDriverState *bs, void *dest, size_t dest_size, | ||
80 | + const void *src, size_t src_size) | ||
81 | +{ | ||
82 | + return qcow2_co_do_compress(bs, dest, dest_size, src, src_size, | ||
83 | + qcow2_decompress); | ||
84 | +} | ||
85 | + | ||
86 | /* XXX: put compressed sectors first, then all the cluster aligned | ||
87 | tables to avoid losing bytes in alignment */ | ||
88 | static coroutine_fn int | ||
89 | @@ -XXX,XX +XXX,XX @@ qcow2_co_preadv_compressed(BlockDriverState *bs, | ||
90 | goto fail; | ||
91 | } | ||
92 | |||
93 | - if (qcow2_decompress(out_buf, s->cluster_size, buf, csize) < 0) { | ||
94 | + if (qcow2_co_decompress(bs, out_buf, s->cluster_size, buf, csize) < 0) { | ||
95 | ret = -EIO; | ||
96 | goto fail; | ||
97 | } | ||
98 | -- | ||
99 | 2.19.2 | ||
100 | |||
101 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | RawPosixAIOData contains a lot of fields for several separate operations | ||
2 | that are to be processed in a worker thread and that need different | ||
3 | parameters. The struct is currently rather unorganised, with unions that | ||
4 | cover some, but not all operations, and even one #define for field names | ||
5 | instead of a union. | ||
6 | 1 | ||
7 | Clean this up to have some common fields and a single union. As a side | ||
8 | effect, on x86_64 the struct shrinks from 72 to 48 bytes. | ||
9 | |||
10 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
11 | --- | ||
12 | block/file-posix.c | 89 +++++++++++++++++++++++++--------------------- | ||
13 | 1 file changed, 49 insertions(+), 40 deletions(-) | ||
14 | |||
15 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/block/file-posix.c | ||
18 | +++ b/block/file-posix.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static int64_t raw_getlength(BlockDriverState *bs); | ||
20 | |||
21 | typedef struct RawPosixAIOData { | ||
22 | BlockDriverState *bs; | ||
23 | + int aio_type; | ||
24 | int aio_fildes; | ||
25 | - union { | ||
26 | - struct iovec *aio_iov; | ||
27 | - void *aio_ioctl_buf; | ||
28 | - }; | ||
29 | - int aio_niov; | ||
30 | - uint64_t aio_nbytes; | ||
31 | -#define aio_ioctl_cmd aio_nbytes /* for QEMU_AIO_IOCTL */ | ||
32 | + | ||
33 | off_t aio_offset; | ||
34 | - int aio_type; | ||
35 | + uint64_t aio_nbytes; | ||
36 | + | ||
37 | union { | ||
38 | + struct { | ||
39 | + struct iovec *iov; | ||
40 | + int niov; | ||
41 | + } io; | ||
42 | + struct { | ||
43 | + uint64_t cmd; | ||
44 | + void *buf; | ||
45 | + } ioctl; | ||
46 | struct { | ||
47 | int aio_fd2; | ||
48 | off_t aio_offset2; | ||
49 | - }; | ||
50 | + } copy_range; | ||
51 | struct { | ||
52 | PreallocMode prealloc; | ||
53 | Error **errp; | ||
54 | - }; | ||
55 | + } truncate; | ||
56 | }; | ||
57 | } RawPosixAIOData; | ||
58 | |||
59 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) | ||
60 | { | ||
61 | int ret; | ||
62 | |||
63 | - ret = ioctl(aiocb->aio_fildes, aiocb->aio_ioctl_cmd, aiocb->aio_ioctl_buf); | ||
64 | + ret = ioctl(aiocb->aio_fildes, aiocb->ioctl.cmd, aiocb->ioctl.buf); | ||
65 | if (ret == -1) { | ||
66 | return -errno; | ||
67 | } | ||
68 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw_vector(RawPosixAIOData *aiocb) | ||
69 | do { | ||
70 | if (aiocb->aio_type & QEMU_AIO_WRITE) | ||
71 | len = qemu_pwritev(aiocb->aio_fildes, | ||
72 | - aiocb->aio_iov, | ||
73 | - aiocb->aio_niov, | ||
74 | + aiocb->io.iov, | ||
75 | + aiocb->io.niov, | ||
76 | aiocb->aio_offset); | ||
77 | else | ||
78 | len = qemu_preadv(aiocb->aio_fildes, | ||
79 | - aiocb->aio_iov, | ||
80 | - aiocb->aio_niov, | ||
81 | + aiocb->io.iov, | ||
82 | + aiocb->io.niov, | ||
83 | aiocb->aio_offset); | ||
84 | } while (len == -1 && errno == EINTR); | ||
85 | |||
86 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
87 | * If there is just a single buffer, and it is properly aligned | ||
88 | * we can just use plain pread/pwrite without any problems. | ||
89 | */ | ||
90 | - if (aiocb->aio_niov == 1) { | ||
91 | - return handle_aiocb_rw_linear(aiocb, aiocb->aio_iov->iov_base); | ||
92 | + if (aiocb->io.niov == 1) { | ||
93 | + return handle_aiocb_rw_linear(aiocb, aiocb->io.iov->iov_base); | ||
94 | } | ||
95 | /* | ||
96 | * We have more than one iovec, and all are properly aligned. | ||
97 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
98 | char *p = buf; | ||
99 | int i; | ||
100 | |||
101 | - for (i = 0; i < aiocb->aio_niov; ++i) { | ||
102 | - memcpy(p, aiocb->aio_iov[i].iov_base, aiocb->aio_iov[i].iov_len); | ||
103 | - p += aiocb->aio_iov[i].iov_len; | ||
104 | + for (i = 0; i < aiocb->io.niov; ++i) { | ||
105 | + memcpy(p, aiocb->io.iov[i].iov_base, aiocb->io.iov[i].iov_len); | ||
106 | + p += aiocb->io.iov[i].iov_len; | ||
107 | } | ||
108 | assert(p - buf == aiocb->aio_nbytes); | ||
109 | } | ||
110 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
111 | size_t count = aiocb->aio_nbytes, copy; | ||
112 | int i; | ||
113 | |||
114 | - for (i = 0; i < aiocb->aio_niov && count; ++i) { | ||
115 | + for (i = 0; i < aiocb->io.niov && count; ++i) { | ||
116 | copy = count; | ||
117 | - if (copy > aiocb->aio_iov[i].iov_len) { | ||
118 | - copy = aiocb->aio_iov[i].iov_len; | ||
119 | + if (copy > aiocb->io.iov[i].iov_len) { | ||
120 | + copy = aiocb->io.iov[i].iov_len; | ||
121 | } | ||
122 | - memcpy(aiocb->aio_iov[i].iov_base, p, copy); | ||
123 | + memcpy(aiocb->io.iov[i].iov_base, p, copy); | ||
124 | assert(count >= copy); | ||
125 | p += copy; | ||
126 | count -= copy; | ||
127 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_copy_range(RawPosixAIOData *aiocb) | ||
128 | { | ||
129 | uint64_t bytes = aiocb->aio_nbytes; | ||
130 | off_t in_off = aiocb->aio_offset; | ||
131 | - off_t out_off = aiocb->aio_offset2; | ||
132 | + off_t out_off = aiocb->copy_range.aio_offset2; | ||
133 | |||
134 | while (bytes) { | ||
135 | ssize_t ret = copy_file_range(aiocb->aio_fildes, &in_off, | ||
136 | - aiocb->aio_fd2, &out_off, | ||
137 | + aiocb->copy_range.aio_fd2, &out_off, | ||
138 | bytes, 0); | ||
139 | trace_file_copy_file_range(aiocb->bs, aiocb->aio_fildes, in_off, | ||
140 | - aiocb->aio_fd2, out_off, bytes, 0, ret); | ||
141 | + aiocb->copy_range.aio_fd2, out_off, bytes, | ||
142 | + 0, ret); | ||
143 | if (ret == 0) { | ||
144 | /* No progress (e.g. when beyond EOF), let the caller fall back to | ||
145 | * buffer I/O. */ | ||
146 | @@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(RawPosixAIOData *aiocb) | ||
147 | struct stat st; | ||
148 | int fd = aiocb->aio_fildes; | ||
149 | int64_t offset = aiocb->aio_offset; | ||
150 | - Error **errp = aiocb->errp; | ||
151 | + PreallocMode prealloc = aiocb->truncate.prealloc; | ||
152 | + Error **errp = aiocb->truncate.errp; | ||
153 | |||
154 | if (fstat(fd, &st) < 0) { | ||
155 | result = -errno; | ||
156 | @@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(RawPosixAIOData *aiocb) | ||
157 | } | ||
158 | |||
159 | current_length = st.st_size; | ||
160 | - if (current_length > offset && aiocb->prealloc != PREALLOC_MODE_OFF) { | ||
161 | + if (current_length > offset && prealloc != PREALLOC_MODE_OFF) { | ||
162 | error_setg(errp, "Cannot use preallocation for shrinking files"); | ||
163 | return -ENOTSUP; | ||
164 | } | ||
165 | |||
166 | - switch (aiocb->prealloc) { | ||
167 | + switch (prealloc) { | ||
168 | #ifdef CONFIG_POSIX_FALLOCATE | ||
169 | case PREALLOC_MODE_FALLOC: | ||
170 | /* | ||
171 | @@ -XXX,XX +XXX,XX @@ static int handle_aiocb_truncate(RawPosixAIOData *aiocb) | ||
172 | default: | ||
173 | result = -ENOTSUP; | ||
174 | error_setg(errp, "Unsupported preallocation mode: %s", | ||
175 | - PreallocMode_str(aiocb->prealloc)); | ||
176 | + PreallocMode_str(prealloc)); | ||
177 | return result; | ||
178 | } | ||
179 | |||
180 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
181 | case QEMU_AIO_READ: | ||
182 | ret = handle_aiocb_rw(aiocb); | ||
183 | if (ret >= 0 && ret < aiocb->aio_nbytes) { | ||
184 | - iov_memset(aiocb->aio_iov, aiocb->aio_niov, ret, | ||
185 | + iov_memset(aiocb->io.iov, aiocb->io.niov, ret, | ||
186 | 0, aiocb->aio_nbytes - ret); | ||
187 | |||
188 | ret = aiocb->aio_nbytes; | ||
189 | @@ -XXX,XX +XXX,XX @@ static int paio_submit_co_full(BlockDriverState *bs, int fd, | ||
190 | acb->bs = bs; | ||
191 | acb->aio_type = type; | ||
192 | acb->aio_fildes = fd; | ||
193 | - acb->aio_fd2 = fd2; | ||
194 | - acb->aio_offset2 = offset2; | ||
195 | |||
196 | acb->aio_nbytes = bytes; | ||
197 | acb->aio_offset = offset; | ||
198 | |||
199 | if (qiov) { | ||
200 | - acb->aio_iov = qiov->iov; | ||
201 | - acb->aio_niov = qiov->niov; | ||
202 | + acb->io.iov = qiov->iov; | ||
203 | + acb->io.niov = qiov->niov; | ||
204 | assert(qiov->size == bytes); | ||
205 | + } else { | ||
206 | + acb->copy_range.aio_fd2 = fd2; | ||
207 | + acb->copy_range.aio_offset2 = offset2; | ||
208 | } | ||
209 | |||
210 | trace_file_paio_submit_co(offset, bytes, type); | ||
211 | @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
212 | .aio_fildes = fd, | ||
213 | .aio_type = QEMU_AIO_TRUNCATE, | ||
214 | .aio_offset = offset, | ||
215 | - .prealloc = prealloc, | ||
216 | - .errp = errp, | ||
217 | + .truncate = { | ||
218 | + .prealloc = prealloc, | ||
219 | + .errp = errp, | ||
220 | + }, | ||
221 | }; | ||
222 | |||
223 | /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ | ||
224 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs, | ||
225 | acb->aio_type = QEMU_AIO_IOCTL; | ||
226 | acb->aio_fildes = s->fd; | ||
227 | acb->aio_offset = 0; | ||
228 | - acb->aio_ioctl_buf = buf; | ||
229 | - acb->aio_ioctl_cmd = req; | ||
230 | + acb->ioctl.buf = buf; | ||
231 | + acb->ioctl.cmd = req; | ||
232 | pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
233 | return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque); | ||
234 | } | ||
235 | -- | ||
236 | 2.19.2 | ||
237 | |||
238 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | Getting the thread pool of the AioContext of a block node and scheduling | ||
2 | some work in it is an operation that is already done twice, and we'll | ||
3 | get more instances. Factor it out into a separate function. | ||
4 | 1 | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | --- | ||
7 | block/file-posix.c | 17 ++++++++++------- | ||
8 | 1 file changed, 10 insertions(+), 7 deletions(-) | ||
9 | |||
10 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
11 | index XXXXXXX..XXXXXXX 100644 | ||
12 | --- a/block/file-posix.c | ||
13 | +++ b/block/file-posix.c | ||
14 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
15 | return ret; | ||
16 | } | ||
17 | |||
18 | +static int coroutine_fn raw_thread_pool_submit(BlockDriverState *bs, | ||
19 | + ThreadPoolFunc func, void *arg) | ||
20 | +{ | ||
21 | + /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ | ||
22 | + ThreadPool *pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
23 | + return thread_pool_submit_co(pool, func, arg); | ||
24 | +} | ||
25 | + | ||
26 | static int paio_submit_co_full(BlockDriverState *bs, int fd, | ||
27 | int64_t offset, int fd2, int64_t offset2, | ||
28 | QEMUIOVector *qiov, | ||
29 | int bytes, int type) | ||
30 | { | ||
31 | RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); | ||
32 | - ThreadPool *pool; | ||
33 | |||
34 | acb->bs = bs; | ||
35 | acb->aio_type = type; | ||
36 | @@ -XXX,XX +XXX,XX @@ static int paio_submit_co_full(BlockDriverState *bs, int fd, | ||
37 | } | ||
38 | |||
39 | trace_file_paio_submit_co(offset, bytes, type); | ||
40 | - pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
41 | - return thread_pool_submit_co(pool, aio_worker, acb); | ||
42 | + return raw_thread_pool_submit(bs, aio_worker, acb); | ||
43 | } | ||
44 | |||
45 | static inline int paio_submit_co(BlockDriverState *bs, int fd, | ||
46 | @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
47 | PreallocMode prealloc, Error **errp) | ||
48 | { | ||
49 | RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); | ||
50 | - ThreadPool *pool; | ||
51 | |||
52 | *acb = (RawPosixAIOData) { | ||
53 | .bs = bs, | ||
54 | @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
55 | }, | ||
56 | }; | ||
57 | |||
58 | - /* @bs can be NULL, bdrv_get_aio_context() returns the main context then */ | ||
59 | - pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
60 | - return thread_pool_submit_co(pool, aio_worker, acb); | ||
61 | + return raw_thread_pool_submit(bs, aio_worker, acb); | ||
62 | } | ||
63 | |||
64 | static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, | ||
65 | -- | ||
66 | 2.19.2 | ||
67 | |||
68 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 12 ++++++------ | ||
11 | 1 file changed, 6 insertions(+), 6 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) | ||
18 | return ret; | ||
19 | } | ||
20 | |||
21 | -static int handle_aiocb_truncate(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_truncate(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | int result = 0; | ||
26 | int64_t current_length = 0; | ||
27 | char *buf = NULL; | ||
28 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
29 | ret = handle_aiocb_copy_range(aiocb); | ||
30 | break; | ||
31 | case QEMU_AIO_TRUNCATE: | ||
32 | - ret = handle_aiocb_truncate(aiocb); | ||
33 | - break; | ||
34 | + g_assert_not_reached(); | ||
35 | default: | ||
36 | error_report("invalid aio request (0x%x)", aiocb->aio_type); | ||
37 | ret = -EINVAL; | ||
38 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn | ||
39 | raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
40 | PreallocMode prealloc, Error **errp) | ||
41 | { | ||
42 | - RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); | ||
43 | + RawPosixAIOData acb; | ||
44 | |||
45 | - *acb = (RawPosixAIOData) { | ||
46 | + acb = (RawPosixAIOData) { | ||
47 | .bs = bs, | ||
48 | .aio_fildes = fd, | ||
49 | .aio_type = QEMU_AIO_TRUNCATE, | ||
50 | @@ -XXX,XX +XXX,XX @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset, | ||
51 | }, | ||
52 | }; | ||
53 | |||
54 | - return raw_thread_pool_submit(bs, aio_worker, acb); | ||
55 | + return raw_thread_pool_submit(bs, handle_aiocb_truncate, &acb); | ||
56 | } | ||
57 | |||
58 | static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset, | ||
59 | -- | ||
60 | 2.19.2 | ||
61 | |||
62 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 22 +++++++++++++++++----- | ||
11 | 1 file changed, 17 insertions(+), 5 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static off_t copy_file_range(int in_fd, off_t *in_off, int out_fd, | ||
18 | } | ||
19 | #endif | ||
20 | |||
21 | -static ssize_t handle_aiocb_copy_range(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_copy_range(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | uint64_t bytes = aiocb->aio_nbytes; | ||
26 | off_t in_off = aiocb->aio_offset; | ||
27 | off_t out_off = aiocb->copy_range.aio_offset2; | ||
28 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
29 | ret = handle_aiocb_write_zeroes_unmap(aiocb); | ||
30 | break; | ||
31 | case QEMU_AIO_COPY_RANGE: | ||
32 | - ret = handle_aiocb_copy_range(aiocb); | ||
33 | - break; | ||
34 | case QEMU_AIO_TRUNCATE: | ||
35 | g_assert_not_reached(); | ||
36 | default: | ||
37 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs, | ||
38 | BdrvRequestFlags read_flags, | ||
39 | BdrvRequestFlags write_flags) | ||
40 | { | ||
41 | + RawPosixAIOData acb; | ||
42 | BDRVRawState *s = bs->opaque; | ||
43 | BDRVRawState *src_s; | ||
44 | |||
45 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_copy_range_to(BlockDriverState *bs, | ||
46 | if (fd_open(src->bs) < 0 || fd_open(dst->bs) < 0) { | ||
47 | return -EIO; | ||
48 | } | ||
49 | - return paio_submit_co_full(bs, src_s->fd, src_offset, s->fd, dst_offset, | ||
50 | - NULL, bytes, QEMU_AIO_COPY_RANGE); | ||
51 | + | ||
52 | + acb = (RawPosixAIOData) { | ||
53 | + .bs = bs, | ||
54 | + .aio_type = QEMU_AIO_COPY_RANGE, | ||
55 | + .aio_fildes = src_s->fd, | ||
56 | + .aio_offset = src_offset, | ||
57 | + .aio_nbytes = bytes, | ||
58 | + .copy_range = { | ||
59 | + .aio_fd2 = s->fd, | ||
60 | + .aio_offset2 = dst_offset, | ||
61 | + }, | ||
62 | + }; | ||
63 | + | ||
64 | + return raw_thread_pool_submit(bs, handle_aiocb_copy_range, &acb); | ||
65 | } | ||
66 | |||
67 | BlockDriver bdrv_file = { | ||
68 | -- | ||
69 | 2.19.2 | ||
70 | |||
71 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 53 +++++++++++++++++++++++++++++----------------- | ||
11 | 1 file changed, 34 insertions(+), 19 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb) | ||
18 | return ret; | ||
19 | } | ||
20 | |||
21 | -static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_write_zeroes(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | #if defined(CONFIG_FALLOCATE) || defined(CONFIG_XFS) | ||
26 | BDRVRawState *s = aiocb->bs->opaque; | ||
27 | #endif | ||
28 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb) | ||
29 | return -ENOTSUP; | ||
30 | } | ||
31 | |||
32 | -static ssize_t handle_aiocb_write_zeroes_unmap(RawPosixAIOData *aiocb) | ||
33 | +static int handle_aiocb_write_zeroes_unmap(void *opaque) | ||
34 | { | ||
35 | + RawPosixAIOData *aiocb = opaque; | ||
36 | BDRVRawState *s G_GNUC_UNUSED = aiocb->bs->opaque; | ||
37 | int ret; | ||
38 | |||
39 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
40 | ret = handle_aiocb_discard(aiocb); | ||
41 | break; | ||
42 | case QEMU_AIO_WRITE_ZEROES: | ||
43 | - ret = handle_aiocb_write_zeroes(aiocb); | ||
44 | - break; | ||
45 | case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD: | ||
46 | - ret = handle_aiocb_write_zeroes_unmap(aiocb); | ||
47 | - break; | ||
48 | case QEMU_AIO_COPY_RANGE: | ||
49 | case QEMU_AIO_TRUNCATE: | ||
50 | g_assert_not_reached(); | ||
51 | @@ -XXX,XX +XXX,XX @@ raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) | ||
52 | return paio_submit_co(bs, s->fd, offset, NULL, bytes, QEMU_AIO_DISCARD); | ||
53 | } | ||
54 | |||
55 | -static int coroutine_fn raw_co_pwrite_zeroes( | ||
56 | - BlockDriverState *bs, int64_t offset, | ||
57 | - int bytes, BdrvRequestFlags flags) | ||
58 | +static int coroutine_fn | ||
59 | +raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int bytes, | ||
60 | + BdrvRequestFlags flags, bool blkdev) | ||
61 | { | ||
62 | BDRVRawState *s = bs->opaque; | ||
63 | - int operation = QEMU_AIO_WRITE_ZEROES; | ||
64 | + RawPosixAIOData acb; | ||
65 | + ThreadPoolFunc *handler; | ||
66 | + | ||
67 | + acb = (RawPosixAIOData) { | ||
68 | + .bs = bs, | ||
69 | + .aio_fildes = s->fd, | ||
70 | + .aio_type = QEMU_AIO_WRITE_ZEROES, | ||
71 | + .aio_offset = offset, | ||
72 | + .aio_nbytes = bytes, | ||
73 | + }; | ||
74 | + | ||
75 | + if (blkdev) { | ||
76 | + acb.aio_type |= QEMU_AIO_BLKDEV; | ||
77 | + } | ||
78 | |||
79 | if (flags & BDRV_REQ_MAY_UNMAP) { | ||
80 | - operation |= QEMU_AIO_DISCARD; | ||
81 | + acb.aio_type |= QEMU_AIO_DISCARD; | ||
82 | + handler = handle_aiocb_write_zeroes_unmap; | ||
83 | + } else { | ||
84 | + handler = handle_aiocb_write_zeroes; | ||
85 | } | ||
86 | |||
87 | - return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation); | ||
88 | + return raw_thread_pool_submit(bs, handler, &acb); | ||
89 | +} | ||
90 | + | ||
91 | +static int coroutine_fn raw_co_pwrite_zeroes( | ||
92 | + BlockDriverState *bs, int64_t offset, | ||
93 | + int bytes, BdrvRequestFlags flags) | ||
94 | +{ | ||
95 | + return raw_do_pwrite_zeroes(bs, offset, bytes, flags, false); | ||
96 | } | ||
97 | |||
98 | static int raw_get_info(BlockDriverState *bs, BlockDriverInfo *bdi) | ||
99 | @@ -XXX,XX +XXX,XX @@ hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) | ||
100 | static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs, | ||
101 | int64_t offset, int bytes, BdrvRequestFlags flags) | ||
102 | { | ||
103 | - BDRVRawState *s = bs->opaque; | ||
104 | - int operation = QEMU_AIO_WRITE_ZEROES | QEMU_AIO_BLKDEV; | ||
105 | int rc; | ||
106 | |||
107 | rc = fd_open(bs); | ||
108 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs, | ||
109 | return rc; | ||
110 | } | ||
111 | |||
112 | - if (flags & BDRV_REQ_MAY_UNMAP) { | ||
113 | - operation |= QEMU_AIO_DISCARD; | ||
114 | - } | ||
115 | - | ||
116 | - return paio_submit_co(bs, s->fd, offset, NULL, bytes, operation); | ||
117 | + return raw_do_pwrite_zeroes(bs, offset, bytes, flags, true); | ||
118 | } | ||
119 | |||
120 | static int coroutine_fn hdev_co_create_opts(const char *filename, QemuOpts *opts, | ||
121 | -- | ||
122 | 2.19.2 | ||
123 | |||
124 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 32 ++++++++++++++++++++++++-------- | ||
11 | 1 file changed, 24 insertions(+), 8 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static int handle_aiocb_copy_range(void *opaque) | ||
18 | return 0; | ||
19 | } | ||
20 | |||
21 | -static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_discard(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | int ret = -EOPNOTSUPP; | ||
26 | BDRVRawState *s = aiocb->bs->opaque; | ||
27 | |||
28 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
29 | ret = handle_aiocb_ioctl(aiocb); | ||
30 | break; | ||
31 | case QEMU_AIO_DISCARD: | ||
32 | - ret = handle_aiocb_discard(aiocb); | ||
33 | - break; | ||
34 | case QEMU_AIO_WRITE_ZEROES: | ||
35 | case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD: | ||
36 | case QEMU_AIO_COPY_RANGE: | ||
37 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs, | ||
38 | } | ||
39 | |||
40 | static coroutine_fn int | ||
41 | -raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) | ||
42 | +raw_do_pdiscard(BlockDriverState *bs, int64_t offset, int bytes, bool blkdev) | ||
43 | { | ||
44 | BDRVRawState *s = bs->opaque; | ||
45 | + RawPosixAIOData acb; | ||
46 | + | ||
47 | + acb = (RawPosixAIOData) { | ||
48 | + .bs = bs, | ||
49 | + .aio_fildes = s->fd, | ||
50 | + .aio_type = QEMU_AIO_DISCARD, | ||
51 | + .aio_offset = offset, | ||
52 | + .aio_nbytes = bytes, | ||
53 | + }; | ||
54 | |||
55 | - return paio_submit_co(bs, s->fd, offset, NULL, bytes, QEMU_AIO_DISCARD); | ||
56 | + if (blkdev) { | ||
57 | + acb.aio_type |= QEMU_AIO_BLKDEV; | ||
58 | + } | ||
59 | + | ||
60 | + return raw_thread_pool_submit(bs, handle_aiocb_discard, &acb); | ||
61 | +} | ||
62 | + | ||
63 | +static coroutine_fn int | ||
64 | +raw_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) | ||
65 | +{ | ||
66 | + return raw_do_pdiscard(bs, offset, bytes, false); | ||
67 | } | ||
68 | |||
69 | static int coroutine_fn | ||
70 | @@ -XXX,XX +XXX,XX @@ static int fd_open(BlockDriverState *bs) | ||
71 | static coroutine_fn int | ||
72 | hdev_co_pdiscard(BlockDriverState *bs, int64_t offset, int bytes) | ||
73 | { | ||
74 | - BDRVRawState *s = bs->opaque; | ||
75 | int ret; | ||
76 | |||
77 | ret = fd_open(bs); | ||
78 | if (ret < 0) { | ||
79 | return ret; | ||
80 | } | ||
81 | - return paio_submit_co(bs, s->fd, offset, NULL, bytes, | ||
82 | - QEMU_AIO_DISCARD | QEMU_AIO_BLKDEV); | ||
83 | + return raw_do_pdiscard(bs, offset, bytes, true); | ||
84 | } | ||
85 | |||
86 | static coroutine_fn int hdev_co_pwrite_zeroes(BlockDriverState *bs, | ||
87 | -- | ||
88 | 2.19.2 | ||
89 | |||
90 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 16 +++++++++++----- | ||
11 | 1 file changed, 11 insertions(+), 5 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_ioctl(RawPosixAIOData *aiocb) | ||
18 | return 0; | ||
19 | } | ||
20 | |||
21 | -static ssize_t handle_aiocb_flush(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_flush(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | BDRVRawState *s = aiocb->bs->opaque; | ||
26 | int ret; | ||
27 | |||
28 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
29 | ret = -EINVAL; | ||
30 | } | ||
31 | break; | ||
32 | - case QEMU_AIO_FLUSH: | ||
33 | - ret = handle_aiocb_flush(aiocb); | ||
34 | - break; | ||
35 | case QEMU_AIO_IOCTL: | ||
36 | ret = handle_aiocb_ioctl(aiocb); | ||
37 | break; | ||
38 | + case QEMU_AIO_FLUSH: | ||
39 | case QEMU_AIO_DISCARD: | ||
40 | case QEMU_AIO_WRITE_ZEROES: | ||
41 | case QEMU_AIO_WRITE_ZEROES | QEMU_AIO_DISCARD: | ||
42 | @@ -XXX,XX +XXX,XX @@ static void raw_aio_unplug(BlockDriverState *bs) | ||
43 | static int raw_co_flush_to_disk(BlockDriverState *bs) | ||
44 | { | ||
45 | BDRVRawState *s = bs->opaque; | ||
46 | + RawPosixAIOData acb; | ||
47 | int ret; | ||
48 | |||
49 | ret = fd_open(bs); | ||
50 | @@ -XXX,XX +XXX,XX @@ static int raw_co_flush_to_disk(BlockDriverState *bs) | ||
51 | return ret; | ||
52 | } | ||
53 | |||
54 | - return paio_submit_co(bs, s->fd, 0, NULL, 0, QEMU_AIO_FLUSH); | ||
55 | + acb = (RawPosixAIOData) { | ||
56 | + .bs = bs, | ||
57 | + .aio_fildes = s->fd, | ||
58 | + .aio_type = QEMU_AIO_FLUSH, | ||
59 | + }; | ||
60 | + | ||
61 | + return raw_thread_pool_submit(bs, handle_aiocb_flush, &acb); | ||
62 | } | ||
63 | |||
64 | static void raw_aio_attach_aio_context(BlockDriverState *bs, | ||
65 | -- | ||
66 | 2.19.2 | ||
67 | |||
68 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() for reads and writes isn't boring enough yet. It still does | ||
2 | some postprocessing for handling short reads and turning the result into | ||
3 | the right return value. | ||
4 | 1 | ||
5 | However, there is no reason why handle_aiocb_rw() couldn't do the same, | ||
6 | and even without duplicating code between the read and write path. So | ||
7 | move the code there. | ||
8 | |||
9 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
10 | --- | ||
11 | block/file-posix.c | 40 ++++++++++++++++++++-------------------- | ||
12 | 1 file changed, 20 insertions(+), 20 deletions(-) | ||
13 | |||
14 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/block/file-posix.c | ||
17 | +++ b/block/file-posix.c | ||
18 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
19 | * we can just use plain pread/pwrite without any problems. | ||
20 | */ | ||
21 | if (aiocb->io.niov == 1) { | ||
22 | - return handle_aiocb_rw_linear(aiocb, aiocb->io.iov->iov_base); | ||
23 | + nbytes = handle_aiocb_rw_linear(aiocb, aiocb->io.iov->iov_base); | ||
24 | + goto out; | ||
25 | } | ||
26 | /* | ||
27 | * We have more than one iovec, and all are properly aligned. | ||
28 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
29 | nbytes = handle_aiocb_rw_vector(aiocb); | ||
30 | if (nbytes == aiocb->aio_nbytes || | ||
31 | (nbytes < 0 && nbytes != -ENOSYS)) { | ||
32 | - return nbytes; | ||
33 | + goto out; | ||
34 | } | ||
35 | preadv_present = false; | ||
36 | } | ||
37 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
38 | */ | ||
39 | buf = qemu_try_blockalign(aiocb->bs, aiocb->aio_nbytes); | ||
40 | if (buf == NULL) { | ||
41 | - return -ENOMEM; | ||
42 | + nbytes = -ENOMEM; | ||
43 | + goto out; | ||
44 | } | ||
45 | |||
46 | if (aiocb->aio_type & QEMU_AIO_WRITE) { | ||
47 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
48 | } | ||
49 | qemu_vfree(buf); | ||
50 | |||
51 | - return nbytes; | ||
52 | +out: | ||
53 | + if (nbytes == aiocb->aio_nbytes) { | ||
54 | + return 0; | ||
55 | + } else if (nbytes >= 0 && nbytes < aiocb->aio_nbytes) { | ||
56 | + if (aiocb->aio_type & QEMU_AIO_WRITE) { | ||
57 | + return -EINVAL; | ||
58 | + } else { | ||
59 | + iov_memset(aiocb->io.iov, aiocb->io.niov, nbytes, | ||
60 | + 0, aiocb->aio_nbytes - nbytes); | ||
61 | + return 0; | ||
62 | + } | ||
63 | + } else { | ||
64 | + assert(nbytes < 0); | ||
65 | + return nbytes; | ||
66 | + } | ||
67 | } | ||
68 | |||
69 | #ifdef CONFIG_XFS | ||
70 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
71 | switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) { | ||
72 | case QEMU_AIO_READ: | ||
73 | ret = handle_aiocb_rw(aiocb); | ||
74 | - if (ret >= 0 && ret < aiocb->aio_nbytes) { | ||
75 | - iov_memset(aiocb->io.iov, aiocb->io.niov, ret, | ||
76 | - 0, aiocb->aio_nbytes - ret); | ||
77 | - | ||
78 | - ret = aiocb->aio_nbytes; | ||
79 | - } | ||
80 | - if (ret == aiocb->aio_nbytes) { | ||
81 | - ret = 0; | ||
82 | - } else if (ret >= 0 && ret < aiocb->aio_nbytes) { | ||
83 | - ret = -EINVAL; | ||
84 | - } | ||
85 | break; | ||
86 | case QEMU_AIO_WRITE: | ||
87 | ret = handle_aiocb_rw(aiocb); | ||
88 | - if (ret == aiocb->aio_nbytes) { | ||
89 | - ret = 0; | ||
90 | - } else if (ret >= 0 && ret < aiocb->aio_nbytes) { | ||
91 | - ret = -EINVAL; | ||
92 | - } | ||
93 | break; | ||
94 | case QEMU_AIO_IOCTL: | ||
95 | ret = handle_aiocb_ioctl(aiocb); | ||
96 | -- | ||
97 | 2.19.2 | ||
98 | |||
99 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | aio_worker() doesn't add anything interesting, it's only a useless | ||
2 | indirection. Call the handler function directly instead. | ||
3 | 1 | ||
4 | As we know that this handler function is only called from coroutine | ||
5 | context and the coroutine stays around until the worker thread finishes, | ||
6 | we can keep RawPosixAIOData on the stack. | ||
7 | |||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/file-posix.c | 27 +++++++++++++++++++-------- | ||
11 | 1 file changed, 19 insertions(+), 8 deletions(-) | ||
12 | |||
13 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/file-posix.c | ||
16 | +++ b/block/file-posix.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static ssize_t handle_aiocb_rw_linear(RawPosixAIOData *aiocb, char *buf) | ||
18 | return offset; | ||
19 | } | ||
20 | |||
21 | -static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb) | ||
22 | +static int handle_aiocb_rw(void *opaque) | ||
23 | { | ||
24 | + RawPosixAIOData *aiocb = opaque; | ||
25 | ssize_t nbytes; | ||
26 | char *buf; | ||
27 | |||
28 | @@ -XXX,XX +XXX,XX @@ static int aio_worker(void *arg) | ||
29 | ssize_t ret = 0; | ||
30 | |||
31 | switch (aiocb->aio_type & QEMU_AIO_TYPE_MASK) { | ||
32 | - case QEMU_AIO_READ: | ||
33 | - ret = handle_aiocb_rw(aiocb); | ||
34 | - break; | ||
35 | - case QEMU_AIO_WRITE: | ||
36 | - ret = handle_aiocb_rw(aiocb); | ||
37 | - break; | ||
38 | case QEMU_AIO_IOCTL: | ||
39 | ret = handle_aiocb_ioctl(aiocb); | ||
40 | break; | ||
41 | + case QEMU_AIO_READ: | ||
42 | + case QEMU_AIO_WRITE: | ||
43 | case QEMU_AIO_FLUSH: | ||
44 | case QEMU_AIO_DISCARD: | ||
45 | case QEMU_AIO_WRITE_ZEROES: | ||
46 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
47 | uint64_t bytes, QEMUIOVector *qiov, int type) | ||
48 | { | ||
49 | BDRVRawState *s = bs->opaque; | ||
50 | + RawPosixAIOData acb; | ||
51 | |||
52 | if (fd_open(bs) < 0) | ||
53 | return -EIO; | ||
54 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
55 | } | ||
56 | } | ||
57 | |||
58 | - return paio_submit_co(bs, s->fd, offset, qiov, bytes, type); | ||
59 | + acb = (RawPosixAIOData) { | ||
60 | + .bs = bs, | ||
61 | + .aio_fildes = s->fd, | ||
62 | + .aio_type = type, | ||
63 | + .aio_offset = offset, | ||
64 | + .aio_nbytes = bytes, | ||
65 | + .io = { | ||
66 | + .iov = qiov->iov, | ||
67 | + .niov = qiov->niov, | ||
68 | + }, | ||
69 | + }; | ||
70 | + | ||
71 | + assert(qiov->size == bytes); | ||
72 | + return raw_thread_pool_submit(bs, handle_aiocb_rw, &acb); | ||
73 | } | ||
74 | |||
75 | static int coroutine_fn raw_co_preadv(BlockDriverState *bs, uint64_t offset, | ||
76 | -- | ||
77 | 2.19.2 | ||
78 | |||
79 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | The function is not used any more, remove it. | ||
2 | 1 | ||
3 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
4 | --- | ||
5 | block/file-posix.c | 34 ---------------------------------- | ||
6 | 1 file changed, 34 deletions(-) | ||
7 | |||
8 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
9 | index XXXXXXX..XXXXXXX 100644 | ||
10 | --- a/block/file-posix.c | ||
11 | +++ b/block/file-posix.c | ||
12 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_thread_pool_submit(BlockDriverState *bs, | ||
13 | return thread_pool_submit_co(pool, func, arg); | ||
14 | } | ||
15 | |||
16 | -static int paio_submit_co_full(BlockDriverState *bs, int fd, | ||
17 | - int64_t offset, int fd2, int64_t offset2, | ||
18 | - QEMUIOVector *qiov, | ||
19 | - int bytes, int type) | ||
20 | -{ | ||
21 | - RawPosixAIOData *acb = g_new(RawPosixAIOData, 1); | ||
22 | - | ||
23 | - acb->bs = bs; | ||
24 | - acb->aio_type = type; | ||
25 | - acb->aio_fildes = fd; | ||
26 | - | ||
27 | - acb->aio_nbytes = bytes; | ||
28 | - acb->aio_offset = offset; | ||
29 | - | ||
30 | - if (qiov) { | ||
31 | - acb->io.iov = qiov->iov; | ||
32 | - acb->io.niov = qiov->niov; | ||
33 | - assert(qiov->size == bytes); | ||
34 | - } else { | ||
35 | - acb->copy_range.aio_fd2 = fd2; | ||
36 | - acb->copy_range.aio_offset2 = offset2; | ||
37 | - } | ||
38 | - | ||
39 | - trace_file_paio_submit_co(offset, bytes, type); | ||
40 | - return raw_thread_pool_submit(bs, aio_worker, acb); | ||
41 | -} | ||
42 | - | ||
43 | -static inline int paio_submit_co(BlockDriverState *bs, int fd, | ||
44 | - int64_t offset, QEMUIOVector *qiov, | ||
45 | - int bytes, int type) | ||
46 | -{ | ||
47 | - return paio_submit_co_full(bs, fd, offset, -1, 0, qiov, bytes, type); | ||
48 | -} | ||
49 | - | ||
50 | static int coroutine_fn raw_co_prw(BlockDriverState *bs, uint64_t offset, | ||
51 | uint64_t bytes, QEMUIOVector *qiov, int type) | ||
52 | { | ||
53 | -- | ||
54 | 2.19.2 | ||
55 | |||
56 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | No real reason to keep using the callback based mechanism here when the | ||
2 | rest of the file-posix driver is coroutine based. Changing it brings | ||
3 | ioctls more in line with how other request types work. | ||
4 | 1 | ||
5 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
6 | --- | ||
7 | include/scsi/pr-manager.h | 8 +++----- | ||
8 | block/file-posix.c | 21 +++++++++++---------- | ||
9 | scsi/pr-manager.c | 21 +++++++++------------ | ||
10 | scsi/trace-events | 2 +- | ||
11 | 4 files changed, 24 insertions(+), 28 deletions(-) | ||
12 | |||
13 | diff --git a/include/scsi/pr-manager.h b/include/scsi/pr-manager.h | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/include/scsi/pr-manager.h | ||
16 | +++ b/include/scsi/pr-manager.h | ||
17 | @@ -XXX,XX +XXX,XX @@ | ||
18 | #include "qapi/visitor.h" | ||
19 | #include "qom/object_interfaces.h" | ||
20 | #include "block/aio.h" | ||
21 | +#include "qemu/coroutine.h" | ||
22 | |||
23 | #define TYPE_PR_MANAGER "pr-manager" | ||
24 | |||
25 | @@ -XXX,XX +XXX,XX @@ typedef struct PRManagerClass { | ||
26 | } PRManagerClass; | ||
27 | |||
28 | bool pr_manager_is_connected(PRManager *pr_mgr); | ||
29 | -BlockAIOCB *pr_manager_execute(PRManager *pr_mgr, | ||
30 | - AioContext *ctx, int fd, | ||
31 | - struct sg_io_hdr *hdr, | ||
32 | - BlockCompletionFunc *complete, | ||
33 | - void *opaque); | ||
34 | +int coroutine_fn pr_manager_execute(PRManager *pr_mgr, AioContext *ctx, int fd, | ||
35 | + struct sg_io_hdr *hdr); | ||
36 | |||
37 | PRManager *pr_manager_lookup(const char *id, Error **errp); | ||
38 | |||
39 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
40 | index XXXXXXX..XXXXXXX 100644 | ||
41 | --- a/block/file-posix.c | ||
42 | +++ b/block/file-posix.c | ||
43 | @@ -XXX,XX +XXX,XX @@ hdev_open_Mac_error: | ||
44 | } | ||
45 | |||
46 | #if defined(__linux__) | ||
47 | - | ||
48 | -static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs, | ||
49 | - unsigned long int req, void *buf, | ||
50 | - BlockCompletionFunc *cb, void *opaque) | ||
51 | +static int coroutine_fn | ||
52 | +hdev_co_ioctl(BlockDriverState *bs, unsigned long int req, void *buf) | ||
53 | { | ||
54 | BDRVRawState *s = bs->opaque; | ||
55 | RawPosixAIOData *acb; | ||
56 | ThreadPool *pool; | ||
57 | + int ret; | ||
58 | |||
59 | - if (fd_open(bs) < 0) | ||
60 | - return NULL; | ||
61 | + ret = fd_open(bs); | ||
62 | + if (ret < 0) { | ||
63 | + return ret; | ||
64 | + } | ||
65 | |||
66 | if (req == SG_IO && s->pr_mgr) { | ||
67 | struct sg_io_hdr *io_hdr = buf; | ||
68 | if (io_hdr->cmdp[0] == PERSISTENT_RESERVE_OUT || | ||
69 | io_hdr->cmdp[0] == PERSISTENT_RESERVE_IN) { | ||
70 | return pr_manager_execute(s->pr_mgr, bdrv_get_aio_context(bs), | ||
71 | - s->fd, io_hdr, cb, opaque); | ||
72 | + s->fd, io_hdr); | ||
73 | } | ||
74 | } | ||
75 | |||
76 | @@ -XXX,XX +XXX,XX @@ static BlockAIOCB *hdev_aio_ioctl(BlockDriverState *bs, | ||
77 | acb->ioctl.buf = buf; | ||
78 | acb->ioctl.cmd = req; | ||
79 | pool = aio_get_thread_pool(bdrv_get_aio_context(bs)); | ||
80 | - return thread_pool_submit_aio(pool, aio_worker, acb, cb, opaque); | ||
81 | + return thread_pool_submit_co(pool, aio_worker, acb); | ||
82 | } | ||
83 | #endif /* linux */ | ||
84 | |||
85 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = { | ||
86 | |||
87 | /* generic scsi device */ | ||
88 | #ifdef __linux__ | ||
89 | - .bdrv_aio_ioctl = hdev_aio_ioctl, | ||
90 | + .bdrv_co_ioctl = hdev_co_ioctl, | ||
91 | #endif | ||
92 | }; | ||
93 | |||
94 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { | ||
95 | .bdrv_lock_medium = cdrom_lock_medium, | ||
96 | |||
97 | /* generic scsi device */ | ||
98 | - .bdrv_aio_ioctl = hdev_aio_ioctl, | ||
99 | + .bdrv_co_ioctl = hdev_co_ioctl, | ||
100 | }; | ||
101 | #endif /* __linux__ */ | ||
102 | |||
103 | diff --git a/scsi/pr-manager.c b/scsi/pr-manager.c | ||
104 | index XXXXXXX..XXXXXXX 100644 | ||
105 | --- a/scsi/pr-manager.c | ||
106 | +++ b/scsi/pr-manager.c | ||
107 | @@ -XXX,XX +XXX,XX @@ static int pr_manager_worker(void *opaque) | ||
108 | } | ||
109 | |||
110 | |||
111 | -BlockAIOCB *pr_manager_execute(PRManager *pr_mgr, | ||
112 | - AioContext *ctx, int fd, | ||
113 | - struct sg_io_hdr *hdr, | ||
114 | - BlockCompletionFunc *complete, | ||
115 | - void *opaque) | ||
116 | +int coroutine_fn pr_manager_execute(PRManager *pr_mgr, AioContext *ctx, int fd, | ||
117 | + struct sg_io_hdr *hdr) | ||
118 | { | ||
119 | - PRManagerData *data = g_new(PRManagerData, 1); | ||
120 | ThreadPool *pool = aio_get_thread_pool(ctx); | ||
121 | + PRManagerData data = { | ||
122 | + .pr_mgr = pr_mgr, | ||
123 | + .fd = fd, | ||
124 | + .hdr = hdr, | ||
125 | + }; | ||
126 | |||
127 | - trace_pr_manager_execute(fd, hdr->cmdp[0], hdr->cmdp[1], opaque); | ||
128 | - data->pr_mgr = pr_mgr; | ||
129 | - data->fd = fd; | ||
130 | - data->hdr = hdr; | ||
131 | + trace_pr_manager_execute(fd, hdr->cmdp[0], hdr->cmdp[1]); | ||
132 | |||
133 | /* The matching object_unref is in pr_manager_worker. */ | ||
134 | object_ref(OBJECT(pr_mgr)); | ||
135 | - return thread_pool_submit_aio(pool, pr_manager_worker, | ||
136 | - data, complete, opaque); | ||
137 | + return thread_pool_submit_co(pool, pr_manager_worker, &data); | ||
138 | } | ||
139 | |||
140 | bool pr_manager_is_connected(PRManager *pr_mgr) | ||
141 | diff --git a/scsi/trace-events b/scsi/trace-events | ||
142 | index XXXXXXX..XXXXXXX 100644 | ||
143 | --- a/scsi/trace-events | ||
144 | +++ b/scsi/trace-events | ||
145 | @@ -XXX,XX +XXX,XX @@ | ||
146 | # scsi/pr-manager.c | ||
147 | -pr_manager_execute(int fd, int cmd, int sa, void *opaque) "fd=%d cmd=0x%02x service action=0x%02x opaque=%p" | ||
148 | +pr_manager_execute(int fd, int cmd, int sa) "fd=%d cmd=0x%02x service action=0x%02x" | ||
149 | pr_manager_run(int fd, int cmd, int sa) "fd=%d cmd=0x%02x service action=0x%02x" | ||
150 | -- | ||
151 | 2.19.2 | ||
152 | |||
153 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | Most callers of bdrv_reopen() only use it to switch a BlockDriverState | ||
4 | between read-only and read-write, so this patch adds a new function | ||
5 | that does just that. | ||
6 | |||
7 | We also want to get rid of the flags parameter in the bdrv_reopen() | ||
8 | API, so this function sets the "read-only" option and passes the | ||
9 | original flags (which will then be updated in bdrv_reopen_prepare()). | ||
10 | |||
11 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
12 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | include/block/block.h | 2 ++ | ||
16 | block.c | 17 +++++++++++++++++ | ||
17 | 2 files changed, 19 insertions(+) | ||
18 | |||
19 | diff --git a/include/block/block.h b/include/block/block.h | ||
20 | index XXXXXXX..XXXXXXX 100644 | ||
21 | --- a/include/block/block.h | ||
22 | +++ b/include/block/block.h | ||
23 | @@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
24 | QDict *options, int flags); | ||
25 | int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **errp); | ||
26 | int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp); | ||
27 | +int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
28 | + Error **errp); | ||
29 | int bdrv_reopen_prepare(BDRVReopenState *reopen_state, | ||
30 | BlockReopenQueue *queue, Error **errp); | ||
31 | void bdrv_reopen_commit(BDRVReopenState *reopen_state); | ||
32 | diff --git a/block.c b/block.c | ||
33 | index XXXXXXX..XXXXXXX 100644 | ||
34 | --- a/block.c | ||
35 | +++ b/block.c | ||
36 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp) | ||
37 | return ret; | ||
38 | } | ||
39 | |||
40 | +int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
41 | + Error **errp) | ||
42 | +{ | ||
43 | + int ret; | ||
44 | + BlockReopenQueue *queue; | ||
45 | + QDict *opts = qdict_new(); | ||
46 | + | ||
47 | + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only); | ||
48 | + | ||
49 | + bdrv_subtree_drained_begin(bs); | ||
50 | + queue = bdrv_reopen_queue(NULL, bs, opts, bdrv_get_flags(bs)); | ||
51 | + ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, errp); | ||
52 | + bdrv_subtree_drained_end(bs); | ||
53 | + | ||
54 | + return ret; | ||
55 | +} | ||
56 | + | ||
57 | static BlockReopenQueueEntry *find_parent_in_reopen_queue(BlockReopenQueue *q, | ||
58 | BdrvChild *c) | ||
59 | { | ||
60 | -- | ||
61 | 2.19.2 | ||
62 | |||
63 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() calls that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block.c | 10 +++++----- | ||
11 | 1 file changed, 5 insertions(+), 5 deletions(-) | ||
12 | |||
13 | diff --git a/block.c b/block.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block.c | ||
16 | +++ b/block.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static int bdrv_backing_update_filename(BdrvChild *c, BlockDriverState *base, | ||
18 | const char *filename, Error **errp) | ||
19 | { | ||
20 | BlockDriverState *parent = c->opaque; | ||
21 | - int orig_flags = bdrv_get_flags(parent); | ||
22 | + bool read_only = bdrv_is_read_only(parent); | ||
23 | int ret; | ||
24 | |||
25 | - if (!(orig_flags & BDRV_O_RDWR)) { | ||
26 | - ret = bdrv_reopen(parent, orig_flags | BDRV_O_RDWR, errp); | ||
27 | + if (read_only) { | ||
28 | + ret = bdrv_reopen_set_read_only(parent, false, errp); | ||
29 | if (ret < 0) { | ||
30 | return ret; | ||
31 | } | ||
32 | @@ -XXX,XX +XXX,XX @@ static int bdrv_backing_update_filename(BdrvChild *c, BlockDriverState *base, | ||
33 | error_setg_errno(errp, -ret, "Could not update backing file link"); | ||
34 | } | ||
35 | |||
36 | - if (!(orig_flags & BDRV_O_RDWR)) { | ||
37 | - bdrv_reopen(parent, orig_flags, NULL); | ||
38 | + if (read_only) { | ||
39 | + bdrv_reopen_set_read_only(parent, true, NULL); | ||
40 | } | ||
41 | |||
42 | return ret; | ||
43 | -- | ||
44 | 2.19.2 | ||
45 | |||
46 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() calls that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/commit.c | 16 ++++++---------- | ||
11 | 1 file changed, 6 insertions(+), 10 deletions(-) | ||
12 | |||
13 | diff --git a/block/commit.c b/block/commit.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/commit.c | ||
16 | +++ b/block/commit.c | ||
17 | @@ -XXX,XX +XXX,XX @@ typedef struct CommitBlockJob { | ||
18 | BlockBackend *base; | ||
19 | BlockDriverState *base_bs; | ||
20 | BlockdevOnError on_error; | ||
21 | - int base_flags; | ||
22 | + bool base_read_only; | ||
23 | char *backing_file_str; | ||
24 | } CommitBlockJob; | ||
25 | |||
26 | @@ -XXX,XX +XXX,XX @@ static void commit_clean(Job *job) | ||
27 | /* restore base open flags here if appropriate (e.g., change the base back | ||
28 | * to r/o). These reopens do not need to be atomic, since we won't abort | ||
29 | * even on failure here */ | ||
30 | - if (s->base_flags != bdrv_get_flags(s->base_bs)) { | ||
31 | - bdrv_reopen(s->base_bs, s->base_flags, NULL); | ||
32 | + if (s->base_read_only) { | ||
33 | + bdrv_reopen_set_read_only(s->base_bs, true, NULL); | ||
34 | } | ||
35 | |||
36 | g_free(s->backing_file_str); | ||
37 | @@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs, | ||
38 | const char *filter_node_name, Error **errp) | ||
39 | { | ||
40 | CommitBlockJob *s; | ||
41 | - int orig_base_flags; | ||
42 | BlockDriverState *iter; | ||
43 | BlockDriverState *commit_top_bs = NULL; | ||
44 | Error *local_err = NULL; | ||
45 | @@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs, | ||
46 | } | ||
47 | |||
48 | /* convert base to r/w, if necessary */ | ||
49 | - orig_base_flags = bdrv_get_flags(base); | ||
50 | - if (!(orig_base_flags & BDRV_O_RDWR)) { | ||
51 | - bdrv_reopen(base, orig_base_flags | BDRV_O_RDWR, &local_err); | ||
52 | - if (local_err != NULL) { | ||
53 | - error_propagate(errp, local_err); | ||
54 | + s->base_read_only = bdrv_is_read_only(base); | ||
55 | + if (s->base_read_only) { | ||
56 | + if (bdrv_reopen_set_read_only(base, false, errp) != 0) { | ||
57 | goto fail; | ||
58 | } | ||
59 | } | ||
60 | @@ -XXX,XX +XXX,XX @@ void commit_start(const char *job_id, BlockDriverState *bs, | ||
61 | goto fail; | ||
62 | } | ||
63 | |||
64 | - s->base_flags = orig_base_flags; | ||
65 | s->backing_file_str = g_strdup(backing_file_str); | ||
66 | s->on_error = on_error; | ||
67 | |||
68 | -- | ||
69 | 2.19.2 | ||
70 | |||
71 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() calls that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/commit.c | 7 +++---- | ||
11 | 1 file changed, 3 insertions(+), 4 deletions(-) | ||
12 | |||
13 | diff --git a/block/commit.c b/block/commit.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/commit.c | ||
16 | +++ b/block/commit.c | ||
17 | @@ -XXX,XX +XXX,XX @@ int bdrv_commit(BlockDriverState *bs) | ||
18 | BlockDriverState *commit_top_bs = NULL; | ||
19 | BlockDriver *drv = bs->drv; | ||
20 | int64_t offset, length, backing_length; | ||
21 | - int ro, open_flags; | ||
22 | + int ro; | ||
23 | int64_t n; | ||
24 | int ret = 0; | ||
25 | uint8_t *buf = NULL; | ||
26 | @@ -XXX,XX +XXX,XX @@ int bdrv_commit(BlockDriverState *bs) | ||
27 | } | ||
28 | |||
29 | ro = bs->backing->bs->read_only; | ||
30 | - open_flags = bs->backing->bs->open_flags; | ||
31 | |||
32 | if (ro) { | ||
33 | - if (bdrv_reopen(bs->backing->bs, open_flags | BDRV_O_RDWR, NULL)) { | ||
34 | + if (bdrv_reopen_set_read_only(bs->backing->bs, false, NULL)) { | ||
35 | return -EACCES; | ||
36 | } | ||
37 | } | ||
38 | @@ -XXX,XX +XXX,XX @@ ro_cleanup: | ||
39 | |||
40 | if (ro) { | ||
41 | /* ignoring error return here */ | ||
42 | - bdrv_reopen(bs->backing->bs, open_flags & ~BDRV_O_RDWR, NULL); | ||
43 | + bdrv_reopen_set_read_only(bs->backing->bs, true, NULL); | ||
44 | } | ||
45 | |||
46 | return ret; | ||
47 | -- | ||
48 | 2.19.2 | ||
49 | |||
50 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() calls that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | block/stream.c | 20 ++++++++++---------- | ||
11 | 1 file changed, 10 insertions(+), 10 deletions(-) | ||
12 | |||
13 | diff --git a/block/stream.c b/block/stream.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/block/stream.c | ||
16 | +++ b/block/stream.c | ||
17 | @@ -XXX,XX +XXX,XX @@ typedef struct StreamBlockJob { | ||
18 | BlockDriverState *base; | ||
19 | BlockdevOnError on_error; | ||
20 | char *backing_file_str; | ||
21 | - int bs_flags; | ||
22 | + bool bs_read_only; | ||
23 | } StreamBlockJob; | ||
24 | |||
25 | static int coroutine_fn stream_populate(BlockBackend *blk, | ||
26 | @@ -XXX,XX +XXX,XX @@ static void stream_clean(Job *job) | ||
27 | BlockDriverState *bs = blk_bs(bjob->blk); | ||
28 | |||
29 | /* Reopen the image back in read-only mode if necessary */ | ||
30 | - if (s->bs_flags != bdrv_get_flags(bs)) { | ||
31 | + if (s->bs_read_only) { | ||
32 | /* Give up write permissions before making it read-only */ | ||
33 | blk_set_perm(bjob->blk, 0, BLK_PERM_ALL, &error_abort); | ||
34 | - bdrv_reopen(bs, s->bs_flags, NULL); | ||
35 | + bdrv_reopen_set_read_only(bs, true, NULL); | ||
36 | } | ||
37 | |||
38 | g_free(s->backing_file_str); | ||
39 | @@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs, | ||
40 | { | ||
41 | StreamBlockJob *s; | ||
42 | BlockDriverState *iter; | ||
43 | - int orig_bs_flags; | ||
44 | + bool bs_read_only; | ||
45 | |||
46 | /* Make sure that the image is opened in read-write mode */ | ||
47 | - orig_bs_flags = bdrv_get_flags(bs); | ||
48 | - if (!(orig_bs_flags & BDRV_O_RDWR)) { | ||
49 | - if (bdrv_reopen(bs, orig_bs_flags | BDRV_O_RDWR, errp) != 0) { | ||
50 | + bs_read_only = bdrv_is_read_only(bs); | ||
51 | + if (bs_read_only) { | ||
52 | + if (bdrv_reopen_set_read_only(bs, false, errp) != 0) { | ||
53 | return; | ||
54 | } | ||
55 | } | ||
56 | @@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs, | ||
57 | |||
58 | s->base = base; | ||
59 | s->backing_file_str = g_strdup(backing_file_str); | ||
60 | - s->bs_flags = orig_bs_flags; | ||
61 | + s->bs_read_only = bs_read_only; | ||
62 | |||
63 | s->on_error = on_error; | ||
64 | trace_stream_start(bs, base, s); | ||
65 | @@ -XXX,XX +XXX,XX @@ void stream_start(const char *job_id, BlockDriverState *bs, | ||
66 | return; | ||
67 | |||
68 | fail: | ||
69 | - if (orig_bs_flags != bdrv_get_flags(bs)) { | ||
70 | - bdrv_reopen(bs, orig_bs_flags, NULL); | ||
71 | + if (bs_read_only) { | ||
72 | + bdrv_reopen_set_read_only(bs, true, NULL); | ||
73 | } | ||
74 | } | ||
75 | -- | ||
76 | 2.19.2 | ||
77 | |||
78 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() calls that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | blockdev.c | 8 ++------ | ||
11 | 1 file changed, 2 insertions(+), 6 deletions(-) | ||
12 | |||
13 | diff --git a/blockdev.c b/blockdev.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/blockdev.c | ||
16 | +++ b/blockdev.c | ||
17 | @@ -XXX,XX +XXX,XX @@ void qmp_change_backing_file(const char *device, | ||
18 | BlockDriverState *image_bs = NULL; | ||
19 | Error *local_err = NULL; | ||
20 | bool ro; | ||
21 | - int open_flags; | ||
22 | int ret; | ||
23 | |||
24 | bs = qmp_get_root_bs(device, errp); | ||
25 | @@ -XXX,XX +XXX,XX @@ void qmp_change_backing_file(const char *device, | ||
26 | } | ||
27 | |||
28 | /* if not r/w, reopen to make r/w */ | ||
29 | - open_flags = image_bs->open_flags; | ||
30 | ro = bdrv_is_read_only(image_bs); | ||
31 | |||
32 | if (ro) { | ||
33 | - bdrv_reopen(image_bs, open_flags | BDRV_O_RDWR, &local_err); | ||
34 | - if (local_err) { | ||
35 | - error_propagate(errp, local_err); | ||
36 | + if (bdrv_reopen_set_read_only(image_bs, false, errp) != 0) { | ||
37 | goto out; | ||
38 | } | ||
39 | } | ||
40 | @@ -XXX,XX +XXX,XX @@ void qmp_change_backing_file(const char *device, | ||
41 | } | ||
42 | |||
43 | if (ro) { | ||
44 | - bdrv_reopen(image_bs, open_flags, &local_err); | ||
45 | + bdrv_reopen_set_read_only(image_bs, true, &local_err); | ||
46 | error_propagate(errp, local_err); | ||
47 | } | ||
48 | |||
49 | -- | ||
50 | 2.19.2 | ||
51 | |||
52 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This patch replaces the bdrv_reopen() call that set and remove the | ||
4 | BDRV_O_RDWR flag with the new bdrv_reopen_set_read_only() function. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | blockdev.c | 3 +-- | ||
11 | 1 file changed, 1 insertion(+), 2 deletions(-) | ||
12 | |||
13 | diff --git a/blockdev.c b/blockdev.c | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/blockdev.c | ||
16 | +++ b/blockdev.c | ||
17 | @@ -XXX,XX +XXX,XX @@ static void external_snapshot_commit(BlkActionState *common) | ||
18 | * bdrv_reopen_multiple() across all the entries at once, because we | ||
19 | * don't want to abort all of them if one of them fails the reopen */ | ||
20 | if (!atomic_read(&state->old_bs->copy_on_read)) { | ||
21 | - bdrv_reopen(state->old_bs, state->old_bs->open_flags & ~BDRV_O_RDWR, | ||
22 | - NULL); | ||
23 | + bdrv_reopen_set_read_only(state->old_bs, true, NULL); | ||
24 | } | ||
25 | |||
26 | aio_context_release(aio_context); | ||
27 | -- | ||
28 | 2.19.2 | ||
29 | |||
30 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | The 'block-commit' QMP command is implemented internally using two | ||
4 | different drivers. If the source image is the active layer then the | ||
5 | mirror driver is used (commit_active_start()), otherwise the commit | ||
6 | driver is used (commit_start()). | ||
7 | |||
8 | In both cases the destination image must be put temporarily in | ||
9 | read-write mode. This is done correctly in the latter case, but what | ||
10 | commit_active_start() does is copy all flags instead. | ||
11 | |||
12 | This patch replaces the bdrv_reopen() calls in that function with | ||
13 | bdrv_reopen_set_read_only() so that only the read-only status is | ||
14 | changed. | ||
15 | |||
16 | A similar change is made in mirror_exit(), which is also used by the | ||
17 | 'drive-mirror' and 'blockdev-mirror' commands. | ||
18 | |||
19 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
20 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
21 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
22 | --- | ||
23 | block/mirror.c | 19 ++++++++++++------- | ||
24 | 1 file changed, 12 insertions(+), 7 deletions(-) | ||
25 | |||
26 | diff --git a/block/mirror.c b/block/mirror.c | ||
27 | index XXXXXXX..XXXXXXX 100644 | ||
28 | --- a/block/mirror.c | ||
29 | +++ b/block/mirror.c | ||
30 | @@ -XXX,XX +XXX,XX @@ static int mirror_exit_common(Job *job) | ||
31 | |||
32 | if (s->should_complete && !abort) { | ||
33 | BlockDriverState *to_replace = s->to_replace ?: src; | ||
34 | + bool ro = bdrv_is_read_only(to_replace); | ||
35 | |||
36 | - if (bdrv_get_flags(target_bs) != bdrv_get_flags(to_replace)) { | ||
37 | - bdrv_reopen(target_bs, bdrv_get_flags(to_replace), NULL); | ||
38 | + if (ro != bdrv_is_read_only(target_bs)) { | ||
39 | + bdrv_reopen_set_read_only(target_bs, ro, NULL); | ||
40 | } | ||
41 | |||
42 | /* The mirror job has no requests in flight any more, but we need to | ||
43 | @@ -XXX,XX +XXX,XX @@ void commit_active_start(const char *job_id, BlockDriverState *bs, | ||
44 | BlockCompletionFunc *cb, void *opaque, | ||
45 | bool auto_complete, Error **errp) | ||
46 | { | ||
47 | - int orig_base_flags; | ||
48 | + bool base_read_only; | ||
49 | Error *local_err = NULL; | ||
50 | |||
51 | - orig_base_flags = bdrv_get_flags(base); | ||
52 | + base_read_only = bdrv_is_read_only(base); | ||
53 | |||
54 | - if (bdrv_reopen(base, bs->open_flags, errp)) { | ||
55 | - return; | ||
56 | + if (base_read_only) { | ||
57 | + if (bdrv_reopen_set_read_only(base, false, errp) < 0) { | ||
58 | + return; | ||
59 | + } | ||
60 | } | ||
61 | |||
62 | mirror_start_job(job_id, bs, creation_flags, base, NULL, speed, 0, 0, | ||
63 | @@ -XXX,XX +XXX,XX @@ void commit_active_start(const char *job_id, BlockDriverState *bs, | ||
64 | error_restore_flags: | ||
65 | /* ignore error and errp for bdrv_reopen, because we want to propagate | ||
66 | * the original error */ | ||
67 | - bdrv_reopen(base, orig_base_flags, NULL); | ||
68 | + if (base_read_only) { | ||
69 | + bdrv_reopen_set_read_only(base, true, NULL); | ||
70 | + } | ||
71 | return; | ||
72 | } | ||
73 | -- | ||
74 | 2.19.2 | ||
75 | |||
76 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | No one is using this function anymore, so we can safely remove it. | ||
4 | |||
5 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
6 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
7 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | --- | ||
9 | include/block/block.h | 1 - | ||
10 | block.c | 21 --------------------- | ||
11 | 2 files changed, 22 deletions(-) | ||
12 | |||
13 | diff --git a/include/block/block.h b/include/block/block.h | ||
14 | index XXXXXXX..XXXXXXX 100644 | ||
15 | --- a/include/block/block.h | ||
16 | +++ b/include/block/block.h | ||
17 | @@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
18 | BlockDriverState *bs, | ||
19 | QDict *options, int flags); | ||
20 | int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **errp); | ||
21 | -int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp); | ||
22 | int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
23 | Error **errp); | ||
24 | int bdrv_reopen_prepare(BDRVReopenState *reopen_state, | ||
25 | diff --git a/block.c b/block.c | ||
26 | index XXXXXXX..XXXXXXX 100644 | ||
27 | --- a/block.c | ||
28 | +++ b/block.c | ||
29 | @@ -XXX,XX +XXX,XX @@ cleanup: | ||
30 | return ret; | ||
31 | } | ||
32 | |||
33 | - | ||
34 | -/* Reopen a single BlockDriverState with the specified flags. */ | ||
35 | -int bdrv_reopen(BlockDriverState *bs, int bdrv_flags, Error **errp) | ||
36 | -{ | ||
37 | - int ret = -1; | ||
38 | - Error *local_err = NULL; | ||
39 | - BlockReopenQueue *queue; | ||
40 | - | ||
41 | - bdrv_subtree_drained_begin(bs); | ||
42 | - | ||
43 | - queue = bdrv_reopen_queue(NULL, bs, NULL, bdrv_flags); | ||
44 | - ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, &local_err); | ||
45 | - if (local_err != NULL) { | ||
46 | - error_propagate(errp, local_err); | ||
47 | - } | ||
48 | - | ||
49 | - bdrv_subtree_drained_end(bs); | ||
50 | - | ||
51 | - return ret; | ||
52 | -} | ||
53 | - | ||
54 | int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
55 | Error **errp) | ||
56 | { | ||
57 | -- | ||
58 | 2.19.2 | ||
59 | |||
60 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | When reopen_f() puts a block device in the reopen queue, some of the | ||
4 | new options are passed using a QDict, but others ("read-only" and the | ||
5 | cache options) are passed as flags. | ||
6 | |||
7 | This patch puts those flags in the QDict. This way the flags parameter | ||
8 | becomes redundant and we'll be able to get rid of it in a subsequent | ||
9 | patch. | ||
10 | |||
11 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
12 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
13 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
14 | --- | ||
15 | qemu-io-cmds.c | 27 ++++++++++++++++++++++++++- | ||
16 | tests/qemu-iotests/133 | 9 +++++++++ | ||
17 | tests/qemu-iotests/133.out | 8 ++++++++ | ||
18 | 3 files changed, 43 insertions(+), 1 deletion(-) | ||
19 | |||
20 | diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c | ||
21 | index XXXXXXX..XXXXXXX 100644 | ||
22 | --- a/qemu-io-cmds.c | ||
23 | +++ b/qemu-io-cmds.c | ||
24 | @@ -XXX,XX +XXX,XX @@ | ||
25 | |||
26 | #include "qemu/osdep.h" | ||
27 | #include "qapi/error.h" | ||
28 | +#include "qapi/qmp/qdict.h" | ||
29 | #include "qemu-io.h" | ||
30 | #include "sysemu/block-backend.h" | ||
31 | #include "block/block.h" | ||
32 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
33 | int flags = bs->open_flags; | ||
34 | bool writethrough = !blk_enable_write_cache(blk); | ||
35 | bool has_rw_option = false; | ||
36 | + bool has_cache_option = false; | ||
37 | |||
38 | BlockReopenQueue *brq; | ||
39 | Error *local_err = NULL; | ||
40 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
41 | error_report("Invalid cache option: %s", optarg); | ||
42 | return -EINVAL; | ||
43 | } | ||
44 | + has_cache_option = true; | ||
45 | break; | ||
46 | case 'o': | ||
47 | if (!qemu_opts_parse_noisily(&reopen_opts, optarg, 0)) { | ||
48 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
49 | } | ||
50 | |||
51 | qopts = qemu_opts_find(&reopen_opts, NULL); | ||
52 | - opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : NULL; | ||
53 | + opts = qopts ? qemu_opts_to_qdict(qopts, NULL) : qdict_new(); | ||
54 | qemu_opts_reset(&reopen_opts); | ||
55 | |||
56 | + if (qdict_haskey(opts, BDRV_OPT_READ_ONLY)) { | ||
57 | + if (has_rw_option) { | ||
58 | + error_report("Cannot set both -r/-w and '" BDRV_OPT_READ_ONLY "'"); | ||
59 | + qobject_unref(opts); | ||
60 | + return -EINVAL; | ||
61 | + } | ||
62 | + } else { | ||
63 | + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !(flags & BDRV_O_RDWR)); | ||
64 | + } | ||
65 | + | ||
66 | + if (qdict_haskey(opts, BDRV_OPT_CACHE_DIRECT) || | ||
67 | + qdict_haskey(opts, BDRV_OPT_CACHE_NO_FLUSH)) { | ||
68 | + if (has_cache_option) { | ||
69 | + error_report("Cannot set both -c and the cache options"); | ||
70 | + qobject_unref(opts); | ||
71 | + return -EINVAL; | ||
72 | + } | ||
73 | + } else { | ||
74 | + qdict_put_bool(opts, BDRV_OPT_CACHE_DIRECT, flags & BDRV_O_NOCACHE); | ||
75 | + qdict_put_bool(opts, BDRV_OPT_CACHE_NO_FLUSH, flags & BDRV_O_NO_FLUSH); | ||
76 | + } | ||
77 | + | ||
78 | bdrv_subtree_drained_begin(bs); | ||
79 | brq = bdrv_reopen_queue(NULL, bs, opts, flags); | ||
80 | bdrv_reopen_multiple(bdrv_get_aio_context(bs), brq, &local_err); | ||
81 | diff --git a/tests/qemu-iotests/133 b/tests/qemu-iotests/133 | ||
82 | index XXXXXXX..XXXXXXX 100755 | ||
83 | --- a/tests/qemu-iotests/133 | ||
84 | +++ b/tests/qemu-iotests/133 | ||
85 | @@ -XXX,XX +XXX,XX @@ echo | ||
86 | IMGOPTSSYNTAX=false $QEMU_IO -f null-co -c 'reopen' -c 'info' \ | ||
87 | "json:{'driver': 'null-co', 'size': 65536}" | ||
88 | |||
89 | +echo | ||
90 | +echo "=== Check that mixing -c/-r/-w and their corresponding options is forbidden ===" | ||
91 | +echo | ||
92 | + | ||
93 | +$QEMU_IO -c 'reopen -r -o read-only=on' $TEST_IMG | ||
94 | +$QEMU_IO -c 'reopen -w -o read-only=on' $TEST_IMG | ||
95 | +$QEMU_IO -c 'reopen -c none -o cache.direct=on' $TEST_IMG | ||
96 | +$QEMU_IO -c 'reopen -c writeback -o cache.direct=on' $TEST_IMG | ||
97 | +$QEMU_IO -c 'reopen -c directsync -o cache.no-flush=on' $TEST_IMG | ||
98 | # success, all done | ||
99 | echo "*** done" | ||
100 | rm -f $seq.full | ||
101 | diff --git a/tests/qemu-iotests/133.out b/tests/qemu-iotests/133.out | ||
102 | index XXXXXXX..XXXXXXX 100644 | ||
103 | --- a/tests/qemu-iotests/133.out | ||
104 | +++ b/tests/qemu-iotests/133.out | ||
105 | @@ -XXX,XX +XXX,XX @@ Cannot change the option 'driver' | ||
106 | |||
107 | format name: null-co | ||
108 | format name: null-co | ||
109 | + | ||
110 | +=== Check that mixing -c/-r/-w and their corresponding options is forbidden === | ||
111 | + | ||
112 | +Cannot set both -r/-w and 'read-only' | ||
113 | +Cannot set both -r/-w and 'read-only' | ||
114 | +Cannot set both -c and the cache options | ||
115 | +Cannot set both -c and the cache options | ||
116 | +Cannot set both -c and the cache options | ||
117 | *** done | ||
118 | -- | ||
119 | 2.19.2 | ||
120 | |||
121 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | This function is used to put the hidden and secondary disks in | ||
4 | read-write mode before launching the backup job, and back in read-only | ||
5 | mode afterwards. | ||
6 | |||
7 | This patch does the following changes: | ||
8 | |||
9 | - Use an options QDict with the "read-only" option instead of | ||
10 | passing the changes as flags only. | ||
11 | |||
12 | - Simplify the code (it was unnecessarily complicated and verbose). | ||
13 | |||
14 | - Fix a bug due to which the secondary disk was not being put back | ||
15 | in read-only mode when writable=false (because in this case | ||
16 | orig_secondary_flags always had the BDRV_O_RDWR flag set). | ||
17 | |||
18 | - Stop clearing the BDRV_O_INACTIVE flag. | ||
19 | |||
20 | The flags parameter to bdrv_reopen_queue() becomes redundant and we'll | ||
21 | be able to get rid of it in a subsequent patch. | ||
22 | |||
23 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
24 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
25 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
26 | --- | ||
27 | block/replication.c | 45 +++++++++++++++++++++------------------------ | ||
28 | 1 file changed, 21 insertions(+), 24 deletions(-) | ||
29 | |||
30 | diff --git a/block/replication.c b/block/replication.c | ||
31 | index XXXXXXX..XXXXXXX 100644 | ||
32 | --- a/block/replication.c | ||
33 | +++ b/block/replication.c | ||
34 | @@ -XXX,XX +XXX,XX @@ | ||
35 | #include "block/block_backup.h" | ||
36 | #include "sysemu/block-backend.h" | ||
37 | #include "qapi/error.h" | ||
38 | +#include "qapi/qmp/qdict.h" | ||
39 | #include "replication.h" | ||
40 | |||
41 | typedef enum { | ||
42 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVReplicationState { | ||
43 | char *top_id; | ||
44 | ReplicationState *rs; | ||
45 | Error *blocker; | ||
46 | - int orig_hidden_flags; | ||
47 | - int orig_secondary_flags; | ||
48 | + bool orig_hidden_read_only; | ||
49 | + bool orig_secondary_read_only; | ||
50 | int error; | ||
51 | } BDRVReplicationState; | ||
52 | |||
53 | @@ -XXX,XX +XXX,XX @@ static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp) | ||
54 | } | ||
55 | } | ||
56 | |||
57 | +/* This function is supposed to be called twice: | ||
58 | + * first with writable = true, then with writable = false. | ||
59 | + * The first call puts s->hidden_disk and s->secondary_disk in | ||
60 | + * r/w mode, and the second puts them back in their original state. | ||
61 | + */ | ||
62 | static void reopen_backing_file(BlockDriverState *bs, bool writable, | ||
63 | Error **errp) | ||
64 | { | ||
65 | BDRVReplicationState *s = bs->opaque; | ||
66 | BlockReopenQueue *reopen_queue = NULL; | ||
67 | - int orig_hidden_flags, orig_secondary_flags; | ||
68 | - int new_hidden_flags, new_secondary_flags; | ||
69 | Error *local_err = NULL; | ||
70 | |||
71 | if (writable) { | ||
72 | - orig_hidden_flags = s->orig_hidden_flags = | ||
73 | - bdrv_get_flags(s->hidden_disk->bs); | ||
74 | - new_hidden_flags = (orig_hidden_flags | BDRV_O_RDWR) & | ||
75 | - ~BDRV_O_INACTIVE; | ||
76 | - orig_secondary_flags = s->orig_secondary_flags = | ||
77 | - bdrv_get_flags(s->secondary_disk->bs); | ||
78 | - new_secondary_flags = (orig_secondary_flags | BDRV_O_RDWR) & | ||
79 | - ~BDRV_O_INACTIVE; | ||
80 | - } else { | ||
81 | - orig_hidden_flags = (s->orig_hidden_flags | BDRV_O_RDWR) & | ||
82 | - ~BDRV_O_INACTIVE; | ||
83 | - new_hidden_flags = s->orig_hidden_flags; | ||
84 | - orig_secondary_flags = (s->orig_secondary_flags | BDRV_O_RDWR) & | ||
85 | - ~BDRV_O_INACTIVE; | ||
86 | - new_secondary_flags = s->orig_secondary_flags; | ||
87 | + s->orig_hidden_read_only = bdrv_is_read_only(s->hidden_disk->bs); | ||
88 | + s->orig_secondary_read_only = bdrv_is_read_only(s->secondary_disk->bs); | ||
89 | } | ||
90 | |||
91 | bdrv_subtree_drained_begin(s->hidden_disk->bs); | ||
92 | bdrv_subtree_drained_begin(s->secondary_disk->bs); | ||
93 | |||
94 | - if (orig_hidden_flags != new_hidden_flags) { | ||
95 | - reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, NULL, | ||
96 | - new_hidden_flags); | ||
97 | + if (s->orig_hidden_read_only) { | ||
98 | + int flags = bdrv_get_flags(s->hidden_disk->bs); | ||
99 | + QDict *opts = qdict_new(); | ||
100 | + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); | ||
101 | + reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, | ||
102 | + opts, flags); | ||
103 | } | ||
104 | |||
105 | - if (!(orig_secondary_flags & BDRV_O_RDWR)) { | ||
106 | + if (s->orig_secondary_read_only) { | ||
107 | + int flags = bdrv_get_flags(s->secondary_disk->bs); | ||
108 | + QDict *opts = qdict_new(); | ||
109 | + qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); | ||
110 | reopen_queue = bdrv_reopen_queue(reopen_queue, s->secondary_disk->bs, | ||
111 | - NULL, new_secondary_flags); | ||
112 | + opts, flags); | ||
113 | } | ||
114 | |||
115 | if (reopen_queue) { | ||
116 | -- | ||
117 | 2.19.2 | ||
118 | |||
119 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | Now that all callers are passing all flag changes as QDict options, | ||
4 | the flags parameter is no longer necessary, so we can get rid of it. | ||
5 | |||
6 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
7 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
8 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | --- | ||
10 | include/block/block.h | 3 +-- | ||
11 | block.c | 5 +++-- | ||
12 | block/replication.c | 6 ++---- | ||
13 | qemu-io-cmds.c | 2 +- | ||
14 | 4 files changed, 7 insertions(+), 9 deletions(-) | ||
15 | |||
16 | diff --git a/include/block/block.h b/include/block/block.h | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/include/block/block.h | ||
19 | +++ b/include/block/block.h | ||
20 | @@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_open(const char *filename, const char *reference, | ||
21 | BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name, | ||
22 | int flags, Error **errp); | ||
23 | BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
24 | - BlockDriverState *bs, | ||
25 | - QDict *options, int flags); | ||
26 | + BlockDriverState *bs, QDict *options); | ||
27 | int bdrv_reopen_multiple(AioContext *ctx, BlockReopenQueue *bs_queue, Error **errp); | ||
28 | int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
29 | Error **errp); | ||
30 | diff --git a/block.c b/block.c | ||
31 | index XXXXXXX..XXXXXXX 100644 | ||
32 | --- a/block.c | ||
33 | +++ b/block.c | ||
34 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
35 | |||
36 | BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
37 | BlockDriverState *bs, | ||
38 | - QDict *options, int flags) | ||
39 | + QDict *options) | ||
40 | { | ||
41 | + int flags = bdrv_get_flags(bs); | ||
42 | return bdrv_reopen_queue_child(bs_queue, bs, options, flags, | ||
43 | NULL, NULL, 0); | ||
44 | } | ||
45 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_set_read_only(BlockDriverState *bs, bool read_only, | ||
46 | qdict_put_bool(opts, BDRV_OPT_READ_ONLY, read_only); | ||
47 | |||
48 | bdrv_subtree_drained_begin(bs); | ||
49 | - queue = bdrv_reopen_queue(NULL, bs, opts, bdrv_get_flags(bs)); | ||
50 | + queue = bdrv_reopen_queue(NULL, bs, opts); | ||
51 | ret = bdrv_reopen_multiple(bdrv_get_aio_context(bs), queue, errp); | ||
52 | bdrv_subtree_drained_end(bs); | ||
53 | |||
54 | diff --git a/block/replication.c b/block/replication.c | ||
55 | index XXXXXXX..XXXXXXX 100644 | ||
56 | --- a/block/replication.c | ||
57 | +++ b/block/replication.c | ||
58 | @@ -XXX,XX +XXX,XX @@ static void reopen_backing_file(BlockDriverState *bs, bool writable, | ||
59 | bdrv_subtree_drained_begin(s->secondary_disk->bs); | ||
60 | |||
61 | if (s->orig_hidden_read_only) { | ||
62 | - int flags = bdrv_get_flags(s->hidden_disk->bs); | ||
63 | QDict *opts = qdict_new(); | ||
64 | qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); | ||
65 | reopen_queue = bdrv_reopen_queue(reopen_queue, s->hidden_disk->bs, | ||
66 | - opts, flags); | ||
67 | + opts); | ||
68 | } | ||
69 | |||
70 | if (s->orig_secondary_read_only) { | ||
71 | - int flags = bdrv_get_flags(s->secondary_disk->bs); | ||
72 | QDict *opts = qdict_new(); | ||
73 | qdict_put_bool(opts, BDRV_OPT_READ_ONLY, !writable); | ||
74 | reopen_queue = bdrv_reopen_queue(reopen_queue, s->secondary_disk->bs, | ||
75 | - opts, flags); | ||
76 | + opts); | ||
77 | } | ||
78 | |||
79 | if (reopen_queue) { | ||
80 | diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c | ||
81 | index XXXXXXX..XXXXXXX 100644 | ||
82 | --- a/qemu-io-cmds.c | ||
83 | +++ b/qemu-io-cmds.c | ||
84 | @@ -XXX,XX +XXX,XX @@ static int reopen_f(BlockBackend *blk, int argc, char **argv) | ||
85 | } | ||
86 | |||
87 | bdrv_subtree_drained_begin(bs); | ||
88 | - brq = bdrv_reopen_queue(NULL, bs, opts, flags); | ||
89 | + brq = bdrv_reopen_queue(NULL, bs, opts); | ||
90 | bdrv_reopen_multiple(bdrv_get_aio_context(bs), brq, &local_err); | ||
91 | bdrv_subtree_drained_end(bs); | ||
92 | |||
93 | -- | ||
94 | 2.19.2 | ||
95 | |||
96 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | Now that all callers are passing the new options using the QDict we no | ||
4 | longer need the 'flags' parameter. | ||
5 | |||
6 | This patch makes the following changes: | ||
7 | |||
8 | 1) The update_options_from_flags() call is no longer necessary | ||
9 | so it can be removed. | ||
10 | |||
11 | 2) The update_flags_from_options() call is now used in all cases, | ||
12 | and is moved down a few lines so it happens after the options | ||
13 | QDict contains the final set of values. | ||
14 | |||
15 | 3) The flags parameter is removed. Now the flags are initialized | ||
16 | using the current value (for the top-level node) or the parent | ||
17 | flags (after inherit_options()). In both cases the initial | ||
18 | values are updated to reflect the new options in the QDict. This | ||
19 | happens in bdrv_reopen_queue_child() (as explained above) and in | ||
20 | bdrv_reopen_prepare(). | ||
21 | |||
22 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
23 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
24 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
25 | --- | ||
26 | block.c | 48 +++++++++++++++++++----------------------------- | ||
27 | 1 file changed, 19 insertions(+), 29 deletions(-) | ||
28 | |||
29 | diff --git a/block.c b/block.c | ||
30 | index XXXXXXX..XXXXXXX 100644 | ||
31 | --- a/block.c | ||
32 | +++ b/block.c | ||
33 | @@ -XXX,XX +XXX,XX @@ BlockDriverState *bdrv_open(const char *filename, const char *reference, | ||
34 | static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
35 | BlockDriverState *bs, | ||
36 | QDict *options, | ||
37 | - int flags, | ||
38 | const BdrvChildRole *role, | ||
39 | QDict *parent_options, | ||
40 | int parent_flags) | ||
41 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
42 | |||
43 | BlockReopenQueueEntry *bs_entry; | ||
44 | BdrvChild *child; | ||
45 | - QDict *old_options, *explicit_options; | ||
46 | + QDict *old_options, *explicit_options, *options_copy; | ||
47 | + int flags; | ||
48 | + QemuOpts *opts; | ||
49 | |||
50 | /* Make sure that the caller remembered to use a drained section. This is | ||
51 | * important to avoid graph changes between the recursive queuing here and | ||
52 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
53 | /* | ||
54 | * Precedence of options: | ||
55 | * 1. Explicitly passed in options (highest) | ||
56 | - * 2. Set in flags (only for top level) | ||
57 | - * 3. Retained from explicitly set options of bs | ||
58 | - * 4. Inherited from parent node | ||
59 | - * 5. Retained from effective options of bs | ||
60 | + * 2. Retained from explicitly set options of bs | ||
61 | + * 3. Inherited from parent node | ||
62 | + * 4. Retained from effective options of bs | ||
63 | */ | ||
64 | |||
65 | - if (!parent_options) { | ||
66 | - /* | ||
67 | - * Any setting represented by flags is always updated. If the | ||
68 | - * corresponding QDict option is set, it takes precedence. Otherwise | ||
69 | - * the flag is translated into a QDict option. The old setting of bs is | ||
70 | - * not considered. | ||
71 | - */ | ||
72 | - update_options_from_flags(options, flags); | ||
73 | - } | ||
74 | - | ||
75 | /* Old explicitly set values (don't overwrite by inherited value) */ | ||
76 | if (bs_entry) { | ||
77 | old_options = qdict_clone_shallow(bs_entry->state.explicit_options); | ||
78 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
79 | |||
80 | /* Inherit from parent node */ | ||
81 | if (parent_options) { | ||
82 | - QemuOpts *opts; | ||
83 | - QDict *options_copy; | ||
84 | - assert(!flags); | ||
85 | + flags = 0; | ||
86 | role->inherit_options(&flags, options, parent_flags, parent_options); | ||
87 | - options_copy = qdict_clone_shallow(options); | ||
88 | - opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); | ||
89 | - qemu_opts_absorb_qdict(opts, options_copy, NULL); | ||
90 | - update_flags_from_options(&flags, opts); | ||
91 | - qemu_opts_del(opts); | ||
92 | - qobject_unref(options_copy); | ||
93 | + } else { | ||
94 | + flags = bdrv_get_flags(bs); | ||
95 | } | ||
96 | |||
97 | /* Old values are used for options that aren't set yet */ | ||
98 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
99 | bdrv_join_options(bs, options, old_options); | ||
100 | qobject_unref(old_options); | ||
101 | |||
102 | + /* We have the final set of options so let's update the flags */ | ||
103 | + options_copy = qdict_clone_shallow(options); | ||
104 | + opts = qemu_opts_create(&bdrv_runtime_opts, NULL, 0, &error_abort); | ||
105 | + qemu_opts_absorb_qdict(opts, options_copy, NULL); | ||
106 | + update_flags_from_options(&flags, opts); | ||
107 | + qemu_opts_del(opts); | ||
108 | + qobject_unref(options_copy); | ||
109 | + | ||
110 | /* bdrv_open_inherit() sets and clears some additional flags internally */ | ||
111 | flags &= ~BDRV_O_PROTOCOL; | ||
112 | if (flags & BDRV_O_RDWR) { | ||
113 | @@ -XXX,XX +XXX,XX @@ static BlockReopenQueue *bdrv_reopen_queue_child(BlockReopenQueue *bs_queue, | ||
114 | qdict_extract_subqdict(options, &new_child_options, child_key_dot); | ||
115 | g_free(child_key_dot); | ||
116 | |||
117 | - bdrv_reopen_queue_child(bs_queue, child->bs, new_child_options, 0, | ||
118 | + bdrv_reopen_queue_child(bs_queue, child->bs, new_child_options, | ||
119 | child->role, options, flags); | ||
120 | } | ||
121 | |||
122 | @@ -XXX,XX +XXX,XX @@ BlockReopenQueue *bdrv_reopen_queue(BlockReopenQueue *bs_queue, | ||
123 | BlockDriverState *bs, | ||
124 | QDict *options) | ||
125 | { | ||
126 | - int flags = bdrv_get_flags(bs); | ||
127 | - return bdrv_reopen_queue_child(bs_queue, bs, options, flags, | ||
128 | - NULL, NULL, 0); | ||
129 | + return bdrv_reopen_queue_child(bs_queue, bs, options, NULL, NULL, 0); | ||
130 | } | ||
131 | |||
132 | /* | ||
133 | -- | ||
134 | 2.19.2 | ||
135 | |||
136 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Alberto Garcia <berto@igalia.com> | ||
2 | 1 | ||
3 | Towards the end of bdrv_reopen_queue_child(), before starting to | ||
4 | process the children, the update_flags_from_options() function is | ||
5 | called in order to have BDRVReopenState.flags in sync with the options | ||
6 | from the QDict. | ||
7 | |||
8 | This is necessary because during the reopen process flags must be | ||
9 | updated for all nodes in the queue so bdrv_is_writable_after_reopen() | ||
10 | and the permission checks work correctly. | ||
11 | |||
12 | Because of that, calling update_flags_from_options() again in | ||
13 | bdrv_reopen_prepare() doesn't really change the flags (they are | ||
14 | already up-to-date). But we need to call it in order to remove those | ||
15 | options from QemuOpts and that way indicate that they have been | ||
16 | processed. | ||
17 | |||
18 | Signed-off-by: Alberto Garcia <berto@igalia.com> | ||
19 | Reviewed-by: Max Reitz <mreitz@redhat.com> | ||
20 | Signed-off-by: Kevin Wolf <kwolf@redhat.com> | ||
21 | --- | ||
22 | block.c | 6 ++++++ | ||
23 | 1 file changed, 6 insertions(+) | ||
24 | |||
25 | diff --git a/block.c b/block.c | ||
26 | index XXXXXXX..XXXXXXX 100644 | ||
27 | --- a/block.c | ||
28 | +++ b/block.c | ||
29 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, | ||
30 | Error **errp) | ||
31 | { | ||
32 | int ret = -1; | ||
33 | + int old_flags; | ||
34 | Error *local_err = NULL; | ||
35 | BlockDriver *drv; | ||
36 | QemuOpts *opts; | ||
37 | @@ -XXX,XX +XXX,XX @@ int bdrv_reopen_prepare(BDRVReopenState *reopen_state, BlockReopenQueue *queue, | ||
38 | goto error; | ||
39 | } | ||
40 | |||
41 | + /* This was already called in bdrv_reopen_queue_child() so the flags | ||
42 | + * are up-to-date. This time we simply want to remove the options from | ||
43 | + * QemuOpts in order to indicate that they have been processed. */ | ||
44 | + old_flags = reopen_state->flags; | ||
45 | update_flags_from_options(&reopen_state->flags, opts); | ||
46 | + assert(old_flags == reopen_state->flags); | ||
47 | |||
48 | discard = qemu_opt_get_del(opts, BDRV_OPT_DISCARD); | ||
49 | if (discard != NULL) { | ||
50 | -- | ||
51 | 2.19.2 | ||
52 | |||
53 | diff view generated by jsdifflib |