mirror of https://github.com/proxmox/mirror_qemu
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>master
parent
a990a42b39
commit
3b78420bb1
|
@ -165,7 +165,12 @@ static void replication_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)
|
||||||
{
|
{
|
||||||
|
if (role & BDRV_CHILD_PRIMARY) {
|
||||||
*nperm = BLK_PERM_CONSISTENT_READ;
|
*nperm = BLK_PERM_CONSISTENT_READ;
|
||||||
|
} else {
|
||||||
|
*nperm = 0;
|
||||||
|
}
|
||||||
|
|
||||||
if ((bs->open_flags & (BDRV_O_INACTIVE | BDRV_O_RDWR)) == BDRV_O_RDWR) {
|
if ((bs->open_flags & (BDRV_O_INACTIVE | BDRV_O_RDWR)) == BDRV_O_RDWR) {
|
||||||
*nperm |= BLK_PERM_WRITE;
|
*nperm |= BLK_PERM_WRITE;
|
||||||
}
|
}
|
||||||
|
@ -557,8 +562,25 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
s->hidden_disk = hidden_disk;
|
bdrv_ref(hidden_disk->bs);
|
||||||
s->secondary_disk = secondary_disk;
|
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 */
|
/* start backup job now */
|
||||||
error_setg(&s->blocker,
|
error_setg(&s->blocker,
|
||||||
|
@ -664,7 +686,9 @@ static void replication_done(void *opaque, int ret)
|
||||||
if (ret == 0) {
|
if (ret == 0) {
|
||||||
s->stage = BLOCK_REPLICATION_DONE;
|
s->stage = BLOCK_REPLICATION_DONE;
|
||||||
|
|
||||||
|
bdrv_unref_child(bs, s->secondary_disk);
|
||||||
s->secondary_disk = NULL;
|
s->secondary_disk = NULL;
|
||||||
|
bdrv_unref_child(bs, s->hidden_disk);
|
||||||
s->hidden_disk = NULL;
|
s->hidden_disk = NULL;
|
||||||
s->error = 0;
|
s->error = 0;
|
||||||
} else {
|
} else {
|
||||||
|
|
Loading…
Reference in New Issue