[PATCH 01/14] kvm tools: Move disk image related code under disk directory

[PATCH 01/14] kvm tools: Move disk image related code under disk directory

[Asias He]

This patch removes disk-image.c and qcow.c under disk directory.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/Makefile                |    4 ++--
 tools/kvm/{ => disk}/disk-image.c |    0
 tools/kvm/{ => disk}/qcow.c       |    0
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename tools/kvm/{ => disk}/disk-image.c (100%)
 rename tools/kvm/{ => disk}/qcow.c (100%)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 45897d5..8938f5f 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -15,7 +15,7 @@ TAGS = ctags
 OBJS	+= 8250-serial.o
 OBJS	+= cpuid.o
 OBJS	+= read-write.o
-OBJS	+= disk-image.o
+OBJS	+= disk/disk-image.o
 OBJS	+= interrupt.o
 OBJS	+= ioport.o
 OBJS	+= kvm.o
@@ -37,7 +37,7 @@ OBJS    += util/strbuf.o
 OBJS    += kvm-help.o
 OBJS    += kvm-cmd.o
 OBJS    += kvm-run.o
-OBJS    += qcow.o
+OBJS    += disk/qcow.o
 OBJS    += mptable.o
 OBJS    += threadpool.o
 OBJS    += irq.o
diff --git a/tools/kvm/disk-image.c b/tools/kvm/disk/disk-image.c
similarity index 100%
rename from tools/kvm/disk-image.c
rename to tools/kvm/disk/disk-image.c
diff --git a/tools/kvm/qcow.c b/tools/kvm/disk/qcow.c
similarity index 100%
rename from tools/kvm/qcow.c
rename to tools/kvm/disk/qcow.c
-- 
1.7.5.1

[PATCH 02/14] kvm tools: Rename disk-image.c to core.c

[Asias He]

This patch prepares the splitting of disk-image.c to core.c,
blk.c and raw.c.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/Makefile                      |    2 +-
 tools/kvm/disk/{disk-image.c => core.c} |    0
 2 files changed, 1 insertions(+), 1 deletions(-)
 rename tools/kvm/disk/{disk-image.c => core.c} (100%)

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index 8938f5f..d7b1233 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -15,7 +15,7 @@ TAGS = ctags
 OBJS	+= 8250-serial.o
 OBJS	+= cpuid.o
 OBJS	+= read-write.o
-OBJS	+= disk/disk-image.o
+OBJS	+= disk/core.o
 OBJS	+= interrupt.o
 OBJS	+= ioport.o
 OBJS	+= kvm.o
diff --git a/tools/kvm/disk/disk-image.c b/tools/kvm/disk/core.c
similarity index 100%
rename from tools/kvm/disk/disk-image.c
rename to tools/kvm/disk/core.c
-- 
1.7.5.1

[PATCH 03/14] kvm tools: Split raw image and blk device code from disk/core.c

[Asias He]

This patch moves raw image and blk device code into disk/raw.c

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/Makefile                 |    3 +-
 tools/kvm/disk/core.c              |  101 +-----------------------------------
 tools/kvm/disk/raw.c               |   84 ++++++++++++++++++++++++++++++
 tools/kvm/include/kvm/disk-image.h |   15 +++++
 4 files changed, 102 insertions(+), 101 deletions(-)
 create mode 100644 tools/kvm/disk/raw.c

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index d7b1233..a19fbe8 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -15,7 +15,6 @@ TAGS = ctags
 OBJS	+= 8250-serial.o
 OBJS	+= cpuid.o
 OBJS	+= read-write.o
-OBJS	+= disk/core.o
 OBJS	+= interrupt.o
 OBJS	+= ioport.o
 OBJS	+= kvm.o
@@ -38,6 +37,8 @@ OBJS    += kvm-help.o
 OBJS    += kvm-cmd.o
 OBJS    += kvm-run.o
 OBJS    += disk/qcow.o
+OBJS	+= disk/core.o
+OBJS    += disk/raw.o
 OBJS    += mptable.o
 OBJS    += threadpool.o
 OBJS    += irq.o
diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 4172b05..4622e99 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -1,21 +1,5 @@
 #include "kvm/disk-image.h"
-
-#include "kvm/read-write.h"
 #include "kvm/qcow.h"
