Message ID | 1348845782-15073-1-git-send-email-pbonzini@redhat.com |
---|---|
State | New |
Headers | show |
On 09/28/2012 09:22 AM, Paolo Bonzini wrote: > Kevin, > > the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43: > > add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500) > > are available in the git repository at: > > git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1 > > for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097: > > qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200) > > This message has the diff from my post of Sep 25th. You reviewed 18 patches, > these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch. I reviewed the interdiff, and it appears to address all the concerns from v3 for these patches, other than my request to include 'type' as part of the new event, possibly as an enum rather than freeform string. But since we agreed to defer that to a followup patch, it does not hold up this series. I also did one last sanity read through the patches. Series: Reviewed-by: Eric Blake <eblake@redhat.com>
Am 28.09.2012 17:22, schrieb Paolo Bonzini: > Kevin, > > the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43: > > add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500) > > are available in the git repository at: > > git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1 > > for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097: > > qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200) > > This message has the diff from my post of Sep 25th. You reviewed 18 patches, > these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch. > > ---------------------------------------------------------------- > Jeff Cody (1): > blockdev: rename block_stream_cb to a generic block_job_cb > > Paolo Bonzini (18): > qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE > block: fix documentation of block_job_cancel_sync > block: move job APIs to separate files > block: add block_job_query > qmp: add 'busy' member to BlockJobInfo > block: add support for job pause/resume > qmp: add block-job-pause and block-job-resume > qemu-iotests: add test for pausing a streaming operation > block: rename block_job_complete to block_job_completed > iostatus: rename BlockErrorAction, BlockQMPEventAction > iostatus: move BlockdevOnError declaration to QAPI > iostatus: change is_read to a bool > iostatus: reorganize io error code > block: introduce block job error > stream: add on-error argument > blkdebug: process all set_state rules in the old state > qemu-iotests: map underscore to dash in QMP argument names > qemu-iotests: add tests for streaming error handling > > Makefile.objs | 5 +- > QMP/qmp-events.txt | 22 ++++ > block.c | 187 ++++++++---------------------- > block.h | 20 ++-- > block/Makefile.objs | 3 +- > block/blkdebug.c | 12 +- > block/stream.c | 33 +++++- > block_int.h | 162 ++------------------------ > blockdev.c | 86 +++++++++----- > blockjob.c | 249 ++++++++++++++++++++++++++++++++++++++++ > blockjob.h | 243 +++++++++++++++++++++++++++++++++++++++ > hmp-commands.hx | 35 +++++- > hmp.c | 26 ++++- > hmp.h | 2 + > hw/fdc.c | 4 +- > hw/ide/core.c | 22 +--- > hw/ide/pci.c | 4 +- > hw/scsi-disk.c | 25 ++-- > hw/scsi-generic.c | 4 +- > hw/virtio-blk.c | 23 ++-- > monitor.c | 1 + > monitor.h | 1 + > qapi-schema.json | 91 ++++++++++++++- > qemu-tool.c | 6 + > qerror.h | 6 + > qmp-commands.hx | 14 ++- > tests/qemu-iotests/030 | 260 +++++++++++++++++++++++++++++++++++++++++- > tests/qemu-iotests/030.out | 4 +- > tests/qemu-iotests/group | 2 +- > tests/qemu-iotests/iotests.py | 15 ++- > trace-events | 4 +- > 31 file modificati, 1152 inserzioni(+), 419 rimozioni(-) > create mode 100644 blockjob.c > create mode 100644 blockjob.h Thanks, applied all to the block branch. Kevin
Il 28/09/2012 20:23, Kevin Wolf ha scritto: > Am 28.09.2012 17:22, schrieb Paolo Bonzini: >> Kevin, >> >> the following changes since commit ac05f3492421caeb05809ffa02c6198ede179e43: >> >> add a boot parameter to set reboot timeout (2012-09-25 20:05:04 -0500) >> >> are available in the git repository at: >> >> git://github.com/bonzini/qemu.git blkmirror-job-1.3-part1 >> >> for you to fetch changes up to ed306a929f16fda8a85561430b1ac370bde65097: >> >> qemu-iotests: add tests for streaming error handling (2012-09-27 15:11:22 +0200) >> >> This message has the diff from my post of Sep 25th. You reviewed 18 patches, >> these are 19 because of the new "qmp: add 'busy' member to BlockJobInfo" patch. >> >> ---------------------------------------------------------------- >> Jeff Cody (1): >> blockdev: rename block_stream_cb to a generic block_job_cb >> >> Paolo Bonzini (18): >> qerror/block: introduce QERR_BLOCK_JOB_NOT_ACTIVE >> block: fix documentation of block_job_cancel_sync >> block: move job APIs to separate files >> block: add block_job_query >> qmp: add 'busy' member to BlockJobInfo >> block: add support for job pause/resume >> qmp: add block-job-pause and block-job-resume >> qemu-iotests: add test for pausing a streaming operation >> block: rename block_job_complete to block_job_completed >> iostatus: rename BlockErrorAction, BlockQMPEventAction >> iostatus: move BlockdevOnError declaration to QAPI >> iostatus: change is_read to a bool >> iostatus: reorganize io error code >> block: introduce block job error >> stream: add on-error argument >> blkdebug: process all set_state rules in the old state >> qemu-iotests: map underscore to dash in QMP argument names >> qemu-iotests: add tests for streaming error handling >> >> Makefile.objs | 5 +- >> QMP/qmp-events.txt | 22 ++++ >> block.c | 187 ++++++++---------------------- >> block.h | 20 ++-- >> block/Makefile.objs | 3 +- >> block/blkdebug.c | 12 +- >> block/stream.c | 33 +++++- >> block_int.h | 162 ++------------------------ >> blockdev.c | 86 +++++++++----- >> blockjob.c | 249 ++++++++++++++++++++++++++++++++++++++++ >> blockjob.h | 243 +++++++++++++++++++++++++++++++++++++++ >> hmp-commands.hx | 35 +++++- >> hmp.c | 26 ++++- >> hmp.h | 2 + >> hw/fdc.c | 4 +- >> hw/ide/core.c | 22 +--- >> hw/ide/pci.c | 4 +- >> hw/scsi-disk.c | 25 ++-- >> hw/scsi-generic.c | 4 +- >> hw/virtio-blk.c | 23 ++-- >> monitor.c | 1 + >> monitor.h | 1 + >> qapi-schema.json | 91 ++++++++++++++- >> qemu-tool.c | 6 + >> qerror.h | 6 + >> qmp-commands.hx | 14 ++- >> tests/qemu-iotests/030 | 260 +++++++++++++++++++++++++++++++++++++++++- >> tests/qemu-iotests/030.out | 4 +- >> tests/qemu-iotests/group | 2 +- >> tests/qemu-iotests/iotests.py | 15 ++- >> trace-events | 4 +- >> 31 file modificati, 1152 inserzioni(+), 419 rimozioni(-) >> create mode 100644 blockjob.c >> create mode 100644 blockjob.h > > Thanks, applied all to the block branch. Great, I rebased and only got mostly trivial conflicts. I pushed the outcome to the branch blkmirror-job-20120925-rebase of my github repository. Do I need to repost, or are you going to continue your review on the previous patchset? The next obvious points to step are: - part 2: "qemu-iotests: add mirroring test case" or "qemu-iotests: add testcases for mirroring on-source-error/on-target-error" - part 3: "mirror: perform COW if the cluster size is bigger than the granularity" or "mirror: allow customizing the granularity" - part 4: "mirror: support arbitrarily-sized iterations" Paolo
diff --git a/blockjob.c b/blockjob.c index 5dd9c1e..b5c16f3 100644 --- a/blockjob.c +++ b/blockjob.c @@ -198,6 +198,7 @@ BlockJobInfo *block_job_query(BlockJob *job) info->type = g_strdup(job->job_type->job_type); info->device = g_strdup(bdrv_get_device_name(job->bs)); info->len = job->len; + info->busy = job->busy; info->paused = job->paused; info->offset = job->offset; info->speed = job->speed; diff --git a/blockjob.h b/blockjob.h index 45cc03d..c2261a9 100644 --- a/blockjob.h +++ b/blockjob.h @@ -199,7 +199,8 @@ void block_job_resume(BlockJob *job); * block_job_is_paused: * @job: The job being queried. * - * Returns whether the job is currently paused. + * Returns whether the job is currently paused, or will pause + * as soon as it reaches a sleeping point. */ bool block_job_is_paused(BlockJob *job); @@ -207,12 +208,10 @@ bool block_job_is_paused(BlockJob *job); * block_job_cancel_sync: * @job: The job to be canceled. * - * Synchronously cancel the job and wait for it to reach a quiescent - * state. Note that the completion callback will still be called - * asynchronously, hence it is *not* valid to call #bdrv_delete - * immediately after #block_job_cancel_sync. Users of block jobs - * will usually protect the BlockDriverState objects with a reference - * count, should this be a concern. + * 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. diff --git a/qapi-schema.json b/qapi-schema.json index 8719a9d..d31ff0e 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -1100,11 +1100,11 @@ # @ignore: ignore the error, only report a QMP event (BLOCK_IO_ERROR # or BLOCK_JOB_ERROR) # +# @enospc: same as @stop on ENOSPC, same as @report otherwise. +# # @stop: for guest operations, stop the virtual machine; # for jobs, pause the job # -# @enospc: same as @stop on ENOSPC, same as @report otherwise. -# # Since: 1.3 ## { 'enum': 'BlockdevOnError', @@ -1121,19 +1121,23 @@ # # @len: the maximum progress value # -# @paused: whether the job is paused (since 1.2) +# @busy: false if the job is known to be in a quiescent state, with +# no pending I/O. Since 1.3. +# +# @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.2) +# @io-status: the status of the job (since 1.3) # # Since: 1.1 ## { 'type': 'BlockJobInfo', 'data': {'type': 'str', 'device': 'str', 'len': 'int', - 'offset': 'int', 'paused': 'bool', 'speed': 'int', + 'offset': 'int', 'busy': 'bool', 'paused': 'bool', 'speed': 'int', 'io-status': 'BlockDeviceIoStatus'} } ## @@ -1833,7 +1837,7 @@ # # @on-error: #optional the action to take on an error (default report). # 'stop' and 'enospc' can only be used if the block device -# supports io-status (see BlockInfo). Since 1.2. +# supports io-status (see BlockInfo). Since 1.3. # # Returns: Nothing on success # If @device does not exist, DeviceNotFound @@ -1886,7 +1890,8 @@ # # @device: the device name # -# @force: #optional whether to allow cancellation of a paused job (default false) +# @force: #optional whether to allow cancellation of a paused job (default +# false). Since 1.3. # # Returns: Nothing on success # If no background operation is active on this device, DeviceNotActive