Am 11/07/2022 um 14:04 schrieb Vladimir Sementsov-Ogievskiy:
> On 7/6/22 23:15, Emanuele Giuseppe Esposito wrote:
>> Just as done with job.h, create _locked() functions in blockjob.h
>>
>> These functions will be later useful when caller has already taken
>> the lock. All blockjob _locked functions call job _locked functions.
>>
>> Note: at this stage, job_{lock/unlock} and job lock guard macros
>> are *nop*.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eespo...@redhat.com>
>
> We not only create _locked() interfaces, but also make some functions
> correct relatively to job_mutex that was not correct pre-patch, for
> example:
>
> block_job_iostatus_reset(): the function doesn't call any job_* APIs,
> but it access Job fields. After patch fields are accessed under mutex
> which is correct, and that's the significant change worth mentioning in
> commit message.
>
> So, more correct is to say, that we make some functions of blockjob API
> correct relatively to job_mutex and Job fields.
>
> With at least this clarified, I'm OK with the patch as is:
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
>
>
> What kept in mind after the patch:
>
> 1. Some functions still need an update, for example
> block_job_error_action, that access Job.user_pause without locking the
> job_mutex. Or, block_job_event_completed that accesses Job.ret..
This comes afterward, I didn't check the patches but the end result
covers what you mention above.
>
> 2. What about BlockJob.nodes field? Shouldn't we protect it by the mutex?
>
As I wrote in the comment in patch 17, seems to be always modified under
GLOBAL_STATE_CODE.
Thank you,
Emanuele