-#include "kvm/util.h"
-
-#include <linux/fs.h>	/* for BLKGETSIZE64 */
-
-#include <sys/ioctl.h>
-#include <sys/types.h>
-#include <linux/types.h>
-#include <sys/mman.h>
-#include <sys/stat.h>
-#include <stdbool.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <unistd.h>
-#include <fcntl.h>
 
 struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops)
 {
@@ -45,89 +29,6 @@ struct disk_image *disk_image__new_readonly(int fd, u64 size, struct disk_image_
 	return disk;
 }
 
-static ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
-{
-	u64 offset = sector << SECTOR_SHIFT;
-
-	return preadv_in_full(disk->fd, iov, iovcount, offset);
-}
-
-static ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
-{
-	u64 offset = sector << SECTOR_SHIFT;
-
-	return pwritev_in_full(disk->fd, iov, iovcount, offset);
-}
-
-static int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
-{
-	u64 offset = sector << SECTOR_SHIFT;
-
-	if (offset + dst_len > disk->size)
-		return -1;
-
-	memcpy(dst, disk->priv + offset, dst_len);
-
-	return 0;
-}
-
-static int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
-{
-	u64 offset = sector << SECTOR_SHIFT;
-
-	if (offset + src_len > disk->size)
-		return -1;
-
-	memcpy(disk->priv + offset, src, src_len);
-
-	return 0;
-}
-
-static void raw_image__close_ro_mmap(struct disk_image *disk)
-{
-	if (disk->priv != MAP_FAILED)
-		munmap(disk->priv, disk->size);
-}
-
-static struct disk_image_operations raw_image_ops = {
-	.read_sector_iov	= raw_image__read_sector_iov,
-	.write_sector_iov	= raw_image__write_sector_iov
-};
-
-static struct disk_image_operations raw_image_ro_mmap_ops = {
-	.read_sector		= raw_image__read_sector_ro_mmap,
-	.write_sector		= raw_image__write_sector_ro_mmap,
-	.close			= raw_image__close_ro_mmap,
-};
-
-static struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
-{
-	if (readonly)
-		return disk_image__new_readonly(fd, st->st_size, &raw_image_ro_mmap_ops);
-	else
-		return disk_image__new(fd, st->st_size, &raw_image_ops);
-}
-
-static struct disk_image *blkdev__probe(const char *filename, struct stat *st)
-{
-	u64 size;
-	int fd;
-
-	if (!S_ISBLK(st->st_mode))
-		return NULL;
-
-	fd		= open(filename, O_RDONLY);
-	if (fd < 0)
-		return NULL;
-
-	if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
-		close(fd);
-		return NULL;
-	}
-
-	return disk_image__new_readonly(fd, size, &raw_image_ro_mmap_ops);
-}
-
 struct disk_image *disk_image__open(const char *filename, bool readonly)
 {
 	struct disk_image *disk;
@@ -209,4 +110,4 @@ ssize_t disk_image__write_sector_iov(struct disk_image *disk, u64 sector, const
 	}
 
 	return (sector - first_sector) << SECTOR_SHIFT;
-}
\ No newline at end of file
+}
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
new file mode 100644
index 0000000..b1a484d
--- /dev/null
+++ b/tools/kvm/disk/raw.c
@@ -0,0 +1,84 @@
+#include "kvm/disk-image.h"
+
+static ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
+{
+	u64 offset = sector << SECTOR_SHIFT;
+
+	return preadv_in_full(disk->fd, iov, iovcount, offset);
+}
+
+static ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
+{
+	u64 offset = sector << SECTOR_SHIFT;
+
+	return pwritev_in_full(disk->fd, iov, iovcount, offset);
+}
+
+static int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
+{
+	u64 offset = sector << SECTOR_SHIFT;
+
+	if (offset + dst_len > disk->size)
+		return -1;
+
+	memcpy(dst, disk->priv + offset, dst_len);
+
+	return 0;
+}
+
+static int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+{
+	u64 offset = sector << SECTOR_SHIFT;
+
+	if (offset + src_len > disk->size)
+		return -1;
+
+	memcpy(disk->priv + offset, src, src_len);
+
+	return 0;
+}
+
+static void raw_image__close_ro_mmap(struct disk_image *disk)
+{
+	if (disk->priv != MAP_FAILED)
+		munmap(disk->priv, disk->size);
+}
+
+static struct disk_image_operations raw_image_ops = {
+	.read_sector_iov	= raw_image__read_sector_iov,
+	.write_sector_iov	= raw_image__write_sector_iov
+};
+
+static struct disk_image_operations raw_image_ro_mmap_ops = {
+	.read_sector		= raw_image__read_sector_ro_mmap,
+	.write_sector		= raw_image__write_sector_ro_mmap,
+	.close			= raw_image__close_ro_mmap,
+};
+
+struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
+{
+	if (readonly)
+		return disk_image__new_readonly(fd, st->st_size, &raw_image_ro_mmap_ops);
+	else
+		return disk_image__new(fd, st->st_size, &raw_image_ops);
+}
+
+struct disk_image *blkdev__probe(const char *filename, struct stat *st)
+{
+	u64 size;
+	int fd;
+
+	if (!S_ISBLK(st->st_mode))
+		return NULL;
+
+	fd		= open(filename, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
+		close(fd);
+		return NULL;
+	}
+
+	return disk_image__new_readonly(fd, size, &raw_image_ro_mmap_ops);
+}
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index cc83f38..87bebd3 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -1,10 +1,22 @@
 #ifndef KVM__DISK_IMAGE_H
 #define KVM__DISK_IMAGE_H
 
+#include "kvm/read-write.h"
+#include "kvm/util.h"
+
 #include <linux/types.h>
+#include <linux/fs.h>	/* for BLKGETSIZE64 */
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
 #include <stdbool.h>
 #include <sys/uio.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdlib.h>
 #include <unistd.h>
+#include <fcntl.h>
 
 #define SECTOR_SHIFT		9
 #define SECTOR_SIZE		(1UL << SECTOR_SHIFT)
@@ -52,4 +64,7 @@ static inline int disk_image__flush(struct disk_image *disk)
 	return fsync(disk->fd);
 }
 
+struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly);
+struct disk_image *blkdev__probe(const char *filename, struct stat *st);
+
 #endif /* KVM__DISK_IMAGE_H */
-- 
1.7.5.1

[PATCH 04/14] kvm tools: Rename disk_image__{read, write}_sector_iov

[Asias He]

This patch renames disk_image__{read, write}_sector_io to
disk_image__{read, write}.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c              |    4 ++--
 tools/kvm/include/kvm/disk-image.h |    4 ++--
 tools/kvm/virtio/blk.c             |    4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 4622e99..3b12fff 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -75,7 +75,7 @@ void disk_image__close(struct disk_image *disk)
 }
 
 /* Fill iov with disk data, starting from sector 'sector'. Return amount of bytes read. */
