Re: [Qemu-block] [Qemu-devel] [PULL 00/46] Block layer patches

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20180523131155.12359-1-kw...@redhat.com
Subject: [Qemu-devel] [PULL 00/46] Block layer patches

=== 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
 t [tag update]
patchew/1527034517-7851-1-git-send-email-...@sifive.com -> 
patchew/1527034517-7851-1-git-send-email-...@sifive.com
 t [tag update]
patchew/1527070950-208350-1-git-send-email-imamm...@redhat.com -> 
patchew/1527070950-208350-1-git-send-email-imamm...@redhat.com
 * [new tag]   patchew/20180523131155.12359-1-kw...@redhat.com -> 
patchew/20180523131155.12359-1-kw...@redhat.com
Switched to a new branch 'test'
aae2eed4b1 qemu-iotests: Test job-* with block jobs
a689b1966f iotests: Move qmp_to_opts() to VM
08f5e0497c blockjob: Remove BlockJob.driver
0085b65c93 job: Add query-jobs QMP command
1d5b7d49fb job: Add lifecycle QMP commands
bea5137eb4 job: Add JOB_STATUS_CHANGE QMP event
6e3f660c6c job: Introduce qapi/job.json
5355b1b58c job: Move progress fields to Job
9d268762dd job: Add job_transition_to_ready()
8e773e9eed job: Add job_is_ready()
f1ec86a985 job: Add job_dismiss()
4759fb0c3b job: Add job_yield()
66517b7d57 block: Cancel job in bdrv_close_all() callers
b4b2931166 job: Move completion and cancellation to Job
f42cb0be41 job: Move transactions to Job
01b9fdb39a job: Switch transactions to JobTxn
97347cade6 job: Move job_finish_sync() to Job
4a29e4bfa9 job: Move .complete callback to Job
3e9ca9c7a3 job: Add job_drain()
d43ca933aa job: Convert block_job_cancel_async() to Job
82596bdc04 job: Move single job finalisation to Job
1364587ef4 job: Add job_event_*()
c21f3e55b5 blockjob: Split block_job_event_pending()
e7c33c45a2 job: Move BlockJobCreateFlags to Job
f954ae0df7 job: Replace BlockJob.completed with job_is_completed()
46ffb33a19 job: Move pause/resume functions to Job
e0b0c50412 job: Add job_sleep_ns()
5d98bd8e34 job: Move coroutine and related code to Job
c4aeef62ce job: Move defer_to_main_loop to Job
8019880d71 job: Add Job.aio_context
d18b3a2985 job: Move cancelled to Job
5c2f0edff8 job: Add reference counting
91973c1d75 job: Move state transitions to Job
f4afc429f9 job: Maintain a list of all jobs
670104491e job: Add job_delete()
50226692ec job: Add JobDriver.job_type
85c2e2b99d job: Rename BlockJobType into JobType
e28a4a625f job: Create Job, JobDriver and job_create()
6e0b9d626f blockjob: Improve BlockJobInfo.offset/len documentation
5eb99c4e19 blockjob: Update block-job-pause/resume documentation
b89609ccfc qemu-iotests: Remove MIG_SOCKET from non-migration tests
19cf39414c qemu-iotests: Add more tests to "migration" group
0bb69f57d5 sheepdog: Remove unnecessary NULL check in sd_prealloc()
87654ad197 qemu-iotests: 086 doesn't work with NFS
d7728954a9 qemu-iotests: Filter NFS paths
ef360f2462 qemu-iotests: Fix paths for NFS

=== OUTPUT BEGIN ===
Checking PATCH 1/46: qemu-iotests: Fix paths for NFS...
Checking PATCH 2/46: qemu-iotests: Filter NFS paths...
Checking PATCH 3/46: qemu-iotests: 086 doesn't work with NFS...
Checking PATCH 4/46: sheepdog: Remove unnecessary NULL check in sd_prealloc()...
Checking PATCH 5/46: qemu-iotests: Add more tests to "migration" group...
Checking PATCH 6/46: qemu-iotests: Remove MIG_SOCKET from non-migration tests...
Checking PATCH 7/46: blockjob: Update block-job-pause/resume documentation...
Checking PATCH 8/46: blockjob: Improve BlockJobInfo.offset/len documentation...
Checking PATCH 9/46: job: Create Job, JobDriver and job_create()...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#353: 
new file mode 100644

total: 0 errors, 1 warnings, 424 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/46: job: Rename BlockJobType into JobType...
Checking PATCH 11/46: job: Add JobDriver.job_type...
Checking PATCH 12/46: job: Add job_delete()...
Checking PATCH 13/46: job: Maintain a list of all jobs...
Checking PATCH 14/46: job: Move state transitions to Job...
ERROR: space prohibited before open square bracket '['
#343: FILE: job.c:38:
+/* U: */ [JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#344: FILE: job.c:39:
+/* C: */ 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 35/40] job: Add JOB_STATUS_CHANGE QMP event

[John Snow]

On 05/18/2018 09:21 AM, Kevin Wolf wrote:
> This adds a QMP event that is emitted whenever a job transitions from
> one status to another.
> 
> Signed-off-by: Kevin Wolf 

That's a lot of events, and a lot are redundant to what we already
transmitted under block jobs; it also has the effect of making internal
state changes explicit behavior that we're responsible for maintaining
for external clients.

Is that what we want here?

(I mean, the answer is probably "Yes" because you're here writing the
patch, but I was hoping to find your motivation.)

Re: [Qemu-block] [Qemu-devel] [PATCH v2 36/40] job: Add lifecycle QMP commands

[John Snow]

On 05/18/2018 09:21 AM, Kevin Wolf wrote:
> +{ 'command': 'job-complete', 'data': { 'id': 'str' } }

Do we have to name it this? I've always disliked how "complete" is used
as both a verb and an adjective in our code (hence why I used such
bizarre phrasings like "concluded" in my jobs patches) ...

I don't necessarily have a better suggestion, but now's the chance to
ask the question.

I guess you don't have a better suggestion either, otherwise you'd have
changed it.

--js

Re: [Qemu-block] [Qemu-devel] [PATCH v2 31/40] job: Add job_is_ready()

[John Snow]

On 05/18/2018 09:21 AM, Kevin Wolf wrote:
> Instead of having a 'bool ready' in BlockJob, add a function that
> derives its value from the job status.
> 
> At the same time, this fixes the behaviour to match what the QAPI
> documentation promises for query-block-job: 'true if the job may be
> completed'. When the ready flag was introduced in commit ef6dbf1e46e,
> the flag never had to be reset to match the description because after
> being ready, the jobs would immediately complete and disappear.
> 
> Job transactions and manual job finalisation were introduced only later.
> With these changes, jobs may stay around even after having completed
> (and they are not ready to be completed a second time), however their
> patches forgot to reset the ready flag.
> 
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Max Reitz 
> ---
>  include/block/blockjob.h |  5 -
>  include/qemu/job.h   |  3 +++
>  blockjob.c   |  3 +--
>  job.c| 22 ++
>  qemu-img.c   |  2 +-
>  tests/test-blockjob.c|  2 +-
>  6 files changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 5a81afc6c3..8e1e1ee0de 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -49,11 +49,6 @@ typedef struct BlockJob {
>  /** The block device on which the job is operating.  */
>  BlockBackend *blk;
>  
> -/**
> - * Set to true when the job is ready to be completed.
> - */
> -bool ready;
> -
>  /** Status that is published by the query-block-jobs QMP API */
>  BlockDeviceIoStatus iostatus;
>  
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index 1e8050c6fb..487f9d9a32 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -367,6 +367,9 @@ bool job_is_cancelled(Job *job);
>  /** Returns whether the job is in a completed state. */
>  bool job_is_completed(Job *job);
>  
> +/** Returns whether the job is ready to be completed. */
> +bool job_is_ready(Job *job);
> +
>  /**
>   * Request @job to pause at the next pause point. Must be paired with
>   * job_resume(). If the job is supposed to be resumed by user action, call
> diff --git a/blockjob.c b/blockjob.c
> index 3ca009bea5..38f18e9ba3 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -269,7 +269,7 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
>  info->offset= job->offset;
>  info->speed = job->speed;
>  info->io_status = job->iostatus;
> -info->ready = job->ready;
> +info->ready = job_is_ready(>job),
>  info->status= job->job.status;
>  info->auto_finalize = job->job.auto_finalize;
>  info->auto_dismiss  = job->job.auto_dismiss;
> @@ -436,7 +436,6 @@ void block_job_user_resume(Job *job)
>  void block_job_event_ready(BlockJob *job)
>  {
>  job_state_transition(>job, JOB_STATUS_READY);
> -job->ready = true;
>  
>  if (block_job_is_internal(job)) {
>  return;
> diff --git a/job.c b/job.c
> index af31de4669..66ee26f2a0 100644
> --- a/job.c
> +++ b/job.c
> @@ -199,6 +199,28 @@ bool job_is_cancelled(Job *job)
>  return job->cancelled;
>  }
>  
> +bool job_is_ready(Job *job)
> +{
> +switch (job->status) {
> +case JOB_STATUS_UNDEFINED:
> +case JOB_STATUS_CREATED:
> +case JOB_STATUS_RUNNING:
> +case JOB_STATUS_PAUSED:
> +case JOB_STATUS_WAITING:
> +case JOB_STATUS_PENDING:
> +case JOB_STATUS_ABORTING:
> +case JOB_STATUS_CONCLUDED:
> +case JOB_STATUS_NULL:
> +return false;
> +case JOB_STATUS_READY:
> +case JOB_STATUS_STANDBY:
> +return true;
> +default:
> +g_assert_not_reached();
> +}
> +return false;
> +}
> +

What's the benefit to a switch statement with a default clause here,
over the shorter:

if (job->status == READY || job->status == STANDBY) {
  return true;
}
return false;

(Yes, I realize you already merged this code, but I'm still curious and
I need to read the series anyway to see what's changed...)

>  bool job_is_completed(Job *job)
>  {
>  switch (job->status) {
> diff --git a/qemu-img.c b/qemu-img.c
> index b2bcfc3df1..eecb7d3228 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -867,7 +867,7 @@ static void run_block_job(BlockJob *job, Error **errp)
>  aio_poll(aio_context, true);
>  qemu_progress_print(job->len ?
>  ((float)job->offset / job->len * 100.f) : 0.0f, 
> 0);
> -} while (!job->ready && !job_is_completed(>job));
> +} while (!job_is_ready(>job) && !job_is_completed(>job));
>  

Semantic change in this variable, but due to the way this function is
written it doesn't matter.

>  if (!job_is_completed(>job)) {
>  ret = job_complete_sync(>job, errp);
> diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
> index 7131cabb16..8180d03a5f 100644
> --- a/tests/test-blockjob.c
> +++ b/tests/test-blockjob.c
> @@ 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 21/40] job: Convert block_job_cancel_async() to Job

[John Snow]

On 05/18/2018 09:20 AM, Kevin Wolf wrote:
> block_job_cancel_async() did two things that were still block job
> specific:
> 
> * Setting job->force. This field makes sense on the Job level, so we can
>   just move it. While at it, rename it to job->force_cancel to make its
>   purpose more obvious.
> 
> * Resetting the I/O status. This can't be moved because generic Jobs
>   don't have an I/O status. What the function really implements is a
>   user resume, except without entering the coroutine. Consequently, it

You know, I had noticed this before but because this is called from
contexts which are NOT user-initiated, I didn't want to share the callback.

... I guess it's fine because the job is about to not exist anymore, so
it probably won't spook anything.

>   makes sense to call the .user_resume driver callback here which
>   already resets the I/O status.
> 
>   The old block_job_cancel_async() has two separate if statements that
>   check job->iostatus != BLOCK_DEVICE_IO_STATUS_OK and job->user_paused.
>   However, the former condition always implies the latter (as is
>   asserted in block_job_iostatus_reset()), so changing the explicit call
>   of block_job_iostatus_reset() on the former condition with the
>   .user_resume callback on the latter condition is equivalent and
>   doesn't need to access any BlockJob specific state.

Oh, cool! This is nice.

> 
> Signed-off-by: Kevin Wolf 
> Reviewed-by: Max Reitz 
> ---
>  include/block/blockjob.h |  6 --
>  include/qemu/job.h   |  6 ++
>  block/mirror.c   |  4 ++--
>  blockjob.c   | 25 +
>  4 files changed, 21 insertions(+), 20 deletions(-)
> 
> diff --git a/include/block/blockjob.h b/include/block/blockjob.h
> index 3f405d1fa7..d975efea20 100644
> --- a/include/block/blockjob.h
> +++ b/include/block/blockjob.h
> @@ -51,12 +51,6 @@ typedef struct BlockJob {
>  BlockBackend *blk;
>  
>  /**
> - * Set to true if the job should abort immediately without waiting
> - * for data to be in sync.
> - */
> -bool force;
> -
> -/**
>   * Set to true when the job is ready to be completed.
>   */
>  bool ready;
> diff --git a/include/qemu/job.h b/include/qemu/job.h
> index 3e817beee9..2648c74281 100644
> --- a/include/qemu/job.h
> +++ b/include/qemu/job.h
> @@ -97,6 +97,12 @@ typedef struct Job {
>   */
>  bool cancelled;
>  
> +/**
> + * Set to true if the job should abort immediately without waiting
> + * for data to be in sync.
> + */
> +bool force_cancel;
> +

Does this comment need an update now, though?

Actually, in terms of "new jobs" API, it'd be really nice if cancel
*always meant cancel*.

I think "cancel" should never be used to mean "successful completion,
but different from the one we'd get if we used job_complete."

i.e., either we need a job setting:

job-set completion-mode=[pivot|no-pivot]

or optional parameters to pass to job-complete:

job-complete mode=[understood-by-job-type]

or some other mechanism that accomplishes the same type of behavior. It
would be nice if it did not have to be determined at job creation time
but instead could be determined later.


>  /** Set to true when the job has deferred work to the main loop. */
>  bool deferred_to_main_loop;
>  
> diff --git a/block/mirror.c b/block/mirror.c
> index e9a90ea730..c3951d1ca2 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -871,7 +871,7 @@ static void coroutine_fn mirror_run(void *opaque)
>  trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
>  job_sleep_ns(>common.job, delay_ns);
>  if (job_is_cancelled(>common.job) &&
> -(!s->synced || s->common.force))
> +(!s->synced || s->common.job.force_cancel))
>  {
>  break;
>  }
> @@ -884,7 +884,7 @@ 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->common.force || !s->synced) &&
> +assert(ret < 0 || ((s->common.job.force_cancel || !s->synced) &&
> job_is_cancelled(>common.job)));
>  assert(need_drain);
>  mirror_wait_for_all_io(s);
> diff --git a/blockjob.c b/blockjob.c
> index 34c57da304..4cac3670ad 100644
> --- a/blockjob.c
> +++ b/blockjob.c
> @@ -270,19 +270,20 @@ static int block_job_prepare(BlockJob *job)
>  return job->job.ret;
>  }
>  
> -static void block_job_cancel_async(BlockJob *job, bool force)
> +static void job_cancel_async(Job *job, bool force)
>  {
> -if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) {
> -block_job_iostatus_reset(job);
> -}
> -if (job->job.user_paused) {
> -/* Do not call block_job_enter here, the caller will handle it.  */
> -job->job.user_paused = false;
> -job->job.pause_count--;
> +if (job->user_paused) {
> + 

Re: [Qemu-block] [Qemu-devel] [PATCH v2 17/40] job: Move BlockJobCreateFlags to Job

[John Snow]

On 05/18/2018 09:20 AM, Kevin Wolf wrote:
> +job->auto_finalize = !(flags & JOB_MANUAL_FINALIZE);
> +job->auto_dismiss  = !(flags & JOB_MANUAL_DISMISS);

Job API might be a good chance to say "No, this is the default behavior
for this API."

I don't know how possible this is, but could we remove these behavior
flags for jobs (but keep them for block jobs), and then any legacy block
job creation interfaces we have can enable/disable them as the user
requested,

and the block job layer itself has hooks that persuade the core job
layer to automatically transition without user input, if appropriate.

(Unless that happens later?)

Re: [Qemu-block] [Qemu-devel] AIO error case

[Nishanth Aravamudan]

On Wed, May 23, 2018 at 10:53 AM, John Snow  wrote:
>
>
>
> On 05/22/2018 06:01 PM, Nishanth Aravamudan via Qemu-devel wrote:
> > Hi!
> >
>
> Hi! CCing qemu-block@nongnu.org;
>
> > I'm tracking an error case in the native AIO path, and was wondering if
> > there was a latent (albeit possibly hard to hit) bug. Specifically
> > util/async.c::aio_get_linux_aio:
> >
> > #ifdef CONFIG_LINUX_AIO
> > LinuxAioState *aio_get_linux_aio(AioContext *ctx)
> > {
> > if (!ctx->linux_aio) {
> > ctx->linux_aio = laio_init();
> > laio_attach_aio_context(ctx->linux_aio, ctx);
> > }
> > return ctx->linux_aio;
> > }
> > #endif
> >
> > laio_init() can in certain conditions return NULL, but that's not
checked
> > here and then the NULL result is passed directly into
> > laio_attach_aio_context, which dereferences it without checking that the
> > pointer is valid.
> >
>
> Looks like a good old-fashioned bug to me:


Agreed!



> Wanna send a patch?

Yep I'll work on this over the next few days. Thanks for reply!

-Nish

Re: [Qemu-block] [PATCH 1/8] parallels: Switch to byte-based calls

[John Snow]

On 05/23/2018 04:09 PM, Eric Blake wrote:
> On 05/23/2018 02:19 PM, John Snow wrote:
>>
>>
>> On 04/25/2018 02:32 PM, Eric Blake wrote:
>>> We are gradually moving away from sector-based interfaces, towards
>>> byte-based.  Make the change for the last few sector-based calls
>>> into the block layer from the parallels driver.
>>>
>>> Ideally, the parallels driver should switch to doing everything
>>> byte-based, but that's a more invasive change that requires a
>>> bit more auditing.
>>>
>>> Signed-off-by: Eric Blake 
>>
>> Older than a month, so I'm assuming this is dropped for now.
> 
> No, it's still awaiting a review; and still applies without any context
> changes.

Oh, OK! then I'll count this as my naive ping-by-proxy.

--js

Re: [Qemu-block] [PATCH 1/8] parallels: Switch to byte-based calls

[Eric Blake]

On 05/23/2018 02:19 PM, John Snow wrote:



On 04/25/2018 02:32 PM, Eric Blake wrote:

We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the last few sector-based calls
into the block layer from the parallels driver.

Ideally, the parallels driver should switch to doing everything
byte-based, but that's a more invasive change that requires a
bit more auditing.

Signed-off-by: Eric Blake 


Older than a month, so I'm assuming this is dropped for now.


No, it's still awaiting a review; and still applies without any context 
changes.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [Qemu-devel] [PULL 41/46] job: Add JOB_STATUS_CHANGE QMP event

[Eric Blake]

On 05/23/2018 08:11 AM, Kevin Wolf wrote:

This adds a QMP event that is emitted whenever a job transitions from
one status to another.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---



+
+##
+# @JOB_STATUS_CHANGE:
+#
+# Emitted when a job transitions to a different status.
+#
+# @id: The job identifier
+# @status: The new job status
+#
+# Since: 2.13


We'll probably have a few straggler commits like this that interleave 
with Peter's patches to renumber things to 3.0; let's make sure we don't 
forget to clean everything up before the release.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [PATCH 1/8] parallels: Switch to byte-based calls

[John Snow]

On 04/25/2018 02:32 PM, Eric Blake wrote:
> We are gradually moving away from sector-based interfaces, towards
> byte-based.  Make the change for the last few sector-based calls
> into the block layer from the parallels driver.
> 
> Ideally, the parallels driver should switch to doing everything
> byte-based, but that's a more invasive change that requires a
> bit more auditing.
> 
> Signed-off-by: Eric Blake 

Older than a month, so I'm assuming this is dropped for now.

--js

Re: [Qemu-block] [PATCH v3 1/5] nbd/server: fix trace

[Eric Blake]

On 05/23/2018 05:24 AM, Vladimir Sementsov-Ogievskiy wrote:

Return code = 1 doesn't mean that we parsed base:allocation. Move
trace point to appropriate place.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)




@@ -768,10 +771,10 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
  }
  
  if (strncmp(query, "allocation", alen) == 0) {

+trace_nbd_negotiate_meta_query_parse("base:allocation");
  meta->base_allocation = true;
  }


It's probably worth tracing even when we successfully skip the request, 
as in:


if (strncmp...) {
trace_nbd_negotiate_meta_query_parse("base:allocation");
meta->base_allocation = true;
} else {
trace_nbd_negotiate_meta_query_skip("unrecognized request %s", query);
}

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [PATCH v5 02/10] raw: Check byte range uniformly

[Eric Blake]

On 05/22/2018 10:04 PM, Fam Zheng wrote:

We don't verify the request range against s->size in the I/O callbacks
except for raw_co_pwritev. This is wrong (especially for
raw_co_pwrite_zeroes and raw_co_pdiscard), so fix them.


I'd also mention ...



Signed-off-by: Fam Zheng 
---
  block/raw-format.c | 64 +-
  1 file changed, 39 insertions(+), 25 deletions(-)


Should this cc: qemu-stable?

Do we have iotests coverage of this?



diff --git a/block/raw-format.c b/block/raw-format.c
index fe33693a2d..b69a0674b3 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -167,16 +167,37 @@ static void raw_reopen_abort(BDRVReopenState *state)
  state->opaque = NULL;
  }
  
+/* Check and adjust the offset, against 'offset' and 'size' options. */

+static inline int raw_adjust_offset(BlockDriverState *bs, uint64_t *offset,
+uint64_t bytes, bool is_write)
+{
+BDRVRawState *s = bs->opaque;
+
+if (s->has_size && (*offset > s->size || bytes > (s->size - *offset))) {
+/* There's not enough space for the write, or the read request is
+ * out-of-range. Don't read/write anything to prevent leaking out of
+ * the size specified in options. */
+return is_write ? -ENOSPC : -EINVAL;;
+}
+
+if (*offset > INT64_MAX - s->offset) {
+return -EINVAL;


...that this change to a 63-bit check...


@@ -186,23 +207,11 @@ static int coroutine_fn raw_co_pwritev(BlockDriverState 
*bs, uint64_t offset,



-if (offset > UINT64_MAX - s->offset) {
-ret = -EINVAL;
-goto fail;
-}


...from a previous 64-bit check is intentional.

With improved commit message, and ideally with followup commits that add 
iotest coverage,


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [Qemu-devel] AIO error case

[John Snow]

On 05/23/2018 02:25 PM, Nishanth Aravamudan wrote:
> On Wed, May 23, 2018 at 10:53 AM, John Snow  > wrote:
>>
>>
>>
>> On 05/22/2018 06:01 PM, Nishanth Aravamudan via Qemu-devel wrote:
>> > Hi!
>> >
>>
>> Hi! CCing qemu-block@nongnu.org ;
>>
>> > I'm tracking an error case in the native AIO path, and was wondering if
>> > there was a latent (albeit possibly hard to hit) bug. Specifically
>> > util/async.c::aio_get_linux_aio:
>> >
>> > #ifdef CONFIG_LINUX_AIO
>> > LinuxAioState *aio_get_linux_aio(AioContext *ctx)
>> > {
>> >     if (!ctx->linux_aio) {
>> >         ctx->linux_aio = laio_init();
>> >         laio_attach_aio_context(ctx->linux_aio, ctx);
>> >     }
>> >     return ctx->linux_aio;
>> > }
>> > #endif
>> >
>> > laio_init() can in certain conditions return NULL, but that's not
> checked
>> > here and then the NULL result is passed directly into
>> > laio_attach_aio_context, which dereferences it without checking that the
>> > pointer is valid.
>> >
>>
>> Looks like a good old-fashioned bug to me:
> 
> 
> Agreed!
>  
> 
> 
>> Wanna send a patch?
> 
> Yep I'll work on this over the next few days. Thanks for reply!
> 
> -Nish

I looked at plug and unplug and it really looks like -- apart from the
memoization of aio_get_linux_aio that might fail -- there's nothing in
those calls that is expected to actually break.

Might be saner to try to force the memoization earlier for virtio-blk
and virtio-scsi and test the return at that time; then just assert that
aio_get_linux_aio actually returns non-null in calls like un/plug that
cannot fail.

Should save you a lot of rewiring work.

--js

Re: [Qemu-block] [Qemu-devel] AIO error case

[John Snow]

On 05/22/2018 06:01 PM, Nishanth Aravamudan via Qemu-devel wrote:
> Hi!
> 

Hi! CCing qemu-block@nongnu.org;

> I'm tracking an error case in the native AIO path, and was wondering if
> there was a latent (albeit possibly hard to hit) bug. Specifically
> util/async.c::aio_get_linux_aio:
> 
> #ifdef CONFIG_LINUX_AIO
> LinuxAioState *aio_get_linux_aio(AioContext *ctx)
> {
> if (!ctx->linux_aio) {
> ctx->linux_aio = laio_init();
> laio_attach_aio_context(ctx->linux_aio, ctx);
> }
> return ctx->linux_aio;
> }
> #endif
> 
> laio_init() can in certain conditions return NULL, but that's not checked
> here and then the NULL result is passed directly into
> laio_attach_aio_context, which dereferences it without checking that the
> pointer is valid.
> 

Looks like a good old-fashioned bug to me:

``
if (event_notifier_init(>e, false) < 0) {
goto out_free_state;
}

if (io_setup(MAX_EVENTS, >ctx) != 0) {
goto out_close_efd;
}

...

out_close_efd:
event_notifier_cleanup(>e);
out_free_state:
g_free(s);
return NULL;
``

event_notifier_init looks like it is... usually(?) going to return
-errno on error, and the man page for io_setup suggests that the libaio
wrapper for io_setup is supposed to return -errno if it fails; we could
probably hold onto that error code.

We probably ought to forward those error codes up to the caller; maybe:

int laio_init(LinuxAioState **linux_aio);

> I'm not sure what is appropriate if laio_init() returns NULL, returning
> NULL back to the caller of aio_get_linux_aio() has its own issues, because
> those callers don't seem to check its return value either.
> 

If laio_init can return NULL,  aio_get_linux_aio needs to check for that.

Then callers of that ought to be amended to return some kind of error:

raw_co_prw can return whatever laio_init returned as an error code.
raw_aio_plug doesn't have a way to communicate errors, yet...
raw_aio_unplug doesn't have an error reporting mechanism either.

Those last two are defined here:

.bdrv_io_plug = raw_aio_plug,
.bdrv_io_unplug = raw_aio_unplug,

but it looks like bdrv_io_un|plug doesn't return errors either.

bdrv_io_plug is called by both blk_io_plug and itself, but has no other
callers.

blk_io_plug is called in two places:

virtio_scsi_handle_cmd_req_prepare, which can report -errno error codes
(so we can amend blk_io_plug and bdrv_io_plug to just bubble up errors), and

virtio_blk_handle_vq -- which doesn't appear to be capable of handling
an error, just indicating "progress".

unplug has three usages, one in virtio-blk and two in virtio-scsi, all
of which only appear to check for "progress" and don't seem to have
explicit error reporting.

Probably unplug can't actually fail like this though -- I'm assuming by
the name that it must occur after a call to plug and that will have
memoized the call to aio_get_linux_aio by then.

So... probably what we need to do here is take a look at the virtio-blk
usage of plug and do error-plumbing as necessary.

Wanna send a patch?

--js

> Thanks in advance!
> -Nish
> 

Re: [Qemu-block] [Qemu-devel] [PATCH 2/2] sheepdog: remove huge BSS object

[Philippe Mathieu-Daudé]

On 05/22/2018 10:43 PM, Fam Zheng wrote:
> On Tue, 05/22 22:10, Paolo Bonzini wrote:
>> block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
>> section.  Replace it with a heap-allocated block, and make it smaller too
>> since only the inode header is actually being used.
>>
>> bss size goes down from 4464280 to 269976.
>>
>> Signed-off-by: Paolo Bonzini 
>> ---
>>  block/sheepdog.c | 21 +++--
>>  1 file changed, 11 insertions(+), 10 deletions(-)
>>
>> diff --git a/block/sheepdog.c b/block/sheepdog.c
>> index 23cf5a8430..2068166e3e 100644
>> --- a/block/sheepdog.c
>> +++ b/block/sheepdog.c
>> @@ -2940,13 +2940,14 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>  QEMUSnapshotInfo *sn_tab = NULL;
>>  unsigned wlen, rlen;
>>  int found = 0;
>> -static SheepdogInode inode;
>> +SheepdogInode *inode;
>>  unsigned long *vdi_inuse;
>>  unsigned int start_nr;
>>  uint64_t hval;
>>  uint32_t vid;
>>  
>>  vdi_inuse = g_malloc(max);
>> +inode = g_malloc(SD_INODE_HEADER_SIZE);
> 
> Should you g_free() it before returning?

Oops good catch

> 
> 
>>  
>>  fd = connect_to_sdog(s, _err);
>>  if (fd < 0) {
>> @@ -2989,7 +2990,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>  }
>>  
>>  /* we don't need to read entire object */
>> -ret = read_object(fd, s->bs, (char *),
>> +ret = read_object(fd, s->bs, (char *)inode,
>>vid_to_vdi_oid(vid),
>>0, SD_INODE_HEADER_SIZE, 0,
>>s->cache_flags);
>> @@ -2998,17 +2999,17 @@ static int sd_snapshot_list(BlockDriverState *bs, 
>> QEMUSnapshotInfo **psn_tab)
>>  continue;
>>  }
>>  
>> -if (!strcmp(inode.name, s->name) && is_snapshot()) {
>> -sn_tab[found].date_sec = inode.snap_ctime >> 32;
>> -sn_tab[found].date_nsec = inode.snap_ctime & 0x;
>> -sn_tab[found].vm_state_size = inode.vm_state_size;
>> -sn_tab[found].vm_clock_nsec = inode.vm_clock_nsec;
>> +if (!strcmp(inode->name, s->name) && is_snapshot(inode)) {
>> +sn_tab[found].date_sec = inode->snap_ctime >> 32;
>> +sn_tab[found].date_nsec = inode->snap_ctime & 0x;
>> +sn_tab[found].vm_state_size = inode->vm_state_size;
>> +sn_tab[found].vm_clock_nsec = inode->vm_clock_nsec;
>>  
>>  snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str),
>> - "%" PRIu32, inode.snap_id);
>> + "%" PRIu32, inode->snap_id);
>>  pstrcpy(sn_tab[found].name,
>> -MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
>> -inode.tag);
>> +MIN(sizeof(sn_tab[found].name), sizeof(inode->tag)),
>> +inode->tag);
>>  found++;
>>  }
>>  }
>> -- 
>> 2.17.0
>>
>>
> 

Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

[Markus Armbruster]

Eduardo Habkost  writes:

