from:"仇大玉"

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

2021-02-01 Thread 仇大玉

I'm so sorry, forgive my mail client(outlook)

I have try your solution, It doesn't work, still cause crash.

The reason is:  we come to bdrv_mirror_top_pwritev() (which means that 
mirror-top node is already inserted into block graph), but its bs->opaque->job 
is not initialized"

But the root cause is that in block_job_create() we released(unnecessary) the 
aio_context, and the iothread get the context.

Script has to part, one is run in the VM (to give some workload) we named 
script A:
#!/bin/sh
For((i=1;i<=1;i++));
Do
dd if=/dev/zero of=./xxx bs=1M count=200
sleep 6
done

Another one is in the hypervisor, we named script B:
#!/bin/sh
for((i=1;i<=1000;i++));
do
virsh snapshot-create-as fqtest --atomic --no-metadata --name fq6 --disk-only  
--diskspec vda,snapshot=external,file=/home/michael/snapshot/fq6.qcow2;
virsh blockcommit fqtest /home/michael/snapshot/fq6.qcow2 --shallow --verbose 
--wait --pivot --top /home/michael/snapshot/fq6.qcow2;
rm -r fq6.qcow2
done

How to reproduce:
1. start a VM, my case is use libvirt, named fqtest
2. run script B in hypervisor
3. after guest boot up, login and run script A in vda.

Make sure, the IO thread enabled for vda.

Mostly, just wait for several minutes, it will crash. 

The whole thread backtrace is:

[Switching to Thread 0x7f7c7d91f700 (LWP 99907)]
0x5576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437
1437../block/mirror.c: No such file or directory.
(gdb) p s->job
$17 = (MirrorBlockJob *) 0x0
(gdb) p s->stop
$18 = false

(gdb) bt
#0  0x5576d0f65aab in bdrv_mirror_top_pwritev at ../block/mirror.c:1437
#1  0x5576d0f7f3ab in bdrv_driver_pwritev at ../block/io.c:1174
#2  0x5576d0f8139d in bdrv_aligned_pwritev at ../block/io.c:1988
#3  0x5576d0f81b65 in bdrv_co_pwritev_part at ../block/io.c:2156
#4  0x5576d0f8e6b7 in blk_do_pwritev_part at ../block/block-backend.c:1260
#5  0x5576d0f8e84d in blk_aio_write_entry at ../block/block-backend.c:1476
#6  0x5576d1060ddb in coroutine_trampoline at 
../util/coroutine-ucontext.c:173
#7  0x7f7c8d3be0d0 in __start_context at /lib/../lib64/libc.so.6
#8  0x7f7b52beb1e0 in  ()
#9  0x in  ()

Switch to qemu main thread:
#0  0x7f903be704ed in __lll_lock_wait at
/lib/../lib64/libpthread.so.0
#1  0x7f903be6bde6 in _L_lock_941 at /lib/../lib64/libpthread.so.0
#2  0x7f903be6bcdf in pthread_mutex_lock at
/lib/../lib64/libpthread.so.0
#3  0x564b21456889 in qemu_mutex_lock_impl at
../util/qemu-thread-posix.c:79
#4  0x564b213af8a5 in block_job_add_bdrv at ../blockjob.c:224
#5  0x564b213b00ad in block_job_create at ../blockjob.c:440
#6  0x564b21357c0a in mirror_start_job at ../block/mirror.c:1622
#7  0x564b2135a9af in commit_active_start at ../block/mirror.c:1867
#8  0x564b2133d132 in qmp_block_commit at ../blockdev.c:2768
#9  0x564b2141fef3 in qmp_marshal_block_commit at
qapi/qapi-commands-block-core.c:346
#10 0x564b214503c9 in do_qmp_dispatch_bh at
../qapi/qmp-dispatch.c:110
#11 0x564b21451996 in aio_bh_poll at ../util/async.c:164
#12 0x564b2146018e in aio_dispatch at ../util/aio-posix.c:381
#13 0x564b2145187e in aio_ctx_dispatch at ../util/async.c:306
#14 0x7f9040239049 in g_main_context_dispatch at
/lib/../lib64/libglib-2.0.so.0
#15 0x564b21447368 in main_loop_wait at ../util/main-loop.c:232
#16 0x564b21447368 in main_loop_wait at ../util/main-loop.c:255
#17 0x564b21447368 in main_loop_wait at ../util/main-loop.c:531
#18 0x564b212304e1 in qemu_main_loop at ../softmmu/runstate.c:721
#19 0x564b20f7975e in main at ../softmmu/main.c:50

