On 08.05.2015 19:21, Kevin Wolf wrote:
This allows iterating over all children of a given BDS, not only
including bs->file and bs->backing_hd, but also driver-specific
ones like VMDK extents or Quorum children.
Signed-off-by: Kevin Wolf <kw...@redhat.com>
---
block.c | 27 +++++++++++++++++++++++++++
include/block/block_int.h | 8 ++++++++
2 files changed, 35 insertions(+)
diff --git a/block.c b/block.c
index c4f0fb4..59f54ed 100644
--- a/block.c
+++ b/block.c
@@ -1301,6 +1301,19 @@ out:
return ret;
}
+static void bdrv_attach_child(BlockDriverState *parent_bs,
+ BlockDriverState *child_bs,
+ const BdrvChildRole *child_role)
+{
+ BdrvChild *child = g_new(BdrvChild, 1);
+ *child = (BdrvChild) {
+ .bs = child_bs,
+ .role = child_role,
+ };
+
+ QLIST_INSERT_HEAD(&parent_bs->children, child, next);
+}
+
/*
* Opens a disk image (raw, qcow2, vmdk, ...)
*
@@ -1353,6 +1366,9 @@ static int bdrv_open_inherit(BlockDriverState **pbs,
const char *filename,
return -ENODEV;
}
bdrv_ref(bs);
+ if (child_role) {
+ bdrv_attach_child(parent, bs, child_role);
+ }
*pbs = bs;
return 0;
}
@@ -1495,6 +1511,10 @@ static int bdrv_open_inherit(BlockDriverState **pbs,
const char *filename,
goto close_and_fail;
}
+ if (child_role) {
+ bdrv_attach_child(parent, bs, child_role);
+ }
+
QDECREF(options);
*pbs = bs;
return 0;
@@ -1789,6 +1809,12 @@ void bdrv_close(BlockDriverState *bs)
notifier_list_notify(&bs->close_notifiers, bs);
if (bs->drv) {
+ BdrvChild *child, *next;
+
+ QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
+ g_free(child);
+ }
+
Not considering the case where the child is closed before the parent
assumes all children are reference-counted from the parent and they
won't be closed (and maybe replaced with another BDS) on purpose. The
first seems reasonable, the second one I'm not so sure about. It works
for now, but I could imagine that we want to modify children of a Quorum
instance at runtime.
But I can't imagine any case where this would break right now, so I
guess I'm fine with it.
if (bs->backing_hd) {
BlockDriverState *backing_hd = bs->backing_hd;
bdrv_set_backing_hd(bs, NULL);
@@ -1999,6 +2025,7 @@ void bdrv_append(BlockDriverState *bs_new,
BlockDriverState *bs_top)
/* The contents of 'tmp' will become bs_top, as we are
* swapping bs_new and bs_top contents. */
bdrv_set_backing_hd(bs_top, bs_new);
+ bdrv_attach_child(bs_top, bs_new, &child_backing);
}
static void bdrv_delete(BlockDriverState *bs)
Using a mirror block job, we can force bdrv_swap() on arbitrary nodes,
right? What happens if you swap e.g. a VMDK and a quorum node? Well,
maybe one simply cannot swap a quorum node due to blockers, but I guess
one can swap a VMDK node with some non-VMDK node. It is actually correct
to leave the extents behind; but the other node cannot do anything with
them, so because they are part of the opaque VMDK structure, they will
de-facto remain with VMDK, while being counted as children of the other
node. But I try to keep so far away from bdrv_swap() that I don't even
know whether this case is even possible.
Max
diff --git a/include/block/block_int.h b/include/block/block_int.h
index eb01ea2..12c7fb3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -334,6 +334,12 @@ struct BdrvChildRole {
extern const BdrvChildRole child_file;
extern const BdrvChildRole child_format;
+typedef struct BdrvChild {
+ BlockDriverState *bs;
+ const BdrvChildRole *role;
+ QLIST_ENTRY(BdrvChild) next;
+} BdrvChild;
+
/*
* Note: the function bdrv_append() copies and swaps contents of
* BlockDriverStates, so if you add new fields to this struct, please
@@ -428,6 +434,8 @@ struct BlockDriverState {
/* long-running background operation */
BlockJob *job;
+ QLIST_HEAD(, BdrvChild) children;
+
QDict *options;
BlockdevDetectZeroesOptions detect_zeroes;