Re: [Qemu-devel] [PATCH 2/8] block: allow block jobs in any arbitrary node

[Eric Blake]

On 04/16/2015 09:12 AM, Alberto Garcia wrote:
> Currently, block jobs can only be owned by root nodes. This patch
> allows block jobs to be in any arbitrary node, by making the following
> changes:
> 
> - Block jobs can now be identified by the node name of their
>   BlockDriverState in addition to the device name. Since both device
>   and node names live in the same namespace there's no ambiguity.
> 
> - The "device" parameter used by all commands that operate on block
>   jobs can also be a node name now.
> 
> - The node name is used as a fallback to fill in the BlockJobInfo
>   structure and all BLOCK_JOB_* events if there is no device name for
>   that job.
> 
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
>  block/mirror.c            |  5 +++--
>  blockdev.c                | 16 ++++++++--------
>  blockjob.c                | 18 ++++++++++--------
>  docs/qmp/qmp-events.txt   |  8 ++++----
>  include/qapi/qmp/qerror.h |  3 ---
>  qapi/block-core.json      | 20 ++++++++++----------
>  6 files changed, 35 insertions(+), 35 deletions(-)
> 

> +++ b/docs/qmp/qmp-events.txt
> @@ -89,7 +89,7 @@ Data:
>  
>  - "type":     Job type (json-string; "stream" for image streaming
>                                       "commit" for block commit)
> -- "device":   Device name (json-string)
> +- "device":   Device name, or node name if not present (json-string)

You used this wording for all the events...

> +++ b/qapi/block-core.json
> @@ -540,7 +540,7 @@
>  #
>  # @type: the job type ('stream' for image streaming)
>  #
> -# @device: the block device name
> +# @device: the block device name, or node name if not present
>  #

and again here,

>  # @len: the maximum progress value
>  #
> @@ -1065,7 +1065,7 @@
>  #
>  # Throttling can be disabled by setting the speed to 0.
>  #
> -# @device: the device name
> +# @device: the device or node name of the owner of the block job.

but this wording is different.  I like it a bit better ("node name if
not present" feels like it could be a bit ambiguous as to what is not
present, and "device or node name" is shorter than "device name, or node
name if not present").

I'm not sure if it's worth changing, and I could live with the patch
as-is, so:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature