aboutsummaryrefslogtreecommitdiff
path: root/block
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
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')
-rw-r--r--block/io.c36
-rw-r--r--block/qapi.c8
-rw-r--r--block/replication.c15
-rw-r--r--block/vvfat.c2
4 files changed, 59 insertions, 2 deletions
diff --git a/block/io.c b/block/io.c
index 3d5ef2c..4fdf93a 100644
--- a/block/io.c
+++ b/block/io.c
@@ -853,6 +853,10 @@ static int coroutine_fn bdrv_driver_preadv(BlockDriverState *bs,
assert(!(flags & ~BDRV_REQ_MASK));
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
if (drv->bdrv_co_preadv) {
return drv->bdrv_co_preadv(bs, offset, bytes, qiov, flags);
}
@@ -894,6 +898,10 @@ static int coroutine_fn bdrv_driver_pwritev(BlockDriverState *bs,
assert(!(flags & ~BDRV_REQ_MASK));
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
if (drv->bdrv_co_pwritev) {
ret = drv->bdrv_co_pwritev(bs, offset, bytes, qiov,
flags & bs->supported_write_flags);
@@ -945,6 +953,10 @@ bdrv_driver_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
{
BlockDriver *drv = bs->drv;
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
if (!drv->bdrv_co_pwritev_compressed) {
return -ENOTSUP;
}
@@ -975,6 +987,10 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child,
BDRV_REQUEST_MAX_BYTES);
unsigned int progress = 0;
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
/* FIXME We cannot require callers to have write permissions when all they
* are doing is a read request. If we did things right, write permissions
* would be obtained anyway, but internally by the copy-on-read code. As
@@ -1291,6 +1307,10 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDriverState *bs,
bs->bl.request_alignment);
int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, MAX_BOUNCE_BUFFER);
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
assert(alignment % bs->bl.request_alignment == 0);
head = offset % alignment;
tail = (offset + bytes) % alignment;
@@ -1397,6 +1417,10 @@ static int coroutine_fn bdrv_aligned_pwritev(BdrvChild *child,
uint64_t bytes_remaining = bytes;
int max_transfer;
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+
if (bdrv_has_readonly_bitmaps(bs)) {
return -EPERM;
}
@@ -1863,6 +1887,8 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
bytes = n;
}
+ /* Must be non-NULL or bdrv_getlength() would have failed */
+ assert(bs->drv);
if (!bs->drv->bdrv_co_get_block_status) {
*pnum = bytes;
ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
@@ -2373,6 +2399,12 @@ int coroutine_fn bdrv_co_flush(BlockDriverState *bs)
}
BLKDBG_EVENT(bs->file, BLKDBG_FLUSH_TO_DISK);
+ if (!bs->drv) {
+ /* bs->drv->bdrv_co_flush() might have ejected the BDS
+ * (even in case of apparent success) */
+ ret = -ENOMEDIUM;
+ goto out;
+ }
if (bs->drv->bdrv_co_flush_to_disk) {
ret = bs->drv->bdrv_co_flush_to_disk(bs);
} else if (bs->drv->bdrv_aio_flush) {
@@ -2542,6 +2574,10 @@ int coroutine_fn bdrv_co_pdiscard(BlockDriverState *bs, int64_t offset,
num = max_pdiscard;
}
+ if (!bs->drv) {
+ ret = -ENOMEDIUM;
+ goto out;
+ }
if (bs->drv->bdrv_co_pdiscard) {
ret = bs->drv->bdrv_co_pdiscard(bs, offset, num);
} else {
diff --git a/block/qapi.c b/block/qapi.c
index 7fa2437..fc10f0a 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -39,8 +39,14 @@ BlockDeviceInfo *bdrv_block_device_info(BlockBackend *blk,
{
ImageInfo **p_image_info;
BlockDriverState *bs0;
- BlockDeviceInfo *info = g_malloc0(sizeof(*info));
+ BlockDeviceInfo *info;
+ if (!bs->drv) {
+ error_setg(errp, "Block device %s is ejected", bs->node_name);
+ return NULL;
+ }
+
+ info = g_malloc0(sizeof(*info));
info->file = g_strdup(bs->filename);
info->ro = bs->read_only;
info->drv = g_strdup(bs->drv->format_name);
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,
diff --git a/block/vvfat.c b/block/vvfat.c
index 0841cc4..a690595 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -2947,7 +2947,7 @@ static int do_commit(BDRVVVFATState* s)
return ret;
}
- if (s->qcow->bs->drv->bdrv_make_empty) {
+ if (s->qcow->bs->drv && s->qcow->bs->drv->bdrv_make_empty) {
s->qcow->bs->drv->bdrv_make_empty(s->qcow->bs);
}