Kevin Wolf <kw...@redhat.com> writes: > Am 16.09.2014 um 20:12 hat Markus Armbruster geschrieben: >> 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 <arm...@redhat.com> > >> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h >> new file mode 100644 >> index 0000000..3f8371c >> --- /dev/null >> +++ b/include/sysemu/block-backend.h >> @@ -0,0 +1,26 @@ >> +/* >> + * QEMU Block backends >> + * >> + * Copyright (C) 2014 Red Hat, Inc. >> + * >> + * Authors: >> + * Markus Armbruster <arm...@redhat.com>, >> + * >> + * This work is licensed under the terms of the GNU GPL, version 2 or >> + * later. See the COPYING file in the top-level directory. >> + */ > > This one should be LGPL as well.
Forgot %-} > The distribution of blk_unref() should be good enough. To be correct it > requires the assumption that the blockdev-add reference to any given BDS > is the last one that goes away. I'm not sure if this is the case here (I > suspect it isn't), but at the end of the series, we should be good. You're doubting the code works when a BB dies, but something else still holds a reference to its BDS, keeping it alive. Yes, that's the troublesome case. Let me review how it develops throughout this series. PATCH 02: BB and BDS are not yet connected, and independently reference counted. Anything that creates a root BDS also creates a BB. The two are unref'ed at the same time. The BB dies for sure, because nothing else has a reference. The BDS may live on, but isn't affected by the death of the BB. PATCH 03: BB and BDS point to each other, but neither owns the other, yet. The BDS is now exposed to the BDS's death: it's pointer bs->blk becomes null. However, it's only ever used right after creation, when the BB surely lives. PATCH 04: BB owns DriveInfo. Adds a few uses of bs->blk, but they're still all in code where the BB surely lives: device model initialization and destruction, drive_del command. PATCH 06: BB owns BDS, taking over the reference from its creator. PATCH 08: First patch that makes the BDS be affected by the death of the BB: the value of bdrv_get_device_name() becomes "" then. The two interesting BB killers are do_drive_del() and blockdev_auto_del(). Both were coded before BDS reference counting, and both used to actually kill the BDS. When do_drive_del() can't kill it, because a device model still has an (uncounted!) reference, it makes it anonymous. Which changes the name to "". I believe both killers need a deep think on what they're supposed to do when the BDS reference count is > 1. PATCH 23: Device models' references to BB become strong. > With the license fixed, you can add: > > Reviewed-by Kevin Wolf <kw...@redhat.com> Thanks!