1. Patchew
  2. QEMU
  3. View series

[Qemu-devel] [PATCH v2 00/40] Generic background jobs

Kevin Wolf posted 40 patches 7 years, 5 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180518132114.4070-1-kwolf@redhat.com
Test checkpatch failed
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test s390x passed
qapi/block-core.json          |  116 +----
qapi/job.json                 |  252 ++++++++++
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/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                   |    3 +
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/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/155        |    2 +-
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/219        |  209 ++++++++
tests/qemu-iotests/219.out    |  327 ++++++++++++
tests/qemu-iotests/group      |    1 +
tests/qemu-iotests/iotests.py |   50 +-
trace-events                  |   14 +
53 files changed, 3578 insertions(+), 1878 deletions(-)
create mode 100644 qapi/job.json
create mode 100644 include/qemu/job.h
create mode 100644 job-qmp.c
create mode 100644 job.c
create mode 100755 tests/qemu-iotests/219
create mode 100644 tests/qemu-iotests/219.out
[Qemu-devel] [PATCH v2 00/40] Generic background jobs
Posted by Kevin Wolf 7 years, 5 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.

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.

v2:
- Already applied a small prefix of the series and rebased, so a few
  patches are missing compared to v1

- Fixed up forgotten renames of identifiers in comments (several
  patches all over the series) [Max]

- Patch 1 and 2 (new):
  QAPI documentation improvements for existing block-job-* commands

- Patch 5 (was patch 10; 'job: Add JobDriver.job_type'):
  Added 'const' to job_type()/job_type_str() parameter [Max]

- Patch 7 (was patch 12; 'job: Maintain a list of all jobs'):
  block_job_next(NULL) doesn't rely on BlockJob.job being the first
  field any more [Eric]

- Patch 8 (was patch 13; 'job: Move state transitions to Job'):
  Renamed JobVerb paramater from 'bv' to 'verb' [Eric]

- Patch 12 (was patch 17; 'job: Move defer_to_main_loop to Job'):
  Added commit message [Max, Eric, John]
  Removed stale comment [Max]

- Patch 26 (was patch 31; 'job: Move transactions to Job'):
  job_event_pending() can keep returning void [Max]