> On Wed, May 23, 2018 at 01:19:46PM +0200, Markus Armbruster wrote:
>> Eduardo Habkost  writes:
>> 
>> > On Mon, May 21, 2018 at 07:44:40PM +0100, Daniel P. Berrangé wrote:
>> >> On Mon, May 21, 2018 at 03:29:28PM -0300, Eduardo Habkost wrote:
>> >> > On Sat, May 19, 2018 at 08:05:06AM +0200, Markus Armbruster wrote:
>> >> > > Eduardo Habkost  writes:
>> >> > > 
>> >> > > [...]
>> >> > > > About being more expressive than just a single list of key,value
>> >> > > > pairs, I don't see any evidence of that being necessary for the
>> >> > > > problems we're trying to address.
>> >> > > 
>> >> > > Short history of a configuration format you might have encountered:
>> >> 
>> >> [snip]
>> >> 
>> >> > > How confident are we a single list of (key, value) is really all we're
>> >> > > going to need?
>> >> > > 
>> >> > > Even if we think it is, would it be possible to provide for a future
>> >> > > extension to trees at next to no cost?
>> >> > 
>> >> > I'm confident that a list of key,values is all we need for the
>> >> > current problem.
>> >> 
>> >> I'm not convinced. A disk image may work with Q35 or i440fx,  or
>> >> work with any of virtio, ide or sata disk. So that already means
>> >> values have to be arrays, not scalars. You could do that with a
>> >> simple key,value list, but only by defining a mapping of arrays
>> >> into a flattened form. eg do we allow repeated keys, or do we
>> >> allow array indexes on keys. 
>> >
>> > No problem, we can support trees if it's necessary.
>> >
>> >
>> >> > The point here is to allow users to simply copy an existing disk
>> >> > image, and it will contain enough hints for a cloud stack to
>> >> > choose reasonable defaults for machine-type and disk type
>> >> > automatically.  Requiring the user to perform a separate step to
>> >> > encapsulate the disk image in another file format defeats the
>> >> > whole purpose of the proposal.
>> >> 
>> >> It doesn't have to mean more work for the user - the application
>> >> that is used to create the image can do that on their behalf.
>> >> oVirt for example can import/export OVA files, containing OVF
>> >> metadata. I could imagine virt-manager, and other tools adding
>> >> export ability without much trouble if this was deemed a desirable
>> >> thing. Bundling gives ability to have multiple disk images in one
>> >> archive, which is something OVF does.
>> >
>> > I have the impression that "the application that is used to
>> > create the image" is a very large set.  It can be virt-manager,
>> > virt-install, virt-manager, or even QEMU itself.
>> >
>> > Today people can simply create a VM on virt-manager, or run QEMU
>> > manually, and upload the qcow2 image directly from its original
>> > location (they don't need to copy/export it).  Don't we want the
>> > same procedure to keep working instead of requiring users to use
>> > another tool?
>> 
>> Today, I can take the disk out of my old computer, put it into my new
>> computer, and it just works.  Don't we want the same procedure to keep
>> working forever?
>
> I don't think the comparison is fair: downloading hard disk
> images for bare metal hardware is not as common as downloading
> guest disk images for cloud infrastructure.

Can't let "fair" get in the way of a witticism!

Seriously, though: are disk images a sane way to package software?

>> Sadly, wanting something badly enough doesn't make actual solutions any
>> easier :)
>
> This part is true.  :)
>
>> 
>> My point is: disk images (real or virtual) keep working in different
>> hardware contexts by a mixture of flexibility built into system software
>> on the image, disciplined evolution of hardware (real or virtual), and
>> dumb luck.  It works until it doesn't.  And then you get to tinker.
>
> Personally, I believe that tools for running and managing virtual
> hardware can and should be smarter than real hardware.

Point taken.

Adding hints to disk images (which is how I understand your proposal)
could make them (with a bit of luck) work more often.  Luck, because the
receiving end needs to interpret the hints in a way that makes the image
work.  In other words, guesswork and duct tape.  Both guesswork and duct
tape are incredibly useful when no better tools are at hand.  Is that
the case here?

>> With OVF, you solve the problem further up the stack: you do virtual
>> appliances instead of disk images.
>> 
>
> I guess the main problem is that people are already using disk
> images as if they were virtual appliances.
>
> We can tell people to stop doing that and use OVF, but then we
> won't make anybody's life any easier: publishers of images might
> need to generate both qcow2 and OVF images if they want it to
> work with older hosts; consumers will need to find out if they
> need qcow2 or OVF.

I'm afraid providing for "hints" in QCOW2 could only add problems.  To
pick the right hints, publishers need to predict how future 

Re: [Qemu-block] [PATCH v2 0/2] sheepdog: remove huge BSS object

[Jeff Cody]

On Wed, May 23, 2018 at 06:07:19PM +0200, Paolo Bonzini wrote:
> block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
> section.  Since it doesn't really have to be static, we can just use a
> heap allocated block.  We can actually make it smaller too. :)
> 
> Patch 1 is a related cleanup since we're touching that area of the code.
> 
> Paolo
> 
> v1->v2: free the data...
> 
> Paolo Bonzini (2):
>   sheepdog: cleanup repeated expression
>   sheepdog: remove huge BSS object
> 
>  block/sheepdog.c | 27 ++-
>  1 file changed, 14 insertions(+), 13 deletions(-)
> 
> -- 
> 2.17.0
> 

Thanks,

Applied to my block branch:

git://github.com/codyprime/qemu-kvm-jtc block

-Jeff

[Qemu-block] [PATCH 1/2] sheepdog: cleanup repeated expression

[Paolo Bonzini]

The expression "SD_INODE_SIZE - sizeof(inode.data_vdi_id)" already has a macro
defined for the same value (though with a nicer definition using offsetof).
Replace it.

Reviewed-by: Fam Zheng 
Reviewed-by: Jeff Cody 
Signed-off-by: Paolo Bonzini 
---
 block/sheepdog.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 4237132419..23cf5a8430 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2337,7 +2337,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t 
offset,
 }
 
 /* we don't need to update entire object */
-datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
+datalen = SD_INODE_HEADER_SIZE;
 s->inode.vdi_size = offset;
 ret = write_object(fd, s->bs, (char *)>inode,
vid_to_vdi_oid(s->inode.vdi_id), s->inode.nr_copies,
@@ -2705,7 +2705,7 @@ static int sd_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
  */
 strncpy(s->inode.tag, sn_info->name, sizeof(s->inode.tag));
 /* we don't need to update entire object */
-datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
+datalen = SD_INODE_HEADER_SIZE;
 inode = g_malloc(datalen);
 
 /* refresh inode. */
@@ -2991,7 +2991,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 /* we don't need to read entire object */
 ret = read_object(fd, s->bs, (char *),
   vid_to_vdi_oid(vid),
-  0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0,
+  0, SD_INODE_HEADER_SIZE, 0,
   s->cache_flags);
 
 if (ret) {
-- 
2.17.0

[Qemu-block] [PATCH 2/2] sheepdog: remove huge BSS object

[Paolo Bonzini]

block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
section.  Replace it with a heap-allocated block, and make it smaller too
since only the inode header is actually being used.

bss size goes down from 4464280 to 269976.

Signed-off-by: Paolo Bonzini 
---
 block/sheepdog.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 23cf5a8430..2068166e3e 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2940,13 +2940,14 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 QEMUSnapshotInfo *sn_tab = NULL;
 unsigned wlen, rlen;
 int found = 0;
-static SheepdogInode inode;
+SheepdogInode *inode;
 unsigned long *vdi_inuse;
 unsigned int start_nr;
 uint64_t hval;
 uint32_t vid;
 
 vdi_inuse = g_malloc(max);
+inode = g_malloc(SD_INODE_HEADER_SIZE);
 
 fd = connect_to_sdog(s, _err);
 if (fd < 0) {
@@ -2989,7 +2990,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 }
 
 /* we don't need to read entire object */
-ret = read_object(fd, s->bs, (char *),
+ret = read_object(fd, s->bs, (char *)inode,
   vid_to_vdi_oid(vid),
   0, SD_INODE_HEADER_SIZE, 0,
   s->cache_flags);
@@ -2998,17 +2999,17 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 continue;
 }
 
-if (!strcmp(inode.name, s->name) && is_snapshot()) {
-sn_tab[found].date_sec = inode.snap_ctime >> 32;
-sn_tab[found].date_nsec = inode.snap_ctime & 0x;
-sn_tab[found].vm_state_size = inode.vm_state_size;
-sn_tab[found].vm_clock_nsec = inode.vm_clock_nsec;
+if (!strcmp(inode->name, s->name) && is_snapshot(inode)) {
+sn_tab[found].date_sec = inode->snap_ctime >> 32;
+sn_tab[found].date_nsec = inode->snap_ctime & 0x;
+sn_tab[found].vm_state_size = inode->vm_state_size;
+sn_tab[found].vm_clock_nsec = inode->vm_clock_nsec;
 
 snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str),
- "%" PRIu32, inode.snap_id);
+ "%" PRIu32, inode->snap_id);
 pstrcpy(sn_tab[found].name,
-MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
-inode.tag);
+MIN(sizeof(sn_tab[found].name), sizeof(inode->tag)),
+inode->tag);
 found++;
 }
 }
@@ -3019,6 +3019,7 @@ out:
 *psn_tab = sn_tab;
 
 g_free(vdi_inuse);
+g_free(inode);
 
 if (ret < 0) {
 return ret;
-- 
2.17.0

[Qemu-block] [PATCH v2 0/2] sheepdog: remove huge BSS object

[Paolo Bonzini]

block/sheepdog.o has a 4M static variable that is 90% of QEMU's whole .bss
section.  Since it doesn't really have to be static, we can just use a
heap allocated block.  We can actually make it smaller too. :)

Patch 1 is a related cleanup since we're touching that area of the code.

Paolo

v1->v2: free the data...

Paolo Bonzini (2):
  sheepdog: cleanup repeated expression
  sheepdog: remove huge BSS object

 block/sheepdog.c | 27 ++-
 1 file changed, 14 insertions(+), 13 deletions(-)

-- 
2.17.0

Re: [Qemu-block] [PATCH 3/4] nbd/client: Support requests of additional block sizing info

[Eric Blake]

On 05/22/2018 10:33 AM, Vladimir Sementsov-Ogievskiy wrote:

02.05.2018 00:13, Eric Blake wrote:

The NBD spec is clarifying [1] that a server may want to advertise
different limits for READ/WRITE (in our case, 32M) than for
TRIM/ZERO (in our case, nearly 4G).  Implement the client
side support for these alternate limits, by always requesting
the new information (a compliant server must ignore the
request if it is unknown), and by validating anything reported
by the server before populating the block layer limits.


I still need to do a v2 of this series based on feedback from the NBD 
list, but answering your question in the meantime:




  bs->bl.request_alignment = min ? min : BDRV_SECTOR_SIZE;
-    bs->bl.max_pdiscard = max;
-    bs->bl.max_pwrite_zeroes = max;
+    if (s->info.max_trim) {
+    bs->bl.max_pdiscard = MIN(s->info.max_trim, 
BDRV_REQUEST_MAX_BYTES);

+    } else {
+    bs->bl.max_pdiscard = max;
+    }
+    bs->bl.pdiscard_alignment = s->info.min_trim;
+    if (s->info.max_zero) {
+    bs->bl.max_pwrite_zeroes = MIN(s->info.max_zero,
+   BDRV_REQUEST_MAX_BYTES);
+    } else {
+    bs->bl.max_pwrite_zeroes = max;
+    }
+    bs->bl.pwrite_zeroes_alignment = s->info.min_zero;
  bs->bl.max_transfer = max;


Should not max_transfer be updated too? Looks like it limits all 
requests, is it right?


max_transfer affects read and write requests, but not write_zero 
requests (where max_pwrite_zeroes is used instead) or trim requests 
(where max_pdiscard is used instead).  Which is precisely the semantics 
we want - the NBD extension is mirroring the fact that qemu already has 
a way to track independent limits for trim/zero that can be larger than 
the normal limits for read/write, because trim/zero do not involve a 
transfer of a large data buffer.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-block] [PATCH 1/2] sheepdog: cleanup repeated expression

[Jeff Cody]

On Tue, May 22, 2018 at 10:10:55PM +0200, Paolo Bonzini wrote:
> The expression "SD_INODE_SIZE - sizeof(inode.data_vdi_id)" already has a macro
> defined for the same value (though with a nicer definition using offsetof).
> Replace it.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  block/sheepdog.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 4237132419..23cf5a8430 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -2337,7 +2337,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t 
> offset,
>  }
>  
>  /* we don't need to update entire object */
> -datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
> +datalen = SD_INODE_HEADER_SIZE;
>  s->inode.vdi_size = offset;
>  ret = write_object(fd, s->bs, (char *)>inode,
> vid_to_vdi_oid(s->inode.vdi_id), s->inode.nr_copies,
> @@ -2705,7 +2705,7 @@ static int sd_snapshot_create(BlockDriverState *bs, 
> QEMUSnapshotInfo *sn_info)
>   */
>  strncpy(s->inode.tag, sn_info->name, sizeof(s->inode.tag));
>  /* we don't need to update entire object */
> -datalen = SD_INODE_SIZE - sizeof(s->inode.data_vdi_id);
> +datalen = SD_INODE_HEADER_SIZE;
>  inode = g_malloc(datalen);
>  
>  /* refresh inode. */
> @@ -2991,7 +2991,7 @@ static int sd_snapshot_list(BlockDriverState *bs, 
> QEMUSnapshotInfo **psn_tab)
>  /* we don't need to read entire object */
>  ret = read_object(fd, s->bs, (char *),
>vid_to_vdi_oid(vid),
> -  0, SD_INODE_SIZE - sizeof(inode.data_vdi_id), 0,
> +  0, SD_INODE_HEADER_SIZE, 0,
>s->cache_flags);
>  
>  if (ret) {
> -- 
> 2.17.0
> 
> 

Reviewed-by: Jeff Cody 

Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

[Michael S. Tsirkin]

On Wed, May 23, 2018 at 11:16:04AM +0200, Kevin Wolf wrote:
> Am 23.05.2018 um 04:12 hat Fam Zheng geschrieben:
> > On Tue, 05/22 17:02, Kevin Wolf wrote:
> > > Am 22.05.2018 um 16:19 hat Michael S. Tsirkin geschrieben:
> > > > On Tue, May 22, 2018 at 09:35:55AM +0200, Gerd Hoffmann wrote:
> > > > >   Hi,
> > > > > 
> > > > > > You must /sometimes/ supply the correct machine type.
> > > > > > 
> > > > > > It is quite dependent on the guest OS you have installed, and even
> > > > > > just how the guest OS is configured.  In general Linux is very
> > > > > > flexible and can adapt to a wide range of hardware, automatically
> > > > > > detecting things as needed. It is possible for a sysadmin to build
> > > > > > a Linux image in a way that would only work with I440FX, but I
> > > > > > don't think it would be common to see that.
> > > > > 
> > > > > I think it would be pretty hard to actually build such an image.
> > > > > 
> > > > > The more critical thing for linux guests is the storage driver which
> > > > > must be included into the initrd so the image can mount the root
> > > > > filesystem.  And the firmware, bios vs. uefi is more critical than
> > > > > pc vs. q35.
> > > > 
> > > > I think we can start by finding a location to embed a string in a qcow
> > > > image, add ability for qemu-img to set and get this string.  We can
> > > > discuss how it's formatted separately.
> > > 
> > > If we want it, we'll find a place to store it.
> > > 
> > > But the first thing we need is a spec for what's actually in it. Just
> > > storing a machine type hint would be a one-off hack that wouldn't last
> > > very long before we want to add the next thing.
> > > 
> > > Essentially, what we need is a description of the virtual machine that
> > > we suggest to use with this image. We can try to reuse something
> > > existing there, like libvirt XML or OVF, or invent something new (a JSON
> > > array describing runtime options?). One difference to existing formats
> > > is probably that we want only frontends and no backends in the
> > > description.
> > > 
> > 
> > Do we really need a uniform way and require compliance to the standard we
> > choose, and implement verification in the block driver, or can we get away 
> > with
> > a description field that accepts any text and leave it to the user to decide
> > what to put there? In the header we could assign a Content-type field that
> > defaults to 'text/plain' to the description, that way apps can mark the 
> > data as
> > "application/ovf" if they want, or whatever the upper layer decides.
> 
> Yes, we can. But I'm not sure if I want. Providing low-level features
> without telling users how they are supposed to be used usually results
> in a big surprise for both sides eventually.
> 
> Kevin

The idea to include a format in there sounds very reasonable to me
though. We can then start with a simple text format just showing the
QEMU command line, and others can reuse it for OVF format, etc.

-- 
MST

[Qemu-block] [PULL 41/46] job: Add JOB_STATUS_CHANGE QMP event

[Kevin Wolf]

This adds a QMP event that is emitted whenever a job transitions from
one status to another.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 qapi/job.json |  14 
 job.c |  10 +++
 tests/qemu-iotests/030|  17 +++-
 tests/qemu-iotests/040|   2 +
 tests/qemu-iotests/041|  17 +++-
 tests/qemu-iotests/094.out|   7 ++
 tests/qemu-iotests/095|   2 +-
 tests/qemu-iotests/095.out|   6 ++
 tests/qemu-iotests/109|   2 +-
 tests/qemu-iotests/109.out| 178 --
 tests/qemu-iotests/124|   8 ++
 tests/qemu-iotests/127.out|   7 ++
 tests/qemu-iotests/141|  13 ++-
 tests/qemu-iotests/141.out|  29 +++
 tests/qemu-iotests/144|   2 +-
 tests/qemu-iotests/144.out|   7 ++
 tests/qemu-iotests/156|   2 +-
 tests/qemu-iotests/156.out|   7 ++
 tests/qemu-iotests/185|  12 +--
 tests/qemu-iotests/185.out|  10 +++
 tests/qemu-iotests/191|   4 +-
 tests/qemu-iotests/191.out| 132 +++
 tests/qemu-iotests/iotests.py |   5 ++
 23 files changed, 449 insertions(+), 44 deletions(-)

diff --git a/qapi/job.json b/qapi/job.json
index a472c0cb29..9fbdd0ccd9 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -92,3 +92,17 @@
 { 'enum': 'JobVerb',
   'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete', 'dismiss',
'finalize' ] }
+
+##
+# @JOB_STATUS_CHANGE:
+#
+# Emitted when a job transitions to a different status.
+#
+# @id: The job identifier
+# @status: The new job status
+#
+# Since: 2.13
+##
+{ 'event': 'JOB_STATUS_CHANGE',
+  'data': { 'id': 'str',
+'status': 'JobStatus' } }
diff --git a/job.c b/job.c
index 2046d2f9d3..599a1042cf 100644
--- a/job.c
+++ b/job.c
@@ -30,6 +30,7 @@
 #include "qemu/id.h"
 #include "qemu/main-loop.h"
 #include "trace-root.h"
+#include "qapi/qapi-events-job.h"
 
 static QLIST_HEAD(, Job) jobs = QLIST_HEAD_INITIALIZER(jobs);
 
@@ -157,6 +158,11 @@ static int job_txn_apply(JobTxn *txn, int fn(Job *), bool 
lock)
 return rc;
 }
 
+static bool job_is_internal(Job *job)
+{
+return (job->id == NULL);
+}
+
 static void job_state_transition(Job *job, JobStatus s1)
 {
 JobStatus s0 = job->status;
@@ -166,6 +172,10 @@ static void job_state_transition(Job *job, JobStatus s1)
JobStatus_str(s0), JobStatus_str(s1));
 assert(JobSTT[s0][s1]);
 job->status = s1;
+
+if (!job_is_internal(job) && s1 != s0) {
+qapi_event_send_job_status_change(job->id, job->status, _abort);
+}
 }
 
 int job_apply_verb(Job *job, JobVerb verb, Error **errp)
diff --git a/tests/qemu-iotests/030 b/tests/qemu-iotests/030
index 640a6dfd10..1dbc2ddc49 100755
--- a/tests/qemu-iotests/030
+++ b/tests/qemu-iotests/030
@@ -304,8 +304,7 @@ class TestParallelOps(iotests.QMPTestCase):
 result = self.vm.qmp('block-stream', device='node5', 
base=self.imgs[3], job_id='stream-node6')
 self.assert_qmp(result, 'error/class', 'GenericError')
 
-event = self.vm.get_qmp_event(wait=True)
-self.assertEqual(event['event'], 'BLOCK_JOB_READY')
+event = self.vm.event_wait(name='BLOCK_JOB_READY')
 self.assert_qmp(event, 'data/device', 'commit-drive0')
 self.assert_qmp(event, 'data/type', 'commit')
 self.assert_qmp_absent(event, 'data/error')
@@ -565,6 +564,8 @@ class TestEIO(TestErrors):
 self.assert_qmp(event, 'data/offset', 
self.STREAM_BUFFER_SIZE)
 self.assert_qmp(event, 'data/len', self.image_len)
 completed = True
+elif event['event'] == 'JOB_STATUS_CHANGE':
+self.assert_qmp(event, 'data/id', 'drive0')
 
 self.assert_no_active_block_jobs()
 self.vm.shutdown()
@@ -596,6 +597,8 @@ class TestEIO(TestErrors):
 self.assert_qmp(event, 'data/offset', self.image_len)
 self.assert_qmp(event, 'data/len', self.image_len)
 completed = True
+elif event['event'] == 'JOB_STATUS_CHANGE':
+self.assert_qmp(event, 'data/id', 'drive0')
 
 self.assert_no_active_block_jobs()
 self.vm.shutdown()
@@ -637,6 +640,8 @@ class TestEIO(TestErrors):
 self.assert_qmp(event, 'data/offset', self.image_len)
 self.assert_qmp(event, 'data/len', self.image_len)
 completed = True
+elif event['event'] == 'JOB_STATUS_CHANGE':
+self.assert_qmp(event, 'data/id', 'drive0')
 
 self.assert_no_active_block_jobs()
 self.vm.shutdown()
@@ -663,6 +668,8 @@ class TestEIO(TestErrors):
 self.assert_qmp(event, 'data/offset', 
self.STREAM_BUFFER_SIZE)
 self.assert_qmp(event, 'data/len', self.image_len)
 

[Qemu-block] [PULL 44/46] blockjob: Remove BlockJob.driver

[Kevin Wolf]

BlockJob.driver is redundant with Job.driver and only used in very few
places any more. Remove it.

Signed-off-by: Kevin Wolf 
---
 include/block/blockjob.h |  3 ---
 blockjob.c   | 17 ++---
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 3021d11126..32c00b7dc0 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -43,9 +43,6 @@ typedef struct BlockJob {
 /** Data belonging to the generic Job infrastructure */
 Job job;
 
-/** The job type, including the job vtable.  */
-const BlockJobDriver *driver;
-
 /** The block device on which the job is operating.  */
 BlockBackend *blk;
 
diff --git a/blockjob.c b/blockjob.c
index 5c8ff6f846..0306533a2e 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -103,10 +103,12 @@ static void block_job_attached_aio_context(AioContext 
*new_context,
void *opaque)
 {
 BlockJob *job = opaque;
+const JobDriver *drv = job->job.driver;
+BlockJobDriver *bjdrv = container_of(drv, BlockJobDriver, job_driver);
 
 job->job.aio_context = new_context;
-if (job->driver->attached_aio_context) {
-job->driver->attached_aio_context(job, new_context);
+if (bjdrv->attached_aio_context) {
+bjdrv->attached_aio_context(job, new_context);
 }
 
 job_resume(>job);
@@ -115,10 +117,12 @@ static void block_job_attached_aio_context(AioContext 
*new_context,
 void block_job_drain(Job *job)
 {
 BlockJob *bjob = container_of(job, BlockJob, job);
+const JobDriver *drv = job->driver;
+BlockJobDriver *bjdrv = container_of(drv, BlockJobDriver, job_driver);
 
 blk_drain(bjob->blk);
-if (bjob->driver->drain) {
-bjob->driver->drain(bjob);
+if (bjdrv->drain) {
+bjdrv->drain(bjob);
 }
 }
 
@@ -201,7 +205,7 @@ bool block_job_is_internal(BlockJob *job)
 
 const BlockJobDriver *block_job_driver(BlockJob *job)
 {
-return job->driver;
+return container_of(job->job.driver, BlockJobDriver, job_driver);
 }
 
 /* Assumes the job_mutex is held */
@@ -386,8 +390,7 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 assert(job->job.driver->user_resume == _job_user_resume);
 assert(job->job.driver->drain == _job_drain);
 
-job->driver= driver;
-job->blk   = blk;
+job->blk = blk;
 
 job->finalize_cancelled_notifier.notify = block_job_event_cancelled;
 job->finalize_completed_notifier.notify = block_job_event_completed;
-- 
2.13.6

[Qemu-block] [PULL 46/46] qemu-iotests: Test job-* with block jobs

[Kevin Wolf]

This adds a test case that tests the new job-* QMP commands with
mirror and backup block jobs.

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/219 | 209 +
 tests/qemu-iotests/219.out | 327 +
 tests/qemu-iotests/group   |   1 +
 3 files changed, 537 insertions(+)
 create mode 100755 tests/qemu-iotests/219
 create mode 100644 tests/qemu-iotests/219.out

diff --git a/tests/qemu-iotests/219 b/tests/qemu-iotests/219
new file mode 100755
index 00..898a26eef0
--- /dev/null
+++ b/tests/qemu-iotests/219
@@ -0,0 +1,209 @@
+#!/usr/bin/env python
+#
+# Copyright (C) 2018 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+# Creator/Owner: Kevin Wolf 
+#
+# Check using the job-* QMP commands with block jobs
+
+import iotests
+
+iotests.verify_image_format(supported_fmts=['qcow2'])
+
+def pause_wait(vm, job_id):
+with iotests.Timeout(3, "Timeout waiting for job to pause"):
+while True:
+result = vm.qmp('query-jobs')
+for job in result['return']:
+if job['id'] == job_id and job['status'] in ['paused', 
'standby']:
+return job
+
+# Test that block-job-pause/resume and job-pause/resume can be mixed
+def test_pause_resume(vm):
+for pause_cmd, pause_arg in [('block-job-pause', 'device'),
+ ('job-pause', 'id')]:
+for resume_cmd, resume_arg in [('block-job-resume', 'device'),
+   ('job-resume', 'id')]:
+iotests.log('=== Testing %s/%s ===' % (pause_cmd, resume_cmd))
+
+iotests.log(vm.qmp(pause_cmd, **{pause_arg: 'job0'}))
+pause_wait(vm, 'job0')
+
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+iotests.log(vm.qmp('query-jobs'))
+
+iotests.log(vm.qmp(resume_cmd, **{resume_arg: 'job0'}))
+
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+iotests.log(vm.qmp('query-jobs'))
+
+def test_job_lifecycle(vm, job, job_args, has_ready=False):
+iotests.log('')
+iotests.log('')
+iotests.log('Starting block job: %s (auto-finalize: %s; auto-dismiss: %s)' 
%
+(job,
+ job_args.get('auto-finalize', True),
+ job_args.get('auto-dismiss', True)))
+iotests.log(vm.qmp(job, job_id='job0', **job_args))
+
+# Depending on the storage, the first request may or may not have completed
+# yet, so filter out the progress. Later query-job calls don't need the
+# filtering because the progress is made deterministic by the block job
+# speed
+result = vm.qmp('query-jobs')
+for j in result['return']:
+del j['current-progress']
+iotests.log(result)
+
+# undefined -> created -> running
+iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+
+# RUNNING state:
+# pause/resume should work, complete/finalize/dismiss should error out
+iotests.log('')
+iotests.log('Pause/resume in RUNNING')
+test_pause_resume(vm)
+
+iotests.log(vm.qmp('job-complete', id='job0'))
+iotests.log(vm.qmp('job-finalize', id='job0'))
+iotests.log(vm.qmp('job-dismiss', id='job0'))
+
+iotests.log(vm.qmp('block-job-complete', device='job0'))
+iotests.log(vm.qmp('block-job-finalize', id='job0'))
+iotests.log(vm.qmp('block-job-dismiss', id='job0'))
+
+# Let the job complete (or transition to READY if it supports that)
+iotests.log(vm.qmp('block-job-set-speed', device='job0', speed=0))
+if has_ready:
+iotests.log('')
+iotests.log('Waiting for READY state...')
+vm.event_wait('BLOCK_JOB_READY')
+
iotests.log(iotests.filter_qmp_event(vm.event_wait('JOB_STATUS_CHANGE')))
+iotests.log(vm.qmp('query-jobs'))
+
+# READY state:
+# pause/resume/complete should work, finalize/dismiss should error out
+iotests.log('')
+iotests.log('Pause/resume in READY')
+test_pause_resume(vm)
+
+iotests.log(vm.qmp('job-finalize', id='job0'))
+iotests.log(vm.qmp('job-dismiss', id='job0'))
+
+iotests.log(vm.qmp('block-job-finalize', id='job0'))
+

[Qemu-block] [PULL 45/46] iotests: Move qmp_to_opts() to VM

[Kevin Wolf]

qmp_to_opts() used to be a method of QMPTestCase, but recently we
started to add more Python test cases that don't make use of
QMPTestCase. In order to make the method usable there, move it to VM.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 tests/qemu-iotests/041|  6 +++---
 tests/qemu-iotests/155|  2 +-
 tests/qemu-iotests/iotests.py | 45 ++-
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041
index e94587950c..c20ac7da87 100755
--- a/tests/qemu-iotests/041
+++ b/tests/qemu-iotests/041
@@ -1030,9 +1030,9 @@ class TestOrphanedSource(iotests.QMPTestCase):
  'read-only': 'on' }
 
 self.vm = iotests.VM()
-self.vm.add_blockdev(self.qmp_to_opts(blk0))
-self.vm.add_blockdev(self.qmp_to_opts(blk1))
-self.vm.add_blockdev(self.qmp_to_opts(blk2))
+self.vm.add_blockdev(self.vm.qmp_to_opts(blk0))
+self.vm.add_blockdev(self.vm.qmp_to_opts(blk1))
+self.vm.add_blockdev(self.vm.qmp_to_opts(blk2))
 self.vm.launch()
 
 def tearDown(self):
diff --git a/tests/qemu-iotests/155 b/tests/qemu-iotests/155
index 42dae04c83..63a5b5e2c0 100755
--- a/tests/qemu-iotests/155
+++ b/tests/qemu-iotests/155
@@ -63,7 +63,7 @@ class BaseClass(iotests.QMPTestCase):
 'driver': iotests.imgfmt,
 'file': {'driver': 'file',
  'filename': source_img}}
-self.vm.add_blockdev(self.qmp_to_opts(blockdev))
+self.vm.add_blockdev(self.vm.qmp_to_opts(blockdev))
 self.vm.add_device('virtio-blk,id=qdev0,drive=source')
 self.vm.launch()
 
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ed08e06e1f..28159d837a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -363,6 +363,27 @@ class VM(qtest.QEMUQtestMachine):
 return self.qmp('human-monitor-command',
 command_line='qemu-io %s "%s"' % (drive, cmd))
 
+def flatten_qmp_object(self, obj, output=None, basestr=''):
+if output is None:
+output = dict()
+if isinstance(obj, list):
+for i in range(len(obj)):
+self.flatten_qmp_object(obj[i], output, basestr + str(i) + '.')
+elif isinstance(obj, dict):
+for key in obj:
+self.flatten_qmp_object(obj[key], output, basestr + key + '.')
+else:
+output[basestr[:-1]] = obj # Strip trailing '.'
+return output
+
+def qmp_to_opts(self, obj):
+obj = self.flatten_qmp_object(obj)
+output_list = list()
+for key in obj:
+output_list += [key + '=' + obj[key]]
+return ','.join(output_list)
+
+
 
 index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')
 
@@ -390,26 +411,6 @@ class QMPTestCase(unittest.TestCase):
 self.fail('invalid index "%s" in path "%s" in "%s"' % 
(idx, path, str(d)))
 return d
 
-def flatten_qmp_object(self, obj, output=None, basestr=''):
-if output is None:
-output = dict()
-if isinstance(obj, list):
-for i in range(len(obj)):
-self.flatten_qmp_object(obj[i], output, basestr + str(i) + '.')
-elif isinstance(obj, dict):
-for key in obj:
-self.flatten_qmp_object(obj[key], output, basestr + key + '.')
-else:
-output[basestr[:-1]] = obj # Strip trailing '.'
-return output
-
-def qmp_to_opts(self, obj):
-obj = self.flatten_qmp_object(obj)
-output_list = list()
-for key in obj:
-output_list += [key + '=' + obj[key]]
-return ','.join(output_list)
-
 def assert_qmp_absent(self, d, path):
 try:
 result = self.dictpath(d, path)
@@ -444,8 +445,8 @@ class QMPTestCase(unittest.TestCase):
 '''Asserts that the given filename is a json: filename and that its
content is equal to the given reference object'''
 self.assertEqual(json_filename[:5], 'json:')
-
self.assertEqual(self.flatten_qmp_object(json.loads(json_filename[5:])),
- self.flatten_qmp_object(reference))
+
self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),
+ self.vm.flatten_qmp_object(reference))
 
 def cancel_and_wait(self, drive='drive0', force=False, resume=False):
 '''Cancel a block job and wait for it to finish, returning the event'''
-- 
2.13.6

[Qemu-block] [PULL 40/46] job: Introduce qapi/job.json

[Kevin Wolf]

This adds a separate schema file for all job-related definitions that
aren't tied to the block layer.

For a start, move the enums JobType, JobStatus and JobVerb.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
---
 qapi/block-core.json  | 90 +---
 qapi/job.json | 94 +++
 qapi/qapi-schema.json |  1 +
 MAINTAINERS   |  1 +
 Makefile  |  9 +
 Makefile.objs |  4 +++
 6 files changed, 110 insertions(+), 89 deletions(-)
 create mode 100644 qapi/job.json

diff --git a/qapi/block-core.json b/qapi/block-core.json
index bb964b4319..7dfa77a05c 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -6,6 +6,7 @@
 
 { 'include': 'common.json' }
 { 'include': 'crypto.json' }
+{ 'include': 'job.json' }
 { 'include': 'sockets.json' }
 
 ##
@@ -1050,95 +1051,6 @@
   'data': ['top', 'full', 'none', 'incremental'] }
 
 ##
