Fixes include:
- Error handling
- Cleanup
- Standard init/uninit
Signed-off-by: Sasha Levin <levinsasha...@gmail.com>
---
tools/kvm/builtin-run.c | 27 +++++++++---
tools/kvm/disk/blk.c | 13 ++++--
tools/kvm/disk/core.c | 76 ++++++++++++++++++++++--------------
tools/kvm/disk/qcow.c | 2 +-
tools/kvm/disk/raw.c | 9 ++--
tools/kvm/include/kvm/disk-image.h | 2 +-
tools/kvm/include/kvm/virtio-blk.h | 5 +-
tools/kvm/virtio/blk.c | 37 +++++++++++++-----
8 files changed, 111 insertions(+), 60 deletions(-)
diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index 2950ea8..0b94ef0 100644
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1052,11 +1052,13 @@ static int kvm_cmd_run_init(int argc, const char **argv)
if (image_count) {
kvm->nr_disks = image_count;
- kvm->disks = disk_image__open_all(image_filename,
readonly_image, image_count);
- if (!kvm->disks)
- die("Unable to load all disk images.");
-
- virtio_blk__init_all(kvm);
+ kvm->disks = disk_image__open_all(image_filename,
readonly_image, image_count);
+ if (IS_ERR(kvm->disks)) {
+ r = PTR_ERR(kvm->disks);
+ pr_error("disk_image__open_all() failed with error
%ld\n",
+ PTR_ERR(kvm->disks));
+ goto fail;
+ }
}
printf(" # kvm run -k %s -m %Lu -c %d --name %s\n", kernel_filename,
ram_size / 1024 / 1024, nrcpus, guest_name);
@@ -1082,6 +1084,12 @@ static int kvm_cmd_run_init(int argc, const char **argv)
goto fail;
}
+ r = virtio_blk__init(kvm);
+ if (r < 0) {
+ pr_error("virtio_blk__init() failed with error %d\n", r);
+ goto fail;
+ }
+
if (active_console == CONSOLE_VIRTIO)
virtio_console__init(kvm);
@@ -1222,10 +1230,15 @@ static void kvm_cmd_run_uninit(int guest_ret)
fb__stop();
- virtio_blk__delete_all(kvm);
+ r = virtio_blk__uninit(kvm);
+ if (r < 0)
+ pr_warning("virtio_blk__uninit() failed with error %d\n", r);
+
virtio_rng__delete_all(kvm);
- disk_image__close_all(kvm->disks, image_count);
+ r = disk_image__close_all(kvm->disks, image_count);
+ if (r < 0)
+ pr_warning("disk_image__close_all() failed with error %d\n", r);
r = serial8250__uninit(kvm);
if (r < 0)
diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
index 59294e8..72c1722 100644
--- a/tools/kvm/disk/blk.c
+++ b/tools/kvm/disk/blk.c
@@ -1,5 +1,7 @@
#include "kvm/disk-image.h"
+#include <linux/err.h>
+
/*
* raw image and blk dev are similar, so reuse raw image ops.
*/
@@ -12,22 +14,23 @@ static struct disk_image_operations blk_dev_ops = {
struct disk_image *blkdev__probe(const char *filename, struct stat *st)
{
u64 size;
- int fd;
+ int fd, r;
if (!S_ISBLK(st->st_mode))
- return NULL;
+ return ERR_PTR(-EINVAL);
/*
* Be careful! We are opening host block device!
* Open it readonly since we do not want to break user's data on disk.
*/
- fd = open(filename, O_RDONLY);
+ fd = open(filename, O_RDONLY);
if (fd < 0)
- return NULL;
+ return ERR_PTR(fd);
if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
+ r = -errno;
close(fd);
- return NULL;
+ return ERR_PTR(r);
}
/*
diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 4915efd..fb547ba 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -2,6 +2,7 @@
#include "kvm/qcow.h"
#include "kvm/virtio-blk.h"
+#include <linux/err.h>
#include <sys/eventfd.h>
#include <sys/poll.h>
@@ -31,14 +32,17 @@ static void *disk_image__thread(void *param)
struct disk_image *disk_image__new(int fd, u64 size, struct
disk_image_operations *ops, int use_mmap)
{
struct disk_image *disk;
+ int r;
- disk = malloc(sizeof *disk);
+ disk = malloc(sizeof *disk);
if (!disk)
- return NULL;
+ return ERR_PTR(-ENOMEM);
- disk->fd = fd;
- disk->size = size;
- disk->ops = ops;
+ *disk = (struct disk_image) {
+ .fd = fd,
+ .size = size,
+ .ops = ops,
+ };
if (use_mmap == DISK_IMAGE_MMAP) {
/*
@@ -46,8 +50,9 @@ struct disk_image *disk_image__new(int fd, u64 size, struct
disk_image_operation
*/
disk->priv = mmap(NULL, size, PROT_RW, MAP_PRIVATE |
MAP_NORESERVE, fd, 0);
if (disk->priv == MAP_FAILED) {
+ r = -errno;
free(disk);
- disk = NULL;
+ return ERR_PTR(r);
}
}
@@ -57,8 +62,12 @@ struct disk_image *disk_image__new(int fd, u64 size, struct
disk_image_operation
disk->evt = eventfd(0, 0);
io_setup(AIO_MAX, &disk->ctx);
- if (pthread_create(&thread, NULL, disk_image__thread, disk) !=
0)
- die("Failed starting IO thread");
+ r = pthread_create(&thread, NULL, disk_image__thread, disk);
+ if (r) {
+ r = -errno;
+ free(disk);
+ return ERR_PTR(r);
+ }
}
#endif
return disk;
@@ -71,54 +80,58 @@ struct disk_image *disk_image__open(const char *filename,
bool readonly)
int fd;
if (stat(filename, &st) < 0)
- return NULL;
+ return ERR_PTR(-errno);
/* blk device ?*/
- disk = blkdev__probe(filename, &st);
- if (disk)
+ disk = blkdev__probe(filename, &st);
+ if (!IS_ERR_OR_NULL(disk))
return disk;
- fd = open(filename, readonly ? O_RDONLY : O_RDWR);
+ fd = open(filename, readonly ? O_RDONLY : O_RDWR);
if (fd < 0)
- return NULL;
+ return ERR_PTR(fd);
/* qcow image ?*/
- disk = qcow_probe(fd, true);
- if (disk) {
+ disk = qcow_probe(fd, true);
+ if (!IS_ERR_OR_NULL(disk)) {
pr_warning("Forcing read-only support for QCOW");
return disk;
}
/* raw image ?*/
- disk = raw_image__probe(fd, &st, readonly);
- if (disk)
+ disk = raw_image__probe(fd, &st, readonly);
+ if (!IS_ERR_OR_NULL(disk))
return disk;
if (close(fd) < 0)
pr_warning("close() failed");
- return NULL;
+ return ERR_PTR(-ENOSYS);
}
struct disk_image **disk_image__open_all(const char **filenames, bool
*readonly, int count)
{
struct disk_image **disks;
int i;
+ void *err;
- if (!count || count > MAX_DISK_IMAGES)
- return NULL;
+ if (!count)
+ return ERR_PTR(-EINVAL);
+ if (count > MAX_DISK_IMAGES)
+ return ERR_PTR(-ENOSPC);
disks = calloc(count, sizeof(*disks));
if (!disks)
- return NULL;
+ return ERR_PTR(-ENOMEM);
for (i = 0; i < count; i++) {
if (!filenames[i])
continue;
disks[i] = disk_image__open(filenames[i], readonly[i]);
- if (!disks[i]) {
+ if (IS_ERR_OR_NULL(disks[i])) {
pr_error("Loading disk image '%s' failed",
filenames[i]);
+ err = disks[i];
goto error;
}
}
@@ -126,10 +139,11 @@ struct disk_image **disk_image__open_all(const char
**filenames, bool *readonly,
return disks;
error:
for (i = 0; i < count; i++)
- disk_image__close(disks[i]);
+ if (!IS_ERR_OR_NULL(disks[i]))
+ disk_image__close(disks[i]);
free(disks);
- return NULL;
+ return err;
}
int disk_image__flush(struct disk_image *disk)
@@ -157,12 +171,14 @@ int disk_image__close(struct disk_image *disk)
return 0;
}
-void disk_image__close_all(struct disk_image **disks, int count)
+int disk_image__close_all(struct disk_image **disks, int count)
{
while (count)
disk_image__close(disks[--count]);
free(disks);
+
+ return 0;
}
/*
@@ -181,7 +197,7 @@ ssize_t disk_image__read(struct disk_image *disk, u64
sector, const struct iovec
total = disk->ops->read_sector(disk, sector, iov, iovcount,
param);
if (total < 0) {
pr_info("disk_image__read error: total=%ld\n",
(long)total);
- return -1;
+ return total;
}
} else {
/* Do nothing */
@@ -213,7 +229,7 @@ ssize_t disk_image__write(struct disk_image *disk, u64
sector, const struct iove
total = disk->ops->write_sector(disk, sector, iov, iovcount,
param);
if (total < 0) {
pr_info("disk_image__write error: total=%ld\n",
(long)total);
- return -1;
+ return total;
}
} else {
/* Do nothing */
@@ -228,9 +244,11 @@ ssize_t disk_image__write(struct disk_image *disk, u64
sector, const struct iove
ssize_t disk_image__get_serial(struct disk_image *disk, void *buffer, ssize_t
*len)
{
struct stat st;
+ int r;
- if (fstat(disk->fd, &st) != 0)
- return 0;
+ r = fstat(disk->fd, &st);
+ if (r)
+ return r;
*len = snprintf(buffer, *len, "%llu%llu%llu", (u64)st.st_dev,
(u64)st.st_rdev, (u64)st.st_ino);
return *len;
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index 712f811..23f11f2 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -1334,7 +1334,7 @@ static struct disk_image *qcow2_probe(int fd, bool
readonly)
else
disk_image = disk_image__new(fd, h->size, &qcow_disk_ops,
DISK_IMAGE_REGULAR);
- if (!disk_image)
+ if (IS_ERR_OR_NULL(disk_image))
goto free_refcount_table;
disk_image->async = 0;
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index caa023c..d2df814 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -1,5 +1,7 @@
#include "kvm/disk-image.h"
+#include <linux/err.h>
+
#ifdef CONFIG_HAS_AIO
#include <libaio.h>
#endif
@@ -116,11 +118,10 @@ struct disk_image *raw_image__probe(int fd, struct stat
*st, bool readonly)
struct disk_image *disk;
disk = disk_image__new(fd, st->st_size, &ro_ops,
DISK_IMAGE_MMAP);
- if (disk == NULL) {
-
+ if (IS_ERR_OR_NULL(disk)) {
disk = disk_image__new(fd, st->st_size,
&ro_ops_nowrite, DISK_IMAGE_REGULAR);
#ifdef CONFIG_HAS_AIO
- if (disk)
+ if (!IS_ERR_OR_NULL(disk))
disk->async = 1;
#endif
}
@@ -132,7 +133,7 @@ struct disk_image *raw_image__probe(int fd, struct stat
*st, bool readonly)
*/
disk = disk_image__new(fd, st->st_size, &raw_image_regular_ops,
DISK_IMAGE_REGULAR);
#ifdef CONFIG_HAS_AIO
- if (disk)
+ if (!IS_ERR_OR_NULL(disk))
disk->async = 1;
#endif
return disk;
diff --git a/tools/kvm/include/kvm/disk-image.h
b/tools/kvm/include/kvm/disk-image.h
index 56c08da..a0b61bf 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -57,7 +57,7 @@ struct disk_image *disk_image__open(const char *filename,
bool readonly);
struct disk_image **disk_image__open_all(const char **filenames, bool
*readonly, int count);
struct disk_image *disk_image__new(int fd, u64 size, struct
disk_image_operations *ops, int mmap);
int disk_image__close(struct disk_image *disk);
-void disk_image__close_all(struct disk_image **disks, int count);
+int disk_image__close_all(struct disk_image **disks, int count);
int disk_image__flush(struct disk_image *disk);
ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct
iovec *iov,
int iovcount, void *param);
diff --git a/tools/kvm/include/kvm/virtio-blk.h
b/tools/kvm/include/kvm/virtio-blk.h
index 63731a9..e0c0919 100644
--- a/tools/kvm/include/kvm/virtio-blk.h
+++ b/tools/kvm/include/kvm/virtio-blk.h
@@ -5,9 +5,8 @@
struct kvm;
-void virtio_blk__init(struct kvm *kvm, struct disk_image *disk);
-void virtio_blk__init_all(struct kvm *kvm);
-void virtio_blk__delete_all(struct kvm *kvm);
+int virtio_blk__init(struct kvm *kvm);
+int virtio_blk__uninit(struct kvm *kvm);
void virtio_blk_complete(void *param, long len);
#endif /* KVM__BLK_VIRTIO_H */
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index d1a0197..e9b836a 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -208,17 +208,17 @@ static struct virtio_ops blk_dev_virtio_ops = (struct
virtio_ops) {
.get_size_vq = get_size_vq,
};
-void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
+static int virtio_blk__init_one(struct kvm *kvm, struct disk_image *disk)
{
struct blk_dev *bdev;
unsigned int i;
if (!disk)
- return;
+ return -EINVAL;
bdev = calloc(1, sizeof(struct blk_dev));
if (bdev == NULL)
- die("Failed allocating bdev");
+ return -ENOMEM;
*bdev = (struct blk_dev) {
.mutex = PTHREAD_MUTEX_INITIALIZER,
@@ -251,23 +251,40 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image
*disk)
"Please make sure that the
guest kernel was "
"compiled with
CONFIG_VIRTIO_BLK=y enabled "
"in its .config");
+ return 0;
}
-void virtio_blk__init_all(struct kvm *kvm)
+static int virtio_blk__uninit_one(struct kvm *kvm, struct blk_dev *bdev)
{
- int i;
+ list_del(&bdev->list);
+ free(bdev);
- for (i = 0; i < kvm->nr_disks; i++)
- virtio_blk__init(kvm, kvm->disks[i]);
+ return 0;
}
-void virtio_blk__delete_all(struct kvm *kvm)
+int virtio_blk__init(struct kvm *kvm)
+{
+ int i, r = 0;
+
+ for (i = 0; i < kvm->nr_disks; i++) {
+ r = virtio_blk__init_one(kvm, kvm->disks[i]);
+ if (r < 0)
+ goto cleanup;
+ }
+
+ return 0;
+cleanup:
+ return virtio_blk__uninit(kvm);
+}
+
+int virtio_blk__uninit(struct kvm *kvm)
{
while (!list_empty(&bdevs)) {
struct blk_dev *bdev;
bdev = list_first_entry(&bdevs, struct blk_dev, list);
- list_del(&bdev->list);
- free(bdev);
+ virtio_blk__uninit_one(kvm, bdev);
}
+
+ return 0;
}
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