- Patch 27 (was patch 32; 'job: Move completion and cancellation to
  Job') and patch 28 (new):
  Added TODO in patch 27, and resolved it in patch 28 by moving the
  job_cancel_all() call out of bdrv_close_all() to the callers [Max]

- Patch 30 (was patch 34; 'job: Add job_dismiss()'):
  Fixed uninitialised use of 'job' [Max]

- Patch 33 (was patch 37; 'job: Move progress fields to Job'):
  Fixed documentation for job_progress_update/set_remaining() [Eric]

- Patch 34 (new):
  Separate patch for adding qapi/job.json that just moves the three
  Job-related enums JobType, JobStatus and JobVerb (without any commands
  or events) [Max]

- Patch 35 (was patch 38; 'job: Add JOB_STATUS_CHANGE QMP event'):
  Checking JOB_STATUS_CHANGE events more consistently in tests [Max]
  Updated qemu-iotests 094 reference output [Max]

- Patch 36 (was patch 39; 'job: Add lifecycle QMP commands'):
  Removed 'force' option from 'job-cancel' and make force=true the only
  behaviour. This makes mirror behave the same as other jobs in the
  context of 'job-cancel'. [Max]
  Fixed 'job-pause' documentation [Max]

- Patch 37 (was patch 40; 'job: Add query-jobs QMP command'):
  QAPI documentation improvements [Max]

- Patch 38 (new):
  Remove redundant BlockJob.driver [Max]

- Patch 40 (was patch 42; 'qemu-iotests: Test job-* with block jobs'):
  Filtered out progress in 'query-jobs' immediately after the job is
  started because on tmpfs the first request will already have
  completed. [Max]

Kevin Wolf (40):
  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

 qapi/block-core.json          |  116 +----
 qapi/job.json                 |  252 ++++++++++
 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/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                   |    3 +
 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/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/155        |    2 +-
 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/219        |  209 ++++++++
 tests/qemu-iotests/219.out    |  327 ++++++++++++
 tests/qemu-iotests/group      |    1 +
 tests/qemu-iotests/iotests.py |   50 +-
 trace-events                  |   14 +
 53 files changed, 3578 insertions(+), 1878 deletions(-)
 create mode 100644 qapi/job.json
 create mode 100644 include/qemu/job.h
 create mode 100644 job-qmp.c
 create mode 100644 job.c
 create mode 100755 tests/qemu-iotests/219
 create mode 100644 tests/qemu-iotests/219.out

-- 
2.13.6
Re: [Qemu-devel] [PATCH v2 00/40] Generic background jobs
Posted by no-reply@patchew.org 7 years, 5 months ago 
Hi,

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

Type: series
Message-id: 20180518132114.4070-1-kwolf@redhat.com
Subject: [Qemu-devel] [PATCH v2 00/40] 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/20180517092513.735-1-kraxel@redhat.com -> patchew/20180517092513.735-1-kraxel@redhat.com
 * [new tag]               patchew/20180518132114.4070-1-kwolf@redhat.com -> patchew/20180518132114.4070-1-kwolf@redhat.com
Switched to a new branch 'test'
be01aa198a qemu-iotests: Test job-* with block jobs
62233b758c iotests: Move qmp_to_opts() to VM
3f946bc442 blockjob: Remove BlockJob.driver
cae6373cbd job: Add query-jobs QMP command
9bf0565e0c job: Add lifecycle QMP commands
f7363a69b4 job: Add JOB_STATUS_CHANGE QMP event
5a63efe4e4 job: Introduce qapi/job.json
2a5e7375fa job: Move progress fields to Job
2ae08c1d06 job: Add job_transition_to_ready()
9d76be11bd job: Add job_is_ready()
9b51cc2a7e job: Add job_dismiss()
555a61a69f job: Add job_yield()
798978bcb1 block: Cancel job in bdrv_close_all() callers
4dcb71e4e5 job: Move completion and cancellation to Job
ffad8fad4b job: Move transactions to Job
0edc41ca48 job: Switch transactions to JobTxn
a1073023f2 job: Move job_finish_sync() to Job
6ec521dbdb job: Move .complete callback to Job
9660a731ab job: Add job_drain()
1958d0ac38 job: Convert block_job_cancel_async() to Job
dc1a8d70b8 job: Move single job finalisation to Job
4e7a12008f job: Add job_event_*()
d973eb6546 blockjob: Split block_job_event_pending()
7d447f8ab3 job: Move BlockJobCreateFlags to Job
ffa1c9ad30 job: Replace BlockJob.completed with job_is_completed()
13e426855c job: Move pause/resume functions to Job
0c52ff1b24 job: Add job_sleep_ns()
5ebf3ba66a job: Move coroutine and related code to Job
4e7b469718 job: Move defer_to_main_loop to Job
896fe05128 job: Add Job.aio_context
d73a94fde9 job: Move cancelled to Job
e386ae83f9 job: Add reference counting
3915da0ada job: Move state transitions to Job
e2ea58c7b0 job: Maintain a list of all jobs
717f2e60bf job: Add job_delete()
f31f336bca job: Add JobDriver.job_type
048bb87d0a job: Rename BlockJobType into JobType
b2707abc22 job: Create Job, JobDriver and job_create()
f7c7867229 blockjob: Improve BlockJobInfo.offset/len documentation
7611d0c75f blockjob: Update block-job-pause/resume documentation

=== OUTPUT BEGIN ===
Checking PATCH 1/40: blockjob: Update block-job-pause/resume documentation...
Checking PATCH 2/40: blockjob: Improve BlockJobInfo.offset/len documentation...
Checking PATCH 3/40: 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 4/40: job: Rename BlockJobType into JobType...
Checking PATCH 5/40: job: Add JobDriver.job_type...
Checking PATCH 6/40: job: Add job_delete()...
Checking PATCH 7/40: job: Maintain a list of all jobs...
Checking PATCH 8/40: job: Move state transitions to Job...
ERROR: space prohibited before open square bracket '['
#342: 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 '['
#343: 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 '['
#344: 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 '['
#345: 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 '['
#346: 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 '['
#347: 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 '['
#348: 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 '['
#349: 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 '['
#350: 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 '['
#351: 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 '['
#352: FILE: job.c:48:
+    /* N: */ [JOB_STATUS_NULL]      = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0},

total: 11 errors, 0 warnings, 528 lines checked

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

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

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

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

total: 2 errors, 1 warnings, 620 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 21/40: job: Convert block_job_cancel_async() to Job...
Checking PATCH 22/40: job: Add job_drain()...
Checking PATCH 23/40: job: Move .complete callback to Job...
Checking PATCH 24/40: job: Move job_finish_sync() to Job...
WARNING: line over 80 characters
#139: FILE: include/qemu/job.h:399:
+int job_finish_sync(Job *job, void (*finish)(Job *, Error **errp), Error **errp);

