qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-6.1? 3/6] jobs: Give Job.force_cancel more meaning


From: Vladimir Sementsov-Ogievskiy
Subject: Re: [PATCH for-6.1? 3/6] jobs: Give Job.force_cancel more meaning
Date: Thu, 22 Jul 2021 21:16:32 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.12.0

22.07.2021 15:26, Max Reitz wrote:
We largely have two cancel modes for jobs:

First, there is actual cancelling.  The job is terminated as soon as
possible, without trying to reach a consistent result.

Second, we have mirror in the READY state.  Technically, the job is not
really cancelled, but it just is a different completion mode.  The job
can still run for an indefinite amount of time while it tries to reach a
consistent result.

We want to be able to clearly distinguish which cancel mode a job is in
(when it has been cancelled).  We can use Job.force_cancel for this, but
right now it only reflects cancel requests from the user with
force=true, but clearly, jobs that do not even distinguish between
force=false and force=true are effectively always force-cancelled.

So this patch has Job.force_cancel signify whether the job will
terminate as soon as possible (force_cancel=true) or whether it will
effectively remain running despite being "cancelled"
(force_cancel=false).

To this end, we let jobs that provide JobDriver.cancel() tell the
generic job code whether they will terminate as soon as possible or not,
and for jobs that do not provide that method we assume they will.

Signed-off-by: Max Reitz<mreitz@redhat.com>


In isolation this patch is rather strange: force_cancel is used only by mirror. 
But we keep in generic job layer. And make a handler to set a value to this 
variable. So in generic layer we ask mirror which value it want to set to 
generic variable, which is used only by mirror.. This probably shows that this 
feature of mirror should be mirror only and generic layer shouldn't take care 
of it (see also my answer to next commit).

But at the end of the series the variable is not more used by mirror directly. 
So, technically the commit is not wrong, and it is a preparation for the 
following ones.

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

--
Best regards,
Vladimir



reply via email to

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