The following changes since commit 22ef7ba8e8ce7fef297549b3defcac333742b804:

Merge remote-tracking branch 'remotes/famz/tags/staging-pull-request' into staging (2018-03-13 11:42:45 +0000)

are available in the git repository at:

for you to fetch changes up to be6c885842efded81a20f4ca24f0d4e123a80c00:

block/mirror: change the semantic of 'force' of block-job-cancel (2018-03-13 16:54:47 +0100)

Fam Zheng (2):

block: Fix flags in reopen queue

iotests: Add regression test for commit base locking

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

Kevin Wolf (14):

luks: Separate image file creation from formatting

luks: Create block_crypto_co_create_generic()

luks: Support .bdrv_co_create

luks: Turn invalid assertion into check

luks: Catch integer overflow for huge sizes

qemu-iotests: Test luks QMP image creation

parallels: Support .bdrv_co_create

qemu-iotests: Enable write tests for parallels

qcow: Support .bdrv_co_create

qed: Support .bdrv_co_create

vdi: Make comments consistent with other drivers

vhdx: Support .bdrv_co_create

vpc: Support .bdrv_co_create

vpc: Require aligned size in .bdrv_co_create

Liang Li (1):

block/mirror: change the semantic of 'force' of block-job-cancel

Max Reitz (3):

vdi: Pull option parsing from vdi_co_create

vdi: Move file creation to vdi_co_create_opts

vdi: Implement .bdrv_co_create

qapi/block-core.json | 363 ++++++++++++++++++++++++++++++++++++++++--

include/block/blockjob.h | 71 ++++++++-

include/block/blockjob_int.h | 17 +-

block.c | 8 +

block/backup.c | 5 +-

block/commit.c | 2 +-

block/crypto.c | 150 ++++++++++++-----

block/mirror.c | 12 +-

block/parallels.c | 199 +++++++++++++++++------

block/qcow.c | 196 +++++++++++++++--------

block/qed.c | 204 ++++++++++++++++--------

block/stream.c | 2 +-

block/vdi.c | 147 +++++++++++++----

block/vhdx.c | 216 +++++++++++++++++++------

block/vpc.c | 241 +++++++++++++++++++++-------

blockdev.c | 71 +++++++--

blockjob.c | 358 +++++++++++++++++++++++++++++++++++------

tests/test-bdrv-drain.c | 5 +-

tests/test-blockjob-txn.c | 27 ++--

tests/test-blockjob.c | 233 ++++++++++++++++++++++++++-

block/trace-events | 7 +

hmp-commands.hx | 3 +-

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/153 | 12 ++

tests/qemu-iotests/153.out | 5 +

tests/qemu-iotests/181 | 2 +-

tests/qemu-iotests/209 | 210 ++++++++++++++++++++++++

tests/qemu-iotests/209.out | 136 ++++++++++++++++

tests/qemu-iotests/check | 1 -

tests/qemu-iotests/common.rc | 2 +-

tests/qemu-iotests/group | 1 +

tests/qemu-iotests/iotests.py | 12 +-

36 files changed, 2642 insertions(+), 514 deletions(-)

create mode 100755 tests/qemu-iotests/209

create mode 100644 tests/qemu-iotests/209.out

From: John Snow <jsnow@redhat.com>

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 PENDING state,

added in a forthcoming commit.

- Upon being finalized, either automatically or explicitly

by the user, jobs prepare to complete.

- If any job fails preparation, all jobs call .abort.

- Otherwise, they succeed and call .commit.

Signed-off-by: John Snow <jsnow@redhat.com>

include/block/blockjob_int.h | 10 ++++++++++

blockjob.c | 30 +++++++++++++++++++++++++++---

2 files changed, 37 insertions(+), 3 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h

--- a/include/block/blockjob_int.h

+++ b/include/block/blockjob_int.h

