block/mirror: Fix child permissions

We cannot use bdrv_child_try_set_perm() to give up all restrictions on
the child edge, and still have bdrv_mirror_top_child_perm() request
BLK_PERM_WRITE.  Fix this by making bdrv_mirror_top_child_perm() return
0/BLK_PERM_ALL when we want to give up all permissions, and replacing
bdrv_child_try_set_perm() by bdrv_child_refresh_perms().

The bdrv_child_try_set_perm() before removing the node with
bdrv_replace_node() is then unnecessary.  No permissions have changed
since the previous invocation of bdrv_child_try_set_perm().

Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This commit is contained in:
Max Reitz 2019-05-22 19:03:47 +02:00 committed by Kevin Wolf
parent c1087f1206
commit f94dc3b414

View file

@ -85,6 +85,7 @@ typedef struct MirrorBlockJob {
typedef struct MirrorBDSOpaque { typedef struct MirrorBDSOpaque {
MirrorBlockJob *job; MirrorBlockJob *job;
bool stop;
} MirrorBDSOpaque; } MirrorBDSOpaque;
struct MirrorOp { struct MirrorOp {
@ -656,8 +657,9 @@ static int mirror_exit_common(Job *job)
/* We don't access the source any more. Dropping any WRITE/RESIZE is /* We don't access the source any more. Dropping any WRITE/RESIZE is
* required before it could become a backing file of target_bs. */ * required before it could become a backing file of target_bs. */
bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, bs_opaque->stop = true;
&error_abort); bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
&error_abort);
if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) { if (!abort && s->backing_mode == MIRROR_SOURCE_BACKING_CHAIN) {
BlockDriverState *backing = s->is_none_mode ? src : s->base; BlockDriverState *backing = s->is_none_mode ? src : s->base;
if (backing_bs(target_bs) != backing) { if (backing_bs(target_bs) != backing) {
@ -704,13 +706,12 @@ static int mirror_exit_common(Job *job)
g_free(s->replaces); g_free(s->replaces);
bdrv_unref(target_bs); bdrv_unref(target_bs);
/* Remove the mirror filter driver from the graph. Before this, get rid of /*
* Remove the mirror filter driver from the graph. Before this, get rid of
* the blockers on the intermediate nodes so that the resulting state is * the blockers on the intermediate nodes so that the resulting state is
* valid. Also give up permissions on mirror_top_bs->backing, which might * valid.
* block the removal. */ */
block_job_remove_all_bdrv(bjob); block_job_remove_all_bdrv(bjob);
bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL,
&error_abort);
bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
/* We just changed the BDS the job BB refers to (with either or both of the /* We just changed the BDS the job BB refers to (with either or both of the
@ -1459,6 +1460,18 @@ static void bdrv_mirror_top_child_perm(BlockDriverState *bs, BdrvChild *c,
uint64_t perm, uint64_t shared, uint64_t perm, uint64_t shared,
uint64_t *nperm, uint64_t *nshared) uint64_t *nperm, uint64_t *nshared)
{ {
MirrorBDSOpaque *s = bs->opaque;
if (s->stop) {
/*
* If the job is to be stopped, we do not need to forward
* anything to the real image.
*/
*nperm = 0;
*nshared = BLK_PERM_ALL;
return;
}
/* Must be able to forward guest writes to the real image */ /* Must be able to forward guest writes to the real image */
*nperm = 0; *nperm = 0;
if (perm & BLK_PERM_WRITE) { if (perm & BLK_PERM_WRITE) {
@ -1681,8 +1694,9 @@ fail:
job_early_fail(&s->common.job); job_early_fail(&s->common.job);
} }
bdrv_child_try_set_perm(mirror_top_bs->backing, 0, BLK_PERM_ALL, bs_opaque->stop = true;
&error_abort); bdrv_child_refresh_perms(mirror_top_bs, mirror_top_bs->backing,
&error_abort);
bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort); bdrv_replace_node(mirror_top_bs, backing_bs(mirror_top_bs), &error_abort);
bdrv_unref(mirror_top_bs); bdrv_unref(mirror_top_bs);