qemu-block
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]