@@ -XXX,XX +XXX,XX @@ struct BlockJobDriver {

void (*complete)(BlockJob *job, Error **errp);

/**

+ * If the callback is not NULL, prepare will be invoked when all the jobs

+ * belonging to the same transaction complete; or upon this job's completion

+ * if it is not in a transaction.

+ *

+ * This callback will not be invoked if the job has already failed.

+ * If it fails, abort and then clean will be called.

+ */

+ int (*prepare)(BlockJob *job);

+

+ /**

* If the callback is not NULL, it will be invoked when all the jobs

* belonging to the same transaction complete; or upon this job's

* completion if it is not in a transaction. Skipped if NULL.

diff --git a/blockjob.c b/blockjob.c

--- a/blockjob.c

+++ b/blockjob.c

@@ -XXX,XX +XXX,XX @@ static void block_job_update_rc(BlockJob *job)

+static int block_job_prepare(BlockJob *job)

+{

+ if (job->ret == 0 && job->driver->prepare) {

+ job->ret = job->driver->prepare(job);

+ }

+ return job->ret;

+}

+

static void block_job_commit(BlockJob *job)

assert(!job->ret);

@@ -XXX,XX +XXX,XX @@ static void block_job_clean(BlockJob *job)

}

-static void block_job_completed_single(BlockJob *job)

+static int block_job_completed_single(BlockJob *job)

assert(job->completed);

@@ -XXX,XX +XXX,XX @@ static void block_job_completed_single(BlockJob *job)

QLIST_REMOVE(job, txn_list);

block_job_txn_unref(job->txn);

block_job_conclude(job);

+ return 0;

}

static void block_job_cancel_async(BlockJob *job)

@@ -XXX,XX +XXX,XX @@ 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;

+ }

}

+ return rc;

}

static int block_job_finish_sync(BlockJob *job,

@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)

{

BlockJobTxn *txn = job->txn;

BlockJob *other_job;

+ int rc = 0;

+

/*

* Successful completion, see if there are other running jobs in this

* txn.

@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_success(BlockJob *job)

}

assert(other_job->ret == 0);

}

+

+ /* Jobs may require some prep-work to complete without failure */

+ rc = block_job_txn_apply(txn, block_job_prepare);

+ if (rc) {

+ block_job_completed_txn_abort(job);

+ return;

+ }

+

/* We are the last completed job, commit the transaction. */

block_job_txn_apply(txn, block_job_completed_single);

2.13.6

From: John Snow <jsnow@redhat.com>

Add a new state that specifically demarcates when we begin to permanently

demolish a job after it has performed all work. This makes the transition

explicit in the STM table and highlights conditions under which a job may

be demolished.

Alongside this state, add a new helper command "block_job_decommission",

which transitions to the NULL state and puts down our implicit reference.

This separates instances in the code for "block_job_unref" which merely

undo a matching "block_job_ref" with instances intended to initiate the

full destruction of the object.

This decommission action also sets a number of fields to make sure that

block internals or external users that are holding a reference to a job

to see when it "finishes" are convinced that the job object is "done."

This is necessary, for instance, to do a block_job_cancel_sync on a

created object which will not make any progress.

Now, all jobs must go through block_job_decommission prior to being

freed, giving us start-to-finish state machine coverage for jobs.

Transitions:

Created -> Null: Early failure event before the job is started

Concluded -> Null: Standard transition.

Verbs:

None. This should not ever be visible to the monitor.

+---------+

|UNDEFINED|

+--+------+

|

+--v----+

+---------+CREATED+------------------+

| +--+----+ |

| | |

| +--v----+ +------+ |

+---------+RUNNING<----->PAUSED| |

| +--+-+--+ +------+ |

| | | |

| | +------------------+ |

| | | |

| +--v--+ +-------+ | |

+---------+READY<------->STANDBY| | |

| +--+--+ +-------+ | |

| | | |

+--v-----+ +--v------+ | |

|ABORTING+--->CONCLUDED<-------------+ |

+--------+ +--+------+ |

| |

+--v-+ |

|NULL<---------------------+

+----+

Signed-off-by: John Snow <jsnow@redhat.com>

qapi/block-core.json | 5 ++++-

blockjob.c | 50 ++++++++++++++++++++++++++++++++------------------

2 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json

--- a/qapi/block-core.json

+++ b/qapi/block-core.json

# @concluded: The job has finished all work. If manual was set to true, the job

# will remain in the query list until it is dismissed.

#

+# @null: The job is in the process of being dismantled. This state should not

+# ever be visible externally.

+#

# Since: 2.12

##

{ 'enum': 'BlockJobStatus',

'data': ['undefined', 'created', 'running', 'paused', 'ready', 'standby',

- 'aborting', 'concluded' ] }

+ 'aborting', 'concluded', 'null' ] }

##

# @BlockJobInfo:

diff --git a/blockjob.c b/blockjob.c

index XXXXXXX..XXXXXXX 100644

--- a/blockjob.c

+++ b/blockjob.c

@@ -XXX,XX +XXX,XX @@ static QemuMutex block_job_mutex;

/* BlockJob State Transition Table */

bool BlockJobSTT[BLOCK_JOB_STATUS__MAX][BLOCK_JOB_STATUS__MAX] = {

- /* U, C, R, P, Y, S, X, E */

- /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0},

- /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 1, 0},

- /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 1},

- /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0},

- /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 1},

