On 02/27/2018 02:56 PM, Eric Blake wrote:
> On 02/23/2018 05:51 PM, John Snow wrote:
>> Some jobs upon finalization may need to perform some work that can
>> still fail. If these jobs are part of a transaction, it's important
>> that these callbacks fail the entire transaction.
>>
>> We allow for a new callback in addition to commit/abort/clean that
>> allows us the opportunity to have fairly late-breaking failures
>> in the transactional process.
>>
>> The expected flow is:
>>
>> - All jobs in a transaction converge to the WAITING state
>> (added in a forthcoming commit)
>> - All jobs prepare to call either commit/abort
>> - If any job fails, is canceled, or fails preparation, all jobs
>> call their .abort callback.
>> - All jobs enter the PENDING state, awaiting manual intervention
>> (also added in a forthcoming commit)
>> - block-job-finalize is issued by the user/management layer
>> - All jobs call their commit callbacks.
>>
>> Signed-off-by: John Snow <js...@redhat.com>
>> ---
>> blockjob.c | 34 +++++++++++++++++++++++++++++++---
>> include/block/blockjob_int.h | 10 ++++++++++
>> 2 files changed, 41 insertions(+), 3 deletions(-)
>>
>
>> @@ -467,17 +480,22 @@ static void block_job_cancel_async(BlockJob *job)
>> job->cancelled = true;
>> }
>> -static void block_job_txn_apply(BlockJobTxn *txn, void fn(BlockJob *))
>> +static int block_job_txn_apply(BlockJobTxn *txn, int fn(BlockJob *))
>> {
>> AioContext *ctx;
>> BlockJob *job, *next;
>> + int rc;
>> QLIST_FOREACH_SAFE(job, &txn->jobs, txn_list, next) {
>> ctx = blk_get_aio_context(job->blk);
>> aio_context_acquire(ctx);
>> - fn(job);
>> + rc = fn(job);
>> aio_context_release(ctx);
>> + if (rc) {
>> + break;
>> + }
>
> This short-circuits the application of the function to the rest of the
> group. Is that ever going to be a problem?
>
With what I've written, I don't think so -- but I can't guarantee
someone won't misunderstand the semantics of it and it will become a
problem. It is a potentially dangerous function in that way.
--js
