From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32918) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSv4j-0004Dt-9Z for qemu-devel@nongnu.org; Sat, 13 Sep 2014 17:41:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XSv4c-0003wx-0u for qemu-devel@nongnu.org; Sat, 13 Sep 2014 17:41:45 -0400 Received: from mx1.redhat.com ([209.132.183.28]:7146) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XSv4b-0003wj-QA for qemu-devel@nongnu.org; Sat, 13 Sep 2014 17:41:37 -0400 Message-ID: <5414B9FC.4010004@redhat.com> Date: Sat, 13 Sep 2014 23:41:16 +0200 From: Max Reitz MIME-Version: 1.0 References: <1410620427-20089-1-git-send-email-armbru@redhat.com> <1410620427-20089-3-git-send-email-armbru@redhat.com> In-Reply-To: <1410620427-20089-3-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 02/23] block: New BlockBackend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , qemu-devel@nongnu.org Cc: kwolf@redhat.com, stefanha@redhat.com, benoit.canet@nodalink.com On 13.09.2014 17:00, Markus Armbruster wrote: > A block device consists of a frontend device model and a backend. > > A block backend has a tree of block drivers doing the actual work. > The tree is managed by the block layer. > > We currently use a single abstraction BlockDriverState both for tree > nodes and the backend as a whole. Drawbacks: > > * Its API includes both stuff that makes sense only at the block > backend level (root of the tree) and stuff that's only for use > within the block layer. This makes the API bigger and more complex > than necessary. Moreover, it's not obvious which interfaces are > meant for device models, and which really aren't. > > * Since device models keep a reference to their backend, the backend > object can't just be destroyed. But for media change, we need to > replace the tree. Our solution is to make the BlockDriverState > generic, with actual driver state in a separate object, pointed to > by member opaque. That lets us replace the tree by deinitializing > and reinitializing its root. This special need of the root makes > the data structure awkward everywhere in the tree. > > The general plan is to separate the APIs into "block backend", for use > by device models, monitor and whatever other code dealing with block > backends, and "block driver", for use by the block layer and whatever > other code (if any) dealing with trees and tree nodes. > > Code dealing with block backends, device models in particular, should > become completely oblivious of BlockDriverState. This should let us > clean up both APIs, and the tree data structures. > > This commit is a first step. It creates a minimal "block backend" > API: type BlockBackend and functions to create, destroy and find them. > > BlockBackend objects are created and destroyed exactly when root > BlockDriverState objects are created and destroyed. "Root" in the > sense of "in bdrv_states". They're not yet used for anything; that'll > come shortly. > > BlockBackend is reference-counted. Its reference count never exceeds > one so far, but that's going to change. > > Signed-off-by: Markus Armbruster > --- > block.c | 3 +- > block/Makefile.objs | 2 +- > block/block-backend.c | 119 +++++++++++++++++++++++++++++++++++++++++ > blockdev.c | 21 +++++++- > hw/block/xen_disk.c | 11 ++++ > include/qemu/typedefs.h | 1 + > include/sysemu/block-backend.h | 26 +++++++++ > qemu-img.c | 70 +++++++++++++++++++++--- > qemu-io.c | 8 +++ > qemu-nbd.c | 5 +- > 10 files changed, 253 insertions(+), 13 deletions(-) > create mode 100644 block/block-backend.c > create mode 100644 include/sysemu/block-backend.h [snip] > diff --git a/qemu-img.c b/qemu-img.c > index 58d59d0..acb272e 100644 > --- a/qemu-img.c > +++ b/qemu-img.c [snip] > @@ -942,6 +952,7 @@ static int check_empty_sectors(BlockDriverState *bs, int64_t sect_num, > static int img_compare(int argc, char **argv) > { > const char *fmt1 = NULL, *fmt2 = NULL, *cache, *filename1, *filename2; > + BlockBackend *blk1, *blk2; > BlockDriverState *bs1, *bs2; By initializing all of these to NULL we could remove out1, out2 and out3, and just always go to out. But of course it's not wrong this way. Reviewed-by: Max Reitz