total: 0 errors, 1 warnings, 144 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/40: job: Switch transactions to JobTxn...
Checking PATCH 26/40: job: Move transactions to Job...
Checking PATCH 27/40: job: Move completion and cancellation to Job...
Checking PATCH 28/40: block: Cancel job in bdrv_close_all() callers...
Checking PATCH 29/40: job: Add job_yield()...
Checking PATCH 30/40: job: Add job_dismiss()...
Checking PATCH 31/40: job: Add job_is_ready()...
Checking PATCH 32/40: job: Add job_transition_to_ready()...
Checking PATCH 33/40: job: Move progress fields to Job...
Checking PATCH 34/40: job: Introduce qapi/job.json...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#235: 
new file mode 100644

total: 0 errors, 1 warnings, 294 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 35/40: job: Add JOB_STATUS_CHANGE QMP event...
Checking PATCH 36/40: job: Add lifecycle QMP commands...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#37: 
new file mode 100644

total: 0 errors, 1 warnings, 265 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 37/40: job: Add query-jobs QMP command...
Checking PATCH 38/40: blockjob: Remove BlockJob.driver...
ERROR: initializer for struct BlockJobDriver should normally be const
#21: FILE: blockjob.c:107:
+    BlockJobDriver *bjdrv = container_of(drv, BlockJobDriver, job_driver);

ERROR: initializer for struct BlockJobDriver should normally be const
#36: FILE: blockjob.c:121:
+    BlockJobDriver *bjdrv = container_of(drv, BlockJobDriver, job_driver);

total: 2 errors, 0 warnings, 54 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 39/40: iotests: Move qmp_to_opts() to VM...
WARNING: line over 80 characters
#109: FILE: tests/qemu-iotests/iotests.py:448:
+        self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),

total: 0 errors, 1 warnings, 83 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 40/40: qemu-iotests: Test job-* with block jobs...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100755

total: 0 errors, 1 warnings, 540 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.
=== 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] [PATCH v2 00/40] Generic background jobs
Posted by Kevin Wolf 7 years, 5 months ago 
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-devel] [PATCH v2 00/40] Generic background jobs
Posted by Dr. David Alan Gilbert 7 years, 5 months ago 
* Kevin Wolf (kwolf@redhat.com) 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.

Is there any relationship between what this does, and what
Marc-André's 'monitor: add asynchronous command type' tries to do?
(See 20180326150916.9602-1-marcandre.lureau@redhat.com 26th March)

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 00/40] Generic background jobs
Posted by Kevin Wolf 7 years, 5 months ago 
Am 18.05.2018 um 20:41 hat Dr. David Alan Gilbert geschrieben:
> * Kevin Wolf (kwolf@redhat.com) 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.
> 
> Is there any relationship between what this does, and what
> Marc-André's 'monitor: add asynchronous command type' tries to do?
> (See 20180326150916.9602-1-marcandre.lureau@redhat.com 26th March)

Depends on who you ask. :-)

In a way, yes. Marc-André's async commands are for long-running
operations and jobs are for long-running operations. However, they are
for a different kind of "long-running".

Basically, Marc-André's work is for commands that are generally quick,
but perform some blocking operation. Usually commands that are currently
implemented synchronously, but always bothered us because they block the
VM for a while. They take maybe a few seconds at most, but you really
don't want them to block the guest even for short time.

The important part here is you don't want to modify the operations while
they're running, they just send their return value when they are done.
(In the latest version, Marc-André even made sure not to modify the wire
protocol, so IIUC the commands aren't really async any more in the sense
that you can have multiple commands running, but they are just
non-blocking in the sense that the guest can keep running while they are
doing their work.)

In comparison, jobs are typically really long running, like several
minutes (like copying a whole disk image). Some of them can even run
indefinitely (like mirroring) until you explicitly tell them to stop.
You want to continue using the monitor while they are running, and to be
able to manage them at runtime (pause/resume/set speed/cancel/...).

So I think both address different problems.

Kevin
Re: [Qemu-devel] [PATCH v2 00/40] Generic background jobs
Posted by Marc-André Lureau 7 years, 5 months ago 
Hi

On Tue, May 22, 2018 at 1:01 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 18.05.2018 um 20:41 hat Dr. David Alan Gilbert geschrieben:
>> * Kevin Wolf (kwolf@redhat.com) 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.
>>
>> Is there any relationship between what this does, and what
>> Marc-André's 'monitor: add asynchronous command type' tries to do?
>> (See 20180326150916.9602-1-marcandre.lureau@redhat.com 26th March)
>
> Depends on who you ask. :-)
>
> In a way, yes. Marc-André's async commands are for long-running
> operations and jobs are for long-running operations. However, they are
> for a different kind of "long-running".
>
> Basically, Marc-André's work is for commands that are generally quick,
> but perform some blocking operation. Usually commands that are currently
> implemented synchronously, but always bothered us because they block the
> VM for a while. They take maybe a few seconds at most, but you really
> don't want them to block the guest even for short time.
>
> The important part here is you don't want to modify the operations while
> they're running, they just send their return value when they are done.
> (In the latest version, Marc-André even made sure not to modify the wire
> protocol, so IIUC the commands aren't really async any more in the sense
> that you can have multiple commands running, but they are just
> non-blocking in the sense that the guest can keep running while they are
> doing their work.)
>
> In comparison, jobs are typically really long running, like several
> minutes (like copying a whole disk image). Some of them can even run
> indefinitely (like mirroring) until you explicitly tell them to stop.
> You want to continue using the monitor while they are running, and to be
> able to manage them at runtime (pause/resume/set speed/cancel/...).
>
> So I think both address different problems.

