* [PATCH] block/mirror: fix core when using iothreads
@ 2020-08-26 13:19 Peng Liang
2020-09-23 14:50 ` John Snow
0 siblings, 1 reply; 3+ messages in thread
From: Peng Liang @ 2020-08-26 13:19 UTC (permalink / raw)
To: qemu-block, kwolf, jsnow
Cc: Peng Liang, qemu-devel, xiexiangyou, zhang.zhanghailiang
We found an issue when doing block-commit with iothreads, which tries to
dereference a NULL pointer.
<main thread> |<IO thread>
|
mirror_start_job |
1. bdrv_ref(mirror_top_bs); |
bdrv_drained_begin(bs); |
bdrv_append(mirror_top_bs, |
bs, &local_err); |
bdrv_drained_end(bs); |
|
2. s = block_job_create(...); |
|bdrv_mirror_top_pwritev
| MirrorBDSOpaque *s = bs->opaque;
| bool copy_to_target;
| copy_to_target = s->job->ret >= 0 &&
| s->job->copy_mode ==
| MIRROR_COPY_MODE_WRITE_BLOCKING;
| (s->job is not NULL until 3!)
3. bs_opaque->job = s; |
Just moving step 2 & 3 before 1 can avoid this.
Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Peng Liang <liangpeng10@huawei.com>
---
block/mirror.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index e8e8844afc40..7c872be71149 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1600,6 +1600,16 @@ static BlockJob *mirror_start_job(
mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
BDRV_REQ_NO_FALLBACK;
bs_opaque = g_new0(MirrorBDSOpaque, 1);
+ /* Make sure that the source is not resized while the job is running */
+ s = block_job_create(job_id, driver, NULL, bs,
+ BLK_PERM_CONSISTENT_READ,
+ BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
+ BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
+ creation_flags, cb, opaque, errp);
+ if (!s) {
+ goto fail;
+ }
+ bs_opaque->job = s;
mirror_top_bs->opaque = bs_opaque;
/* bdrv_append takes ownership of the mirror_top_bs reference, need to keep
@@ -1612,19 +1622,8 @@ static BlockJob *mirror_start_job(
if (local_err) {
bdrv_unref(mirror_top_bs);
error_propagate(errp, local_err);
- return NULL;
- }
-
- /* Make sure that the source is not resized while the job is running */
- s = block_job_create(job_id, driver, NULL, mirror_top_bs,
- BLK_PERM_CONSISTENT_READ,
- BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
- BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
- creation_flags, cb, opaque, errp);
- if (!s) {
goto fail;
}
- bs_opaque->job = s;
/* The block job now has a reference to this node */
bdrv_unref(mirror_top_bs);
--
2.18.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] block/mirror: fix core when using iothreads
2020-08-26 13:19 [PATCH] block/mirror: fix core when using iothreads Peng Liang
@ 2020-09-23 14:50 ` John Snow
2021-02-01 12:17 ` Peng Liang
0 siblings, 1 reply; 3+ messages in thread
From: John Snow @ 2020-09-23 14:50 UTC (permalink / raw)
To: Peng Liang, qemu-block, kwolf
Cc: qemu-devel, xiexiangyou, zhang.zhanghailiang
On 8/26/20 9:19 AM, Peng Liang wrote:
> We found an issue when doing block-commit with iothreads, which tries to
> dereference a NULL pointer.
>
I'm clearing out my patch backlog. I am a bit out of the loop on block
issues at the moment, did this issue get addressed in a later patch that
I am not seeing, or does it still need attention?
--js
> <main thread> |<IO thread>
> |
> mirror_start_job |
> 1. bdrv_ref(mirror_top_bs); |
> bdrv_drained_begin(bs); |
> bdrv_append(mirror_top_bs, |
> bs, &local_err); |
> bdrv_drained_end(bs); |
> |
> 2. s = block_job_create(...); |
> |bdrv_mirror_top_pwritev
> | MirrorBDSOpaque *s = bs->opaque;
> | bool copy_to_target;
> | copy_to_target = s->job->ret >= 0 &&
> | s->job->copy_mode ==
> | MIRROR_COPY_MODE_WRITE_BLOCKING;
> | (s->job is not NULL until 3!)
> 3. bs_opaque->job = s; |
>
> Just moving step 2 & 3 before 1 can avoid this.
>
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Peng Liang <liangpeng10@huawei.com>
> ---
> block/mirror.c | 21 ++++++++++-----------
> 1 file changed, 10 insertions(+), 11 deletions(-)
>
> diff --git a/block/mirror.c b/block/mirror.c
> index e8e8844afc40..7c872be71149 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1600,6 +1600,16 @@ static BlockJob *mirror_start_job(
> mirror_top_bs->supported_zero_flags = BDRV_REQ_WRITE_UNCHANGED |
> BDRV_REQ_NO_FALLBACK;
> bs_opaque = g_new0(MirrorBDSOpaque, 1);
> + /* Make sure that the source is not resized while the job is running */
> + s = block_job_create(job_id, driver, NULL, bs,
> + BLK_PERM_CONSISTENT_READ,
> + BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
> + BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
> + creation_flags, cb, opaque, errp);
> + if (!s) {
> + goto fail;
> + }
> + bs_opaque->job = s;
> mirror_top_bs->opaque = bs_opaque;
>
> /* bdrv_append takes ownership of the mirror_top_bs reference, need to keep
> @@ -1612,19 +1622,8 @@ static BlockJob *mirror_start_job(
> if (local_err) {
> bdrv_unref(mirror_top_bs);
> error_propagate(errp, local_err);
> - return NULL;
> - }
> -
> - /* Make sure that the source is not resized while the job is running */
> - s = block_job_create(job_id, driver, NULL, mirror_top_bs,
> - BLK_PERM_CONSISTENT_READ,
> - BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE_UNCHANGED |
> - BLK_PERM_WRITE | BLK_PERM_GRAPH_MOD, speed,
> - creation_flags, cb, opaque, errp);
> - if (!s) {
> goto fail;
> }
> - bs_opaque->job = s;
>
> /* The block job now has a reference to this node */
> bdrv_unref(mirror_top_bs);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] block/mirror: fix core when using iothreads
2020-09-23 14:50 ` John Snow
@ 2021-02-01 12:17 ` Peng Liang
0 siblings, 0 replies; 3+ messages in thread
From: Peng Liang @ 2021-02-01 12:17 UTC (permalink / raw)
To: John Snow, qemu-block, kwolf; +Cc: qemu-devel, xiexiangyou, zhang.zhanghailiang
On 9/23/2020 10:50 PM, John Snow wrote:
> On 8/26/20 9:19 AM, Peng Liang wrote:
>> We found an issue when doing block-commit with iothreads, which tries to
>> dereference a NULL pointer.
>>
>
> I'm clearing out my patch backlog. I am a bit out of the loop on block
> issues at the moment, did this issue get addressed in a later patch that
> I am not seeing, or does it still need attention?
>
> --js
>
Hi John,
I'm very sorry for missing your reply.
Michael encountered the problem too and gave a reproducer script.
Thanks,
Peng
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-01 12:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-26 13:19 [PATCH] block/mirror: fix core when using iothreads Peng Liang
2020-09-23 14:50 ` John Snow
2021-02-01 12:17 ` Peng Liang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).