Fix XM driver handling of disk source paths

diff --git a/ChangeLog b/ChangeLog
index a740d6da77b26db4caaf6d3c12979606c0ef77b7..2cf7e04e3ca85c0c3d7d6fb5ee5bbb1c394b3bdc 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+Fri Nov 28 11:21:40 GMT 2008 Daniel P. Berrange <berrange@redhat.com>
+
+       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 <berrange@redhat.com>
 
        * libvirt.spec.in: Add missing numa-ctl BuildRequires

diff --git a/src/domain_conf.c b/src/domain_conf.c
index 414b7ff944c6d5db7b021d31491eefc0906a1f28..4adab699756a9296271ae06080a03e13c0fa8a70 100644 (file)
--- 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 b98b13f125c9c150cab967efa7122a5873e27294..6bdf64e4fcd48a4c253fc5f25428acda2fee3d82 100644 (file)
--- 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,32 +876,41 @@ 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';
 
-            /* 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;

diff --git a/tests/xmconfigdata/test-no-source-cdrom.cfg b/tests/xmconfigdata/test-no-source-cdrom.cfg
new file mode 100644 (file)
index 0000000..1f200cd
--- /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 (file)
index 0000000..a28fcce
--- /dev/null
+++ b/tests/xmconfigdata/test-no-source-cdrom.xml
@@ -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>

diff --git a/tests/xmconfigtest.c b/tests/xmconfigtest.c
index 0577da5ae6b0dffffd935b0711761925e62dc8d8..7ac5afb4eca9b47af2998faa7cdeee36c04ce88c 100644 (file)
--- 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);