- /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0},

- /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 1},

- /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0},

+ /* U, C, R, P, Y, S, X, E, N */

+ /* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0},

+ /* C: */ [BLOCK_JOB_STATUS_CREATED] = {0, 0, 1, 0, 0, 0, 1, 0, 1},

+ /* R: */ [BLOCK_JOB_STATUS_RUNNING] = {0, 0, 0, 1, 1, 0, 1, 1, 0},

+ /* P: */ [BLOCK_JOB_STATUS_PAUSED] = {0, 0, 1, 0, 0, 0, 0, 0, 0},

+ /* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 1, 0},

+ /* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0, 0, 0, 0},

+ /* X: */ [BLOCK_JOB_STATUS_ABORTING] = {0, 0, 0, 0, 0, 0, 0, 1, 0},

+ /* E: */ [BLOCK_JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0, 1},

+ /* N: */ [BLOCK_JOB_STATUS_NULL] = {0, 0, 0, 0, 0, 0, 0, 0, 0},

bool BlockJobVerbTable[BLOCK_JOB_VERB__MAX][BLOCK_JOB_STATUS__MAX] = {

- /* U, C, R, P, Y, S, X, E */

- [BLOCK_JOB_VERB_CANCEL] = {0, 1, 1, 1, 1, 1, 0, 0},

- [BLOCK_JOB_VERB_PAUSE] = {0, 1, 1, 1, 1, 1, 0, 0},

- [BLOCK_JOB_VERB_RESUME] = {0, 1, 1, 1, 1, 1, 0, 0},

- [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1, 1, 0, 0},

- [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 0, 0, 0},

+ /* U, C, R, P, Y, S, X, E, N */

+ [BLOCK_JOB_VERB_CANCEL] = {0, 1, 1, 1, 1, 1, 0, 0, 0},

+ [BLOCK_JOB_VERB_PAUSE] = {0, 1, 1, 1, 1, 1, 0, 0, 0},

+ [BLOCK_JOB_VERB_RESUME] = {0, 1, 1, 1, 1, 1, 0, 0, 0},

+ [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1, 1, 0, 0, 0},

+ [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 0, 0, 0, 0},

};

static void block_job_state_transition(BlockJob *job, BlockJobStatus s1)

@@ -XXX,XX +XXX,XX @@ static void block_job_detach_aio_context(void *opaque);

void block_job_unref(BlockJob *job)

if (--job->refcnt == 0) {

+ assert(job->status == BLOCK_JOB_STATUS_NULL);

BlockDriverState *bs = blk_bs(job->blk);

QLIST_REMOVE(job, job_list);

bs->job = NULL;

@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job)

bdrv_coroutine_enter(blk_bs(job->blk), job->co);

+static void block_job_decommission(BlockJob *job)

+{

+ assert(job);

+ job->completed = true;

+ job->busy = false;

+ job->paused = false;

+ job->deferred_to_main_loop = true;

+ block_job_state_transition(job, BLOCK_JOB_STATUS_NULL);

+ block_job_unref(job);

+}

+

static void block_job_conclude(BlockJob *job)

block_job_state_transition(job, BLOCK_JOB_STATUS_CONCLUDED);

@@ -XXX,XX +XXX,XX @@ static void block_job_completed_single(BlockJob *job)

QLIST_REMOVE(job, txn_list);

block_job_txn_unref(job->txn);

block_job_conclude(job);

- block_job_unref(job);

+ block_job_decommission(job);

2.13.6

From: John Snow <jsnow@redhat.com>

Which commands ("verbs") are appropriate for jobs in which state is

also somewhat burdensome to keep track of.

As of this commit, it looks rather useless, but begins to look more

interesting the more states we add to the STM table.

A recurring theme is that no verb will apply to an 'undefined' job.

Further, it's not presently possible to restrict the "pause" or "resume"

verbs any more than they are in this commit because of the asynchronous

nature of how jobs enter the PAUSED state; justifications for some

seemingly erroneous applications are given below.

=====

Verbs

=====

Cancel: Any state except undefined.

Pause: Any state except undefined;

'created': Requests that the job pauses as it starts.

'running': Normal usage. (PAUSED)

'paused': The job may be paused for internal reasons,

but the user may wish to force an indefinite

user-pause, so this is allowed.

'ready': Normal usage. (STANDBY)

'standby': Same logic as above.

Resume: Any state except undefined;

'created': Will lift a user's pause-on-start request.

'running': Will lift a pause request before it takes effect.

'paused': Normal usage.

'ready': Will lift a pause request before it takes effect.

'standby': Normal usage.

