1 | The following changes since commit 3f3bbfc7cef4490c5ed5550766a81e7d18f08db1: | 1 | The following changes since commit 661c2e1ab29cd9c4d268ae3f44712e8d421c0e56: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2019-03-12' into staging (2019-03-12 21:06:26 +0000) | 3 | scripts/checkpatch: Fix a typo (2025-03-04 09:30:26 +0800) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://github.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 f357fcd890a8d6ced6d261338b859a41414561e9: | 9 | for you to fetch changes up to 2ad638a3d160923ef3dbf87c73944e6e44bdc724: |
10 | 10 | ||
11 | file-posix: add drop-cache=on|off option (2019-03-13 10:54:55 +0000) | 11 | block/qed: fix use-after-free by nullifying timer pointer after free (2025-03-06 10:19:54 +0800) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | * Add 'drop-cache=on|off' option to file-posix.c. The default is on. | 16 | QED need_check_timer use-after-free fix |
17 | Disabling the option fixes a QEMU 3.0.0 performance regression when live | ||
18 | migrating on the same host with cache.direct=off. | ||
19 | 17 | ||
20 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
21 | 19 | ||
22 | Stefan Hajnoczi (1): | 20 | Denis Rastyogin (1): |
23 | file-posix: add drop-cache=on|off option | 21 | block/qed: fix use-after-free by nullifying timer pointer after free |
24 | 22 | ||
25 | qapi/block-core.json | 6 ++++++ | 23 | block/qed.c | 1 + |
26 | block/file-posix.c | 16 ++++++++++++++++ | 24 | 1 file changed, 1 insertion(+) |
27 | 2 files changed, 22 insertions(+) | ||
28 | 25 | ||
29 | -- | 26 | -- |
30 | 2.20.1 | 27 | 2.48.1 |
31 | |||
32 | diff view generated by jsdifflib |
1 | Commit dd577a26ff03b6829721b1ffbbf9e7c411b72378 ("block/file-posix: | 1 | From: Denis Rastyogin <gerben@altlinux.org> |
---|---|---|---|
2 | implement bdrv_co_invalidate_cache() on Linux") introduced page cache | ||
3 | invalidation so that cache.direct=off live migration is safe on Linux. | ||
4 | 2 | ||
5 | The invalidation takes a significant amount of time when the file is | 3 | This error was discovered by fuzzing qemu-img. |
6 | large and present in the page cache. Normally this is not the case for | ||
7 | cross-host live migration but it can happen when migrating between QEMU | ||
8 | processes on the same host. | ||
9 | 4 | ||
10 | On same-host migration we don't need to invalidate pages for correctness | 5 | In the QED block driver, the need_check_timer timer is freed in |
11 | anyway, so an option to skip page cache invalidation is useful. I | 6 | bdrv_qed_detach_aio_context, but the pointer to the timer is not |
12 | investigated optimizing invalidation and detecting same-host migration, | 7 | set to NULL. This can lead to a use-after-free scenario |
13 | but both are hard to achieve so a user-visible option will suffice. | 8 | in bdrv_qed_drain_begin(). |
14 | 9 | ||
15 | As a bonus this option means that the cache invalidation feature will | 10 | The need_check_timer pointer is set to NULL after freeing the timer. |
16 | now be detectable by libvirt via QMP schema introspection. | 11 | Which helps catch this condition when checking in bdrv_qed_drain_begin(). |
17 | 12 | ||
18 | Suggested-by: Neil Skrypuch <neil@tembosocial.com> | 13 | Closes: https://gitlab.com/qemu-project/qemu/-/issues/2852 |
19 | Tested-by: Neil Skrypuch <neil@tembosocial.com> | 14 | Signed-off-by: Denis Rastyogin <gerben@altlinux.org> |
20 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | 15 | Message-ID: <20250304083927.37681-1-gerben@altlinux.org> |
21 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
22 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
23 | Message-id: 20190307164941.3322-1-stefanha@redhat.com | ||
24 | Message-Id: <20190307164941.3322-1-stefanha@redhat.com> | ||
25 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
26 | --- | 17 | --- |
27 | qapi/block-core.json | 6 ++++++ | 18 | block/qed.c | 1 + |
28 | block/file-posix.c | 16 ++++++++++++++++ | 19 | 1 file changed, 1 insertion(+) |
29 | 2 files changed, 22 insertions(+) | ||
30 | 20 | ||
31 | diff --git a/qapi/block-core.json b/qapi/block-core.json | 21 | diff --git a/block/qed.c b/block/qed.c |
32 | index XXXXXXX..XXXXXXX 100644 | 22 | index XXXXXXX..XXXXXXX 100644 |
33 | --- a/qapi/block-core.json | 23 | --- a/block/qed.c |
34 | +++ b/qapi/block-core.json | 24 | +++ b/block/qed.c |
35 | @@ -XXX,XX +XXX,XX @@ | 25 | @@ -XXX,XX +XXX,XX @@ static void bdrv_qed_detach_aio_context(BlockDriverState *bs) |
36 | # @locking: whether to enable file locking. If set to 'auto', only enable | 26 | |
37 | # when Open File Descriptor (OFD) locking API is available | 27 | qed_cancel_need_check_timer(s); |
38 | # (default: auto, since 2.10) | 28 | timer_free(s->need_check_timer); |
39 | +# @drop-cache: invalidate page cache during live migration. This prevents | 29 | + s->need_check_timer = NULL; |
40 | +# stale data on the migration destination with cache.direct=off. | 30 | } |
41 | +# Currently only supported on Linux hosts. | 31 | |
42 | +# (default: on, since: 4.0) | 32 | static void bdrv_qed_attach_aio_context(BlockDriverState *bs, |
43 | # @x-check-cache-dropped: whether to check that page cache was dropped on live | ||
44 | # migration. May cause noticeable delays if the image | ||
45 | # file is large, do not use in production. | ||
46 | @@ -XXX,XX +XXX,XX @@ | ||
47 | '*pr-manager': 'str', | ||
48 | '*locking': 'OnOffAuto', | ||
49 | '*aio': 'BlockdevAioOptions', | ||
50 | + '*drop-cache': {'type': 'bool', | ||
51 | + 'if': 'defined(CONFIG_LINUX)'}, | ||
52 | '*x-check-cache-dropped': 'bool' } } | ||
53 | |||
54 | ## | ||
55 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
56 | index XXXXXXX..XXXXXXX 100644 | ||
57 | --- a/block/file-posix.c | ||
58 | +++ b/block/file-posix.c | ||
59 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState { | ||
60 | bool page_cache_inconsistent:1; | ||
61 | bool has_fallocate; | ||
62 | bool needs_alignment; | ||
63 | + bool drop_cache; | ||
64 | bool check_cache_dropped; | ||
65 | |||
66 | PRManager *pr_mgr; | ||
67 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVRawState { | ||
68 | typedef struct BDRVRawReopenState { | ||
69 | int fd; | ||
70 | int open_flags; | ||
71 | + bool drop_cache; | ||
72 | bool check_cache_dropped; | ||
73 | } BDRVRawReopenState; | ||
74 | |||
75 | @@ -XXX,XX +XXX,XX @@ static QemuOptsList raw_runtime_opts = { | ||
76 | .type = QEMU_OPT_STRING, | ||
77 | .help = "id of persistent reservation manager object (default: none)", | ||
78 | }, | ||
79 | +#if defined(__linux__) | ||
80 | + { | ||
81 | + .name = "drop-cache", | ||
82 | + .type = QEMU_OPT_BOOL, | ||
83 | + .help = "invalidate page cache during live migration (default: on)", | ||
84 | + }, | ||
85 | +#endif | ||
86 | { | ||
87 | .name = "x-check-cache-dropped", | ||
88 | .type = QEMU_OPT_BOOL, | ||
89 | @@ -XXX,XX +XXX,XX @@ static int raw_open_common(BlockDriverState *bs, QDict *options, | ||
90 | } | ||
91 | } | ||
92 | |||
93 | + s->drop_cache = qemu_opt_get_bool(opts, "drop-cache", true); | ||
94 | s->check_cache_dropped = qemu_opt_get_bool(opts, "x-check-cache-dropped", | ||
95 | false); | ||
96 | |||
97 | @@ -XXX,XX +XXX,XX @@ static int raw_reopen_prepare(BDRVReopenState *state, | ||
98 | goto out; | ||
99 | } | ||
100 | |||
101 | + rs->drop_cache = qemu_opt_get_bool_del(opts, "drop-cache", true); | ||
102 | rs->check_cache_dropped = | ||
103 | qemu_opt_get_bool_del(opts, "x-check-cache-dropped", false); | ||
104 | |||
105 | @@ -XXX,XX +XXX,XX @@ static void raw_reopen_commit(BDRVReopenState *state) | ||
106 | BDRVRawState *s = state->bs->opaque; | ||
107 | Error *local_err = NULL; | ||
108 | |||
109 | + s->drop_cache = rs->drop_cache; | ||
110 | s->check_cache_dropped = rs->check_cache_dropped; | ||
111 | s->open_flags = rs->open_flags; | ||
112 | |||
113 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_invalidate_cache(BlockDriverState *bs, | ||
114 | return; | ||
115 | } | ||
116 | |||
117 | + if (!s->drop_cache) { | ||
118 | + return; | ||
119 | + } | ||
120 | + | ||
121 | if (s->open_flags & O_DIRECT) { | ||
122 | return; /* No host kernel page cache */ | ||
123 | } | ||
124 | -- | 33 | -- |
125 | 2.20.1 | 34 | 2.48.1 |
126 | |||
127 | diff view generated by jsdifflib |