From d470ad42acfc73c45d3e8ed5311a491160b4c100 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 10 Nov 2017 21:31:09 +0100 Subject: 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 Buglink: https://bugs.launchpad.net/qemu/+bug/1728660 Signed-off-by: Max Reitz Message-id: 20171110203111.7666-4-mreitz@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- block/replication.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) (limited to 'block/replication.c') diff --git a/block/replication.c b/block/replication.c index 1c95d673ff..e41e293d2b 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, -- cgit v1.2.1