diff options
| author | Lukas Straub <lukasstraub2@web.de> | 2021-07-18 16:48:33 +0200 |
|---|---|---|
| committer | Kevin Wolf <kwolf@redhat.com> | 2021-07-20 16:11:53 +0200 |
| commit | 3b78420bb14f7e439a079aee28eeec997a229c5e (patch) | |
| tree | 86cea7550198c3219206d9f8691a5a32080a8fd9 /block | |
| parent | a990a42b39338ffd12fb9640d792276313f75ed5 (diff) | |
replication: Properly attach children
The replication driver needs access to the children block-nodes of
it's child so it can issue bdrv_make_empty() and bdrv_co_pwritev()
to manage the replication. However, it does this by directly copying
the BdrvChilds, which is wrong.
Fix this by properly attaching the block-nodes with
bdrv_attach_child() and requesting the required permissions.
This ultimatively fixes a potential crash in replication_co_writev(),
because it may write to s->secondary_disk if it is in state
BLOCK_REPLICATION_FAILOVER_FAILED, without requesting write
permissions first. And now the workaround in
secondary_do_checkpoint() can be removed.
Signed-off-by: Lukas Straub <lukasstraub2@web.de>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <5d0539d729afb8072d0d7cde977c5066285591b4.1626619393.git.lukasstraub2@web.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'block')
| -rw-r--r-- | block/replication.c | 30 |
1 files changed, 27 insertions, 3 deletions
diff --git a/block/replication.c b/block/replication.c index 25bbdf5d4b..b74192f795 100644 --- a/block/replication.c +++ b/block/replication.c @@ -165,7 +165,12 @@ static void replication_child_perm(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared) { - *nperm = BLK_PERM_CONSISTENT_READ; + if (role & BDRV_CHILD_PRIMARY) { + *nperm = BLK_PERM_CONSISTENT_READ; + } else { + *nperm = 0; + } + if ((bs->open_flags & (BDRV_O_INACTIVE | BDRV_O_RDWR)) == BDRV_O_RDWR) { *nperm |= BLK_PERM_WRITE; } @@ -557,8 +562,25 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode, return; } - s->hidden_disk = hidden_disk; - s->secondary_disk = secondary_disk; + bdrv_ref(hidden_disk->bs); + s->hidden_disk = bdrv_attach_child(bs, hidden_disk->bs, "hidden disk", + &child_of_bds, BDRV_CHILD_DATA, + &local_err); + if (local_err) { + error_propagate(errp, local_err); + aio_context_release(aio_context); + return; + } + + bdrv_ref(secondary_disk->bs); + s->secondary_disk = bdrv_attach_child(bs, secondary_disk->bs, + "secondary disk", &child_of_bds, + BDRV_CHILD_DATA, &local_err); + if (local_err) { + error_propagate(errp, local_err); + aio_context_release(aio_context); + return; + } /* start backup job now */ error_setg(&s->blocker, @@ -664,7 +686,9 @@ static void replication_done(void *opaque, int ret) if (ret == 0) { s->stage = BLOCK_REPLICATION_DONE; + bdrv_unref_child(bs, s->secondary_disk); s->secondary_disk = NULL; + bdrv_unref_child(bs, s->hidden_disk); s->hidden_disk = NULL; s->error = 0; } else { |