[Qemu-devel] [RFC PATCH 00/33] Generic background jobs

Kevin Wolf posted 33 patches 7 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180424152515.25664-1-kwolf@redhat.com
Test checkpatch failed
Test docker-build@min-glib passed
Test docker-mingw@fedora passed
Test s390x passed
There is a newer version of this series
qapi/block-core.json         |   28 +-
include/block/block_int.h    |    2 +-
include/block/blockjob.h     |  306 ++----------
include/block/blockjob_int.h |  179 +------
include/qemu/job.h           |  513 ++++++++++++++++++++
block.c                      |    2 +-
block/backup.c               |  111 +++--
block/commit.c               |   75 ++-
block/mirror.c               |  131 +++---
block/replication.c          |   10 +-
block/stream.c               |   68 ++-
blockdev.c                   |   66 +--
blockjob.c                   | 1054 ++++++------------------------------------
job.c                        |  950 +++++++++++++++++++++++++++++++++++++
qemu-img.c                   |   14 +-
tests/test-bdrv-drain.c      |   61 +--
tests/test-blockjob-txn.c    |   74 +--
tests/test-blockjob.c        |  137 +++---
MAINTAINERS                  |    2 +
Makefile.objs                |    2 +-
block/trace-events           |    5 -
trace-events                 |    5 +
22 files changed, 2049 insertions(+), 1746 deletions(-)
create mode 100644 include/qemu/job.h
create mode 100644 job.c
[Qemu-devel] [RFC PATCH 00/33] Generic background jobs
Posted by Kevin Wolf 7 years, 6 months ago
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.

The split between BlockJob and Job is reasonably complete as of this RFC
(though there are still some rough edges). Missing are mainly:

* Some cleanups. This means mostly TODOs left for functions moved to
  job.c that should become static again, but were still called from
  blockjob.c. At this point, most of these public declarations aren't
  actually necessary any more, or very little is missing to make them
  unnecessary.

* A QMP interface that can be used with non-block job. The existing
  block-job-* QMP commands will tell the user that they don't know the
  job if you pass the ID of a non-block job.

* The actual conversion of x-blockdev-create to Job, as a proof of
  concept that the generalised infrastructure actually works.

Kevin Wolf (33):
  blockjob: Wrappers for progress counter access
  blockjob: Move RateLimit to BlockJob
  blockjob: Implement block_job_set_speed() centrally
  blockjob: Introduce block_job_ratelimit_get_delay()
  blockjob: Add block_job_driver()
  blockjob: Remove block_job_pause/resume_all()
  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
  job: Add job_yield()
  job: Add job_dismiss()

 qapi/block-core.json         |   28 +-
 include/block/block_int.h    |    2 +-
 include/block/blockjob.h     |  306 ++----------
 include/block/blockjob_int.h |  179 +------
 include/qemu/job.h           |  513 ++++++++++++++++++++
 block.c                      |    2 +-
 block/backup.c               |  111 +++--
 block/commit.c               |   75 ++-
 block/mirror.c               |  131 +++---
 block/replication.c          |   10 +-
 block/stream.c               |   68 ++-
 blockdev.c                   |   66 +--
 blockjob.c                   | 1054 ++++++------------------------------------
 job.c                        |  950 +++++++++++++++++++++++++++++++++++++
 qemu-img.c                   |   14 +-
 tests/test-bdrv-drain.c      |   61 +--
 tests/test-blockjob-txn.c    |   74 +--
 tests/test-blockjob.c        |  137 +++---
 MAINTAINERS                  |    2 +
 Makefile.objs                |    2 +-
 block/trace-events           |    5 -
 trace-events                 |    5 +
 22 files changed, 2049 insertions(+), 1746 deletions(-)
 create mode 100644 include/qemu/job.h
 create mode 100644 job.c

-- 
2.13.6


Re: [Qemu-devel] [RFC PATCH 00/33] Generic background jobs
Posted by no-reply@patchew.org 7 years, 6 months ago
Hi,

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

Type: series
Message-id: 20180424152515.25664-1-kwolf@redhat.com
Subject: [Qemu-devel] [RFC PATCH 00/33] Generic background jobs

