aboutsummaryrefslogtreecommitdiff
path: root/block/replication.c
diff options
context:
space:
mode:
authorMax Reitz <mreitz@redhat.com>2017-11-10 21:31:09 +0100
committerMax Reitz <mreitz@redhat.com>2017-11-17 18:21:31 +0100
commitd470ad42acfc73c45d3e8ed5311a491160b4c100 (patch)
treecd64f938783d3c530c25f7d8c8d5de8735264fe0 /block/replication.c
parent93bbaf03ff7fd490e823814b8f5d6849a7b71a64 (diff)
downloadqemu-d470ad42acfc73c45d3e8ed5311a491160b4c100.zip
qemu-d470ad42acfc73c45d3e8ed5311a491160b4c100.tar.gz
qemu-d470ad42acfc73c45d3e8ed5311a491160b4c100.tar.bz2
block: Guard against NULL bs->drv
We currently do not guard everywhere against a NULL bs->drv where we should be doing so. Most of the places fixed here just do not care about that case at all. Some care implicitly, e.g. through a prior function call to bdrv_getlength() which would always fail for an ejected BDS. Add an assert there to make it more obvious. Other places seem to care, but do so insufficiently: Freeing clusters in a qcow2 image is an error-free operation, but it may leave the image in an unusable state anyway. Giving qcow2_free_clusters() an error code is not really viable, it is much easier to note that bs->drv may be NULL even after a successful driver call. This concerns bdrv_co_flush(), and the way the check is added to bdrv_co_pdiscard() (in every iteration instead of only once). Finally, some places employ at least an assert(bs->drv); somewhere, that may be reasonable (such as in the reopen code), but in bdrv_has_zero_init(), it is definitely not. Returning 0 there in case of an ejected BDS saves us much headache instead. Reported-by: R. Nageswara Sastry <nasastry@in.ibm.com> Buglink: https://bugs.launchpad.net/qemu/+bug/1728660 Signed-off-by: Max Reitz <mreitz@redhat.com> Message-id: 20171110203111.7666-4-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Diffstat (limited to 'block/replication.c')
-rw-r--r--block/replication.c15
1 files changed, 15 insertions, 0 deletions
diff --git a/block/replication.c b/block/replication.c
index 1c95d67..e41e293 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -342,12 +342,24 @@ static void secondary_do_checkpoint(BDRVReplicationState *s, Error **errp)
return;
}
+ if (!s->active_disk->bs->drv) {
+ error_setg(errp, "Active disk %s is ejected",
+ s->active_disk->bs->node_name);
+ return;
+ }
+
ret = s->active_disk->bs->drv->bdrv_make_empty(s->active_disk->bs);
if (ret < 0) {
error_setg(errp, "Cannot make active disk empty");
return;
}
+ if (!s->hidden_disk->bs->drv) {
+ error_setg(errp, "Hidden disk %s is ejected",
+ s->hidden_disk->bs->node_name);
+ return;
+ }
+
ret = s->hidden_disk->bs->drv->bdrv_make_empty(s->hidden_disk->bs);
if (ret < 0) {
error_setg(errp, "Cannot make hidden disk empty");
@@ -511,6 +523,9 @@ static void replication_start(ReplicationState *rs, ReplicationMode mode,
return;
}
+ /* Must be true, or the bdrv_getlength() calls would have failed */
+ assert(s->active_disk->bs->drv && s->hidden_disk->bs->drv);
+
if (!s->active_disk->bs->drv->bdrv_make_empty ||
!s->hidden_disk->bs->drv->bdrv_make_empty) {
error_setg(errp,