-ssize_t disk_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
+ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
 {
 	u64 first_sector = sector;
 
@@ -94,7 +94,7 @@ ssize_t disk_image__read_sector_iov(struct disk_image *disk, u64 sector, const s
 }
 
 /* Write iov to disk, starting from sector 'sector'. Return amount of bytes written. */
-ssize_t disk_image__write_sector_iov(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)
 {
 	u64 first_sector = sector;
 
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 87bebd3..d43457e 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -54,8 +54,8 @@ static inline int disk_image__write_sector(struct disk_image *disk, u64 sector,
 	return disk->ops->write_sector(disk, sector, src, src_len);
 }
 
-ssize_t disk_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
-ssize_t disk_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
+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);
 
 static inline int disk_image__flush(struct disk_image *disk)
 {
diff --git a/tools/kvm/virtio/blk.c b/tools/kvm/virtio/blk.c
index c9b8a54..de88fe9 100644
--- a/tools/kvm/virtio/blk.c
+++ b/tools/kvm/virtio/blk.c
@@ -140,10 +140,10 @@ static bool virtio_blk_do_io_request(struct kvm *kvm,
 
 	switch (req->type) {
 	case VIRTIO_BLK_T_IN:
-		block_cnt	= disk_image__read_sector_iov(bdev->disk, req->sector, iov + 1, in + out - 2);
+		block_cnt	= disk_image__read(bdev->disk, req->sector, iov + 1, in + out - 2);
 		break;
 	case VIRTIO_BLK_T_OUT:
-		block_cnt	= disk_image__write_sector_iov(bdev->disk, req->sector, iov + 1, in + out - 2);
+		block_cnt	= disk_image__write(bdev->disk, req->sector, iov + 1, in + out - 2);
 		break;
 	case VIRTIO_BLK_T_FLUSH:
 		block_cnt       = disk_image__flush(bdev->disk);
-- 
1.7.5.1

[PATCH 05/14] kvm tools: Remove dead coe disk_image__{read, write}_sector

[Asias He]

These code are not used anymore. Let's remove it.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/include/kvm/disk-image.h |   10 ----------
 1 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index d43457e..e6d9e8e 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -44,16 +44,6 @@ struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operation
 struct disk_image *disk_image__new_readonly(int fd, u64 size, struct disk_image_operations *ops);
 void disk_image__close(struct disk_image *disk);
 
-static inline int disk_image__read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
-{
-	return disk->ops->read_sector(disk, sector, dst, dst_len);
-}
-
-static inline int disk_image__write_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
-{
-	return disk->ops->write_sector(disk, sector, src, src_len);
-}
-
 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);
 
-- 
1.7.5.1

[PATCH 06/14] kvm tools: Consolidate disk_image__{new, new_readonly}

[Asias He]

This patch simplifies the disk image API.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c              |   22 +++++++++-------------
 tools/kvm/disk/qcow.c              |   14 ++++++++++----
 tools/kvm/disk/raw.c               |   14 +++++++++++---
 tools/kvm/include/kvm/disk-image.h |    6 ++++--
 4 files changed, 34 insertions(+), 22 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 3b12fff..2990caa 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -1,7 +1,7 @@
 #include "kvm/disk-image.h"
 #include "kvm/qcow.h"
 
-struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops)
+struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops, int use_mmap)
 {
 	struct disk_image *disk;
 
@@ -12,20 +12,16 @@ struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operation
 	disk->fd	= fd;
 	disk->size	= size;
 	disk->ops	= ops;
-	return disk;
-}
-
-struct disk_image *disk_image__new_readonly(int fd, u64 size, struct disk_image_operations *ops)
-{
-	struct disk_image *disk;
 
-	disk = disk_image__new(fd, size, ops);
-	if (!disk)
-		return NULL;
+	if (use_mmap == DISK_IMAGE_MMAP) {
+		/*
+		 * The write to disk image will be discarded
+		 */
+		disk->priv = mmap(NULL, size, PROT_RW, MAP_PRIVATE | MAP_NORESERVE, fd, 0);
+		if (disk->priv == MAP_FAILED)
+			die("mmap() failed");
+	}
 
-	disk->priv = mmap(NULL, size, PROT_RW, MAP_PRIVATE | MAP_NORESERVE, fd, 0);
-	if (disk->priv == MAP_FAILED)
-		die("mmap() failed");
 	return disk;
 }
 
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index bb2345c..b29efb4 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -433,10 +433,13 @@ static struct disk_image *qcow2_probe(int fd, bool readonly)
 	if (qcow_read_l1_table(q) < 0)
 		goto error;
 
+	/*
+	 * Do not use mmap use read/write instead
+	 */
 	if (readonly)
-		disk_image = disk_image__new(fd, h->size, &qcow1_disk_readonly_ops);
+		disk_image = disk_image__new(fd, h->size, &qcow1_disk_readonly_ops, DISK_IMAGE_NOMMAP);
 	else
-		disk_image = disk_image__new(fd, h->size, &qcow1_disk_ops);
+		disk_image = disk_image__new(fd, h->size, &qcow1_disk_ops, DISK_IMAGE_NOMMAP);
 
 	if (!disk_image)
 		goto error;
@@ -527,10 +530,13 @@ static struct disk_image *qcow1_probe(int fd, bool readonly)
 	if (qcow_read_l1_table(q) < 0)
 		goto error;
 
+	/*
+	 * Do not use mmap use read/write instead
+	 */
 	if (readonly)
-		disk_image = disk_image__new(fd, h->size, &qcow1_disk_readonly_ops);
+		disk_image = disk_image__new(fd, h->size, &qcow1_disk_readonly_ops, DISK_IMAGE_NOMMAP);
 	else
-		disk_image = disk_image__new(fd, h->size, &qcow1_disk_ops);
+		disk_image = disk_image__new(fd, h->size, &qcow1_disk_ops, DISK_IMAGE_NOMMAP);
 
 	if (!disk_image)
 		goto error;
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index b1a484d..a419c32 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -57,10 +57,18 @@ static struct disk_image_operations raw_image_ro_mmap_ops = {
 
 struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
 {
+
 	if (readonly)
-		return disk_image__new_readonly(fd, st->st_size, &raw_image_ro_mmap_ops);
+		/*
+		 * Use mmap's MAP_PRIVATE to implement non-persistent write
+		 * FIXME: This does not work on 32-bit host.
+		 */
+		return disk_image__new(fd, st->st_size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
 	else
-		return disk_image__new(fd, st->st_size, &raw_image_ops);
+		/*
+		 * Use read/write instead of mmap
+		 */
+		return disk_image__new(fd, st->st_size, &raw_image_ops, DISK_IMAGE_NOMMAP);
 }
 
 struct disk_image *blkdev__probe(const char *filename, struct stat *st)
@@ -80,5 +88,5 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st)
 		return NULL;
 	}
 
-	return disk_image__new_readonly(fd, size, &raw_image_ro_mmap_ops);
+	return disk_image__new(fd, size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
 }
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index e6d9e8e..efa3e42 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -21,6 +21,9 @@
 #define SECTOR_SHIFT		9
 #define SECTOR_SIZE		(1UL << SECTOR_SHIFT)
 
+#define DISK_IMAGE_MMAP		0
+#define DISK_IMAGE_NOMMAP	1
+
 struct disk_image;
 
 struct disk_image_operations {
@@ -40,8 +43,7 @@ struct disk_image {
 };
 
 struct disk_image *disk_image__open(const char *filename, bool readonly);
-struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops);
-struct disk_image *disk_image__new_readonly(int fd, u64 size, struct disk_image_operations *ops);
+struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops, int mmap);
 void disk_image__close(struct disk_image *disk);
 
 ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
