From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54700) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKt57-00060G-UQ for qemu-devel@nongnu.org; Mon, 09 Feb 2015 13:29:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YKt52-00088O-Vi for qemu-devel@nongnu.org; Mon, 09 Feb 2015 13:29:13 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60510) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YKt52-00088C-O0 for qemu-devel@nongnu.org; Mon, 09 Feb 2015 13:29:08 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t19IT72d028102 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 9 Feb 2015 13:29:08 -0500 Message-ID: <54D8FC70.5050107@redhat.com> Date: Mon, 09 Feb 2015 13:29:04 -0500 From: Max Reitz MIME-Version: 1.0 References: <1423501897-30410-1-git-send-email-mreitz@redhat.com> <1423501897-30410-2-git-send-email-mreitz@redhat.com> <54D8F9D2.5000506@redhat.com> In-Reply-To: <54D8F9D2.5000506@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 01/37] blockdev: Allow creation of BDS trees without BB List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , John Snow , Markus Armbruster , Stefan Hajnoczi On 2015-02-09 at 13:17, Eric Blake wrote: > 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 >> --- >> 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 > > >> +++ 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"? Well, it's not a guest-visible device; a BlockBackend is the connector between a BDS tree and a guest-visible device. However, I thought about the same thing, but there are already occurrences of "block driver state", so I decided to just go with it (I mean, we could use "block backend", but I don't know if that's really better). Max > 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.