kvm tools: Fixes for disk image module

Fixes include:
 - Error handling
 - Cleanup
 - Standard init/uninit

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>

diff --git a/tools/kvm/builtin-run.c b/tools/kvm/builtin-run.c
index d18efceb4b60e92cd11488f21f973ade0b16ad67..a3fed41a9e5bfcb5b212581fb800cc947c50348e 100644 (file)
--- a/tools/kvm/builtin-run.c
+++ b/tools/kvm/builtin-run.c
@@ -1094,11 +1094,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_err("disk_image__open_all() failed with error %ld\n",
+                                       PTR_ERR(kvm->disks));
+                       goto fail;
+               }
        }
 
        printf("  # %s run -k %s -m %Lu -c %d --name %s\n", KVM_BINARY_NAME,
@@ -1129,6 +1131,12 @@ static int kvm_cmd_run_init(int argc, const char **argv)
                goto fail;
        }
 
+       r = virtio_blk__init(kvm);
+       if (r < 0) {
+               pr_err("virtio_blk__init() failed with error %d\n", r);
+               goto fail;
+       }
+
        if (active_console == CONSOLE_VIRTIO)
                virtio_console__init(kvm);
 
@@ -1272,10 +1280,15 @@ static void kvm_cmd_run_exit(int guest_ret)
 
        fb__stop();
 
-       virtio_blk__delete_all(kvm);
+       r = virtio_blk__exit(kvm);
+       if (r < 0)
+               pr_warning("virtio_blk__exit() 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);
        free(kvm_cpus);
 
        r = serial8250__exit(kvm);

diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
index 59294e841dc168f52051dacbfa33490ee664d383..72c17225538ed1a1bb4f973b669b7da62e6521b6 100644 (file)
--- 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 3da0efea779d7b3961c78bf4fe8e5d5b998f1449..ed338e731e1c7d88ad357c8beed5eb06eeaacc87 100644 (file)
--- 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_err("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 712f81100cd61d0e4862bae00f54fd7758270b39..23f11f27dbcf060314253c70d55c337ecf776b2f 100644 (file)
--- 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 caa023c31998d2697ac81d5acc361e49d86499a2..d2df814e64215c579389549e6c0dcfb640772e95 100644 (file)
--- 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 56c08da80b485f8ddd4695dca3cee019ccc7c509..a0b61bf76b99948aed5391aa3b70455cb3a5edb8 100644 (file)
--- 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 63731a9bac7a2cdd29c6914a0bf8c72e32ddcbbc..12e59b6b21fa54b7129066ec46f5c8456468d837 100644 (file)
--- 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__exit(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 1d921a80e6d151487ef75262923e90a8e0f67bcb..c230dd463475a3dbecbd28859a0b5c0094377f7b 100644 (file)
--- 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__exit_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__exit(kvm);
+}
+
+int virtio_blk__exit(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__exit_one(kvm, bdev);
        }
+
+       return 0;
 }