-- 
1.7.5.1

[PATCH 07/14] kvm tools: Split blk device code from raw.c to blk.c

[Asias He]

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/Makefile                 |    1 +
 tools/kvm/disk/blk.c               |   30 ++++++++++++++++++++++++++++++
 tools/kvm/disk/raw.c               |   32 ++++++--------------------------
 tools/kvm/include/kvm/disk-image.h |    6 ++++++
 4 files changed, 43 insertions(+), 26 deletions(-)
 create mode 100644 tools/kvm/disk/blk.c

diff --git a/tools/kvm/Makefile b/tools/kvm/Makefile
index a19fbe8..ffafcba 100644
--- a/tools/kvm/Makefile
+++ b/tools/kvm/Makefile
@@ -39,6 +39,7 @@ OBJS    += kvm-run.o
 OBJS    += disk/qcow.o
 OBJS	+= disk/core.o
 OBJS    += disk/raw.o
+OBJS    += disk/blk.o
 OBJS    += mptable.o
 OBJS    += threadpool.o
 OBJS    += irq.o
diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
new file mode 100644
index 0000000..ece4d2a
--- /dev/null
+++ b/tools/kvm/disk/blk.c
@@ -0,0 +1,30 @@
+#include "kvm/disk-image.h"
+
+/*
+ * raw image and blk dev are similar, so reuse raw image ops.
+ */
+static struct disk_image_operations raw_image_ro_mmap_ops = {
+	.read_sector		= raw_image__read_sector_ro_mmap,
+	.write_sector		= raw_image__write_sector_ro_mmap,
+	.close			= raw_image__close_ro_mmap,
+};
+
+struct disk_image *blkdev__probe(const char *filename, struct stat *st)
+{
+	u64 size;
+	int fd;
+
+	if (!S_ISBLK(st->st_mode))
+		return NULL;
+
+	fd		= open(filename, O_RDONLY);
+	if (fd < 0)
+		return NULL;
+
+	if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
+		close(fd);
+		return NULL;
+	}
+
+	return disk_image__new(fd, size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
+}
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index a419c32..8ee7e3f 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -1,20 +1,20 @@
 #include "kvm/disk-image.h"
 
-static ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
+ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
 	return preadv_in_full(disk->fd, iov, iovcount, offset);
 }
 
-static ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
+ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
 	return pwritev_in_full(disk->fd, iov, iovcount, offset);
 }
 
-static int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
+int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -26,7 +26,7 @@ static int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, v
 	return 0;
 }
 
-static int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -38,7 +38,7 @@ static int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector,
 	return 0;
 }
 
-static void raw_image__close_ro_mmap(struct disk_image *disk)
+void raw_image__close_ro_mmap(struct disk_image *disk)
 {
 	if (disk->priv != MAP_FAILED)
 		munmap(disk->priv, disk->size);
@@ -46,7 +46,7 @@ static void raw_image__close_ro_mmap(struct disk_image *disk)
 
 static struct disk_image_operations raw_image_ops = {
 	.read_sector_iov	= raw_image__read_sector_iov,
-	.write_sector_iov	= raw_image__write_sector_iov
+	.write_sector_iov	= raw_image__write_sector_iov,
 };
 
 static struct disk_image_operations raw_image_ro_mmap_ops = {
@@ -70,23 +70,3 @@ struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
 		 */
 		return disk_image__new(fd, st->st_size, &raw_image_ops, DISK_IMAGE_NOMMAP);
 }
-
-struct disk_image *blkdev__probe(const char *filename, struct stat *st)
-{
-	u64 size;
-	int fd;
-
-	if (!S_ISBLK(st->st_mode))
-		return NULL;
-
-	fd		= open(filename, O_RDONLY);
-	if (fd < 0)
-		return NULL;
-
-	if (ioctl(fd, BLKGETSIZE64, &size) < 0) {
-		close(fd);
-		return NULL;
-	}
-
-	return disk_image__new(fd, size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
-}
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index efa3e42..463801e 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -59,4 +59,10 @@ static inline int disk_image__flush(struct disk_image *disk)
 struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly);
 struct disk_image *blkdev__probe(const char *filename, struct stat *st);
 
+ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
+ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
+int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
+int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len);
+void raw_image__close_ro_mmap(struct disk_image *disk);
+
 #endif /* KVM__DISK_IMAGE_H */
-- 
1.7.5.1

[PATCH 08/14] kvm tools: Tune up ops in 'struct disk_image_operations'

[Asias He]

Make read/write ops in 'struct disk_image_operations'
always return the number of bytes read/written and close/flush
ops return int.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c              |    8 +++++---
 tools/kvm/disk/qcow.c              |   18 ++++++++++--------
 tools/kvm/disk/raw.c               |   16 ++++++++++------
 tools/kvm/include/kvm/disk-image.h |   22 +++++++++++++++-------
 4 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 2990caa..a0ccdc5 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -55,19 +55,21 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
 	return NULL;
 }
 
-void disk_image__close(struct disk_image *disk)
+int disk_image__close(struct disk_image *disk)
 {
 	/* If there was no disk image then there's nothing to do: */
 	if (!disk)
-		return;
+		return 0;
 
 	if (disk->ops->close)
-		disk->ops->close(disk);
+		return disk->ops->close(disk);
 
 	if (close(disk->fd) < 0)
 		warning("close() failed");
 
 	free(disk);
+
+	return 0;
 }
 
 /* Fill iov with disk data, starting from sector 'sector'. Return amount of bytes read. */
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index b29efb4..956775e 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -103,7 +103,7 @@ out_error:
 	goto out;
 }
 
