[PATCH v2 1/4] kvm tools: Add a wrapper function to open disk images

[PATCH v2 1/4] kvm tools: Add a wrapper function to open disk images

[Prasad Joshi]

The patch was suggested by Ingo to move the disk image subsystem code
from the kvm-run.c file. The code to open all of the specified disk
images is now moved to a wrapper function in disk/core.c.

Suggested-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
---
 tools/kvm/disk/core.c              |   30 ++++++++++++++++++++++++++++++
 tools/kvm/include/kvm/disk-image.h |    2 ++
 tools/kvm/include/kvm/kvm.h        |    1 +
 tools/kvm/kvm-run.c                |   15 ++++++---------
 4 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 34852a2..c53944f 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -59,6 +59,36 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
 	return NULL;
 }
 
+struct disk_image **disk_image__open_all(const char **filenames, bool *readonly, int count)
+{
+	struct disk_image **disks;
+	int i;
+
+	if (count > MAX_DISK_IMAGES)
+		return NULL;
+
+	disks = malloc(sizeof(struct disk_image *) * count);
+	if (!disks)
+		return NULL;
+
+	for (i = 0; i < count; i++) {
+		if (!filenames[i]) {
+			disks[i] = NULL;
+			continue;
+		}
+
+		disks[i] = disk_image__open(filenames[i], readonly[i]);
+		if (!disks[i])
+			goto error;
+	}
+	return disks;
+error:
+	while (--i)
+		disk_image__close(disks[i]);
+
+	return NULL;
+}
+
 int disk_image__flush(struct disk_image *disk)
 {
 	if (disk->ops->flush)
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index ac394e8..bf2bfa9 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -23,6 +23,7 @@
 
 #define DISK_IMAGE_MMAP		0
 #define DISK_IMAGE_NOMMAP	1
+#define MAX_DISK_IMAGES         4
 
 struct disk_image;
 
@@ -51,6 +52,7 @@ struct disk_image {
 };
 
 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);
 int disk_image__flush(struct disk_image *disk);
diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index 3cf6e6c..f419634 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -32,6 +32,7 @@ struct kvm {
 	struct interrupt_table	interrupt_table;
 
 	const char		*vmlinux;
+	struct disk_image       **disks;
 };
 
 struct kvm *kvm__init(const char *kvm_dev, unsigned long ram_size);
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index f7de0fb..e16caac 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -44,7 +44,6 @@
 #define MB_SHIFT		(20)
 #define MIN_RAM_SIZE_MB		(64ULL)
 #define MIN_RAM_SIZE_BYTE	(MIN_RAM_SIZE_MB << MB_SHIFT)
-#define MAX_DISK_IMAGES		4
 
 static struct kvm *kvm;
 static struct kvm_cpu *kvm_cpus[KVM_NR_CPUS];
@@ -530,17 +529,15 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	if (!strstr(real_cmdline, "root="))
 		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
 
-	for (i = 0; i < image_count; i++) {
-		if (image_filename[i]) {
-			struct disk_image *disk = disk_image__open(image_filename[i], readonly_image[i]);
-			if (!disk)
-				die("unable to load disk image %s", image_filename[i]);
+	kvm->disks  = disk_image__open_all(image_filename, readonly_image, image_count);
+	if (!kvm->disks)
+		die("unable to load disk image");
 
-			virtio_blk__init(kvm, disk);
-		}
-	}
 	free(hi);
 
+	for (i = 0; i < image_count; i++)
+		virtio_blk__init(kvm, kvm->disks[i]);
+
 	printf("  # kvm run -k %s -m %Lu -c %d\n", kernel_filename, ram_size / 1024 / 1024, nrcpus);
 
 	if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,
-- 
1.7.4.1

[PATCH v2 2/4] kvm tools: Close the disk images after the guest shuts down

[Prasad Joshi]

Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
---
 tools/kvm/disk/core.c              |    8 ++++++++
 tools/kvm/include/kvm/disk-image.h |    1 +
 tools/kvm/kvm-run.c                |    1 +
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index c53944f..155c95e 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -114,6 +114,14 @@ int disk_image__close(struct disk_image *disk)
 	return 0;
 }
 
