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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Yang, Xiaowei wrote:> 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.> 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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
>-----Original Message----- >From: Stefano Stabellini [mailto:stefano.stabellini@eu.citrix.com] >Sent: Monday, September 29, 2008 7:32 PM >To: Yang, Xiaowei >Cc: xen-devel@lists.xensource.com >Subject: Re: [Xen-devel] Stubdom and blktap > >Yang, Xiaowei wrote: > >> 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. > >> 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.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! diff -r f4552d9f6afb tools/blktap/drivers/block-qcow.c --- a/tools/blktap/drivers/block-qcow.c Tue Sep 23 17:11:33 2008 +0100 +++ b/tools/blktap/drivers/block-qcow.c Wed Oct 08 05:45:29 2008 +0800 @@ -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 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
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; } _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Seemingly Similar Threads
- [PATCH 00 of 18] [v2] tools: fix bugs and build errors triggered by -O2 -Wall -Werror
- [PATCH 0 of 8] blktap3/libvhd: Introduce VHD library.
- classification with nnet: handling unequal class sizes
- R igraph clusters component
- [PATCHv11 0/6] libvirt/qemu - persistent modification of devices