Set-speed: Any state except undefined, though ready may not be meaningful.

Complete: Only a 'ready' job may accept a complete request.

=======

Changes

=======

(1)

To facilitate "nice" error checking, all five major block-job verb

interfaces in blockjob.c now support an errp parameter:

- block_job_user_cancel is added as a new interface.

- block_job_user_pause gains an errp paramter

- block_job_user_resume gains an errp parameter

- block_job_set_speed already had an errp parameter.

- block_job_complete already had an errp parameter.

(2)

block-job-pause and block-job-resume will no longer no-op when trying

to pause an already paused job, or trying to resume a job that isn't

paused. These functions will now report that they did not perform the

action requested because it was not possible.

iotests have been adjusted to address this new behavior.

(3)

block-job-complete doesn't worry about checking !block_job_started,

because the permission table guards against this.

(4)

test-bdrv-drain's job implementation needs to announce that it is

'ready' now, in order to be completed.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

qapi/block-core.json | 20 ++++++++++++++

include/block/blockjob.h | 13 +++++++--

blockdev.c | 10 +++----

blockjob.c | 71 ++++++++++++++++++++++++++++++++++++++++++------

tests/test-bdrv-drain.c | 1 +

block/trace-events | 1 +

6 files changed, 100 insertions(+), 16 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json

--- a/qapi/block-core.json

+++ b/qapi/block-core.json

@@ -XXX,XX +XXX,XX @@

'data': ['commit', 'stream', 'mirror', 'backup'] }

##

+# @BlockJobVerb:

+#

+# Represents command verbs that can be applied to a blockjob.

+#

+# @cancel: see @block-job-cancel

+#

+# @pause: see @block-job-pause

+#

+# @resume: see @block-job-resume

+#

+# @set-speed: see @block-job-set-speed

+#

+# @complete: see @block-job-complete

+#

+# Since: 2.12

+##

+{ 'enum': 'BlockJobVerb',

+ 'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete' ] }

+

+##

# @BlockJobStatus:

#

# Indicates the present state of a given blockjob in its lifetime.

diff --git a/include/block/blockjob.h b/include/block/blockjob.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/blockjob.h

+++ b/include/block/blockjob.h

@@ -XXX,XX +XXX,XX @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp);

* Asynchronously pause the specified job.

* Do not allow a resume until a matching call to block_job_user_resume.

*/

-void block_job_user_pause(BlockJob *job);

+void block_job_user_pause(BlockJob *job, Error **errp);

/**

* block_job_paused:

@@ -XXX,XX +XXX,XX @@ bool block_job_user_paused(BlockJob *job);

* Resume the specified job.

* Must be paired with a preceding block_job_user_pause.

*/

-void block_job_user_resume(BlockJob *job);

+void block_job_user_resume(BlockJob *job, Error **errp);

+

+/**

+ * block_job_user_cancel:

+ * @job: The job to be cancelled.

+ *

+ * Cancels the specified job, but may refuse to do so if the

+ * operation isn't currently meaningful.

+ */

+void block_job_user_cancel(BlockJob *job, Error **errp);

/**

* block_job_cancel_sync:

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ void qmp_block_job_cancel(const char *device,

}

trace_qmp_block_job_cancel(job);

- block_job_cancel(job);

+ block_job_user_cancel(job, errp);

out:

aio_context_release(aio_context);

}

@@ -XXX,XX +XXX,XX @@ void qmp_block_job_pause(const char *device, Error **errp)

AioContext *aio_context;

BlockJob *job = find_block_job(device, &aio_context, errp);

- if (!job || block_job_user_paused(job)) {

+ if (!job) {

return;

}

trace_qmp_block_job_pause(job);

- block_job_user_pause(job);

+ block_job_user_pause(job, errp);

aio_context_release(aio_context);

}

@@ -XXX,XX +XXX,XX @@ void qmp_block_job_resume(const char *device, Error **errp)

AioContext *aio_context;

BlockJob *job = find_block_job(device, &aio_context, errp);

- if (!job || !block_job_user_paused(job)) {

+ if (!job) {

return;

}

trace_qmp_block_job_resume(job);

- block_job_user_resume(job);

+ block_job_user_resume(job, errp);

aio_context_release(aio_context);

}

diff --git a/blockjob.c b/blockjob.c

index XXXXXXX..XXXXXXX 100644

--- a/blockjob.c

+++ b/blockjob.c

@@ -XXX,XX +XXX,XX @@ bool BlockJobSTT[BLOCK_JOB_STATUS__MAX][BLOCK_JOB_STATUS__MAX] = {

/* S: */ [BLOCK_JOB_STATUS_STANDBY] = {0, 0, 0, 0, 1, 0},

};

