[PATCH] Stubdom and blktap

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

Yang, Xiaowei wrote:

> Typically a QCOW image only presents changes made to a backing image.
> When a QCOW image is specified to be used in VM configuration file,
> blktap uses QCOW driver for it indeed. However, blktap also needs to
> access the backing file and before that it needs to choose a driver for
> it. As I said, the backing file is a raw one normally, and Qemu assumes it.
> 
> Here's a possibly simple (though with less flexibility) patch to it. Any
> comments are welcome!




You are right about the backing file driver issue in blktap and I think
your patch is correct too.
I am sending a new patch to include an equivalent fix for
block-qcow2 too and also to disable O_DIRECT on block-qcow since our
reads and writes are not always aligned to 512 mb.

It has to be said that this patch still doesn't solve all the qcow and
stubdoms related issues found in the last weekly status report.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

---

diff -r 4129f0f2f2ba tools/blktap/drivers/block-qcow.c
--- a/tools/blktap/drivers/block-qcow.c	Fri Oct 17 14:15:37 2008 +0100
+++ b/tools/blktap/drivers/block-qcow.c	Mon Oct 20 13:03:02 2008 +0100
@@ -734,8 +734,8 @@ static int tdqcow_open (struct disk_driv
 
  	DPRINTF("QCOW: Opening %s\n",name);
 
-	o_flags = O_DIRECT | O_LARGEFILE | 
-		((flags == TD_RDONLY) ? O_RDONLY : O_RDWR);
+	/* Since we don't handle O_DIRECT correctly, don't use it */
+	o_flags = O_LARGEFILE | ((flags == TD_RDONLY) ? O_RDONLY : O_RDWR);
 	fd = open(name, o_flags);
 	if (fd < 0) {
 		DPRINTF("Unable to open %s (%d)\n",name,0 - errno);
@@ -1385,7 +1385,7 @@ static int tdqcow_get_parent_id(struct d
 	filename[len]  = '\0';
 
 	id->name       = strdup(filename);
-	id->drivertype = DISK_TYPE_QCOW;
+	id->drivertype = DISK_TYPE_AIO;
 	err            = 0;
  out:
 	free(buf);
@@ -1397,17 +1397,15 @@ static int tdqcow_validate_parent(struct
 {
 	struct stat stats;
 	uint64_t psize, csize;
-	struct tdqcow_state *c = (struct tdqcow_state *)child->private;
-	struct tdqcow_state *p = (struct tdqcow_state *)parent->private;
-	
-	if (stat(p->name, &stats))
+	
+	if (stat(parent->name, &stats))
 		return -EINVAL;
-	if (get_filesize(p->name, &psize, &stats))
+	if (get_filesize(parent->name, &psize, &stats))
 		return -EINVAL;
 
-	if (stat(c->name, &stats))
+	if (stat(child->name, &stats))
 		return -EINVAL;
-	if (get_filesize(c->name, &csize, &stats))
+	if (get_filesize(child->name, &csize, &stats))
 		return -EINVAL;
 
 	if (csize != psize)
diff -r 4129f0f2f2ba tools/blktap/drivers/block-qcow2.c
--- a/tools/blktap/drivers/block-qcow2.c	Fri Oct 17 14:15:37 2008 +0100
+++ b/tools/blktap/drivers/block-qcow2.c	Mon Oct 20 13:00:18 2008 +0100
@@ -34,6 +34,7 @@
 #include "tapdisk.h"
 #include "tapaio.h"
 #include "bswap.h"
+#include "blk.h"
 
 #define USE_AIO
 
@@ -1902,6 +1903,42 @@ repeat:
 
 #endif	
 
+static int get_filesize(char *filename, uint64_t *size, struct stat *st)
+{
+	int fd;
+	QCowHeader header;
+
+	/*Set to the backing file size*/
+	fd = open(filename, O_RDONLY);
+	if (fd < 0)
+		return -1;
+	if (read(fd, &header, sizeof(header)) < sizeof(header)) {
+		close(fd);
+		return -1;
+	}
+	close(fd);
+	
+	be32_to_cpus(&header.magic);
+	be32_to_cpus(&header.version);
+	be64_to_cpus(&header.size);
+	if (header.magic == QCOW_MAGIC && header.version == QCOW_VERSION) {
+		*size = header.size >> SECTOR_SHIFT;
+		return 0;
+	}
+
+	if(S_ISBLK(st->st_mode)) {
+		fd = open(filename, O_RDONLY);
+		if (fd < 0)
+			return -1;
+		if (blk_getimagesize(fd, size) != 0) {
+			close(fd);
+			return -1;
+		}
+		close(fd);
+	} else *size = (st->st_size >> SECTOR_SHIFT);	
+	return 0;
+}
+
 /**
  * @return 
  *	   0 if parent id successfully retrieved;
@@ -1916,7 +1953,7 @@ static int qcow_get_parent_id(struct dis
 		return TD_NO_PARENT;
 
 	id->name = strdup(s->backing_file);
-	id->drivertype = DISK_TYPE_QCOW2;
+	id->drivertype = DISK_TYPE_AIO;
 
 	return 0;
 }
@@ -1924,15 +1961,22 @@ static int qcow_validate_parent(struct d
 static int qcow_validate_parent(struct disk_driver *child, 
 		struct disk_driver *parent, td_flag_t flags)
 {
-	struct BDRVQcowState *cs = (struct BDRVQcowState*) child->private;
-	struct BDRVQcowState *ps = (struct BDRVQcowState*) parent->private;
-
-	if (ps->total_sectors != cs->total_sectors) {
-		DPRINTF("qcow_validate_parent(): %#"PRIx64" != %#"PRIx64"\n",
-			ps->total_sectors, cs->total_sectors);
+	struct stat stats;
+	uint64_t psize, csize;
+	
+	if (stat(parent->name, &stats))
 		return -EINVAL;
-	}
-	
+	if (get_filesize(parent->name, &psize, &stats))
+		return -EINVAL;
+
+	if (stat(child->name, &stats))
+		return -EINVAL;
+	if (get_filesize(child->name, &csize, &stats))
+		return -EINVAL;
+
+	if (csize != psize)
+		return -EINVAL;
+
 	return 0;
 }