On 20/12/2021 11:47, Vladimir Sementsov-Ogievskiy wrote:
20.12.2021 13:34, Emanuele Giuseppe Esposito wrote:
On 18/12/2021 12:53, Vladimir Sementsov-Ogievskiy wrote:
04.11.2021 17:53, Emanuele Giuseppe Esposito wrote:
Once job lock is used and aiocontext is removed, mirror has
to perform job operations under the same critical section,
using the helpers prepared in previous commit.
Note: at this stage, job_{lock/unlock} and job lock guard macros
are *nop*.
Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
---
block/mirror.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/block/mirror.c b/block/mirror.c
index 00089e519b..f22fa7da6e 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -653,7 +653,7 @@ static int mirror_exit_common(Job *job)
BlockDriverState *target_bs;
BlockDriverState *mirror_top_bs;
Error *local_err = NULL;
- bool abort = job->ret < 0;
+ bool abort = job_has_failed(job);
int ret = 0;
if (s->prepared) {
@@ -1161,9 +1161,7 @@ static void mirror_complete(Job *job, Error
**errp)
s->should_complete = true;
/* If the job is paused, it will be re-entered when it is
resumed */
- if (!job->paused) {
- job_enter(job);
- }
+ job_enter_not_paused(job);
}
static void coroutine_fn mirror_pause(Job *job)
@@ -1182,7 +1180,7 @@ static bool mirror_drained_poll(BlockJob *job)
* from one of our own drain sections, to avoid a deadlock
waiting for
* ourselves.
*/
- if (!s->common.job.paused && !job_is_cancelled(&job->job) &&
!s->in_drain) {
+ if (job_not_paused_nor_cancelled(&s->common.job) &&
!s->in_drain) {
return true;
}
Why to introduce a separate API function for every use case?
Could we instead just use WITH_JOB_LOCK_GUARD() ?
This implies making the struct job_mutex public. Is that ok for you?
Yes, I think it's OK.
Alternatively, you can use job_lock() / job_unlock(), or even rewrite
WITH_JOB_LOCK_GUARD() macro using job_lock/job_unlock, to keep mutex
private.. But I don't think it really worth it now.
Note that struct Job is already public, so if we'll use per-job mutex in
future it still is not a problem. Only when we decide to make struct Job
private, we'll have to decide something about JOB_LOCK_GUARD(), and at
this point we'll just rewrite it to work through some helper function
instead of directly touching the mutex.
Ok I will do that. Just FYI the initial idea was that drivers like
monitor would not need to know about job_mutex lock, that is why I made
the helpers in mirror.c.
Thank you,
Emanuele