+bool BlockJobVerbTable[BLOCK_JOB_VERB__MAX][BLOCK_JOB_STATUS__MAX] = {

+ /* U, C, R, P, Y, S */

+ [BLOCK_JOB_VERB_CANCEL] = {0, 1, 1, 1, 1, 1},

+ [BLOCK_JOB_VERB_PAUSE] = {0, 1, 1, 1, 1, 1},

+ [BLOCK_JOB_VERB_RESUME] = {0, 1, 1, 1, 1, 1},

+ [BLOCK_JOB_VERB_SET_SPEED] = {0, 1, 1, 1, 1, 1},

+ [BLOCK_JOB_VERB_COMPLETE] = {0, 0, 0, 0, 1, 0},

+};

+

static void block_job_state_transition(BlockJob *job, BlockJobStatus s1)

{

BlockJobStatus s0 = job->status;

@@ -XXX,XX +XXX,XX @@ static void block_job_state_transition(BlockJob *job, BlockJobStatus s1)

job->status = s1;

}

+static int block_job_apply_verb(BlockJob *job, BlockJobVerb bv, Error **errp)

+{

+ assert(bv >= 0 && bv <= BLOCK_JOB_VERB__MAX);

+ trace_block_job_apply_verb(job, qapi_enum_lookup(&BlockJobStatus_lookup,

+ job->status),

+ qapi_enum_lookup(&BlockJobVerb_lookup, bv),

+ BlockJobVerbTable[bv][job->status] ?

+ "allowed" : "prohibited");

+ if (BlockJobVerbTable[bv][job->status]) {

+ return 0;

+ }

+ error_setg(errp, "Job '%s' in state '%s' cannot accept command verb '%s'",

+ job->id, qapi_enum_lookup(&BlockJobStatus_lookup, job->status),

+ qapi_enum_lookup(&BlockJobVerb_lookup, bv));

+ return -EPERM;

+}

+

static void block_job_lock(void)

{

qemu_mutex_lock(&block_job_mutex);

@@ -XXX,XX +XXX,XX @@ void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp)

error_setg(errp, QERR_UNSUPPORTED);

return;

}

+ if (block_job_apply_verb(job, BLOCK_JOB_VERB_SET_SPEED, errp)) {

+ return;

+ }

job->driver->set_speed(job, speed, &local_err);

if (local_err) {

error_propagate(errp, local_err);

@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp)

{

/* Should not be reachable via external interface for internal jobs */

assert(job->id);

- if (job->pause_count || job->cancelled ||

- !block_job_started(job) || !job->driver->complete) {

+ if (block_job_apply_verb(job, BLOCK_JOB_VERB_COMPLETE, errp)) {

+ return;

+ }

+ if (job->pause_count || job->cancelled || !job->driver->complete) {

error_setg(errp, "The active block job '%s' cannot be completed",

job->id);

return;

@@ -XXX,XX +XXX,XX @@ void block_job_complete(BlockJob *job, Error **errp)

job->driver->complete(job, errp);

}

-void block_job_user_pause(BlockJob *job)

+void block_job_user_pause(BlockJob *job, Error **errp)

{

+ if (block_job_apply_verb(job, BLOCK_JOB_VERB_PAUSE, errp)) {

+ return;

+ }

+ if (job->user_paused) {

+ error_setg(errp, "Job is already paused");

+ return;

+ }

job->user_paused = true;

block_job_pause(job);

}

@@ -XXX,XX +XXX,XX @@ bool block_job_user_paused(BlockJob *job)

return job->user_paused;

}

-void block_job_user_resume(BlockJob *job)

+void block_job_user_resume(BlockJob *job, Error **errp)

{

- if (job && job->user_paused && job->pause_count > 0) {

- block_job_iostatus_reset(job);

- job->user_paused = false;

- block_job_resume(job);

+ assert(job);

+ if (!job->user_paused || job->pause_count <= 0) {

+ error_setg(errp, "Can't resume a job that was not paused");

+ return;

+ }

+ if (block_job_apply_verb(job, BLOCK_JOB_VERB_RESUME, errp)) {

+ return;

}

+ block_job_iostatus_reset(job);

+ job->user_paused = false;

+ block_job_resume(job);

}

void block_job_cancel(BlockJob *job)

@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)

}

}

+void block_job_user_cancel(BlockJob *job, Error **errp)

+{

+ if (block_job_apply_verb(job, BLOCK_JOB_VERB_CANCEL, errp)) {

+ return;

+ }

+ block_job_cancel(job);

+}

+

