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