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.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'block.c') diff --git a/block.c b/block.c index 70c6d7cf94..996778cfa0 100644 --- a/block.c +++ b/block.c @@ -720,6 +720,10 @@ static int refresh_total_sectors(BlockDriverState *bs, int64_t hint) { BlockDriver *drv = bs->drv; + if (!drv) { + return -ENOMEDIUM; + } + /* Do not attempt drv->bdrv_getlength() on scsi-generic devices */ if (bdrv_is_sg(bs)) return 0; @@ -3431,6 +3435,10 @@ int bdrv_change_backing_file(BlockDriverState *bs, BlockDriver *drv = bs->drv; int ret; + if (!drv) { + return -ENOMEDIUM; + } + /* Backing file format doesn't make sense without a backing file */ if (backing_fmt && !backing_file) { return -EINVAL; @@ -3916,7 +3924,9 @@ int bdrv_has_zero_init_1(BlockDriverState *bs) int bdrv_has_zero_init(BlockDriverState *bs) { - assert(bs->drv); + if (!bs->drv) { + return 0; + } /* If BS is a copy on write image, it is initialized to the contents of the base image, which may not be zeroes. */ @@ -4256,6 +4266,10 @@ static int bdrv_inactivate_recurse(BlockDriverState *bs, BdrvChild *child, *parent; int ret; + if (!bs->drv) { + return -ENOMEDIUM; + } + if (!setting_flag && bs->drv->bdrv_inactivate) { ret = bs->drv->bdrv_inactivate(bs); if (ret < 0) { @@ -4790,6 +4804,9 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs, int bdrv_amend_options(BlockDriverState *bs, QemuOpts *opts, BlockDriverAmendStatusCB *status_cb, void *cb_opaque) { + if (!bs->drv) { + return -ENOMEDIUM; + } if (!bs->drv->bdrv_amend_options) { return -ENOTSUP; } -- cgit v1.2.1