-# @JobType:
-#
-# Type of a background job.
-#
-# @commit: block commit job type, see "block-commit"
-#
-# @stream: block stream job type, see "block-stream"
-#
-# @mirror: drive mirror job type, see "drive-mirror"
-#
-# @backup: drive backup job type, see "drive-backup"
-#
-# Since: 1.7
-##
-{ 'enum': 'JobType',
-  'data': ['commit', 'stream', 'mirror', 'backup'] }
-
-##
-# @JobVerb:
-#
-# Represents command verbs that can be applied to a job.
-#
-# @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
-#
-# @dismiss: see @block-job-dismiss
-#
-# @finalize: see @block-job-finalize
-#
-# Since: 2.12
-##
-{ 'enum': 'JobVerb',
-  'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete', 'dismiss',
-   'finalize' ] }
-
-##
-# @JobStatus:
-#
-# Indicates the present state of a given job in its lifetime.
-#
-# @undefined: Erroneous, default state. Should not ever be visible.
-#
-# @created: The job has been created, but not yet started.
-#
-# @running: The job is currently running.
-#
-# @paused: The job is running, but paused. The pause may be requested by
-#  either the QMP user or by internal processes.
-#
-# @ready: The job is running, but is ready for the user to signal completion.
-# This is used for long-running jobs like mirror that are designed to
-# run indefinitely.
-#
-# @standby: The job is ready, but paused. This is nearly identical to @paused.
-#   The job may return to @ready or otherwise be canceled.
-#
-# @waiting: The job is waiting for other jobs in the transaction to converge
-#   to the waiting state. This status will likely not be visible for
-#   the last job in a transaction.
-#
-# @pending: The job has finished its work, but has finalization steps that it
-#   needs to make prior to completing. These changes may require
-#   manual intervention by the management process if manual was set
-#   to true. These changes may still fail.
-#
-# @aborting: The job is in the process of being aborted, and will finish with
-#an error. The job will afterwards report that it is @concluded.
-#This status may not be visible to the management process.
-#
-# @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': 'JobStatus',
-  'data': ['undefined', 'created', 'running', 'paused', 'ready', 'standby',
-   'waiting', 'pending', 'aborting', 'concluded', 'null' ] }
-
-##
 # @BlockJobInfo:
 #
 # Information about a long-running block device operation.
diff --git a/qapi/job.json b/qapi/job.json
new file mode 100644
index 00..a472c0cb29
--- /dev/null
+++ b/qapi/job.json
@@ -0,0 +1,94 @@
+# -*- Mode: Python -*-
+
+##
+# == Background jobs
+##
+
+##
+# @JobType:
+#
+# Type of a background job.
+#
+# @commit: block commit job type, see "block-commit"
+#
+# @stream: block stream job type, see "block-stream"
+#
+# @mirror: drive mirror job type, see "drive-mirror"
+#
+# @backup: drive backup job type, see "drive-backup"
+#
+# Since: 1.7
+##
+{ 'enum': 'JobType',
+  'data': ['commit', 'stream', 'mirror', 'backup'] }
+
+##
+# @JobStatus:
+#
+# Indicates the present state of a given job in its lifetime.
+#
+# @undefined: Erroneous, default state. Should not ever be visible.
+#
+# @created: The job has been created, but not yet started.
+#
+# @running: The job is currently running.
+#
+# @paused: The job is running, but paused. The pause may be requested by
+#  either the QMP user or by internal processes.
+#
+# @ready: The job is running, but is ready for the user to signal completion.
+# This is used for long-running jobs like 

[Qemu-block] [PULL 37/46] job: Add job_is_ready()

[Kevin Wolf]

Instead of having a 'bool ready' in BlockJob, add a function that
derives its value from the job status.

At the same time, this fixes the behaviour to match what the QAPI
documentation promises for query-block-job: 'true if the job may be
completed'. When the ready flag was introduced in commit ef6dbf1e46e,
the flag never had to be reset to match the description because after
being ready, the jobs would immediately complete and disappear.

Job transactions and manual job finalisation were introduced only later.
With these changes, jobs may stay around even after having completed
(and they are not ready to be completed a second time), however their
patches forgot to reset the ready flag.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  5 -
 include/qemu/job.h   |  3 +++
 blockjob.c   |  3 +--
 job.c| 22 ++
 qemu-img.c   |  2 +-
 tests/test-blockjob.c|  2 +-
 6 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 5a81afc6c3..8e1e1ee0de 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -49,11 +49,6 @@ typedef struct BlockJob {
 /** The block device on which the job is operating.  */
 BlockBackend *blk;
 
-/**
- * Set to true when the job is ready to be completed.
- */
-bool ready;
-
 /** Status that is published by the query-block-jobs QMP API */
 BlockDeviceIoStatus iostatus;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 1e8050c6fb..487f9d9a32 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -367,6 +367,9 @@ bool job_is_cancelled(Job *job);
 /** Returns whether the job is in a completed state. */
 bool job_is_completed(Job *job);
 
+/** Returns whether the job is ready to be completed. */
+bool job_is_ready(Job *job);
+
 /**
  * Request @job to pause at the next pause point. Must be paired with
  * job_resume(). If the job is supposed to be resumed by user action, call
diff --git a/blockjob.c b/blockjob.c
index 3ca009bea5..38f18e9ba3 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -269,7 +269,7 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
 info->offset= job->offset;
 info->speed = job->speed;
 info->io_status = job->iostatus;
-info->ready = job->ready;
+info->ready = job_is_ready(>job),
 info->status= job->job.status;
 info->auto_finalize = job->job.auto_finalize;
 info->auto_dismiss  = job->job.auto_dismiss;
@@ -436,7 +436,6 @@ void block_job_user_resume(Job *job)
 void block_job_event_ready(BlockJob *job)
 {
 job_state_transition(>job, JOB_STATUS_READY);
-job->ready = true;
 
 if (block_job_is_internal(job)) {
 return;
diff --git a/job.c b/job.c
index 7cd3602782..aa4c746c8a 100644
--- a/job.c
+++ b/job.c
@@ -199,6 +199,28 @@ bool job_is_cancelled(Job *job)
 return job->cancelled;
 }
 
+bool job_is_ready(Job *job)
+{
+switch (job->status) {
+case JOB_STATUS_UNDEFINED:
+case JOB_STATUS_CREATED:
+case JOB_STATUS_RUNNING:
+case JOB_STATUS_PAUSED:
+case JOB_STATUS_WAITING:
+case JOB_STATUS_PENDING:
+case JOB_STATUS_ABORTING:
+case JOB_STATUS_CONCLUDED:
+case JOB_STATUS_NULL:
+return false;
+case JOB_STATUS_READY:
+case JOB_STATUS_STANDBY:
+return true;
+default:
+g_assert_not_reached();
+}
+return false;
+}
+
 bool job_is_completed(Job *job)
 {
 switch (job->status) {
diff --git a/qemu-img.c b/qemu-img.c
index efa7b755d4..700e4e187b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -866,7 +866,7 @@ static void run_block_job(BlockJob *job, Error **errp)
 aio_poll(aio_context, true);
 qemu_progress_print(job->len ?
 ((float)job->offset / job->len * 100.f) : 0.0f, 0);
-} while (!job->ready && !job_is_completed(>job));
+} while (!job_is_ready(>job) && !job_is_completed(>job));
 
 if (!job_is_completed(>job)) {
 ret = job_complete_sync(>job, errp);
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index 7131cabb16..8180d03a5f 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -185,7 +185,7 @@ static void coroutine_fn cancel_job_start(void *opaque)
 goto defer;
 }
 
-if (!s->common.ready && s->should_converge) {
+if (!job_is_ready(>common.job) && s->should_converge) {
 block_job_event_ready(>common);
 }
 
-- 
2.13.6

[Qemu-block] [PULL 39/46] job: Move progress fields to Job

[Kevin Wolf]

BlockJob has fields .offset and .len, which are actually misnomers today
because they are no longer tied to block device sizes, but just progress
counters. As such they make a lot of sense in generic Jobs.

This patch moves the fields to Job and renames them to .progress_current
and .progress_total to describe their function better.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h | 25 -
 include/qemu/job.h   | 28 
 block/backup.c   |  8 
 block/commit.c   |  4 ++--
 block/mirror.c   |  4 ++--
 block/stream.c   |  4 ++--
 blockjob.c   | 26 --
 job.c| 10 ++
 qemu-img.c   |  8 ++--
 9 files changed, 62 insertions(+), 55 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 4fca45f6a1..3021d11126 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -52,12 +52,6 @@ typedef struct BlockJob {
 /** Status that is published by the query-block-jobs QMP API */
 BlockDeviceIoStatus iostatus;
 
-/** Offset that is published by the query-block-jobs QMP API */
-int64_t offset;
-
-/** Length that is published by the query-block-jobs QMP API */
-int64_t len;
-
 /** Speed that was set with @block_job_set_speed.  */
 int64_t speed;
 
@@ -139,25 +133,6 @@ void block_job_remove_all_bdrv(BlockJob *job);
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
 
 /**
- * block_job_progress_update:
- * @job: The job that has made progress
- * @done: How much progress the job made
- *
- * Updates the progress counter of the job.
- */
-void block_job_progress_update(BlockJob *job, uint64_t done);
-
-/**
- * block_job_progress_set_remaining:
- * @job: The job whose expected progress end value is set
- * @remaining: Expected end value of the progress counter of the job
- *
- * Sets the expected end value of the progress counter of a job so that a
- * completion percentage can be calculated when the progress is updated.
- */
-void block_job_progress_set_remaining(BlockJob *job, uint64_t remaining);
-
-/**
  * block_job_query:
  * @job: The job to get information about.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index bfc2bc5611..92d1d249fe 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -114,6 +114,16 @@ typedef struct Job {
 /** True if this job should automatically dismiss itself */
 bool auto_dismiss;
 
+/**
+ * Current progress. The unit is arbitrary as long as the ratio between
+ * progress_current and progress_total represents the estimated percentage
+ * of work already done.
+ */
+int64_t progress_current;
+
+/** Estimated progress_current value at the completion of the job */
+int64_t progress_total;
+
 /** ret code passed to job_completed. */
 int ret;
 
@@ -304,6 +314,24 @@ void job_ref(Job *job);
  */
 void job_unref(Job *job);
 
+/**
+ * @job: The job that has made progress
+ * @done: How much progress the job made since the last call
+ *
+ * Updates the progress counter of the job.
+ */
+void job_progress_update(Job *job, uint64_t done);
+
+/**
+ * @job: The job whose expected progress end value is set
+ * @remaining: Missing progress (on top of the current progress counter value)
+ * until the new expected end value is reached
+ *
+ * Sets the expected end value of the progress counter of a job so that a
+ * completion percentage can be calculated when the progress is updated.
+ */
+void job_progress_set_remaining(Job *job, uint64_t remaining);
+
 /** To be called when a cancelled job is finalised. */
 void job_event_cancelled(Job *job);
 
diff --git a/block/backup.c b/block/backup.c
index 6f4f3df229..4e228e959b 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -160,7 +160,7 @@ static int coroutine_fn backup_do_cow(BackupBlockJob *job,
  * offset field is an opaque progress value, it is not a disk offset.
  */
 job->bytes_read += n;
-block_job_progress_update(>common, n);
+job_progress_update(>common.job, n);
 }
 
 out:
@@ -406,8 +406,8 @@ static void 
backup_incremental_init_copy_bitmap(BackupBlockJob *job)
 bdrv_set_dirty_iter(dbi, next_cluster * job->cluster_size);
 }
 
-/* TODO block_job_progress_set_remaining() would make more sense */
-block_job_progress_update(>common,
+/* TODO job_progress_set_remaining() would make more sense */
+job_progress_update(>common.job,
 job->len - hbitmap_count(job->copy_bitmap) * job->cluster_size);
 
 bdrv_dirty_iter_free(dbi);
@@ -425,7 +425,7 @@ static void coroutine_fn backup_run(void *opaque)
 qemu_co_rwlock_init(>flush_rwlock);
 
 nb_clusters = DIV_ROUND_UP(job->len, job->cluster_size);
-block_job_progress_set_remaining(>common, job->len);
+

[Qemu-block] [PULL 35/46] job: Add job_yield()

[Kevin Wolf]

This moves block_job_yield() to the Job layer.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob_int.h |  8 
 include/qemu/job.h   |  9 +++--
 block/backup.c   |  2 +-
 block/mirror.c   |  2 +-
 blockjob.c   | 16 
 job.c| 20 ++--
 tests/test-blockjob-txn.c|  2 +-
 7 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 7df07b20cf..806ac64d87 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -108,14 +108,6 @@ void block_job_user_resume(Job *job);
 void block_job_drain(Job *job);
 
 /**
- * block_job_yield:
- * @job: The job that calls the function.
- *
- * Yield the block job coroutine.
- */
-void block_job_yield(BlockJob *job);
-
-/**
  * block_job_ratelimit_get_delay:
  *
  * Calculate and return delay for the next request in ns. See the documentation
diff --git a/include/qemu/job.h b/include/qemu/job.h
index bbe1b0cd1a..94900ec008 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -339,6 +339,13 @@ void coroutine_fn job_pause_point(Job *job);
 
 /**
  * @job: The job that calls the function.
+ *
+ * Yield the job coroutine.
+ */
+void job_yield(Job *job);
+
+/**
+ * @job: The job that calls the function.
  * @ns: How many nanoseconds to stop for.
  *
  * Put the job to sleep (assuming that it wasn't canceled) for @ns
@@ -508,8 +515,6 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error 
**errp), Error **errp)
 
 /* TODO To be removed from the public interface */
 void job_state_transition(Job *job, JobStatus s1);
-void coroutine_fn job_do_yield(Job *job, uint64_t ns);
-bool job_should_pause(Job *job);
 void job_do_dismiss(Job *job);
 
 #endif
diff --git a/block/backup.c b/block/backup.c
index b13f91d8a7..6f4f3df229 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -444,7 +444,7 @@ static void coroutine_fn backup_run(void *opaque)
 while (!job_is_cancelled(>common.job)) {
 /* Yield until the job is cancelled.  We just let our before_write
  * notify callback service CoW requests. */
-block_job_yield(>common);
+job_yield(>common.job);
 }
 } else if (job->sync_mode == MIRROR_SYNC_MODE_INCREMENTAL) {
 ret = backup_run_incremental(job);
diff --git a/block/mirror.c b/block/mirror.c
index c63cf7cbc9..687f955c22 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -731,7 +731,7 @@ static void coroutine_fn mirror_run(void *opaque)
 block_job_event_ready(>common);
 s->synced = true;
 while (!job_is_cancelled(>common.job) && !s->should_complete) {
-block_job_yield(>common);
+job_yield(>common.job);
 }
 s->common.job.cancelled = false;
 goto immediate_exit;
diff --git a/blockjob.c b/blockjob.c
index 438baa1778..f146fe0cbd 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -431,22 +431,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 return job;
 }
 
-void block_job_yield(BlockJob *job)
-{
-assert(job->job.busy);
-
-/* Check cancellation *before* setting busy = false, too!  */
-if (job_is_cancelled(>job)) {
-return;
-}
-
-if (!job_should_pause(>job)) {
-job_do_yield(>job, -1);
-}
-
-job_pause_point(>job);
-}
-
 void block_job_iostatus_reset(BlockJob *job)
 {
 if (job->iostatus == BLOCK_DEVICE_IO_STATUS_OK) {
diff --git a/job.c b/job.c
index 2e453f60bc..eede6802ae 100644
--- a/job.c
+++ b/job.c
@@ -226,7 +226,7 @@ static bool job_started(Job *job)
 return job->co;
 }
 
-bool job_should_pause(Job *job)
+static bool job_should_pause(Job *job)
 {
 return job->pause_count > 0;
 }
@@ -396,7 +396,7 @@ void job_enter(Job *job)
  *
  * If @ns is (uint64_t) -1, no timer is scheduled and job_enter() must be
  * called explicitly. */
-void coroutine_fn job_do_yield(Job *job, uint64_t ns)
+static void coroutine_fn job_do_yield(Job *job, uint64_t ns)
 {
 job_lock();
 if (ns != -1) {
@@ -441,6 +441,22 @@ void coroutine_fn job_pause_point(Job *job)
 }
 }
 
+void job_yield(Job *job)
+{
+assert(job->busy);
+
+/* Check cancellation *before* setting busy = false, too!  */
+if (job_is_cancelled(job)) {
+return;
+}
+
+if (!job_should_pause(job)) {
+job_do_yield(job, -1);
+}
+
+job_pause_point(job);
+}
+
 void coroutine_fn job_sleep_ns(Job *job, int64_t ns)
 {
 assert(job->busy);
diff --git a/tests/test-blockjob-txn.c b/tests/test-blockjob-txn.c
index 34ee179574..fce836639a 100644
--- a/tests/test-blockjob-txn.c
+++ b/tests/test-blockjob-txn.c
@@ -47,7 +47,7 @@ static void coroutine_fn test_block_job_run(void *opaque)
 if (s->use_timer) {
 job_sleep_ns(>job, 0);
 } else {
-block_job_yield(job);
+

[Qemu-block] [PULL 42/46] job: Add lifecycle QMP commands

[Kevin Wolf]

This adds QMP commands that control the transition between states of the
job lifecycle.

Signed-off-by: Kevin Wolf 
---
 qapi/job.json |  99 +++
 job-qmp.c | 134 ++
 MAINTAINERS   |   1 +
 Makefile.objs |   1 +
 trace-events  |   9 
 5 files changed, 244 insertions(+)
 create mode 100644 job-qmp.c

diff --git a/qapi/job.json b/qapi/job.json
index 9fbdd0ccd9..b84dc6c820 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -106,3 +106,102 @@
 { 'event': 'JOB_STATUS_CHANGE',
   'data': { 'id': 'str',
 'status': 'JobStatus' } }
+
+##
+# @job-pause:
+#
+# Pause an active job.
+#
+# This command returns immediately after marking the active job for pausing.
+# Pausing an already paused job is an error.
+#
+# The job will pause as soon as possible, which means transitioning into the
+# PAUSED state if it was RUNNING, or into STANDBY if it was READY. The
+# corresponding JOB_STATUS_CHANGE event will be emitted.
+#
+# Cancelling a paused job automatically resumes it.
+#
+# @id: The job identifier.
+#
+# Since: 2.13
+##
+{ 'command': 'job-pause', 'data': { 'id': 'str' } }
+
+##
+# @job-resume:
+#
+# Resume a paused job.
+#
+# This command returns immediately after resuming a paused job. Resuming an
+# already running job is an error.
+#
+# @id : The job identifier.
+#
+# Since: 2.13
+##
+{ 'command': 'job-resume', 'data': { 'id': 'str' } }
+
+##
+# @job-cancel:
+#
+# Instruct an active background job to cancel at the next opportunity.
+# This command returns immediately after marking the active job for
+# cancellation.
+#
+# The job will cancel as soon as possible and then emit a JOB_STATUS_CHANGE
+# event. Usually, the status will change to ABORTING, but it is possible that
+# a job successfully completes (e.g. because it was almost done and there was
+# no opportunity to cancel earlier than completing the job) and transitions to
+# PENDING instead.
+#
+# @id: The job identifier.
+#
+# Since: 2.13
+##
+{ 'command': 'job-cancel', 'data': { 'id': 'str' } }
+
+
+##
+# @job-complete:
+#
+# Manually trigger completion of an active job in the READY state.
+#
+# @id: The job identifier.
+#
+# Since: 2.13
+##
+{ 'command': 'job-complete', 'data': { 'id': 'str' } }
+
+##
+# @job-dismiss:
+#
+# Deletes a job that is in the CONCLUDED state. This command only needs to be
+# run explicitly for jobs that don't have automatic dismiss enabled.
+#
+# This command will refuse to operate on any job that has not yet reached its
+# terminal state, JOB_STATUS_CONCLUDED. For jobs that make use of JOB_READY
+# event, job-cancel or job-complete will still need to be used as appropriate.
+#
+# @id: The job identifier.
+#
+# Since: 2.13
+##
+{ 'command': 'job-dismiss', 'data': { 'id': 'str' } }
+
+##
+# @job-finalize:
+#
+# Instructs all jobs in a transaction (or a single job if it is not part of any
+# transaction) to finalize any graph changes and do any necessary cleanup. This
+# command requires that all involved jobs are in the PENDING state.
+#
+# For jobs in a transaction, instructing one job to finalize will force
+# ALL jobs in the transaction to finalize, so it is only necessary to instruct
+# a single member job to finalize.
+#
+# @id: The identifier of any job in the transaction, or of a job that is not
+#  part of any transaction.
+#
+# Since: 2.13
+##
+{ 'command': 'job-finalize', 'data': { 'id': 'str' } }
diff --git a/job-qmp.c b/job-qmp.c
new file mode 100644
index 00..b2e18cfd9c
--- /dev/null
+++ b/job-qmp.c
@@ -0,0 +1,134 @@
+/*
+ * QMP interface for background jobs
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012, 2018 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/job.h"
+#include "qapi/qapi-commands-job.h"
+#include "qapi/error.h"
+#include "trace-root.h"

[Qemu-block] [PULL 43/46] job: Add query-jobs QMP command

[Kevin Wolf]

This adds a minimal query-jobs implementation that shouldn't pose many
design questions. It can later be extended to expose more information,
and especially job-specific information.

Signed-off-by: Kevin Wolf 
---
 qapi/job.json  | 46 ++
 include/qemu/job.h |  3 +++
 job-qmp.c  | 54 ++
 job.c  |  2 +-
 4 files changed, 104 insertions(+), 1 deletion(-)

diff --git a/qapi/job.json b/qapi/job.json
index b84dc6c820..970124de76 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -205,3 +205,49 @@
 # Since: 2.13
 ##
 { 'command': 'job-finalize', 'data': { 'id': 'str' } }
+
+##
+# @JobInfo:
+#
+# Information about a job.
+#
+# @id:  The job identifier
+#
+# @type:The kind of job that is being performed
+#
+# @status:  Current job state/status
+#
+# @current-progress:Progress made until now. The unit is arbitrary and the
+#   value can only meaningfully be used for the ratio of
+#   @current-progress to @total-progress. The value is
+#   monotonically increasing.
+#
+# @total-progress:  Estimated @current-progress value at the completion of
+#   the job. This value can arbitrarily change while the
+#   job is running, in both directions.
+#
+# @error:   If this field is present, the job failed; if it is
+#   still missing in the CONCLUDED state, this indicates
+#   successful completion.
+#
+#   The value is a human-readable error message to describe
+#   the reason for the job failure. It should not be parsed
+#   by applications.
+#
+# Since: 2.13
+##
+{ 'struct': 'JobInfo',
+  'data': { 'id': 'str', 'type': 'JobType', 'status': 'JobStatus',
+'current-progress': 'int', 'total-progress': 'int',
+'*error': 'str' } }
+
+##
+# @query-jobs:
+#
+# Return information about jobs.
+#
+# Returns: a list with a @JobInfo for each active job
+#
+# Since: 2.13
+##
+{ 'command': 'query-jobs', 'returns': ['JobInfo'] }
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 92d1d249fe..8c8badf75e 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -392,6 +392,9 @@ JobType job_type(const Job *job);
 /** Returns the enum string for the JobType of a given Job. */
 const char *job_type_str(const Job *job);
 
+/** Returns true if the job should not be visible to the management layer. */
+bool job_is_internal(Job *job);
+
 /** Returns whether the job is scheduled for cancellation. */
 bool job_is_cancelled(Job *job);
 
diff --git a/job-qmp.c b/job-qmp.c
index b2e18cfd9c..7f38f63336 100644
--- a/job-qmp.c
+++ b/job-qmp.c
@@ -132,3 +132,57 @@ void qmp_job_dismiss(const char *id, Error **errp)
 job_dismiss(, errp);
 aio_context_release(aio_context);
 }
+
+static JobInfo *job_query_single(Job *job, Error **errp)
+{
+JobInfo *info;
+const char *errmsg = NULL;
+
+assert(!job_is_internal(job));
+
+if (job->ret < 0) {
+errmsg = strerror(-job->ret);
+}
+
+info = g_new(JobInfo, 1);
+*info = (JobInfo) {
+.id = g_strdup(job->id),
+.type   = job_type(job),
+.status = job->status,
+.current_progress   = job->progress_current,
+.total_progress = job->progress_total,
+.has_error  = !!errmsg,
+.error  = g_strdup(errmsg),
+};
+
+return info;
+}
+
+JobInfoList *qmp_query_jobs(Error **errp)
+{
+JobInfoList *head = NULL, **p_next = 
+Job *job;
+
+for (job = job_next(NULL); job; job = job_next(job)) {
+JobInfoList *elem;
+AioContext *aio_context;
+
+if (job_is_internal(job)) {
+continue;
+}
+elem = g_new0(JobInfoList, 1);
+aio_context = job->aio_context;
+aio_context_acquire(aio_context);
+elem->value = job_query_single(job, errp);
+aio_context_release(aio_context);
+if (!elem->value) {
+g_free(elem);
+qapi_free_JobInfoList(head);
+return NULL;
+}
+*p_next = elem;
+p_next = >next;
+}
+
+return head;
+}
diff --git a/job.c b/job.c
index 599a1042cf..f026661b0f 100644
--- a/job.c
+++ b/job.c
@@ -158,7 +158,7 @@ static int job_txn_apply(JobTxn *txn, int fn(Job *), bool 
lock)
 return rc;
 }
 
-static bool job_is_internal(Job *job)
+bool job_is_internal(Job *job)
 {
 return (job->id == NULL);
 }
-- 
2.13.6

[Qemu-block] [PULL 32/46] job: Move transactions to Job

[Kevin Wolf]

This moves the logic that implements job transactions from BlockJob to
Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  54 --
 include/block/blockjob_int.h |  10 --
 include/qemu/job.h   |  71 +++--
 blockdev.c   |   6 +-
 blockjob.c   | 238 +--
 job.c| 234 --
 tests/test-blockjob-txn.c|  12 +--
 tests/test-blockjob.c|   2 +-
 8 files changed, 303 insertions(+), 324 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 44df025bd0..09e6bb42bf 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -33,7 +33,6 @@
 #define BLOCK_JOB_SLICE_TIME 1ULL /* ns */
 
 typedef struct BlockJobDriver BlockJobDriver;
-typedef struct JobTxn JobTxn;
 
 /**
  * BlockJob:
@@ -84,8 +83,6 @@ typedef struct BlockJob {
 
 /** BlockDriverStates that are involved in this block job */
 GSList *nodes;
-
-JobTxn *txn;
 } BlockJob;
 
 /**
@@ -153,22 +150,6 @@ void block_job_set_speed(BlockJob *job, int64_t speed, 
Error **errp);
 void block_job_cancel(BlockJob *job, bool force);
 
 /**
- * block_job_finalize:
- * @job: The job to fully commit and finish.
- * @errp: Error object.
- *
- * For jobs that have finished their work and are pending
- * awaiting explicit acknowledgement to commit their work,
- * This will commit that work.
- *
- * FIXME: Make the below statement universally true:
- * For jobs that support the manual workflow mode, all graph
- * changes that occur as a result will occur after this command
- * and before a successful reply.
- */
-void block_job_finalize(BlockJob *job, Error **errp);
-
-/**
  * block_job_dismiss:
  * @job: The job to be dismissed.
  * @errp: Error object.
@@ -260,41 +241,6 @@ int block_job_complete_sync(BlockJob *job, Error **errp);
 void block_job_iostatus_reset(BlockJob *job);
 
 /**
- * block_job_txn_new:
- *
- * Allocate and return a new block job transaction.  Jobs can be added to the
- * transaction using block_job_txn_add_job().
- *
- * The transaction is automatically freed when the last job completes or is
- * cancelled.
- *
- * All jobs in the transaction either complete successfully or fail/cancel as a
- * group.  Jobs wait for each other before completing.  Cancelling one job
- * cancels all jobs in the transaction.
- */
-JobTxn *block_job_txn_new(void);
-
-/**
- * block_job_txn_unref:
- *
- * Release a reference that was previously acquired with block_job_txn_add_job
- * or block_job_txn_new. If it's the last reference to the object, it will be
- * freed.
- */
-void block_job_txn_unref(JobTxn *txn);
-
-/**
- * block_job_txn_add_job:
- * @txn: The transaction (may be NULL)
- * @job: Job to add to the transaction
- *
- * Add @job to the transaction.  The @job must not already be in a transaction.
- * The caller must call either block_job_txn_unref() or block_job_completed()
- * to release the reference that is automatically grabbed here.
- */
-void block_job_txn_add_job(JobTxn *txn, BlockJob *job);
-
-/**
  * block_job_is_internal:
  * @job: The job to determine if it is user-visible or not.
  *
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index ce66a9b51c..29a28020ac 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -38,16 +38,6 @@ struct BlockJobDriver {
 /** Generic JobDriver callbacks and settings */
 JobDriver job_driver;
 