+void disk_image__close_all(struct disk_image **disks, int count)
+{
+	while (--count)
+		disk_image__close(disks[count]);
+
+	free(disks);
+}
+
 /*
  * Fill iov with disk data, starting from sector 'sector'.
  * Return amount of bytes read.
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index bf2bfa9..1363fe1 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -55,6 +55,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__flush(struct disk_image *disk);
 ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
 ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index e16caac..9bfe8fe 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -615,6 +615,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 			exit_code = 1;
 	}
 
+	disk_image__close_all(kvm->disks, image_count);
 	kvm__delete(kvm);
 
 	if (!exit_code)
-- 
1.7.4.1

[PATCH v2 3/4] kvm tools: Add a wrapper function to initialize all virtio block devices

[Prasad Joshi]

The patch moves the code for initialization of all of the virtio block
devices to virtio subsystem.

Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
---
 tools/kvm/include/kvm/kvm.h        |    1 +
 tools/kvm/include/kvm/virtio-blk.h |    1 +
 tools/kvm/kvm-run.c                |    4 ++--
 tools/kvm/virtio/blk.c             |    8 ++++++++
 4 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
index f419634..9592ce7 100644
--- a/tools/kvm/include/kvm/kvm.h
+++ b/tools/kvm/include/kvm/kvm.h
@@ -33,6 +33,7 @@ struct kvm {
 
 	const char		*vmlinux;
 	struct disk_image       **disks;
+	int                     ndisks;
 };
 
 struct kvm *kvm__init(const char *kvm_dev, unsigned long ram_size);
diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/kvm/virtio-blk.h
index 22983e8..72f6491 100644
--- a/tools/kvm/include/kvm/virtio-blk.h
+++ b/tools/kvm/include/kvm/virtio-blk.h
@@ -6,5 +6,6 @@
 struct kvm;
 
 void virtio_blk__init(struct kvm *kvm, struct disk_image *disk);
+void virtio_blk__init_all(struct kvm *kvm);
 
 #endif /* KVM__BLK_VIRTIO_H */
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index 9bfe8fe..6b0c1fd 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -529,14 +529,14 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 	if (!strstr(real_cmdline, "root="))
 		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
 
+	kvm->ndisks = image_count;
 	kvm->disks  = disk_image__open_all(image_filename, readonly_image, image_count);
 	if (!kvm->disks)
 		die("unable to load disk image");
 
 	free(hi);
 
-	for (i = 0; i < image_count; i++)
-		virtio_blk__init(kvm, kvm->disks[i]);
+	virtio_blk__init_all(kvm);
 
 	printf("  # kvm run -k %s -m %Lu -c %d\n", kernel_filename, ram_size / 1024 / 1024, nrcpus);
 
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index c5d1bb8..2ee5854 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -320,3 +320,11 @@ void virtio_blk__init(struct kvm *kvm, struct disk_image *disk)
 
 	ioport__register(blk_dev_base_addr, &virtio_blk_io_ops, IOPORT_VIRTIO_BLK_SIZE);
 }
+
+void virtio_blk__init_all(struct kvm *kvm)
+{
+	int i;
+
+	for (i = 0; i < kvm->ndisks; i++)
+		virtio_blk__init(kvm, kvm->disks[i]);
+}
-- 
1.7.4.1

[PATCH v2 4/4] kvm tools: Release memory allocated during virtio block initialization

[Prasad Joshi]

Add a new function virtio_blk__delete() goes through array of block
devices and releases memory allocated for block device.

Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
---
 tools/kvm/include/kvm/virtio-blk.h |    1 +
 tools/kvm/kvm-run.c                |    2 ++
 tools/kvm/virtio/blk.c             |    8 ++++++++
 3 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/include/kvm/virtio-blk.h b/tools/kvm/include/kvm/virtio-blk.h
index 72f6491..8c4fb91 100644
--- a/tools/kvm/include/kvm/virtio-blk.h
+++ b/tools/kvm/include/kvm/virtio-blk.h
@@ -7,5 +7,6 @@ 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);
 
 #endif /* KVM__BLK_VIRTIO_H */
diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
index 6b0c1fd..1acfa36 100644
--- a/tools/kvm/kvm-run.c
+++ b/tools/kvm/kvm-run.c
@@ -615,6 +615,8 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
 			exit_code = 1;
 	}
 
+	virtio_blk__delete_all(kvm);
+
 	disk_image__close_all(kvm->disks, image_count);
 	kvm__delete(kvm);
 
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index 2ee5854..3981cb4 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -328,3 +328,11 @@ void virtio_blk__init_all(struct kvm *kvm)
 	for (i = 0; i < kvm->ndisks; i++)
 		virtio_blk__init(kvm, kvm->disks[i]);
 }
+
+void virtio_blk__delete_all(struct kvm *kvm)
+{
+	int i;
+
+	for (i = 0; i < kvm->ndisks; i++)
+		free(bdevs[i]);
+}
-- 
1.7.4.1

Re: [PATCH v2 1/4] kvm tools: Add a wrapper function to open disk images

[Sasha Levin]

On Fri, 2011-05-20 at 16:02 +0100, Prasad Joshi wrote:
> The patch was suggested by Ingo to move the disk image subsystem code
> from the kvm-run.c file. The code to open all of the specified disk
> images is now moved to a wrapper function in disk/core.c.
> 
> Suggested-by: Ingo Molnar <mingo@elte.hu>
> Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
> ---
>  tools/kvm/disk/core.c              |   30 ++++++++++++++++++++++++++++++
>  tools/kvm/include/kvm/disk-image.h |    2 ++
>  tools/kvm/include/kvm/kvm.h        |    1 +
>  tools/kvm/kvm-run.c                |   15 ++++++---------
>  4 files changed, 39 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
> index 34852a2..c53944f 100644
> --- a/tools/kvm/disk/core.c
> +++ b/tools/kvm/disk/core.c
> @@ -59,6 +59,36 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
>  	return NULL;
>  }
>  
> +struct disk_image **disk_image__open_all(const char **filenames, bool *readonly, int count)
> +{
> +	struct disk_image **disks;
> +	int i;
> +
> +	if (count > MAX_DISK_IMAGES)
> +		return NULL;
> +
> +	disks = malloc(sizeof(struct disk_image *) * count);
			sizeof(*disks).

> +	if (!disks)
> +		return NULL;
> +
> +	for (i = 0; i < count; i++) {
> +		if (!filenames[i]) {
> +			disks[i] = NULL;
> +			continue;
> +		}
> +
> +		disks[i] = disk_image__open(filenames[i], readonly[i]);
> +		if (!disks[i])
> +			goto error;
> +	}
> +	return disks;
> +error:
> +	while (--i)
> +		disk_image__close(disks[i]);

Bad things happen here if the first image failed to open (i = -1).

> +
> +	return NULL;
> +}
> +
>  int disk_image__flush(struct disk_image *disk)
>  {
>  	if (disk->ops->flush)
> diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
> index ac394e8..bf2bfa9 100644
> --- a/tools/kvm/include/kvm/disk-image.h
> +++ b/tools/kvm/include/kvm/disk-image.h
> @@ -23,6 +23,7 @@
>  
>  #define DISK_IMAGE_MMAP		0
>  #define DISK_IMAGE_NOMMAP	1
> +#define MAX_DISK_IMAGES         4
>  
>  struct disk_image;
>  
> @@ -51,6 +52,7 @@ struct disk_image {
>  };
>  
>  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);
>  int disk_image__flush(struct disk_image *disk);
> diff --git a/tools/kvm/include/kvm/kvm.h b/tools/kvm/include/kvm/kvm.h
> index 3cf6e6c..f419634 100644
> --- a/tools/kvm/include/kvm/kvm.h
> +++ b/tools/kvm/include/kvm/kvm.h
> @@ -32,6 +32,7 @@ struct kvm {
>  	struct interrupt_table	interrupt_table;
>  
>  	const char		*vmlinux;
> +	struct disk_image       **disks;
>  };
>  
>  struct kvm *kvm__init(const char *kvm_dev, unsigned long ram_size);
> diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
> index f7de0fb..e16caac 100644
> --- a/tools/kvm/kvm-run.c
> +++ b/tools/kvm/kvm-run.c
> @@ -44,7 +44,6 @@
>  #define MB_SHIFT		(20)
>  #define MIN_RAM_SIZE_MB		(64ULL)
>  #define MIN_RAM_SIZE_BYTE	(MIN_RAM_SIZE_MB << MB_SHIFT)
> -#define MAX_DISK_IMAGES		4
>  
>  static struct kvm *kvm;
>  static struct kvm_cpu *kvm_cpus[KVM_NR_CPUS];
> @@ -530,17 +529,15 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  	if (!strstr(real_cmdline, "root="))
>  		strlcat(real_cmdline, " root=/dev/vda rw ", sizeof(real_cmdline));
>  
> -	for (i = 0; i < image_count; i++) {
> -		if (image_filename[i]) {
> -			struct disk_image *disk = disk_image__open(image_filename[i], readonly_image[i]);
> -			if (!disk)
> -				die("unable to load disk image %s", image_filename[i]);
> +	kvm->disks  = disk_image__open_all(image_filename, readonly_image, image_count);
> +	if (!kvm->disks)
> +		die("unable to load disk image");

Maybe it's worth printing also which image failed to load, If the user
passed 4 images he'll need to guess which one of the 4 failed.

>  
> -			virtio_blk__init(kvm, disk);
> -		}
> -	}
>  	free(hi);
>  
> +	for (i = 0; i < image_count; i++)
> +		virtio_blk__init(kvm, kvm->disks[i]);
> +
>  	printf("  # kvm run -k %s -m %Lu -c %d\n", kernel_filename, ram_size / 1024 / 1024, nrcpus);
>  
>  	if (!kvm__load_kernel(kvm, kernel_filename, initrd_filename,

-- 

Sasha.

Re: [PATCH v2 2/4] kvm tools: Close the disk images after the guest shuts down

[Sasha Levin]

On Fri, 2011-05-20 at 16:02 +0100, Prasad Joshi wrote:
> Signed-off-by: Prasad Joshi <prasadjoshi124@gmail.com>
> ---
>  tools/kvm/disk/core.c              |    8 ++++++++
>  tools/kvm/include/kvm/disk-image.h |    1 +
>  tools/kvm/kvm-run.c                |    1 +
>  3 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
> index c53944f..155c95e 100644
> --- a/tools/kvm/disk/core.c
> +++ b/tools/kvm/disk/core.c
> @@ -114,6 +114,14 @@ int disk_image__close(struct disk_image *disk)
>  	return 0;
>  }
>  
> +void disk_image__close_all(struct disk_image **disks, int count)
> +{

It's worth adding a small 'if (count > 0)' since this function is
currently called unconditionally.
No one assures we'll always have a default image.

> +	while (--count)
> +		disk_image__close(disks[count]);
> +
> +	free(disks);
> +}
> +
>  /*
>   * Fill iov with disk data, starting from sector 'sector'.
>   * Return amount of bytes read.
> diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
> index bf2bfa9..1363fe1 100644
> --- a/tools/kvm/include/kvm/disk-image.h
> +++ b/tools/kvm/include/kvm/disk-image.h
> @@ -55,6 +55,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__flush(struct disk_image *disk);
>  ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
>  ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
> diff --git a/tools/kvm/kvm-run.c b/tools/kvm/kvm-run.c
> index e16caac..9bfe8fe 100644
> --- a/tools/kvm/kvm-run.c
> +++ b/tools/kvm/kvm-run.c
> @@ -615,6 +615,7 @@ int kvm_cmd_run(int argc, const char **argv, const char *prefix)
>  			exit_code = 1;
>  	}
>  
> +	disk_image__close_all(kvm->disks, image_count);
>  	kvm__delete(kvm);
>  
>  	if (!exit_code)

-- 

Sasha.