On 02/20/2015 06:53 AM, Alberto Garcia wrote: > This adds the 'top' parameter to the 'block-stream' QMP command and > checks that its value is valid before passing it to stream_start(). > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > blockdev.c | 19 +++++++++++++++---- > hmp.c | 2 +- > include/qapi/qmp/qerror.h | 3 +++ > qapi/block-core.json | 18 ++++++++++++++---- > qmp-commands.hx | 2 +- > 5 files changed, 34 insertions(+), 10 deletions(-)
> @@ -2123,12 +2125,21 @@ void qmp_block_stream(const char *device,
> aio_context = bdrv_get_aio_context(bs);
> aio_context_acquire(aio_context);
>
> - if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_STREAM, errp)) {
> + if (has_top) {
> + top_bs = bdrv_find_backing_image(bs, top);
> + if (top_bs == NULL) {
> + error_set(errp, QERR_TOP_NOT_FOUND, top);
> + goto out;
> + }
If I understand correctly, bdrv_find_backing_image has problems for
backing nodes that don't have a file name. Given our shift towards node
names, I think we really want to target node names rather than file
names when specifying which node we will use as the top bound receiving
the stream operations.
> +++ b/include/qapi/qmp/qerror.h
> @@ -127,6 +127,9 @@ void qerror_report_err(Error *err);
> #define QERR_SET_PASSWD_FAILED \
> ERROR_CLASS_GENERIC_ERROR, "Could not set password"
>
> +#define QERR_TOP_NOT_FOUND \
> + ERROR_CLASS_GENERIC_ERROR, "Top '%s' not found"
> +
Please don't. Just use error_setg() at the right place with the direct
message (existing QERR_ macros are a legacy holdover, and we shouldn't
be creating more of them).
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
