mbox series

[00/14] block: Make blockdev-create a job and stable API

Message ID 20180525163327.23097-1-kwolf@redhat.com
Headers show
Series block: Make blockdev-create a job and stable API | expand

Message

Kevin Wolf May 25, 2018, 4:33 p.m. UTC
This changes the x-blockdev-create QMP command so that it doesn't block
the monitor and the main loop any more, but starts a background job that
performs the image creation.

The basic job as implemented here is all that is necessary to make image
creation asynchronous and to provide a QMP interface that can be marked
stable, but it still lacks a few features that jobs usually provide: The
job will ignore pause commands and it doesn't publish progress yet (so
both current-progress and total-progress stay at 0). These features can
be added later without breaking compatibility.

At the end of the series, the interface is declared stable and the x-
prefix is removed.

Kevin Wolf (14):
  vdi: Fix vdi_co_do_create() return value
  vhdx: Fix vhdx_co_create() return value
  job: Add error message for failing jobs
  block/create: Make x-blockdev-create a job
  qemu-iotests: Add VM.get_qmp_events_filtered()
  qemu-iotests: Add VM.qmp_log()
  qemu-iotests: Add iotests.img_info_log()
  qemu-iotests: Rewrite 206 for blockdev-create job
  qemu-iotests: Rewrite 207 for blockdev-create job
  qemu-iotests: Rewrite 210 for blockdev-create job
  qemu-iotests: Rewrite 211 for blockdev-create job
  qemu-iotests: Rewrite 212 for blockdev-create job
  qemu-iotests: Rewrite 213 for blockdev-create job
  block/create: Mark blockdev-create stable

 qapi/block-core.json          |  18 +-
 qapi/job.json                 |   4 +-
 include/qemu/job.h            |   7 +-
 block/backup.c                |   2 +-
 block/commit.c                |   2 +-
 block/create.c                |  61 ++--
 block/mirror.c                |   2 +-
 block/stream.c                |   2 +-
 block/vdi.c                   |   1 +
 block/vhdx.c                  |   2 +-
 job-qmp.c                     |   9 +-
 job.c                         |  15 +-
 tests/qemu-iotests/206        | 705 +++++++++++++++++-------------------------
 tests/qemu-iotests/206.out    | 241 +++++++++------
 tests/qemu-iotests/207        | 435 ++++++++++++--------------
 tests/qemu-iotests/207.out    |  89 +++---
 tests/qemu-iotests/210        | 393 ++++++++++-------------
 tests/qemu-iotests/210.out    | 189 +++++++----
 tests/qemu-iotests/211        | 384 ++++++++++-------------
 tests/qemu-iotests/211.out    | 123 ++++----
 tests/qemu-iotests/212        | 483 +++++++++++------------------
 tests/qemu-iotests/212.out    | 181 +++++++----
 tests/qemu-iotests/213        | 520 ++++++++++++-------------------
 tests/qemu-iotests/213.out    | 198 +++++++-----
 tests/qemu-iotests/iotests.py |  74 +++++
 25 files changed, 1955 insertions(+), 2185 deletions(-)

Comments

no-reply@patchew.org May 25, 2018, 4:52 p.m. UTC | #1
Hi,

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

Type: series
Message-id: 20180525163327.23097-1-kwolf@redhat.com
Subject: [Qemu-devel] [PATCH 00/14] block: Make blockdev-create a job and stable API

=== 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/20180425183223.580566-1-eblake@redhat.com -> patchew/20180425183223.580566-1-eblake@redhat.com
 t [tag update]            patchew/20180501142222.19154-1-kbastian@mail.uni-paderborn.de -> patchew/20180501142222.19154-1-kbastian@mail.uni-paderborn.de
 * [new tag]               patchew/20180525163327.23097-1-kwolf@redhat.com -> patchew/20180525163327.23097-1-kwolf@redhat.com
Switched to a new branch 'test'
7c537677c5 block/create: Mark blockdev-create stable
2e27092a14 qemu-iotests: Rewrite 213 for blockdev-create job
d6251d43f0 qemu-iotests: Rewrite 212 for blockdev-create job
d6bc0f4894 qemu-iotests: Rewrite 211 for blockdev-create job
5b0814d1e4 qemu-iotests: Rewrite 210 for blockdev-create job
03bf59b254 qemu-iotests: Rewrite 207 for blockdev-create job
f602a6610b qemu-iotests: Rewrite 206 for blockdev-create job
a9d8a5c88e qemu-iotests: Add iotests.img_info_log()
eb9248b0ec qemu-iotests: Add VM.qmp_log()
7c9e5e8154 qemu-iotests: Add VM.get_qmp_events_filtered()
711b0d9ad8 block/create: Make x-blockdev-create a job
bc7cfcbd4b job: Add error message for failing jobs
3efdd6905e vhdx: Fix vhdx_co_create() return value
0a3f99ead0 vdi: Fix vdi_co_do_create() return value

