12.07.2021 14:54, Lukas Straub wrote:
Assert that the children are writable where it's needed.
While there is no test-case for the
BLOCK_REPLICATION_FAILOVER_FAILED state, this at least ensures that
s->secondary_disk is always writable in case replication might go
into that state.
Signed-off-by: Lukas Straub <lukasstra...@web.de>
---
block/replication.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/block/replication.c b/block/replication.c
index b74192f795..772bb63374 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -261,6 +261,13 @@ static coroutine_fn int
replication_co_writev(BlockDriverState *bs,
int64_t n;
assert(!flags);
+ assert(top->perm & BLK_PERM_WRITE);
+ if (s->mode == REPLICATION_MODE_SECONDARY &&
+ s->stage != BLOCK_REPLICATION_NONE &&
+ s->stage != BLOCK_REPLICATION_DONE) {
+ assert(base->perm & BLK_PERM_WRITE);
+ }
+
write has assertions in generic code so actually we don't need this.
Also using this additional conditions is not obvious. Better is assert about
base without extra conditiions exactly before while loop.
ret = replication_get_io_status(s);
if (ret < 0) {
goto out;
@@ -318,6 +325,9 @@ static void secondary_do_checkpoint(BlockDriverState *bs,
Error **errp)
Error *local_err = NULL;
int ret;
+ assert(active_disk->perm & BLK_PERM_WRITE);
+ assert(s->hidden_disk->perm & BLK_PERM_WRITE);
Oops, bdrv_make_empty also has this assertion inside. It also is satisfied by
BLK_PERM_WRITE_UNCHANGED, but we don't work with it here anyway. So we don't
need it.
+
if (!s->backup_job) {
error_setg(errp, "Backup job was cancelled unexpectedly");
return;
--
2.20.1
Sorry, seems my suggestion to add assertions was bad, as we already have them
in generic code.
--
Best regards,
Vladimir