-static int qcow1_read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
+static ssize_t qcow1_read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
 {
 	struct qcow *q = disk->priv;
 	struct qcow_header *header = q->header;
@@ -129,7 +129,7 @@ static int qcow1_read_sector(struct disk_image *disk, u64 sector, void *dst, u32
 		sector		+= (nr >> SECTOR_SHIFT);
 	}
 
-	return 0;
+	return dst_len;
 }
 
 static inline u64 file_size(int fd)
@@ -291,7 +291,7 @@ error:
 	return -1;
 }
 
-static int qcow1_write_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+static ssize_t qcow1_write_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	struct qcow *q = disk->priv;
 	struct qcow_header *header = q->header;
@@ -317,33 +317,35 @@ static int qcow1_write_sector(struct disk_image *disk, u64 sector, void *src, u3
 		offset		+= nr;
 	}
 
-	return 0;
+	return nr_written;
 }
 
-static int qcow1_nowrite_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+static ssize_t qcow1_nowrite_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	/* I/O error */
 	return -1;
 }
 
-static void qcow1_disk_close(struct disk_image *disk)
+static int qcow1_disk_close(struct disk_image *disk)
 {
 	struct qcow *q;
 
 	if (!disk)
-		return;
+		return 0;
 
 	q = disk->priv;
 
 	free(q->table.l1_table);
 	free(q->header);
 	free(q);
+
+	return 0;
 }
 
 static struct disk_image_operations qcow1_disk_readonly_ops = {
 	.read_sector		= qcow1_read_sector,
 	.write_sector		= qcow1_nowrite_sector,
-	.close			= qcow1_disk_close
+	.close			= qcow1_disk_close,
 };
 
 static struct disk_image_operations qcow1_disk_ops = {
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index 8ee7e3f..b58837b 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -14,7 +14,7 @@ ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const s
 	return pwritev_in_full(disk->fd, iov, iovcount, offset);
 }
 
-int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
+ssize_t raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -23,10 +23,10 @@ int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *ds
 
 	memcpy(dst, disk->priv + offset, dst_len);
 
-	return 0;
+	return dst_len;
 }
 
-int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+ssize_t raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -35,13 +35,17 @@ int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *s
 
 	memcpy(disk->priv + offset, src, src_len);
 
-	return 0;
+	return src_len;
 }
 
-void raw_image__close_ro_mmap(struct disk_image *disk)
+int raw_image__close_ro_mmap(struct disk_image *disk)
 {
+	int ret = 0;
+
 	if (disk->priv != MAP_FAILED)
-		munmap(disk->priv, disk->size);
+		ret = munmap(disk->priv, disk->size);
+
+	return ret;
 }
 
 static struct disk_image_operations raw_image_ops = {
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 463801e..6531a93 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -27,12 +27,20 @@
 struct disk_image;
 
 struct disk_image_operations {
-	int (*read_sector)(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
-	int (*write_sector)(struct disk_image *disk, u64 sector, void *src, u32 src_len);
+	/*
+	 * The following two are used for reading or writing with a single buffer.
+	 * The implentation can use readv/writev or memcpy.
+	 */
+	ssize_t (*read_sector)(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
+	ssize_t (*write_sector)(struct disk_image *disk, u64 sector, void *src, u32 src_len);
+	/*
+	 * The following two are used for reading or writing with multiple buffers.
+	 * The implentation can use readv/writev or memcpy.
+	 */
 	ssize_t (*read_sector_iov)(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
 	ssize_t (*write_sector_iov)(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
 	int (*flush)(struct disk_image *disk);
-	void (*close)(struct disk_image *disk);
+	int (*close)(struct disk_image *disk);
 };
 
 struct disk_image {
@@ -44,7 +52,7 @@ struct disk_image {
 
 struct disk_image *disk_image__open(const char *filename, bool readonly);
 struct disk_image *disk_image__new(int fd, u64 size, struct disk_image_operations *ops, int mmap);
-void disk_image__close(struct disk_image *disk);
+int disk_image__close(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);
@@ -61,8 +69,8 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st);
 
 ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
 ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
-int raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
-int raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len);
-void raw_image__close_ro_mmap(struct disk_image *disk);
+ssize_t raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
+ssize_t raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len);
+int raw_image__close_ro_mmap(struct disk_image *disk);
 
 #endif /* KVM__DISK_IMAGE_H */
-- 
1.7.5.1

[PATCH 09/14] kvm tools: Rename struct disk_image_operations ops name for raw image

[Asias He]

This patch renames:

raw_image__read_sector_ro_mmap to raw_image__read_sector
raw_image__write_sector_ro_mmap to raw_image__write_sector
raw_image__close_ro_mmap to raw_image__close

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/blk.c               |   10 +++++-----
 tools/kvm/disk/qcow.c              |    2 +-
 tools/kvm/disk/raw.c               |   26 ++++++++++++++++----------
 tools/kvm/include/kvm/disk-image.h |    6 +++---
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
index ece4d2a..7383967 100644
--- a/tools/kvm/disk/blk.c
+++ b/tools/kvm/disk/blk.c
@@ -3,10 +3,10 @@
 /*
  * raw image and blk dev are similar, so reuse raw image ops.
  */
-static struct disk_image_operations raw_image_ro_mmap_ops = {
-	.read_sector		= raw_image__read_sector_ro_mmap,
-	.write_sector		= raw_image__write_sector_ro_mmap,
-	.close			= raw_image__close_ro_mmap,
+static struct disk_image_operations raw_image_ops = {
+	.read_sector		= raw_image__read_sector,
+	.write_sector		= raw_image__write_sector,
+	.close			= raw_image__close,
 };
 
 struct disk_image *blkdev__probe(const char *filename, struct stat *st)
@@ -26,5 +26,5 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st)
 		return NULL;
 	}
 
-	return disk_image__new(fd, size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
+	return disk_image__new(fd, size, &raw_image_ops, DISK_IMAGE_MMAP);
 }
diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index 956775e..dd11ed97 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -351,7 +351,7 @@ static struct disk_image_operations qcow1_disk_readonly_ops = {
 static struct disk_image_operations qcow1_disk_ops = {
 	.read_sector		= qcow1_read_sector,
 	.write_sector		= qcow1_write_sector,
-	.close			= qcow1_disk_close
+	.close			= qcow1_disk_close,
 };
 
 static int qcow_read_l1_table(struct qcow *q)
diff --git a/tools/kvm/disk/raw.c b/tools/kvm/disk/raw.c
index b58837b..7f3f8db 100644
--- a/tools/kvm/disk/raw.c
+++ b/tools/kvm/disk/raw.c
@@ -14,7 +14,7 @@ ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const s
 	return pwritev_in_full(disk->fd, iov, iovcount, offset);
 }
 
-ssize_t raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
+ssize_t raw_image__read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -26,7 +26,7 @@ ssize_t raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void
 	return dst_len;
 }
 
-ssize_t raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len)
+ssize_t raw_image__write_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	u64 offset = sector << SECTOR_SHIFT;
 
@@ -38,7 +38,7 @@ ssize_t raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, voi
 	return src_len;
 }
 