I agree with Kevin that both address different needs and complement
each other. Right now, QMP commands are handled synchronous, so they
block the main thread (or the "OOB" thread). I needed a simple way to
handle them asynchronously, *without* breaking or changing an existing
QMP command API. Job, on the other hand, provides a lot of facilities
that are unnecessary for most short living commands, while adding
complexity, at the protocol level (for instance, by using an extra
"job-id" space and not being tied to the qmp session). Depending on
the facilities Job provide, and the guarantee that must hold when
using it, it may make sense or help to transform an existing command
to an asynchronous version, but I doubt many of the existing commands
will need it. However, if there is a need to introduce job-like
facilities to QMP async (such as running concurrent commands, listing
running commands, being able to cancel them, being notified on
progression etc), then I think we should be careful to use Job. For
now, this is unneeded, as QMP async is internal to qemu, the client
can't run anything  to manage an ongoing async command, so I think
both are improvements and will coexist harmoniously.
Re: [Qemu-devel] [PATCH v2 00/40] Generic background jobs
Posted by Dr. David Alan Gilbert 7 years, 5 months ago 
* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> Hi
> 
> On Tue, May 22, 2018 at 1:01 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> > Am 18.05.2018 um 20:41 hat Dr. David Alan Gilbert geschrieben:
> >> * Kevin Wolf (kwolf@redhat.com) 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.
> >>
> >> Is there any relationship between what this does, and what
> >> Marc-André's 'monitor: add asynchronous command type' tries to do?
> >> (See 20180326150916.9602-1-marcandre.lureau@redhat.com 26th March)
> >
> > Depends on who you ask. :-)
> >
> > In a way, yes. Marc-André's async commands are for long-running
> > operations and jobs are for long-running operations. However, they are
> > for a different kind of "long-running".
> >
> > Basically, Marc-André's work is for commands that are generally quick,
> > but perform some blocking operation. Usually commands that are currently
> > implemented synchronously, but always bothered us because they block the
> > VM for a while. They take maybe a few seconds at most, but you really
> > don't want them to block the guest even for short time.
> >
> > The important part here is you don't want to modify the operations while
> > they're running, they just send their return value when they are done.
> > (In the latest version, Marc-André even made sure not to modify the wire
> > protocol, so IIUC the commands aren't really async any more in the sense
> > that you can have multiple commands running, but they are just
> > non-blocking in the sense that the guest can keep running while they are
> > doing their work.)
> >
> > In comparison, jobs are typically really long running, like several
> > minutes (like copying a whole disk image). Some of them can even run
> > indefinitely (like mirroring) until you explicitly tell them to stop.
> > You want to continue using the monitor while they are running, and to be
> > able to manage them at runtime (pause/resume/set speed/cancel/...).
> >
> > So I think both address different problems.
> 
> I agree with Kevin that both address different needs and complement
> each other. Right now, QMP commands are handled synchronous, so they
> block the main thread (or the "OOB" thread). I needed a simple way to
> handle them asynchronously, *without* breaking or changing an existing
> QMP command API. Job, on the other hand, provides a lot of facilities
> that are unnecessary for most short living commands, while adding
> complexity, at the protocol level (for instance, by using an extra
> "job-id" space and not being tied to the qmp session). Depending on
> the facilities Job provide, and the guarantee that must hold when
> using it, it may make sense or help to transform an existing command
> to an asynchronous version, but I doubt many of the existing commands
> will need it. However, if there is a need to introduce job-like
> facilities to QMP async (such as running concurrent commands, listing
> running commands, being able to cancel them, being notified on
> progression etc), then I think we should be careful to use Job. For
> now, this is unneeded, as QMP async is internal to qemu, the client
> can't run anything  to manage an ongoing async command, so I think
> both are improvements and will coexist harmoniously.

OK; it seems we are carving stuff up into a whole bunch of different
categories based on their longevity and blockableness.
(Normal, OOB, Async, Job)

I do worry a bit that pretty much anything that's not 'instant' means
it's waiting for something somewhere, and because you're waiting
for something that means you've got to be able to fail or cancel
it in case that thing never happens.

Dave
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.