Skip to content
Snippets Groups Projects
  • Stefan Reiter's avatar
    b660a84b
    job: take each job's lock individually in job_txn_apply · b660a84b
    Stefan Reiter authored
    
    All callers of job_txn_apply hold a single job's lock, but different
    jobs within a transaction can have different contexts, thus we need to
    lock each one individually before applying the callback function.
    
    Similar to job_completed_txn_abort this also requires releasing the
    caller's context before and reacquiring it after to avoid recursive
    locks which might break AIO_WAIT_WHILE in the callback. This is safe, since
    existing code would already have to take this into account, lest
    job_completed_txn_abort might have broken.
    
    This also brings to light a different issue: When a callback function in
    job_txn_apply moves it's job to a different AIO context, callers will
    try to release the wrong lock (now that we re-acquire the lock
    correctly, previously it would just continue with the old lock, leaving
    the job unlocked for the rest of the return path). Fix this by not caching
    the job's context.
    
    This is only necessary for qmp_block_job_finalize, qmp_job_finalize and
    job_exit, since everyone else calls through job_exit.
    
    One test needed adapting, since it calls job_finalize directly, so it
    manually needs to acquire the correct context.
    
    Signed-off-by: default avatarStefan Reiter <s.reiter@proxmox.com>
    Message-Id: <20200407115651.69472-2-s.reiter@proxmox.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>
    b660a84b
    History
    job: take each job's lock individually in job_txn_apply
    Stefan Reiter authored
    
    All callers of job_txn_apply hold a single job's lock, but different
    jobs within a transaction can have different contexts, thus we need to
    lock each one individually before applying the callback function.
    
    Similar to job_completed_txn_abort this also requires releasing the
    caller's context before and reacquiring it after to avoid recursive
    locks which might break AIO_WAIT_WHILE in the callback. This is safe, since
    existing code would already have to take this into account, lest
    job_completed_txn_abort might have broken.
    
    This also brings to light a different issue: When a callback function in
    job_txn_apply moves it's job to a different AIO context, callers will
    try to release the wrong lock (now that we re-acquire the lock
    correctly, previously it would just continue with the old lock, leaving
    the job unlocked for the rest of the return path). Fix this by not caching
    the job's context.
    
    This is only necessary for qmp_block_job_finalize, qmp_job_finalize and
    job_exit, since everyone else calls through job_exit.
    
    One test needed adapting, since it calls job_finalize directly, so it
    manually needs to acquire the correct context.
    
    Signed-off-by: default avatarStefan Reiter <s.reiter@proxmox.com>
    Message-Id: <20200407115651.69472-2-s.reiter@proxmox.com>
    Signed-off-by: default avatarKevin Wolf <kwolf@redhat.com>