block/backup.c | 5 +- block/commit.c | 2 +- block/mirror.c | 2 +- block/stream.c | 2 +- block/trace-events | 7 + blockdev.c | 69 ++++++++- blockjob.c | 348 ++++++++++++++++++++++++++++++++++++------ include/block/blockjob.h | 61 +++++++- include/block/blockjob_int.h | 17 ++- qapi/block-core.json | 184 +++++++++++++++++++++- tests/qemu-iotests/030 | 6 +- tests/qemu-iotests/055 | 17 +-- tests/qemu-iotests/056 | 187 +++++++++++++++++++++++ tests/qemu-iotests/056.out | 4 +- tests/qemu-iotests/109.out | 24 +-- tests/qemu-iotests/iotests.py | 12 +- tests/test-bdrv-drain.c | 5 +- tests/test-blockjob-txn.c | 19 +-- tests/test-blockjob.c | 233 +++++++++++++++++++++++++++- 19 files changed, 1076 insertions(+), 128 deletions(-)
This series seeks to address two distinct but closely related issues
concerning the job management API.
(1) For jobs that complete when a monitor is not attached and receiving
events or notifications, there's no way to discern the job's final
return code. Jobs must remain in the query list until dismissed
for reliable management.
(2) Jobs that change the block graph structure at an indeterminate point
after the job starts compete with the management layer that relies
on that graph structure to issue meaningful commands.
This structure should change only at the behest of the management
API, and not asynchronously at unknown points in time. Before a job
issues such changes, it must rely on explicit and synchronous
confirmation from the management API.
These changes are implemented by formalizing a State Transition Machine
for the BlockJob subsystem.
Job States:
UNDEFINED Default state. Internal state only.
CREATED Job has been created
RUNNING Job has been started and is running
PAUSED Job is not ready and has been paused
READY Job is ready and is running
STANDBY Job is ready and is paused
WAITING Job is waiting on peers in transaction
PENDING Job is waiting on ACK from QMP
ABORTING Job is aborting or has been cancelled
CONCLUDED Job has finished and has a retcode available
NULL Job is being dismantled. Internal state only.
Job Verbs:
CANCEL Instructs a running job to terminate with error,
(Except when that job is READY, which produces no error.)
PAUSE Request a job to pause.
RESUME Request a job to resume from a pause.
SET-SPEED Change the speed limiting parameter of a job.
COMPLETE Ask a READY job to finish and exit.
FINALIZE Ask a PENDING job to perform its graph finalization.
DISMISS Finish cleaning up an empty job.
And here's my stab at a diagram:
+---------+
|UNDEFINED|
+--+------+
|
+--v----+
+---------+CREATED+-----------------+
| +--+----+ |
| | |
| +--+----+ +------+ |
+---------+RUNNING<----->PAUSED| |
| +--+-+--+ +------+ |
| | | |
| | +------------------+ |
| | | |
| +--v--+ +-------+ | |
+---------+READY<------->STANDBY| | |
| +--+--+ +-------+ | |
| | | |
| +--v----+ | |
+---------+WAITING+---------------+ |
| +--+----+ |
| | |
| +--v----+ |
+---------+PENDING| |
| +--+----+ |
| | |
+--v-----+ +--v------+ |
|ABORTING+--->CONCLUDED| |
+--------+ +--+------+ |
| |
+--v-+ |
|NULL+--------------------+
+----+
v5:
001/21:[----] [--] 'blockjobs: fix set-speed kick'
002/21:[0001] [FC] 'blockjobs: model single jobs as transactions'
003/21:[down] 'Blockjobs: documentation touchup'
004/21:[0004] [FC] 'blockjobs: add status enum'
005/21:[0004] [FC] 'blockjobs: add state transition table'
006/21:[----] [--] 'iotests: add pause_wait'
007/21:[----] [--] 'blockjobs: add block_job_verb permission table'
008/21:[0004] [FC] 'blockjobs: add ABORTING state'
009/21:[0022] [FC] 'blockjobs: add CONCLUDED state'
010/21:[0025] [FC] 'blockjobs: add NULL state'
011/21:[0025] [FC] 'blockjobs: add block_job_dismiss'
012/21:[0002] [FC] 'blockjobs: ensure abort is called for cancelled jobs'
013/21:[----] [--] 'blockjobs: add commit, abort, clean helpers'
014/21:[0005] [FC] 'blockjobs: add block_job_txn_apply function'
015/21:[0006] [FC] 'blockjobs: add prepare callback'
016/21:[0031] [FC] 'blockjobs: add waiting status'
017/21:[0018] [FC] 'blockjobs: add PENDING status and event'
018/21:[0043] [FC] 'blockjobs: add block-job-finalize'
019/21:[0118] [FC] 'blockjobs: Expose manual property'
020/21:[0036] [FC] 'iotests: test manual job dismissal'
021/21:[down] 'tests/test-blockjob: test cancellations'
Big changes:
- Disallow implicit loopback transitions
- Allow CREATED-->ABORTING transitions; this is modeling how canceling
jobs that aren't started already works.
- Use block_job_decommission to invalidate job objects instead of using
a context-less 'unref'
- Add cancellation unit test.
- May have not added all the R-Bs I should have, and likely added some
I shouldn't have.
02: Removed stale comment
03: New patch touching up comments, replacing the 'manual' property patch.
04: Contextual, removed doc touchup for comments previously added in 03
05: Disallow implicit loopback transitions
Removed initial state assignment in create
08: Allow CREATED-->ABORTING transition.
Removed forward reference to @concluded state
09: Re-added doc reference to @concluded
Replaced block_job_event_concluded with block_job_conclude
Fixed erroneous transition avoidance for internal jobs
STM table change fallout from #08
10: Added assertion that jobs with no refcounts are in status NULL.
Added block_job_decommission for the transition to the NULL state.
block_job_create now uses block_job_early_fail on error pathways
block_job_early_fail now uses block_job_decommission
11: block_job_do_dismiss now just calls block_job_decommission,
see commit message
added job->auto_dismiss property
removed extra reference for dismiss functionality, it was an artifact
of an older implementation and isn't needed
12: Allow ABORTING -> ABORTING transitions (kwolf)
14: Keep the assertion that completed jobs have an RC of 0.
15: Rewrote block_job_prepare to be 35% less stupid (by volume)
Updated commit message, which is now 59% less wrong.
16: Fixed typos and diagram.
Removed waiting event entirely.
STM table change fallout from #08 and #12.
17: Touched up the diagram again.
Added the MANUAL_FINALIZE and auto_finalize flag and property.
18: Changed commit message.
Removed special casing for mixed-mode transactions for finalization step.
block_job_cancel gets a new case for handling the cancellation of
jobs deferred to the main loop.
19: Fallout from splitting property names, almost entirely different now.
20: Changed property names, job now tests 'dismiss' exclusively again.
21: New patch to test cancellation modes.
V4:
- All jobs are now transactions.
- All jobs now transition through states in a uniform way.
- Verb permissions are now enforced.
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:
- Still need more tests.
- STANDBY is still a dumb name. See v4's cover letter.
- 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.
________________________________________________________________________________
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-v5:
https://github.com/jnsnow/qemu/releases/tag/block-job-reap-v5
John Snow (21):
blockjobs: fix set-speed kick
blockjobs: model single jobs as transactions
Blockjobs: documentation touchup
blockjobs: add status enum
blockjobs: add state transition table
iotests: add pause_wait
blockjobs: add block_job_verb permission table
blockjobs: add ABORTING state
blockjobs: add CONCLUDED state
blockjobs: add NULL state
blockjobs: add block_job_dismiss
blockjobs: ensure abort is called for cancelled jobs
blockjobs: add commit, abort, clean helpers
blockjobs: add block_job_txn_apply function
blockjobs: add prepare callback
blockjobs: add waiting status
blockjobs: add PENDING status and event
blockjobs: add block-job-finalize
blockjobs: Expose manual property
iotests: test manual job dismissal
tests/test-blockjob: test cancellations
block/backup.c | 5 +-
block/commit.c | 2 +-
block/mirror.c | 2 +-
block/stream.c | 2 +-
block/trace-events | 7 +
blockdev.c | 69 ++++++++-
blockjob.c | 348 ++++++++++++++++++++++++++++++++++++------
include/block/blockjob.h | 61 +++++++-
include/block/blockjob_int.h | 17 ++-
qapi/block-core.json | 184 +++++++++++++++++++++-
tests/qemu-iotests/030 | 6 +-
tests/qemu-iotests/055 | 17 +--
tests/qemu-iotests/056 | 187 +++++++++++++++++++++++
tests/qemu-iotests/056.out | 4 +-
tests/qemu-iotests/109.out | 24 +--
tests/qemu-iotests/iotests.py | 12 +-
tests/test-bdrv-drain.c | 5 +-
tests/test-blockjob-txn.c | 19 +--
tests/test-blockjob.c | 233 +++++++++++++++++++++++++++-
19 files changed, 1076 insertions(+), 128 deletions(-)
--
2.14.3
Am 10.03.2018 um 09:27 hat John Snow geschrieben: > This series seeks to address two distinct but closely related issues > concerning the job management API. > > (1) For jobs that complete when a monitor is not attached and receiving > events or notifications, there's no way to discern the job's final > return code. Jobs must remain in the query list until dismissed > for reliable management. > > (2) Jobs that change the block graph structure at an indeterminate point > after the job starts compete with the management layer that relies > on that graph structure to issue meaningful commands. > > This structure should change only at the behest of the management > API, and not asynchronously at unknown points in time. Before a job > issues such changes, it must rely on explicit and synchronous > confirmation from the management API. > > These changes are implemented by formalizing a State Transition Machine > for the BlockJob subsystem. Thanks, applied to the block branch. Kevin
John Snow <jsnow@redhat.com> writes: > This series seeks to address two distinct but closely related issues > concerning the job management API. > > (1) For jobs that complete when a monitor is not attached and receiving > events or notifications, there's no way to discern the job's final > return code. Jobs must remain in the query list until dismissed > for reliable management. > > (2) Jobs that change the block graph structure at an indeterminate point > after the job starts compete with the management layer that relies > on that graph structure to issue meaningful commands. > > This structure should change only at the behest of the management > API, and not asynchronously at unknown points in time. Before a job > issues such changes, it must rely on explicit and synchronous > confirmation from the management API. > > These changes are implemented by formalizing a State Transition Machine > for the BlockJob subsystem. > > Job States: > > UNDEFINED Default state. Internal state only. > CREATED Job has been created > RUNNING Job has been started and is running > PAUSED Job is not ready and has been paused > READY Job is ready and is running > STANDBY Job is ready and is paused > > WAITING Job is waiting on peers in transaction > PENDING Job is waiting on ACK from QMP > ABORTING Job is aborting or has been cancelled > CONCLUDED Job has finished and has a retcode available > NULL Job is being dismantled. Internal state only. > > Job Verbs: > > CANCEL Instructs a running job to terminate with error, > (Except when that job is READY, which produces no error.) > PAUSE Request a job to pause. > RESUME Request a job to resume from a pause. > SET-SPEED Change the speed limiting parameter of a job. > COMPLETE Ask a READY job to finish and exit. > > FINALIZE Ask a PENDING job to perform its graph finalization. > DISMISS Finish cleaning up an empty job. For each job verb and job state: what's the new job state? > And here's my stab at a diagram: > > +---------+ > |UNDEFINED| > +--+------+ > | > +--v----+ > +---------+CREATED+-----------------+ > | +--+----+ | > | | | > | +--+----+ +------+ | > +---------+RUNNING<----->PAUSED| | > | +--+-+--+ +------+ | > | | | | > | | +------------------+ | > | | | | > | +--v--+ +-------+ | | > +---------+READY<------->STANDBY| | | > | +--+--+ +-------+ | | > | | | | > | +--v----+ | | > +---------+WAITING+---------------+ | > | +--+----+ | > | | | > | +--v----+ | > +---------+PENDING| | > | +--+----+ | > | | | > +--v-----+ +--v------+ | > |ABORTING+--->CONCLUDED| | > +--------+ +--+------+ | > | | > +--v-+ | > |NULL+--------------------+ > +----+ Is this diagram missing a few arrowheads? E.g. on the edge between RUNNING and WAITING. Might push the limits of ASCII art, but here goes anyway: can we label the arrows with job verbs? Can you briefly explain how this state machine addresses (1) and (2)?
Forgot to mention: yes, I know this has been merged already. I'm merely trying to catch up with recent block layer progress.
On 04/17/2018 09:44 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> This series seeks to address two distinct but closely related issues >> concerning the job management API. >> >> (1) For jobs that complete when a monitor is not attached and receiving >> events or notifications, there's no way to discern the job's final >> return code. Jobs must remain in the query list until dismissed >> for reliable management. >> >> (2) Jobs that change the block graph structure at an indeterminate point >> after the job starts compete with the management layer that relies >> on that graph structure to issue meaningful commands. >> >> This structure should change only at the behest of the management >> API, and not asynchronously at unknown points in time. Before a job >> issues such changes, it must rely on explicit and synchronous >> confirmation from the management API. >> >> These changes are implemented by formalizing a State Transition Machine >> for the BlockJob subsystem. >> >> Job States: >> >> UNDEFINED Default state. Internal state only. >> CREATED Job has been created >> RUNNING Job has been started and is running >> PAUSED Job is not ready and has been paused >> READY Job is ready and is running >> STANDBY Job is ready and is paused >> >> WAITING Job is waiting on peers in transaction >> PENDING Job is waiting on ACK from QMP >> ABORTING Job is aborting or has been cancelled >> CONCLUDED Job has finished and has a retcode available >> NULL Job is being dismantled. Internal state only. >> >> Job Verbs: >> Backporting your quote up here: > For each job verb and job state: what's the new job state? > That's not always 1:1, though I tried to address it in the commit messages. >> CANCEL Instructs a running job to terminate with error, >> (Except when that job is READY, which produces no error.) CANCEL will take a job to either NULL... (this is the early abort pathway, prior to the job being fully realized.) ...or to ABORTING (from CREATED once it has fully realized the job, or from RUNNING, READY, WAITING, or PENDING.) >> PAUSE Request a job to pause. issued to RUNNING or READY, transitions to PAUSED or STANDBY respectively. >> RESUME Request a job to resume from a pause. issued to PAUSED or STANDBY, transitions to RUNNING or READY respectively. >> SET-SPEED Change the speed limiting parameter of a job. No run state change. >> COMPLETE Ask a READY job to finish and exit. >> Issued to a READY job, transitions to WAITING. >> FINALIZE Ask a PENDING job to perform its graph finalization. Issued to a PENDING job, transitions to CONCLUDED. >> DISMISS Finish cleaning up an empty job. > Issued to a CONCLUDED job, transitions to NULL. >> And here's my stab at a diagram: >> >> +---------+ >> |UNDEFINED| >> +--+------+ >> | >> +--v----+ >> +---------+CREATED+-----------------+ >> | +--+----+ | >> | | | >> | +--+----+ +------+ | >> +---------+RUNNING<----->PAUSED| | >> | +--+-+--+ +------+ | >> | | | | >> | | +------------------+ | >> | | | | >> | +--v--+ +-------+ | | >> +---------+READY<------->STANDBY| | | >> | +--+--+ +-------+ | | >> | | | | >> | +--v----+ | | >> +---------+WAITING<---------------+ | >> | +--+----+ | >> | | | >> | +--v----+ | >> +---------+PENDING| | >> | +--+----+ | >> | | | >> +--v-----+ +--v------+ | >> |ABORTING+--->CONCLUDED| | >> +--------+ +--+------+ | >> | | >> +--v-+ | >> |NULL<--------------------+ >> +----+ > > Is this diagram missing a few arrowheads? E.g. on the edge between > RUNNING and WAITING. > Apparently yes. :\ (Secretly fixed up in my reply.) > Might push the limits of ASCII art, but here goes anyway: can we label > the arrows with job verbs? > Can you recommend a tool to help me do that? I've been using asciiflow infinity (http://asciiflow.com) and it's not very good, but I don't have anything better. > Can you briefly explain how this state machine addresses (1) and (2)? > (1) The CONCLUDED state allows jobs to persist in the job query list after they would have disappeared in 2.11-era QEMU. This lets us query for completion codes and to dismiss the job at our own leisure. (2) The PENDING state allows jobs to wait in a nearly-completed state, pending authorization from the QMP client to make graph changes. Otherwise, the job has to asynchronously perform this cleanup and the exact point in time is unknowable to the QMP client. By making a PENDING state and a finalize callback (.prepare), we can make this portion of a job's task synchronous. "John, you added more than two states..." Yup, this was to help simplify the existing state machine, believe it or not. I modeled all jobs as transactions to eliminate different cleanup routing and added two new interim states; - WAITING - ABORTING to help make assertions about the valid transitions jobs can make. The ABORTING state helps make it clear when a job is allowed to fail (and emit QMP events related to such). The WAITING state is simply advisory to help a client know that a job is "finished" but cannot yet receive further instruction because of peers in a transaction. This helps me to add nice QMP errors for any verbs issued to such jobs. "Sorry pal, this job is waiting and can't hear you right now!" This kept the code cleaner than adding a bunch of very fragile boolean error-checking pathways in dozens of helper functions to help avoid illegal instructions on jobs not prepared to receive those instructions. So these two new states don't help accomplish (1) or (2) strictly, but they do facilitate the code additions that _do_ a lot less ugly. --js
John Snow <jsnow@redhat.com> writes: > On 04/17/2018 09:44 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> This series seeks to address two distinct but closely related issues >>> concerning the job management API. >>> >>> (1) For jobs that complete when a monitor is not attached and receiving >>> events or notifications, there's no way to discern the job's final >>> return code. Jobs must remain in the query list until dismissed >>> for reliable management. >>> >>> (2) Jobs that change the block graph structure at an indeterminate point >>> after the job starts compete with the management layer that relies >>> on that graph structure to issue meaningful commands. >>> >>> This structure should change only at the behest of the management >>> API, and not asynchronously at unknown points in time. Before a job >>> issues such changes, it must rely on explicit and synchronous >>> confirmation from the management API. >>> >>> These changes are implemented by formalizing a State Transition Machine >>> for the BlockJob subsystem. >>> >>> Job States: >>> >>> UNDEFINED Default state. Internal state only. >>> CREATED Job has been created >>> RUNNING Job has been started and is running >>> PAUSED Job is not ready and has been paused >>> READY Job is ready and is running >>> STANDBY Job is ready and is paused >>> >>> WAITING Job is waiting on peers in transaction >>> PENDING Job is waiting on ACK from QMP >>> ABORTING Job is aborting or has been cancelled >>> CONCLUDED Job has finished and has a retcode available >>> NULL Job is being dismantled. Internal state only. >>> >>> Job Verbs: >>> > > Backporting your quote up here: > >> For each job verb and job state: what's the new job state? >> > > That's not always 1:1, though I tried to address it in the commit messages. Let me rephrase my question then. For each job verb and job state: what are the possible new job states? If there's more than one, what's the condition for each? I appreciate commit messages explaining that, but having complete state machine documentation in one place (a comment or in docs/) would be nice, wouldn't it? >>> CANCEL Instructs a running job to terminate with error, >>> (Except when that job is READY, which produces no error.) > > CANCEL will take a job to either NULL... (this is the early abort > pathway, prior to the job being fully realized.) > > ...or to ABORTING (from CREATED once it has fully realized the job, or > from RUNNING, READY, WAITING, or PENDING.) > >>> PAUSE Request a job to pause. > > issued to RUNNING or READY, transitions to PAUSED or STANDBY respectively. > >>> RESUME Request a job to resume from a pause. > > issued to PAUSED or STANDBY, transitions to RUNNING or READY respectively. > >>> SET-SPEED Change the speed limiting parameter of a job. > > No run state change. > >>> COMPLETE Ask a READY job to finish and exit. >>> > > Issued to a READY job, transitions to WAITING. > >>> FINALIZE Ask a PENDING job to perform its graph finalization. > > Issued to a PENDING job, transitions to CONCLUDED. > >>> DISMISS Finish cleaning up an empty job. >> > > Issued to a CONCLUDED job, transitions to NULL. > > >>> And here's my stab at a diagram: >>> >>> +---------+ >>> |UNDEFINED| >>> +--+------+ >>> | >>> +--v----+ >>> +---------+CREATED+-----------------+ >>> | +--+----+ | >>> | | | >>> | +--+----+ +------+ | >>> +---------+RUNNING<----->PAUSED| | >>> | +--+-+--+ +------+ | >>> | | | | >>> | | +------------------+ | >>> | | | | >>> | +--v--+ +-------+ | | >>> +---------+READY<------->STANDBY| | | >>> | +--+--+ +-------+ | | >>> | | | | >>> | +--v----+ | | >>> +---------+WAITING<---------------+ | >>> | +--+----+ | >>> | | | >>> | +--v----+ | >>> +---------+PENDING| | >>> | +--+----+ | >>> | | | >>> +--v-----+ +--v------+ | >>> |ABORTING+--->CONCLUDED| | >>> +--------+ +--+------+ | >>> | | >>> +--v-+ | >>> |NULL<--------------------+ >>> +----+ >> >> Is this diagram missing a few arrowheads? E.g. on the edge between >> RUNNING and WAITING. >> > > Apparently yes. :\ > > (Secretly fixed up in my reply.) > >> Might push the limits of ASCII art, but here goes anyway: can we label >> the arrows with job verbs? >> > > Can you recommend a tool to help me do that? I've been using asciiflow > infinity (http://asciiflow.com) and it's not very good, but I don't have > anything better. I do my ASCII art in Emacs picture-mode. >> Can you briefly explain how this state machine addresses (1) and (2)? >> > > (1) The CONCLUDED state allows jobs to persist in the job query list > after they would have disappeared in 2.11-era QEMU. This lets us query > for completion codes and to dismiss the job at our own leisure. Got it. > (2) The PENDING state allows jobs to wait in a nearly-completed state, > pending authorization from the QMP client to make graph changes. > Otherwise, the job has to asynchronously perform this cleanup and the > exact point in time is unknowable to the QMP client. By making a PENDING > state and a finalize callback (.prepare), we can make this portion of a > job's task synchronous. This provides for jobs modifying the graph on job completion. It doesn't provide for jobs modifying the graph while they run. Fine with me; we're not aware of a use for messing with the graph in the middle of a job. > "John, you added more than two states..." > > Yup, this was to help simplify the existing state machine, believe it or > not. I modeled all jobs as transactions to eliminate different cleanup > routing and added two new interim states; > > - WAITING > - ABORTING > > to help make assertions about the valid transitions jobs can make. The > ABORTING state helps make it clear when a job is allowed to fail (and > emit QMP events related to such). > > The WAITING state is simply advisory to help a client know that a job is > "finished" but cannot yet receive further instruction because of peers > in a transaction. This helps me to add nice QMP errors for any verbs > issued to such jobs. "Sorry pal, this job is waiting and can't hear you > right now!" > > This kept the code cleaner than adding a bunch of very fragile boolean > error-checking pathways in dozens of helper functions to help avoid > illegal instructions on jobs not prepared to receive those instructions. > > So these two new states don't help accomplish (1) or (2) strictly, but > they do facilitate the code additions that _do_ a lot less ugly. Thanks! Looks like a fine starting point for in-tree state machine documentation :)
On 04/18/2018 03:25 AM, Markus Armbruster wrote: > John Snow <jsnow@redhat.com> writes: > >> On 04/17/2018 09:44 AM, Markus Armbruster wrote: >>> John Snow <jsnow@redhat.com> writes: >>> >>>> This series seeks to address two distinct but closely related issues >>>> concerning the job management API. >>>> >>>> (1) For jobs that complete when a monitor is not attached and receiving >>>> events or notifications, there's no way to discern the job's final >>>> return code. Jobs must remain in the query list until dismissed >>>> for reliable management. >>>> >>>> (2) Jobs that change the block graph structure at an indeterminate point >>>> after the job starts compete with the management layer that relies >>>> on that graph structure to issue meaningful commands. >>>> >>>> This structure should change only at the behest of the management >>>> API, and not asynchronously at unknown points in time. Before a job >>>> issues such changes, it must rely on explicit and synchronous >>>> confirmation from the management API. >>>> >>>> These changes are implemented by formalizing a State Transition Machine >>>> for the BlockJob subsystem. >>>> >>>> Job States: >>>> >>>> UNDEFINED Default state. Internal state only. >>>> CREATED Job has been created >>>> RUNNING Job has been started and is running >>>> PAUSED Job is not ready and has been paused >>>> READY Job is ready and is running >>>> STANDBY Job is ready and is paused >>>> >>>> WAITING Job is waiting on peers in transaction >>>> PENDING Job is waiting on ACK from QMP >>>> ABORTING Job is aborting or has been cancelled >>>> CONCLUDED Job has finished and has a retcode available >>>> NULL Job is being dismantled. Internal state only. >>>> >>>> Job Verbs: >>>> >> >> Backporting your quote up here: >> >>> For each job verb and job state: what's the new job state? >>> >> >> That's not always 1:1, though I tried to address it in the commit messages. > > Let me rephrase my question then. For each job verb and job state: what > are the possible new job states? If there's more than one, what's the > condition for each? > Is my answer below not sufficient? Maybe you're asking "Can you write this up in a formal document" instead, or did I miss explaining something? > I appreciate commit messages explaining that, but having complete state > machine documentation in one place (a comment or in docs/) would be > nice, wouldn't it? > >>>> CANCEL Instructs a running job to terminate with error, >>>> (Except when that job is READY, which produces no error.) >> >> CANCEL will take a job to either NULL... (this is the early abort >> pathway, prior to the job being fully realized.) >> >> ...or to ABORTING (from CREATED once it has fully realized the job, or >> from RUNNING, READY, WAITING, or PENDING.) >> >>>> PAUSE Request a job to pause. >> >> issued to RUNNING or READY, transitions to PAUSED or STANDBY respectively. >> >>>> RESUME Request a job to resume from a pause. >> >> issued to PAUSED or STANDBY, transitions to RUNNING or READY respectively. >> >>>> SET-SPEED Change the speed limiting parameter of a job. >> >> No run state change. >> >>>> COMPLETE Ask a READY job to finish and exit. >>>> >> >> Issued to a READY job, transitions to WAITING. >> >>>> FINALIZE Ask a PENDING job to perform its graph finalization. >> >> Issued to a PENDING job, transitions to CONCLUDED. >> >>>> DISMISS Finish cleaning up an empty job. >>> >> >> Issued to a CONCLUDED job, transitions to NULL. >> >> >>>> And here's my stab at a diagram: >>>> >>>> +---------+ >>>> |UNDEFINED| >>>> +--+------+ >>>> | >>>> +--v----+ >>>> +---------+CREATED+-----------------+ >>>> | +--+----+ | >>>> | | | >>>> | +--+----+ +------+ | >>>> +---------+RUNNING<----->PAUSED| | >>>> | +--+-+--+ +------+ | >>>> | | | | >>>> | | +------------------+ | >>>> | | | | >>>> | +--v--+ +-------+ | | >>>> +---------+READY<------->STANDBY| | | >>>> | +--+--+ +-------+ | | >>>> | | | | >>>> | +--v----+ | | >>>> +---------+WAITING<---------------+ | >>>> | +--+----+ | >>>> | | | >>>> | +--v----+ | >>>> +---------+PENDING| | >>>> | +--+----+ | >>>> | | | >>>> +--v-----+ +--v------+ | >>>> |ABORTING+--->CONCLUDED| | >>>> +--------+ +--+------+ | >>>> | | >>>> +--v-+ | >>>> |NULL<--------------------+ >>>> +----+ >>> >>> Is this diagram missing a few arrowheads? E.g. on the edge between >>> RUNNING and WAITING. >>> >> >> Apparently yes. :\ >> >> (Secretly fixed up in my reply.) >> >>> Might push the limits of ASCII art, but here goes anyway: can we label >>> the arrows with job verbs? >>> >> >> Can you recommend a tool to help me do that? I've been using asciiflow >> infinity (http://asciiflow.com) and it's not very good, but I don't have >> anything better. > > I do my ASCII art in Emacs picture-mode. > >>> Can you briefly explain how this state machine addresses (1) and (2)? >>> >> >> (1) The CONCLUDED state allows jobs to persist in the job query list >> after they would have disappeared in 2.11-era QEMU. This lets us query >> for completion codes and to dismiss the job at our own leisure. > > Got it. > >> (2) The PENDING state allows jobs to wait in a nearly-completed state, >> pending authorization from the QMP client to make graph changes. >> Otherwise, the job has to asynchronously perform this cleanup and the >> exact point in time is unknowable to the QMP client. By making a PENDING >> state and a finalize callback (.prepare), we can make this portion of a >> job's task synchronous. > > This provides for jobs modifying the graph on job completion. It > doesn't provide for jobs modifying the graph while they run. Fine with > me; we're not aware of a use for messing with the graph in the middle of > a job. > I didn't consider this possibility. The concept could in theory be expanded to arbitrary sync points, but I'm not going to worry about that until the need arises. >> "John, you added more than two states..." >> >> Yup, this was to help simplify the existing state machine, believe it or >> not. I modeled all jobs as transactions to eliminate different cleanup >> routing and added two new interim states; >> >> - WAITING >> - ABORTING >> >> to help make assertions about the valid transitions jobs can make. The >> ABORTING state helps make it clear when a job is allowed to fail (and >> emit QMP events related to such). >> >> The WAITING state is simply advisory to help a client know that a job is >> "finished" but cannot yet receive further instruction because of peers >> in a transaction. This helps me to add nice QMP errors for any verbs >> issued to such jobs. "Sorry pal, this job is waiting and can't hear you >> right now!" >> >> This kept the code cleaner than adding a bunch of very fragile boolean >> error-checking pathways in dozens of helper functions to help avoid >> illegal instructions on jobs not prepared to receive those instructions. >> >> So these two new states don't help accomplish (1) or (2) strictly, but >> they do facilitate the code additions that _do_ a lot less ugly. > I really bungled that sentence. > Thanks! > > Looks like a fine starting point for in-tree state machine documentation > :) >
John Snow <jsnow@redhat.com> writes: > On 04/18/2018 03:25 AM, Markus Armbruster wrote: >> John Snow <jsnow@redhat.com> writes: >> >>> On 04/17/2018 09:44 AM, Markus Armbruster wrote: >>>> John Snow <jsnow@redhat.com> writes: >>>> >>>>> This series seeks to address two distinct but closely related issues >>>>> concerning the job management API. >>>>> >>>>> (1) For jobs that complete when a monitor is not attached and receiving >>>>> events or notifications, there's no way to discern the job's final >>>>> return code. Jobs must remain in the query list until dismissed >>>>> for reliable management. >>>>> >>>>> (2) Jobs that change the block graph structure at an indeterminate point >>>>> after the job starts compete with the management layer that relies >>>>> on that graph structure to issue meaningful commands. >>>>> >>>>> This structure should change only at the behest of the management >>>>> API, and not asynchronously at unknown points in time. Before a job >>>>> issues such changes, it must rely on explicit and synchronous >>>>> confirmation from the management API. >>>>> >>>>> These changes are implemented by formalizing a State Transition Machine >>>>> for the BlockJob subsystem. >>>>> >>>>> Job States: >>>>> >>>>> UNDEFINED Default state. Internal state only. >>>>> CREATED Job has been created >>>>> RUNNING Job has been started and is running >>>>> PAUSED Job is not ready and has been paused >>>>> READY Job is ready and is running >>>>> STANDBY Job is ready and is paused >>>>> >>>>> WAITING Job is waiting on peers in transaction >>>>> PENDING Job is waiting on ACK from QMP >>>>> ABORTING Job is aborting or has been cancelled >>>>> CONCLUDED Job has finished and has a retcode available >>>>> NULL Job is being dismantled. Internal state only. >>>>> >>>>> Job Verbs: >>>>> >>> >>> Backporting your quote up here: >>> >>>> For each job verb and job state: what's the new job state? >>>> >>> >>> That's not always 1:1, though I tried to address it in the commit messages. >> >> Let me rephrase my question then. For each job verb and job state: what >> are the possible new job states? If there's more than one, what's the >> condition for each? >> > > Is my answer below not sufficient? Maybe you're asking "Can you write > this up in a formal document" instead, or did I miss explaining something? Your answer was fine. I blame my one-pass reply writing. Nevertheless: >> I appreciate commit messages explaining that, but having complete state >> machine documentation in one place (a comment or in docs/) would be >> nice, wouldn't it? Pretty-please? >>>>> CANCEL Instructs a running job to terminate with error, >>>>> (Except when that job is READY, which produces no error.) >>> >>> CANCEL will take a job to either NULL... (this is the early abort >>> pathway, prior to the job being fully realized.) >>> >>> ...or to ABORTING (from CREATED once it has fully realized the job, or >>> from RUNNING, READY, WAITING, or PENDING.) >>> >>>>> PAUSE Request a job to pause. >>> >>> issued to RUNNING or READY, transitions to PAUSED or STANDBY respectively. >>> >>>>> RESUME Request a job to resume from a pause. >>> >>> issued to PAUSED or STANDBY, transitions to RUNNING or READY respectively. >>> >>>>> SET-SPEED Change the speed limiting parameter of a job. >>> >>> No run state change. >>> >>>>> COMPLETE Ask a READY job to finish and exit. >>>>> >>> >>> Issued to a READY job, transitions to WAITING. >>> >>>>> FINALIZE Ask a PENDING job to perform its graph finalization. >>> >>> Issued to a PENDING job, transitions to CONCLUDED. >>> >>>>> DISMISS Finish cleaning up an empty job. >>>> >>> >>> Issued to a CONCLUDED job, transitions to NULL. >>> >>> >>>>> And here's my stab at a diagram: >>>>> >>>>> +---------+ >>>>> |UNDEFINED| >>>>> +--+------+ >>>>> | >>>>> +--v----+ >>>>> +---------+CREATED+-----------------+ >>>>> | +--+----+ | >>>>> | | | >>>>> | +--+----+ +------+ | >>>>> +---------+RUNNING<----->PAUSED| | >>>>> | +--+-+--+ +------+ | >>>>> | | | | >>>>> | | +------------------+ | >>>>> | | | | >>>>> | +--v--+ +-------+ | | >>>>> +---------+READY<------->STANDBY| | | >>>>> | +--+--+ +-------+ | | >>>>> | | | | >>>>> | +--v----+ | | >>>>> +---------+WAITING<---------------+ | >>>>> | +--+----+ | >>>>> | | | >>>>> | +--v----+ | >>>>> +---------+PENDING| | >>>>> | +--+----+ | >>>>> | | | >>>>> +--v-----+ +--v------+ | >>>>> |ABORTING+--->CONCLUDED| | >>>>> +--------+ +--+------+ | >>>>> | | >>>>> +--v-+ | >>>>> |NULL<--------------------+ >>>>> +----+ >>>> >>>> Is this diagram missing a few arrowheads? E.g. on the edge between >>>> RUNNING and WAITING. >>>> >>> >>> Apparently yes. :\ >>> >>> (Secretly fixed up in my reply.) >>> >>>> Might push the limits of ASCII art, but here goes anyway: can we label >>>> the arrows with job verbs? >>>> >>> >>> Can you recommend a tool to help me do that? I've been using asciiflow >>> infinity (http://asciiflow.com) and it's not very good, but I don't have >>> anything better. >> >> I do my ASCII art in Emacs picture-mode. >> >>>> Can you briefly explain how this state machine addresses (1) and (2)? >>>> >>> >>> (1) The CONCLUDED state allows jobs to persist in the job query list >>> after they would have disappeared in 2.11-era QEMU. This lets us query >>> for completion codes and to dismiss the job at our own leisure. >> >> Got it. >> >>> (2) The PENDING state allows jobs to wait in a nearly-completed state, >>> pending authorization from the QMP client to make graph changes. >>> Otherwise, the job has to asynchronously perform this cleanup and the >>> exact point in time is unknowable to the QMP client. By making a PENDING >>> state and a finalize callback (.prepare), we can make this portion of a >>> job's task synchronous. >> >> This provides for jobs modifying the graph on job completion. It >> doesn't provide for jobs modifying the graph while they run. Fine with >> me; we're not aware of a use for messing with the graph in the middle of >> a job. >> > > I didn't consider this possibility. The concept could in theory be > expanded to arbitrary sync points, but I'm not going to worry about that > until the need arises. Makes sense. >>> "John, you added more than two states..." >>> >>> Yup, this was to help simplify the existing state machine, believe it or >>> not. I modeled all jobs as transactions to eliminate different cleanup >>> routing and added two new interim states; >>> >>> - WAITING >>> - ABORTING >>> >>> to help make assertions about the valid transitions jobs can make. The >>> ABORTING state helps make it clear when a job is allowed to fail (and >>> emit QMP events related to such). >>> >>> The WAITING state is simply advisory to help a client know that a job is >>> "finished" but cannot yet receive further instruction because of peers >>> in a transaction. This helps me to add nice QMP errors for any verbs >>> issued to such jobs. "Sorry pal, this job is waiting and can't hear you >>> right now!" >>> >>> This kept the code cleaner than adding a bunch of very fragile boolean >>> error-checking pathways in dozens of helper functions to help avoid >>> illegal instructions on jobs not prepared to receive those instructions. >>> >>> So these two new states don't help accomplish (1) or (2) strictly, but >>> they do facilitate the code additions that _do_ a lot less ugly. >> > > I really bungled that sentence. No problem, I got it anyway :) >> Thanks! >> >> Looks like a fine starting point for in-tree state machine documentation >> :)
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20180310082746.24198-1-jsnow@redhat.com
Subject: [Qemu-devel] [PATCH v5 00/21] blockjobs: add explicit job management
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/1520692378-1835-1-git-send-email-lidongchen@tencent.com -> patchew/1520692378-1835-1-git-send-email-lidongchen@tencent.com
* [new tag] patchew/1520716927-17068-1-git-send-email-zhangckid@gmail.com -> patchew/1520716927-17068-1-git-send-email-zhangckid@gmail.com
* [new tag] patchew/1520723090-22130-1-git-send-email-linux@roeck-us.net -> patchew/1520723090-22130-1-git-send-email-linux@roeck-us.net
* [new tag] patchew/1520754314-5969-1-git-send-email-zhangckid@gmail.com -> patchew/1520754314-5969-1-git-send-email-zhangckid@gmail.com
* [new tag] patchew/1520839658-20499-1-git-send-email-thuth@redhat.com -> patchew/1520839658-20499-1-git-send-email-thuth@redhat.com
* [new tag] patchew/152084575964.24079.3708480492746701627.stgit@bahia.lan -> patchew/152084575964.24079.3708480492746701627.stgit@bahia.lan
* [new tag] patchew/1520849818-6915-1-git-send-email-anton.nefedov@virtuozzo.com -> patchew/1520849818-6915-1-git-send-email-anton.nefedov@virtuozzo.com
* [new tag] patchew/1520850690-23245-1-git-send-email-abdallah.bouassida@lauterbach.com -> patchew/1520850690-23245-1-git-send-email-abdallah.bouassida@lauterbach.com
t [tag update] patchew/20180309165212.97144-1-vsementsov@virtuozzo.com -> patchew/20180309165212.97144-1-vsementsov@virtuozzo.com
* [new tag] patchew/20180309172713.26318-1-kwolf@redhat.com -> patchew/20180309172713.26318-1-kwolf@redhat.com
t [tag update] patchew/20180309175453.41548-1-dgilbert@redhat.com -> patchew/20180309175453.41548-1-dgilbert@redhat.com
t [tag update] patchew/20180309182202.31206-1-farosas@linux.vnet.ibm.com -> patchew/20180309182202.31206-1-farosas@linux.vnet.ibm.com
* [new tag] patchew/20180310082746.24198-1-jsnow@redhat.com -> patchew/20180310082746.24198-1-jsnow@redhat.com
* [new tag] patchew/20180310214554.157155-1-eblake@redhat.com -> patchew/20180310214554.157155-1-eblake@redhat.com
* [new tag] patchew/20180311201239.25506-1-nia.alarie@gmail.com -> patchew/20180311201239.25506-1-nia.alarie@gmail.com
* [new tag] patchew/20180312094308.21716-1-pbonzini@redhat.com -> patchew/20180312094308.21716-1-pbonzini@redhat.com
* [new tag] patchew/20180312104241.24965-1-kraxel@redhat.com -> patchew/20180312104241.24965-1-kraxel@redhat.com
* [new tag] patchew/20180312105941.15439-1-kraxel@redhat.com -> patchew/20180312105941.15439-1-kraxel@redhat.com
* [new tag] patchew/20180312110532.30967-1-kraxel@redhat.com -> patchew/20180312110532.30967-1-kraxel@redhat.com
Switched to a new branch 'test'
68abd0377d tests/test-blockjob: test cancellations
de58cfc7f7 iotests: test manual job dismissal
f39d922c64 blockjobs: Expose manual property
b488c52fc3 blockjobs: add block-job-finalize
eb5fcb71bb blockjobs: add PENDING status and event
0069db2cf3 blockjobs: add waiting status
9fed33f1b8 blockjobs: add prepare callback
4b3e0a2c89 blockjobs: add block_job_txn_apply function
f1c8da8844 blockjobs: add commit, abort, clean helpers
bde9d79765 blockjobs: ensure abort is called for cancelled jobs
7fb719574c blockjobs: add block_job_dismiss
960357f88d blockjobs: add NULL state
d55cd56bc8 blockjobs: add CONCLUDED state
5c9762922f blockjobs: add ABORTING state
fa1056efe6 blockjobs: add block_job_verb permission table
10f12bcc22 iotests: add pause_wait
0c88b0db61 blockjobs: add state transition table
53301418d9 blockjobs: add status enum
fca226d934 Blockjobs: documentation touchup
d8a93c4755 blockjobs: model single jobs as transactions
158eee13ab blockjobs: fix set-speed kick
=== OUTPUT BEGIN ===
Checking PATCH 1/21: blockjobs: fix set-speed kick...
Checking PATCH 2/21: blockjobs: model single jobs as transactions...
Checking PATCH 3/21: Blockjobs: documentation touchup...
Checking PATCH 4/21: blockjobs: add status enum...
Checking PATCH 5/21: blockjobs: add state transition table...
ERROR: space prohibited before open square bracket '['
#81: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#82: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#83: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#84: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#85: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#86: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0},
total: 6 errors, 0 warnings, 88 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 6/21: iotests: add pause_wait...
Checking PATCH 7/21: blockjobs: add block_job_verb permission table...
Checking PATCH 8/21: blockjobs: add ABORTING state...
ERROR: space prohibited before open square bracket '['
#64: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#65: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#66: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1},
ERROR: space prohibited before open square bracket '['
#67: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#68: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1},
ERROR: space prohibited before open square bracket '['
#69: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#70: FILE: blockjob.c:54:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0},
total: 7 errors, 0 warnings, 62 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 9/21: blockjobs: add CONCLUDED state...
ERROR: space prohibited before open square bracket '['
#63: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#64: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 1, 0},
ERROR: space prohibited before open square bracket '['
#65: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 1},
ERROR: space prohibited before open square bracket '['
#66: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#67: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 1},
ERROR: space prohibited before open square bracket '['
#68: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#69: FILE: blockjob.c:54:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#70: FILE: blockjob.c:55:
+ /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0},
total: 8 errors, 0 warnings, 85 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 10/21: blockjobs: add NULL state...
ERROR: space prohibited before open square bracket '['
#80: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#81: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 1, 0, 1},
ERROR: space prohibited before open square bracket '['
#82: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#83: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#84: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#85: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#86: FILE: blockjob.c:54:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 1, 0},
ERROR: space prohibited before open square bracket '['
#87: FILE: blockjob.c:55:
+ /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#88: FILE: blockjob.c:56:
+ /* N: */ [BLOCK_JOB_STATUS_NULL] = {0, 0, 0, 0, 0, 0, 0, 0, 0},
total: 9 errors, 0 warnings, 104 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 11/21: blockjobs: add block_job_dismiss...
Checking PATCH 12/21: blockjobs: ensure abort is called for cancelled jobs...
ERROR: space prohibited before open square bracket '['
#75: FILE: blockjob.c:54:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 1, 1, 0},
total: 1 errors, 0 warnings, 58 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 13/21: blockjobs: add commit, abort, clean helpers...
Checking PATCH 14/21: blockjobs: add block_job_txn_apply function...
Checking PATCH 15/21: blockjobs: add prepare callback...
Checking PATCH 16/21: blockjobs: add waiting status...
ERROR: space prohibited before open square bracket '['
#80: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#81: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 0, 1, 0, 1},
ERROR: space prohibited before open square bracket '['
#82: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#83: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#84: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#85: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#86: FILE: blockjob.c:54:
+ /* W: */ [BLOCK_JOB_STATUS_WAITING] = {0, 0, 0, 0, 0, 0, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#87: FILE: blockjob.c:55:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#88: FILE: blockjob.c:56:
+ /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#89: FILE: blockjob.c:57:
+ /* N: */ [BLOCK_JOB_STATUS_NULL] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
total: 10 errors, 0 warnings, 70 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 17/21: blockjobs: add PENDING status and event...
ERROR: space prohibited before open square bracket '['
#84: FILE: blockjob.c:48:
+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#85: FILE: blockjob.c:49:
+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 1},
ERROR: space prohibited before open square bracket '['
#86: FILE: blockjob.c:50:
+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 0, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#87: FILE: blockjob.c:51:
+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#88: FILE: blockjob.c:52:
+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#89: FILE: blockjob.c:53:
+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0},
ERROR: space prohibited before open square bracket '['
#90: FILE: blockjob.c:54:
+ /* W: */ [BLOCK_JOB_STATUS_WAITING] = {0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0},
ERROR: space prohibited before open square bracket '['
#91: FILE: blockjob.c:55:
+ /* D: */ [BLOCK_JOB_STATUS_PENDING] = {0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#92: FILE: blockjob.c:56:
+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0},
ERROR: space prohibited before open square bracket '['
#93: FILE: blockjob.c:57:
+ /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1},
ERROR: space prohibited before open square bracket '['
#94: FILE: blockjob.c:58:
+ /* N: */ [BLOCK_JOB_STATUS_NULL] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},
total: 11 errors, 0 warnings, 185 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 18/21: blockjobs: add block-job-finalize...
Checking PATCH 19/21: blockjobs: Expose manual property...
Checking PATCH 20/21: iotests: test manual job dismissal...
Checking PATCH 21/21: tests/test-blockjob: test cancellations...
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org
© 2016 - 2024 Red Hat, Inc.