Re: [Qemu-devel] [RFC v3 00/14] blockjobs: add explicit job management

[John Snow]

On 01/26/2018 09:05 PM, John Snow wrote:
> For jobs that complete when a monitor isn't looking, there's no way to
> tell what the job's final return code was. We need to allow jobs to
> remain in the list until queried for reliable management.
> 
> Furthermore, it's not viable to have graph changes when the monitor
> isn't looking either. We need at least another event for that.
> 
> This series is a rough sketch for the WAITING, PENDING and CONCLUDED
> events that accompany an expanded job management scheme that a management
> client can opt-in to.
> 
> V3:
>  - Added WAITING and PENDING events
>  - Added block_job_finalize verb
>  - Added .pending() callback for jobs
>  - Tweaked how .commit/.abort work
> 
> V2:
>  - Added tests!
>  - Changed property name (Jeff, Paolo)
> 
> RFC / Known problems:
> - I need a lot more tests.
> - Jobs need to actually implement their .pending callback for this to be of 
> any
>   actual use.
> - Mirror needs to be refactored to use the commit/abort/pending/clean 
> callbacks
>   to fulfill the promise made by "no graph changes without user authorization"
>   that PENDING is supposed to offer
> - Jobs beyond backup will be able to opt-in to the new management scheme in 
> the
>   next version.
> - V4 will include a forced synchronicity for jobs in a transaction; i.e. all
>   jobs will be forced to use either the old or new styles, but not a mix.
> 
> Please take a look and shout loudly if I'm wandering in the wrong direction.
> 
> ________________________________________________________________________________
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch block-job-reap
> https://github.com/jnsnow/qemu/tree/block-job-reap
> 
> This version is tagged block-job-reap-v3:
> https://github.com/jnsnow/qemu/releases/tag/block-job-reap-v3
> 
> John Snow (14):
>   blockjobs: add manual property
>   blockjobs: Add status enum
>   blockjobs: add state transition table
>   blockjobs: RFC add block_job_verb permission table
>   blockjobs: add block_job_dismiss
>   blockjobs: add CONCLUDED state
>   blockjobs: ensure abort is called for cancelled jobs
>   blockjobs: add commit, abort, clean helpers
>   blockjobs: add prepare callback
>   blockjobs: Add waiting event
>   blockjobs: add PENDING status and event
>   blockjobs: add block-job-finalize
>   blockjobs: Expose manual property
>   iotests: test manual job dismissal
> 
>  block/backup.c               |  22 ++--
>  block/commit.c               |   2 +-
>  block/mirror.c               |   2 +-
>  block/replication.c          |   5 +-
>  block/stream.c               |   2 +-
>  block/trace-events           |   2 +
>  blockdev.c                   |  42 ++++++-
>  blockjob.c                   | 279 
> ++++++++++++++++++++++++++++++++++++++++---
>  include/block/block_int.h    |   9 +-
>  include/block/blockjob.h     |  38 ++++++
>  include/block/blockjob_int.h |  12 +-
>  qapi/block-core.json         | 135 +++++++++++++++++++--
>  tests/qemu-iotests/056       | 241 +++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/056.out   |   4 +-
>  tests/test-bdrv-drain.c      |   4 +-
>  tests/test-blockjob-txn.c    |   2 +-
>  tests/test-blockjob.c        |   4 +-
>  17 files changed, 750 insertions(+), 55 deletions(-)
> 

NACK

There are a lot of changes I've already made to this series based on
Kevin's recommendations; please wait for the next revision.

And a lot of bugs in this series I've already found.

--js