| Message ID | 20200218124242.584644-13-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: Introduce real BdrvChildRole | expand |
Am 18.02.2020 um 13:42 hat Max Reitz geschrieben: > Make bdrv_child_cb_detach() call bdrv_backing_detach() for children with > a COW role (and drop the reverse call from bdrv_backing_detach()), so it > can be used for any child (with a proper role set). > > Because so far no child has a proper role set, we need a temporary new > callback for child_backing.detach that ensures bdrv_backing_detach() is > called for all COW children that do not have their role set yet. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > block.c | 15 ++++++++++++++- > 1 file changed, 14 insertions(+), 1 deletion(-) > > diff --git a/block.c b/block.c > index 3cf1293a7b..0f24546863 100644 > --- a/block.c > +++ b/block.c > @@ -943,6 +943,7 @@ static void bdrv_child_cb_drained_end(BdrvChild *child, > } > > static void bdrv_backing_attach(BdrvChild *c); > +static void bdrv_backing_detach(BdrvChild *c); This series leaves a few static forward declarations behind, and even in the middle of the code rather than at the top. Does anything stop us from adding bdrv_inherited_options() after all the old functions instead? This will require a temporary forward declaration, too, but it can go away at the end of the series when there is only child_of_bds left. Kevin
On 06.05.20 14:41, Kevin Wolf wrote: > Am 18.02.2020 um 13:42 hat Max Reitz geschrieben: >> Make bdrv_child_cb_detach() call bdrv_backing_detach() for children with >> a COW role (and drop the reverse call from bdrv_backing_detach()), so it >> can be used for any child (with a proper role set). >> >> Because so far no child has a proper role set, we need a temporary new >> callback for child_backing.detach that ensures bdrv_backing_detach() is >> called for all COW children that do not have their role set yet. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> block.c | 15 ++++++++++++++- >> 1 file changed, 14 insertions(+), 1 deletion(-) >> >> diff --git a/block.c b/block.c >> index 3cf1293a7b..0f24546863 100644 >> --- a/block.c >> +++ b/block.c >> @@ -943,6 +943,7 @@ static void bdrv_child_cb_drained_end(BdrvChild *child, >> } >> >> static void bdrv_backing_attach(BdrvChild *c); >> +static void bdrv_backing_detach(BdrvChild *c); > > This series leaves a few static forward declarations behind, and even > in the middle of the code rather than at the top. > > Does anything stop us from adding bdrv_inherited_options() after all the > old functions instead? This will require a temporary forward > declaration, too, but it can go away at the end of the series when there > is only child_of_bds left. Personally, I have nothing against forward declarations, although they probably should reside at the top, yes. I suppose I can indeed just put the whole code after all the current code (i.e., behind child_backing), though. Max
diff --git a/block.c b/block.c index 3cf1293a7b..0f24546863 100644 --- a/block.c +++ b/block.c @@ -943,6 +943,7 @@ static void bdrv_child_cb_drained_end(BdrvChild *child, } static void bdrv_backing_attach(BdrvChild *c); +static void bdrv_backing_detach(BdrvChild *c); static void bdrv_child_cb_attach(BdrvChild *child) { @@ -958,6 +959,11 @@ static void bdrv_child_cb_attach(BdrvChild *child) static void bdrv_child_cb_detach(BdrvChild *child) { BlockDriverState *bs = child->opaque; + + if (child->role & BDRV_CHILD_COW) { + bdrv_backing_detach(child); + } + bdrv_unapply_subtree_drain(child, bs); } @@ -1204,7 +1210,14 @@ static void bdrv_backing_detach(BdrvChild *c) bdrv_op_unblock_all(c->bs, parent->backing_blocker); error_free(parent->backing_blocker); parent->backing_blocker = NULL; +} +/* XXX: Will be removed along with child_backing */ +static void bdrv_child_cb_detach_backing(BdrvChild *c) +{ + if (!(c->role & BDRV_CHILD_COW)) { + bdrv_backing_detach(c); + } bdrv_child_cb_detach(c); } @@ -1252,7 +1265,7 @@ const BdrvChildClass child_backing = { .parent_is_bds = true, .get_parent_desc = bdrv_child_get_parent_desc, .attach = bdrv_child_cb_attach_backing, - .detach = bdrv_backing_detach, + .detach = bdrv_child_cb_detach_backing, .inherit_options = bdrv_backing_options, .drained_begin = bdrv_child_cb_drained_begin, .drained_poll = bdrv_child_cb_drained_poll,