Am 06.10.2016 um 15:02 hat Alberto Garcia geschrieben: > This patch makes the 'device' parameter of the 'block-stream' command > accept a node name that is not a root node. > > In addition to that, operation blockers will be checked in all > intermediate nodes between the top and the base node. > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > blockdev.c | 11 +++++++++-- > qapi/block-core.json | 10 +++++++--- > 2 files changed, 16 insertions(+), 5 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index f13fc69..57c8329 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -2937,7 +2937,7 @@ void qmp_block_stream(bool has_job_id, const char > *job_id, const char *device, > bool has_on_error, BlockdevOnError on_error, > Error **errp) > { > - BlockDriverState *bs; > + BlockDriverState *bs, *iter; > BlockDriverState *base_bs = NULL; > AioContext *aio_context; > Error *local_err = NULL; > @@ -2947,7 +2947,7 @@ void qmp_block_stream(bool has_job_id, const char > *job_id, const char *device, > on_error = BLOCKDEV_ON_ERROR_REPORT; > } > > - bs = qmp_get_root_bs(device, errp); > + bs = bdrv_lookup_bs(device, device, errp); > if (!bs) { > return; > } > aio_context = bdrv_get_aio_context(bs); > aio_context_acquire(aio_context); > > if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) { > goto out; > }
Added a bit more context. This check is redundant now... > if (has_base) { > base_bs = bdrv_find_backing_image(bs, base); > if (base_bs == NULL) { > error_setg(errp, QERR_BASE_NOT_FOUND, base); > goto out; > } > assert(bdrv_get_aio_context(base_bs) == aio_context); > base_name = base; > } > > + /* Check for op blockers in the whole chain between bs and base */ > + for (iter = bs; iter && iter != base_bs; iter = backing_bs(iter)) { > + if (bdrv_op_is_blocked(iter, BLOCK_OP_TYPE_STREAM, errp)) { > + goto out; > + } > + } ...because you start with iter = bs here. Another thought I had while looking at the previous few patches is whether all the op blocker checks wouldn't be better moved to the actual block job code (i.e. stream_start in this case). In this case it doesn't matter much because this is the only caller of stream_start(), but for other jobs the situation is more complicated and it would be easy for a caller to forget the checks. Probably also matter for another series, but I wanted to mention it. > + > /* if we are streaming the entire chain, the result will have no backing > * file, and specifying one is therefore an error */ > if (base_bs == NULL && has_backing_file) { Kevin