-int raw_image__close_ro_mmap(struct disk_image *disk)
+int raw_image__close(struct disk_image *disk)
 {
 	int ret = 0;
 
@@ -48,15 +48,21 @@ int raw_image__close_ro_mmap(struct disk_image *disk)
 	return ret;
 }
 
-static struct disk_image_operations raw_image_ops = {
+/*
+ * multiple buffer based disk image operations
+ */
+static struct disk_image_operations raw_image_iov_ops = {
 	.read_sector_iov	= raw_image__read_sector_iov,
 	.write_sector_iov	= raw_image__write_sector_iov,
 };
 
-static struct disk_image_operations raw_image_ro_mmap_ops = {
-	.read_sector		= raw_image__read_sector_ro_mmap,
-	.write_sector		= raw_image__write_sector_ro_mmap,
-	.close			= raw_image__close_ro_mmap,
+/*
+ * single buffer based disk image operations
+ */
+static struct disk_image_operations raw_image_ops = {
+	.read_sector		= raw_image__read_sector,
+	.write_sector		= raw_image__write_sector,
+	.close			= raw_image__close,
 };
 
 struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
@@ -67,10 +73,10 @@ struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly)
 		 * Use mmap's MAP_PRIVATE to implement non-persistent write
 		 * FIXME: This does not work on 32-bit host.
 		 */
-		return disk_image__new(fd, st->st_size, &raw_image_ro_mmap_ops, DISK_IMAGE_MMAP);
+		return disk_image__new(fd, st->st_size, &raw_image_ops, DISK_IMAGE_MMAP);
 	else
 		/*
 		 * Use read/write instead of mmap
 		 */
-		return disk_image__new(fd, st->st_size, &raw_image_ops, DISK_IMAGE_NOMMAP);
+		return disk_image__new(fd, st->st_size, &raw_image_iov_ops, DISK_IMAGE_NOMMAP);
 }
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 6531a93..82884fb 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -69,8 +69,8 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st);
 
 ssize_t raw_image__read_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
 ssize_t raw_image__write_sector_iov(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount);
-ssize_t raw_image__read_sector_ro_mmap(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
-ssize_t raw_image__write_sector_ro_mmap(struct disk_image *disk, u64 sector, void *src, u32 src_len);
-int raw_image__close_ro_mmap(struct disk_image *disk);
+ssize_t raw_image__read_sector(struct disk_image *disk, u64 sector, void *dst, u32 dst_len);
+ssize_t raw_image__write_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len);
+int raw_image__close(struct disk_image *disk);
 
 #endif /* KVM__DISK_IMAGE_H */
-- 
1.7.5.1

[PATCH 10/14] kvm tools: Rename raw_image_ops to blk_dev_ops

[Asias He]

This patch also adds some comments to disk/blk.c

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/blk.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/kvm/disk/blk.c b/tools/kvm/disk/blk.c
index 7383967..59294e8 100644
--- a/tools/kvm/disk/blk.c
+++ b/tools/kvm/disk/blk.c
@@ -3,7 +3,7 @@
 /*
  * raw image and blk dev are similar, so reuse raw image ops.
  */
-static struct disk_image_operations raw_image_ops = {
+static struct disk_image_operations blk_dev_ops = {
 	.read_sector		= raw_image__read_sector,
 	.write_sector		= raw_image__write_sector,
 	.close			= raw_image__close,
@@ -17,7 +17,11 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st)
 	if (!S_ISBLK(st->st_mode))
 		return NULL;
 
-	fd		= open(filename, O_RDONLY);
+	/*
+	 * 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);
 	if (fd < 0)
 		return NULL;
 
@@ -26,5 +30,10 @@ struct disk_image *blkdev__probe(const char *filename, struct stat *st)
 		return NULL;
 	}
 
-	return disk_image__new(fd, size, &raw_image_ops, DISK_IMAGE_MMAP);
+	/*
+	 * FIXME: This will not work on 32-bit host because we can not
+	 * mmap large disk. There is not enough virtual address space
+	 * in 32-bit host. However, this works on 64-bit host.
+	 */
+	return disk_image__new(fd, size, &blk_dev_ops, DISK_IMAGE_MMAP);
 }
-- 
1.7.5.1

Re: [PATCH 10/14] kvm tools: Rename raw_image_ops to blk_dev_ops

[Ingo Molnar]

* Asias He <asias.hejun@gmail.com> wrote:

> -	fd		= open(filename, O_RDONLY);
> +	/*
> +	 * 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);
>  	if (fd < 0)
>  		return NULL;

btw., this is a repeating pattern i noticed: you align assignment vertically 
even if it's a stand-alone assignment.

We want to apply vertical alignment only when it helps readability - and 
repeated assingments such as:

        *job = (struct thread_pool__job) {
                .kvm            = kvm,
                .data           = data,
                .callback       = callback,
                .mutex          = PTHREAD_MUTEX_INITIALIZER
        };

indeed look *much* better when aligned vertically. Same goes for structure 
definitions. Thanks for applying those concepts uniformly around tools/kvm/,
it makes the code visibly more pleasant to read.

But the above standalone assignment of 'fd' does not seem to be such a case: 
the right side of the assignment just 'floats' freely in space with no other 
similar assingment next to it giving it structure.

Thus the old-fashioned:

	fd = open(filename, O_RDONLY);
 	if (fd < 0)
 		return NULL;

is a lot more readable form IMO.

There's many similar examples of isolated assignments looking weird, all around 
the code.

Thanks,

	Ingo

[PATCH 11/14] kvm tools: Remove unnecessary S_ISBLK check

[Asias He]

Let's do it in blkdev__probe.

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index a0ccdc5..6894b17 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -34,18 +34,22 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
 	if (stat(filename, &st) < 0)
 		return NULL;
 
-	if (S_ISBLK(st.st_mode))
-		return blkdev__probe(filename, &st);
+	/* blk device ?*/
+	disk		= blkdev__probe(filename, &st);
+	if (disk)
+		return disk;
 
 	fd		= open(filename, readonly ? O_RDONLY : O_RDWR);
 	if (fd < 0)
 		return NULL;
 
-	disk = qcow_probe(fd, readonly);
+	/* qcow image ?*/
+	disk		= qcow_probe(fd, readonly);
 	if (disk)
 		return disk;
 
-	disk = raw_image__probe(fd, &st, readonly);
+	/* raw image ?*/
+	disk		= raw_image__probe(fd, &st, readonly);
 	if (disk)
 		return disk;
 
-- 
1.7.5.1

[PATCH 12/14] kvm tools: Do not use 'inline' for disk_image__flush

[Asias He]

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c              |    8 ++++++++
 tools/kvm/include/kvm/disk-image.h |    9 +--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index 6894b17..f3dd0bb 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -59,6 +59,14 @@ struct disk_image *disk_image__open(const char *filename, bool readonly)
 	return NULL;
 }
 
+int disk_image__flush(struct disk_image *disk)
+{
+	if (disk->ops->flush)
+		return disk->ops->flush(disk);
+
+	return fsync(disk->fd);
+}
+
 int disk_image__close(struct disk_image *disk)
 {
 	/* If there was no disk image then there's nothing to do: */
diff --git a/tools/kvm/include/kvm/disk-image.h b/tools/kvm/include/kvm/disk-image.h
index 82884fb..ac394e8 100644
--- a/tools/kvm/include/kvm/disk-image.h
+++ b/tools/kvm/include/kvm/disk-image.h
@@ -53,17 +53,10 @@ struct disk_image {
 struct disk_image *disk_image__open(const char *filename, bool readonly);
 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);
 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);
 
-static inline int disk_image__flush(struct disk_image *disk)
-{
-	if (disk->ops->flush)
-		return disk->ops->flush(disk);
-	return fsync(disk->fd);
-}
-
 struct disk_image *raw_image__probe(int fd, struct stat *st, bool readonly);
 struct disk_image *blkdev__probe(const char *filename, struct stat *st);
 
-- 
1.7.5.1

[PATCH 13/14] kvm tools: Add debug info for disk_image__{read, write}

[Asias He]

Print debug info when read/write error occurs

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/core.c |   91 +++++++++++++++++++++++++++++++++++--------------
 1 files changed, 65 insertions(+), 26 deletions(-)

diff --git a/tools/kvm/disk/core.c b/tools/kvm/disk/core.c
index f3dd0bb..b229f83 100644
--- a/tools/kvm/disk/core.c
+++ b/tools/kvm/disk/core.c
@@ -84,40 +84,79 @@ int disk_image__close(struct disk_image *disk)
 	return 0;
 }
 
