diff --git a/ChangeLog b/ChangeLog index a740d6da77..2cf7e04e3c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,14 @@ +Fri Nov 28 11:21:40 GMT 2008 Daniel P. Berrange + + Fix XM driver disk parsing with no source CDROMs + * src/domain_conf.c: Translate "" into NULL for disk source + path to deal with broken apps + * src/xm_internal.c: Fix disk source parsing to work with + no-source disk definitions (eg CDROM without media) + * tests/xmconfigdata/test-no-source-cdrom.cfg, + tests/xmconfigdata/test-no-source-cdrom.xml, + tests/xmconfigtest.c: Add test case for no-src CDROM + Fri Nov 28 11:17:40 GMT 2008 Daniel P. Berrange * libvirt.spec.in: Add missing numa-ctl BuildRequires diff --git a/src/domain_conf.c b/src/domain_conf.c index 414b7ff944..4adab69975 100644 --- a/src/domain_conf.c +++ b/src/domain_conf.c @@ -546,6 +546,14 @@ virDomainDiskDefParseXML(virConnectPtr conn, 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"); diff --git a/src/xm_internal.c b/src/xm_internal.c index b98b13f125..6bdf64e4fc 100644 --- a/src/xm_internal.c +++ b/src/xm_internal.c @@ -828,7 +828,7 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { 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 conn, virConfPtr conf) { 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,31 +876,40 @@ xenXMDomainConfigParse(virConnectPtr conn, virConfPtr conf) { /* 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'; + + /* 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)); + } } - /* 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)); - } + /* 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") ? diff --git a/tests/xmconfigdata/test-no-source-cdrom.cfg b/tests/xmconfigdata/test-no-source-cdrom.cfg new file mode 100644 index 0000000000..1f200cd259 --- /dev/null +++ b/tests/xmconfigdata/test-no-source-cdrom.cfg @@ -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" diff --git a/tests/xmconfigdata/test-no-source-cdrom.xml b/tests/xmconfigdata/test-no-source-cdrom.xml new file mode 100644 index 0000000000..a28fcce013 --- /dev/null +++ b/tests/xmconfigdata/test-no-source-cdrom.xml @@ -0,0 +1,46 @@ + + test + cc2315e7-d26a-307a-438c-6d188ec4c09c + 391168 + 358400 + 1 + + hvm + /usr/lib/xen/boot/hvmloader + + + + + + + + + destroy + destroy + destroy + + /usr/lib/xen/bin/qemu-dm + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c index 0577da5ae6..7ac5afb4ec 100644 --- a/tests/xmconfigtest.c +++ b/tests/xmconfigtest.c @@ -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);