1 | The following changes since commit 1d60bb4b14601e38ed17384277aa4c30c57925d3: | 1 | The following changes since commit 417296c8d8588f782018d01a317f88957e9786d6: |
---|---|---|---|
2 | 2 | ||
3 | Merge tag 'pull-request-2022-03-15v2' of https://gitlab.com/thuth/qemu into staging (2022-03-16 10:43:58 +0000) | 3 | tests/qtest/netdev-socket: Raise connection timeout to 60 seconds (2023-02-09 11:23:53 +0000) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to fc8796465c6cd4091efe6a2f8b353f07324f49c7: | 9 | for you to fetch changes up to acbc8aee5b09222dc6a5cb88306b67bcbe37e30b: |
10 | 10 | ||
11 | aio-posix: fix spurious ->poll_ready() callbacks in main loop (2022-03-17 11:23:18 +0000) | 11 | iotests/detect-zeroes-registered-buf: add new test (2023-02-09 10:22:30 -0500) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | Bug fixes for 7.0. | 16 | A few fixes that I've picked up. |
17 | 17 | ||
18 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
19 | 19 | ||
20 | Haiyue Wang (1): | 20 | Akihiko Odaki (1): |
21 | aio-posix: fix build failure io_uring 2.2 | 21 | vhost-user-fs: Back up vqs before cleaning up vhost_dev |
22 | 22 | ||
23 | Stefan Hajnoczi (1): | 23 | Emanuele Giuseppe Esposito (1): |
24 | aio-posix: fix spurious ->poll_ready() callbacks in main loop | 24 | virtio-blk: add missing AioContext lock |
25 | 25 | ||
26 | util/aio-posix.h | 1 + | 26 | Stefan Hajnoczi (4): |
27 | util/aio-posix.c | 32 ++++++++++++++++++-------------- | 27 | block: fix detect-zeroes= with BDRV_REQ_REGISTERED_BUF |
28 | util/fdmon-io_uring.c | 4 ++++ | 28 | qemu-io: use BdrvRequestFlags instead of int |
29 | 3 files changed, 23 insertions(+), 14 deletions(-) | 29 | qemu-io: add -r option to register I/O buffer |
30 | iotests/detect-zeroes-registered-buf: add new test | ||
31 | |||
32 | block/io.c | 3 + | ||
33 | hw/block/virtio-blk.c | 5 + | ||
34 | hw/virtio/vhost-user-fs.c | 4 +- | ||
35 | qemu-io-cmds.c | 215 +++++++++++------- | ||
36 | .../tests/detect-zeroes-registered-buf | 58 +++++ | ||
37 | .../tests/detect-zeroes-registered-buf.out | 7 + | ||
38 | 6 files changed, 210 insertions(+), 82 deletions(-) | ||
39 | create mode 100755 tests/qemu-iotests/tests/detect-zeroes-registered-buf | ||
40 | create mode 100644 tests/qemu-iotests/tests/detect-zeroes-registered-buf.out | ||
30 | 41 | ||
31 | -- | 42 | -- |
32 | 2.35.1 | 43 | 2.39.1 |
33 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Akihiko Odaki <akihiko.odaki@daynix.com> | ||
1 | 2 | ||
3 | vhost_dev_cleanup() clears vhost_dev so back up its vqs member to free | ||
4 | the memory pointed by the member. | ||
5 | |||
6 | Fixes: 98fc1ada4c ("virtio: add vhost-user-fs base device") | ||
7 | Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> | ||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | Message-Id: <20230130140225.77964-1-akihiko.odaki@daynix.com> | ||
10 | --- | ||
11 | hw/virtio/vhost-user-fs.c | 4 ++-- | ||
12 | 1 file changed, 2 insertions(+), 2 deletions(-) | ||
13 | |||
14 | diff --git a/hw/virtio/vhost-user-fs.c b/hw/virtio/vhost-user-fs.c | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/hw/virtio/vhost-user-fs.c | ||
17 | +++ b/hw/virtio/vhost-user-fs.c | ||
18 | @@ -XXX,XX +XXX,XX @@ static void vuf_device_unrealize(DeviceState *dev) | ||
19 | { | ||
20 | VirtIODevice *vdev = VIRTIO_DEVICE(dev); | ||
21 | VHostUserFS *fs = VHOST_USER_FS(dev); | ||
22 | + struct vhost_virtqueue *vhost_vqs = fs->vhost_dev.vqs; | ||
23 | int i; | ||
24 | |||
25 | /* This will stop vhost backend if appropriate. */ | ||
26 | @@ -XXX,XX +XXX,XX @@ static void vuf_device_unrealize(DeviceState *dev) | ||
27 | } | ||
28 | g_free(fs->req_vqs); | ||
29 | virtio_cleanup(vdev); | ||
30 | - g_free(fs->vhost_dev.vqs); | ||
31 | - fs->vhost_dev.vqs = NULL; | ||
32 | + g_free(vhost_vqs); | ||
33 | } | ||
34 | |||
35 | static struct vhost_dev *vuf_get_vhost(VirtIODevice *vdev) | ||
36 | -- | ||
37 | 2.39.1 | diff view generated by jsdifflib |
1 | From: Haiyue Wang <haiyue.wang@intel.com> | 1 | From: Emanuele Giuseppe Esposito <eesposit@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The io_uring fixed "Don't truncate addr fields to 32-bit on 32-bit": | 3 | virtio_blk_update_config() calls blk_get_geometry and blk_getlength, |
4 | https://git.kernel.dk/cgit/liburing/commit/?id=d84c29b19ed0b130000619cff40141bb1fc3615b | 4 | and both functions eventually end up calling bdrv_poll_co when not |
5 | running in a coroutine: | ||
6 | - blk_getlength is a co_wrapper_mixed function | ||
7 | - blk_get_geometry calls bdrv_get_geometry -> bdrv_nb_sectors, a | ||
8 | co_wrapper_mixed function too | ||
5 | 9 | ||
6 | This leads to build failure: | 10 | Since we are not running in a coroutine, we need to take s->blk |
7 | ../util/fdmon-io_uring.c: In function ‘add_poll_remove_sqe’: | 11 | AioContext lock, otherwise bdrv_poll_co will inevitably call |
8 | ../util/fdmon-io_uring.c:182:36: error: passing argument 2 of ‘io_uring_prep_poll_remove’ makes integer from pointer without a cast [-Werror=int-conversion] | 12 | AIO_WAIT_WHILE and therefore try to un unlock() an AioContext lock |
9 | 182 | io_uring_prep_poll_remove(sqe, node); | 13 | that was never acquired. |
10 | | ^~~~ | ||
11 | | | | ||
12 | | AioHandler * | ||
13 | In file included from /root/io/qemu/include/block/aio.h:18, | ||
14 | from ../util/aio-posix.h:20, | ||
15 | from ../util/fdmon-io_uring.c:49: | ||
16 | /usr/include/liburing.h:415:17: note: expected ‘__u64’ {aka ‘long long unsigned int’} but argument is of type ‘AioHandler *’ | ||
17 | 415 | __u64 user_data) | ||
18 | | ~~~~~~^~~~~~~~~ | ||
19 | cc1: all warnings being treated as errors | ||
20 | 14 | ||
21 | Use LIBURING_HAVE_DATA64 to check whether the io_uring supports 64-bit | 15 | RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=2167838 |
22 | variants of the get/set userdata, to convert the paramter to the right | ||
23 | data type. | ||
24 | 16 | ||
25 | Signed-off-by: Haiyue Wang <haiyue.wang@intel.com> | 17 | Steps to reproduce the issue: simply boot a VM with |
26 | Message-Id: <20220221162401.45415-1-haiyue.wang@intel.com> | 18 | -object '{"qom-type":"iothread","id":"iothread1"}' \ |
19 | -blockdev '{"driver":"file","filename":"$QCOW2","aio":"native","node-name":"libvirt-1-storage","cache":{"direct":true,"no-flush":false},"auto-read-only":true,"discard":"unmap"}' \ | ||
20 | -blockdev '{"node-name":"libvirt-1-format","read-only":false,"cache":{"direct":true,"no-flush":false},"driver":"qcow2","file":"libvirt-1-storage"}' \ | ||
21 | -device virtio-blk-pci,iothread=iothread1,drive=libvirt-1-format,id=virtio-disk0,bootindex=1,write-cache=on | ||
22 | |||
23 | and observe that it will fail not manage to boot with "qemu_mutex_unlock_impl: Operation not permitted" | ||
24 | |||
25 | Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> | ||
26 | Acked-by: Michael S. Tsirkin <mst@redhat.com> | ||
27 | Tested-by: Lukáš Doktor <ldoktor@redhat.com> | ||
27 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 28 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
29 | Message-Id: <20230208111148.1040083-1-eesposit@redhat.com> | ||
28 | --- | 30 | --- |
29 | util/fdmon-io_uring.c | 4 ++++ | 31 | hw/block/virtio-blk.c | 5 +++++ |
30 | 1 file changed, 4 insertions(+) | 32 | 1 file changed, 5 insertions(+) |
31 | 33 | ||
32 | diff --git a/util/fdmon-io_uring.c b/util/fdmon-io_uring.c | 34 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c |
33 | index XXXXXXX..XXXXXXX 100644 | 35 | index XXXXXXX..XXXXXXX 100644 |
34 | --- a/util/fdmon-io_uring.c | 36 | --- a/hw/block/virtio-blk.c |
35 | +++ b/util/fdmon-io_uring.c | 37 | +++ b/hw/block/virtio-blk.c |
36 | @@ -XXX,XX +XXX,XX @@ static void add_poll_remove_sqe(AioContext *ctx, AioHandler *node) | 38 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) |
37 | { | 39 | uint64_t capacity; |
38 | struct io_uring_sqe *sqe = get_sqe(ctx); | 40 | int64_t length; |
39 | 41 | int blk_size = conf->logical_block_size; | |
40 | +#ifdef LIBURING_HAVE_DATA64 | 42 | + AioContext *ctx; |
41 | + io_uring_prep_poll_remove(sqe, (__u64)(uintptr_t)node); | 43 | + |
42 | +#else | 44 | + ctx = blk_get_aio_context(s->blk); |
43 | io_uring_prep_poll_remove(sqe, node); | 45 | + aio_context_acquire(ctx); |
44 | +#endif | 46 | |
45 | } | 47 | blk_get_geometry(s->blk, &capacity); |
46 | 48 | memset(&blkcfg, 0, sizeof(blkcfg)); | |
47 | /* Add a timeout that self-cancels when another cqe becomes ready */ | 49 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config) |
50 | * per track (cylinder). | ||
51 | */ | ||
52 | length = blk_getlength(s->blk); | ||
53 | + aio_context_release(ctx); | ||
54 | if (length > 0 && length / conf->heads / conf->secs % blk_size) { | ||
55 | blkcfg.geometry.sectors = conf->secs & ~s->sector_mask; | ||
56 | } else { | ||
48 | -- | 57 | -- |
49 | 2.35.1 | 58 | 2.39.1 |
50 | 59 | ||
51 | 60 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | When a write request is converted into a write zeroes request by the | ||
2 | detect-zeroes= feature, it is no longer associated with an I/O buffer. | ||
3 | The BDRV_REQ_REGISTERED_BUF flag doesn't make sense without an I/O | ||
4 | buffer and must be cleared because bdrv_co_do_pwrite_zeroes() fails with | ||
5 | -EINVAL when it's set. | ||
1 | 6 | ||
7 | Fiona Ebner <f.ebner@proxmox.com> bisected and diagnosed this QEMU 7.2 | ||
8 | regression where writes containing zeroes to a blockdev with | ||
9 | discard=unmap,detect-zeroes=unmap fail. | ||
10 | |||
11 | Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1404 | ||
12 | Fixes: e8b6535533be ("block: add BDRV_REQ_REGISTERED_BUF request flag") | ||
13 | Tested-by: Fiona Ebner <f.ebner@proxmox.com> | ||
14 | Cc: qemu-stable@nongnu.org | ||
15 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
16 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
17 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
18 | Message-Id: <20230207203719.242926-2-stefanha@redhat.com> | ||
19 | --- | ||
20 | block/io.c | 3 +++ | ||
21 | 1 file changed, 3 insertions(+) | ||
22 | |||
23 | diff --git a/block/io.c b/block/io.c | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/block/io.c | ||
26 | +++ b/block/io.c | ||
27 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child, | ||
28 | if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) { | ||
29 | flags |= BDRV_REQ_MAY_UNMAP; | ||
30 | } | ||
31 | + | ||
32 | + /* Can't use optimization hint with bufferless zero write */ | ||
33 | + flags &= ~BDRV_REQ_REGISTERED_BUF; | ||
34 | } | ||
35 | |||
36 | if (ret < 0) { | ||
37 | -- | ||
38 | 2.39.1 | diff view generated by jsdifflib |
1 | When ->poll() succeeds the AioHandler is placed on the ready list with | 1 | The block layer APIs use BdrvRequestFlags while qemu-io code uses int. |
---|---|---|---|
2 | revents set to the magic value 0. This magic value causes | 2 | Although the code compiles and runs fine, BdrvRequestFlags is clearer |
3 | aio_dispatch_handler() to invoke ->poll_ready() instead of ->io_read() | 3 | because it differentiates between other types of flags like bdrv_open() |
4 | for G_IO_IN or ->io_write() for G_IO_OUT. | 4 | flags. |
5 | 5 | ||
6 | This magic value 0 hack works for the IOThread where AioHandlers are | 6 | This is purely refactoring. |
7 | placed on ->ready_list and processed by aio_dispatch_ready_handlers(). | ||
8 | It does not work for the main loop where all AioHandlers are processed | ||
9 | by aio_dispatch_handlers(), even those that are not ready and have a | ||
10 | revents value of 0. | ||
11 | 7 | ||
12 | As a result the main loop invokes ->poll_ready() on AioHandlers that are | 8 | Reviewed-by: Eric Blake <eblake@redhat.com> |
13 | not ready. These spurious ->poll_ready() calls waste CPU cycles and | 9 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> |
14 | could lead to crashes if the code assumes ->poll() must have succeeded | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | before ->poll_ready() is called (a reasonable asumption but I haven't | 11 | Message-Id: <20230207203719.242926-3-stefanha@redhat.com> |
16 | seen it in practice). | 12 | --- |
13 | qemu-io-cmds.c | 13 +++++++------ | ||
14 | 1 file changed, 7 insertions(+), 6 deletions(-) | ||
17 | 15 | ||
18 | Stop using revents to track whether ->poll_ready() will be called on an | 16 | diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c |
19 | AioHandler. Introduce a separate AioHandler->poll_ready field instead. | ||
20 | This eliminates spurious ->poll_ready() calls in the main loop. | ||
21 | |||
22 | Fixes: 826cc32423db2a99d184dbf4f507c737d7e7a4ae ("aio-posix: split poll check from ready handler") | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
24 | Reported-by: Jason Wang <jasowang@redhat.com> | ||
25 | Tested-by: Jason Wang <jasowang@redhat.com> | ||
26 | Message-id: 20220223155703.136833-1-stefanha@redhat.com | ||
27 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
28 | --- | ||
29 | util/aio-posix.h | 1 + | ||
30 | util/aio-posix.c | 32 ++++++++++++++++++-------------- | ||
31 | 2 files changed, 19 insertions(+), 14 deletions(-) | ||
32 | |||
33 | diff --git a/util/aio-posix.h b/util/aio-posix.h | ||
34 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
35 | --- a/util/aio-posix.h | 18 | --- a/qemu-io-cmds.c |
36 | +++ b/util/aio-posix.h | 19 | +++ b/qemu-io-cmds.c |
37 | @@ -XXX,XX +XXX,XX @@ struct AioHandler { | 20 | @@ -XXX,XX +XXX,XX @@ static int do_pread(BlockBackend *blk, char *buf, int64_t offset, |
38 | unsigned flags; /* see fdmon-io_uring.c */ | ||
39 | #endif | ||
40 | int64_t poll_idle_timeout; /* when to stop userspace polling */ | ||
41 | + bool poll_ready; /* has polling detected an event? */ | ||
42 | bool is_external; | ||
43 | }; | ||
44 | |||
45 | diff --git a/util/aio-posix.c b/util/aio-posix.c | ||
46 | index XXXXXXX..XXXXXXX 100644 | ||
47 | --- a/util/aio-posix.c | ||
48 | +++ b/util/aio-posix.c | ||
49 | @@ -XXX,XX +XXX,XX @@ | ||
50 | #include "trace.h" | ||
51 | #include "aio-posix.h" | ||
52 | |||
53 | -/* | ||
54 | - * G_IO_IN and G_IO_OUT are not appropriate revents values for polling, since | ||
55 | - * the handler may not need to access the file descriptor. For example, the | ||
56 | - * handler doesn't need to read from an EventNotifier if it polled a memory | ||
57 | - * location and a read syscall would be slow. Define our own unique revents | ||
58 | - * value to indicate that polling determined this AioHandler is ready. | ||
59 | - */ | ||
60 | -#define REVENTS_POLL_READY 0 | ||
61 | - | ||
62 | /* Stop userspace polling on a handler if it isn't active for some time */ | ||
63 | #define POLL_IDLE_INTERVAL_NS (7 * NANOSECONDS_PER_SECOND) | ||
64 | |||
65 | @@ -XXX,XX +XXX,XX @@ void aio_add_ready_handler(AioHandlerList *ready_list, | ||
66 | QLIST_INSERT_HEAD(ready_list, node, node_ready); | ||
67 | } | 21 | } |
68 | 22 | ||
69 | +static void aio_add_poll_ready_handler(AioHandlerList *ready_list, | 23 | static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, |
70 | + AioHandler *node) | 24 | - int64_t bytes, int flags, int64_t *total) |
71 | +{ | 25 | + int64_t bytes, BdrvRequestFlags flags, int64_t *total) |
72 | + QLIST_SAFE_REMOVE(node, node_ready); /* remove from nested parent's list */ | ||
73 | + node->poll_ready = true; | ||
74 | + QLIST_INSERT_HEAD(ready_list, node, node_ready); | ||
75 | +} | ||
76 | + | ||
77 | static AioHandler *find_aio_handler(AioContext *ctx, int fd) | ||
78 | { | 26 | { |
79 | AioHandler *node; | 27 | int ret; |
80 | @@ -XXX,XX +XXX,XX @@ static bool aio_remove_fd_handler(AioContext *ctx, AioHandler *node) | 28 | |
81 | } | 29 | @@ -XXX,XX +XXX,XX @@ static int do_pwrite(BlockBackend *blk, char *buf, int64_t offset, |
82 | 30 | } | |
83 | node->pfd.revents = 0; | 31 | |
84 | + node->poll_ready = false; | 32 | static int do_pwrite_zeroes(BlockBackend *blk, int64_t offset, |
85 | 33 | - int64_t bytes, int flags, int64_t *total) | |
86 | /* If the fd monitor has already marked it deleted, leave it alone */ | 34 | + int64_t bytes, BdrvRequestFlags flags, |
87 | if (QLIST_IS_INSERTED(node, node_deleted)) { | 35 | + int64_t *total) |
88 | @@ -XXX,XX +XXX,XX @@ static bool poll_set_started(AioContext *ctx, AioHandlerList *ready_list, | ||
89 | |||
90 | /* Poll one last time in case ->io_poll_end() raced with the event */ | ||
91 | if (!started && node->io_poll(node->opaque)) { | ||
92 | - aio_add_ready_handler(ready_list, node, REVENTS_POLL_READY); | ||
93 | + aio_add_poll_ready_handler(ready_list, node); | ||
94 | progress = true; | ||
95 | } | ||
96 | } | ||
97 | @@ -XXX,XX +XXX,XX @@ bool aio_pending(AioContext *ctx) | ||
98 | QLIST_FOREACH_RCU(node, &ctx->aio_handlers, node) { | ||
99 | int revents; | ||
100 | |||
101 | + /* TODO should this check poll ready? */ | ||
102 | revents = node->pfd.revents & node->pfd.events; | ||
103 | if (revents & (G_IO_IN | G_IO_HUP | G_IO_ERR) && node->io_read && | ||
104 | aio_node_check(ctx, node->is_external)) { | ||
105 | @@ -XXX,XX +XXX,XX @@ static void aio_free_deleted_handlers(AioContext *ctx) | ||
106 | static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node) | ||
107 | { | 36 | { |
108 | bool progress = false; | 37 | int ret = blk_pwrite_zeroes(blk, offset, bytes, |
109 | + bool poll_ready; | 38 | flags | BDRV_REQ_ZERO_WRITE); |
110 | int revents; | 39 | @@ -XXX,XX +XXX,XX @@ static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov, |
111 | 40 | } | |
112 | revents = node->pfd.revents & node->pfd.events; | 41 | |
113 | node->pfd.revents = 0; | 42 | static int do_aio_writev(BlockBackend *blk, QEMUIOVector *qiov, |
114 | 43 | - int64_t offset, int flags, int *total) | |
115 | + poll_ready = node->poll_ready; | 44 | + int64_t offset, BdrvRequestFlags flags, int *total) |
116 | + node->poll_ready = false; | 45 | { |
117 | + | 46 | int async_ret = NOT_DONE; |
118 | /* | 47 | |
119 | * Start polling AioHandlers when they become ready because activity is | 48 | @@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv) |
120 | * likely to continue. Note that starvation is theoretically possible when | 49 | struct timespec t1, t2; |
121 | @@ -XXX,XX +XXX,XX @@ static bool aio_dispatch_handler(AioContext *ctx, AioHandler *node) | 50 | bool Cflag = false, qflag = false, bflag = false; |
122 | QLIST_INSERT_HEAD(&ctx->poll_aio_handlers, node, node_poll); | 51 | bool Pflag = false, zflag = false, cflag = false, sflag = false; |
123 | } | 52 | - int flags = 0; |
124 | if (!QLIST_IS_INSERTED(node, node_deleted) && | 53 | + BdrvRequestFlags flags = 0; |
125 | - revents == 0 && | 54 | int c, cnt, ret; |
126 | + poll_ready && revents == 0 && | 55 | char *buf = NULL; |
127 | aio_node_check(ctx, node->is_external) && | 56 | int64_t offset; |
128 | node->io_poll_ready) { | 57 | @@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv) |
129 | node->io_poll_ready(node->opaque); | 58 | { |
130 | @@ -XXX,XX +XXX,XX @@ static bool run_poll_handlers_once(AioContext *ctx, | 59 | struct timespec t1, t2; |
131 | QLIST_FOREACH_SAFE(node, &ctx->poll_aio_handlers, node_poll, tmp) { | 60 | bool Cflag = false, qflag = false; |
132 | if (aio_node_check(ctx, node->is_external) && | 61 | - int flags = 0; |
133 | node->io_poll(node->opaque)) { | 62 | + BdrvRequestFlags flags = 0; |
134 | - aio_add_ready_handler(ready_list, node, REVENTS_POLL_READY); | 63 | int c, cnt, ret; |
135 | + aio_add_poll_ready_handler(ready_list, node); | 64 | char *buf; |
136 | 65 | int64_t offset; | |
137 | node->poll_idle_timeout = now + POLL_IDLE_INTERVAL_NS; | 66 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) |
138 | 67 | int nr_iov, c; | |
139 | @@ -XXX,XX +XXX,XX @@ static bool remove_idle_poll_handlers(AioContext *ctx, | 68 | int pattern = 0xcd; |
140 | * this causes progress. | 69 | struct aio_ctx *ctx = g_new0(struct aio_ctx, 1); |
141 | */ | 70 | - int flags = 0; |
142 | if (node->io_poll(node->opaque)) { | 71 | + BdrvRequestFlags flags = 0; |
143 | - aio_add_ready_handler(ready_list, node, | 72 | |
144 | - REVENTS_POLL_READY); | 73 | ctx->blk = blk; |
145 | + aio_add_poll_ready_handler(ready_list, node); | 74 | while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) { |
146 | progress = true; | ||
147 | } | ||
148 | } | ||
149 | -- | 75 | -- |
150 | 2.35.1 | 76 | 2.39.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | The blk_register_buf() API is an optimization hint that allows some | ||
2 | block drivers to avoid I/O buffer housekeeping or bounce buffers. | ||
1 | 3 | ||
4 | Add an -r option to register the I/O buffer so that qemu-io can be used | ||
5 | to test the blk_register_buf() API. The next commit will add a test that | ||
6 | uses the new option. | ||
7 | |||
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
9 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | Message-Id: <20230207203719.242926-4-stefanha@redhat.com> | ||
12 | --- | ||
13 | qemu-io-cmds.c | 204 +++++++++++++++++++++++++++++++------------------ | ||
14 | 1 file changed, 129 insertions(+), 75 deletions(-) | ||
15 | |||
16 | diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/qemu-io-cmds.c | ||
19 | +++ b/qemu-io-cmds.c | ||
20 | @@ -XXX,XX +XXX,XX @@ static int parse_pattern(const char *arg) | ||
21 | */ | ||
22 | |||
23 | #define MISALIGN_OFFSET 16 | ||
24 | -static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern) | ||
25 | +static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern, | ||
26 | + bool register_buf) | ||
27 | { | ||
28 | void *buf; | ||
29 | |||
30 | @@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc(BlockBackend *blk, size_t len, int pattern) | ||
31 | } | ||
32 | buf = blk_blockalign(blk, len); | ||
33 | memset(buf, pattern, len); | ||
34 | + if (register_buf) { | ||
35 | + blk_register_buf(blk, buf, len, &error_abort); | ||
36 | + } | ||
37 | if (qemuio_misalign) { | ||
38 | buf += MISALIGN_OFFSET; | ||
39 | } | ||
40 | return buf; | ||
41 | } | ||
42 | |||
43 | -static void qemu_io_free(void *p) | ||
44 | +static void qemu_io_free(BlockBackend *blk, void *p, size_t len, | ||
45 | + bool unregister_buf) | ||
46 | { | ||
47 | if (qemuio_misalign) { | ||
48 | p -= MISALIGN_OFFSET; | ||
49 | + len += MISALIGN_OFFSET; | ||
50 | + } | ||
51 | + if (unregister_buf) { | ||
52 | + blk_unregister_buf(blk, p, len); | ||
53 | } | ||
54 | qemu_vfree(p); | ||
55 | } | ||
56 | @@ -XXX,XX +XXX,XX @@ static void qemu_io_free(void *p) | ||
57 | * @blk - the block backend where the buffer content is going to be written to | ||
58 | * @len - the buffer length | ||
59 | * @file_name - the file to read the content from | ||
60 | + * @register_buf - call blk_register_buf() | ||
61 | * | ||
62 | * Returns: the buffer pointer on success | ||
63 | * NULL on error | ||
64 | */ | ||
65 | static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len, | ||
66 | - const char *file_name) | ||
67 | + const char *file_name, bool register_buf) | ||
68 | { | ||
69 | - char *buf, *buf_origin; | ||
70 | + size_t alloc_len = len + (qemuio_misalign ? MISALIGN_OFFSET : 0); | ||
71 | + char *alloc_buf, *buf, *end; | ||
72 | FILE *f = fopen(file_name, "r"); | ||
73 | int pattern_len; | ||
74 | |||
75 | @@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len, | ||
76 | return NULL; | ||
77 | } | ||
78 | |||
79 | - if (qemuio_misalign) { | ||
80 | - len += MISALIGN_OFFSET; | ||
81 | - } | ||
82 | - | ||
83 | - buf_origin = buf = blk_blockalign(blk, len); | ||
84 | + alloc_buf = buf = blk_blockalign(blk, alloc_len); | ||
85 | |||
86 | if (qemuio_misalign) { | ||
87 | - buf_origin += MISALIGN_OFFSET; | ||
88 | buf += MISALIGN_OFFSET; | ||
89 | - len -= MISALIGN_OFFSET; | ||
90 | } | ||
91 | |||
92 | - pattern_len = fread(buf_origin, 1, len, f); | ||
93 | + pattern_len = fread(buf, 1, len, f); | ||
94 | |||
95 | if (ferror(f)) { | ||
96 | perror(file_name); | ||
97 | @@ -XXX,XX +XXX,XX @@ static void *qemu_io_alloc_from_file(BlockBackend *blk, size_t len, | ||
98 | fclose(f); | ||
99 | f = NULL; | ||
100 | |||
101 | - if (len > pattern_len) { | ||
102 | - len -= pattern_len; | ||
103 | - buf += pattern_len; | ||
104 | - | ||
105 | - while (len > 0) { | ||
106 | - size_t len_to_copy = MIN(pattern_len, len); | ||
107 | - | ||
108 | - memcpy(buf, buf_origin, len_to_copy); | ||
109 | + if (register_buf) { | ||
110 | + blk_register_buf(blk, alloc_buf, alloc_len, &error_abort); | ||
111 | + } | ||
112 | |||
113 | - len -= len_to_copy; | ||
114 | - buf += len_to_copy; | ||
115 | - } | ||
116 | + end = buf + len; | ||
117 | + for (char *p = buf + pattern_len; p < end; p += pattern_len) { | ||
118 | + memcpy(p, buf, MIN(pattern_len, end - p)); | ||
119 | } | ||
120 | |||
121 | - return buf_origin; | ||
122 | + return buf; | ||
123 | |||
124 | error: | ||
125 | - qemu_io_free(buf_origin); | ||
126 | + /* | ||
127 | + * This code path is only taken before blk_register_buf() is called, so | ||
128 | + * hardcode the qemu_io_free() unregister_buf argument to false. | ||
129 | + */ | ||
130 | + qemu_io_free(blk, alloc_buf, alloc_len, false); | ||
131 | if (f) { | ||
132 | fclose(f); | ||
133 | } | ||
134 | @@ -XXX,XX +XXX,XX @@ static void print_report(const char *op, struct timespec *t, int64_t offset, | ||
135 | */ | ||
136 | static void * | ||
137 | create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov, | ||
138 | - int pattern) | ||
139 | + int pattern, bool register_buf) | ||
140 | { | ||
141 | size_t *sizes = g_new0(size_t, nr_iov); | ||
142 | size_t count = 0; | ||
143 | @@ -XXX,XX +XXX,XX @@ create_iovec(BlockBackend *blk, QEMUIOVector *qiov, char **argv, int nr_iov, | ||
144 | |||
145 | qemu_iovec_init(qiov, nr_iov); | ||
146 | |||
147 | - buf = p = qemu_io_alloc(blk, count, pattern); | ||
148 | + buf = p = qemu_io_alloc(blk, count, pattern, register_buf); | ||
149 | |||
150 | for (i = 0; i < nr_iov; i++) { | ||
151 | qemu_iovec_add(qiov, p, sizes[i]); | ||
152 | @@ -XXX,XX +XXX,XX @@ fail: | ||
153 | } | ||
154 | |||
155 | static int do_pread(BlockBackend *blk, char *buf, int64_t offset, | ||
156 | - int64_t bytes, int64_t *total) | ||
157 | + int64_t bytes, BdrvRequestFlags flags, int64_t *total) | ||
158 | { | ||
159 | int ret; | ||
160 | |||
161 | @@ -XXX,XX +XXX,XX @@ static int do_pread(BlockBackend *blk, char *buf, int64_t offset, | ||
162 | return -ERANGE; | ||
163 | } | ||
164 | |||
165 | - ret = blk_pread(blk, offset, bytes, (uint8_t *)buf, 0); | ||
166 | + ret = blk_pread(blk, offset, bytes, (uint8_t *)buf, flags); | ||
167 | if (ret < 0) { | ||
168 | return ret; | ||
169 | } | ||
170 | @@ -XXX,XX +XXX,XX @@ static void aio_rw_done(void *opaque, int ret) | ||
171 | } | ||
172 | |||
173 | static int do_aio_readv(BlockBackend *blk, QEMUIOVector *qiov, | ||
174 | - int64_t offset, int *total) | ||
175 | + int64_t offset, BdrvRequestFlags flags, int *total) | ||
176 | { | ||
177 | int async_ret = NOT_DONE; | ||
178 | |||
179 | - blk_aio_preadv(blk, offset, qiov, 0, aio_rw_done, &async_ret); | ||
180 | + blk_aio_preadv(blk, offset, qiov, flags, aio_rw_done, &async_ret); | ||
181 | while (async_ret == NOT_DONE) { | ||
182 | main_loop_wait(false); | ||
183 | } | ||
184 | @@ -XXX,XX +XXX,XX @@ static void read_help(void) | ||
185 | " -p, -- ignored for backwards compatibility\n" | ||
186 | " -P, -- use a pattern to verify read data\n" | ||
187 | " -q, -- quiet mode, do not show I/O statistics\n" | ||
188 | +" -r, -- register I/O buffer\n" | ||
189 | " -s, -- start offset for pattern verification (only with -P)\n" | ||
190 | " -v, -- dump buffer to standard output\n" | ||
191 | "\n"); | ||
192 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t read_cmd = { | ||
193 | .cfunc = read_f, | ||
194 | .argmin = 2, | ||
195 | .argmax = -1, | ||
196 | - .args = "[-abCqv] [-P pattern [-s off] [-l len]] off len", | ||
197 | + .args = "[-abCqrv] [-P pattern [-s off] [-l len]] off len", | ||
198 | .oneline = "reads a number of bytes at a specified offset", | ||
199 | .help = read_help, | ||
200 | }; | ||
201 | @@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv) | ||
202 | int64_t total = 0; | ||
203 | int pattern = 0; | ||
204 | int64_t pattern_offset = 0, pattern_count = 0; | ||
205 | + BdrvRequestFlags flags = 0; | ||
206 | |||
207 | - while ((c = getopt(argc, argv, "bCl:pP:qs:v")) != -1) { | ||
208 | + while ((c = getopt(argc, argv, "bCl:pP:qrs:v")) != -1) { | ||
209 | switch (c) { | ||
210 | case 'b': | ||
211 | bflag = true; | ||
212 | @@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv) | ||
213 | case 'q': | ||
214 | qflag = true; | ||
215 | break; | ||
216 | + case 'r': | ||
217 | + flags |= BDRV_REQ_REGISTERED_BUF; | ||
218 | + break; | ||
219 | case 's': | ||
220 | sflag = true; | ||
221 | pattern_offset = cvtnum(optarg); | ||
222 | @@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv) | ||
223 | count); | ||
224 | return -EINVAL; | ||
225 | } | ||
226 | + if (flags & BDRV_REQ_REGISTERED_BUF) { | ||
227 | + printf("I/O buffer registration is not supported when reading " | ||
228 | + "from vmstate\n"); | ||
229 | + return -EINVAL; | ||
230 | + } | ||
231 | } | ||
232 | |||
233 | - buf = qemu_io_alloc(blk, count, 0xab); | ||
234 | + buf = qemu_io_alloc(blk, count, 0xab, flags & BDRV_REQ_REGISTERED_BUF); | ||
235 | |||
236 | clock_gettime(CLOCK_MONOTONIC, &t1); | ||
237 | if (bflag) { | ||
238 | ret = do_load_vmstate(blk, buf, offset, count, &total); | ||
239 | } else { | ||
240 | - ret = do_pread(blk, buf, offset, count, &total); | ||
241 | + ret = do_pread(blk, buf, offset, count, flags, &total); | ||
242 | } | ||
243 | clock_gettime(CLOCK_MONOTONIC, &t2); | ||
244 | |||
245 | @@ -XXX,XX +XXX,XX @@ static int read_f(BlockBackend *blk, int argc, char **argv) | ||
246 | print_report("read", &t2, offset, count, total, cnt, Cflag); | ||
247 | |||
248 | out: | ||
249 | - qemu_io_free(buf); | ||
250 | + qemu_io_free(blk, buf, count, flags & BDRV_REQ_REGISTERED_BUF); | ||
251 | return ret; | ||
252 | } | ||
253 | |||
254 | @@ -XXX,XX +XXX,XX @@ static void readv_help(void) | ||
255 | " Uses multiple iovec buffers if more than one byte range is specified.\n" | ||
256 | " -C, -- report statistics in a machine parsable format\n" | ||
257 | " -P, -- use a pattern to verify read data\n" | ||
258 | -" -v, -- dump buffer to standard output\n" | ||
259 | " -q, -- quiet mode, do not show I/O statistics\n" | ||
260 | +" -r, -- register I/O buffer\n" | ||
261 | +" -v, -- dump buffer to standard output\n" | ||
262 | "\n"); | ||
263 | } | ||
264 | |||
265 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t readv_cmd = { | ||
266 | .cfunc = readv_f, | ||
267 | .argmin = 2, | ||
268 | .argmax = -1, | ||
269 | - .args = "[-Cqv] [-P pattern] off len [len..]", | ||
270 | + .args = "[-Cqrv] [-P pattern] off len [len..]", | ||
271 | .oneline = "reads a number of bytes at a specified offset", | ||
272 | .help = readv_help, | ||
273 | }; | ||
274 | @@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv) | ||
275 | QEMUIOVector qiov; | ||
276 | int pattern = 0; | ||
277 | bool Pflag = false; | ||
278 | + BdrvRequestFlags flags = 0; | ||
279 | |||
280 | - while ((c = getopt(argc, argv, "CP:qv")) != -1) { | ||
281 | + while ((c = getopt(argc, argv, "CP:qrv")) != -1) { | ||
282 | switch (c) { | ||
283 | case 'C': | ||
284 | Cflag = true; | ||
285 | @@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv) | ||
286 | case 'q': | ||
287 | qflag = true; | ||
288 | break; | ||
289 | + case 'r': | ||
290 | + flags |= BDRV_REQ_REGISTERED_BUF; | ||
291 | + break; | ||
292 | case 'v': | ||
293 | vflag = true; | ||
294 | break; | ||
295 | @@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv) | ||
296 | optind++; | ||
297 | |||
298 | nr_iov = argc - optind; | ||
299 | - buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab); | ||
300 | + buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, 0xab, | ||
301 | + flags & BDRV_REQ_REGISTERED_BUF); | ||
302 | if (buf == NULL) { | ||
303 | return -EINVAL; | ||
304 | } | ||
305 | |||
306 | clock_gettime(CLOCK_MONOTONIC, &t1); | ||
307 | - ret = do_aio_readv(blk, &qiov, offset, &total); | ||
308 | + ret = do_aio_readv(blk, &qiov, offset, flags, &total); | ||
309 | clock_gettime(CLOCK_MONOTONIC, &t2); | ||
310 | |||
311 | if (ret < 0) { | ||
312 | @@ -XXX,XX +XXX,XX @@ static int readv_f(BlockBackend *blk, int argc, char **argv) | ||
313 | print_report("read", &t2, offset, qiov.size, total, cnt, Cflag); | ||
314 | |||
315 | out: | ||
316 | + qemu_io_free(blk, buf, qiov.size, flags & BDRV_REQ_REGISTERED_BUF); | ||
317 | qemu_iovec_destroy(&qiov); | ||
318 | - qemu_io_free(buf); | ||
319 | return ret; | ||
320 | } | ||
321 | |||
322 | @@ -XXX,XX +XXX,XX @@ static void write_help(void) | ||
323 | " filled with a set pattern (0xcdcdcdcd).\n" | ||
324 | " -b, -- write to the VM state rather than the virtual disk\n" | ||
325 | " -c, -- write compressed data with blk_write_compressed\n" | ||
326 | +" -C, -- report statistics in a machine parsable format\n" | ||
327 | " -f, -- use Force Unit Access semantics\n" | ||
328 | " -n, -- with -z, don't allow slow fallback\n" | ||
329 | " -p, -- ignored for backwards compatibility\n" | ||
330 | " -P, -- use different pattern to fill file\n" | ||
331 | +" -q, -- quiet mode, do not show I/O statistics\n" | ||
332 | +" -r, -- register I/O buffer\n" | ||
333 | " -s, -- use a pattern file to fill the write buffer\n" | ||
334 | -" -C, -- report statistics in a machine parsable format\n" | ||
335 | -" -q, -- quiet mode, do not show I/O statistics\n" | ||
336 | " -u, -- with -z, allow unmapping\n" | ||
337 | " -z, -- write zeroes using blk_pwrite_zeroes\n" | ||
338 | "\n"); | ||
339 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t write_cmd = { | ||
340 | .perm = BLK_PERM_WRITE, | ||
341 | .argmin = 2, | ||
342 | .argmax = -1, | ||
343 | - .args = "[-bcCfnquz] [-P pattern | -s source_file] off len", | ||
344 | + .args = "[-bcCfnqruz] [-P pattern | -s source_file] off len", | ||
345 | .oneline = "writes a number of bytes at a specified offset", | ||
346 | .help = write_help, | ||
347 | }; | ||
348 | @@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv) | ||
349 | int pattern = 0xcd; | ||
350 | const char *file_name = NULL; | ||
351 | |||
352 | - while ((c = getopt(argc, argv, "bcCfnpP:qs:uz")) != -1) { | ||
353 | + while ((c = getopt(argc, argv, "bcCfnpP:qrs:uz")) != -1) { | ||
354 | switch (c) { | ||
355 | case 'b': | ||
356 | bflag = true; | ||
357 | @@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv) | ||
358 | case 'q': | ||
359 | qflag = true; | ||
360 | break; | ||
361 | + case 'r': | ||
362 | + flags |= BDRV_REQ_REGISTERED_BUF; | ||
363 | + break; | ||
364 | case 's': | ||
365 | sflag = true; | ||
366 | file_name = optarg; | ||
367 | @@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv) | ||
368 | } | ||
369 | } | ||
370 | |||
371 | - if (!zflag) { | ||
372 | + if (zflag) { | ||
373 | + if (flags & BDRV_REQ_REGISTERED_BUF) { | ||
374 | + printf("cannot combine zero write with registered I/O buffer\n"); | ||
375 | + return -EINVAL; | ||
376 | + } | ||
377 | + } else { | ||
378 | if (sflag) { | ||
379 | - buf = qemu_io_alloc_from_file(blk, count, file_name); | ||
380 | + buf = qemu_io_alloc_from_file(blk, count, file_name, | ||
381 | + flags & BDRV_REQ_REGISTERED_BUF); | ||
382 | if (!buf) { | ||
383 | return -EINVAL; | ||
384 | } | ||
385 | } else { | ||
386 | - buf = qemu_io_alloc(blk, count, pattern); | ||
387 | + buf = qemu_io_alloc(blk, count, pattern, | ||
388 | + flags & BDRV_REQ_REGISTERED_BUF); | ||
389 | } | ||
390 | } | ||
391 | |||
392 | @@ -XXX,XX +XXX,XX @@ static int write_f(BlockBackend *blk, int argc, char **argv) | ||
393 | |||
394 | out: | ||
395 | if (!zflag) { | ||
396 | - qemu_io_free(buf); | ||
397 | + qemu_io_free(blk, buf, count, flags & BDRV_REQ_REGISTERED_BUF); | ||
398 | } | ||
399 | return ret; | ||
400 | } | ||
401 | @@ -XXX,XX +XXX,XX @@ writev_help(void) | ||
402 | "\n" | ||
403 | " Writes into a segment of the currently open file, using a buffer\n" | ||
404 | " filled with a set pattern (0xcdcdcdcd).\n" | ||
405 | -" -P, -- use different pattern to fill file\n" | ||
406 | " -C, -- report statistics in a machine parsable format\n" | ||
407 | " -f, -- use Force Unit Access semantics\n" | ||
408 | +" -P, -- use different pattern to fill file\n" | ||
409 | " -q, -- quiet mode, do not show I/O statistics\n" | ||
410 | +" -r, -- register I/O buffer\n" | ||
411 | "\n"); | ||
412 | } | ||
413 | |||
414 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t writev_cmd = { | ||
415 | .perm = BLK_PERM_WRITE, | ||
416 | .argmin = 2, | ||
417 | .argmax = -1, | ||
418 | - .args = "[-Cfq] [-P pattern] off len [len..]", | ||
419 | + .args = "[-Cfqr] [-P pattern] off len [len..]", | ||
420 | .oneline = "writes a number of bytes at a specified offset", | ||
421 | .help = writev_help, | ||
422 | }; | ||
423 | @@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv) | ||
424 | int pattern = 0xcd; | ||
425 | QEMUIOVector qiov; | ||
426 | |||
427 | - while ((c = getopt(argc, argv, "CfqP:")) != -1) { | ||
428 | + while ((c = getopt(argc, argv, "CfP:qr")) != -1) { | ||
429 | switch (c) { | ||
430 | case 'C': | ||
431 | Cflag = true; | ||
432 | @@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv) | ||
433 | case 'q': | ||
434 | qflag = true; | ||
435 | break; | ||
436 | + case 'r': | ||
437 | + flags |= BDRV_REQ_REGISTERED_BUF; | ||
438 | + break; | ||
439 | case 'P': | ||
440 | pattern = parse_pattern(optarg); | ||
441 | if (pattern < 0) { | ||
442 | @@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv) | ||
443 | optind++; | ||
444 | |||
445 | nr_iov = argc - optind; | ||
446 | - buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern); | ||
447 | + buf = create_iovec(blk, &qiov, &argv[optind], nr_iov, pattern, | ||
448 | + flags & BDRV_REQ_REGISTERED_BUF); | ||
449 | if (buf == NULL) { | ||
450 | return -EINVAL; | ||
451 | } | ||
452 | @@ -XXX,XX +XXX,XX @@ static int writev_f(BlockBackend *blk, int argc, char **argv) | ||
453 | t2 = tsub(t2, t1); | ||
454 | print_report("wrote", &t2, offset, qiov.size, total, cnt, Cflag); | ||
455 | out: | ||
456 | + qemu_io_free(blk, buf, qiov.size, flags & BDRV_REQ_REGISTERED_BUF); | ||
457 | qemu_iovec_destroy(&qiov); | ||
458 | - qemu_io_free(buf); | ||
459 | return ret; | ||
460 | } | ||
461 | |||
462 | @@ -XXX,XX +XXX,XX @@ struct aio_ctx { | ||
463 | bool zflag; | ||
464 | BlockAcctCookie acct; | ||
465 | int pattern; | ||
466 | + BdrvRequestFlags flags; | ||
467 | struct timespec t1; | ||
468 | }; | ||
469 | |||
470 | @@ -XXX,XX +XXX,XX @@ static void aio_write_done(void *opaque, int ret) | ||
471 | ctx->qiov.size, 1, ctx->Cflag); | ||
472 | out: | ||
473 | if (!ctx->zflag) { | ||
474 | - qemu_io_free(ctx->buf); | ||
475 | + qemu_io_free(ctx->blk, ctx->buf, ctx->qiov.size, | ||
476 | + ctx->flags & BDRV_REQ_REGISTERED_BUF); | ||
477 | qemu_iovec_destroy(&ctx->qiov); | ||
478 | } | ||
479 | g_free(ctx); | ||
480 | @@ -XXX,XX +XXX,XX @@ static void aio_read_done(void *opaque, int ret) | ||
481 | print_report("read", &t2, ctx->offset, ctx->qiov.size, | ||
482 | ctx->qiov.size, 1, ctx->Cflag); | ||
483 | out: | ||
484 | - qemu_io_free(ctx->buf); | ||
485 | + qemu_io_free(ctx->blk, ctx->buf, ctx->qiov.size, | ||
486 | + ctx->flags & BDRV_REQ_REGISTERED_BUF); | ||
487 | qemu_iovec_destroy(&ctx->qiov); | ||
488 | g_free(ctx); | ||
489 | } | ||
490 | @@ -XXX,XX +XXX,XX @@ static void aio_read_help(void) | ||
491 | " considered successful once the request is submitted, independently\n" | ||
492 | " of potential I/O errors or pattern mismatches.\n" | ||
493 | " -C, -- report statistics in a machine parsable format\n" | ||
494 | -" -P, -- use a pattern to verify read data\n" | ||
495 | " -i, -- treat request as invalid, for exercising stats\n" | ||
496 | -" -v, -- dump buffer to standard output\n" | ||
497 | +" -P, -- use a pattern to verify read data\n" | ||
498 | " -q, -- quiet mode, do not show I/O statistics\n" | ||
499 | +" -r, -- register I/O buffer\n" | ||
500 | +" -v, -- dump buffer to standard output\n" | ||
501 | "\n"); | ||
502 | } | ||
503 | |||
504 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t aio_read_cmd = { | ||
505 | .cfunc = aio_read_f, | ||
506 | .argmin = 2, | ||
507 | .argmax = -1, | ||
508 | - .args = "[-Ciqv] [-P pattern] off len [len..]", | ||
509 | + .args = "[-Ciqrv] [-P pattern] off len [len..]", | ||
510 | .oneline = "asynchronously reads a number of bytes", | ||
511 | .help = aio_read_help, | ||
512 | }; | ||
513 | @@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) | ||
514 | struct aio_ctx *ctx = g_new0(struct aio_ctx, 1); | ||
515 | |||
516 | ctx->blk = blk; | ||
517 | - while ((c = getopt(argc, argv, "CP:iqv")) != -1) { | ||
518 | + while ((c = getopt(argc, argv, "CiP:qrv")) != -1) { | ||
519 | switch (c) { | ||
520 | case 'C': | ||
521 | ctx->Cflag = true; | ||
522 | @@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) | ||
523 | case 'q': | ||
524 | ctx->qflag = true; | ||
525 | break; | ||
526 | + case 'r': | ||
527 | + ctx->flags |= BDRV_REQ_REGISTERED_BUF; | ||
528 | + break; | ||
529 | case 'v': | ||
530 | ctx->vflag = true; | ||
531 | break; | ||
532 | @@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) | ||
533 | optind++; | ||
534 | |||
535 | nr_iov = argc - optind; | ||
536 | - ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, 0xab); | ||
537 | + ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, 0xab, | ||
538 | + ctx->flags & BDRV_REQ_REGISTERED_BUF); | ||
539 | if (ctx->buf == NULL) { | ||
540 | block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_READ); | ||
541 | g_free(ctx); | ||
542 | @@ -XXX,XX +XXX,XX @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv) | ||
543 | clock_gettime(CLOCK_MONOTONIC, &ctx->t1); | ||
544 | block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size, | ||
545 | BLOCK_ACCT_READ); | ||
546 | - blk_aio_preadv(blk, ctx->offset, &ctx->qiov, 0, aio_read_done, ctx); | ||
547 | + blk_aio_preadv(blk, ctx->offset, &ctx->qiov, ctx->flags, aio_read_done, | ||
548 | + ctx); | ||
549 | return 0; | ||
550 | } | ||
551 | |||
552 | @@ -XXX,XX +XXX,XX @@ static void aio_write_help(void) | ||
553 | " Note that due to its asynchronous nature, this command will be\n" | ||
554 | " considered successful once the request is submitted, independently\n" | ||
555 | " of potential I/O errors or pattern mismatches.\n" | ||
556 | -" -P, -- use different pattern to fill file\n" | ||
557 | " -C, -- report statistics in a machine parsable format\n" | ||
558 | " -f, -- use Force Unit Access semantics\n" | ||
559 | " -i, -- treat request as invalid, for exercising stats\n" | ||
560 | +" -P, -- use different pattern to fill file\n" | ||
561 | " -q, -- quiet mode, do not show I/O statistics\n" | ||
562 | +" -r, -- register I/O buffer\n" | ||
563 | " -u, -- with -z, allow unmapping\n" | ||
564 | " -z, -- write zeroes using blk_aio_pwrite_zeroes\n" | ||
565 | "\n"); | ||
566 | @@ -XXX,XX +XXX,XX @@ static const cmdinfo_t aio_write_cmd = { | ||
567 | .perm = BLK_PERM_WRITE, | ||
568 | .argmin = 2, | ||
569 | .argmax = -1, | ||
570 | - .args = "[-Cfiquz] [-P pattern] off len [len..]", | ||
571 | + .args = "[-Cfiqruz] [-P pattern] off len [len..]", | ||
572 | .oneline = "asynchronously writes a number of bytes", | ||
573 | .help = aio_write_help, | ||
574 | }; | ||
575 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) | ||
576 | int nr_iov, c; | ||
577 | int pattern = 0xcd; | ||
578 | struct aio_ctx *ctx = g_new0(struct aio_ctx, 1); | ||
579 | - BdrvRequestFlags flags = 0; | ||
580 | |||
581 | ctx->blk = blk; | ||
582 | - while ((c = getopt(argc, argv, "CfiqP:uz")) != -1) { | ||
583 | + while ((c = getopt(argc, argv, "CfiP:qruz")) != -1) { | ||
584 | switch (c) { | ||
585 | case 'C': | ||
586 | ctx->Cflag = true; | ||
587 | break; | ||
588 | case 'f': | ||
589 | - flags |= BDRV_REQ_FUA; | ||
590 | + ctx->flags |= BDRV_REQ_FUA; | ||
591 | break; | ||
592 | case 'q': | ||
593 | ctx->qflag = true; | ||
594 | break; | ||
595 | + case 'r': | ||
596 | + ctx->flags |= BDRV_REQ_REGISTERED_BUF; | ||
597 | + break; | ||
598 | case 'u': | ||
599 | - flags |= BDRV_REQ_MAY_UNMAP; | ||
600 | + ctx->flags |= BDRV_REQ_MAY_UNMAP; | ||
601 | break; | ||
602 | case 'P': | ||
603 | pattern = parse_pattern(optarg); | ||
604 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) | ||
605 | return -EINVAL; | ||
606 | } | ||
607 | |||
608 | - if ((flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) { | ||
609 | + if ((ctx->flags & BDRV_REQ_MAY_UNMAP) && !ctx->zflag) { | ||
610 | printf("-u requires -z to be specified\n"); | ||
611 | g_free(ctx); | ||
612 | return -EINVAL; | ||
613 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) | ||
614 | return -EINVAL; | ||
615 | } | ||
616 | |||
617 | + if (ctx->zflag && (ctx->flags & BDRV_REQ_REGISTERED_BUF)) { | ||
618 | + printf("cannot combine zero write with registered I/O buffer\n"); | ||
619 | + g_free(ctx); | ||
620 | + return -EINVAL; | ||
621 | + } | ||
622 | + | ||
623 | ctx->offset = cvtnum(argv[optind]); | ||
624 | if (ctx->offset < 0) { | ||
625 | int ret = ctx->offset; | ||
626 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) | ||
627 | } | ||
628 | |||
629 | ctx->qiov.size = count; | ||
630 | - blk_aio_pwrite_zeroes(blk, ctx->offset, count, flags, aio_write_done, | ||
631 | - ctx); | ||
632 | + blk_aio_pwrite_zeroes(blk, ctx->offset, count, ctx->flags, | ||
633 | + aio_write_done, ctx); | ||
634 | } else { | ||
635 | nr_iov = argc - optind; | ||
636 | ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov, | ||
637 | - pattern); | ||
638 | + pattern, ctx->flags & BDRV_REQ_REGISTERED_BUF); | ||
639 | if (ctx->buf == NULL) { | ||
640 | block_acct_invalid(blk_get_stats(blk), BLOCK_ACCT_WRITE); | ||
641 | g_free(ctx); | ||
642 | @@ -XXX,XX +XXX,XX @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv) | ||
643 | block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size, | ||
644 | BLOCK_ACCT_WRITE); | ||
645 | |||
646 | - blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, flags, aio_write_done, | ||
647 | - ctx); | ||
648 | + blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, ctx->flags, | ||
649 | + aio_write_done, ctx); | ||
650 | } | ||
651 | |||
652 | return 0; | ||
653 | -- | ||
654 | 2.39.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | This regression test demonstrates that detect-zeroes works with | ||
2 | registered buffers. Bug details: | ||
3 | https://gitlab.com/qemu-project/qemu/-/issues/1404 | ||
1 | 4 | ||
5 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
6 | Reviewed-by: Hanna Czenczek <hreitz@redhat.com> | ||
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
8 | Message-Id: <20230207203719.242926-5-stefanha@redhat.com> | ||
9 | --- | ||
10 | .../tests/detect-zeroes-registered-buf | 58 +++++++++++++++++++ | ||
11 | .../tests/detect-zeroes-registered-buf.out | 7 +++ | ||
12 | 2 files changed, 65 insertions(+) | ||
13 | create mode 100755 tests/qemu-iotests/tests/detect-zeroes-registered-buf | ||
14 | create mode 100644 tests/qemu-iotests/tests/detect-zeroes-registered-buf.out | ||
15 | |||
16 | diff --git a/tests/qemu-iotests/tests/detect-zeroes-registered-buf b/tests/qemu-iotests/tests/detect-zeroes-registered-buf | ||
17 | new file mode 100755 | ||
18 | index XXXXXXX..XXXXXXX | ||
19 | --- /dev/null | ||
20 | +++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf | ||
21 | @@ -XXX,XX +XXX,XX @@ | ||
22 | +#!/usr/bin/env bash | ||
23 | +# group: rw auto quick | ||
24 | +# | ||
25 | +# Check that detect-zeroes=unmap works on writes with registered I/O buffers. | ||
26 | +# This is a regression test for | ||
27 | +# https://gitlab.com/qemu-project/qemu/-/issues/1404 where I/O requests failed | ||
28 | +# unexpectedly. | ||
29 | +# | ||
30 | +# Copyright Red Hat | ||
31 | +# | ||
32 | +# This program is free software; you can redistribute it and/or modify | ||
33 | +# it under the terms of the GNU General Public License as published by | ||
34 | +# the Free Software Foundation; either version 2 of the License, or | ||
35 | +# (at your option) any later version. | ||
36 | +# | ||
37 | +# This program is distributed in the hope that it will be useful, | ||
38 | +# but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
39 | +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
40 | +# GNU General Public License for more details. | ||
41 | +# | ||
42 | +# You should have received a copy of the GNU General Public License | ||
43 | +# along with this program. If not, see <http://www.gnu.org/licenses/>. | ||
44 | +# | ||
45 | + | ||
46 | +# creator | ||
47 | +owner=stefanha@redhat.com | ||
48 | + | ||
49 | +seq=`basename $0` | ||
50 | +echo "QA output created by $seq" | ||
51 | + | ||
52 | +status=1 # failure is the default! | ||
53 | + | ||
54 | +_cleanup() | ||
55 | +{ | ||
56 | + _cleanup_test_img | ||
57 | +} | ||
58 | +trap "_cleanup; exit \$status" 0 1 2 3 15 | ||
59 | + | ||
60 | +# get standard environment, filters and checks | ||
61 | +cd .. | ||
62 | +. ./common.rc | ||
63 | +. ./common.filter | ||
64 | + | ||
65 | +_supported_fmt qcow2 | ||
66 | +_supported_proto generic | ||
67 | + | ||
68 | +size=128M | ||
69 | +_make_test_img $size | ||
70 | +IMGSPEC="driver=$IMGFMT,file.filename=$TEST_IMG,discard=unmap,detect-zeroes=unmap" | ||
71 | + | ||
72 | +echo | ||
73 | +echo "== writing zero buffer to image ==" | ||
74 | +QEMU_IO_OPTIONS="$QEMU_IO_OPTIONS_NO_FMT" $QEMU_IO -c "write -r -P 0 0 4k" --image-opts "$IMGSPEC" | _filter_qemu_io | ||
75 | + | ||
76 | +# success, all done | ||
77 | +echo "*** done" | ||
78 | +rm -f $seq.full | ||
79 | +status=0 | ||
80 | diff --git a/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out b/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out | ||
81 | new file mode 100644 | ||
82 | index XXXXXXX..XXXXXXX | ||
83 | --- /dev/null | ||
84 | +++ b/tests/qemu-iotests/tests/detect-zeroes-registered-buf.out | ||
85 | @@ -XXX,XX +XXX,XX @@ | ||
86 | +QA output created by detect-zeroes-registered-buf | ||
87 | +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 | ||
88 | + | ||
89 | +== writing zero buffer to image == | ||
90 | +wrote 4096/4096 bytes at offset 0 | ||
91 | +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) | ||
92 | +*** done | ||
93 | -- | ||
94 | 2.39.1 | diff view generated by jsdifflib |