Re: [Qemu-devel] [PATCH v2 05/10] block: add block job transactions

[Stefan Hajnoczi]

On Tue, Jul 07, 2015 at 03:32:45PM +0800, Fam Zheng wrote:
> On Mon, 07/06 15:24, Stefan Hajnoczi wrote:
> > +/**
> > + * block_job_txn_add_job:
> > + * @txn: The transaction (may be NULL)
> > + * @job: Job to add to the transaction
> > + *
> > + * Add @job to the transaction.  The @job must not already be in a 
> > transaction.
> > + * The block job driver must call block_job_txn_prepare_to_complete() 
> > before
> 
> s/block_job_txn_prepare_to_complete/block_job_txn_job_done/
> 
> Reading this for a second time I start to feel it too complicated for the 
> good.
> 
> I have another idea: in block_job_completed, check if other jobs have failed,
> and call this job driver's (imaginary) "abort()" callback accordingly; if all
> jobs has succeeded, call a "commit" callback during last block_job_completed.
> 
> Does that make sense?

I think you've skipped the hard part: immediate cancellation.  If a job
is cancelled by the user or a job fails, then all other jobs are
cancelled immediately.

Immediate cancellation has the problem that jobs could be running in any
AioContext, so you need to handle concurrency.  That's where the
locking, juggling AioContexts, and interaction between blockjobs comes
in.

If immediate cancellation is removed then this patch becomes simple:
block_job_txn_job_done() becomes a yield and until the jobs_pending
counter reaches 0.

But now the user must handle cancellation and error cases manually by
invoking QMP block-job-cancel on all the other jobs.  In other words,
we're requiring that the user implements their own BlockJobTxn struct
that keeps track of related jobs.  If they don't keep track then they
may not be able to terminate jobs after failure.

This is why QEMU should be the one to complete or fail all block jobs in
the transaction.  It's simpler if we don't do that but the API then
requires more complexity in the user to be used correctly.

Stefan

pgpBCJGio2gGQ.pgp
Description: PGP signature