/* A wrapper around block_job_cancel() taking an Error ** parameter so it may be

* used with block_job_finish_sync() without the need for (rather nasty)

* function pointer casts there. */

@@ -XXX,XX +XXX,XX @@ BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,

action, &error_abort);

}

if (action == BLOCK_ERROR_ACTION_STOP) {

+ block_job_pause(job);

/* make the pause user visible, which will be resumed from QMP. */

- block_job_user_pause(job);

+ job->user_paused = true;

block_job_iostatus_set_err(job, error);

}

return action;

diff --git a/tests/test-bdrv-drain.c b/tests/test-bdrv-drain.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/test-bdrv-drain.c

+++ b/tests/test-bdrv-drain.c

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn test_job_start(void *opaque)

{

TestBlockJob *s = opaque;

+ block_job_event_ready(&s->common);

while (!s->should_complete) {

block_job_sleep_ns(&s->common, 100000);

}

diff --git a/block/trace-events b/block/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/block/trace-events

+++ b/block/trace-events

@@ -XXX,XX +XXX,XX @@ bdrv_lock_medium(void *bs, bool locked) "bs %p locked %d"

# blockjob.c

block_job_state_transition(void *job, int ret, const char *legal, const char *s0, const char *s1) "job %p (ret: %d) attempting %s transition (%s-->%s)"

+block_job_apply_verb(void *job, const char *state, const char *verb, const char *legal) "job %p in state %s; applying verb %s (%s)"

# block/block-backend.c

blk_co_preadv(void *blk, void *bs, int64_t offset, unsigned int bytes, int flags) "blk %p bs %p offset %"PRId64" bytes %u flags 0x%x"

2.13.6

From: Liang Li <liliang.opensource@gmail.com>

When doing drive mirror to a low speed shared storage, if there was heavy

BLK IO write workload in VM after the 'ready' event, drive mirror block job

can't be canceled immediately, it would keep running until the heavy BLK IO

workload stopped in the VM.

Libvirt depends on the current block-job-cancel semantics, which is that

when used without a flag after the 'ready' event, the command blocks

until data is in sync. However, these semantics are awkward in other

situations, for example, people may use drive mirror for realtime

backups while still wanting to use block live migration. Libvirt cannot

start a block live migration while another drive mirror is in progress,

but the user would rather abandon the backup attempt as broken and

proceed with the live migration than be stuck waiting for the current

drive mirror backup to finish.

The drive-mirror command already includes a 'force' flag, which libvirt

does not use, although it documented the flag as only being useful to

quit a job which is paused. However, since quitting a paused job has

the same effect as abandoning a backup in a non-paused job (namely, the

destination file is not in sync, and the command completes immediately),

we can just improve the documentation to make the force flag obviously

useful.

Cc: Paolo Bonzini <pbonzini@redhat.com>

Cc: Jeff Cody <jcody@redhat.com>

Cc: Kevin Wolf <kwolf@redhat.com>

Cc: Max Reitz <mreitz@redhat.com>

Cc: Eric Blake <eblake@redhat.com>

Cc: John Snow <jsnow@redhat.com>

Reported-by: Huaitong Han <huanhuaitong@didichuxing.com>

Signed-off-by: Huaitong Han <huanhuaitong@didichuxing.com>

Signed-off-by: Liang Li <liliangleo@didichuxing.com>

Signed-off-by: Jeff Cody <jcody@redhat.com>

qapi/block-core.json | 5 +++--

include/block/blockjob.h | 12 ++++++++++--

block/mirror.c | 10 ++++------

blockdev.c | 4 ++--

blockjob.c | 16 +++++++++-------

tests/test-blockjob-txn.c | 8 ++++----

hmp-commands.hx | 3 ++-

7 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json

--- a/qapi/block-core.json

+++ b/qapi/block-core.json

@@ -XXX,XX +XXX,XX @@

# the name of the parameter), but since QEMU 2.7 it can have

# other values.

#

-# @force: whether to allow cancellation of a paused job (default

-# false). Since 1.3.

+# @force: If true, and the job has already emitted the event BLOCK_JOB_READY,

+# abandon the job immediately (even if it is paused) instead of waiting

+# for the destination to complete its final synchronization (since 1.3)

#

# Returns: Nothing on success

# If no background operation is active on this device, DeviceNotActive

diff --git a/include/block/blockjob.h b/include/block/blockjob.h

index XXXXXXX..XXXXXXX 100644

--- a/include/block/blockjob.h

+++ b/include/block/blockjob.h

@@ -XXX,XX +XXX,XX @@ typedef struct BlockJob {

bool cancelled;

/**

+ * Set to true if the job should abort immediately without waiting

+ * for data to be in sync.

+ */

+ bool force;

+

+ /**

* Counter for pause request. If non-zero, the block job is either paused,

* or if busy == true will pause itself as soon as possible.

*/

@@ -XXX,XX +XXX,XX @@ void block_job_start(BlockJob *job);

/**

* block_job_cancel:

* @job: The job to be canceled.

+ * @force: Quit a job without waiting for data to be in sync.

*

* Asynchronously cancel the specified job.

*/

-void block_job_cancel(BlockJob *job);

+void block_job_cancel(BlockJob *job, bool force);

/**

* block_job_complete:

@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job, Error **errp);

/**

* block_job_user_cancel:

* @job: The job to be cancelled.

+ * @force: Quit a job without waiting for data to be in sync.

*

* Cancels the specified job, but may refuse to do so if the

* operation isn't currently meaningful.

*/

-void block_job_user_cancel(BlockJob *job, Error **errp);

+void block_job_user_cancel(BlockJob *job, bool force, Error **errp);

/**

* block_job_cancel_sync:

diff --git a/block/mirror.c b/block/mirror.c

index XXXXXXX..XXXXXXX 100644

--- a/block/mirror.c

+++ b/block/mirror.c

@@ -XXX,XX +XXX,XX @@ static void coroutine_fn mirror_run(void *opaque)

ret = 0;

trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);

- if (!s->synced) {

- block_job_sleep_ns(&s->common, delay_ns);

- if (block_job_is_cancelled(&s->common)) {

- break;

- }

+ if (block_job_is_cancelled(&s->common) && s->common.force) {

+ break;

} else if (!should_complete) {

delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);

block_job_sleep_ns(&s->common, delay_ns);

@@ -XXX,XX +XXX,XX @@ immediate_exit:

* or it was cancelled prematurely so that we do not guarantee that

* the target is a copy of the source.

*/

- assert(ret < 0 || (!s->synced && block_job_is_cancelled(&s->common)));

+ assert(ret < 0 || ((s->common.force || !s->synced) &&

+ block_job_is_cancelled(&s->common)));

assert(need_drain);

mirror_wait_for_all_io(s);

}

diff --git a/blockdev.c b/blockdev.c

index XXXXXXX..XXXXXXX 100644

--- a/blockdev.c

+++ b/blockdev.c

@@ -XXX,XX +XXX,XX @@ void blockdev_mark_auto_del(BlockBackend *blk)

aio_context_acquire(aio_context);

if (bs->job) {

- block_job_cancel(bs->job);

+ block_job_cancel(bs->job, false);

}

aio_context_release(aio_context);

@@ -XXX,XX +XXX,XX @@ void qmp_block_job_cancel(const char *device,

}

trace_qmp_block_job_cancel(job);

- block_job_user_cancel(job, errp);

+ block_job_user_cancel(job, force, errp);

out:

aio_context_release(aio_context);

}