Thanks,
Michael

-Original Message-
From: Vladimir Sementsov-Ogievskiy  
Sent: 2021年2月1日 18:28
To: 08005...@163.com; kw...@redhat.com; mre...@redhat.com; js...@redhat.com
Cc: 仇大玉 ; qemu-de...@nongnu.org; qemu-block@nongnu.org
Subject: Re: [PATCH v4] blockjob: Fix crash with IOthread when block commit 
after snapshot

Hi!

Tanks for fixing and sorry for a delay!

Please send each new version of a patch as a separate branch. It's a rule from 
https://wiki.qemu.org/Contribute/SubmitAPatch and it is more readable and less 
probable that your patch will be missed.

28.01.2021 04:30, 08005...@163.com wrote:
> From: Michael Qiu 
> 
> v4: rebase to latest code
> 
> v3: reformat the commit log, remove duplicate content
> 
> v2: modify the coredump backtrace within commit log with the newest
>  qemu with master branch

Such things shouldn't be in a commit message. You may put such comments after 
--- line[*] in a generated patch email

> 
> Currently, if guest has workloads, IO thread will acquire aio_context 
> lock before do io_submit, it leads to segmentfault when do block 
> commit after snapshot. Just like below:

Do you have some reproducer script?

> 
> Program received signal SIGSEGV, Segmentation fault.
> [Switching to Thread 0x7f7c7d91f700 (LWP 99907)] 0x

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

2021-01-31 Thread 仇大玉

Any comments?

It's really a bug and can cause the qemu to segmentfault.

Thanks,
Michael

-Original Message-
From: 仇大玉 
Sent: 2021年1月28日 13:16
To: qemu-block@nongnu.org; qemu-de...@nongnu.org
Cc: kw...@redhat.com; mre...@redhat.com; js...@redhat.com; 08005...@163.com
Subject: RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit 
after snapshot

Any comments?

-Original Message-
From: 08005...@163.com <08005...@163.com> 
Sent: 2021年1月28日 9:31
To: kw...@redhat.com; mre...@redhat.com; js...@redhat.com
Cc: qemu-block@nongnu.org; qemu-de...@nongnu.org; 仇大玉 
Subject: [PATCH v4] blockjob: Fix crash with IOthread when block commit after 
snapshot

From: Michael Qiu 

v4: rebase to latest code

v3: reformat the commit log, remove duplicate content

v2: modify the coredump backtrace within commit log with the newest
qemu with master branch

Currently, if guest has workloads, IO thread will acquire aio_context lock 
before do io_submit, it leads to segmentfault when do block commit after 
snapshot. Just like below:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f7c7d91f700 (LWP 99907)] 0x5576d0f65aab in 
bdrv_mirror_top_pwritev at ../block/mirror.c:1437
1437../block/mirror.c: No such file or directory.
(gdb) p s->job
$17 = (MirrorBlockJob *) 0x0
(gdb) p s->stop
$18 = false

(gdb) bt

Switch to qemu main thread:
/lib/../lib64/libpthread.so.0
/lib/../lib64/libpthread.so.0
../util/qemu-thread-posix.c:79
qapi/qapi-commands-block-core.c:346
../qapi/qmp-dispatch.c:110
/lib/../lib64/libglib-2.0.so.0

In IO thread when do bdrv_mirror_top_pwritev, the job is NULL, and stop field 
is false, this means the MirrorBDSOpaque "s" object has not been initialized 
yet, and this object is initialized by block_job_create(), but the initialize 
process is stuck in acquiring the lock.

The rootcause is that qemu do release/acquire when hold the lock, at the same 
time, IO thread get the lock after release stage, and the crash occured.

Actually, in this situation, job->job.aio_context will not equal to 
qemu_get_aio_context(), and will be the same as bs->aio_context, thus, no need 
to release the lock, becasue bdrv_root_attach_child() will not change the 
context.

This patch fix this issue.