=== 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/20180424045112.12963-1-peterx@redhat.com -> patchew/20180424045112.12963-1-peterx@redhat.com
 * [new tag]               patchew/20180424152515.25664-1-kwolf@redhat.com -> patchew/20180424152515.25664-1-kwolf@redhat.com
Switched to a new branch 'test'
a535175023 job: Add job_dismiss()
0745f6ead0 job: Add job_yield()
30ec314f7c job: Move completion and cancellation to Job
55feb0dd01 job: Move transactions to Job
01fdee4ef4 job: Switch transactions to JobTxn
08f97e3203 job: Move job_finish_sync() to Job
29083331c0 job: Move .complete callback to Job
48109bdf06 job: Add job_drain()
9cf6eff254 job: Convert block_job_cancel_async() to Job
6f2e5d1e76 job: Move single job finalisation to Job
5484bd3641 job: Add job_event_*()
2b16ee88f7 blockjob: Split block_job_event_pending()
eff654a865 job: Move BlockJobCreateFlags to Job
29a541953f job: Replace BlockJob.completed with job_is_completed()
d8835d9376 job: Move pause/resume functions to Job
0f78a91558 job: Add job_sleep_ns()
8767231b63 job: Move coroutine and related code to Job
64d19df958 job: Move defer_to_main_loop to Job
bdfe0a2294 job: Add Job.aio_context
203771e4ce job: Move cancelled to Job
0b5941f5df job: Add reference counting
c7f701c66d job: Move state transitions to Job
bd4fa23c86 job: Maintain a list of all jobs
03fb0888f2 job: Add job_delete()
d3efb916b0 job: Add JobDriver.job_type
6a3ee68500 job: Rename BlockJobType into JobType
b1abdb48ca job: Create Job, JobDriver and job_create()
49bfc63bc0 blockjob: Remove block_job_pause/resume_all()
77ae27705e blockjob: Add block_job_driver()
6b3f90fab4 blockjob: Introduce block_job_ratelimit_get_delay()
6fb872a85f blockjob: Implement block_job_set_speed() centrally
8b39f71653 blockjob: Move RateLimit to BlockJob
7d836020ea blockjob: Wrappers for progress counter access

=== OUTPUT BEGIN ===
Checking PATCH 1/33: blockjob: Wrappers for progress counter access...
Checking PATCH 2/33: blockjob: Move RateLimit to BlockJob...
Checking PATCH 3/33: blockjob: Implement block_job_set_speed() centrally...
Checking PATCH 4/33: blockjob: Introduce block_job_ratelimit_get_delay()...
Checking PATCH 5/33: blockjob: Add block_job_driver()...
Checking PATCH 6/33: blockjob: Remove block_job_pause/resume_all()...
Checking PATCH 7/33: job: Create Job, JobDriver and job_create()...
Checking PATCH 8/33: job: Rename BlockJobType into JobType...
Checking PATCH 9/33: job: Add JobDriver.job_type...
Checking PATCH 10/33: job: Add job_delete()...
Checking PATCH 11/33: job: Maintain a list of all jobs...
Checking PATCH 12/33: job: Move state transitions to Job...
ERROR: space prohibited before open square bracket '['
#334: 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 '['
#335: FILE: job.c:39:
+    /* C: */ [JOB_STATUS_CREATED]   = {0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 1},

ERROR: space prohibited before open square bracket '['
#336: FILE: job.c:40:
+    /* R: */ [JOB_STATUS_RUNNING]   = {0, 0, 0, 1, 1, 0, 1, 0, 1, 0, 0},