diff --git a/blockjob.c b/blockjob.c

index XXXXXXX..XXXXXXX 100644

--- a/blockjob.c

+++ b/blockjob.c

@@ -XXX,XX +XXX,XX @@ static int block_job_finalize_single(BlockJob *job)

return 0;

}

-static void block_job_cancel_async(BlockJob *job)

+static void block_job_cancel_async(BlockJob *job, bool force)

{

if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) {

block_job_iostatus_reset(job);

@@ -XXX,XX +XXX,XX @@ static void block_job_cancel_async(BlockJob *job)

job->pause_count--;

}

job->cancelled = true;

+ /* To prevent 'force == false' overriding a previous 'force == true' */

+ job->force |= force;

}

static int block_job_txn_apply(BlockJobTxn *txn, int fn(BlockJob *), bool lock)

@@ -XXX,XX +XXX,XX @@ static void block_job_completed_txn_abort(BlockJob *job)

* on the caller, so leave it. */

QLIST_FOREACH(other_job, &txn->jobs, txn_list) {

if (other_job != job) {

- block_job_cancel_async(other_job);

+ block_job_cancel_async(other_job, false);

}

}

while (!QLIST_EMPTY(&txn->jobs)) {

@@ -XXX,XX +XXX,XX @@ void block_job_user_resume(BlockJob *job, Error **errp)

block_job_resume(job);

}

-void block_job_cancel(BlockJob *job)

+void block_job_cancel(BlockJob *job, bool force)

{

if (job->status == BLOCK_JOB_STATUS_CONCLUDED) {

block_job_do_dismiss(job);

return;

}

- block_job_cancel_async(job);

+ block_job_cancel_async(job, force);

if (!block_job_started(job)) {

block_job_completed(job, -ECANCELED);

} else if (job->deferred_to_main_loop) {

@@ -XXX,XX +XXX,XX @@ void block_job_cancel(BlockJob *job)

}

}