Signed-off-by: Michael Qiu 
---
 blockjob.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 98ac8af982..51a09f3b60 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -214,13 +214,15 @@ int block_job_add_bdrv(BlockJob *job, const char *name, 
BlockDriverState *bs,
 BdrvChild *c;
 
 bdrv_ref(bs);
-if (job->job.aio_context != qemu_get_aio_context()) {
+if (bdrv_get_aio_context(bs) != job->job.aio_context &&
+job->job.aio_context != qemu_get_aio_context()) {
 aio_context_release(job->job.aio_context);
 }
 c = bdrv_root_attach_child(bs, name, _job, 0,
job->job.aio_context, perm, shared_perm, job,
errp);
-if (job->job.aio_context != qemu_get_aio_context()) {
+if (bdrv_get_aio_context(bs) != job->job.aio_context &&
+job->job.aio_context != qemu_get_aio_context()) {
 aio_context_acquire(job->job.aio_context);
 }
 if (c == NULL) {
--
2.22.0

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

2021-01-28 Thread 仇大玉

Any comments?

-Original Message-
From: 08005...@163.com <08005...@163.com> 
Sent: 2021年1月28日 9:31
To: kw...@redhat.com; mre...@redhat.com; js...@redhat.com
Cc: qemu-block@nongnu.org; qemu-de...@nongnu.org; 仇大玉 
Subject: [PATCH v4] blockjob: Fix crash with IOthread when block commit after 
snapshot

From: Michael Qiu 

v4: rebase to latest code

v3: reformat the commit log, remove duplicate content

v2: modify the coredump backtrace within commit log with the newest
qemu with master branch

Currently, if guest has workloads, IO thread will acquire aio_context lock 
before do io_submit, it leads to segmentfault when do block commit after 
snapshot. Just like below:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7f7c7d91f700 (LWP 99907)] 0x5576d0f65aab in 
bdrv_mirror_top_pwritev at ../block/mirror.c:1437
1437../block/mirror.c: No such file or directory.
(gdb) p s->job
$17 = (MirrorBlockJob *) 0x0
(gdb) p s->stop
$18 = false

(gdb) bt

Switch to qemu main thread:
/lib/../lib64/libpthread.so.0
/lib/../lib64/libpthread.so.0
../util/qemu-thread-posix.c:79
qapi/qapi-commands-block-core.c:346
../qapi/qmp-dispatch.c:110
/lib/../lib64/libglib-2.0.so.0

In IO thread when do bdrv_mirror_top_pwritev, the job is NULL, and stop field 
is false, this means the MirrorBDSOpaque "s" object has not been initialized 
yet, and this object is initialized by block_job_create(), but the initialize 
process is stuck in acquiring the lock.

The rootcause is that qemu do release/acquire when hold the lock, at the same 
time, IO thread get the lock after release stage, and the crash occured.

Actually, in this situation, job->job.aio_context will not equal to 
qemu_get_aio_context(), and will be the same as bs->aio_context, thus, no need 
to release the lock, becasue bdrv_root_attach_child() will not change the 
context.

This patch fix this issue.

Signed-off-by: Michael Qiu 
---
 blockjob.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index 98ac8af982..51a09f3b60 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -214,13 +214,15 @@ int block_job_add_bdrv(BlockJob *job, const char *name, 
BlockDriverState *bs,
 BdrvChild *c;

 bdrv_ref(bs);
-if (job->job.aio_context != qemu_get_aio_context()) {
+if (bdrv_get_aio_context(bs) != job->job.aio_context &&
+job->job.aio_context != qemu_get_aio_context()) {
 aio_context_release(job->job.aio_context);
 }
 c = bdrv_root_attach_child(bs, name, _job, 0,
job->job.aio_context, perm, shared_perm, job,
errp);
-if (job->job.aio_context != qemu_get_aio_context()) {
+if (bdrv_get_aio_context(bs) != job->job.aio_context &&
+job->job.aio_context != qemu_get_aio_context()) {
 aio_context_acquire(job->job.aio_context);
 }
 if (c == NULL) {
--
2.22.0

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

RE: [PATCH v4] blockjob: Fix crash with IOthread when block commit after snapshot

3 matches

Site Navigation

Mail list logo

Footer information