Re: [Qemu-devel] [PATCH for-2.7 v9 00/11] Support streaming to an intermediate layer

[Stefan Hajnoczi]

On Mon, Apr 04, 2016 at 04:43:50PM +0300, Alberto Garcia wrote:
> Hi!
> 
> Half a year later :) here's the new version of the intermediate block
> streaming patches.

CCing Jeff Cody, block jobs maintainer.

> Quick summary: the feature was already working since June but there
> were two problems that made it impossible to merge the patches. Here's
> the full description:
> 
>    https://lists.gnu.org/archive/html/qemu-block/2015-12/msg00180.html
> 
> And here's the TL;DR version:
> 
>    1) Opening a BDS in read-write mode with bdrv_reopen() has the side
>       effect of reopening its backing chain in read-only mode.
> 
>    2) bdrv_reopen() can trigger the completion of an existing block
>       job. If that job modifies the backing chain in which that reopen
>       call is operating, it can crash QEMU.
> 
> Now, these problems are only relevant if there are two block jobs
> running at the same time in the same backing chain. This is a nice
> feature, but it's not essential for the intermediate block streaming
> operation. Therefore I decided to disable it in this version of the
> series.
> 
> The way it works is that if you run block-stream on an intermediate
> node, it will block the active layer as well, preventing other block
> jobs in the same chain.
> 
> Other than that there are no significant changes compared to v8, but a
> couple of things are different:
> 
>  - The code has been rebased
>  - The patch that introduced block_job_next() has been split into
>    several: one that adds the function and the rest that use it in
>    different parts of the QEMU code.
>  - Some patches have already been merged into QEMU so they have been
>    removed from the series.
>  - The test that ran several block-stream operations in parallel now
>    verifies that they are forbidden.
> 
> I think that's all. Comments and questions are welcome!
> 
> Berto
> 
> v9:
> - Rebase the code
> - Block the active layer in order to forbid other block jobs in the
>   same chain.
> - Split the patch that adds block_job_next() into 4 (new patches 1-4).
> - Replace the test that performs several block-stream operations in
>   parallel with one that check that they're forbidden.
> - Remove patches that have already been merged.
> 
> v8: https://lists.gnu.org/archive/html/qemu-devel/2015-06/msg05754.html
> - Rebased on top of Stefan's block branch (0a35bce416)
> - The loop that pauses the block jobs in bdrv_drain_all() is now split
>   in two: one that iterates the list of block jobs to stop them, and
>   one that iterates the root bds in order to get the aio contexts.
> 
> v7: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg02580.html
> - Rebased against the current master
> - Updated bdrv_drain_all() to use the new block_job_next() API.
> 
> v6: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg03046.html
> - fix the no-op test following Max's suggestions
> 
> v5: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg03006.html
> - Fix a few typos
> - Minor documentation updates
> - Update test_stream_partial() to test no-ops
> - New test case: test_stream_parallel()
> - New test case: test_stream_overlapping()
> 
> v4: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg01878.html
> - Refactor find_block_job to use the error from bdrv_lookup_bs()
> - Don't use QERR_DEVICE_IN_USE in block_job_create() since we can be
>   dealing with nodes now.
> - Fix @device comment in the BlockJobInfo documentation
> - stream_start(): simplify the bdrv_reopen() call and use
>   bdrv_get_device_or_node_name() for error messages.
> - Use a different variable name for BlockDriverState *i
> - Documentation fixes in docs/live-block-ops.txt
> - Update iotest 30 since now test_device_not_found() returns
>   GenericError
> - Fix test case test_stream_partial()
> - Add new test case test_stream_intermediate()
> - Fix typos
> 
> v3: https://lists.gnu.org/archive/html/qemu-devel/2015-04/msg00806.html
> - Keep a list of block jobs and make qmp_query_block_jobs() iterate
>   over it.
> 
> v2: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg04798.html
> - The 'block-stream' command does not have a 'node-name' parameter
>   anymore and reuses 'device' for that purpose.
> - Block jobs can now be owned by any intermediate node, and not just
>   by the ones at the root. query-block-jobs is updated to reflect that
>   change.
> - The 'device' parameter of all 'block-job-*' commands can now take a
>   node name.
> - The BlockJobInfo type and all BLOCK_JOB_* events report the node
>   name in the 'device' field if the node does not have a device name.
> - All intermediate nodes are blocked (and checked for blockers) during
>   the streaming operation.
> 
> v1: https://lists.gnu.org/archive/html/qemu-devel/2015-02/msg04116.html
> 
> Alberto Garcia (11):
>   block: keep a list of block jobs
>   block: use the block job list in bdrv_drain_all()
>   block: use the block job list in qmp_query_block_jobs()
>   block: use the block job list in bdrv_close()
>   block: allow block jobs in any arbitrary node
>   block: Support streaming to an intermediate layer
>   block: Add QMP support for streaming to an intermediate layer
>   docs: Document how to stream to an intermediate layer
>   qemu-iotests: test streaming to an intermediate layer
>   qemu-iotests: test overlapping block-stream operations
>   qemu-iotests: test non-overlapping block-stream operations
> 
>  block.c                    |  29 ++++---------
>  block/io.c                 |  21 ++++++----
>  block/stream.c             |  39 ++++++++++++++++-
>  blockdev.c                 |  68 ++++++++++++++++++------------
>  blockjob.c                 |  18 +++++++-
>  docs/live-block-ops.txt    |  31 +++++++++-----
>  docs/qmp-events.txt        |   8 ++--
>  include/block/block_int.h  |   5 ++-
>  include/block/blockjob.h   |  14 +++++++
>  qapi/block-core.json       |  30 ++++++++------
>  tests/qemu-iotests/030     | 101 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/030.out |   4 +-
>  12 files changed, 276 insertions(+), 92 deletions(-)
> 
> -- 
> 2.8.0.rc3
> 
>

signature.asc
Description: PGP signature