-/**
- * 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 before the job is
  * resumed in a new AioContext.  This is the place to move any resources
diff --git a/include/qemu/job.h b/include/qemu/job.h
index d4aa7fa981..39d74abdec 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -32,6 +32,8 @@
 #include "block/aio.h"
 
 typedef struct JobDriver JobDriver;
+typedef struct JobTxn JobTxn;
+
 
 /**
  * Long-running operation.
@@ -133,6 +135,9 @@ typedef struct Job {
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 
+/** Transaction this job is part of */
+JobTxn *txn;
+
 /** Element of the list of jobs in a job transaction */
 QLIST_ENTRY(Job) txn_list;
 } Job;
@@ -184,6 +189,16 @@ struct JobDriver {
 void (*drain)(Job *job);
 
 /**
+ * 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 

[Qemu-block] [PULL 36/46] job: Add job_dismiss()

[Kevin Wolf]

This moves block_job_dismiss() to the Job layer.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  9 -
 include/qemu/job.h   |  7 ++-
 blockdev.c   | 10 ++
 blockjob.c   | 13 -
 job.c| 15 ++-
 tests/test-blockjob.c|  4 ++--
 6 files changed, 28 insertions(+), 30 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index e9ed7b8b78..5a81afc6c3 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -141,15 +141,6 @@ void block_job_remove_all_bdrv(BlockJob *job);
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
 
 /**
- * block_job_dismiss:
- * @job: The job to be dismissed.
- * @errp: Error object.
- *
- * Remove a concluded job from the query list.
- */
-void block_job_dismiss(BlockJob **job, Error **errp);
-
-/**
  * block_job_progress_update:
  * @job: The job that has made progress
  * @done: How much progress the job made
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 94900ec008..1e8050c6fb 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -487,6 +487,12 @@ int job_complete_sync(Job *job, Error **errp);
  */
 void job_finalize(Job *job, Error **errp);
 
+/**
+ * Remove the concluded @job from the query list and resets the passed pointer
+ * to %NULL. Returns an error if the job is not actually concluded.
+ */
+void job_dismiss(Job **job, Error **errp);
+
 typedef void JobDeferToMainLoopFn(Job *job, void *opaque);
 
 /**
@@ -515,6 +521,5 @@ int job_finish_sync(Job *job, void (*finish)(Job *, Error 
**errp), Error **errp)
 
 /* TODO To be removed from the public interface */
 void job_state_transition(Job *job, JobStatus s1);
-void job_do_dismiss(Job *job);
 
 #endif
diff --git a/blockdev.c b/blockdev.c
index 31319a6d5a..8de95be8f4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3915,14 +3915,16 @@ void qmp_block_job_finalize(const char *id, Error 
**errp)
 void qmp_block_job_dismiss(const char *id, Error **errp)
 {
 AioContext *aio_context;
-BlockJob *job = find_block_job(id, _context, errp);
+BlockJob *bjob = find_block_job(id, _context, errp);
+Job *job;
 
-if (!job) {
+if (!bjob) {
 return;
 }
 
-trace_qmp_block_job_dismiss(job);
-block_job_dismiss(, errp);
+trace_qmp_block_job_dismiss(bjob);
+job = >job;
+job_dismiss(, errp);
 aio_context_release(aio_context);
 }
 
diff --git a/blockjob.c b/blockjob.c
index f146fe0cbd..3ca009bea5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -242,19 +242,6 @@ int64_t block_job_ratelimit_get_delay(BlockJob *job, 
uint64_t n)
 return ratelimit_calculate_delay(>limit, n);
 }
 
-void block_job_dismiss(BlockJob **jobptr, Error **errp)
-{
-BlockJob *job = *jobptr;
-/* similarly to _complete, this is QMP-interface only. */
-assert(job->job.id);
-if (job_apply_verb(>job, JOB_VERB_DISMISS, errp)) {
-return;
-}
-
-job_do_dismiss(>job);
-*jobptr = NULL;
-}
-
 void block_job_progress_update(BlockJob *job, uint64_t done)
 {
 job->offset += done;
diff --git a/job.c b/job.c
index eede6802ae..7cd3602782 100644
--- a/job.c
+++ b/job.c
@@ -568,7 +568,7 @@ void job_user_resume(Job *job, Error **errp)
 job_resume(job);
 }
 
-void job_do_dismiss(Job *job)
+static void job_do_dismiss(Job *job)
 {
 assert(job);
 job->busy = false;
@@ -581,6 +581,19 @@ void job_do_dismiss(Job *job)
 job_unref(job);
 }
 
+void job_dismiss(Job **jobptr, Error **errp)
+{
+Job *job = *jobptr;
+/* similarly to _complete, this is QMP-interface only. */
+assert(job->id);
+if (job_apply_verb(job, JOB_VERB_DISMISS, errp)) {
+return;
+}
+
+job_do_dismiss(job);
+*jobptr = NULL;
+}
+
 void job_early_fail(Job *job)
 {
 assert(job->status == JOB_STATUS_CREATED);
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index 46a78739aa..7131cabb16 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -233,8 +233,8 @@ static void cancel_common(CancelJob *s)
 
 job_cancel_sync(>job);
 if (sts != JOB_STATUS_CREATED && sts != JOB_STATUS_CONCLUDED) {
-BlockJob *dummy = job;
-block_job_dismiss(, _abort);
+Job *dummy = >job;
+job_dismiss(, _abort);
 }
 assert(job->job.status == JOB_STATUS_NULL);
 job_unref(>job);
-- 
2.13.6

[Qemu-block] [PULL 33/46] job: Move completion and cancellation to Job

[Kevin Wolf]

This moves the top-level job completion and cancellation functions from
BlockJob to Job.

Signed-off-by: Kevin Wolf 
---
 include/block/blockjob.h | 55 -
 include/block/blockjob_int.h | 18 --
 include/qemu/job.h   | 68 +--
 block.c  |  4 ++-
 block/backup.c   |  3 +-
 block/commit.c   |  6 ++--
 block/mirror.c   |  6 ++--
 block/replication.c  |  4 +--
 block/stream.c   |  2 +-
 blockdev.c   |  8 ++---
 blockjob.c   | 76 ---
 job.c| 84 +++-
 qemu-img.c   |  2 +-
 tests/test-bdrv-drain.c  |  5 ++-
 tests/test-blockjob-txn.c| 14 
 tests/test-blockjob.c| 21 ++-
 block/trace-events   |  3 --
 trace-events |  1 +
 18 files changed, 171 insertions(+), 209 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 09e6bb42bf..e9ed7b8b78 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -141,15 +141,6 @@ void block_job_remove_all_bdrv(BlockJob *job);
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
 
 /**
- * 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, bool force);
-
-/**
  * block_job_dismiss:
  * @job: The job to be dismissed.
  * @errp: Error object.
@@ -186,52 +177,6 @@ void block_job_progress_set_remaining(BlockJob *job, 
uint64_t remaining);
 BlockJobInfo *block_job_query(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, bool force, Error **errp);
-
-/**
- * block_job_cancel_sync:
- * @job: The job to be canceled.
- *
- * Synchronously cancel the job.  The completion callback is called
- * before the function returns.  The job may actually complete
- * instead of canceling itself; the circumstances under which this
- * happens depend on the kind of job that is active.
- *
- * Returns the return value from the job if the job actually completed
- * during the call, or -ECANCELED if it was canceled.
- */
-int block_job_cancel_sync(BlockJob *job);
-
-/**
- * block_job_cancel_sync_all:
- *
- * Synchronously cancels all jobs using block_job_cancel_sync().
- */
-void block_job_cancel_sync_all(void);
-
-/**
- * block_job_complete_sync:
- * @job: The job to be completed.
- * @errp: Error object which may be set by block_job_complete(); this is not
- *necessarily set on every error, the job return value has to be
- *checked as well.
- *
- * Synchronously complete the job.  The completion callback is called before 
the
- * function returns, unless it is NULL (which is permissible when using this
- * function).
- *
- * Returns the return value from the job.
- */
-int block_job_complete_sync(BlockJob *job, Error **errp);
-
-/**
  * block_job_iostatus_reset:
  * @job: The job whose I/O status should be reset.
  *
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 29a28020ac..7df07b20cf 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -124,24 +124,6 @@ void block_job_yield(BlockJob *job);
 int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n);
 
 /**
- * block_job_completed:
- * @job: The job being completed.
- * @ret: The status code.
- *
- * Call the completion function that was registered at creation time, and
- * free @job.
- */
-void block_job_completed(BlockJob *job, int ret);
-
-/**
- * block_job_enter:
- * @job: The job to enter.
- *
- * Continue the specified job by entering the coroutine.
- */
-void block_job_enter(BlockJob *job);
-
-/**
  * block_job_event_ready:
  * @job: The job which is now ready to be completed.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 39d74abdec..bbe1b0cd1a 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -74,8 +74,8 @@ typedef struct Job {
 
 /**
  * Set to false by the job while the coroutine has yielded and may be
- * re-entered by block_job_enter().  There may still be I/O or event loop
- * activity pending.  Accessed under block_job_mutex (in blockjob.c).
+ * re-entered by job_enter(). There may still be I/O or event loop activity
+ * pending. Accessed under block_job_mutex (in blockjob.c).
  */
 bool busy;
 
@@ -114,7 +114,7 @@ typedef struct Job {
 /** True if this job should automatically dismiss itself */
 bool auto_dismiss;
 
-/** ret code passed to 

[Qemu-block] [PULL 38/46] job: Add job_transition_to_ready()

[Kevin Wolf]

The transition to the READY state was still performed in the BlockJob
layer, in the same function that sent the BLOCK_JOB_READY QMP event.

This patch brings the state transition to the Job layer and implements
the QMP event using a notifier called from the Job layer, like we
already do for other events related to state transitions.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  3 +++
 include/block/blockjob_int.h |  8 
 include/qemu/job.h   |  9 ++---
 block/mirror.c   |  6 +++---
 blockjob.c   | 33 ++---
 job.c| 16 +---
 tests/test-bdrv-drain.c  |  2 +-
 tests/test-blockjob.c|  2 +-
 8 files changed, 45 insertions(+), 34 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 8e1e1ee0de..4fca45f6a1 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -76,6 +76,9 @@ typedef struct BlockJob {
 /** Called when the job transitions to PENDING */
 Notifier pending_notifier;
 
+/** Called when the job transitions to READY */
+Notifier ready_notifier;
+
 /** BlockDriverStates that are involved in this block job */
 GSList *nodes;
 } BlockJob;
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 806ac64d87..5cd50c6639 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -116,14 +116,6 @@ void block_job_drain(Job *job);
 int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n);
 
 /**
- * block_job_event_ready:
- * @job: The job which is now ready to be completed.
- *
- * Send a BLOCK_JOB_READY event for the specified job.
- */
-void block_job_event_ready(BlockJob *job);
-
-/**
  * block_job_error_action:
  * @job: The job to signal an error for.
  * @on_err: The error action setting.
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 487f9d9a32..bfc2bc5611 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -132,6 +132,9 @@ typedef struct Job {
 /** Notifiers called when the job transitions to PENDING */
 NotifierList on_pending;
 
+/** Notifiers called when the job transitions to READY */
+NotifierList on_ready;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 
@@ -426,6 +429,9 @@ int job_apply_verb(Job *job, JobVerb verb, Error **errp);
 /** The @job could not be started, free it. */
 void job_early_fail(Job *job);
 
+/** Moves the @job from RUNNING to READY */
+void job_transition_to_ready(Job *job);
+
 /**
  * @job: The job being completed.
  * @ret: The status code.
@@ -522,7 +528,4 @@ void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn 
*fn, void *opaque);
  */
 int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error 
**errp);
 
-/* TODO To be removed from the public interface */
-void job_state_transition(Job *job, JobStatus s1);
-
 #endif
diff --git a/block/mirror.c b/block/mirror.c
index 687f955c22..bdc1b5b7b9 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -727,8 +727,8 @@ static void coroutine_fn mirror_run(void *opaque)
 }
 
 if (s->bdev_length == 0) {
-/* Report BLOCK_JOB_READY and wait for complete. */
-block_job_event_ready(>common);
+/* Transition to the READY state and wait for complete. */
+job_transition_to_ready(>common.job);
 s->synced = true;
 while (!job_is_cancelled(>common.job) && !s->should_complete) {
 job_yield(>common.job);
@@ -824,7 +824,7 @@ static void coroutine_fn mirror_run(void *opaque)
  * report completion.  This way, block-job-cancel will leave
  * the target in a consistent state.
  */
-block_job_event_ready(>common);
+job_transition_to_ready(>common.job);
 s->synced = true;
 }
 
diff --git a/blockjob.c b/blockjob.c
index 38f18e9ba3..da11b3b763 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -338,6 +338,22 @@ static void block_job_event_pending(Notifier *n, void 
*opaque)
   _abort);
 }
 
+static void block_job_event_ready(Notifier *n, void *opaque)
+{
+BlockJob *job = opaque;
+
+if (block_job_is_internal(job)) {
+return;
+}
+
+qapi_event_send_block_job_ready(job_type(>job),
+job->job.id,
+job->len,
+job->offset,
+job->speed, _abort);
+}
+
+
 /*
  * API for block job drivers and the block layer.  These functions are
  * declared in blockjob_int.h.
@@ -386,12 +402,14 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 job->finalize_cancelled_notifier.notify = block_job_event_cancelled;
 job->finalize_completed_notifier.notify = block_job_event_completed;
 

[Qemu-block] [PULL 31/46] job: Switch transactions to JobTxn

[Kevin Wolf]

This doesn't actually move any transaction code to Job yet, but it
renames the type for transactions from BlockJobTxn to JobTxn and makes
them contain Jobs rather than BlockJobs

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/block_int.h|  2 +-
 include/block/blockjob.h | 11 
 include/block/blockjob_int.h |  2 +-
 include/qemu/job.h   |  3 +++
 block/backup.c   |  2 +-
 blockdev.c   | 14 +--
 blockjob.c   | 60 +++-
 tests/test-blockjob-txn.c|  8 +++---
 8 files changed, 54 insertions(+), 48 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 76b589da57..6c0927bce3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -1029,7 +1029,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
 BlockdevOnError on_target_error,
 int creation_flags,
 BlockCompletionFunc *cb, void *opaque,
-BlockJobTxn *txn, Error **errp);
+JobTxn *txn, Error **errp);
 
 void hmp_drive_add_node(Monitor *mon, const char *optstr);
 
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 85ce18a381..44df025bd0 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -33,7 +33,7 @@
 #define BLOCK_JOB_SLICE_TIME 1ULL /* ns */
 
 typedef struct BlockJobDriver BlockJobDriver;
-typedef struct BlockJobTxn BlockJobTxn;
+typedef struct JobTxn JobTxn;
 
 /**
  * BlockJob:
@@ -85,8 +85,7 @@ typedef struct BlockJob {
 /** BlockDriverStates that are involved in this block job */
 GSList *nodes;
 
-BlockJobTxn *txn;
-QLIST_ENTRY(BlockJob) txn_list;
+JobTxn *txn;
 } BlockJob;
 
 /**
@@ -273,7 +272,7 @@ void block_job_iostatus_reset(BlockJob *job);
  * group.  Jobs wait for each other before completing.  Cancelling one job
  * cancels all jobs in the transaction.
  */
-BlockJobTxn *block_job_txn_new(void);
+JobTxn *block_job_txn_new(void);
 
 /**
  * block_job_txn_unref:
@@ -282,7 +281,7 @@ BlockJobTxn *block_job_txn_new(void);
  * or block_job_txn_new. If it's the last reference to the object, it will be
  * freed.
  */
-void block_job_txn_unref(BlockJobTxn *txn);
+void block_job_txn_unref(JobTxn *txn);
 
 /**
  * block_job_txn_add_job:
@@ -293,7 +292,7 @@ void block_job_txn_unref(BlockJobTxn *txn);
  * The caller must call either block_job_txn_unref() or block_job_completed()
  * to release the reference that is automatically grabbed here.
  */
-void block_job_txn_add_job(BlockJobTxn *txn, BlockJob *job);
+void block_job_txn_add_job(JobTxn *txn, BlockJob *job);
 
 /**
  * block_job_is_internal:
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index b8ca7bb0c9..ce66a9b51c 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -91,7 +91,7 @@ struct BlockJobDriver {
  * called from a wrapper that is specific to the job type.
  */
 void *block_job_create(const char *job_id, const BlockJobDriver *driver,
-   BlockJobTxn *txn, BlockDriverState *bs, uint64_t perm,
+   JobTxn *txn, BlockDriverState *bs, uint64_t perm,
uint64_t shared_perm, int64_t speed, int flags,
BlockCompletionFunc *cb, void *opaque, Error **errp);
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 17e2ceca87..d4aa7fa981 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -132,6 +132,9 @@ typedef struct Job {
 
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
+
+/** Element of the list of jobs in a job transaction */
+QLIST_ENTRY(Job) txn_list;
 } Job;
 
 /**
diff --git a/block/backup.c b/block/backup.c
index ca7d990b21..6172f90c90 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -547,7 +547,7 @@ BlockJob *backup_job_create(const char *job_id, 
BlockDriverState *bs,
   BlockdevOnError on_target_error,
   int creation_flags,
   BlockCompletionFunc *cb, void *opaque,
-  BlockJobTxn *txn, Error **errp)
+  JobTxn *txn, Error **errp)
 {
 int64_t len;
 BlockDriverInfo bdi;
diff --git a/blockdev.c b/blockdev.c
index 0967f6ab66..817c3848c0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1446,7 +1446,7 @@ typedef struct BlkActionOps {
 struct BlkActionState {
 TransactionAction *action;
 const BlkActionOps *ops;
-BlockJobTxn *block_job_txn;
+JobTxn *block_job_txn;
 TransactionProperties *txn_props;
 QSIMPLEQ_ENTRY(BlkActionState) entry;
 };
@@ -1864,7 +1864,7 @@ typedef struct DriveBackupState {
 BlockJob *job;
 } DriveBackupState;
 
-static BlockJob *do_drive_backup(DriveBackup *backup, BlockJobTxn *txn,
+static BlockJob *do_drive_backup(DriveBackup 

[Qemu-block] [PULL 30/46] job: Move job_finish_sync() to Job

[Kevin Wolf]

block_job_finish_sync() doesn't contain anything block job specific any
more, so it can be moved to Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/qemu/job.h |  9 +
 block/commit.c |  6 +++---
 blockjob.c | 55 +-
 job.c  | 28 +++
 4 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 8f7f71a9b1..17e2ceca87 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -389,6 +389,15 @@ typedef void JobDeferToMainLoopFn(Job *job, void *opaque);
  */
 void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaque);
 
+/**
+ * Synchronously finishes the given @job. If @finish is given, it is called to
+ * trigger completion or cancellation of the job.
+ *
+ * Returns 0 if the job is successfully completed, -ECANCELED if the job was
+ * cancelled before completing, and -errno in other error cases.
+ */
+int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error 
**errp);
+
 /* TODO To be removed from the public interface */
 void job_state_transition(Job *job, JobStatus s1);
 void coroutine_fn job_do_yield(Job *job, uint64_t ns);
diff --git a/block/commit.c b/block/commit.c
index 02a8af9127..40d97a35a5 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -112,9 +112,9 @@ static void commit_complete(Job *job, void *opaque)
 blk_unref(s->top);
 
 /* If there is more than one reference to the job (e.g. if called from
- * block_job_finish_sync()), block_job_completed() won't free it and
- * therefore the blockers on the intermediate nodes remain. This would
- * cause bdrv_set_backing_hd() to fail. */
+ * job_finish_sync()), block_job_completed() won't free it and therefore
+ * the blockers on the intermediate nodes remain. This would cause
+ * bdrv_set_backing_hd() to fail. */
 block_job_remove_all_bdrv(bjob);
 
 block_job_completed(>common, ret);
diff --git a/blockjob.c b/blockjob.c
index 0ca7672941..1ed3e9c88d 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -307,40 +307,6 @@ static int block_job_txn_apply(BlockJobTxn *txn, int 
fn(BlockJob *), bool lock)
 return rc;
 }
 
-static int block_job_finish_sync(BlockJob *job,
- void (*finish)(BlockJob *, Error **errp),
- Error **errp)
-{
-Error *local_err = NULL;
-int ret;
-
-assert(blk_bs(job->blk)->job == job);
-
-job_ref(>job);
-
-if (finish) {
-finish(job, _err);
-}
-if (local_err) {
-error_propagate(errp, local_err);
-job_unref(>job);
-return -EBUSY;
-}
-/* job_drain calls job_enter, and it should be enough to induce progress
- * until the job completes or moves to the main thread.
-*/
-while (!job->job.deferred_to_main_loop && !job_is_completed(>job)) {
-job_drain(>job);
-}
-while (!job_is_completed(>job)) {
-aio_poll(qemu_get_aio_context(), true);
-}
-ret = (job_is_cancelled(>job) && job->job.ret == 0)
-  ? -ECANCELED : job->job.ret;
-job_unref(>job);
-return ret;
-}
-
 static void block_job_completed_txn_abort(BlockJob *job)
 {
 AioContext *ctx;
@@ -375,7 +341,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
 ctx = blk_get_aio_context(other_job->blk);
 if (!job_is_completed(_job->job)) {
 assert(job_is_cancelled(_job->job));
-block_job_finish_sync(other_job, NULL, NULL);
+job_finish_sync(_job->job, NULL, NULL);
 }
 job_finalize_single(_job->job);
 aio_context_release(ctx);
@@ -528,16 +494,18 @@ void block_job_user_cancel(BlockJob *job, bool force, 
Error **errp)
 }
 
 /* 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. */
-static void block_job_cancel_err(BlockJob *job, Error **errp)
+ * used with job_finish_sync() without the need for (rather nasty) function
+ * pointer casts there. */
+static void block_job_cancel_err(Job *job, Error **errp)
 {
-block_job_cancel(job, false);
+BlockJob *bjob = container_of(job, BlockJob, job);
+assert(is_block_job(job));
+block_job_cancel(bjob, false);
 }
 
 int block_job_cancel_sync(BlockJob *job)
 {
-return block_job_finish_sync(job, _job_cancel_err, NULL);
+return job_finish_sync(>job, _job_cancel_err, NULL);
 }
 
 void block_job_cancel_sync_all(void)
@@ -553,14 +521,9 @@ void block_job_cancel_sync_all(void)
 }
 }
 
-static void block_job_complete(BlockJob *job, Error **errp)
-{
-job_complete(>job, errp);
-}
-
 int block_job_complete_sync(BlockJob *job, Error **errp)
 {
-return block_job_finish_sync(job, _job_complete, errp);
+return job_finish_sync(>job, job_complete, errp);
 }
 
 

[Qemu-block] [PULL 26/46] job: Move single job finalisation to Job

[Kevin Wolf]

This moves the finalisation of a single job from BlockJob to Job.

Some part of this code depends on job transactions, and job transactions
call this code, we introduce some temporary calls from Job functions to
BlockJob ones. This will be fixed once transactions move to Job, too.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |   9 ---
 include/block/blockjob_int.h |  36 ---
 include/qemu/job.h   |  53 +++-
 block/backup.c   |  22 +++
 block/commit.c   |   2 +-
 block/mirror.c   |   2 +-
 blockjob.c   | 142 ---
 job.c| 100 +-
 qemu-img.c   |   2 +-
 tests/test-blockjob.c|  10 +--
 10 files changed, 194 insertions(+), 184 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index aef06295f6..3f405d1fa7 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -76,9 +76,6 @@ typedef struct BlockJob {
 /** Rate limiting data structure for implementing @speed. */
 RateLimit limit;
 
-/** The completion function that will be called when the job completes.  */
-BlockCompletionFunc *cb;
-
 /** Block other operations when block job is running */
 Error *blocker;
 
@@ -94,12 +91,6 @@ typedef struct BlockJob {
 /** BlockDriverStates that are involved in this block job */
 GSList *nodes;
 
-/** The opaque value that is passed to the completion function.  */
-void *opaque;
-
-/** ret code passed to block_job_completed. */
-int ret;
-
 BlockJobTxn *txn;
 QLIST_ENTRY(BlockJob) txn_list;
 } BlockJob;
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 88639f7efc..bf2b762808 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -54,34 +54,6 @@ struct BlockJobDriver {
  */
 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.
- *
- * All jobs will complete with a call to either .commit() or .abort() but
- * never both.
- */
-void (*commit)(BlockJob *job);
-
-/**
- * If the callback is not NULL, it will be invoked when any job in the
- * same transaction fails; or upon this job's failure (due to error or
- * cancellation) if it is not in a transaction. Skipped if NULL.
- *
- * All jobs will complete with a call to either .commit() or .abort() but
- * never both.
- */
-void (*abort)(BlockJob *job);
-
-/**
- * If the callback is not NULL, it will be invoked after a call to either
- * .commit() or .abort(). Regardless of which callback is invoked after
- * completion, .clean() will always be called, even if the job does not
- * belong to a transaction group.
- */
-void (*clean)(BlockJob *job);
-
 /*
  * If the callback is not NULL, it will be invoked before the job is
  * resumed in a new AioContext.  This is the place to move any resources
@@ -156,14 +128,6 @@ void block_job_yield(BlockJob *job);
 int64_t block_job_ratelimit_get_delay(BlockJob *job, uint64_t n);
 
 /**
- * block_job_early_fail:
- * @bs: The block device.
- *
- * The block job could not be started, free it.
- */
-void block_job_early_fail(BlockJob *job);
-
-/**
  * block_job_completed:
  * @job: The job being completed.
  * @ret: The status code.
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 14d93778f3..3e817beee9 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -29,6 +29,7 @@
 #include "qapi/qapi-types-block-core.h"
 #include "qemu/queue.h"
 #include "qemu/coroutine.h"
+#include "block/aio.h"
 
 typedef struct JobDriver JobDriver;
 
@@ -105,6 +106,15 @@ typedef struct Job {
 /** True if this job should automatically dismiss itself */
 bool auto_dismiss;
 
+/** ret code passed to block_job_completed. */
+int ret;
+
+/** The completion function that will be called when the job completes.  */
+BlockCompletionFunc *cb;
+
+/** The opaque value that is passed to the completion function.  */
+void *opaque;
+
 /** Notifiers called when a cancelled job is finalised */
 NotifierList on_finalize_cancelled;
 
@@ -151,6 +161,35 @@ struct JobDriver {
  */
 void (*user_resume)(Job *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.
+ *
+ * All jobs will complete with a call to either .commit() or .abort() but
+ * never both.
+ */
+void (*commit)(Job *job);
+
+/**
+ * If the callback 

[Qemu-block] [PULL 27/46] job: Convert block_job_cancel_async() to Job

[Kevin Wolf]

block_job_cancel_async() did two things that were still block job
specific:

* Setting job->force. This field makes sense on the Job level, so we can
  just move it. While at it, rename it to job->force_cancel to make its
  purpose more obvious.

* Resetting the I/O status. This can't be moved because generic Jobs
  don't have an I/O status. What the function really implements is a
  user resume, except without entering the coroutine. Consequently, it
  makes sense to call the .user_resume driver callback here which
  already resets the I/O status.

  The old block_job_cancel_async() has two separate if statements that
  check job->iostatus != BLOCK_DEVICE_IO_STATUS_OK and job->user_paused.
  However, the former condition always implies the latter (as is
  asserted in block_job_iostatus_reset()), so changing the explicit call
  of block_job_iostatus_reset() on the former condition with the
  .user_resume callback on the latter condition is equivalent and
  doesn't need to access any BlockJob specific state.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  6 --
 include/qemu/job.h   |  6 ++
 block/mirror.c   |  4 ++--
 blockjob.c   | 25 +
 4 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 3f405d1fa7..d975efea20 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -51,12 +51,6 @@ typedef struct BlockJob {
 BlockBackend *blk;
 
 /**
- * Set to true if the job should abort immediately without waiting
- * for data to be in sync.
- */
-bool force;
-
-/**
  * Set to true when the job is ready to be completed.
  */
 bool ready;
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 3e817beee9..2648c74281 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -97,6 +97,12 @@ typedef struct Job {
  */
 bool cancelled;
 
+/**
+ * Set to true if the job should abort immediately without waiting
+ * for data to be in sync.
+ */
+bool force_cancel;
+
 /** Set to true when the job has deferred work to the main loop. */
 bool deferred_to_main_loop;
 
diff --git a/block/mirror.c b/block/mirror.c
index e9a90ea730..c3951d1ca2 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -871,7 +871,7 @@ static void coroutine_fn mirror_run(void *opaque)
 trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
 job_sleep_ns(>common.job, delay_ns);
 if (job_is_cancelled(>common.job) &&
-(!s->synced || s->common.force))
+(!s->synced || s->common.job.force_cancel))
 {
 break;
 }
@@ -884,7 +884,7 @@ 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->common.force || !s->synced) &&
+assert(ret < 0 || ((s->common.job.force_cancel || !s->synced) &&
job_is_cancelled(>common.job)));
 assert(need_drain);
 mirror_wait_for_all_io(s);
diff --git a/blockjob.c b/blockjob.c
index 34c57da304..4cac3670ad 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -270,19 +270,20 @@ static int block_job_prepare(BlockJob *job)
 return job->job.ret;
 }
 
-static void block_job_cancel_async(BlockJob *job, bool force)
+static void job_cancel_async(Job *job, bool force)
 {
-if (job->iostatus != BLOCK_DEVICE_IO_STATUS_OK) {
-block_job_iostatus_reset(job);
-}
-if (job->job.user_paused) {
-/* Do not call block_job_enter here, the caller will handle it.  */
-job->job.user_paused = false;
-job->job.pause_count--;
+if (job->user_paused) {
+/* Do not call job_enter here, the caller will handle it.  */
+job->user_paused = false;
+if (job->driver->user_resume) {
+job->driver->user_resume(job);
+}
+assert(job->pause_count > 0);
+job->pause_count--;
 }
-job->job.cancelled = true;
+job->cancelled = true;
 /* To prevent 'force == false' overriding a previous 'force == true' */
-job->force |= force;
+job->force_cancel |= force;
 }
 
 static int block_job_txn_apply(BlockJobTxn *txn, int fn(BlockJob *), bool lock)
@@ -367,7 +368,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
  * on the caller, so leave it. */
 QLIST_FOREACH(other_job, >jobs, txn_list) {
 if (other_job != job) {
-block_job_cancel_async(other_job, false);
+job_cancel_async(_job->job, false);
 }
 }
 while (!QLIST_EMPTY(>jobs)) {
@@ -527,7 +528,7 @@ void block_job_cancel(BlockJob *job, bool force)
 job_do_dismiss(>job);
 return;
 }
-block_job_cancel_async(job, force);
+job_cancel_async(>job, force);
 if (!job_started(>job)) {
 

[Qemu-block] [PULL 29/46] job: Move .complete callback to Job

[Kevin Wolf]

This moves the .complete callback that tells a READY job to complete
from BlockJobDriver to JobDriver. The wrapper function job_complete()
doesn't require anything block job specific any more and can be moved
to Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h | 10 --
 include/block/blockjob_int.h |  6 --
 include/qemu/job.h   |  8 
 block/mirror.c   | 10 +-
 blockdev.c   |  2 +-
 blockjob.c   | 23 +--
 job.c| 16 
 tests/test-bdrv-drain.c  |  6 +++---
 tests/test-blockjob.c| 10 +-
 9 files changed, 43 insertions(+), 48 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index d975efea20..85ce18a381 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -154,16 +154,6 @@ void block_job_set_speed(BlockJob *job, int64_t speed, 
Error **errp);
 void block_job_cancel(BlockJob *job, bool force);
 
 /**
- * block_job_complete:
- * @job: The job to be completed.
- * @errp: Error object.
- *
- * Asynchronously complete the specified job.
- */
-void block_job_complete(BlockJob *job, Error **errp);
-
-
-/**
  * block_job_finalize:
  * @job: The job to fully commit and finish.
  * @errp: Error object.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 38fe22d7e0..b8ca7bb0c9 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -39,12 +39,6 @@ struct BlockJobDriver {
 JobDriver job_driver;
 
 /**
- * Optional callback for job types whose completion must be triggered
- * manually.
- */
-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.
diff --git a/include/qemu/job.h b/include/qemu/job.h
index aebc1959e6..8f7f71a9b1 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -167,6 +167,12 @@ struct JobDriver {
  */
 void (*user_resume)(Job *job);
 
+/**
+ * Optional callback for job types whose completion must be triggered
+ * manually.
+ */
+void (*complete)(Job *job, Error **errp);
+
 /*
  * If the callback is not NULL, it will be invoked when the job has to be
  * synchronously cancelled or completed; it should drain any activities
@@ -363,6 +369,8 @@ int job_apply_verb(Job *job, JobVerb verb, Error **errp);
 /** The @job could not be started, free it. */
 void job_early_fail(Job *job);
 
+/** Asynchronously complete the specified @job. */
+void job_complete(Job *job, Error **errp);;
 
 typedef void JobDeferToMainLoopFn(Job *job, void *opaque);
 
diff --git a/block/mirror.c b/block/mirror.c
index a579bd8cd1..656237af5c 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -905,16 +905,16 @@ immediate_exit:
 job_defer_to_main_loop(>common.job, mirror_exit, data);
 }
 
-static void mirror_complete(BlockJob *job, Error **errp)
+static void mirror_complete(Job *job, Error **errp)
 {
-MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
+MirrorBlockJob *s = container_of(job, MirrorBlockJob, common.job);
 BlockDriverState *target;
 
 target = blk_bs(s->target);
 
 if (!s->synced) {
 error_setg(errp, "The active block job '%s' cannot be completed",
-   job->job.id);
+   job->id);
 return;
 }
 
@@ -995,8 +995,8 @@ static const BlockJobDriver mirror_job_driver = {
 .drain  = block_job_drain,
 .start  = mirror_run,
 .pause  = mirror_pause,
+.complete   = mirror_complete,
 },
-.complete   = mirror_complete,
 .attached_aio_context   = mirror_attached_aio_context,
 .drain  = mirror_drain,
 };
@@ -1010,8 +1010,8 @@ static const BlockJobDriver commit_active_job_driver = {
 .drain  = block_job_drain,
 .start  = mirror_run,
 .pause  = mirror_pause,
+.complete   = mirror_complete,
 },
-.complete   = mirror_complete,
 .attached_aio_context   = mirror_attached_aio_context,
 .drain  = mirror_drain,
 };
diff --git a/blockdev.c b/blockdev.c
index 278b92ce03..0967f6ab66 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3894,7 +3894,7 @@ void qmp_block_job_complete(const char *device, Error 
**errp)
 }
 
 trace_qmp_block_job_complete(job);
-block_job_complete(job, errp);
+job_complete(>job, errp);
 aio_context_release(aio_context);
 }
 