ERROR: space prohibited before open square bracket '['
#337: FILE: job.c:41:
+    /* P: */ [JOB_STATUS_PAUSED]    = {0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#338: FILE: job.c:42:
+    /* Y: */ [JOB_STATUS_READY]     = {0, 0, 0, 0, 0, 1, 1, 0, 1, 0, 0},

ERROR: space prohibited before open square bracket '['
#339: FILE: job.c:43:
+    /* S: */ [JOB_STATUS_STANDBY]   = {0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0},

ERROR: space prohibited before open square bracket '['
#340: FILE: job.c:44:
+    /* W: */ [JOB_STATUS_WAITING]   = {0, 0, 0, 0, 0, 0, 0, 1, 1, 0, 0},

ERROR: space prohibited before open square bracket '['
#341: FILE: job.c:45:
+    /* D: */ [JOB_STATUS_PENDING]   = {0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0},

ERROR: space prohibited before open square bracket '['
#342: FILE: job.c:46:
+    /* X: */ [JOB_STATUS_ABORTING]  = {0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 0},

ERROR: space prohibited before open square bracket '['
#343: FILE: job.c:47:
+    /* E: */ [JOB_STATUS_CONCLUDED] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1},

ERROR: space prohibited before open square bracket '['
#344: FILE: job.c:48:
+    /* N: */ [JOB_STATUS_NULL]      = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},

total: 11 errors, 0 warnings, 515 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 13/33: job: Add reference counting...
Checking PATCH 14/33: job: Move cancelled to Job...
Checking PATCH 15/33: job: Add Job.aio_context...
Checking PATCH 16/33: job: Move defer_to_main_loop to Job...
Checking PATCH 17/33: job: Move coroutine and related code to Job...
Checking PATCH 18/33: job: Add job_sleep_ns()...
Checking PATCH 19/33: job: Move pause/resume functions to Job...
Checking PATCH 20/33: job: Replace BlockJob.completed with job_is_completed()...
Checking PATCH 21/33: job: Move BlockJobCreateFlags to Job...
Checking PATCH 22/33: blockjob: Split block_job_event_pending()...
Checking PATCH 23/33: job: Add job_event_*()...
Checking PATCH 24/33: job: Move single job finalisation to Job...
WARNING: line over 80 characters
#496: FILE: include/qemu/job.h:221:
+                 int flags, BlockCompletionFunc *cb, void *opaque, Error **errp);

ERROR: "(foo*)" should be "(foo *)"
#567: FILE: job.c:462:
+    block_job_txn_del_job((BlockJob*) job);

ERROR: "(foo*)" should be "(foo *)"
#648: FILE: job.c:543:
+    block_job_txn_del_job((BlockJob*) job);

total: 2 errors, 1 warnings, 611 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 25/33: job: Convert block_job_cancel_async() to Job...
Checking PATCH 26/33: job: Add job_drain()...
Checking PATCH 27/33: job: Move .complete callback to Job...
Checking PATCH 28/33: job: Move job_finish_sync() to Job...
WARNING: line over 80 characters
#121: FILE: include/qemu/job.h:375:
+int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp);

total: 0 errors, 1 warnings, 132 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 29/33: job: Switch transactions to JobTxn...
Checking PATCH 30/33: job: Move transactions to Job...
Checking PATCH 31/33: job: Move completion and cancellation to Job...
Checking PATCH 32/33: job: Add job_yield()...
Checking PATCH 33/33: job: Add job_dismiss()...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [RFC PATCH 00/33] Generic background jobs
Posted by Eric Blake 7 years, 6 months ago
On 04/24/2018 10:24 AM, Kevin Wolf wrote:
> 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.
> 
> The split between BlockJob and Job is reasonably complete as of this RFC
> (though there are still some rough edges). Missing are mainly:
> 
> * Some cleanups. This means mostly TODOs left for functions moved to
>   job.c that should become static again, but were still called from
>   blockjob.c. At this point, most of these public declarations aren't
>   actually necessary any more, or very little is missing to make them
>   unnecessary.
> 
> * A QMP interface that can be used with non-block job. The existing
>   block-job-* QMP commands will tell the user that they don't know the
>   job if you pass the ID of a non-block job.
> 
> * The actual conversion of x-blockdev-create to Job, as a proof of
>   concept that the generalised infrastructure actually works.

I didn't review the second half as closely, but did glance through the
whole series.  Overall, I like what I'm seeing; it looks like you have
indeed extracted the non-block portions into a reusable framework that
can be extended to other uses.  It would also be interesting to see what
other long-running operations we can map into this scheme, such as
migration or capturing a screenshot.

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