=== OUTPUT BEGIN ===
Checking PATCH 1/14: vdi: Fix vdi_co_do_create() return value...
Checking PATCH 2/14: vhdx: Fix vhdx_co_create() return value...
Checking PATCH 3/14: job: Add error message for failing jobs...
Checking PATCH 4/14: block/create: Make x-blockdev-create a job...
Checking PATCH 5/14: qemu-iotests: Add VM.get_qmp_events_filtered()...
Checking PATCH 6/14: qemu-iotests: Add VM.qmp_log()...
Checking PATCH 7/14: qemu-iotests: Add iotests.img_info_log()...
ERROR: line over 90 characters
#40: FILE: tests/qemu-iotests/iotests.py:225:
+        line = re.sub('uuid: [-a-f0-9]+', 'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX', line)

total: 1 errors, 0 warnings, 28 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 8/14: qemu-iotests: Rewrite 206 for blockdev-create job...
Checking PATCH 9/14: qemu-iotests: Rewrite 207 for blockdev-create job...
Checking PATCH 10/14: qemu-iotests: Rewrite 210 for blockdev-create job...
Checking PATCH 11/14: qemu-iotests: Rewrite 211 for blockdev-create job...
Checking PATCH 12/14: qemu-iotests: Rewrite 212 for blockdev-create job...
Checking PATCH 13/14: qemu-iotests: Rewrite 213 for blockdev-create job...
Checking PATCH 14/14: block/create: Mark blockdev-create stable...
=== 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
Eric Blake May 25, 2018, 6:13 p.m. UTC | #2
On 05/25/2018 11:33 AM, Kevin Wolf wrote:
> This changes the x-blockdev-create QMP command so that it doesn't block
> the monitor and the main loop any more, but starts a background job that
> performs the image creation.
> 
> The basic job as implemented here is all that is necessary to make image
> creation asynchronous and to provide a QMP interface that can be marked
> stable, but it still lacks a few features that jobs usually provide: The
> job will ignore pause commands and it doesn't publish progress yet (so
> both current-progress and total-progress stay at 0). These features can
> be added later without breaking compatibility.

Can we at least have total-progress start at 1, and current-progress 
move from 0 to 1 at completion?  Seeing a 0/1 => 1/1 transition is 
better than a divide-by-zero 0/0 ratio throughout the entire job; and 
libvirt doesn't want to add any more special-casing of 0/0 than it 
already has (where it wants to treat that as "job not yet started" 
rather than the more usual sense that if total==current the job is 
hopefully complete).

> 
> At the end of the series, the interface is declared stable and the x-
> prefix is removed.
> 
> Kevin Wolf (14):
>    vdi: Fix vdi_co_do_create() return value
>    vhdx: Fix vhdx_co_create() return value
>    job: Add error message for failing jobs
>    block/create: Make x-blockdev-create a job
>    qemu-iotests: Add VM.get_qmp_events_filtered()
>    qemu-iotests: Add VM.qmp_log()
>    qemu-iotests: Add iotests.img_info_log()
>    qemu-iotests: Rewrite 206 for blockdev-create job
>    qemu-iotests: Rewrite 207 for blockdev-create job
>    qemu-iotests: Rewrite 210 for blockdev-create job
>    qemu-iotests: Rewrite 211 for blockdev-create job
>    qemu-iotests: Rewrite 212 for blockdev-create job
>    qemu-iotests: Rewrite 213 for blockdev-create job
>    block/create: Mark blockdev-create stable
Kevin Wolf May 28, 2018, 8:42 a.m. UTC | #3
Am 25.05.2018 um 20:13 hat Eric Blake geschrieben:
> On 05/25/2018 11:33 AM, Kevin Wolf wrote:
> > This changes the x-blockdev-create QMP command so that it doesn't block
> > the monitor and the main loop any more, but starts a background job that
> > performs the image creation.
> > 
> > The basic job as implemented here is all that is necessary to make image
> > creation asynchronous and to provide a QMP interface that can be marked
> > stable, but it still lacks a few features that jobs usually provide: The
> > job will ignore pause commands and it doesn't publish progress yet (so
> > both current-progress and total-progress stay at 0). These features can
> > be added later without breaking compatibility.
> 
> Can we at least have total-progress start at 1, and current-progress move
> from 0 to 1 at completion?  Seeing a 0/1 => 1/1 transition is better than a
> divide-by-zero 0/0 ratio throughout the entire job; and libvirt doesn't want
> to add any more special-casing of 0/0 than it already has (where it wants to
> treat that as "job not yet started" rather than the more usual sense that if
> total==current the job is hopefully complete).

Sure, I can do that.

Kevin