block: Handle permission errors in change_parent_backing_link()
Instead of just trying to change parents by parent over to reference @to instead of @from, and abort()ing whenever the permissions don't allow this, do proper permission checking beforehand and pass any error to the callers. Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
parent
46181129ea
commit
234ac1a902
50
block.c
50
block.c
@ -2933,21 +2933,53 @@ static bool should_update_child(BdrvChild *c, BlockDriverState *to)
|
|||||||
}
|
}
|
||||||
|
|
||||||
static void change_parent_backing_link(BlockDriverState *from,
|
static void change_parent_backing_link(BlockDriverState *from,
|
||||||
BlockDriverState *to)
|
BlockDriverState *to, Error **errp)
|
||||||
{
|
{
|
||||||
BdrvChild *c, *next;
|
BdrvChild *c, *next;
|
||||||
|
GSList *list = NULL, *p;
|
||||||
|
uint64_t old_perm, old_shared;
|
||||||
|
uint64_t perm = 0, shared = BLK_PERM_ALL;
|
||||||
|
int ret;
|
||||||
|
|
||||||
|
/* Make sure that @from doesn't go away until we have successfully attached
|
||||||
|
* all of its parents to @to. */
|
||||||
|
bdrv_ref(from);
|
||||||
|
|
||||||
|
/* Put all parents into @list and calculate their cumulative permissions */
|
||||||
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
|
QLIST_FOREACH_SAFE(c, &from->parents, next_parent, next) {
|
||||||
if (!should_update_child(c, to)) {
|
if (!should_update_child(c, to)) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
list = g_slist_prepend(list, c);
|
||||||
|
perm |= c->perm;
|
||||||
|
shared &= c->shared_perm;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Check whether the required permissions can be granted on @to, ignoring
|
||||||
|
* all BdrvChild in @list so that they can't block themselves. */
|
||||||
|
ret = bdrv_check_update_perm(to, perm, shared, list, errp);
|
||||||
|
if (ret < 0) {
|
||||||
|
bdrv_abort_perm_update(to);
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Now actually perform the change. We performed the permission check for
|
||||||
|
* all elements of @list at once, so set the permissions all at once at the
|
||||||
|
* very end. */
|
||||||
|
for (p = list; p != NULL; p = p->next) {
|
||||||
|
c = p->data;
|
||||||
|
|
||||||
bdrv_ref(to);
|
bdrv_ref(to);
|
||||||
/* FIXME Are we sure that bdrv_replace_child() can't run into
|
bdrv_replace_child_noperm(c, to);
|
||||||
* &error_abort because of permissions? */
|
|
||||||
bdrv_replace_child(c, to, true);
|
|
||||||
bdrv_unref(from);
|
bdrv_unref(from);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
bdrv_get_cumulative_perm(to, &old_perm, &old_shared);
|
||||||
|
bdrv_set_perm(to, old_perm | perm, old_shared | shared);
|
||||||
|
|
||||||
|
out:
|
||||||
|
g_slist_free(list);
|
||||||
|
bdrv_unref(from);
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
@ -2980,7 +3012,12 @@ void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top,
|
|||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
change_parent_backing_link(bs_top, bs_new);
|
change_parent_backing_link(bs_top, bs_new, &local_err);
|
||||||
|
if (local_err) {
|
||||||
|
error_propagate(errp, local_err);
|
||||||
|
bdrv_set_backing_hd(bs_new, NULL, &error_abort);
|
||||||
|
goto out;
|
||||||
|
}
|
||||||
|
|
||||||
/* bs_new is now referenced by its new parents, we don't need the
|
/* bs_new is now referenced by its new parents, we don't need the
|
||||||
* additional reference any more. */
|
* additional reference any more. */
|
||||||
@ -2995,7 +3032,8 @@ void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverState *new)
|
|||||||
|
|
||||||
bdrv_ref(old);
|
bdrv_ref(old);
|
||||||
|
|
||||||
change_parent_backing_link(old, new);
|
/* FIXME Proper error handling */
|
||||||
|
change_parent_backing_link(old, new, &error_abort);
|
||||||
|
|
||||||
bdrv_unref(old);
|
bdrv_unref(old);
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user