Stubdom and blktap

PATCH New Yang, Xiaowei 2 participants 4 messages Sep 28, 2008 – Oct 20, 2008

Share & tools

Yang, Xiaowei

author

2008-09-28 07:40 UTC #0 lore raw

Recently we had a try of stubdom, whose general status is good.

One issue we found is with aio/sync blktap as backend, disk performance is obviously slower, and there are some IDE kernel error messages (like irq timeout) at boot time.

Another issue (not stubdom specific) comes from blktap's support for QCOW image which is based on a backing file. We never tried this configure before. The root cause is that in tdqcow_get_parent_id(), the backing file is hard-coded to be QCOW type and tdqcow_validate_parent() also presumes it, though the backing file is a raw image normally. With the image type set correctly and a good method for image validation, QCOW works.

Thanks,
Xiaowei

Stefano Stabellini

2008-09-29 11:31 UTC #1 lore raw

Quoted 5 lines · Xiaowei

Recently we had a try of stubdom, whose general status is good.
One issue we found is with aio/sync blktap as backend, disk performance is obviously slower, and there are some IDE kernel error messages (like irq timeout) at boot time.

I'll try to reproduce the problem, thanks for reporting it.

Quoted 8 lines

Another issue (not stubdom specific) comes from blktap's support for QCOW image which is based on a backing file. We never tried this configure before. The root cause is that in tdqcow_get_parent_id(), the backing file is hard-coded to be QCOW type and tdqcow_validate_parent() also presumes it, though the backing file is a raw image normally. With the image type set correctly and a good method for image validation, QCOW works.

I am not sure to understand what the issue is exactly: there is no image format guessing using blktap, the user is supposed to specify correctly the format in the VM configuration file.

Yang, Xiaowei

author

2008-10-08 08:36 UTC #2 lore raw

Quoted 27 lines

-----Original Message-----
From: Stefano Stabellini [mailto:[email protected]]
Sent: Monday, September 29, 2008 7:32 PM
To: Yang, Xiaowei
Cc: [email protected]
Subject: Re: [Xen-devel] Stubdom and blktap
Quoted 5 lines · Xiaowei
Recently we had a try of stubdom, whose general status is good.
One issue we found is with aio/sync blktap as backend, disk performance is obviously slower, and there are some IDE kernel error messages (like irq timeout) at boot time.
I'll try to reproduce the problem, thanks for reporting it.
Quoted 8 lines
Another issue (not stubdom specific) comes from blktap's support for QCOW image which is based on a backing file. We never tried this configure before. The root cause is that in tdqcow_get_parent_id(), the backing file is hard-coded to be QCOW type and tdqcow_validate_parent() also presumes it, though the backing file is a raw image normally. With the image type set correctly and a good method for image validation, QCOW works.

Quoted 3 lines

I am not sure to understand what the issue is exactly: there is no image format guessing using blktap, the user is supposed to specify correctly the format in the VM configuration file.

Sorry for later response. I was on vacation.

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!

tools/blktap/drivers/block-qcow.c +6 -8

diff -r f4552d9f6afb tools/blktap/drivers/block-qcow.c
@@ -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)

Thanks,
Xiaowei

Stefano Stabellini

2008-10-20 12:14 UTC #3 lore raw

PATCH 1/1 Stubdom and blktap

Quoted 8 lines · Xiaowei

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 <[email protected]>

---

tools/blktap/drivers/block-qcow.c +8 ~~-10~~

diff -r 4129f0f2f2ba tools/blktap/drivers/block-qcow.c
@@ -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)

tools/blktap/drivers/block-qcow2.c +53 -9

86 lines not on this page · raw message