1 | The following changes since commit fa54abb8c298f892639ffc4bc2f61448ac3be4a1: | 1 | The following changes since commit 0db1851becbefe3e50cfc03776fb1f75817376af: |
---|---|---|---|
2 | 2 | ||
3 | Drop QEMU_GNUC_PREREQ() checks for gcc older than 4.1 (2017-04-20 18:33:33 +0100) | 3 | Merge remote-tracking branch 'remotes/vivier/tags/m68k-for-2.10-pull-request' into staging (2017-06-07 11:56:00 +0100) |
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 | git://github.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to 3ccc0a0163b932fe980dce8d26db4bf98b1900e9: | 9 | for you to fetch changes up to 11cde1c81093a33c46c7a4039bf750bb61551087: |
10 | 10 | ||
11 | MAINTAINERS: update my email address (2017-04-21 10:36:12 +0100) | 11 | configure: split c and cxx extra flags (2017-06-07 15:29:46 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | 14 | ||
15 | ---------------------------------------------------------------- | 15 | ---------------------------------------------------------------- |
16 | 16 | ||
17 | Changlong Xie (1): | 17 | Bruno Dominguez (1): |
18 | MAINTAINERS: update Wen's email address | 18 | configure: split c and cxx extra flags |
19 | 19 | ||
20 | Lidong Chen (1): | 20 | Philippe Mathieu-Daudé (2): |
21 | migration/block: use blk_pwrite_zeroes for each zero cluster | 21 | oslib: strip trailing '\n' from error_setg() string argument |
22 | coccinelle: fix typo in comment | ||
22 | 23 | ||
23 | Stefan Hajnoczi (3): | 24 | Roman Pen (1): |
24 | qemu-options: explain disk I/O throttling options | 25 | coroutine-lock: do not touch coroutine after another one has been |
25 | throttle: do not use invalid config in test | 26 | entered |
26 | throttle: make throttle_config(throttle_get_config()) symmetric | ||
27 | 27 | ||
28 | Zhang Chen (1): | 28 | Stefan Hajnoczi (1): |
29 | MAINTAINERS: update my email address | 29 | .gdbinit: load QEMU sub-commands when gdb starts |
30 | 30 | ||
31 | MAINTAINERS | 4 ++-- | 31 | configure | 75 ++++++++++++++++++-------------- |
32 | migration/block.c | 35 +++++++++++++++++++++++++++++++++-- | 32 | disas/libvixl/Makefile.objs | 4 +- |
33 | tests/test-throttle.c | 8 ++++---- | 33 | util/oslib-posix.c | 2 +- |
34 | util/throttle.c | 14 ++++++++++++++ | 34 | util/qemu-coroutine-lock.c | 19 +++++++- |
35 | qemu-options.hx | 24 ++++++++++++++++++++++++ | 35 | util/qemu-coroutine.c | 5 +++ |
36 | 5 files changed, 77 insertions(+), 8 deletions(-) | 36 | .gdbinit | 8 ++++ |
37 | rules.mak | 3 -- | ||
38 | scripts/coccinelle/return_directly.cocci | 2 +- | ||
39 | 8 files changed, 77 insertions(+), 41 deletions(-) | ||
40 | create mode 100644 .gdbinit | ||
37 | 41 | ||
38 | -- | 42 | -- |
39 | 2.9.3 | 43 | 2.9.4 |
40 | 44 | ||
41 | 45 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | The disk I/O throttling options have been listed for a long time but | ||
2 | never explained on the QEMU man page. | ||
3 | 1 | ||
4 | Suggested-by: Nini Gu <ngu@redhat.com> | ||
5 | Cc: Alberto Garcia <berto@igalia.com> | ||
6 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
7 | Reviewed-by: Alberto Garcia <berto@igalia.com> | ||
8 | Reviewed-by: Greg Kurz <groug@kaod.org> | ||
9 | Message-id: 20170301115026.22621-2-stefanha@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | ||
12 | qemu-options.hx | 24 ++++++++++++++++++++++++ | ||
13 | 1 file changed, 24 insertions(+) | ||
14 | |||
15 | diff --git a/qemu-options.hx b/qemu-options.hx | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/qemu-options.hx | ||
18 | +++ b/qemu-options.hx | ||
19 | @@ -XXX,XX +XXX,XX @@ file sectors into the image file. | ||
20 | conversion of plain zero writes by the OS to driver specific optimized | ||
21 | zero write commands. You may even choose "unmap" if @var{discard} is set | ||
22 | to "unmap" to allow a zero write to be converted to an UNMAP operation. | ||
23 | +@item bps=@var{b},bps_rd=@var{r},bps_wr=@var{w} | ||
24 | +Specify bandwidth throttling limits in bytes per second, either for all request | ||
25 | +types or for reads or writes only. Small values can lead to timeouts or hangs | ||
26 | +inside the guest. A safe minimum for disks is 2 MB/s. | ||
27 | +@item bps_max=@var{bm},bps_rd_max=@var{rm},bps_wr_max=@var{wm} | ||
28 | +Specify bursts in bytes per second, either for all request types or for reads | ||
29 | +or writes only. Bursts allow the guest I/O to spike above the limit | ||
30 | +temporarily. | ||
31 | +@item iops=@var{i},iops_rd=@var{r},iops_wr=@var{w} | ||
32 | +Specify request rate limits in requests per second, either for all request | ||
33 | +types or for reads or writes only. | ||
34 | +@item iops_max=@var{bm},iops_rd_max=@var{rm},iops_wr_max=@var{wm} | ||
35 | +Specify bursts in requests per second, either for all request types or for reads | ||
36 | +or writes only. Bursts allow the guest I/O to spike above the limit | ||
37 | +temporarily. | ||
38 | +@item iops_size=@var{is} | ||
39 | +Let every @var{is} bytes of a request count as a new request for iops | ||
40 | +throttling purposes. Use this option to prevent guests from circumventing iops | ||
41 | +limits by sending fewer but larger requests. | ||
42 | +@item group=@var{g} | ||
43 | +Join a throttling quota group with given name @var{g}. All drives that are | ||
44 | +members of the same group are accounted for together. Use this option to | ||
45 | +prevent guests from circumventing throttling limits by using many small disks | ||
46 | +instead of a single larger disk. | ||
47 | @end table | ||
48 | |||
49 | By default, the @option{cache=writeback} mode is used. It will report data | ||
50 | -- | ||
51 | 2.9.3 | ||
52 | |||
53 | diff view generated by jsdifflib |
1 | From: Zhang Chen <zhangchen.fnst@cn.fujitsu.com> | 1 | From: Philippe Mathieu-Daudé <f4bug@amsat.org> |
---|---|---|---|
2 | 2 | ||
3 | I'm leaving my job at Fujitsu, this email address will stop working | 3 | spotted by Coccinelle script scripts/coccinelle/err-bad-newline.cocci |
4 | this week. Update it to one that I will have access to later. | ||
5 | 4 | ||
6 | Signed-off-by: Xie Changlong <xiecl.fnst@cn.fujitsu.com> | 5 | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> |
7 | Message-id: 1492758767-19716-1-git-send-email-xiecl.fnst@cn.fujitsu.com | 6 | Reviewed-by: Eric Blake <eblake@redhat.com> |
7 | Reviewed-by: Laurent Vivier <lvivier@redhat.com> | ||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
9 | --- | 9 | --- |
10 | MAINTAINERS | 2 +- | 10 | util/oslib-posix.c | 2 +- |
11 | 1 file changed, 1 insertion(+), 1 deletion(-) | 11 | 1 file changed, 1 insertion(+), 1 deletion(-) |
12 | 12 | ||
13 | diff --git a/MAINTAINERS b/MAINTAINERS | 13 | diff --git a/util/oslib-posix.c b/util/oslib-posix.c |
14 | index XXXXXXX..XXXXXXX 100644 | 14 | index XXXXXXX..XXXXXXX 100644 |
15 | --- a/MAINTAINERS | 15 | --- a/util/oslib-posix.c |
16 | +++ b/MAINTAINERS | 16 | +++ b/util/oslib-posix.c |
17 | @@ -XXX,XX +XXX,XX @@ F: tests/image-fuzzer/ | 17 | @@ -XXX,XX +XXX,XX @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus, |
18 | 18 | /* touch pages simultaneously */ | |
19 | Replication | 19 | if (touch_all_pages(area, hpagesize, numpages, smp_cpus)) { |
20 | M: Wen Congyang <wencongyang2@huawei.com> | 20 | error_setg(errp, "os_mem_prealloc: Insufficient free host memory " |
21 | -M: Changlong Xie <xiecl.fnst@cn.fujitsu.com> | 21 | - "pages available to allocate guest RAM\n"); |
22 | +M: Xie Changlong <xiechanglong.d@gmail.com> | 22 | + "pages available to allocate guest RAM"); |
23 | S: Supported | 23 | } |
24 | F: replication* | 24 | |
25 | F: block/replication.c | 25 | ret = sigaction(SIGBUS, &oldact, NULL); |
26 | -- | 26 | -- |
27 | 2.9.3 | 27 | 2.9.4 |
28 | 28 | ||
29 | 29 | diff view generated by jsdifflib |
1 | From: Changlong Xie <xiecl.fnst@cn.fujitsu.com> | 1 | From: Philippe Mathieu-Daudé <f4bug@amsat.org> |
---|---|---|---|
2 | 2 | ||
3 | So he can get CC'ed on future patches and bugs for this feature | 3 | Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> |
4 | 4 | Reviewed-by: Eric Blake <eblake@redhat.com> | |
5 | Signed-off-by: Changlong Xie <xiecl.fnst@cn.fujitsu.com> | ||
6 | Message-id: 1492484893-23435-1-git-send-email-xiecl.fnst@cn.fujitsu.com | ||
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
8 | --- | 6 | --- |
9 | MAINTAINERS | 2 +- | 7 | scripts/coccinelle/return_directly.cocci | 2 +- |
10 | 1 file changed, 1 insertion(+), 1 deletion(-) | 8 | 1 file changed, 1 insertion(+), 1 deletion(-) |
11 | 9 | ||
12 | diff --git a/MAINTAINERS b/MAINTAINERS | 10 | diff --git a/scripts/coccinelle/return_directly.cocci b/scripts/coccinelle/return_directly.cocci |
13 | index XXXXXXX..XXXXXXX 100644 | 11 | index XXXXXXX..XXXXXXX 100644 |
14 | --- a/MAINTAINERS | 12 | --- a/scripts/coccinelle/return_directly.cocci |
15 | +++ b/MAINTAINERS | 13 | +++ b/scripts/coccinelle/return_directly.cocci |
16 | @@ -XXX,XX +XXX,XX @@ S: Supported | 14 | @@ -XXX,XX +XXX,XX @@ |
17 | F: tests/image-fuzzer/ | 15 | -// replace 'R = X; return R;' with 'return R;' |
18 | 16 | +// replace 'R = X; return R;' with 'return X;' | |
19 | Replication | 17 | @@ |
20 | -M: Wen Congyang <wency@cn.fujitsu.com> | 18 | identifier VAR; |
21 | +M: Wen Congyang <wencongyang2@huawei.com> | 19 | expression E; |
22 | M: Changlong Xie <xiecl.fnst@cn.fujitsu.com> | ||
23 | S: Supported | ||
24 | F: replication* | ||
25 | -- | 20 | -- |
26 | 2.9.3 | 21 | 2.9.4 |
27 | 22 | ||
28 | 23 | diff view generated by jsdifflib |
1 | The (burst) max parameter cannot be smaller than the avg parameter. | 1 | The scripts/qemu-gdb.py file is not easily discoverable. Add a .gdbinit |
---|---|---|---|
2 | There is a test case that uses avg = 56, max = 1 and gets away with it | 2 | file so GDB either loads qemu-gdb.py automatically or prints a message |
3 | because no input validation is performed by the test case. | 3 | informing the user how to enable them (some systems disable ./.gdbinit |
4 | loading for security reasons). | ||
4 | 5 | ||
5 | This patch switches to valid test input parameters. | 6 | Symlink .gdbinit and the scripts directory in order to make out-of-tree |
7 | builds work. The scripts directory is used to find the qemu-gdb.py file | ||
8 | specified by a relative path in .gdbinit. | ||
6 | 9 | ||
10 | Suggested-by: Eric Blake <eblake@redhat.com> | ||
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
8 | Reviewed-by: Alberto Garcia <berto@igalia.com> | 12 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> |
9 | Message-id: 20170301115026.22621-3-stefanha@redhat.com | 13 | Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> |
14 | Tested-by: Eric Blake <eblake@redhat.com> | ||
15 | Message-id: 20170517124042.1430-1-stefanha@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
11 | --- | 17 | --- |
12 | tests/test-throttle.c | 8 ++++---- | 18 | configure | 1 + |
13 | 1 file changed, 4 insertions(+), 4 deletions(-) | 19 | .gdbinit | 8 ++++++++ |
20 | 2 files changed, 9 insertions(+) | ||
21 | create mode 100644 .gdbinit | ||
14 | 22 | ||
15 | diff --git a/tests/test-throttle.c b/tests/test-throttle.c | 23 | diff --git a/configure b/configure |
16 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100755 |
17 | --- a/tests/test-throttle.c | 25 | --- a/configure |
18 | +++ b/tests/test-throttle.c | 26 | +++ b/configure |
19 | @@ -XXX,XX +XXX,XX @@ static void test_config_functions(void) | 27 | @@ -XXX,XX +XXX,XX @@ FILES="$FILES pc-bios/spapr-rtas/Makefile" |
20 | orig_cfg.buckets[THROTTLE_OPS_READ].avg = 69; | 28 | FILES="$FILES pc-bios/s390-ccw/Makefile" |
21 | orig_cfg.buckets[THROTTLE_OPS_WRITE].avg = 23; | 29 | FILES="$FILES roms/seabios/Makefile roms/vgabios/Makefile" |
22 | 30 | FILES="$FILES pc-bios/qemu-icon.bmp" | |
23 | - orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */ | 31 | +FILES="$FILES .gdbinit scripts" # scripts needed by relative path in .gdbinit |
24 | - orig_cfg.buckets[THROTTLE_BPS_READ].max = 1; /* should not be corrected */ | 32 | for bios_file in \ |
25 | + orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */ | 33 | $source_path/pc-bios/*.bin \ |
26 | + orig_cfg.buckets[THROTTLE_BPS_READ].max = 56; /* should not be corrected */ | 34 | $source_path/pc-bios/*.lid \ |
27 | orig_cfg.buckets[THROTTLE_BPS_WRITE].max = 120; | 35 | diff --git a/.gdbinit b/.gdbinit |
28 | 36 | new file mode 100644 | |
29 | orig_cfg.buckets[THROTTLE_OPS_TOTAL].max = 150; | 37 | index XXXXXXX..XXXXXXX |
30 | @@ -XXX,XX +XXX,XX @@ static void test_config_functions(void) | 38 | --- /dev/null |
31 | g_assert(final_cfg.buckets[THROTTLE_OPS_READ].avg == 69); | 39 | +++ b/.gdbinit |
32 | g_assert(final_cfg.buckets[THROTTLE_OPS_WRITE].avg == 23); | 40 | @@ -XXX,XX +XXX,XX @@ |
33 | 41 | +# GDB may have ./.gdbinit loading disabled by default. In that case you can | |
34 | - g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3);/* fixed */ | 42 | +# follow the instructions it prints. They boil down to adding the following to |
35 | - g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 1); /* not fixed */ | 43 | +# your home directory's ~/.gdbinit file: |
36 | + g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3); /* fixed */ | 44 | +# |
37 | + g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56); /* not fixed */ | 45 | +# add-auto-load-safe-path /path/to/qemu/.gdbinit |
38 | g_assert(final_cfg.buckets[THROTTLE_BPS_WRITE].max == 120); | 46 | + |
39 | 47 | +# Load QEMU-specific sub-commands and settings | |
40 | g_assert(final_cfg.buckets[THROTTLE_OPS_TOTAL].max == 150); | 48 | +source scripts/qemu-gdb.py |
41 | -- | 49 | -- |
42 | 2.9.3 | 50 | 2.9.4 |
43 | 51 | ||
44 | 52 | diff view generated by jsdifflib |
1 | Throttling has a weird property that throttle_get_config() does not | 1 | From: Roman Pen <roman.penyaev@profitbricks.com> |
---|---|---|---|
2 | always return the same throttling settings that were given with | ||
3 | throttle_config(). In other words, the set and get functions aren't | ||
4 | symmetric. | ||
5 | 2 | ||
6 | If .max is 0 then the throttling code assigns a default value of .avg / | 3 | Submission of requests on linux aio is a bit tricky and can lead to |
7 | 10 in throttle_config(). This is an implementation detail of the | 4 | requests completions on submission path: |
8 | throttling algorithm. When throttle_get_config() is called the .max | ||
9 | value returned should still be 0. | ||
10 | 5 | ||
11 | Users are exposed to this quirk via "info block" or "query-block" | 6 | 44713c9e8547 ("linux-aio: Handle io_submit() failure gracefully") |
12 | monitor commands. This has caused confusion because it looks like a bug | 7 | 0ed93d84edab ("linux-aio: process completions from ioq_submit()") |
13 | when an unexpected value is reported. | ||
14 | 8 | ||
15 | This patch hides the .max value adjustment in throttle_get_config() and | 9 | That means that any coroutine which has been yielded in order to wait |
16 | updates test-throttle.c appropriately. | 10 | for completion can be resumed from submission path and be eventually |
11 | terminated (freed). | ||
17 | 12 | ||
18 | Reported-by: Nini Gu <ngu@redhat.com> | 13 | The following use-after-free crash was observed when IO throttling |
19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | was enabled: |
20 | Reviewed-by: Alberto Garcia <berto@igalia.com> | 15 | |
21 | Message-id: 20170301115026.22621-4-stefanha@redhat.com | 16 | Program received signal SIGSEGV, Segmentation fault. |
17 | [Switching to Thread 0x7f5813dff700 (LWP 56417)] | ||
18 | virtqueue_unmap_sg (elem=0x7f5804009a30, len=1, vq=<optimized out>) at virtio.c:252 | ||
19 | (gdb) bt | ||
20 | #0 virtqueue_unmap_sg (elem=0x7f5804009a30, len=1, vq=<optimized out>) at virtio.c:252 | ||
21 | ^^^^^^^^^^^^^^ | ||
22 | remember the address | ||
23 | |||
24 | #1 virtqueue_fill (vq=0x5598b20d21b0, elem=0x7f5804009a30, len=1, idx=0) at virtio.c:282 | ||
25 | #2 virtqueue_push (vq=0x5598b20d21b0, elem=elem@entry=0x7f5804009a30, len=<optimized out>) at virtio.c:308 | ||
26 | #3 virtio_blk_req_complete (req=req@entry=0x7f5804009a30, status=status@entry=0 '\000') at virtio-blk.c:61 | ||
27 | #4 virtio_blk_rw_complete (opaque=<optimized out>, ret=0) at virtio-blk.c:126 | ||
28 | #5 blk_aio_complete (acb=0x7f58040068d0) at block-backend.c:923 | ||
29 | #6 coroutine_trampoline (i0=<optimized out>, i1=<optimized out>) at coroutine-ucontext.c:78 | ||
30 | |||
31 | (gdb) p * elem | ||
32 | $8 = {index = 77, out_num = 2, in_num = 1, | ||
33 | in_addr = 0x7f5804009ad8, out_addr = 0x7f5804009ae0, | ||
34 | in_sg = 0x0, out_sg = 0x7f5804009a50} | ||
35 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
36 | 'in_sg' and 'out_sg' are invalid. | ||
37 | e.g. it is impossible that 'in_sg' is zero, | ||
38 | instead its value must be equal to: | ||
39 | |||
40 | (gdb) p/x 0x7f5804009ad8 + sizeof(elem->in_addr[0]) + 2 * sizeof(elem->out_addr[0]) | ||
41 | $26 = 0x7f5804009af0 | ||
42 | |||
43 | Seems 'elem' was corrupted. Meanwhile another thread raised an abort: | ||
44 | |||
45 | Thread 12 (Thread 0x7f57f2ffd700 (LWP 56426)): | ||
46 | #0 raise () from /lib/x86_64-linux-gnu/libc.so.6 | ||
47 | #1 abort () from /lib/x86_64-linux-gnu/libc.so.6 | ||
48 | #2 qemu_coroutine_enter (co=0x7f5804009af0) at qemu-coroutine.c:113 | ||
49 | #3 qemu_co_queue_run_restart (co=0x7f5804009a30) at qemu-coroutine-lock.c:60 | ||
50 | #4 qemu_coroutine_enter (co=0x7f5804009a30) at qemu-coroutine.c:119 | ||
51 | ^^^^^^^^^^^^^^^^^^ | ||
52 | WTF?? this is equal to elem from crashed thread | ||
53 | |||
54 | #5 qemu_co_queue_run_restart (co=0x7f57e7f16ae0) at qemu-coroutine-lock.c:60 | ||
55 | #6 qemu_coroutine_enter (co=0x7f57e7f16ae0) at qemu-coroutine.c:119 | ||
56 | #7 qemu_co_queue_run_restart (co=0x7f5807e112a0) at qemu-coroutine-lock.c:60 | ||
57 | #8 qemu_coroutine_enter (co=0x7f5807e112a0) at qemu-coroutine.c:119 | ||
58 | #9 qemu_co_queue_run_restart (co=0x7f5807f17820) at qemu-coroutine-lock.c:60 | ||
59 | #10 qemu_coroutine_enter (co=0x7f5807f17820) at qemu-coroutine.c:119 | ||
60 | #11 qemu_co_queue_run_restart (co=0x7f57e7f18e10) at qemu-coroutine-lock.c:60 | ||
61 | #12 qemu_coroutine_enter (co=0x7f57e7f18e10) at qemu-coroutine.c:119 | ||
62 | #13 qemu_co_enter_next (queue=queue@entry=0x5598b1e742d0) at qemu-coroutine-lock.c:106 | ||
63 | #14 timer_cb (blk=0x5598b1e74280, is_write=<optimized out>) at throttle-groups.c:419 | ||
64 | |||
65 | Crash can be explained by access of 'co' object from the loop inside | ||
66 | qemu_co_queue_run_restart(): | ||
67 | |||
68 | while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) { | ||
69 | QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next); | ||
70 | ^^^^^^^^^^^^^^^^^^^^ | ||
71 | on each iteration 'co' is accessed, | ||
72 | but 'co' can be already freed | ||
73 | |||
74 | qemu_coroutine_enter(next); | ||
75 | } | ||
76 | |||
77 | When 'next' coroutine is resumed (entered) it can in its turn resume | ||
78 | 'co', and eventually free it. That's why we see 'co' (which was freed) | ||
79 | has the same address as 'elem' from the first backtrace. | ||
80 | |||
81 | The fix is obvious: use temporary queue and do not touch coroutine after | ||
82 | first qemu_coroutine_enter() is invoked. | ||
83 | |||
84 | The issue is quite rare and happens every ~12 hours on very high IO | ||
85 | and CPU load (building linux kernel with -j512 inside guest) when IO | ||
86 | throttling is enabled. With the fix applied guest is running ~35 hours | ||
87 | and is still alive so far. | ||
88 | |||
89 | Signed-off-by: Roman Pen <roman.penyaev@profitbricks.com> | ||
90 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
91 | Message-id: 20170601160847.23720-1-roman.penyaev@profitbricks.com | ||
92 | Cc: Paolo Bonzini <pbonzini@redhat.com> | ||
93 | Cc: Fam Zheng <famz@redhat.com> | ||
94 | Cc: Stefan Hajnoczi <stefanha@redhat.com> | ||
95 | Cc: Kevin Wolf <kwolf@redhat.com> | ||
96 | Cc: qemu-devel@nongnu.org | ||
22 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 97 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
23 | --- | 98 | --- |
24 | tests/test-throttle.c | 8 ++++---- | 99 | util/qemu-coroutine-lock.c | 19 +++++++++++++++++-- |
25 | util/throttle.c | 14 ++++++++++++++ | 100 | util/qemu-coroutine.c | 5 +++++ |
26 | 2 files changed, 18 insertions(+), 4 deletions(-) | 101 | 2 files changed, 22 insertions(+), 2 deletions(-) |
27 | 102 | ||
28 | diff --git a/tests/test-throttle.c b/tests/test-throttle.c | 103 | diff --git a/util/qemu-coroutine-lock.c b/util/qemu-coroutine-lock.c |
29 | index XXXXXXX..XXXXXXX 100644 | 104 | index XXXXXXX..XXXXXXX 100644 |
30 | --- a/tests/test-throttle.c | 105 | --- a/util/qemu-coroutine-lock.c |
31 | +++ b/tests/test-throttle.c | 106 | +++ b/util/qemu-coroutine-lock.c |
32 | @@ -XXX,XX +XXX,XX @@ static void test_config_functions(void) | 107 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_queue_wait(CoQueue *queue, CoMutex *mutex) |
33 | orig_cfg.buckets[THROTTLE_OPS_READ].avg = 69; | 108 | void qemu_co_queue_run_restart(Coroutine *co) |
34 | orig_cfg.buckets[THROTTLE_OPS_WRITE].avg = 23; | 109 | { |
35 | 110 | Coroutine *next; | |
36 | - orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; /* should be corrected */ | 111 | + QSIMPLEQ_HEAD(, Coroutine) tmp_queue_wakeup = |
37 | - orig_cfg.buckets[THROTTLE_BPS_READ].max = 56; /* should not be corrected */ | 112 | + QSIMPLEQ_HEAD_INITIALIZER(tmp_queue_wakeup); |
38 | + orig_cfg.buckets[THROTTLE_BPS_TOTAL].max = 0; | 113 | |
39 | + orig_cfg.buckets[THROTTLE_BPS_READ].max = 56; | 114 | trace_qemu_co_queue_run_restart(co); |
40 | orig_cfg.buckets[THROTTLE_BPS_WRITE].max = 120; | 115 | - while ((next = QSIMPLEQ_FIRST(&co->co_queue_wakeup))) { |
41 | 116 | - QSIMPLEQ_REMOVE_HEAD(&co->co_queue_wakeup, co_queue_next); | |
42 | orig_cfg.buckets[THROTTLE_OPS_TOTAL].max = 150; | 117 | + |
43 | @@ -XXX,XX +XXX,XX @@ static void test_config_functions(void) | 118 | + /* Because "co" has yielded, any coroutine that we wakeup can resume it. |
44 | g_assert(final_cfg.buckets[THROTTLE_OPS_READ].avg == 69); | 119 | + * If this happens and "co" terminates, co->co_queue_wakeup becomes |
45 | g_assert(final_cfg.buckets[THROTTLE_OPS_WRITE].avg == 23); | 120 | + * invalid memory. Therefore, use a temporary queue and do not touch |
46 | 121 | + * the "co" coroutine as soon as you enter another one. | |
47 | - g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 15.3); /* fixed */ | 122 | + * |
48 | - g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56); /* not fixed */ | 123 | + * In its turn resumed "co" can pupulate "co_queue_wakeup" queue with |
49 | + g_assert(final_cfg.buckets[THROTTLE_BPS_TOTAL].max == 0); | 124 | + * new coroutines to be woken up. The caller, who has resumed "co", |
50 | + g_assert(final_cfg.buckets[THROTTLE_BPS_READ].max == 56); | 125 | + * will be responsible for traversing the same queue, which may cause |
51 | g_assert(final_cfg.buckets[THROTTLE_BPS_WRITE].max == 120); | 126 | + * a different wakeup order but not any missing wakeups. |
52 | 127 | + */ | |
53 | g_assert(final_cfg.buckets[THROTTLE_OPS_TOTAL].max == 150); | 128 | + QSIMPLEQ_CONCAT(&tmp_queue_wakeup, &co->co_queue_wakeup); |
54 | diff --git a/util/throttle.c b/util/throttle.c | 129 | + |
55 | index XXXXXXX..XXXXXXX 100644 | 130 | + while ((next = QSIMPLEQ_FIRST(&tmp_queue_wakeup))) { |
56 | --- a/util/throttle.c | 131 | + QSIMPLEQ_REMOVE_HEAD(&tmp_queue_wakeup, co_queue_next); |
57 | +++ b/util/throttle.c | 132 | qemu_coroutine_enter(next); |
58 | @@ -XXX,XX +XXX,XX @@ static void throttle_fix_bucket(LeakyBucket *bkt) | ||
59 | } | 133 | } |
60 | } | 134 | } |
61 | 135 | diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c | |
62 | +/* undo internal bucket parameter changes (see throttle_fix_bucket()) */ | 136 | index XXXXXXX..XXXXXXX 100644 |
63 | +static void throttle_unfix_bucket(LeakyBucket *bkt) | 137 | --- a/util/qemu-coroutine.c |
64 | +{ | 138 | +++ b/util/qemu-coroutine.c |
65 | + if (bkt->max < bkt->avg) { | 139 | @@ -XXX,XX +XXX,XX @@ void qemu_aio_coroutine_enter(AioContext *ctx, Coroutine *co) |
66 | + bkt->max = 0; | 140 | |
67 | + } | 141 | qemu_co_queue_run_restart(co); |
68 | +} | 142 | |
143 | + /* Beware, if ret == COROUTINE_YIELD and qemu_co_queue_run_restart() | ||
144 | + * has started any other coroutine, "co" might have been reentered | ||
145 | + * and even freed by now! So be careful and do not touch it. | ||
146 | + */ | ||
69 | + | 147 | + |
70 | /* take care of canceling a timer */ | 148 | switch (ret) { |
71 | static void throttle_cancel_timer(QEMUTimer *timer) | 149 | case COROUTINE_YIELD: |
72 | { | 150 | return; |
73 | @@ -XXX,XX +XXX,XX @@ void throttle_config(ThrottleState *ts, | ||
74 | */ | ||
75 | void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg) | ||
76 | { | ||
77 | + int i; | ||
78 | + | ||
79 | *cfg = ts->cfg; | ||
80 | + | ||
81 | + for (i = 0; i < BUCKETS_COUNT; i++) { | ||
82 | + throttle_unfix_bucket(&cfg->buckets[i]); | ||
83 | + } | ||
84 | } | ||
85 | |||
86 | |||
87 | -- | 151 | -- |
88 | 2.9.3 | 152 | 2.9.4 |
89 | 153 | ||
90 | 154 | diff view generated by jsdifflib |
1 | From: Lidong Chen <lidongchen@tencent.com> | 1 | From: Bruno Dominguez <bru.dominguez@gmail.com> |
---|---|---|---|
2 | 2 | ||
3 | BLOCK_SIZE is (1 << 20), qcow2 cluster size is 65536 by default, | 3 | There was no possibility to add specific cxx flags using the configure |
4 | this may cause the qcow2 file size to be bigger after migration. | 4 | file. So A new entrance has been created to support it. |
5 | This patch checks each cluster, using blk_pwrite_zeroes for each | ||
6 | zero cluster. | ||
7 | 5 | ||
8 | [Initialize cluster_size to BLOCK_SIZE to prevent a gcc uninitialized | 6 | Duplication of information in configure and rules.mak. Taking |
9 | variable compiler warning. In reality we always initialize cluster_size | 7 | QEMU_CFLAGS and add them to QEMU_CXXFLAGS, now the value of |
10 | in a conditional but gcc doesn't know that. | 8 | QEMU_CXXFLAGS is stored in config-host.mak, so there is no need for |
11 | --Stefan] | 9 | it. |
12 | 10 | ||
11 | The makefile for libvixl was adding flags for QEMU_CXXFLAGS in | ||
12 | QEMU_CFLAGS because of the addition in rules.mak. That was removed, so | ||
13 | adding them where it should be. | ||
14 | |||
15 | Signed-off-by: Bruno Dominguez <bru.dominguez@gmail.com> | ||
13 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
14 | Signed-off-by: Lidong Chen <lidongchen@tencent.com> | 17 | Message-id: 1496754467-20893-1-git-send-email-bru.dominguez@gmail.com |
15 | Message-id: 1492050868-16200-1-git-send-email-lidongchen@tencent.com | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
17 | --- | 19 | --- |
18 | migration/block.c | 35 +++++++++++++++++++++++++++++++++-- | 20 | configure | 74 +++++++++++++++++++++++++-------------------- |
19 | 1 file changed, 33 insertions(+), 2 deletions(-) | 21 | disas/libvixl/Makefile.objs | 4 +-- |
22 | rules.mak | 3 -- | ||
23 | 3 files changed, 44 insertions(+), 37 deletions(-) | ||
20 | 24 | ||
21 | diff --git a/migration/block.c b/migration/block.c | 25 | diff --git a/configure b/configure |
26 | index XXXXXXX..XXXXXXX 100755 | ||
27 | --- a/configure | ||
28 | +++ b/configure | ||
29 | @@ -XXX,XX +XXX,XX @@ update_cxxflags() { | ||
30 | # Set QEMU_CXXFLAGS from QEMU_CFLAGS by filtering out those | ||
31 | # options which some versions of GCC's C++ compiler complain about | ||
32 | # because they only make sense for C programs. | ||
33 | - QEMU_CXXFLAGS= | ||
34 | + QEMU_CXXFLAGS="$QEMU_CXXFLAGS -D__STDC_LIMIT_MACROS" | ||
35 | + | ||
36 | for arg in $QEMU_CFLAGS; do | ||
37 | case $arg in | ||
38 | -Wstrict-prototypes|-Wmissing-prototypes|-Wnested-externs|\ | ||
39 | @@ -XXX,XX +XXX,XX @@ for opt do | ||
40 | --extra-cflags=*) QEMU_CFLAGS="$QEMU_CFLAGS $optarg" | ||
41 | EXTRA_CFLAGS="$optarg" | ||
42 | ;; | ||
43 | + --extra-cxxflags=*) QEMU_CXXFLAGS="$QEMU_CXXFLAGS $optarg" | ||
44 | + EXTRA_CXXFLAGS="$optarg" | ||
45 | + ;; | ||
46 | --extra-ldflags=*) LDFLAGS="$LDFLAGS $optarg" | ||
47 | EXTRA_LDFLAGS="$optarg" | ||
48 | ;; | ||
49 | @@ -XXX,XX +XXX,XX @@ for opt do | ||
50 | ;; | ||
51 | --extra-cflags=*) | ||
52 | ;; | ||
53 | + --extra-cxxflags=*) | ||
54 | + ;; | ||
55 | --extra-ldflags=*) | ||
56 | ;; | ||
57 | --enable-debug-info) | ||
58 | @@ -XXX,XX +XXX,XX @@ Advanced options (experts only): | ||
59 | --cxx=CXX use C++ compiler CXX [$cxx] | ||
60 | --objcc=OBJCC use Objective-C compiler OBJCC [$objcc] | ||
61 | --extra-cflags=CFLAGS append extra C compiler flags QEMU_CFLAGS | ||
62 | + --extra-cxxflags=CXXFLAGS append extra C++ compiler flags QEMU_CXXFLAGS | ||
63 | --extra-ldflags=LDFLAGS append extra linker flags LDFLAGS | ||
64 | --make=MAKE use specified make [$make] | ||
65 | --install=INSTALL use specified install [$install] | ||
66 | @@ -XXX,XX +XXX,XX @@ if test "$bogus_os" = "yes"; then | ||
67 | error_exit "Unrecognized host OS $targetos" | ||
68 | fi | ||
69 | |||
70 | -# Check that the C++ compiler exists and works with the C compiler | ||
71 | -if has $cxx; then | ||
72 | - cat > $TMPC <<EOF | ||
73 | -int c_function(void); | ||
74 | -int main(void) { return c_function(); } | ||
75 | -EOF | ||
76 | - | ||
77 | - compile_object | ||
78 | - | ||
79 | - cat > $TMPCXX <<EOF | ||
80 | -extern "C" { | ||
81 | - int c_function(void); | ||
82 | -} | ||
83 | -int c_function(void) { return 42; } | ||
84 | -EOF | ||
85 | - | ||
86 | - update_cxxflags | ||
87 | - | ||
88 | - if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPCXX $TMPO $LDFLAGS; then | ||
89 | - # C++ compiler $cxx works ok with C compiler $cc | ||
90 | - : | ||
91 | - else | ||
92 | - echo "C++ compiler $cxx does not work with C compiler $cc" | ||
93 | - echo "Disabling C++ specific optional code" | ||
94 | - cxx= | ||
95 | - fi | ||
96 | -else | ||
97 | - echo "No C++ compiler available; disabling C++ specific optional code" | ||
98 | - cxx= | ||
99 | -fi | ||
100 | - | ||
101 | gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits" | ||
102 | gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags" | ||
103 | gcc_flags="-Wno-missing-include-dirs -Wempty-body -Wnested-externs $gcc_flags" | ||
104 | @@ -XXX,XX +XXX,XX @@ EOF | ||
105 | fi | ||
106 | fi | ||
107 | |||
108 | +# Check that the C++ compiler exists and works with the C compiler. | ||
109 | +# All the QEMU_CXXFLAGS are based on QEMU_CFLAGS. Keep this at the end to don't miss any other that could be added. | ||
110 | +if has $cxx; then | ||
111 | + cat > $TMPC <<EOF | ||
112 | +int c_function(void); | ||
113 | +int main(void) { return c_function(); } | ||
114 | +EOF | ||
115 | + | ||
116 | + compile_object | ||
117 | + | ||
118 | + cat > $TMPCXX <<EOF | ||
119 | +extern "C" { | ||
120 | + int c_function(void); | ||
121 | +} | ||
122 | +int c_function(void) { return 42; } | ||
123 | +EOF | ||
124 | + | ||
125 | + update_cxxflags | ||
126 | + | ||
127 | + if do_cxx $QEMU_CXXFLAGS -o $TMPE $TMPCXX $TMPO $LDFLAGS; then | ||
128 | + # C++ compiler $cxx works ok with C compiler $cc | ||
129 | + : | ||
130 | + else | ||
131 | + echo "C++ compiler $cxx does not work with C compiler $cc" | ||
132 | + echo "Disabling C++ specific optional code" | ||
133 | + cxx= | ||
134 | + fi | ||
135 | +else | ||
136 | + echo "No C++ compiler available; disabling C++ specific optional code" | ||
137 | + cxx= | ||
138 | +fi | ||
139 | + | ||
140 | echo_version() { | ||
141 | if test "$1" = "yes" ; then | ||
142 | echo "($2)" | ||
143 | @@ -XXX,XX +XXX,XX @@ if test "$mingw32" = "no" ; then | ||
144 | fi | ||
145 | echo "qemu_helperdir=$libexecdir" >> $config_host_mak | ||
146 | echo "extra_cflags=$EXTRA_CFLAGS" >> $config_host_mak | ||
147 | +echo "extra_cxxflags=$EXTRA_CXXFLAGS" >> $config_host_mak | ||
148 | echo "extra_ldflags=$EXTRA_LDFLAGS" >> $config_host_mak | ||
149 | echo "qemu_localedir=$qemu_localedir" >> $config_host_mak | ||
150 | echo "libs_softmmu=$libs_softmmu" >> $config_host_mak | ||
151 | @@ -XXX,XX +XXX,XX @@ echo "WINDRES=$windres" >> $config_host_mak | ||
152 | echo "CFLAGS=$CFLAGS" >> $config_host_mak | ||
153 | echo "CFLAGS_NOPIE=$CFLAGS_NOPIE" >> $config_host_mak | ||
154 | echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak | ||
155 | +echo "QEMU_CXXFLAGS=$QEMU_CXXFLAGS" >> $config_host_mak | ||
156 | echo "QEMU_INCLUDES=$QEMU_INCLUDES" >> $config_host_mak | ||
157 | if test "$sparse" = "yes" ; then | ||
158 | echo "CC := REAL_CC=\"\$(CC)\" cgcc" >> $config_host_mak | ||
159 | diff --git a/disas/libvixl/Makefile.objs b/disas/libvixl/Makefile.objs | ||
22 | index XXXXXXX..XXXXXXX 100644 | 160 | index XXXXXXX..XXXXXXX 100644 |
23 | --- a/migration/block.c | 161 | --- a/disas/libvixl/Makefile.objs |
24 | +++ b/migration/block.c | 162 | +++ b/disas/libvixl/Makefile.objs |
25 | @@ -XXX,XX +XXX,XX @@ static int block_load(QEMUFile *f, void *opaque, int version_id) | 163 | @@ -XXX,XX +XXX,XX @@ libvixl_OBJS = vixl/utils.o \ |
26 | int64_t total_sectors = 0; | 164 | |
27 | int nr_sectors; | 165 | # The -Wno-sign-compare is needed only for gcc 4.6, which complains about |
28 | int ret; | 166 | # some signed-unsigned equality comparisons which later gcc versions do not. |
29 | + BlockDriverInfo bdi; | 167 | -$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CFLAGS := -I$(SRC_PATH)/disas/libvixl $(QEMU_CFLAGS) -Wno-sign-compare |
30 | + int cluster_size = BLOCK_SIZE; | 168 | +$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CXXFLAGS := -I$(SRC_PATH)/disas/libvixl $(QEMU_CXXFLAGS) -Wno-sign-compare |
31 | 169 | # Ensure that C99 macros are defined regardless of the inclusion order of | |
32 | do { | 170 | # headers in vixl. This is required at least on NetBSD. |
33 | addr = qemu_get_be64(f); | 171 | -$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CFLAGS += -D__STDC_CONSTANT_MACROS -D__STDC_LIMIT_MACROS -D__STDC_FORMAT_MACROS |
34 | @@ -XXX,XX +XXX,XX @@ static int block_load(QEMUFile *f, void *opaque, int version_id) | 172 | +$(addprefix $(obj)/,$(libvixl_OBJS)): QEMU_CXXFLAGS += -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS |
35 | error_report_err(local_err); | 173 | |
36 | return -EINVAL; | 174 | common-obj-$(CONFIG_ARM_A64_DIS) += $(libvixl_OBJS) |
37 | } | 175 | diff --git a/rules.mak b/rules.mak |
38 | + | 176 | index XXXXXXX..XXXXXXX 100644 |
39 | + ret = bdrv_get_info(blk_bs(blk), &bdi); | 177 | --- a/rules.mak |
40 | + if (ret == 0 && bdi.cluster_size > 0 && | 178 | +++ b/rules.mak |
41 | + bdi.cluster_size <= BLOCK_SIZE && | 179 | @@ -XXX,XX +XXX,XX @@ MAKEFLAGS += -rR |
42 | + BLOCK_SIZE % bdi.cluster_size == 0) { | 180 | %.mak: |
43 | + cluster_size = bdi.cluster_size; | 181 | clean-target: |
44 | + } else { | 182 | |
45 | + cluster_size = BLOCK_SIZE; | 183 | -# Flags for C++ compilation |
46 | + } | 184 | -QEMU_CXXFLAGS = -D__STDC_LIMIT_MACROS $(filter-out -Wstrict-prototypes -Wmissing-prototypes -Wnested-externs -Wold-style-declaration -Wold-style-definition -Wredundant-decls, $(QEMU_CFLAGS)) |
47 | } | 185 | - |
48 | 186 | # Flags for dependency generation | |
49 | if (total_sectors - addr < BDRV_SECTORS_PER_DIRTY_CHUNK) { | 187 | QEMU_DGFLAGS += -MMD -MP -MT $@ -MF $(@D)/$(*F).d |
50 | @@ -XXX,XX +XXX,XX @@ static int block_load(QEMUFile *f, void *opaque, int version_id) | ||
51 | nr_sectors * BDRV_SECTOR_SIZE, | ||
52 | BDRV_REQ_MAY_UNMAP); | ||
53 | } else { | ||
54 | + int i; | ||
55 | + int64_t cur_addr; | ||
56 | + uint8_t *cur_buf; | ||
57 | + | ||
58 | buf = g_malloc(BLOCK_SIZE); | ||
59 | qemu_get_buffer(f, buf, BLOCK_SIZE); | ||
60 | - ret = blk_pwrite(blk, addr * BDRV_SECTOR_SIZE, buf, | ||
61 | - nr_sectors * BDRV_SECTOR_SIZE, 0); | ||
62 | + for (i = 0; i < BLOCK_SIZE / cluster_size; i++) { | ||
63 | + cur_addr = addr * BDRV_SECTOR_SIZE + i * cluster_size; | ||
64 | + cur_buf = buf + i * cluster_size; | ||
65 | + | ||
66 | + if ((!block_mig_state.zero_blocks || | ||
67 | + cluster_size < BLOCK_SIZE) && | ||
68 | + buffer_is_zero(cur_buf, cluster_size)) { | ||
69 | + ret = blk_pwrite_zeroes(blk, cur_addr, | ||
70 | + cluster_size, | ||
71 | + BDRV_REQ_MAY_UNMAP); | ||
72 | + } else { | ||
73 | + ret = blk_pwrite(blk, cur_addr, cur_buf, | ||
74 | + cluster_size, 0); | ||
75 | + } | ||
76 | + if (ret < 0) { | ||
77 | + break; | ||
78 | + } | ||
79 | + } | ||
80 | g_free(buf); | ||
81 | } | ||
82 | 188 | ||
83 | -- | 189 | -- |
84 | 2.9.3 | 190 | 2.9.4 |
85 | 191 | ||
86 | 192 | diff view generated by jsdifflib |