[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH] block/replication.c: Fix crash issue after fail
From: |
Zhang, Chen |
Subject: |
Re: [Qemu-block] [PATCH] block/replication.c: Fix crash issue after failover |
Date: |
Sat, 15 Jun 2019 19:05:31 +0000 |
> -----Original Message-----
> From: Kevin Wolf [mailto:address@hidden]
> Sent: Friday, June 14, 2019 6:18 PM
> To: Zhang, Chen <address@hidden>
> Cc: Xie Changlong <address@hidden>; Max Reitz
> <address@hidden>; qemu-block <address@hidden>; qemu-dev
> <address@hidden>; Zhang Chen <address@hidden>;
> address@hidden
> Subject: Re: [PATCH] block/replication.c: Fix crash issue after failover
>
> Am 14.06.2019 um 11:28 hat Zhang Chen geschrieben:
> > From: Zhang Chen <address@hidden>
> >
> > No block job on active disk after failover.
> > In the replication_stop() function have canceled the block job, we
> > check it again here.
> >
> > Signed-off-by: Zhang Chen <address@hidden>
> > ---
> > block/replication.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/block/replication.c b/block/replication.c index
> > 3d4dedddfc..bdf2bf4bbc 100644
> > --- a/block/replication.c
> > +++ b/block/replication.c
> > @@ -146,7 +146,9 @@ static void replication_close(BlockDriverState *bs)
> > replication_stop(s->rs, false, NULL);
> > }
> > if (s->stage == BLOCK_REPLICATION_FAILOVER) {
> > - job_cancel_sync(&s->active_disk->bs->job->job);
> > + if (s->secondary_disk->bs->job) {
> > + job_cancel_sync(&s->secondary_disk->bs->job->job);
> > + }
>
> Why are you changing the code from active_disk to secondary_disk?
>
Sorry, It seems that I misunderstood the original code.
I have re-checked the code, looks in the "commit_active_start()" create a job
for the active_disk.
But in my test, when occur failover, running to the " replication_close() " the
active_disk's job always = 0,
It will crash here:
job_cancel_sync(&s->active_disk->bs->job->job);
So, I will add a check here:
if (s->active_disk->bs->job) {
job_cancel_sync(&s->active_disk->bs->job->job);
}
What do you think?
> Also, please rebase on top of Vladimir's '[PATCH 0/4] block: drop
> bs->job'.
Sure.
Thanks
Zhang Chen
>
> Kevin