-void block_job_user_cancel(BlockJob *job, Error **errp)

+void block_job_user_cancel(BlockJob *job, bool force, Error **errp)

{

if (block_job_apply_verb(job, BLOCK_JOB_VERB_CANCEL, errp)) {

return;

}

- block_job_cancel(job);

+ block_job_cancel(job, force);

}

/* A wrapper around block_job_cancel() taking an Error ** parameter so it may be

@@ -XXX,XX +XXX,XX @@ void block_job_user_cancel(BlockJob *job, Error **errp)

* function pointer casts there. */

static void block_job_cancel_err(BlockJob *job, Error **errp)

{

- block_job_cancel(job);

+ block_job_cancel(job, false);

}

int block_job_cancel_sync(BlockJob *job)

diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c

index XXXXXXX..XXXXXXX 100644

--- a/tests/test-blockjob-txn.c

+++ b/tests/test-blockjob-txn.c

@@ -XXX,XX +XXX,XX @@ static void test_single_job(int expected)

block_job_start(job);

if (expected == -ECANCELED) {

- block_job_cancel(job);

+ block_job_cancel(job, false);

}

while (result == -EINPROGRESS) {

@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs(int expected1, int expected2)

block_job_txn_unref(txn);

if (expected1 == -ECANCELED) {

- block_job_cancel(job1);

+ block_job_cancel(job1, false);

}

if (expected2 == -ECANCELED) {

- block_job_cancel(job2);

+ block_job_cancel(job2, false);

}

while (result1 == -EINPROGRESS || result2 == -EINPROGRESS) {

@@ -XXX,XX +XXX,XX @@ static void test_pair_jobs_fail_cancel_race(void)

block_job_start(job1);

block_job_start(job2);

- block_job_cancel(job1);

+ block_job_cancel(job1, false);

/* Now make job2 finish before the main loop kicks jobs. This simulates

* the race between a pending kick and another job completing.

diff --git a/hmp-commands.hx b/hmp-commands.hx

index XXXXXXX..XXXXXXX 100644

--- a/hmp-commands.hx

+++ b/hmp-commands.hx

@@ -XXX,XX +XXX,XX @@ ETEXI

.args_type = "force:-f,device:B",

.params = "[-f] device",

.help = "stop an active background block operation (use -f"

- "\n\t\t\t if the operation is currently paused)",

+ "\n\t\t\t if you want to abort the operation immediately"

+ "\n\t\t\t instead of keep running until data is in sync)",

.cmd = hmp_block_job_cancel,

},

2.13.6

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

tests/qemu-iotests/153 | 12 ++++++++++++

tests/qemu-iotests/153.out | 5 +++++

2 files changed, 17 insertions(+)

diff --git a/tests/qemu-iotests/153 b/tests/qemu-iotests/153

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/153

+++ b/tests/qemu-iotests/153

@@ -XXX,XX +XXX,XX @@ rm -f "${TEST_IMG}.lnk" &>/dev/null

ln -s ${TEST_IMG} "${TEST_IMG}.lnk" || echo "Failed to create link"

_run_qemu_with_images "${TEST_IMG}.lnk" "${TEST_IMG}"

+echo

+echo "== Active commit to intermediate layer should work when base in use =="

+_launch_qemu -drive format=$IMGFMT,file="${TEST_IMG}.a",id=drive0,if=none \

+ -device virtio-blk,drive=drive0

+_send_qemu_cmd $QEMU_HANDLE \

+ "{ 'execute': 'qmp_capabilities' }" \

+ 'return'

+_run_cmd $QEMU_IMG commit -b "${TEST_IMG}.b" "${TEST_IMG}.c"

+

+_cleanup_qemu

+

_launch_qemu

_send_qemu_cmd $QEMU_HANDLE \

diff --git a/tests/qemu-iotests/153.out b/tests/qemu-iotests/153.out

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/153.out

+++ b/tests/qemu-iotests/153.out

@@ -XXX,XX +XXX,XX @@ Is another process using the image?

== Symbolic link ==

QEMU_PROG: -drive if=none,file=TEST_DIR/t.qcow2: Failed to get "write" lock

Is another process using the image?

+

+== Active commit to intermediate layer should work when base in use ==

+{"return": {}}

+

+_qemu_img_wrapper commit -b TEST_DIR/t.qcow2.b TEST_DIR/t.qcow2.c

{"return": {}}

Adding drive