Re: [PATCH 04/11] job: add .cancel handler for the driver

[Vladimir Sementsov-Ogievskiy]

21.01.2021 02:17, Eric Blake wrote:
> On 11/18/20 12:04 PM, Vladimir Sementsov-Ogievskiy wrote:
> > To be used in mirror in the following commit to cancel in-flight io on
> > target to not waste the time.
> >
> > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> > ---
> >   include/qemu/job.h | 5 +++++
> >   job.c              | 3 +++
> >   2 files changed, 8 insertions(+)
> >
> > diff --git a/include/qemu/job.h b/include/qemu/job.h
> > index 32aabb1c60..efc6fa7544 100644
> > --- a/include/qemu/job.h
> > +++ b/include/qemu/job.h
> > @@ -251,6 +251,11 @@ struct JobDriver {
> >        */
> >       void (*clean)(Job *job);
> >   
> > +    /**
> > +     * If the callback is not NULL, it will be invoked in job_cancel_async
> > +     */
> > +    void (*cancel)(Job *job);
> > +
>
> Does the call need to be re-entrant or even worry about being invoked
> more than once on the same BDS?  Or worded differently,
>
> > +++ b/job.c
> > @@ -712,6 +712,9 @@ static int job_finalize_single(Job *job)
> >   
> >   static void job_cancel_async(Job *job, bool force)
> >   {
> > +    if (job->driver->cancel) {
> > +        job->driver->cancel(job);
> > +    }
> >       if (job->user_paused) {
>
> can job_cancel_async be reached more than once on the same BDS?

what do you mean by same BDS? On same job?

I don't think that job_cancel_async should be called several times during one 
"cancel" operation. But if it is, it's still safe enough with only .cancel 
implementation in [05], which just calls bdrv_cancel_in_flight() (which of course should 
be safe to call several times).

> >           /* Do not call job_enter here, the caller will handle it.  */
> >           if (job->driver->user_resume) {


--
Best regards,
Vladimir