Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2695 bytes --]

On 02/09/2015 10:11 AM, Max Reitz wrote:
> If the "id" field is missing from the options given to blockdev-add,
> just omit the BlockBackend and create the BlockDriverState tree alone.
> 
> However, if "id" is missing, "node-name" must be specified; otherwise,
> the BDS tree would no longer be accessible.
> 

Well, if we ever revived Jeff Cody's attempt at auto-assigning node
names (so that we never have an unnamed node), then this patch will have
to be partially reverted at that time (omitting id and node-name then
results in a BDS with an auto-assigned node name and no BB).  But that's
a decision for that series (if we ever revive it); for now, your policy
is just fine.

> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  blockdev.c                 | 44 +++++++++++++++++++++++++++++++-------------
>  qapi/block-core.json       | 13 +++++++++----
>  tests/qemu-iotests/087     |  2 +-
>  tests/qemu-iotests/087.out |  4 ++--
>  4 files changed, 43 insertions(+), 20 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>


> +++ b/qapi/block-core.json
> @@ -1260,9 +1260,12 @@
>  #
>  # @driver:        block driver name
>  # @id:            #optional id by which the new block device can be referred to.
> -#                 This is a required option on the top level of blockdev-add, and
> -#                 currently not allowed on any other level.
> -# @node-name:     #optional the name of a block driver state node (Since 2.0)
> +#                 This option is only allowed on the top level of blockdev-add.
> +#                 A BlockBackend will be created by blockdev-add if and only if
> +#                 this option is given.

I know what you mean here, but it feels a tiny bit like we are leaking
implementation details.  Would it be any better to state that: "A
guest-visible device will be created by blockdev-add if and only if this
option is given"?  That is, instead of BlockDriverState and BlockBackend
(which are internal naming conventions), should our documentation be
favoring "node within a tree of host-accessible resources that provide
the media content to a guest device" and "guest-visible device"?  But
just in typing that out, it gets tedious, and even if we do make such a
change in documentation, it would be better to do it over all existing
.json files rather than just this patch.  Furthermore, we may use
BlockBackend for things like NBD fleecing operations, which really
aren't guest-visible devices.  So my idle ramblings here don't affect my
R-b for the patch as-is.


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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]