diff --git a/blockjob.c b/blockjob.c
index 63e166927a..0ca7672941 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -481,24 +481,6 @@ int64_t 

[Qemu-block] [PULL 14/46] job: Move state transitions to Job

[Kevin Wolf]

This moves BlockJob.status and the closely related functions
(block_)job_state_transition() and (block_)job_apply_verb to Job. The
two QAPI enums are renamed to JobStatus and JobVerb.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
Reviewed-by: Eric Blake 
---
 qapi/block-core.json |  16 
 include/block/blockjob.h |   3 --
 include/qemu/job.h   |  13 ++
 blockjob.c   | 102 +++
 job.c|  56 ++
 tests/test-blockjob.c|  39 +-
 block/trace-events   |   2 -
 trace-events |   4 ++
 8 files changed, 123 insertions(+), 112 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 63c6011411..bb964b4319 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1068,9 +1068,9 @@
   'data': ['commit', 'stream', 'mirror', 'backup'] }
 
 ##
-# @BlockJobVerb:
+# @JobVerb:
 #
-# Represents command verbs that can be applied to a blockjob.
+# Represents command verbs that can be applied to a job.
 #
 # @cancel: see @block-job-cancel
 #
@@ -1088,14 +1088,14 @@
 #
 # Since: 2.12
 ##
-{ 'enum': 'BlockJobVerb',
+{ 'enum': 'JobVerb',
   'data': ['cancel', 'pause', 'resume', 'set-speed', 'complete', 'dismiss',
'finalize' ] }
 
 ##
-# @BlockJobStatus:
+# @JobStatus:
 #
-# Indicates the present state of a given blockjob in its lifetime.
+# Indicates the present state of a given job in its lifetime.
 #
 # @undefined: Erroneous, default state. Should not ever be visible.
 #
@@ -1134,7 +1134,7 @@
 #
 # Since: 2.12
 ##
-{ 'enum': 'BlockJobStatus',
+{ 'enum': 'JobStatus',
   'data': ['undefined', 'created', 'running', 'paused', 'ready', 'standby',
'waiting', 'pending', 'aborting', 'concluded', 'null' ] }
 
@@ -1184,7 +1184,7 @@
   'data': {'type': 'str', 'device': 'str', 'len': 'int',
'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int',
'io-status': 'BlockDeviceIoStatus', 'ready': 'bool',
-   'status': 'BlockJobStatus',
+   'status': 'JobStatus',
'auto-finalize': 'bool', 'auto-dismiss': 'bool',
'*error': 'str' } }
 
@@ -2416,7 +2416,7 @@
 # QEMU 2.12+ job lifetime management semantics.
 #
 # This command will refuse to operate on any job that has not yet reached
-# its terminal state, BLOCK_JOB_STATUS_CONCLUDED. For jobs that make use of
+# its terminal state, JOB_STATUS_CONCLUDED. For jobs that make use of the
 # BLOCK_JOB_READY event, block-job-cancel or block-job-complete will still need
 # to be used as appropriate.
 #
diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 10bd9f7059..01cdee6d15 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -147,9 +147,6 @@ typedef struct BlockJob {
  */
 QEMUTimer sleep_timer;
 
-/** Current state; See @BlockJobStatus for details. */
-BlockJobStatus status;
-
 /** True if this job should automatically finalize itself */
 bool auto_finalize;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index bae2b0920c..0b78778b9e 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -41,6 +41,9 @@ typedef struct Job {
 /** The type of this job. */
 const JobDriver *driver;
 
+/** Current state; See @JobStatus for details. */
+JobStatus status;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 } Job;
@@ -90,4 +93,14 @@ Job *job_next(Job *job);
  */
 Job *job_get(const char *id);
 
+/**
+ * Check whether the verb @verb can be applied to @job in its current state.
+ * Returns 0 if the verb can be applied; otherwise errp is set and -EPERM
+ * returned.
+ */
+int job_apply_verb(Job *job, JobVerb verb, Error **errp);
+
+/* TODO To be removed from the public interface */
+void job_state_transition(Job *job, JobStatus s1);
+
 #endif
diff --git a/blockjob.c b/blockjob.c
index c69b2e7cf5..0fba01edd4 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -41,61 +41,6 @@
  * block_job_enter. */
 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, W, D, X, E, N */
-/* U: */ [BLOCK_JOB_STATUS_UNDEFINED] = {0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0},
-/* C: */ [BLOCK_JOB_STATUS_CREATED]   = {0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 1},
-/* R: */ [BLOCK_JOB_STATUS_RUNNING]   = {0, 0, 0, 1, 1, 0, 1, 0, 1, 0, 0},
-/* P: */ [BLOCK_JOB_STATUS_PAUSED]= {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0},
-/* Y: */ [BLOCK_JOB_STATUS_READY] = {0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0},
-/* S: */ [BLOCK_JOB_STATUS_STANDBY]   = {0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0},
-/* W: */ [BLOCK_JOB_STATUS_WAITING]   = {0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0},
-/* D: */ [BLOCK_JOB_STATUS_PENDING]   = {0, 0, 0, 0, 0, 0, 0, 0, 1, 

[Qemu-block] [PULL 16/46] job: Move cancelled to Job

[Kevin Wolf]

We cannot yet move the whole logic around job cancelling to Job because
it depends on quite a few other things that are still only in BlockJob,
but we can move the cancelled field at least.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  8 
 include/block/blockjob_int.h |  8 
 include/qemu/job.h   | 11 +++
 block/backup.c   |  6 +++---
 block/commit.c   |  4 ++--
 block/mirror.c   | 20 ++--
 block/stream.c   |  4 ++--
 blockjob.c   | 28 +---
 job.c|  5 +
 tests/test-blockjob-txn.c|  6 +++---
 tests/test-blockjob.c|  2 +-
 11 files changed, 50 insertions(+), 52 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 087e7820f5..1e708f468a 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -57,14 +57,6 @@ typedef struct BlockJob {
 Coroutine *co;
 
 /**
- * Set to true if the job should cancel itself.  The flag must
- * always be tested just before toggling the busy flag from false
- * to true.  After a job has been cancelled, it should only yield
- * if #aio_poll will ("sooner or later") reenter the coroutine.
- */
-bool cancelled;
-
-/**
  * Set to true if the job should abort immediately without waiting
  * for data to be in sync.
  */
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 6f0fe3c48d..d64f30e6b0 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -196,14 +196,6 @@ void block_job_early_fail(BlockJob *job);
 void block_job_completed(BlockJob *job, int ret);
 
 /**
- * block_job_is_cancelled:
- * @job: The job being queried.
- *
- * Returns whether the job is scheduled for cancellation.
- */
-bool block_job_is_cancelled(BlockJob *job);
-
-/**
  * block_job_pause_point:
  * @job: The job that is ready to pause.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 0751e2afab..5dfbec5d69 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -47,6 +47,14 @@ typedef struct Job {
 /** Current state; See @JobStatus for details. */
 JobStatus status;
 
+/**
+ * Set to true if the job should cancel itself.  The flag must
+ * always be tested just before toggling the busy flag from false
+ * to true.  After a job has been cancelled, it should only yield
+ * if #aio_poll will ("sooner or later") reenter the coroutine.
+ */
+bool cancelled;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 } Job;
@@ -93,6 +101,9 @@ JobType job_type(const Job *job);
 /** Returns the enum string for the JobType of a given Job. */
 const char *job_type_str(const Job *job);
 
+/** Returns whether the job is scheduled for cancellation. */
+bool job_is_cancelled(Job *job);
+
 /**
  * Get the next element from the list of block jobs after @job, or the
  * first one if @job is %NULL.
diff --git a/block/backup.c b/block/backup.c
index cfdb89d977..ef0aa0e24e 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -329,7 +329,7 @@ static bool coroutine_fn yield_and_check(BackupBlockJob 
*job)
 {
 uint64_t delay_ns;
 
-if (block_job_is_cancelled(>common)) {
+if (job_is_cancelled(>common.job)) {
 return true;
 }
 
@@ -339,7 +339,7 @@ static bool coroutine_fn yield_and_check(BackupBlockJob 
*job)
 job->bytes_read = 0;
 block_job_sleep_ns(>common, delay_ns);
 
-if (block_job_is_cancelled(>common)) {
+if (job_is_cancelled(>common.job)) {
 return true;
 }
 
@@ -441,7 +441,7 @@ static void coroutine_fn backup_run(void *opaque)
 if (job->sync_mode == MIRROR_SYNC_MODE_NONE) {
 /* All bits are set in copy_bitmap to allow any cluster to be copied.
  * This does not actually require them to be copied. */
-while (!block_job_is_cancelled(>common)) {
+while (!job_is_cancelled(>common.job)) {
 /* Yield until the job is cancelled.  We just let our before_write
  * notify callback service CoW requests. */
 block_job_yield(>common);
diff --git a/block/commit.c b/block/commit.c
index 925c96abe7..85baea8f92 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -90,7 +90,7 @@ static void commit_complete(BlockJob *job, void *opaque)
  * the normal backing chain can be restored. */
 blk_unref(s->base);
 
-if (!block_job_is_cancelled(>common) && ret == 0) {
+if (!job_is_cancelled(>common.job) && ret == 0) {
 /* success */
 ret = bdrv_drop_intermediate(s->commit_top_bs, base,
  s->backing_file_str);
@@ -172,7 +172,7 @@ static void coroutine_fn commit_run(void *opaque)
  * with no pending I/O here so that bdrv_drain_all() returns.
  */
 

[Qemu-block] [PULL 21/46] job: Move pause/resume functions to Job

[Kevin Wolf]

While we already moved the state related to job pausing to Job, the
functions to do were still BlockJob only. This commit moves them over to
Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h | 32 -
 include/block/blockjob_int.h |  7 
 include/qemu/job.h   | 37 
 block/backup.c   |  1 +
 block/commit.c   |  1 +
 block/mirror.c   |  2 ++
 block/stream.c   |  1 +
 blockdev.c   |  6 ++--
 blockjob.c   | 81 +---
 job.c| 59 
 tests/test-bdrv-drain.c  |  1 +
 tests/test-blockjob-txn.c|  1 +
 tests/test-blockjob.c|  6 ++--
 13 files changed, 133 insertions(+), 102 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index b60d919fbf..556a8f6375 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -57,12 +57,6 @@ typedef struct BlockJob {
 bool force;
 
 /**
- * Set to true if the job is paused by user.  Can be unpaused with the
- * block-job-resume QMP command.
- */
-bool user_paused;
-
-/**
  * Set to true when the job is ready to be completed.
  */
 bool ready;
@@ -248,32 +242,6 @@ void block_job_progress_set_remaining(BlockJob *job, 
uint64_t remaining);
 BlockJobInfo *block_job_query(BlockJob *job, Error **errp);
 
 /**
- * block_job_user_pause:
- * @job: The job to be paused.
- *
- * 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, Error **errp);
-
-/**
- * block_job_paused:
- * @job: The job to query.
- *
- * Returns true if the job is user-paused.
- */
-bool block_job_user_paused(BlockJob *job);
-
-/**
- * block_job_user_resume:
- * @job: The job to be resumed.
- *
- * Resume the specified job.
- * Must be paired with a preceding block_job_user_pause.
- */
-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.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 8937f5b163..7e705ae0e9 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -134,6 +134,13 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 void block_job_free(Job *job);
 
 /**
+ * block_job_user_resume:
+ * Callback to be used for JobDriver.user_resume in all block jobs. Resets the
+ * iostatus when the user resumes @job.
+ */
+void block_job_user_resume(Job *job);
+
+/**
  * block_job_yield:
  * @job: The job that calls the function.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 509408f747..bc6398568f 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -83,6 +83,12 @@ typedef struct Job {
 bool paused;
 
 /**
+ * Set to true if the job is paused by user.  Can be unpaused with the
+ * block-job-resume QMP command.
+ */
+bool user_paused;
+
+/**
  * Set to true if the job should cancel itself.  The flag must
  * always be tested just before toggling the busy flag from false
  * to true.  After a job has been cancelled, it should only yield
@@ -124,6 +130,12 @@ struct JobDriver {
  */
 void coroutine_fn (*resume)(Job *job);
 
+/**
+ * Called when the job is resumed by the user (i.e. user_paused becomes
+ * false). .user_resume is called before .resume.
+ */
+void (*user_resume)(Job *job);
+
 /** Called when the job is freed */
 void (*free)(Job *job);
 };
@@ -203,6 +215,31 @@ const char *job_type_str(const Job *job);
 bool job_is_cancelled(Job *job);
 
 /**
+ * Request @job to pause at the next pause point. Must be paired with
+ * job_resume(). If the job is supposed to be resumed by user action, call
+ * job_user_pause() instead.
+ */
+void job_pause(Job *job);
+
+/** Resumes a @job paused with job_pause. */
+void job_resume(Job *job);
+
+/**
+ * Asynchronously pause the specified @job.
+ * Do not allow a resume until a matching call to job_user_resume.
+ */
+void job_user_pause(Job *job, Error **errp);
+
+/** Returns true if the job is user-paused. */
+bool job_user_paused(Job *job);
+
+/**
+ * Resume the specified @job.
+ * Must be paired with a preceding job_user_pause.
+ */
+void job_user_resume(Job *job, Error **errp);
+
+/**
  * Get the next element from the list of block jobs after @job, or the
  * first one if @job is %NULL.
  *
diff --git a/block/backup.c b/block/backup.c
index f3a4f7c898..4d011d5a5c 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -528,6 +528,7 @@ static const BlockJobDriver backup_job_driver = {
 .instance_size  = sizeof(BackupBlockJob),
  

[Qemu-block] [PULL 20/46] job: Add job_sleep_ns()

[Kevin Wolf]

There is nothing block layer specific about block_job_sleep_ns(), so
move the function to Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: John Snow 
Reviewed-by: Max Reitz 
---
 include/block/blockjob_int.h | 11 ---
 include/qemu/job.h   | 19 ++-
 block/backup.c   |  2 +-
 block/commit.c   |  2 +-
 block/mirror.c   |  4 ++--
 block/stream.c   |  2 +-
 blockjob.c   | 27 ---
 job.c| 32 
 tests/test-bdrv-drain.c  |  8 
 tests/test-blockjob-txn.c|  2 +-
 tests/test-blockjob.c|  2 +-
 11 files changed, 61 insertions(+), 50 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 0a614a89b8..8937f5b163 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -134,17 +134,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 void block_job_free(Job *job);
 
 /**
- * block_job_sleep_ns:
- * @job: The job that calls the function.
- * @ns: How many nanoseconds to stop for.
- *
- * Put the job to sleep (assuming that it wasn't canceled) for @ns
- * %QEMU_CLOCK_REALTIME nanoseconds.  Canceling the job will immediately
- * interrupt the wait.
- */
-void block_job_sleep_ns(BlockJob *job, int64_t ns);
-
-/**
  * block_job_yield:
  * @job: The job that calls the function.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 9dcff12283..509408f747 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -58,7 +58,7 @@ typedef struct Job {
 Coroutine *co;
 
 /**
- * Timer that is used by @block_job_sleep_ns. Accessed under job_mutex (in
+ * Timer that is used by @job_sleep_ns. Accessed under job_mutex (in
  * job.c).
  */
 QEMUTimer sleep_timer;
@@ -168,6 +168,13 @@ void job_enter_cond(Job *job, bool(*fn)(Job *job));
 void job_start(Job *job);
 
 /**
+ * @job: The job to enter.
+ *
+ * Continue the specified job by entering the coroutine.
+ */
+void job_enter(Job *job);
+
+/**
  * @job: The job that is ready to pause.
  *
  * Pause now if job_pause() has been called. Jobs that perform lots of I/O
@@ -175,6 +182,16 @@ void job_start(Job *job);
  */
 void coroutine_fn job_pause_point(Job *job);
 
+/**
+ * @job: The job that calls the function.
+ * @ns: How many nanoseconds to stop for.
+ *
+ * Put the job to sleep (assuming that it wasn't canceled) for @ns
+ * %QEMU_CLOCK_REALTIME nanoseconds.  Canceling the job will immediately
+ * interrupt the wait.
+ */
+void coroutine_fn job_sleep_ns(Job *job, int64_t ns);
+
 
 /** Returns the JobType of a given Job. */
 JobType job_type(const Job *job);
diff --git a/block/backup.c b/block/backup.c
index 7d9aad9749..f3a4f7c898 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -338,7 +338,7 @@ static bool coroutine_fn yield_and_check(BackupBlockJob 
*job)
  * return. Without a yield, the VM would not reboot. */
 delay_ns = block_job_ratelimit_get_delay(>common, job->bytes_read);
 job->bytes_read = 0;
-block_job_sleep_ns(>common, delay_ns);
+job_sleep_ns(>common.job, delay_ns);
 
 if (job_is_cancelled(>common.job)) {
 return true;
diff --git a/block/commit.c b/block/commit.c
index 2fbc31077a..1c6cb6c298 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -172,7 +172,7 @@ static void coroutine_fn commit_run(void *opaque)
 /* Note that even when no rate limit is applied we need to yield
  * with no pending I/O here so that bdrv_drain_all() returns.
  */
-block_job_sleep_ns(>common, delay_ns);
+job_sleep_ns(>common.job, delay_ns);
 if (job_is_cancelled(>common.job)) {
 break;
 }
diff --git a/block/mirror.c b/block/mirror.c
index 95fc8072b0..5d8f75c677 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -595,7 +595,7 @@ static void mirror_throttle(MirrorBlockJob *s)
 
 if (now - s->last_pause_ns > BLOCK_JOB_SLICE_TIME) {
 s->last_pause_ns = now;
-block_job_sleep_ns(>common, 0);
+job_sleep_ns(>common.job, 0);
 } else {
 job_pause_point(>common.job);
 }
@@ -869,7 +869,7 @@ static void coroutine_fn mirror_run(void *opaque)
 cnt == 0 ? BLOCK_JOB_SLICE_TIME : 0);
 }
 trace_mirror_before_sleep(s, cnt, s->synced, delay_ns);
-block_job_sleep_ns(>common, delay_ns);
+job_sleep_ns(>common.job, delay_ns);
 if (job_is_cancelled(>common.job) &&
 (!s->synced || s->common.force))
 {
diff --git a/block/stream.c b/block/stream.c
index 6d8b7b6eee..1faab02086 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -140,7 +140,7 @@ static void coroutine_fn stream_run(void *opaque)
 /* Note that even when no rate limit is applied we need to yield
  * with no pending I/O here so that bdrv_drain_all() 

[Qemu-block] [PULL 15/46] job: Add reference counting

[Kevin Wolf]

This moves reference counting from BlockJob to Job.

In order to keep calling the BlockJob cleanup code when the job is
deleted via job_unref(), introduce a new JobDriver.free callback. Every
block job must use block_job_free() for this callback, this is asserted
in block_job_create().

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h | 21 ---
 include/block/blockjob_int.h |  7 +++
 include/qemu/job.h   | 19 --
 block/backup.c   |  1 +
 block/commit.c   |  1 +
 block/mirror.c   |  2 ++
 block/stream.c   |  1 +
 blockjob.c   | 48 +++-
 job.c| 22 
 qemu-img.c   |  4 ++--
 tests/test-bdrv-drain.c  |  1 +
 tests/test-blockjob-txn.c|  1 +
 tests/test-blockjob.c|  6 --
 13 files changed, 76 insertions(+), 58 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 01cdee6d15..087e7820f5 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -132,9 +132,6 @@ typedef struct BlockJob {
 /** The opaque value that is passed to the completion function.  */
 void *opaque;
 
-/** Reference count of the block job */
-int refcnt;
-
 /** True when job has reported completion by calling block_job_completed. 
*/
 bool completed;
 
@@ -400,24 +397,6 @@ void block_job_iostatus_reset(BlockJob *job);
 BlockJobTxn *block_job_txn_new(void);
 
 /**
- * block_job_ref:
- *
- * Add a reference to BlockJob refcnt, it will be decreased with
- * block_job_unref, and then be freed if it comes to be the last
- * reference.
- */
-void block_job_ref(BlockJob *job);
-
-/**
- * block_job_unref:
- *
- * Release a reference that was previously acquired with block_job_ref
- * or block_job_create. If it's the last reference to the object, it will be
- * freed.
- */
-void block_job_unref(BlockJob *job);
-
-/**
  * block_job_txn_unref:
  *
  * Release a reference that was previously acquired with block_job_txn_add_job
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 1e62d6dd30..6f0fe3c48d 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -144,6 +144,13 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
BlockCompletionFunc *cb, void *opaque, Error **errp);
 
 /**
+ * block_job_free:
+ * Callback to be used for JobDriver.free in all block jobs. Frees block job
+ * specific resources in @job.
+ */
+void block_job_free(Job *job);
+
+/**
  * block_job_sleep_ns:
  * @job: The job that calls the function.
  * @ns: How many nanoseconds to stop for.
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 0b78778b9e..0751e2afab 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -41,6 +41,9 @@ typedef struct Job {
 /** The type of this job. */
 const JobDriver *driver;
 
+/** Reference count of the block job */
+int refcnt;
+
 /** Current state; See @JobStatus for details. */
 JobStatus status;
 
@@ -57,6 +60,9 @@ struct JobDriver {
 
 /** Enum describing the operation */
 JobType job_type;
+
+/** Called when the job is freed */
+void (*free)(Job *job);
 };
 
 
@@ -69,8 +75,17 @@ struct JobDriver {
  */
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
 
-/** Frees the @job object. */
-void job_delete(Job *job);
+/**
+ * Add a reference to Job refcnt, it will be decreased with job_unref, and then
+ * be freed if it comes to be the last reference.
+ */
+void job_ref(Job *job);
+
+/**
+ * Release a reference that was previously acquired with job_ref() or
+ * job_create(). If it's the last reference to the object, it will be freed.
+ */
+void job_unref(Job *job);
 
 /** Returns the JobType of a given Job. */
 JobType job_type(const Job *job);
diff --git a/block/backup.c b/block/backup.c
index baf8d432da..cfdb89d977 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -526,6 +526,7 @@ static const BlockJobDriver backup_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(BackupBlockJob),
 .job_type   = JOB_TYPE_BACKUP,
+.free   = block_job_free,
 },
 .start  = backup_run,
 .commit = backup_commit,
diff --git a/block/commit.c b/block/commit.c
index 32d29c890e..925c96abe7 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -218,6 +218,7 @@ static const BlockJobDriver commit_job_driver = {
 .job_driver = {
 .instance_size = sizeof(CommitBlockJob),
 .job_type  = JOB_TYPE_COMMIT,
+.free  = block_job_free,
 },
 .start = commit_run,
 };
diff --git a/block/mirror.c b/block/mirror.c
index 35fcc1f0b7..0df4f709fd 

[Qemu-block] [PULL 28/46] job: Add job_drain()

[Kevin Wolf]

block_job_drain() contains a blk_drain() call which cannot be moved to
Job, so add a new JobDriver callback JobDriver.drain which has a common
implementation for all BlockJobs. In addition to this we keep the
existing BlockJobDriver.drain callback that is called by the common
drain implementation for all block jobs.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob_int.h | 12 
 include/qemu/job.h   | 13 +
 block/backup.c   |  1 +
 block/commit.c   |  1 +
 block/mirror.c   |  2 ++
 block/stream.c   |  1 +
 blockjob.c   | 20 ++--
 job.c| 11 +++
 tests/test-bdrv-drain.c  |  1 +
 tests/test-blockjob-txn.c|  1 +
 tests/test-blockjob.c|  2 ++
 11 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index bf2b762808..38fe22d7e0 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -65,6 +65,10 @@ struct BlockJobDriver {
  * If the callback is not NULL, it will be invoked when the job has to be
  * synchronously cancelled or completed; it should drain BlockDriverStates
  * as required to ensure progress.
+ *
+ * Block jobs must use the default implementation for job_driver.drain,
+ * which will in turn call this callback after doing generic block job
+ * stuff.
  */
 void (*drain)(BlockJob *job);
 };
@@ -112,6 +116,14 @@ void block_job_free(Job *job);
 void block_job_user_resume(Job *job);
 
 /**
+ * block_job_drain:
+ * Callback to be used for JobDriver.drain in all block jobs. Drains the main
+ * block node associated with the block jobs and calls BlockJobDriver.drain for
+ * job-specific actions.
+ */
+void block_job_drain(Job *job);
+
+/**
  * block_job_yield:
  * @job: The job that calls the function.
  *
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 2648c74281..aebc1959e6 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -167,6 +167,13 @@ struct JobDriver {
  */
 void (*user_resume)(Job *job);
 
+/*
+ * If the callback is not NULL, it will be invoked when the job has to be
+ * synchronously cancelled or completed; it should drain any activities
+ * as required to ensure progress.
+ */
+void (*drain)(Job *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
@@ -325,6 +332,12 @@ bool job_user_paused(Job *job);
  */
 void job_user_resume(Job *job, Error **errp);
 
+/*
+ * Drain any activities as required to ensure progress. This can be called in a
+ * loop to synchronously complete a job.
+ */
+void job_drain(Job *job);
+
 /**
  * Get the next element from the list of block jobs after @job, or the
  * first one if @job is %NULL.
diff --git a/block/backup.c b/block/backup.c
index bd31282924..ca7d990b21 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -529,6 +529,7 @@ static const BlockJobDriver backup_job_driver = {
 .job_type   = JOB_TYPE_BACKUP,
 .free   = block_job_free,
 .user_resume= block_job_user_resume,
+.drain  = block_job_drain,
 .start  = backup_run,
 .commit = backup_commit,
 .abort  = backup_abort,
diff --git a/block/commit.c b/block/commit.c
index e53b2d7d55..02a8af9127 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -221,6 +221,7 @@ static const BlockJobDriver commit_job_driver = {
 .job_type  = JOB_TYPE_COMMIT,
 .free  = block_job_free,
 .user_resume   = block_job_user_resume,
+.drain = block_job_drain,
 .start = commit_run,
 },
 };
diff --git a/block/mirror.c b/block/mirror.c
index c3951d1ca2..a579bd8cd1 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -992,6 +992,7 @@ static const BlockJobDriver mirror_job_driver = {
 .job_type   = JOB_TYPE_MIRROR,
 .free   = block_job_free,
 .user_resume= block_job_user_resume,
+.drain  = block_job_drain,
 .start  = mirror_run,
 .pause  = mirror_pause,
 },
@@ -1006,6 +1007,7 @@ static const BlockJobDriver commit_active_job_driver = {
 .job_type   = JOB_TYPE_COMMIT,
 .free   = block_job_free,
 .user_resume= block_job_user_resume,
+.drain  = block_job_drain,
 .start  = mirror_run,
 .pause  = mirror_pause,
 },
diff --git a/block/stream.c b/block/stream.c
index eee02538ed..b996278ab3 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ 

[Qemu-block] [PULL 25/46] job: Add job_event_*()

[Kevin Wolf]

Go through the Job layer in order to send QMP events. For the moment,
these functions only call a notifier in the BlockJob layer that sends
the existing commands.

This uses notifiers rather than JobDriver callbacks because internal
users of jobs won't receive QMP events, but might still be interested
in getting notified for the events.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h |  9 +
 include/qemu/job.h   | 18 ++
 blockjob.c   | 41 +++--
 job.c| 19 +++
 4 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index f9a835..aef06295f6 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -82,6 +82,15 @@ typedef struct BlockJob {
 /** Block other operations when block job is running */
 Error *blocker;
 
+/** Called when a cancelled job is finalised. */
+Notifier finalize_cancelled_notifier;
+
+/** Called when a successfully completed job is finalised. */
+Notifier finalize_completed_notifier;
+
+/** Called when the job transitions to PENDING */
+Notifier pending_notifier;
+
 /** BlockDriverStates that are involved in this block job */
 GSList *nodes;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 9783e4049b..14d93778f3 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -105,6 +105,15 @@ typedef struct Job {
 /** True if this job should automatically dismiss itself */
 bool auto_dismiss;
 
+/** Notifiers called when a cancelled job is finalised */
+NotifierList on_finalize_cancelled;
+
+/** Notifiers called when a successfully completed job is finalised */
+NotifierList on_finalize_completed;
+
+/** Notifiers called when the job transitions to PENDING */
+NotifierList on_pending;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 } Job;
@@ -182,6 +191,15 @@ void job_ref(Job *job);
  */
 void job_unref(Job *job);
 
+/** To be called when a cancelled job is finalised. */
+void job_event_cancelled(Job *job);
+
+/** To be called when a successfully completed job is finalised. */
+void job_event_completed(Job *job);
+
+/** To be called when the job transitions to PENDING */
+void job_event_pending(Job *job);
+
 /**
  * Conditionally enter the job coroutine if the job is ready to run, not
  * already busy and fn() returns true. fn() is called while under the job_lock
diff --git a/blockjob.c b/blockjob.c
index b4334fb1bf..05d7921b3f 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -36,10 +36,6 @@
 #include "qemu/coroutine.h"
 #include "qemu/timer.h"
 
-static void block_job_event_cancelled(BlockJob *job);
-static void block_job_event_completed(BlockJob *job, const char *msg);
-static void block_job_event_pending(BlockJob *job);
-
 /* Transactional group of block jobs */
 struct BlockJobTxn {
 
@@ -352,13 +348,9 @@ static int block_job_finalize_single(BlockJob *job)
 /* Emit events only if we actually started */
 if (job_started(>job)) {
 if (job_is_cancelled(>job)) {
-block_job_event_cancelled(job);
+job_event_cancelled(>job);
 } else {
-const char *msg = NULL;
-if (job->ret < 0) {
-msg = strerror(-job->ret);
-}
-block_job_event_completed(job, msg);
+job_event_completed(>job);
 }
 }
 
@@ -504,7 +496,7 @@ static int block_job_transition_to_pending(BlockJob *job)
 {
 job_state_transition(>job, JOB_STATUS_PENDING);
 if (!job->job.auto_finalize) {
-block_job_event_pending(job);
+job_event_pending(>job);
 }
 return 0;
 }
@@ -712,8 +704,10 @@ static void block_job_iostatus_set_err(BlockJob *job, int 
error)
 }
 }
 
-static void block_job_event_cancelled(BlockJob *job)
+static void block_job_event_cancelled(Notifier *n, void *opaque)
 {
+BlockJob *job = opaque;
+
 if (block_job_is_internal(job)) {
 return;
 }
@@ -726,12 +720,19 @@ static void block_job_event_cancelled(BlockJob *job)
 _abort);
 }
 
-static void block_job_event_completed(BlockJob *job, const char *msg)
+static void block_job_event_completed(Notifier *n, void *opaque)
 {
+BlockJob *job = opaque;
+const char *msg = NULL;
+
 if (block_job_is_internal(job)) {
 return;
 }
 
+if (job->ret < 0) {
+msg = strerror(-job->ret);
+}
+
 qapi_event_send_block_job_completed(job_type(>job),
 job->job.id,
 job->len,
@@ -742,8 +743,10 @@ static void block_job_event_completed(BlockJob *job, const 
char *msg)
 _abort);
 }
 
-static void block_job_event_pending(BlockJob *job)
+static void 

[Qemu-block] [PULL 23/46] job: Move BlockJobCreateFlags to Job

[Kevin Wolf]

This renames the BlockJobCreateFlags constants, moves a few JOB_INTERNAL
checks to job_create() and the auto_{finalize,dismiss} fields from
BlockJob to Job.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 include/block/blockjob.h | 17 -
 include/block/blockjob_int.h |  3 +--
 include/qemu/job.h   | 20 +++-
 block/commit.c   |  2 +-
 block/mirror.c   |  2 +-
 block/replication.c  |  4 ++--
 block/stream.c   |  2 +-
 blockdev.c   | 14 +++---
 blockjob.c   | 27 +++
 job.c| 11 ++-
 qemu-img.c   |  2 +-
 tests/test-blockjob-txn.c|  2 +-
 tests/test-blockjob.c|  4 ++--
 13 files changed, 53 insertions(+), 57 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 3e94e18850..f9a835 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -91,27 +91,10 @@ typedef struct BlockJob {
 /** ret code passed to block_job_completed. */
 int ret;
 
-/** True if this job should automatically finalize itself */
-bool auto_finalize;
-
-/** True if this job should automatically dismiss itself */
-bool auto_dismiss;
-
 BlockJobTxn *txn;
 QLIST_ENTRY(BlockJob) txn_list;
 } BlockJob;
 
-typedef enum BlockJobCreateFlags {
-/* Default behavior */
-BLOCK_JOB_DEFAULT = 0x00,
-/* BlockJob is not QMP-created and should not send QMP events */
-BLOCK_JOB_INTERNAL = 0x01,
-/* BlockJob requires manual finalize step */
-BLOCK_JOB_MANUAL_FINALIZE = 0x02,
-/* BlockJob requires manual dismiss step */
-BLOCK_JOB_MANUAL_DISMISS = 0x04,
-} BlockJobCreateFlags;
-
 /**
  * block_job_next:
  * @job: A block job, or %NULL.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 7e705ae0e9..88639f7efc 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -106,8 +106,7 @@ struct BlockJobDriver {
  * @bs: The block
  * @perm, @shared_perm: Permissions to request for @bs
  * @speed: The maximum speed, in bytes per second, or 0 for unlimited.
- * @flags: Creation flags for the Block Job.
- * See @BlockJobCreateFlags
+ * @flags: Creation flags for the Block Job. See @JobCreateFlags.
  * @cb: Completion function for the job.
  * @opaque: Opaque pointer value passed to @cb.
  * @errp: Error object.
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 858f3beea4..9783e4049b 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -99,6 +99,12 @@ typedef struct Job {
 /** Set to true when the job has deferred work to the main loop. */
 bool deferred_to_main_loop;
 
+/** True if this job should automatically finalize itself */
+bool auto_finalize;
+
+/** True if this job should automatically dismiss itself */
+bool auto_dismiss;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 } Job;
@@ -140,6 +146,17 @@ struct JobDriver {
 void (*free)(Job *job);
 };
 
+typedef enum JobCreateFlags {
+/* Default behavior */
+JOB_DEFAULT = 0x00,
+/* Job is not QMP-created and should not send QMP events */
+JOB_INTERNAL = 0x01,
+/* Job requires manual finalize step */
+JOB_MANUAL_FINALIZE = 0x02,
+/* Job requires manual dismiss step */
+JOB_MANUAL_DISMISS = 0x04,
+} JobCreateFlags;
+
 
 /**
  * Create a new long-running job and return it.
@@ -147,10 +164,11 @@ struct JobDriver {
  * @job_id: The id of the newly-created job, or %NULL for internal jobs
  * @driver: The class object for the newly-created job.
  * @ctx: The AioContext to run the job coroutine in.
+ * @flags: Creation flags for the job. See @JobCreateFlags.
  * @errp: Error object.
  */
 void *job_create(const char *job_id, const JobDriver *driver, AioContext *ctx,
- Error **errp);
+ int flags, Error **errp);
 
 /**
  * Add a reference to Job refcnt, it will be decreased with job_unref, and then
diff --git a/block/commit.c b/block/commit.c
index c4a98e5804..7a6ae59d42 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -282,7 +282,7 @@ void commit_start(const char *job_id, BlockDriverState *bs,
 }
 
 s = block_job_create(job_id, _job_driver, NULL, bs, 0, BLK_PERM_ALL,
- speed, BLOCK_JOB_DEFAULT, NULL, NULL, errp);
+ speed, JOB_DEFAULT, NULL, NULL, errp);
 if (!s) {
 return;
 }
diff --git a/block/mirror.c b/block/mirror.c
index 9a7226f38c..5091e72554 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1284,7 +1284,7 @@ void mirror_start(const char *job_id, BlockDriverState 
*bs,
 }
 is_none_mode = mode == MIRROR_SYNC_MODE_NONE;
 base = mode == MIRROR_SYNC_MODE_TOP ? backing_bs(bs) : NULL;
-mirror_start_job(job_id, bs, BLOCK_JOB_DEFAULT, target, replaces,
+mirror_start_job(job_id, 

[Qemu-block] [PULL 19/46] job: Move coroutine and related code to Job

[Kevin Wolf]

This commit moves some core functions for dealing with the job coroutine
from BlockJob to Job. This includes primarily entering the coroutine
(both for the first and reentering) and yielding explicitly and at pause
points.

Signed-off-by: Kevin Wolf 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  40 
 include/block/blockjob_int.h |  26 -
 include/qemu/job.h   |  76 +++
 block/backup.c   |   2 +-
 block/commit.c   |   4 +-
 block/mirror.c   |  22 ++---
 block/replication.c  |   2 +-
 block/stream.c   |   4 +-
 blockdev.c   |   8 +-
 blockjob.c   | 219 ---
 job.c| 137 +++
 tests/test-bdrv-drain.c  |  38 
 tests/test-blockjob-txn.c|  12 +--
 tests/test-blockjob.c|  14 +--
 14 files changed, 305 insertions(+), 299 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 2a9e865e31..b60d919fbf 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -51,43 +51,18 @@ typedef struct BlockJob {
 BlockBackend *blk;
 
 /**
- * The coroutine that executes the job.  If not NULL, it is
- * reentered when busy is false and the job is cancelled.
- */
-Coroutine *co;
-
-/**
  * 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.
- */
-int pause_count;
-
-/**
  * Set to true if the job is paused by user.  Can be unpaused with the
  * block-job-resume QMP command.
  */
 bool user_paused;
 
 /**
- * Set to false by the job while the coroutine has yielded and may be
- * re-entered by block_job_enter().  There may still be I/O or event loop
- * activity pending.  Accessed under block_job_mutex (in blockjob.c).
- */
-bool busy;
-
-/**
- * Set to true by the job while it is in a quiescent state, where
- * no I/O or event loop activity is pending.
- */
-bool paused;
-
-/**
  * Set to true when the job is ready to be completed.
  */
 bool ready;
@@ -125,12 +100,6 @@ typedef struct BlockJob {
 /** ret code passed to block_job_completed. */
 int ret;
 
-/**
- * Timer that is used by @block_job_sleep_ns. Accessed under
- * block_job_mutex (in blockjob.c).
- */
-QEMUTimer sleep_timer;
-
 /** True if this job should automatically finalize itself */
 bool auto_finalize;
 
@@ -208,15 +177,6 @@ void block_job_remove_all_bdrv(BlockJob *job);
 void block_job_set_speed(BlockJob *job, int64_t speed, Error **errp);
 
 /**
- * block_job_start:
- * @job: A job that has not yet been started.
- *
- * Begins execution of a block job.
- * Takes ownership of one reference to the job object.
- */
-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.
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 0c2f8de381..0a614a89b8 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -38,9 +38,6 @@ struct BlockJobDriver {
 /** Generic JobDriver callbacks and settings */
 JobDriver job_driver;
 
-/** Mandatory: Entrypoint for the Coroutine. */
-CoroutineEntry *start;
-
 /**
  * Optional callback for job types whose completion must be triggered
  * manually.
@@ -85,20 +82,6 @@ struct BlockJobDriver {
  */
 void (*clean)(BlockJob *job);
 
-/**
- * If the callback is not NULL, it will be invoked when the job transitions
- * into the paused state.  Paused jobs must not perform any asynchronous
- * I/O or event loop activity.  This callback is used to quiesce jobs.
- */
-void coroutine_fn (*pause)(BlockJob *job);
-
-/**
- * If the callback is not NULL, it will be invoked when the job transitions
- * out of the paused state.  Any asynchronous I/O or event loop activity
- * should be restarted from this callback.
- */
-void coroutine_fn (*resume)(BlockJob *job);
-
 /*
  * If the callback is not NULL, it will be invoked before the job is
  * resumed in a new AioContext.  This is the place to move any resources
@@ -196,15 +179,6 @@ void block_job_early_fail(BlockJob *job);
 void block_job_completed(BlockJob *job, int ret);
 
 /**
- * block_job_pause_point:
- * @job: The job that is ready to pause.
- *
- * Pause now if block_job_pause() has been called.  Block jobs that perform
- * lots of I/O must call this between requests so that the job can be paused.
- */
-void coroutine_fn block_job_pause_point(BlockJob *job);
-

[Qemu-block] [PULL 22/46] job: Replace BlockJob.completed with job_is_completed()

[Kevin Wolf]

Since we introduced an explicit status to block job, BlockJob.completed
is redundant because it can be derived from the status. Remove the field
from BlockJob and add a function to derive it from the status at the Job
level.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  3 ---
 include/qemu/job.h   |  3 +++
 blockjob.c   | 16 +++-
 job.c| 22 ++
 qemu-img.c   |  4 ++--
 5 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 556a8f6375..3e94e18850 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -88,9 +88,6 @@ typedef struct BlockJob {
 /** The opaque value that is passed to the completion function.  */
 void *opaque;
 
-/** True when job has reported completion by calling block_job_completed. 
*/
-bool completed;
-
 /** ret code passed to block_job_completed. */
 int ret;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index bc6398568f..858f3beea4 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -214,6 +214,9 @@ const char *job_type_str(const Job *job);
 /** Returns whether the job is scheduled for cancellation. */
 bool job_is_cancelled(Job *job);
 
+/** Returns whether the job is in a completed state. */
+bool job_is_completed(Job *job);
+
 /**
  * Request @job to pause at the next pause point. Must be paired with
  * job_resume(). If the job is supposed to be resumed by user action, call
diff --git a/blockjob.c b/blockjob.c
index 6334a54f50..a1d1f488e9 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -193,7 +193,7 @@ static void block_job_detach_aio_context(void *opaque)
 
 job_pause(>job);
 
-while (!job->job.paused && !job->completed) {
+while (!job->job.paused && !job_is_completed(>job)) {
 block_job_drain(job);
 }
 
@@ -269,7 +269,6 @@ const BlockJobDriver *block_job_driver(BlockJob *job)
 static void block_job_decommission(BlockJob *job)
 {
 assert(job);
-job->completed = true;
 job->job.busy = false;
 job->job.paused = false;
 job->job.deferred_to_main_loop = true;
@@ -334,7 +333,7 @@ static void block_job_clean(BlockJob *job)
 
 static int block_job_finalize_single(BlockJob *job)
 {
-assert(job->completed);
+assert(job_is_completed(>job));
 
 /* Ensure abort is called for late-transactional failures */
 block_job_update_rc(job);
@@ -427,10 +426,10 @@ static int block_job_finish_sync(BlockJob *job,
 /* block_job_drain calls block_job_enter, and it should be enough to
  * induce progress until the job completes or moves to the main thread.
 */
-while (!job->job.deferred_to_main_loop && !job->completed) {
+while (!job->job.deferred_to_main_loop && !job_is_completed(>job)) {
 block_job_drain(job);
 }
-while (!job->completed) {
+while (!job_is_completed(>job)) {
 aio_poll(qemu_get_aio_context(), true);
 }
 ret = (job_is_cancelled(>job) && job->ret == 0)
@@ -471,7 +470,7 @@ static void block_job_completed_txn_abort(BlockJob *job)
 while (!QLIST_EMPTY(>jobs)) {
 other_job = QLIST_FIRST(>jobs);
 ctx = blk_get_aio_context(other_job->blk);
-if (!other_job->completed) {
+if (!job_is_completed(_job->job)) {
 assert(job_is_cancelled(_job->job));
 block_job_finish_sync(other_job, NULL, NULL);
 }
@@ -513,7 +512,7 @@ static void block_job_completed_txn_success(BlockJob *job)
  * txn.
  */
 QLIST_FOREACH(other_job, >jobs, txn_list) {
-if (!other_job->completed) {
+if (!job_is_completed(_job->job)) {
 return;
 }
 assert(other_job->ret == 0);
@@ -847,9 +846,8 @@ void block_job_early_fail(BlockJob *job)
 
 void block_job_completed(BlockJob *job, int ret)
 {
-assert(job && job->txn && !job->completed);
+assert(job && job->txn && !job_is_completed(>job));
 assert(blk_bs(job->blk)->job == job);
-job->completed = true;
 job->ret = ret;
 block_job_update_rc(job);
 trace_block_job_completed(job, ret, job->ret);
diff --git a/job.c b/job.c
index fd10b1d267..aaacfcc93f 100644
--- a/job.c
+++ b/job.c
@@ -121,6 +121,28 @@ bool job_is_cancelled(Job *job)
 return job->cancelled;
 }
 
+bool job_is_completed(Job *job)
+{
+switch (job->status) {
+case JOB_STATUS_UNDEFINED:
+case JOB_STATUS_CREATED:
+case JOB_STATUS_RUNNING:
+case JOB_STATUS_PAUSED:
+case JOB_STATUS_READY:
+case JOB_STATUS_STANDBY:
+return false;
+case JOB_STATUS_WAITING:
+case JOB_STATUS_PENDING:
+case JOB_STATUS_ABORTING:
+case JOB_STATUS_CONCLUDED:
+case JOB_STATUS_NULL:
+return true;
+default:
+g_assert_not_reached();
+}
+return false;
+}
+
 bool job_started(Job *job)
 {
 return job->co;

[Qemu-block] [PULL 17/46] job: Add Job.aio_context

[Kevin Wolf]

When block jobs need an AioContext, they just take it from their main
block node. Generic jobs don't have a main block node, so we need to
assign them an AioContext explicitly.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/qemu/job.h | 7 ++-
 blockjob.c | 5 -
 job.c  | 4 +++-
 3 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 5dfbec5d69..01e083f97a 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -47,6 +47,9 @@ typedef struct Job {
 /** Current state; See @JobStatus for details. */
 JobStatus status;
 
+/** AioContext to run the job coroutine in */
+AioContext *aio_context;
+
 /**
  * Set to true if the job should cancel itself.  The flag must
  * always be tested just before toggling the busy flag from false
@@ -79,9 +82,11 @@ struct JobDriver {
  *
  * @job_id: The id of the newly-created job, or %NULL for internal jobs
  * @driver: The class object for the newly-created job.
+ * @ctx: The AioContext to run the job coroutine in.
  * @errp: Error object.
  */
-void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
+void *job_create(const char *job_id, const JobDriver *driver, AioContext *ctx,
+ Error **errp);
 
 /**
  * Add a reference to Job refcnt, it will be decreased with job_unref, and then
diff --git a/blockjob.c b/blockjob.c
index f4f9956678..0a0b1c41dd 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -216,6 +216,7 @@ static void block_job_attached_aio_context(AioContext 
*new_context,
 {
 BlockJob *job = opaque;
 
+job->job.aio_context = new_context;
 if (job->driver->attached_aio_context) {
 job->driver->attached_aio_context(job, new_context);
 }
@@ -247,6 +248,7 @@ static void block_job_detach_aio_context(void *opaque)
 block_job_drain(job);
 }
 
+job->job.aio_context = NULL;
 job_unref(>job);
 }
 
@@ -899,7 +901,8 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 return NULL;
 }
 
-job = job_create(job_id, >job_driver, errp);
+job = job_create(job_id, >job_driver, blk_get_aio_context(blk),
+ errp);
 if (job == NULL) {
 blk_unref(blk);
 return NULL;
diff --git a/job.c b/job.c
index 1abca6a2fc..01074d0fae 100644
--- a/job.c
+++ b/job.c
@@ -121,7 +121,8 @@ Job *job_get(const char *id)
 return NULL;
 }
 
-void *job_create(const char *job_id, const JobDriver *driver, Error **errp)
+void *job_create(const char *job_id, const JobDriver *driver, AioContext *ctx,
+ Error **errp)
 {
 Job *job;
 
@@ -140,6 +141,7 @@ void *job_create(const char *job_id, const JobDriver 
*driver, Error **errp)
 job->driver= driver;
 job->id= g_strdup(job_id);
 job->refcnt= 1;
+job->aio_context   = ctx;
 
 job_state_transition(job, JOB_STATUS_CREATED);
 
-- 
2.13.6

[Qemu-block] [PULL 24/46] blockjob: Split block_job_event_pending()

[Kevin Wolf]

block_job_event_pending() doesn't only send a QMP event, but it also
transitions to the PENDING state. Split the function so that we get one
part only sending the event (like other block_job_event_* functions) and
another part that does the state transition.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
---
 blockjob.c | 27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/blockjob.c b/blockjob.c
index d9d8ff7f5c..b4334fb1bf 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -38,7 +38,7 @@
 
 static void block_job_event_cancelled(BlockJob *job);
 static void block_job_event_completed(BlockJob *job, const char *msg);
-static int block_job_event_pending(BlockJob *job);
+static void block_job_event_pending(BlockJob *job);
 
 /* Transactional group of block jobs */
 struct BlockJobTxn {
@@ -500,6 +500,15 @@ static void block_job_do_finalize(BlockJob *job)
 }
 }
 
+static int block_job_transition_to_pending(BlockJob *job)
+{
+job_state_transition(>job, JOB_STATUS_PENDING);
+if (!job->job.auto_finalize) {
+block_job_event_pending(job);
+}
+return 0;
+}
+
 static void block_job_completed_txn_success(BlockJob *job)
 {
 BlockJobTxn *txn = job->txn;
@@ -518,7 +527,7 @@ static void block_job_completed_txn_success(BlockJob *job)
 assert(other_job->ret == 0);
 }
 
-block_job_txn_apply(txn, block_job_event_pending, false);
+block_job_txn_apply(txn, block_job_transition_to_pending, false);
 
 /* If no jobs need manual finalization, automatically do so */
 if (block_job_txn_apply(txn, block_job_needs_finalize, false) == 0) {
@@ -733,15 +742,15 @@ static void block_job_event_completed(BlockJob *job, 
const char *msg)
 _abort);
 }
 
-static int block_job_event_pending(BlockJob *job)
+static void block_job_event_pending(BlockJob *job)
 {
-job_state_transition(>job, JOB_STATUS_PENDING);
-if (!job->job.auto_finalize && !block_job_is_internal(job)) {
-qapi_event_send_block_job_pending(job_type(>job),
-  job->job.id,
-  _abort);
+if (block_job_is_internal(job)) {
+return;
 }
-return 0;
+
+qapi_event_send_block_job_pending(job_type(>job),
+  job->job.id,
+  _abort);
 }
 
 /*
-- 
2.13.6

[Qemu-block] [PULL 12/46] job: Add job_delete()

[Kevin Wolf]

This moves freeing the Job object and its fields from block_job_unref()
to job_delete().

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/qemu/job.h | 3 +++
 blockjob.c | 3 +--
 job.c  | 6 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/qemu/job.h b/include/qemu/job.h
index 279ce688fd..43dc2e4a7d 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -62,6 +62,9 @@ struct JobDriver {
  */
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
 
+/** Frees the @job object. */
+void job_delete(Job *job);
+
 /** Returns the JobType of a given Job. */
 JobType job_type(const Job *job);
 
diff --git a/blockjob.c b/blockjob.c
index ea71ec0129..430a67ba63 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -261,9 +261,8 @@ void block_job_unref(BlockJob *job)
 block_job_detach_aio_context, job);
 blk_unref(job->blk);
 error_free(job->blocker);
-g_free(job->job.id);
 assert(!timer_pending(>sleep_timer));
-g_free(job);
+job_delete(>job);
 }
 }
 
diff --git a/job.c b/job.c
index 83724a43de..cfdd008c52 100644
--- a/job.c
+++ b/job.c
@@ -56,3 +56,9 @@ void *job_create(const char *job_id, const JobDriver *driver, 
Error **errp)
 
 return job;
 }
+
+void job_delete(Job *job)
+{
+g_free(job->id);
+g_free(job);
+}
-- 
2.13.6

[Qemu-block] [PULL 18/46] job: Move defer_to_main_loop to Job

[Kevin Wolf]

Move the defer_to_main_loop functionality from BlockJob to Job.

The code can be simplified because we can use job->aio_context in
job_defer_to_main_loop_bh() now, instead of having to access the
BlockDriverState.

Probably taking the data->aio_context lock in addition was already
unnecessary in the old code because we didn't actually make use of
anything protected by the old AioContext except getting the new
AioContext, in case it changed between scheduling the BH and running it.
But it's certainly unnecessary now that the BDS isn't accessed at all
any more.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  5 
 include/block/blockjob_int.h | 19 ---
 include/qemu/job.h   | 20 
 block/backup.c   |  7 +++---
 block/commit.c   | 11 +
 block/mirror.c   | 15 ++--
 block/stream.c   | 14 +--
 blockjob.c   | 57 
 job.c| 32 +
 tests/test-bdrv-drain.c  |  7 +++---
 tests/test-blockjob-txn.c| 13 +-
 tests/test-blockjob.c|  7 +++---
 12 files changed, 97 insertions(+), 110 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 1e708f468a..2a9e865e31 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -92,11 +92,6 @@ typedef struct BlockJob {
  */
 bool ready;
 
-/**
- * Set to true when the job has deferred work to the main loop.
- */
-bool deferred_to_main_loop;
-
 /** Status that is published by the query-block-jobs QMP API */
 BlockDeviceIoStatus iostatus;
 
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index d64f30e6b0..0c2f8de381 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -233,23 +233,4 @@ void block_job_event_ready(BlockJob *job);
 BlockErrorAction block_job_error_action(BlockJob *job, BlockdevOnError on_err,
 int is_read, int error);
 
-typedef void BlockJobDeferToMainLoopFn(BlockJob *job, void *opaque);
-
-/**
- * block_job_defer_to_main_loop:
- * @job: The job
- * @fn: The function to run in the main loop
- * @opaque: The opaque value that is passed to @fn
- *
- * This function must be called by the main job coroutine just before it
- * returns.  @fn is executed in the main loop with the BlockDriverState
- * AioContext acquired.  Block jobs must call bdrv_unref(), bdrv_close(), and
- * anything that uses bdrv_drain_all() in the main loop.
- *
- * The @job AioContext is held while @fn executes.
- */
-void block_job_defer_to_main_loop(BlockJob *job,
-  BlockJobDeferToMainLoopFn *fn,
-  void *opaque);
-
 #endif
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 01e083f97a..933e0ab328 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -58,6 +58,9 @@ typedef struct Job {
  */
 bool cancelled;
 
+/** Set to true when the job has deferred work to the main loop. */
+bool deferred_to_main_loop;
+
 /** Element of the list of jobs */
 QLIST_ENTRY(Job) job_list;
 } Job;
@@ -131,6 +134,23 @@ Job *job_get(const char *id);
  */
 int job_apply_verb(Job *job, JobVerb verb, Error **errp);
 
+typedef void JobDeferToMainLoopFn(Job *job, void *opaque);
+
+/**
+ * @job: The job
+ * @fn: The function to run in the main loop
+ * @opaque: The opaque value that is passed to @fn
+ *
+ * This function must be called by the main job coroutine just before it
+ * returns.  @fn is executed in the main loop with the job AioContext acquired.
+ *
+ * Block jobs must call bdrv_unref(), bdrv_close(), and anything that uses
+ * bdrv_drain_all() in the main loop.
+ *
+ * The @job AioContext is held while @fn executes.
+ */
+void job_defer_to_main_loop(Job *job, JobDeferToMainLoopFn *fn, void *opaque);
+
 /* TODO To be removed from the public interface */
 void job_state_transition(Job *job, JobStatus s1);
 
diff --git a/block/backup.c b/block/backup.c
index ef0aa0e24e..22dd368c90 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -317,11 +317,12 @@ typedef struct {
 int ret;
 } BackupCompleteData;
 
-static void backup_complete(BlockJob *job, void *opaque)
+static void backup_complete(Job *job, void *opaque)
 {
+BlockJob *bjob = container_of(job, BlockJob, job);
 BackupCompleteData *data = opaque;
 
-block_job_completed(job, data->ret);
+block_job_completed(bjob, data->ret);
 g_free(data);
 }
 
@@ -519,7 +520,7 @@ static void coroutine_fn backup_run(void *opaque)
 
 data = g_malloc(sizeof(*data));
 data->ret = ret;
-block_job_defer_to_main_loop(>common, backup_complete, data);
+job_defer_to_main_loop(>common.job, backup_complete, data);
 }
 
 static const 

[Qemu-block] [PULL 13/46] job: Maintain a list of all jobs

[Kevin Wolf]

This moves the job list from BlockJob to Job. Now we can check for
duplicate IDs in job_create().

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  3 ---
 include/qemu/job.h   | 19 +++
 blockjob.c   | 46 --
 job.c| 31 +++
 4 files changed, 74 insertions(+), 25 deletions(-)

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 640e649034..10bd9f7059 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -105,9 +105,6 @@ typedef struct BlockJob {
  */
 bool deferred_to_main_loop;
 
-/** Element of the list of block jobs */
-QLIST_ENTRY(BlockJob) job_list;
-
 /** Status that is published by the query-block-jobs QMP API */
 BlockDeviceIoStatus iostatus;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index 43dc2e4a7d..bae2b0920c 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -27,6 +27,7 @@
 #define JOB_H
 
 #include "qapi/qapi-types-block-core.h"
+#include "qemu/queue.h"
 
 typedef struct JobDriver JobDriver;
 
@@ -39,6 +40,9 @@ typedef struct Job {
 
 /** The type of this job. */
 const JobDriver *driver;
+
+/** Element of the list of jobs */
+QLIST_ENTRY(Job) job_list;
 } Job;
 
 /**
@@ -71,4 +75,19 @@ JobType job_type(const Job *job);
 /** Returns the enum string for the JobType of a given Job. */
 const char *job_type_str(const Job *job);
 
+/**
+ * Get the next element from the list of block jobs after @job, or the
+ * first one if @job is %NULL.
+ *
+ * Returns the requested job, or %NULL if there are no more jobs left.
+ */
+Job *job_next(Job *job);
+
+/**
+ * Get the job identified by @id (which must not be %NULL).
+ *
+ * Returns the requested job, or %NULL if it doesn't exist.
+ */
+Job *job_get(const char *id);
+
 #endif
diff --git a/blockjob.c b/blockjob.c
index 430a67ba63..c69b2e7cf5 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -129,8 +129,6 @@ struct BlockJobTxn {
 int refcnt;
 };
 
-static QLIST_HEAD(, BlockJob) block_jobs = QLIST_HEAD_INITIALIZER(block_jobs);
-
 /*
  * The block job API is composed of two categories of functions.
  *
@@ -146,25 +144,34 @@ static QLIST_HEAD(, BlockJob) block_jobs = 
QLIST_HEAD_INITIALIZER(block_jobs);
  * blockjob_int.h.
  */
 
-BlockJob *block_job_next(BlockJob *job)
+static bool is_block_job(Job *job)
 {
-if (!job) {
-return QLIST_FIRST(_jobs);
-}
-return QLIST_NEXT(job, job_list);
+return job_type(job) == JOB_TYPE_BACKUP ||
+   job_type(job) == JOB_TYPE_COMMIT ||
+   job_type(job) == JOB_TYPE_MIRROR ||
+   job_type(job) == JOB_TYPE_STREAM;
+}
+
+BlockJob *block_job_next(BlockJob *bjob)
+{
+Job *job = bjob ? >job : NULL;
+
+do {
+job = job_next(job);
+} while (job && !is_block_job(job));
+
+return job ? container_of(job, BlockJob, job) : NULL;
 }
 
 BlockJob *block_job_get(const char *id)
 {
-BlockJob *job;
+Job *job = job_get(id);
 
-QLIST_FOREACH(job, _jobs, job_list) {
-if (job->job.id && !strcmp(id, job->job.id)) {
-return job;
-}
+if (job && is_block_job(job)) {
+return container_of(job, BlockJob, job);
+} else {
+return NULL;
 }
-
-return NULL;
 }
 
 BlockJobTxn *block_job_txn_new(void)
@@ -253,7 +260,6 @@ void block_job_unref(BlockJob *job)
 assert(job->status == BLOCK_JOB_STATUS_NULL);
 assert(!job->txn);
 BlockDriverState *bs = blk_bs(job->blk);
-QLIST_REMOVE(job, job_list);
 bs->job = NULL;
 block_job_remove_all_bdrv(job);
 blk_remove_aio_context_notifier(job->blk,
@@ -812,7 +818,7 @@ void block_job_cancel_sync_all(void)
 BlockJob *job;
 AioContext *aio_context;
 
-while ((job = QLIST_FIRST(_jobs))) {
+while ((job = block_job_next(NULL))) {
 aio_context = blk_get_aio_context(job->blk);
 aio_context_acquire(aio_context);
 block_job_cancel_sync(job);
@@ -942,10 +948,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 error_setg(errp, "Cannot specify job ID for internal block job");
 return NULL;
 }
-if (block_job_get(job_id)) {
-error_setg(errp, "Job ID '%s' already in use", job_id);
-return NULL;
-}
 }
 
 blk = blk_new(perm, shared_perm);
@@ -961,6 +963,8 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 return NULL;
 }
 
+assert(is_block_job(>job));
+
 job->driver= driver;
 job->blk   = blk;
 job->cb= cb;
@@ -983,8 +987,6 @@ void *block_job_create(const char *job_id, const 
BlockJobDriver *driver,
 
 bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
 
-QLIST_INSERT_HEAD(_jobs, 

[Qemu-block] [PULL 05/46] qemu-iotests: Add more tests to "migration" group

[Kevin Wolf]

grep for "migrate" turns up a few test cases which use migration, but
haven't been in the "migration" group so far. Add them to the group.

Signed-off-by: Kevin Wolf 
Reviewed-by: Juan Quintela 
---
 tests/qemu-iotests/group | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index cc8cd8cc8e..60ba31b292 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -97,7 +97,7 @@
 088 rw auto quick
 089 rw auto quick
 090 rw auto quick
-091 rw auto
+091 rw auto migration
 092 rw auto quick
 093 auto
 094 rw auto quick
@@ -169,7 +169,7 @@
 162 auto quick
 163 rw auto
 165 rw auto quick
-169 rw auto quick
+169 rw auto quick migration
 170 rw auto quick
 171 rw auto quick
 172 auto
@@ -194,14 +194,14 @@
 192 rw auto quick
 194 rw auto migration quick
 195 rw auto quick
-196 rw auto quick
+196 rw auto quick migration
 197 rw auto quick
 198 rw auto
-199 rw auto
+199 rw auto migration
 200 rw auto
 201 rw auto migration
 202 rw auto quick
-203 rw auto
+203 rw auto migration
 204 rw auto quick
 205 rw auto quick
 206 rw auto
-- 
2.13.6

[Qemu-block] [PULL 04/46] sheepdog: Remove unnecessary NULL check in sd_prealloc()

[Kevin Wolf]

From: Peter Maydell 

In commit 8b9ad56e9cbfd852a, we removed the code that could result
in our getting to sd_prealloc()'s out_with_err_set label with a
NULL blk pointer. That makes the NULL check in the error-handling
path unnecessary, and Coverity gripes about it (CID 1390636).
Delete the redundant check.

Signed-off-by: Peter Maydell 
Signed-off-by: Kevin Wolf 
---
 block/sheepdog.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 4237132419..2a5bc0a59a 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1859,9 +1859,7 @@ out:
 error_setg_errno(errp, -ret, "Can't pre-allocate");
 }
 out_with_err_set:
-if (blk) {
-blk_unref(blk);
-}
+blk_unref(blk);
 g_free(buf);
 
 return ret;
-- 
2.13.6

[Qemu-block] [PULL 00/46] Block layer patches

[Kevin Wolf]

The following changes since commit 4f50c1673a89b07f376ce5c42d22d79a79cd466d:

  Merge remote-tracking branch 'remotes/ehabkost/tags/x86-next-pull-request' 
into staging (2018-05-22 09:43:58 +0100)

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to bdebdc712b06ba82e103d617c335830682cde242:

  qemu-iotests: Test job-* with block jobs (2018-05-23 14:30:52 +0200)


Block layer patches:

- Generic background jobs
- qemu-iotests fixes for NFS and the 'migration' group
- sheepdog: Minor code simplification


Kevin Wolf (45):
  qemu-iotests: Fix paths for NFS
  qemu-iotests: Filter NFS paths
  qemu-iotests: 086 doesn't work with NFS
  qemu-iotests: Add more tests to "migration" group
  qemu-iotests: Remove MIG_SOCKET from non-migration tests
  blockjob: Update block-job-pause/resume documentation
  blockjob: Improve BlockJobInfo.offset/len documentation
  job: Create Job, JobDriver and job_create()
  job: Rename BlockJobType into JobType
  job: Add JobDriver.job_type
  job: Add job_delete()
  job: Maintain a list of all jobs
  job: Move state transitions to Job
  job: Add reference counting
  job: Move cancelled to Job
  job: Add Job.aio_context
  job: Move defer_to_main_loop to Job
  job: Move coroutine and related code to Job
  job: Add job_sleep_ns()
  job: Move pause/resume functions to Job
  job: Replace BlockJob.completed with job_is_completed()
  job: Move BlockJobCreateFlags to Job
  blockjob: Split block_job_event_pending()
  job: Add job_event_*()
  job: Move single job finalisation to Job
  job: Convert block_job_cancel_async() to Job
  job: Add job_drain()
  job: Move .complete callback to Job
  job: Move job_finish_sync() to Job
  job: Switch transactions to JobTxn
  job: Move transactions to Job
  job: Move completion and cancellation to Job
  block: Cancel job in bdrv_close_all() callers
  job: Add job_yield()
  job: Add job_dismiss()
  job: Add job_is_ready()
  job: Add job_transition_to_ready()
  job: Move progress fields to Job
  job: Introduce qapi/job.json
  job: Add JOB_STATUS_CHANGE QMP event
  job: Add lifecycle QMP commands
  job: Add query-jobs QMP command
  blockjob: Remove BlockJob.driver
  iotests: Move qmp_to_opts() to VM
  qemu-iotests: Test job-* with block jobs

Peter Maydell (1):
  sheepdog: Remove unnecessary NULL check in sd_prealloc()

 qapi/block-core.json |  116 +---
 qapi/job.json|  253 +
 qapi/qapi-schema.json|1 +
 include/block/block_int.h|2 +-
 include/block/blockjob.h |  324 +--
 include/block/blockjob_int.h |  176 +-
 include/qemu/job.h   |  562 
 block.c  |2 +-
 block/backup.c   |   59 +-
 block/commit.c   |   44 +-
 block/mirror.c   |  113 ++--
 block/replication.c  |   10 +-
 block/sheepdog.c |4 +-
 block/stream.c   |   39 +-
 blockdev.c   |   68 +--
 blockjob.c   | 1094 ++
 job-qmp.c|  188 +++
 job.c| 1000 ++
 qemu-img.c   |   22 +-
 qemu-nbd.c   |8 +-
 tests/test-bdrv-drain.c  |   63 ++-
 tests/test-blockjob-txn.c|   74 +--
 tests/test-blockjob.c|  141 ++---
 vl.c |1 +
 MAINTAINERS  |4 +
 Makefile |9 +
 Makefile.objs|7 +-
 block/trace-events   |5 -
 tests/qemu-iotests/030   |   17 +-
 tests/qemu-iotests/040   |2 +
 tests/qemu-iotests/041   |   23 +-
 tests/qemu-iotests/086   |2 +-
 tests/qemu-iotests/094.out   |7 +
 tests/qemu-iotests/095   |2 +-
 tests/qemu-iotests/095.out   |6 +
 tests/qemu-iotests/109   |2 +-
 tests/qemu-iotests/109.out   |  178 ++-
 tests/qemu-iotests/124   |8 +
 tests/qemu-iotests/126.out   |2 +-
 tests/qemu-iotests/127.out   |7 +
 tests/qemu-iotests/141   |   13 +-
 tests/qemu-iotests/141.out   |   29 +
 tests/qemu-iotests/144   |2 +-
 tests/qemu-iotests/144.out   |7 +
 tests/qemu-iotests/155   |2 +-
 tests/qemu-iotests/156   |2 +-
 tests/qemu-iotests/156.out   |7 +
 tests/qemu-iotests/185   |   14 +-
 tests/qemu-iotests/185.out   |   10 +
 tests/qemu-iotests/191   |

[Qemu-block] [PULL 07/46] blockjob: Update block-job-pause/resume documentation

[Kevin Wolf]

Commit 0ec4dfb8d changed block-job_pause/resume so that they return an
error if they don't do anything because the job is already
paused/running. It forgot to update the documentation, so do that now.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
---
 qapi/block-core.json | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 55728cb823..d32ec95666 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2338,8 +2338,7 @@
 #
 # This command returns immediately after marking the active background block
 # operation for pausing.  It is an error to call this command if no
-# operation is in progress.  Pausing an already paused job has no cumulative
-# effect; a single block-job-resume command will resume the job.
+# operation is in progress or if the job is already paused.
 #
 # The operation will pause as soon as possible.  No event is emitted when
 # the operation is actually paused.  Cancelling a paused job automatically
@@ -2363,7 +2362,7 @@
 #
 # This command returns immediately after resuming a paused background block
 # operation.  It is an error to call this command if no operation is in
-# progress.  Resuming an already running job is not an error.
+# progress or if the job is not paused.
 #
 # This command also clears the error status of the job.
 #
-- 
2.13.6

[Qemu-block] [PULL 09/46] job: Create Job, JobDriver and job_create()

[Kevin Wolf]

This is the first step towards creating an infrastructure for generic
background jobs that aren't tied to a block device. For now, Job only
stores its ID and JobDriver, the rest stays in BlockJob.

The following patches will move over more parts of BlockJob to Job if
they are meaningful outside the context of a block job.

BlockJob.driver is now redundant, but this patch leaves it around to
avoid unnecessary churn. The next patches will get rid of almost all of
its uses anyway so that it can be removed later with much less churn.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob.h |  9 +++
 include/block/blockjob_int.h |  4 +--
 include/qemu/job.h   | 60 
 block/backup.c   |  4 ++-
 block/commit.c   |  4 ++-
 block/mirror.c   | 10 +---
 block/stream.c   |  4 ++-
 blockjob.c   | 46 -
 job.c| 48 +++
 tests/test-bdrv-drain.c  |  4 ++-
 tests/test-blockjob-txn.c|  4 ++-
 tests/test-blockjob.c| 12 ++---
 MAINTAINERS  |  2 ++
 Makefile.objs|  2 +-
 14 files changed, 169 insertions(+), 44 deletions(-)
 create mode 100644 include/qemu/job.h
 create mode 100644 job.c

diff --git a/include/block/blockjob.h b/include/block/blockjob.h
index 0f56f723de..640e649034 100644
--- a/include/block/blockjob.h
+++ b/include/block/blockjob.h
@@ -26,6 +26,7 @@
 #ifndef BLOCKJOB_H
 #define BLOCKJOB_H
 
+#include "qemu/job.h"
 #include "block/block.h"
 #include "qemu/ratelimit.h"
 
@@ -40,6 +41,9 @@ typedef struct BlockJobTxn BlockJobTxn;
  * Long-running operation on a BlockDriverState.
  */
 typedef struct BlockJob {
+/** Data belonging to the generic Job infrastructure */
+Job job;
+
 /** The job type, including the job vtable.  */
 const BlockJobDriver *driver;
 
@@ -47,11 +51,6 @@ typedef struct BlockJob {
 BlockBackend *blk;
 
 /**
- * The ID of the block job. May be NULL for internal jobs.
- */
-char *id;
-
-/**
  * The coroutine that executes the job.  If not NULL, it is
  * reentered when busy is false and the job is cancelled.
  */
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 62ec964d09..e8eca44747 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -35,8 +35,8 @@
  * A class type for block job driver.
  */
 struct BlockJobDriver {
-/** Derived BlockJob struct size */
-size_t instance_size;
+/** Generic JobDriver callbacks and settings */
+JobDriver job_driver;
 
 /** String describing the operation, part of query-block-jobs QMP API */
 BlockJobType job_type;
diff --git a/include/qemu/job.h b/include/qemu/job.h
new file mode 100644
index 00..b4b49f19e1
--- /dev/null
+++ b/include/qemu/job.h
@@ -0,0 +1,60 @@
+/*
+ * Declarations for background jobs
+ *
+ * Copyright (c) 2011 IBM Corp.
+ * Copyright (c) 2012, 2018 Red Hat, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef JOB_H
+#define JOB_H
+
+typedef struct JobDriver JobDriver;
+
+/**
+ * Long-running operation.
+ */
+typedef struct Job {
+/** The ID of the job. May be NULL for internal jobs. */
+char *id;
+
+/** The type of this job. */
+const JobDriver *driver;
+} Job;
+
+/**
+ * Callbacks and other information about a Job driver.
+ */
+struct JobDriver {
+/** Derived Job struct size */
+size_t instance_size;
+};
+
+
+/**
+ * Create a new long-running job and return it.
+ *
+ * @job_id: The id of the newly-created job, or %NULL for internal jobs
+ * @driver: The class object for the newly-created job.
+ * @errp: Error object.
+ */
+void *job_create(const char *job_id, const JobDriver 

[Qemu-block] [PULL 11/46] job: Add JobDriver.job_type

[Kevin Wolf]

This moves the job_type field from BlockJobDriver to JobDriver.

Signed-off-by: Kevin Wolf 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 include/block/blockjob_int.h |  3 ---
 include/qemu/job.h   | 11 +++
 block/backup.c   |  2 +-
 block/commit.c   |  2 +-
 block/mirror.c   |  4 ++--
 block/stream.c   |  2 +-
 blockjob.c   | 16 +++-
 job.c| 10 ++
 8 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index 8e7e0a2f57..1e62d6dd30 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -38,9 +38,6 @@ struct BlockJobDriver {
 /** Generic JobDriver callbacks and settings */
 JobDriver job_driver;
 
-/** String describing the operation, part of query-block-jobs QMP API */
-JobType job_type;
-
 /** Mandatory: Entrypoint for the Coroutine. */
 CoroutineEntry *start;
 
diff --git a/include/qemu/job.h b/include/qemu/job.h
index b4b49f19e1..279ce688fd 100644
--- a/include/qemu/job.h
+++ b/include/qemu/job.h
@@ -26,6 +26,8 @@
 #ifndef JOB_H
 #define JOB_H
 
+#include "qapi/qapi-types-block-core.h"
+
 typedef struct JobDriver JobDriver;
 
 /**
@@ -45,6 +47,9 @@ typedef struct Job {
 struct JobDriver {
 /** Derived Job struct size */
 size_t instance_size;
+
+/** Enum describing the operation */
+JobType job_type;
 };
 
 
@@ -57,4 +62,10 @@ struct JobDriver {
  */
 void *job_create(const char *job_id, const JobDriver *driver, Error **errp);
 
+/** Returns the JobType of a given Job. */
+JobType job_type(const Job *job);
+
+/** Returns the enum string for the JobType of a given Job. */
+const char *job_type_str(const Job *job);
+
 #endif
diff --git a/block/backup.c b/block/backup.c
index c49ea92dca..baf8d432da 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -525,8 +525,8 @@ static void coroutine_fn backup_run(void *opaque)
 static const BlockJobDriver backup_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(BackupBlockJob),
+.job_type   = JOB_TYPE_BACKUP,
 },
-.job_type   = JOB_TYPE_BACKUP,
 .start  = backup_run,
 .commit = backup_commit,
 .abort  = backup_abort,
diff --git a/block/commit.c b/block/commit.c
index afa2b2bacf..32d29c890e 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -217,8 +217,8 @@ out:
 static const BlockJobDriver commit_job_driver = {
 .job_driver = {
 .instance_size = sizeof(CommitBlockJob),
+.job_type  = JOB_TYPE_COMMIT,
 },
-.job_type  = JOB_TYPE_COMMIT,
 .start = commit_run,
 };
 
diff --git a/block/mirror.c b/block/mirror.c
index ed72656a23..35fcc1f0b7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -988,8 +988,8 @@ static void mirror_drain(BlockJob *job)
 static const BlockJobDriver mirror_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
+.job_type   = JOB_TYPE_MIRROR,
 },
-.job_type   = JOB_TYPE_MIRROR,
 .start  = mirror_run,
 .complete   = mirror_complete,
 .pause  = mirror_pause,
@@ -1000,8 +1000,8 @@ static const BlockJobDriver mirror_job_driver = {
 static const BlockJobDriver commit_active_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
+.job_type   = JOB_TYPE_COMMIT,
 },
-.job_type   = JOB_TYPE_COMMIT,
 .start  = mirror_run,
 .complete   = mirror_complete,
 .pause  = mirror_pause,
diff --git a/block/stream.c b/block/stream.c
index 048bceb5d0..cb723f190a 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -211,8 +211,8 @@ out:
 static const BlockJobDriver stream_job_driver = {
 .job_driver = {
 .instance_size = sizeof(StreamBlockJob),
+.job_type  = JOB_TYPE_STREAM,
 },
-.job_type  = JOB_TYPE_STREAM,
 .start = stream_run,
 };
 
diff --git a/blockjob.c b/blockjob.c
index 2a3844721e..ea71ec0129 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -309,9 +309,7 @@ static void block_job_detach_aio_context(void *opaque)
 static char *child_job_get_parent_desc(BdrvChild *c)
 {
 BlockJob *job = c->opaque;
-return g_strdup_printf("%s job '%s'",
-   JobType_str(job->driver->job_type),
-   job->job.id);
+return g_strdup_printf("%s job '%s'", job_type_str(>job), 
job->job.id);
 }
 
 static void child_job_drained_begin(BdrvChild *c)
@@ -847,7 +845,7 @@ BlockJobInfo *block_job_query(BlockJob *job, Error **errp)
 return NULL;
 }
 info = g_new0(BlockJobInfo, 1);
-info->type  = 

[Qemu-block] [PULL 08/46] blockjob: Improve BlockJobInfo.offset/len documentation

[Kevin Wolf]

Clarify that len is just an estimation of the end value of offset, and
that offset increases monotonically while len can change arbitrarily.

While touching the documentation of offset, move it directly after len
to match the order of the declaration below.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: John Snow 
---
 qapi/block-core.json | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index d32ec95666..0e29abf099 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1148,7 +1148,12 @@
 # @device: The job identifier. Originally the device name but other
 #  values are allowed since QEMU 2.7
 #
-# @len: the maximum progress value
+# @len: Estimated @offset value at the completion of the job. This value can
+#   arbitrarily change while the job is running, in both directions.
+#
+# @offset: Progress made until now. The unit is arbitrary and the value can
+#  only meaningfully be used for the ratio of @offset to @len. The
+#  value is monotonically increasing.
 #
 # @busy: false if the job is known to be in a quiescent state, with
 #no pending I/O.  Since 1.3.
@@ -1156,8 +1161,6 @@
 # @paused: whether the job is paused or, if @busy is true, will
 #  pause itself as soon as possible.  Since 1.3.
 #
-# @offset: the current progress value
-#
 # @speed: the rate limit, bytes per second
 #
 # @io-status: the status of the job (since 1.3)
-- 
2.13.6

[Qemu-block] [PULL 03/46] qemu-iotests: 086 doesn't work with NFS

[Kevin Wolf]

The reference output file only works for file. 'qemu-img convert -p'
makes a lot more progress updates for NFS than for file, so disable the
test for NFS.

Signed-off-by: Kevin Wolf 
Reviewed-by: Fam Zheng 
---
 tests/qemu-iotests/086 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/086 b/tests/qemu-iotests/086
index cd4494a660..84e3835071 100755
--- a/tests/qemu-iotests/086
+++ b/tests/qemu-iotests/086
@@ -38,7 +38,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 . ./common.filter
 
 _supported_fmt qcow2 raw
-_supported_proto file nfs
+_supported_proto file
 _supported_os Linux
 
 function run_qemu_img()
-- 
2.13.6

[Qemu-block] [PULL 06/46] qemu-iotests: Remove MIG_SOCKET from non-migration tests

[Kevin Wolf]

185 and 191 define a MIG_SOCKET even though they don't do anything with
migration. Remove the useless variable.

Signed-off-by: Kevin Wolf 
Reviewed-by: Juan Quintela 
---
 tests/qemu-iotests/185 | 2 --
 tests/qemu-iotests/191 | 2 --
 2 files changed, 4 deletions(-)

diff --git a/tests/qemu-iotests/185 b/tests/qemu-iotests/185
index 9a2d317414..deb42cc886 100755
--- a/tests/qemu-iotests/185
+++ b/tests/qemu-iotests/185
@@ -27,8 +27,6 @@ echo "QA output created by $seq"
 here=`pwd`
 status=1 # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
-
 _cleanup()
 {
 rm -f "${TEST_IMG}.mid"
diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191
index dfad6555e4..77224eb151 100755
--- a/tests/qemu-iotests/191
+++ b/tests/qemu-iotests/191
@@ -27,8 +27,6 @@ echo "QA output created by $seq"
 here=`pwd`
 status=1 # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
-
 _cleanup()
 {
 rm -f "${TEST_IMG}.mid"
-- 
2.13.6

[Qemu-block] [PULL 10/46] job: Rename BlockJobType into JobType

[Kevin Wolf]

QAPI types aren't externally visible, so we can rename them without
causing problems. Before we add a job type to Job, rename the enum
so it can be used for more than just block jobs.

Signed-off-by: Kevin Wolf 
Reviewed-by: Eric Blake 
Reviewed-by: Max Reitz 
Reviewed-by: John Snow 
---
 qapi/block-core.json | 14 +++---
 include/block/blockjob_int.h |  2 +-
 block/backup.c   |  2 +-
 block/commit.c   |  2 +-
 block/mirror.c   |  4 ++--
 block/stream.c   |  2 +-
 blockjob.c   |  6 +++---
 7 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 0e29abf099..63c6011411 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1050,9 +1050,9 @@
   'data': ['top', 'full', 'none', 'incremental'] }
 
 ##
-# @BlockJobType:
+# @JobType:
 #
-# Type of a block job.
+# Type of a background job.
 #
 # @commit: block commit job type, see "block-commit"
 #
@@ -1064,7 +1064,7 @@
 #
 # Since: 1.7
 ##
-{ 'enum': 'BlockJobType',
+{ 'enum': 'JobType',
   'data': ['commit', 'stream', 'mirror', 'backup'] }
 
 ##
@@ -4499,7 +4499,7 @@
 #
 ##
 { 'event': 'BLOCK_JOB_COMPLETED',
-  'data': { 'type'  : 'BlockJobType',
+  'data': { 'type'  : 'JobType',
 'device': 'str',
 'len'   : 'int',
 'offset': 'int',
@@ -4535,7 +4535,7 @@
 #
 ##
 { 'event': 'BLOCK_JOB_CANCELLED',
-  'data': { 'type'  : 'BlockJobType',
+  'data': { 'type'  : 'JobType',
 'device': 'str',
 'len'   : 'int',
 'offset': 'int',
@@ -4600,7 +4600,7 @@
 #
 ##
 { 'event': 'BLOCK_JOB_READY',
-  'data': { 'type'  : 'BlockJobType',
+  'data': { 'type'  : 'JobType',
 'device': 'str',
 'len'   : 'int',
 'offset': 'int',
@@ -4627,7 +4627,7 @@
 #
 ##
 { 'event': 'BLOCK_JOB_PENDING',
-  'data': { 'type'  : 'BlockJobType',
+  'data': { 'type'  : 'JobType',
 'id': 'str' } }
 
 ##
diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h
index e8eca44747..8e7e0a2f57 100644
--- a/include/block/blockjob_int.h
+++ b/include/block/blockjob_int.h
@@ -39,7 +39,7 @@ struct BlockJobDriver {
 JobDriver job_driver;
 
 /** String describing the operation, part of query-block-jobs QMP API */
-BlockJobType job_type;
+JobType job_type;
 
 /** Mandatory: Entrypoint for the Coroutine. */
 CoroutineEntry *start;
diff --git a/block/backup.c b/block/backup.c
index 9e672bbd5e..c49ea92dca 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -526,7 +526,7 @@ static const BlockJobDriver backup_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(BackupBlockJob),
 },
-.job_type   = BLOCK_JOB_TYPE_BACKUP,
+.job_type   = JOB_TYPE_BACKUP,
 .start  = backup_run,
 .commit = backup_commit,
 .abort  = backup_abort,
diff --git a/block/commit.c b/block/commit.c
index 18cbb2f9c4..afa2b2bacf 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -218,7 +218,7 @@ static const BlockJobDriver commit_job_driver = {
 .job_driver = {
 .instance_size = sizeof(CommitBlockJob),
 },
-.job_type  = BLOCK_JOB_TYPE_COMMIT,
+.job_type  = JOB_TYPE_COMMIT,
 .start = commit_run,
 };
 
diff --git a/block/mirror.c b/block/mirror.c
index aa1d6b742e..ed72656a23 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -989,7 +989,7 @@ static const BlockJobDriver mirror_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
 },
-.job_type   = BLOCK_JOB_TYPE_MIRROR,
+.job_type   = JOB_TYPE_MIRROR,
 .start  = mirror_run,
 .complete   = mirror_complete,
 .pause  = mirror_pause,
@@ -1001,7 +1001,7 @@ static const BlockJobDriver commit_active_job_driver = {
 .job_driver = {
 .instance_size  = sizeof(MirrorBlockJob),
 },
-.job_type   = BLOCK_JOB_TYPE_COMMIT,
+.job_type   = JOB_TYPE_COMMIT,
 .start  = mirror_run,
 .complete   = mirror_complete,
 .pause  = mirror_pause,
diff --git a/block/stream.c b/block/stream.c
index f88fc75141..048bceb5d0 100644
--- a/block/stream.c
+++ b/block/stream.c
@@ -212,7 +212,7 @@ static const BlockJobDriver stream_job_driver = {
 .job_driver = {
 .instance_size = sizeof(StreamBlockJob),
 },
-.job_type  = BLOCK_JOB_TYPE_STREAM,
+.job_type  = JOB_TYPE_STREAM,
 .start = stream_run,
 };
 
diff --git a/blockjob.c b/blockjob.c
index 1464856eb5..2a3844721e 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -310,7 +310,7 @@ static char *child_job_get_parent_desc(BdrvChild *c)
 {
 BlockJob *job = c->opaque;
 return 

[Qemu-block] [PULL 01/46] qemu-iotests: Fix paths for NFS

[Kevin Wolf]

Test cases were trying to use nfs:// URLs as local filenames, which made
every test fail for NFS. With TEST_IMG and TEST_IMG_FILE set like for
the other protocols, NFS tests can pass again.

Signed-off-by: Kevin Wolf 
Reviewed-by: Fam Zheng 
---
 tests/qemu-iotests/common.rc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 9a65a11026..cb5fa14e7f 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -147,8 +147,8 @@ else
 TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
 TEST_IMG="ssh://127.0.0.1$TEST_IMG_FILE"
 elif [ "$IMGPROTO" = "nfs" ]; then
-TEST_DIR="nfs://127.0.0.1/$TEST_DIR"
-TEST_IMG=$TEST_DIR/t.$IMGFMT
+TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
+TEST_IMG="nfs://127.0.0.1$TEST_IMG_FILE"
 elif [ "$IMGPROTO" = "vxhs" ]; then
 TEST_IMG_FILE=$TEST_DIR/t.$IMGFMT
 TEST_IMG="vxhs://127.0.0.1:/t.$IMGFMT"
-- 
2.13.6

Re: [Qemu-block] [PATCH v2 00/40] Generic background jobs

[Kevin Wolf]

Am 18.05.2018 um 15:20 hat Kevin Wolf geschrieben:
> Before we can make x-blockdev-create a background job, we need to
> generalise the job infrastructure so that it can be used without any
> associated block node.
> 
> This series extracts a Job object from the block job infrastructure,
> which should contain everything related to jobs that doesn't require the
> block layer to be involved.
> 
> Job creation with a centralised job-create command (if we even want
> this) as well as the actual conversion of x-blockdev-create to Job is
> left for another series.

Applied to the block branch.

Kevin

Re: [Qemu-block] [PATCH 0/2] qemu-iotests: Add more tests to "migration" group

[Kevin Wolf]

Am 23.05.2018 um 11:58 hat Kevin Wolf geschrieben:
> Kevin Wolf (2):
>   qemu-iotests: Add more tests to "migration" group
>   qemu-iotests: Remove MIG_SOCKET from non-migration tests

Applied to the block branch.

Kevin

Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

[Eduardo Habkost]

On Wed, May 23, 2018 at 01:19:46PM +0200, Markus Armbruster wrote:
> Eduardo Habkost  writes:
> 
> > On Mon, May 21, 2018 at 07:44:40PM +0100, Daniel P. Berrangé wrote:
> >> On Mon, May 21, 2018 at 03:29:28PM -0300, Eduardo Habkost wrote:
> >> > On Sat, May 19, 2018 at 08:05:06AM +0200, Markus Armbruster wrote:
> >> > > Eduardo Habkost  writes:
> >> > > 
> >> > > [...]
> >> > > > About being more expressive than just a single list of key,value
> >> > > > pairs, I don't see any evidence of that being necessary for the
> >> > > > problems we're trying to address.
> >> > > 
> >> > > Short history of a configuration format you might have encountered:
> >> 
> >> [snip]
> >> 
> >> > > How confident are we a single list of (key, value) is really all we're
> >> > > going to need?
> >> > > 
> >> > > Even if we think it is, would it be possible to provide for a future
> >> > > extension to trees at next to no cost?
> >> > 
> >> > I'm confident that a list of key,values is all we need for the
> >> > current problem.
> >> 
> >> I'm not convinced. A disk image may work with Q35 or i440fx,  or
> >> work with any of virtio, ide or sata disk. So that already means
> >> values have to be arrays, not scalars. You could do that with a
> >> simple key,value list, but only by defining a mapping of arrays
> >> into a flattened form. eg do we allow repeated keys, or do we
> >> allow array indexes on keys. 
> >
> > No problem, we can support trees if it's necessary.
> >
> >
> >> > The point here is to allow users to simply copy an existing disk
> >> > image, and it will contain enough hints for a cloud stack to
> >> > choose reasonable defaults for machine-type and disk type
> >> > automatically.  Requiring the user to perform a separate step to
> >> > encapsulate the disk image in another file format defeats the
> >> > whole purpose of the proposal.
> >> 
> >> It doesn't have to mean more work for the user - the application
> >> that is used to create the image can do that on their behalf.
> >> oVirt for example can import/export OVA files, containing OVF
> >> metadata. I could imagine virt-manager, and other tools adding
> >> export ability without much trouble if this was deemed a desirable
> >> thing. Bundling gives ability to have multiple disk images in one
> >> archive, which is something OVF does.
> >
> > I have the impression that "the application that is used to
> > create the image" is a very large set.  It can be virt-manager,
> > virt-install, virt-manager, or even QEMU itself.
> >
> > Today people can simply create a VM on virt-manager, or run QEMU
> > manually, and upload the qcow2 image directly from its original
> > location (they don't need to copy/export it).  Don't we want the
> > same procedure to keep working instead of requiring users to use
> > another tool?
> 
> Today, I can take the disk out of my old computer, put it into my new
> computer, and it just works.  Don't we want the same procedure to keep
> working forever?

I don't think the comparison is fair: downloading hard disk
images for bare metal hardware is not as common as downloading
guest disk images for cloud infrastructure.

> 
> Sadly, wanting something badly enough doesn't make actual solutions any
> easier :)

This part is true.  :)

> 
> My point is: disk images (real or virtual) keep working in different
> hardware contexts by a mixture of flexibility built into system software
> on the image, disciplined evolution of hardware (real or virtual), and
> dumb luck.  It works until it doesn't.  And then you get to tinker.

Personally, I believe that tools for running and managing virtual
hardware can and should be smarter than real hardware.

> 
> With OVF, you solve the problem further up the stack: you do virtual
> appliances instead of disk images.
> 

I guess the main problem is that people are already using disk
images as if they were virtual appliances.

We can tell people to stop doing that and use OVF, but then we
won't make anybody's life any easier: publishers of images might
need to generate both qcow2 and OVF images if they want it to
work with older hosts; consumers will need to find out if they
need qcow2 or OVF.

But I work too deep down the stack to tell if it's really
important to avoid these problems or not.  And as you said, this
doesn't make actual solutions any easier.


> How much space that leaves for useful solutions at the level of QEMU I
> can't say.

I have no doubt about "useful", but I'm not sure about
"important".

I guess the question is if we have people with time and resources
to work on solutions (whether using qcow2 or OVF).

-- 
Eduardo

Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

[Markus Armbruster]

Eduardo Habkost  writes:

> On Mon, May 21, 2018 at 07:44:40PM +0100, Daniel P. Berrangé wrote:
>> On Mon, May 21, 2018 at 03:29:28PM -0300, Eduardo Habkost wrote:
>> > On Sat, May 19, 2018 at 08:05:06AM +0200, Markus Armbruster wrote:
>> > > Eduardo Habkost  writes:
>> > > 
>> > > [...]
>> > > > About being more expressive than just a single list of key,value
>> > > > pairs, I don't see any evidence of that being necessary for the
>> > > > problems we're trying to address.
>> > > 
>> > > Short history of a configuration format you might have encountered:
>> 
>> [snip]
>> 
>> > > How confident are we a single list of (key, value) is really all we're
>> > > going to need?
>> > > 
>> > > Even if we think it is, would it be possible to provide for a future
>> > > extension to trees at next to no cost?
>> > 
>> > I'm confident that a list of key,values is all we need for the
>> > current problem.
>> 
>> I'm not convinced. A disk image may work with Q35 or i440fx,  or
>> work with any of virtio, ide or sata disk. So that already means
>> values have to be arrays, not scalars. You could do that with a
>> simple key,value list, but only by defining a mapping of arrays
>> into a flattened form. eg do we allow repeated keys, or do we
>> allow array indexes on keys. 
>
> No problem, we can support trees if it's necessary.
>
>
>> > The point here is to allow users to simply copy an existing disk
>> > image, and it will contain enough hints for a cloud stack to
>> > choose reasonable defaults for machine-type and disk type
>> > automatically.  Requiring the user to perform a separate step to
>> > encapsulate the disk image in another file format defeats the
>> > whole purpose of the proposal.
>> 
>> It doesn't have to mean more work for the user - the application
>> that is used to create the image can do that on their behalf.
>> oVirt for example can import/export OVA files, containing OVF
>> metadata. I could imagine virt-manager, and other tools adding
>> export ability without much trouble if this was deemed a desirable
>> thing. Bundling gives ability to have multiple disk images in one
>> archive, which is something OVF does.
>
> I have the impression that "the application that is used to
> create the image" is a very large set.  It can be virt-manager,
> virt-install, virt-manager, or even QEMU itself.
>
> Today people can simply create a VM on virt-manager, or run QEMU
> manually, and upload the qcow2 image directly from its original
> location (they don't need to copy/export it).  Don't we want the
> same procedure to keep working instead of requiring users to use
> another tool?

Today, I can take the disk out of my old computer, put it into my new
computer, and it just works.  Don't we want the same procedure to keep
working forever?

Sadly, wanting something badly enough doesn't make actual solutions any
easier :)

My point is: disk images (real or virtual) keep working in different
hardware contexts by a mixture of flexibility built into system software
on the image, disciplined evolution of hardware (real or virtual), and
dumb luck.  It works until it doesn't.  And then you get to tinker.

With OVF, you solve the problem further up the stack: you do virtual
appliances instead of disk images.

How much space that leaves for useful solutions at the level of QEMU I
can't say.

Re: [Qemu-block] [PATCH 2/2] qemu-iotests: Remove MIG_SOCKET from non-migration tests

[Juan Quintela]

Kevin Wolf  wrote:
> 185 and 191 define a MIG_SOCKET even though they don't do anything with
> migration. Remove the useless variable.
>
> Signed-off-by: Kevin Wolf 

Reviewed-by: Juan Quintela 

[Qemu-block] [PATCH v3 3/5] nbd/server: implement dirty bitmap export

[Vladimir Sementsov-Ogievskiy]

Handle new NBD meta namespace: "qemu", and corresponding queries:
"qemu:dirty-bitmap:".

With new metadata context negotiated, BLOCK_STATUS query will reply
with dirty-bitmap data, converted to extents. New public function
nbd_export_bitmap selects bitmap to export. For now, only one bitmap
may be exported.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 include/block/nbd.h |   6 ++
 nbd/server.c| 265 
 2 files changed, 253 insertions(+), 18 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index fcdcd54502..a653d0fba7 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -234,6 +234,10 @@ enum {
 #define NBD_STATE_HOLE (1 << 0)
 #define NBD_STATE_ZERO (1 << 1)
 
+/* Flags for extents (NBDExtent.flags) of NBD_REPLY_TYPE_BLOCK_STATUS,
+ * for qemu:dirty-bitmap:* meta contexts */
+#define NBD_STATE_DIRTY (1 << 0)
+
 static inline bool nbd_reply_type_is_error(int type)
 {
 return type & (1 << 15);
@@ -315,6 +319,8 @@ void nbd_client_put(NBDClient *client);
 void nbd_server_start(SocketAddress *addr, const char *tls_creds,
   Error **errp);
 
+void nbd_export_bitmap(NBDExport *exp, const char *bitmap,
+   const char *bitmap_export_name, Error **errp);
 
 /* nbd_read
  * Reads @size bytes from @ioc. Returns 0 on success.
diff --git a/nbd/server.c b/nbd/server.c
index 8869dfe1f9..df1c581aeb 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -23,6 +23,12 @@
 #include "nbd-internal.h"
 
 #define NBD_META_ID_BASE_ALLOCATION 0
+#define NBD_META_ID_DIRTY_BITMAP 1
+
+/* NBD_MAX_BITMAP_EXTENTS: 1 mb of extents data. An empirical constant. If need
+ * to increase, note that NBD protocol defines 32 mb as a limit, after which 
the
+ * reply may be considered as a denial of service attack. */
+#define NBD_MAX_BITMAP_EXTENTS (0x10 / 8)
 
 static int system_errno_to_nbd_errno(int err)
 {
@@ -80,6 +86,9 @@ struct NBDExport {
 
 BlockBackend *eject_notifier_blk;
 Notifier eject_notifier;
+
+BdrvDirtyBitmap *export_bitmap;
+char *export_bitmap_context;
 };
 
 static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
@@ -92,6 +101,7 @@ typedef struct NBDExportMetaContexts {
 bool valid; /* means that negotiation of the option finished without
errors */
 bool base_allocation; /* export base:allocation context (block status) */
+bool dirty_bitmap; /* export qemu:dirty-bitmap: */
 } NBDExportMetaContexts;
 
 struct NBDClient {
@@ -809,6 +819,55 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
  >base_allocation, errp);
 }
 
+/* nbd_meta_bitmap_query
+ *
+ * Handle query to 'qemu:' namespace.
+ * @len is the amount of text remaining to be read from the current name, after
+ * the 'qemu:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_qemu_query(NBDClient *client, NBDExportMetaContexts *meta,
+   uint32_t len, Error **errp)
+{
+bool dirty_bitmap = false;
+int dirty_bitmap_len = strlen("dirty-bitmap:");
+int ret;
+
+if (!client->exp->export_bitmap) {
+return nbd_opt_skip(client, len, errp);
+}
+
+if (len == 0) {
+if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
+meta->dirty_bitmap = true;
+}
+trace_nbd_negotiate_meta_query_parse("empty");
+return 1;
+}
+
+if (len < dirty_bitmap_len) {
+trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+return nbd_opt_skip(client, len, errp);
+}
+
+len -= dirty_bitmap_len;
+ret = nbd_meta_pattern(client, "dirty-bitmap:", _bitmap, errp);
+if (ret <= 0) {
+return ret;
+}
+if (!dirty_bitmap) {
+trace_nbd_negotiate_meta_query_skip("not dirty-bitmap:");
+return nbd_opt_skip(client, len, errp);
+}
+
+trace_nbd_negotiate_meta_query_parse("dirty-bitmap:");
+
+return nbd_meta_empty_or_pattern(
+client, client->exp->export_bitmap_context +
+strlen("qemu:dirty_bitmap:"), len, >dirty_bitmap, errp);
+}
+
 /* nbd_negotiate_meta_query
  *
  * Parse namespace name and call corresponding function to parse body of the
@@ -825,8 +884,10 @@ static int nbd_negotiate_meta_query(NBDClient *client,
 NBDExportMetaContexts *meta, Error **errp)
 {
 int ret;
-char query[sizeof("base:") - 1];
-size_t baselen = strlen("base:");
+size_t ns_len = 5;
+char ns[5]; /* Both 'qemu' and 'base' namespaces have length = 5 including 
a
+   colon. If it's needed to introduce a namespace of the other
+   length, this should be certainly refactored. */
 uint32_t len;
 
 ret = nbd_opt_read(client, , 

[Qemu-block] [PATCH v3 1/5] nbd/server: fix trace

[Vladimir Sementsov-Ogievskiy]

Return code = 1 doesn't mean that we parsed base:allocation. Move
trace point to appropriate place.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 9e1f227178..7584ff7dcc 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -741,7 +741,10 @@ static int nbd_negotiate_send_meta_context(NBDClient 
*client,
  * the current name, after the 'base:' portion has been stripped.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
- * sending a reply about inconsistent lengths, or 1 on success. */
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
+ * namespace. It only means that there are no errors.*/
 static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
uint32_t len, Error **errp)
 {
@@ -768,10 +771,10 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
 }
 
 if (strncmp(query, "allocation", alen) == 0) {
+trace_nbd_negotiate_meta_query_parse("base:allocation");
 meta->base_allocation = true;
 }
 
-trace_nbd_negotiate_meta_query_parse("base:allocation");
 return 1;
 }
 
-- 
2.11.1

[Qemu-block] [PATCH v3 5/5] docs/interop: add nbd.txt

[Vladimir Sementsov-Ogievskiy]

Describe new metadata namespace: "qemu".

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 docs/interop/nbd.txt | 37 +
 MAINTAINERS  |  1 +
 2 files changed, 38 insertions(+)
 create mode 100644 docs/interop/nbd.txt

diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt
new file mode 100644
index 00..7366269fc0
--- /dev/null
+++ b/docs/interop/nbd.txt
@@ -0,0 +1,37 @@
+Qemu supports NBD protocol, and has internal NBD client (look at
+block/nbd.c), internal NBD server (look at blockdev-nbd.c) as well as
+external NBD server tool - qemu-nbd.c. The common code is placed in
+nbd/*.
+
+NBD protocol is specified here:
+https://github.com/NetworkBlockDevice/nbd/blob/master/doc/proto.md
+
+This following paragraphs describe some specific properties of NBD
+protocol realization in Qemu.
+
+
+= Metadata namespaces =
+
+Qemu supports "base:allocation" metadata context as defined in the NBD
+protocol specification and defines own metadata namespace: "qemu".
+
+
+== "qemu" namespace ==
+
+For now, the only type of metadata context in the namespace is dirty
+bitmap. All available metadata contexts have the following form:
+
+   qemu:dirty-bitmap:
+
+Each dirty-bitmap metadata context defines the only one flag for
+extents in reply for NBD_CMD_BLOCK_STATUS:
+
+bit 0: NBD_STATE_DIRTY, means that the extent is "dirty"
+
+For NBD_OPT_LIST_META_CONTEXT the following queries are supported
+additionally to "qemu:dirty-bitmap:":
+
+* "qemu:" : returns list of all available metadata contexts in the
+namespace.
+* "qemu:dirty-bitmap:" : returns list of all available dirty-bitmap
+ metadata contexts.
diff --git a/MAINTAINERS b/MAINTAINERS
index e187b1f18f..887b479440 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1923,6 +1923,7 @@ F: nbd/
 F: include/block/nbd*
 F: qemu-nbd.*
 F: blockdev-nbd.c
+F: docs/interop/nbd.txt
 T: git git://repo.or.cz/qemu/ericb.git nbd
 
 NFS
-- 
2.11.1

Re: [Qemu-block] [PATCH 1/2] qemu-iotests: Add more tests to "migration" group

[Juan Quintela]

Kevin Wolf  wrote:
> grep for "migrate" turns up a few test cases which use migration, but
> haven't been in the "migration" group so far. Add them to the group.
>
> Signed-off-by: Kevin Wolf 

Reviewed-by: Juan Quintela 

Thanks

[Qemu-block] [PATCH v3 2/5] nbd/server: add nbd_meta_empty_or_pattern helper

[Vladimir Sementsov-Ogievskiy]

Add nbd_meta_pattern() and nbd_meta_empty_or_pattern() helpers for
metadata query parsing. nbd_meta_pattern() will be reused for "qemu"
namespace in following patches.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 nbd/server.c | 82 ++--
 1 file changed, 57 insertions(+), 25 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 7584ff7dcc..8869dfe1f9 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -734,48 +734,79 @@ static int nbd_negotiate_send_meta_context(NBDClient 
*client,
 return qio_channel_writev_all(client->ioc, iov, 2, errp) < 0 ? -EIO : 0;
 }
 
-/* nbd_meta_base_query
- *
- * Handle query to 'base' namespace. For now, only base:allocation context is
- * available in it.  'len' is the amount of text remaining to be read from
- * the current name, after the 'base:' portion has been stripped.
+/* Read strlen(@pattern) bytes, and set @match to true if they match @pattern.
+ * @match is never set to false.
  *
  * Return -errno on I/O error, 0 if option was completely handled by
  * sending a reply about inconsistent lengths, or 1 on success.
  *
- * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
- * namespace. It only means that there are no errors.*/
-static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
-   uint32_t len, Error **errp)
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_pattern(NBDClient *client, const char *pattern, bool 
*match,
+Error **errp)
 {
 int ret;
-char query[sizeof("allocation") - 1];
-size_t alen = strlen("allocation");
+char *query;
+int len = strlen(pattern);
+
+assert(len);
+
+query = g_malloc(len);
+ret = nbd_opt_read(client, query, len, errp);
+if (ret <= 0) {
+g_free(query);
+return ret;
+}
+
+if (strncmp(query, pattern, len) == 0) {
+trace_nbd_negotiate_meta_query_parse(pattern);
+*match = true;
+}
+g_free(query);
 
+return 1;
+}
+
+/* Read @len bytes, and set @match to true if they match @pattern, or if @len
+ * is 0 and the client is performing _LIST_. @match is never set to false.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've read exactly @pattern
+ * It only means that there are no errors. */
+static int nbd_meta_empty_or_pattern(NBDClient *client, const char *pattern,
+ uint32_t len, bool *match, Error **errp)
+{
 if (len == 0) {
 if (client->opt == NBD_OPT_LIST_META_CONTEXT) {
-meta->base_allocation = true;
+*match = true;
 }
-trace_nbd_negotiate_meta_query_parse("base:");
+trace_nbd_negotiate_meta_query_parse("empty");
 return 1;
 }
 
-if (len != alen) {
-trace_nbd_negotiate_meta_query_skip("not base:allocation");
+if (len != strlen(pattern)) {
+trace_nbd_negotiate_meta_query_skip("different lengths");
 return nbd_opt_skip(client, len, errp);
 }
 
-ret = nbd_opt_read(client, query, len, errp);
-if (ret <= 0) {
-return ret;
-}
-
-if (strncmp(query, "allocation", alen) == 0) {
-trace_nbd_negotiate_meta_query_parse("base:allocation");
-meta->base_allocation = true;
-}
+return nbd_meta_pattern(client, pattern, match, errp);
+}
 
-return 1;
+/* nbd_meta_base_query
+ *
+ * Handle query to 'base' namespace. For now, only base:allocation context is
+ * available in it.  'len' is the amount of text remaining to be read from
+ * the current name, after the 'base:' portion has been stripped.
+ *
+ * Return -errno on I/O error, 0 if option was completely handled by
+ * sending a reply about inconsistent lengths, or 1 on success. */
+static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
+   uint32_t len, Error **errp)
+{
+return nbd_meta_empty_or_pattern(client, "allocation", len,
+ >base_allocation, errp);
 }
 
 /* nbd_negotiate_meta_query
@@ -821,6 +852,7 @@ static int nbd_negotiate_meta_query(NBDClient *client,
 return nbd_opt_skip(client, len, errp);
 }
 
+trace_nbd_negotiate_meta_query_parse("base:");
 return nbd_meta_base_query(client, meta, len, errp);
 }
 
-- 
2.11.1

[Qemu-block] [PATCH v3 0/5] NBD export bitmaps

[Vladimir Sementsov-Ogievskiy]

Hi all.

This is a proposal and realization of new NBD meta context:
qemu. (I hope to send corresponding proposal to NBD protocol soon)

New possible queries will look like:
qemu:dirty-bitmap:

Mapping from export-bitmap-name to BdrvDirtyBitmap is done through qmp
command nbd-server-add-bitmap. For now, only one bitmap export is
allowed per NBD export, however it may be easily improved if needed 
(we don't have such cases for now)

Client and testing.
I wrote client code for Virtuozzo, but it turned out to be unused,
actually it's used only for tests. We don't have cases, where we need
to import dirty bitmap through qemu nbd-client. All this done for
exporting dirty bitmaps to the third tool. So, I think, it is not worth
refactoring, rebasing and merging client part upstream, if there are no
real usage cases.

v3:
01: new
02: rewritten to satisfy changes in 03, drop r-b
03: - fix comments
- rewrite nbd_meta_bitmap_query() and rename it to
  nbd_meta_qemu_query(): parse 'qemu:dirty-bitmaps:' for LIST
  option to represent list of all dirty-bitmap contexts.
- trace points
- s/512/BDRV_SECTOR_SIZE/
  drop TODO comment
04: s/2.13/3.0
05: new

v2:
01 from v1 is dropped: actually, we don't need generic namespace
parsing for now (especially, after moving to qemu: namespace, which has
the same length as base:), lets postpone it.

01: Improve comment wording (Eric), add Eric's r-b
02: improve commit message
move NBD_STATE_DIRTY to header
add comment on NBD_MAX_BITMAP_EXTENTS
remove MAX_EXTENT_LENGTH and instead update add_extents() which
  uses it
use export_bitmap_context instead of export_bitmap_name to reduce
  operations on it
move from qemu-dirty-bitmap to qemu:dirty-bitmap
other way to parse namespace name
handle FLAG_DF
03: Improve specification of new qmp command (Eric)

Vladimir Sementsov-Ogievskiy (5):
  nbd/server: fix trace
  nbd/server: add nbd_meta_empty_or_pattern helper
  nbd/server: implement dirty bitmap export
  qapi: new qmp command nbd-server-add-bitmap
  docs/interop: add nbd.txt

 docs/interop/nbd.txt |  37 ++
 qapi/block.json  |  23 
 include/block/nbd.h  |   6 +
 blockdev-nbd.c   |  23 
 nbd/server.c | 322 ++-
 MAINTAINERS  |   1 +
 6 files changed, 383 insertions(+), 29 deletions(-)
 create mode 100644 docs/interop/nbd.txt

-- 
2.11.1

[Qemu-block] [PATCH v3 4/5] qapi: new qmp command nbd-server-add-bitmap

[Vladimir Sementsov-Ogievskiy]

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 qapi/block.json | 23 +++
 blockdev-nbd.c  | 23 +++
 2 files changed, 46 insertions(+)

diff --git a/qapi/block.json b/qapi/block.json
index c694524002..ddbca2e286 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -269,6 +269,29 @@
   'data': {'name': 'str', '*mode': 'NbdServerRemoveMode'} }
 
 ##
+# @nbd-server-add-bitmap:
+#
+# Expose a dirty bitmap associated with the selected export. The bitmap search
+# starts at the device attached to the export, and includes all backing files.
+# The exported bitmap is then locked until the NBD export is removed.
+#
+# @name: Export name.
+#
+# @bitmap: Bitmap name to search for.
+#
+# @bitmap-export-name: How the bitmap will be seen by nbd clients
+#  (default @bitmap)
+#
+# Note: the client must use NBD_OPT_SET_META_CONTEXT with a query of
+# "qemu:dirty-bitmap:NAME" (where NAME matches @bitmap-export-name) to access
+# the exposed bitmap.
+#
+# Since: 3.0
+##
+  { 'command': 'nbd-server-add-bitmap',
+'data': {'name': 'str', 'bitmap': 'str', '*bitmap-export-name': 'str'} }
+
+##
 # @nbd-server-stop:
 #
 # Stop QEMU's embedded NBD server, and unregister all devices previously
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 65a84739ed..6b0c50732c 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -220,3 +220,26 @@ void qmp_nbd_server_stop(Error **errp)
 nbd_server_free(nbd_server);
 nbd_server = NULL;
 }
+
+void qmp_nbd_server_add_bitmap(const char *name, const char *bitmap,
+   bool has_bitmap_export_name,
+   const char *bitmap_export_name,
+   Error **errp)
+{
+NBDExport *exp;
+
+if (!nbd_server) {
+error_setg(errp, "NBD server not running");
+return;
+}
+
+exp = nbd_export_find(name);
+if (exp == NULL) {
+error_setg(errp, "Export '%s' is not found", name);
+return;
+}
+
+nbd_export_bitmap(exp, bitmap,
+  has_bitmap_export_name ? bitmap_export_name : bitmap,
+  errp);
+}
-- 
2.11.1

[Qemu-block] [PATCH 0/2] qemu-iotests: Add more tests to "migration" group

[Kevin Wolf]

Kevin Wolf (2):
  qemu-iotests: Add more tests to "migration" group
  qemu-iotests: Remove MIG_SOCKET from non-migration tests

 tests/qemu-iotests/185   |  2 --
 tests/qemu-iotests/191   |  2 --
 tests/qemu-iotests/group | 10 +-
 3 files changed, 5 insertions(+), 9 deletions(-)

-- 
2.13.6

[Qemu-block] [PATCH 2/2] qemu-iotests: Remove MIG_SOCKET from non-migration tests

[Kevin Wolf]

185 and 191 define a MIG_SOCKET even though they don't do anything with
migration. Remove the useless variable.

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/185 | 2 --
 tests/qemu-iotests/191 | 2 --
 2 files changed, 4 deletions(-)

diff --git a/tests/qemu-iotests/185 b/tests/qemu-iotests/185
index 9a2d317414..deb42cc886 100755
--- a/tests/qemu-iotests/185
+++ b/tests/qemu-iotests/185
@@ -27,8 +27,6 @@ echo "QA output created by $seq"
 here=`pwd`
 status=1 # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
-
 _cleanup()
 {
 rm -f "${TEST_IMG}.mid"
diff --git a/tests/qemu-iotests/191 b/tests/qemu-iotests/191
index dfad6555e4..77224eb151 100755
--- a/tests/qemu-iotests/191
+++ b/tests/qemu-iotests/191
@@ -27,8 +27,6 @@ echo "QA output created by $seq"
 here=`pwd`
 status=1 # failure is the default!
 
-MIG_SOCKET="${TEST_DIR}/migrate"
-
 _cleanup()
 {
 rm -f "${TEST_IMG}.mid"
-- 
2.13.6

[Qemu-block] [PATCH 1/2] qemu-iotests: Add more tests to "migration" group

[Kevin Wolf]

grep for "migrate" turns up a few test cases which use migration, but
haven't been in the "migration" group so far. Add them to the group.

Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/group | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index cc8cd8cc8e..60ba31b292 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -97,7 +97,7 @@
 088 rw auto quick
 089 rw auto quick
 090 rw auto quick
-091 rw auto
+091 rw auto migration
 092 rw auto quick
 093 auto
 094 rw auto quick
@@ -169,7 +169,7 @@
 162 auto quick
 163 rw auto
 165 rw auto quick
-169 rw auto quick
+169 rw auto quick migration
 170 rw auto quick
 171 rw auto quick
 172 auto
@@ -194,14 +194,14 @@
 192 rw auto quick
 194 rw auto migration quick
 195 rw auto quick
-196 rw auto quick
+196 rw auto quick migration
 197 rw auto quick
 198 rw auto
-199 rw auto
+199 rw auto migration
 200 rw auto
 201 rw auto migration
 202 rw auto quick
-203 rw auto
+203 rw auto migration
 204 rw auto quick
 205 rw auto quick
 206 rw auto
-- 
2.13.6

Re: [Qemu-block] [PATCH] nbd/server: fix trace

[Vladimir Sementsov-Ogievskiy]

will resend with "NBD export bitmaps" series v3

22.05.2018 16:41, Vladimir Sementsov-Ogievskiy wrote:

Return code = 1 doesn't mean that we parsed base:allocation. Move
trace point to appropriate place.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/nbd/server.c b/nbd/server.c
index 9e1f227178..84381baaa8 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -741,7 +741,10 @@ static int nbd_negotiate_send_meta_context(NBDClient 
*client,
   * the current name, after the 'base:' portion has been stripped.
   *
   * Return -errno on I/O error, 0 if option was completely handled by
- * sending a reply about inconsistent lengths, or 1 on success. */
+ * sending a reply about inconsistent lengths, or 1 on success.
+ *
+ * Note: return code = 1 doesn't mean that we've parsed "base:allocation"
+ * namespace. It only mean that there are no errors.*/
  static int nbd_meta_base_query(NBDClient *client, NBDExportMetaContexts *meta,
 uint32_t len, Error **errp)
  {
@@ -768,10 +771,10 @@ static int nbd_meta_base_query(NBDClient *client, 
NBDExportMetaContexts *meta,
  }
  
  if (strncmp(query, "allocation", alen) == 0) {

+trace_nbd_negotiate_meta_query_parse("base:allocation");
  meta->base_allocation = true;
  }
  
-trace_nbd_negotiate_meta_query_parse("base:allocation");

  return 1;
  }
  



--
Best regards,
Vladimir

Re: [Qemu-block] [Qemu-devel] storing machine data in qcow images?

[Kevin Wolf]

Am 23.05.2018 um 04:12 hat Fam Zheng geschrieben:
> On Tue, 05/22 17:02, Kevin Wolf wrote:
> > Am 22.05.2018 um 16:19 hat Michael S. Tsirkin geschrieben:
> > > On Tue, May 22, 2018 at 09:35:55AM +0200, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > You must /sometimes/ supply the correct machine type.
> > > > > 
> > > > > It is quite dependent on the guest OS you have installed, and even
> > > > > just how the guest OS is configured.  In general Linux is very
> > > > > flexible and can adapt to a wide range of hardware, automatically
> > > > > detecting things as needed. It is possible for a sysadmin to build
> > > > > a Linux image in a way that would only work with I440FX, but I
> > > > > don't think it would be common to see that.
> > > > 
> > > > I think it would be pretty hard to actually build such an image.
> > > > 
> > > > The more critical thing for linux guests is the storage driver which
> > > > must be included into the initrd so the image can mount the root
> > > > filesystem.  And the firmware, bios vs. uefi is more critical than
> > > > pc vs. q35.
> > > 
> > > I think we can start by finding a location to embed a string in a qcow
> > > image, add ability for qemu-img to set and get this string.  We can
> > > discuss how it's formatted separately.
> > 
> > If we want it, we'll find a place to store it.
> > 
> > But the first thing we need is a spec for what's actually in it. Just
> > storing a machine type hint would be a one-off hack that wouldn't last
> > very long before we want to add the next thing.
> > 
> > Essentially, what we need is a description of the virtual machine that
> > we suggest to use with this image. We can try to reuse something
> > existing there, like libvirt XML or OVF, or invent something new (a JSON
> > array describing runtime options?). One difference to existing formats
> > is probably that we want only frontends and no backends in the
> > description.
> > 
> 
> Do we really need a uniform way and require compliance to the standard we
> choose, and implement verification in the block driver, or can we get away 
> with
> a description field that accepts any text and leave it to the user to decide
> what to put there? In the header we could assign a Content-type field that
> defaults to 'text/plain' to the description, that way apps can mark the data 
> as
> "application/ovf" if they want, or whatever the upper layer decides.

Yes, we can. But I'm not sure if I want. Providing low-level features
without telling users how they are supposed to be used usually results
in a big surprise for both sides eventually.

Kevin