6:20 a.m.
It is possible for disks to be listed without a source file against
them, eg a CDROM device with no media loaded. The XenD driver handles
this, but the XM driver incorrectly generates XML with a <source file=''/>
element instead of omitting the element entirely. This causes a bogus
SXEXPR to be sent to XenD when starting the domain. This patch does
three things
- Makes the generic domain_conf.c XML parser accept XML docs with
a bogus <source file=''/> and convert the source to NULL, instead
of passing along the empty string "". This protects against broken
apps
- Makes the XM driver correctly generate XML in the first place,
so omitting the <source> tag entirely. This is the root cause fix
- Adds a test case for the XM driver to validate handling of devices
with a source file
src/domain_conf.c | 8 +++
src/xm_internal.c | 70 ++++++++++++++++------------
tests/xmconfigdata/test-no-source-cdrom.cfg | 23 +++++++++
tests/xmconfigdata/test-no-source-cdrom.xml | 46 ++++++++++++++++++
tests/xmconfigtest.c | 1
5 files changed, 120 insertions(+), 28 deletions(-)
Daniel
Index: src/xm_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/xm_internal.c,v
retrieving revision 1.102
diff -u -p -r1.102 xm_internal.c
--- src/xm_internal.c 25 Nov 2008 11:18:08 -0000 1.102
+++ src/xm_internal.c 27 Nov 2008 12:14:57 -0000
@@ -828,7 +828,7 @@ xenXMDomainConfigParse(virConnectPtr con
while (list) {
char *head;
char *offset;
- char *tmp, *tmp1;
+ char *tmp;
if ((list->type != VIR_CONF_STRING) || (list->str == NULL))
goto skipdisk;
@@ -850,10 +850,15 @@ xenXMDomainConfigParse(virConnectPtr con
goto skipdisk;
if ((offset - head) >= (PATH_MAX-1))
goto skipdisk;
- if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0)
- goto no_memory;
- strncpy(disk->src, head, (offset - head));
- disk->src[(offset-head)] = '\0';
+
+ if (offset == head) {
+ disk->src = NULL; /* No source file given, eg CDROM with no media */
+ } else {
+ if (VIR_ALLOC_N(disk->src, (offset - head) + 1) < 0)
+ goto no_memory;
+ strncpy(disk->src, head, (offset - head));
+ disk->src[(offset-head)] = '\0';
+ }
head = offset + 1;
/* Remove legacy ioemu: junk */
@@ -871,32 +876,41 @@ xenXMDomainConfigParse(virConnectPtr con
/* Extract source driver type */
- if (disk->src &&
- (tmp = strchr(disk->src, ':')) != NULL) {
- if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) < 0)
- goto no_memory;
- strncpy(disk->driverName, disk->src, (tmp - disk->src));
- disk->driverName[tmp - disk->src] = '\0';
- } else {
- if (!(disk->driverName = strdup("phy")))
- goto no_memory;
- tmp = disk->src;
- }
+ if (disk->src) {
+ /* The main type phy:, file:, tap: ... */
+ if ((tmp = strchr(disk->src, ':')) != NULL) {
+ if (VIR_ALLOC_N(disk->driverName, (tmp - disk->src) + 1) <
0)
+ goto no_memory;
+ strncpy(disk->driverName, disk->src, (tmp - disk->src));
+ disk->driverName[tmp - disk->src] = '\0';
- /* And the source driver sub-type */
- if (STRPREFIX(disk->driverName, "tap")) {
- if (!(tmp1 = strchr(tmp+1, ':')) || !tmp1[0])
- goto skipdisk;
- if (VIR_ALLOC_N(disk->driverType, (tmp1-(tmp+1))) < 0)
- goto no_memory;
- strncpy(disk->driverType, tmp+1, (tmp1-(tmp+1)));
- memmove(disk->src, disk->src+(tmp1-disk->src)+1,
strlen(disk->src)-(tmp1-disk->src));
- } else {
- disk->driverType = NULL;
- if (disk->src[0] && tmp)
- memmove(disk->src, disk->src+(tmp-disk->src)+1,
strlen(disk->src)-(tmp-disk->src));
+ /* Strip the prefix we found off the source file name */
+ memmove(disk->src, disk->src+(tmp-disk->src)+1,
+ strlen(disk->src)-(tmp-disk->src));
+ }
+
+ /* And the sub-type for tap:XXX: type */
+ if (disk->driverName &&
+ STREQ(disk->driverName, "tap")) {
+ if (!(tmp = strchr(disk->src, ':')))
+ goto skipdisk;
+ if (VIR_ALLOC_N(disk->driverType, (tmp - disk->src) + 1) <
0)
+ goto no_memory;
+ strncpy(disk->driverType, disk->src, (tmp - disk->src));
+ disk->driverType[tmp - disk->src] = '\0';
+
+ /* Strip the prefix we found off the source file name */
+ memmove(disk->src, disk->src+(tmp-disk->src)+1,
+ strlen(disk->src)-(tmp-disk->src));
+ }
}
+ /* No source, or driver name, so fix to phy: */
+ if (!disk->driverName &&
+ !(disk->driverName = strdup("phy")))
+ goto no_memory;
+
+
/* phy: type indicates a block device */
disk->type = STREQ(disk->driverName, "phy") ?
VIR_DOMAIN_DISK_TYPE_BLOCK : VIR_DOMAIN_DISK_TYPE_FILE;
Index: src/domain_conf.c
===================================================================
RCS file: /data/cvs/libvirt/src/domain_conf.c,v
retrieving revision 1.39
diff -u -p -r1.39 domain_conf.c
--- src/domain_conf.c 21 Nov 2008 11:42:51 -0000 1.39
+++ src/domain_conf.c 27 Nov 2008 12:15:06 -0000
@@ -546,6 +546,14 @@ virDomainDiskDefParseXML(virConnectPtr c
source = virXMLPropString(cur, "file");
else
source = virXMLPropString(cur, "dev");
+
+ /* People sometimes pass a bogus '' source path
+ when they mean to omit the source element
+ completely. eg CDROM without media. This is
+ just a little compatability check to help
+ those broken apps */
+ if (source && STREQ(source, ""))
+ VIR_FREE(source);
} else if ((target == NULL) &&
(xmlStrEqual(cur->name, BAD_CAST "target"))) {
target = virXMLPropString(cur, "dev");
Index: tests/xmconfigtest.c
===================================================================
RCS file: /data/cvs/libvirt/tests/xmconfigtest.c,v
retrieving revision 1.24
diff -u -p -r1.24 xmconfigtest.c
--- tests/xmconfigtest.c 24 Nov 2008 19:23:39 -0000 1.24
+++ tests/xmconfigtest.c 27 Nov 2008 12:15:13 -0000
@@ -229,6 +229,7 @@ mymain(int argc, char **argv)
DO_TEST("fullvirt-sound", 2);
DO_TEST("escape-paths", 2);
+ DO_TEST("no-source-cdrom", 2);
virCapabilitiesFree(caps);
Index: tests/xmconfigdata/test-no-source-cdrom.cfg
===================================================================
RCS file: tests/xmconfigdata/test-no-source-cdrom.cfg
diff -N tests/xmconfigdata/test-no-source-cdrom.cfg
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-no-source-cdrom.cfg 27 Nov 2008 12:15:27 -0000
@@ -0,0 +1,23 @@
+name = "test"
+uuid = "cc2315e7-d26a-307a-438c-6d188ec4c09c"
+maxmem = 382
+memory = 350
+vcpus = 1
+builder = "hvm"
+kernel = "/usr/lib/xen/boot/hvmloader"
+boot = "c"
+pae = 1
+acpi = 1
+apic = 1
+localtime = 0
+on_poweroff = "destroy"
+on_reboot = "destroy"
+on_crash = "destroy"
+device_model = "/usr/lib/xen/bin/qemu-dm"
+sdl = 0
+vnc = 1
+vncunused = 1
+disk = [ "phy:/dev/sda8,hda,w", ",hdc:cdrom,r" ]
+vif = [ "mac=00:16:3e:0a:7b:39,bridge=xenbr0,type=ioemu" ]
+parallel = "none"
+serial = "pty"
Index: tests/xmconfigdata/test-no-source-cdrom.xml
===================================================================
RCS file: tests/xmconfigdata/test-no-source-cdrom.xml
diff -N tests/xmconfigdata/test-no-source-cdrom.xml
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/xmconfigdata/test-no-source-cdrom.xml 27 Nov 2008 12:15:27 -0000
@@ -0,0 +1,46 @@
+<domain type='xen'>
+ <name>test</name>
+ <uuid>cc2315e7-d26a-307a-438c-6d188ec4c09c</uuid>
+ <memory>391168</memory>
+ <currentMemory>358400</currentMemory>
+ <vcpu>1</vcpu>
+ <os>
+ <type arch='i686' machine='xenfv'>hvm</type>
+ <loader>/usr/lib/xen/boot/hvmloader</loader>
+ <boot dev='hd'/>
+ </os>
+ <features>
+ <acpi/>
+ <apic/>
+ <pae/>
+ </features>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>destroy</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+ <disk type='block' device='disk'>
+ <driver name='phy'/>
+ <source dev='/dev/sda8'/>
+ <target dev='hda' bus='ide'/>
+ </disk>
+ <disk type='block' device='cdrom'>
+ <driver name='phy'/>
+ <target dev='hdc' bus='ide'/>
+ <readonly/>
+ </disk>
+ <interface type='bridge'>
+ <mac address='00:16:3e:0a:7b:39'/>
+ <source bridge='xenbr0'/>
+ </interface>
+ <serial type='pty'>
+ <target port='0'/>
+ </serial>
+ <console type='pty'>
+ <target port='0'/>
+ </console>
+ <input type='mouse' bus='ps2'/>
+ <graphics type='vnc' port='-1' autoport='yes'/>
+ </devices>
+</domain>
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|