From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50616) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRgRm-0001O5-Op for qemu-devel@nongnu.org; Wed, 10 Sep 2014 07:52:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XRgRg-0003j3-En for qemu-devel@nongnu.org; Wed, 10 Sep 2014 07:52:26 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:39023 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XRgRg-0003ie-3w for qemu-devel@nongnu.org; Wed, 10 Sep 2014 07:52:20 -0400 Date: Wed, 10 Sep 2014 13:51:25 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140910115124.GA26939@irqsave.net> References: <1410336832-22160-1-git-send-email-armbru@redhat.com> <1410336832-22160-3-git-send-email-armbru@redhat.com> <20140910113417.GA22376@irqsave.net> <20140910114417.GB844@noname.str.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140910114417.GB844@noname.str.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 02/23] block: New BlockBackend List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: =?iso-8859-1?Q?Beno=EEt?= Canet , famz@redhat.com, Markus Armbruster , stefanha@redhat.com, qemu-devel@nongnu.org The Wednesday 10 Sep 2014 =E0 13:44:17 (+0200), Kevin Wolf wrote : > Am 10.09.2014 um 13:34 hat Beno=EEt Canet geschrieben: > > The Wednesday 10 Sep 2014 =E0 10:13:31 (+0200), Markus Armbruster wro= te : > > > A block device consists of a frontend device model and a backend. > > >=20 > > > A block backend has a tree of block drivers doing the actual work. > > > The tree is managed by the block layer. > > >=20 > > > We currently use a single abstraction BlockDriverState both for tre= e > > > nodes and the backend as a whole. Drawbacks: > > >=20 > > > * 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 compl= ex > > > than necessary. Moreover, it's not obvious which interfaces are > > > meant for device models, and which really aren't. > > >=20 > > > * Since device models keep a reference to their backend, the backen= d > > > 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 t= o > > > by member opaque. That lets us replace the tree by deinitializin= g > > > and reinitializing its root. This special need of the root makes > > > the data structure awkward everywhere in the tree. > > >=20 > > > The general plan is to separate the APIs into "block backend", for = use > > > by device models, monitor and whatever other code dealing with bloc= k > > > backends, and "block driver", for use by the block layer and whatev= er > > > other code (if any) dealing with trees and tree nodes. > > >=20 > > > Code dealing with block backends, device models in particular, shou= ld > > > become completely oblivious of BlockDriverState. This should let u= s > > > clean up both APIs, and the tree data structures. > > >=20 > > > This commit is a first step. It creates a minimal "block backend" > > > API: type BlockBackend and functions to create, destroy and find th= em. > > > BlockBackend objects are created and destroyed, but not yet used fo= r > > > anything; that'll come shortly. > > >=20 > > > BlockBackend is reference-counted. Its reference count never excee= ds > > > one so far, but that's going to change. > > >=20 > > > Signed-off-by: Markus Armbruster >=20 > > > +/** > > > + * blk_ref: > > > + * > > > + * Increment @blk's reference count. > > > + */ > > > +void blk_ref(BlockBackend *blk) > > > +{ > >=20 > > if blk_unref you take care of doing > > + if (blk) { > > to make sur the user does not pass a NULL pointer. > > Transforming blk into a NULL pointer is not a side effect > > of blk_unref so this test is designed to prevent a user > > brain damage. >=20 > Not really, I'd rather consider it a convenience feature, just like > you're allowed to call free(NULL) or bdrv_unref(NULL) without having a > check for !=3D NULL everywhere. This will be handy especially in error > paths. ok I see the spirit of it. Benoit >=20 > > If the user can be brain damaged to pass a NULL to blk_unref he > > could be equally stupid passing a NULL to blk_ref. > > Why not adding the same test here ? >=20 > Whereas in blk_ref() it really wouldn't make any sense. >=20 > Kevin >=20