[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB |
Date: |
Mon, 09 Feb 2015 11:17:54 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
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 <address@hidden>
> ---
> 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 <address@hidden>
> +++ 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
signature.asc
Description: OpenPGP digital signature
- [Qemu-devel] [PATCH v2 00/37] blockdev: BlockBackend and media, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 04/37] hw/usb-storage: Check whether BB is inserted, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 06/37] block: Make bdrv_is_inserted() return a bool, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 07/37] block: Add blk_is_available(), Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 05/37] block: Fix BB AIOCB AioContext without BDS, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB, Max Reitz, 2015/02/09
- Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB,
Eric Blake <=
- [Qemu-devel] [PATCH v2 03/37] hw/block/fdc: Implement tray status, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 02/37] iotests: Only create BB if necessary, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 08/37] block: Make bdrv_is_inserted() recursive, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 10/37] block: Move guest_block_size into BlockBackend, Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 09/37] block/quorum: Implement bdrv_is_inserted(), Max Reitz, 2015/02/09
- [Qemu-devel] [PATCH v2 11/37] block: Remove wr_highest_sector from BlockAcctStats, Max Reitz, 2015/02/09