-/* Fill iov with disk data, starting from sector 'sector'. Return amount of bytes read. */
+/*
+ * Fill iov with disk data, starting from sector 'sector'.
+ * Return amount of bytes read.
+ */
 ssize_t disk_image__read(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
 {
-	u64 first_sector = sector;
+	ssize_t total = 0;
+	ssize_t nr;
 
-	if (disk->ops->read_sector_iov)
-		return disk->ops->read_sector_iov(disk, sector, iov, iovcount);
-
-	while (iovcount--) {
-		if (disk->ops->read_sector(disk, sector, iov->iov_base, iov->iov_len) < 0)
+	if (disk->ops->read_sector_iov) {
+		/*
+		 * Try mulitple buffer based operation first
+		 */
+		total		= disk->ops->read_sector_iov(disk, sector, iov, iovcount);
+		if (total < 0) {
+			info("disk_image__read error: total=%ld\n", (long)total);
 			return -1;
-
-		sector += iov->iov_len >> SECTOR_SHIFT;
-		iov++;
-	}
-
-	return (sector - first_sector) << SECTOR_SHIFT;
+		}
+	} else if (disk->ops->read_sector) {
+		/*
+		 * Fallback to single buffer based operation
+		 */
+		while (iovcount--) {
+			nr 	= disk->ops->read_sector(disk, sector, iov->iov_base, iov->iov_len);
+			if (nr != (ssize_t)iov->iov_len) {
+				info("disk_image__read error: nr = %ld iov_len=%ld\n", (long)nr, (long)iov->iov_len);
+				return -1;
+			}
+			sector	+= iov->iov_len >> SECTOR_SHIFT;
+			iov++;
+			total 	+= nr;
+		}
+	} else
+		die("No disk image operation for read\n");
+
+	return total;
 }
 
-/* Write iov to disk, starting from sector 'sector'. Return amount of bytes written. */
+/*
+ * Write iov to disk, starting from sector 'sector'.
+ * Return amount of bytes written.
+ */
 ssize_t disk_image__write(struct disk_image *disk, u64 sector, const struct iovec *iov, int iovcount)
 {
-	u64 first_sector = sector;
+	ssize_t total = 0;
+	ssize_t nr;
 
-	if (disk->ops->write_sector_iov)
-		return disk->ops->write_sector_iov(disk, sector, iov, iovcount);
-
-	while (iovcount--) {
-		if (disk->ops->write_sector(disk, sector, iov->iov_base, iov->iov_len) < 0)
+	if (disk->ops->write_sector_iov) {
+		/*
+		 * Try writev based operation first
+		 */
+		total = disk->ops->write_sector_iov(disk, sector, iov, iovcount);
+		if (total < 0) {
+			info("disk_image__write error: total=%ld\n", (long)total);
 			return -1;
-
-		sector += iov->iov_len >> SECTOR_SHIFT;
-		iov++;
-	}
-
-	return (sector - first_sector) << SECTOR_SHIFT;
+		}
+	} else if (disk->ops->write_sector) {
+		/*
+		 * Fallback to single buffer based operation
+		 */
+		while (iovcount--) {
+			nr	 = disk->ops->write_sector(disk, sector, iov->iov_base, iov->iov_len);
+			if (nr != (ssize_t)iov->iov_len) {
+				info("disk_image__write error: nr=%ld iov_len=%ld\n", (long)nr, (long)iov->iov_len);
+				return -1;
+			}
+
+			sector	+= iov->iov_len >> SECTOR_SHIFT;
+			iov++;
+			total	+= nr;
+		}
+	} else
+		die("No disk image operation for read\n");
+
+	return total;
 }
-- 
1.7.5.1

[PATCH 14/14] kvm tools: Print debug info for qcow1_nowrite_sector

[Asias He]

Print debug info when we are in qcow1_nowrite_sector

Signed-off-by: Asias He <asias.hejun@gmail.com>
---
 tools/kvm/disk/qcow.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
index dd11ed97..29964b9 100644
--- a/tools/kvm/disk/qcow.c
+++ b/tools/kvm/disk/qcow.c
@@ -323,6 +323,7 @@ static ssize_t qcow1_write_sector(struct disk_image *disk, u64 sector, void *src
 static ssize_t qcow1_nowrite_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
 {
 	/* I/O error */
+	info("qcow1_nowrite_sector: no write support\n");
 	return -1;
 }
 
-- 
1.7.5.1

Re: [PATCH 14/14] kvm tools: Print debug info for qcow1_nowrite_sector

[Ingo Molnar]

* Asias He <asias.hejun@gmail.com> wrote:

> Print debug info when we are in qcow1_nowrite_sector
> 
> Signed-off-by: Asias He <asias.hejun@gmail.com>
> ---
>  tools/kvm/disk/qcow.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
> index dd11ed97..29964b9 100644
> --- a/tools/kvm/disk/qcow.c
> +++ b/tools/kvm/disk/qcow.c
> @@ -323,6 +323,7 @@ static ssize_t qcow1_write_sector(struct disk_image *disk, u64 sector, void *src
>  static ssize_t qcow1_nowrite_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
>  {
>  	/* I/O error */
> +	info("qcow1_nowrite_sector: no write support\n");

Could we make this pr_info(), to roughly match the in-kernel equivalent?

There's also pr_warn(), pr_err(), etc.

For us poor kernel namespace infested folks :)

Thanks,

	Ingo

Re: [PATCH 14/14] kvm tools: Print debug info for qcow1_nowrite_sector

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> Could we make this pr_info(), to roughly match the in-kernel equivalent?
> 
> There's also pr_warn(), pr_err(), etc.
> 
> For us poor kernel namespace infested folks :)

btw., if it's fine with Pekka: because you are already carrying a large number 
of patches my suggestions could be done on top, no need to redo the series.

Thanks,

	Ingo

Re: [PATCH 14/14] kvm tools: Print debug info for qcow1_nowrite_sector

[Cyrill Gorcunov]

On Wed, May 18, 2011 at 12:45 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Asias He <asias.hejun@gmail.com> wrote:
>
>> Print debug info when we are in qcow1_nowrite_sector
>>
>> Signed-off-by: Asias He <asias.hejun@gmail.com>
>> ---
>>  tools/kvm/disk/qcow.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/tools/kvm/disk/qcow.c b/tools/kvm/disk/qcow.c
>> index dd11ed97..29964b9 100644
>> --- a/tools/kvm/disk/qcow.c
>> +++ b/tools/kvm/disk/qcow.c
>> @@ -323,6 +323,7 @@ static ssize_t qcow1_write_sector(struct disk_image *disk, u64 sector, void *src
>>  static ssize_t qcow1_nowrite_sector(struct disk_image *disk, u64 sector, void *src, u32 src_len)
>>  {
>>       /* I/O error */
>> +     info("qcow1_nowrite_sector: no write support\n");
>
> Could we make this pr_info(), to roughly match the in-kernel equivalent?
>
> There's also pr_warn(), pr_err(), etc.
>
> For us poor kernel namespace infested folks :)
>
> Thanks,
>
>        Ingo
>

It's my fault Ingo, I've introduced them. Will fix them at evening, ok?

Re: [PATCH 01/14] kvm tools: Move disk image related code under disk directory

[Ingo Molnar]

* Asias He <asias.hejun@gmail.com> wrote:

> This patch removes disk-image.c and qcow.c under disk directory.
> 
> Signed-off-by: Asias He <asias.hejun@gmail.com>
> ---
>  tools/kvm/Makefile                |    4 ++--
>  tools/kvm/{ => disk}/disk-image.c |    0

btw., you may want to name it 'image.c' and thus have the intuitive:

 tools/kvm/disk-image.c =>
 tools/kvm/disk/image.c 

replacement instead of tools/kvm/disk/disk-image.c which repeats 'disk' twice.

Thanks,

	Ingo

Re: [PATCH 01/14] kvm tools: Move disk image related code under disk directory

[Ingo Molnar]

* Ingo Molnar <mingo@elte.hu> wrote:

> 
> * Asias He <asias.hejun@gmail.com> wrote:
> 
> > This patch removes disk-image.c and qcow.c under disk directory.
> > 
> > Signed-off-by: Asias He <asias.hejun@gmail.com>
> > ---
> >  tools/kvm/Makefile                |    4 ++--
> >  tools/kvm/{ => disk}/disk-image.c |    0
> 
> btw., you may want to name it 'image.c' and thus have the intuitive:
> 
>  tools/kvm/disk-image.c =>
>  tools/kvm/disk/image.c 
> 
> replacement instead of tools/kvm/disk/disk-image.c which repeats 'disk' twice.

oh, stupid me: you rename it to core.c in patch #2!

I should really do what i do normally, read patch series from the end backwards 
;-)

Thanks,

	Ingo