* src/xend_internal.c src/xml.c: applied patch from Nobuhiro Itou

  to handle CDRom devices with no device name
* tests/sexpr2xmltest.c tests/xml2sexprtest.c
  tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr
  tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml
  tests/xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr
  tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml: added regression
  tests for this case based on Nobuhiro Itou test inputs.
Daniel

diff --git a/ChangeLog b/ChangeLog
index b423a63d0196fe4f30aac839eb28a1a2c2a453e3..c6ae12cb30c08f1e84d2651d95a90999ba353ea1 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+Wed Apr 11 18:04:00 CEST 2007 Daniel Veillard <veillard@redhat.com>
+
+       * src/xend_internal.c src/xml.c: applied patch from Nobuhiro Itou
+         to handle CDRom devices with no device name
+       * tests/sexpr2xmltest.c tests/xml2sexprtest.c
+         tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr
+         tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml
+         tests/xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr
+         tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml: added regression
+         tests for this case based on Nobuhiro Itou test inputs.
+
 Tue Apr 10 19:17:29 EST 2007 Daniel P. Berrange <berrange@redhat.com>
 
        * qemud/conf.c, qemu/internal.h, qemud/iptables.c, qemud/iptables.h,

diff --git a/src/xend_internal.c b/src/xend_internal.c
index 257abc3edb65729e966ee40d2a1369bdc99f363b..c296ad615dfeb209ecfda6182d3ae3ce6d6c50be 100644 (file)
--- a/src/xend_internal.c
+++ b/src/xend_internal.c
@@ -1441,6 +1441,7 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, int xendConfigVersi
             char *offset;
             int isBlock = 0;
             int cdrom = 0;
+            int isNoSrcCdrom = 0;
             char *drvName = NULL;
             char *drvType = NULL;
             const char *src = NULL;
@@ -1458,64 +1459,75 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, int xendConfigVersi
                 mode = sexpr_node(node, "device/tap/mode");
             }
 
-            if (src == NULL) {
-                virXendError(conn, VIR_ERR_INTERNAL_ERROR,
-                             _("domain information incomplete, vbd has no src"));
-                goto bad_parse;
-            }
-
             if (dst == NULL) {
                 virXendError(conn, VIR_ERR_INTERNAL_ERROR,
                              _("domain information incomplete, vbd has no dev"));
                 goto bad_parse;
             }
 
-
-            offset = strchr(src, ':');
-            if (!offset) {
-                virXendError(conn, VIR_ERR_INTERNAL_ERROR,
-                             _("cannot parse vbd filename, missing driver name"));
-                goto bad_parse;
-            }
-
-            drvName = malloc((offset-src)+1);
-            if (!drvName) {
-                virXendError(conn, VIR_ERR_NO_MEMORY,
-                             _("allocate new buffer"));
-                goto bad_parse;
+            if (src == NULL) {
+                /* There is a case without the uname to the CD-ROM device */
+                offset = strchr(dst, ':');
+                if (offset) {
+                    if (hvm && !strcmp( offset , ":cdrom")) {
+                        isNoSrcCdrom = 1;
+                    }
+                    offset[0] = '\0';
+                }
+                if (!isNoSrcCdrom) {
+                    virXendError(conn, VIR_ERR_INTERNAL_ERROR,
+                                 _("domain information incomplete, vbd has no src"));
+                    goto bad_parse;
+                }
             }
-            strncpy(drvName, src, (offset-src));
-            drvName[offset-src] = '\0';
 
-            src = offset + 1;
-
-            if (!strcmp(drvName, "tap")) {
+            if (!isNoSrcCdrom) {
                 offset = strchr(src, ':');
                 if (!offset) {
                     virXendError(conn, VIR_ERR_INTERNAL_ERROR,
-                                 _("cannot parse vbd filename, missing driver type"));
+                                 _("cannot parse vbd filename, missing driver name"));
                     goto bad_parse;
                 }
 
-                drvType = malloc((offset-src)+1);
-                if (!drvType) {
+                drvName = malloc((offset-src)+1);
+                if (!drvName) {
                     virXendError(conn, VIR_ERR_NO_MEMORY,
                                  _("allocate new buffer"));
                     goto bad_parse;
                 }
-                strncpy(drvType, src, (offset-src));
-                drvType[offset-src] = '\0';
+                strncpy(drvName, src, (offset-src));
+                drvName[offset-src] = '\0';
+
                 src = offset + 1;
-                /* Its possible to use blktap driver for block devs
-                   too, but kinda pointless because blkback is better,
-                   so we assume common case here. If blktap becomes
-                   omnipotent, we can revisit this, perhaps stat()'ing
-                   the src file in question */
-                isBlock = 0;
-            } else if (!strcmp(drvName, "phy")) {
-                isBlock = 1;
-            } else if (!strcmp(drvName, "file")) {
-                isBlock = 0;
+
+                if (!strcmp(drvName, "tap")) {
+                    offset = strchr(src, ':');
+                    if (!offset) {
+                        virXendError(conn, VIR_ERR_INTERNAL_ERROR,
+                                     _("cannot parse vbd filename, missing driver type"));
+                        goto bad_parse;
+                    }
+
+                    drvType = malloc((offset-src)+1);
+                    if (!drvType) {
+                        virXendError(conn, VIR_ERR_NO_MEMORY,
+                                     _("allocate new buffer"));
+                        goto bad_parse;
+                    }
+                    strncpy(drvType, src, (offset-src));
+                    drvType[offset-src] = '\0';
+                    src = offset + 1;
+                    /* Its possible to use blktap driver for block devs
+                       too, but kinda pointless because blkback is better,
+                       so we assume common case here. If blktap becomes
+                       omnipotent, we can revisit this, perhaps stat()'ing
+                       the src file in question */
+                    isBlock = 0;
+                } else if (!strcmp(drvName, "phy")) {
+                    isBlock = 1;
+                } else if (!strcmp(drvName, "file")) {
+                    isBlock = 0;
+                }
             }
 
             if (!strncmp(dst, "ioemu:", 6))
@@ -1536,18 +1548,23 @@ xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, int xendConfigVersi
                 }
             }
 
-            virBufferVSprintf(&buf, "    <disk type='%s' device='%s'>\n",
-                              isBlock ? "block" : "file",
-                              cdrom ? "cdrom" : "disk");
-            if (drvType) {
-                virBufferVSprintf(&buf, "      <driver name='%s' type='%s'/>\n", drvName, drvType);
-            } else {
-                virBufferVSprintf(&buf, "      <driver name='%s'/>\n", drvName);
-            }
-            if (isBlock) {
-                virBufferVSprintf(&buf, "      <source dev='%s'/>\n", src);
+            if (!isNoSrcCdrom) {
+                virBufferVSprintf(&buf, "    <disk type='%s' device='%s'>\n",
+                                  isBlock ? "block" : "file",
+                                  cdrom ? "cdrom" : "disk");
+                if (drvType) {
+                    virBufferVSprintf(&buf, "      <driver name='%s' type='%s'/>\n", drvName, drvType);
+                } else {
+                    virBufferVSprintf(&buf, "      <driver name='%s'/>\n", drvName);
+                }
+                if (isBlock) {
+                    virBufferVSprintf(&buf, "      <source dev='%s'/>\n", src);
+                } else {
+                    virBufferVSprintf(&buf, "      <source file='%s'/>\n", src);
+                }
             } else {
-                virBufferVSprintf(&buf, "      <source file='%s'/>\n", src);
+                /* This case is the cdrom device only */
+                virBufferVSprintf(&buf, "    <disk device='cdrom'>\n");
             }
             virBufferVSprintf(&buf, "      <target dev='%s'/>\n", dst);
 

diff --git a/src/xml.c b/src/xml.c
index 4e04a2e78c614b5474b98574bad66ccde44345e3..89913cf6b2e579f693cdef847dd7a4fa7276d689 100644 (file)
--- a/src/xml.c
+++ b/src/xml.c
@@ -888,6 +888,7 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node, virBufferPtr buf,
     int shareable = 0;
     int typ = 0;
     int cdrom = 0;
+    int isNoSrcCdrom = 0;
 
     type = xmlGetProp(node, BAD_CAST "type");
     if (type != NULL) {
@@ -927,13 +928,23 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node, virBufferPtr buf,
     }
 
     if (source == NULL) {
-        virXMLError(conn, VIR_ERR_NO_SOURCE, (const char *) target, 0);
+        /* There is a case without the source
+         * to the CD-ROM device
+         */
+        if (hvm &&
+            device &&
+            !strcmp((const char *)device, "cdrom")) {
+            isNoSrcCdrom = 1;
+        }
+        if (!isNoSrcCdrom) {
+            virXMLError(conn, VIR_ERR_NO_SOURCE, (const char *) target, 0);
 
-        if (target != NULL)
-            xmlFree(target);
-        if (device != NULL)
-            xmlFree(device);
-        return (-1);
+            if (target != NULL)
+                xmlFree(target);
+            if (device != NULL)
+                xmlFree(device);
+            return (-1);
+        }
     }
     if (target == NULL) {
         virXMLError(conn, VIR_ERR_NO_TARGET, (const char *) source, 0);
@@ -988,7 +999,7 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node, virBufferPtr buf,
     } else
         virBufferVSprintf(buf, "(dev '%s')", (const char *)target);
 
-    if (drvName) {
+    if (drvName && !isNoSrcCdrom) {
         if (!strcmp((const char *)drvName, "tap")) {
             virBufferVSprintf(buf, "(uname '%s:%s:%s')",
                               (const char *)drvName,
@@ -999,7 +1010,7 @@ virDomainParseXMLDiskDesc(virConnectPtr conn, xmlNodePtr node, virBufferPtr buf,
                               (const char *)drvName,
                               (const char *)source);
         }
-    } else {
+    } else if (!isNoSrcCdrom) {
         if (typ == 0)
             virBufferVSprintf(buf, "(uname 'file:%s')", source);
         else if (typ == 1) {

diff --git a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr
new file mode 100644 (file)
index 0000000..f3e4901
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr
@@ -0,0 +1,66 @@
+(domain
+    (domid 6)
+    (on_crash destroy)
+    (memory 350)
+    (uuid cc2315e7-d26a-307a-438c-6d188ec4c09c)
+    (bootloader_args )
+    (name test)
+    (maxmem 382)
+    (on_reboot destroy)
+    (on_poweroff destroy)
+    (localtime 0)
+    (vcpus 1)
+    (bootloader )
+    (shadow_memory 4)
+    (cpu_weight 256)
+    (cpu_cap 0)
+    (features )
+    (on_xend_start ignore)
+    (on_xend_stop ignore)
+    (start_time 1175165307.38)
+    (cpu_time 0.956663338)
+    (online_vcpus 1)
+    (image
+        (hvm
+            (kernel /usr/lib/xen/boot/hvmloader)
+            (vnc 1)
+            (vncunused 1)
+            (device_model /usr/lib/xen/bin/qemu-dm)
+            (pae 1)
+            (apic 1)
+            (acpi 1)
+            (boot c)
+            (serial pty)
+            (vcpus 1)
+        )
+    )
+    (status 0)
+    (memory_dynamic_min 358)
+    (memory_dynamic_max 382)
+    (store_mfn 89598)
+    (device
+        (vif
+            (bridge xenbr0)
+            (mac 00:16:3e:0a:7b:39)
+            (type ioemu)
+            (uuid 127fee09-a0c8-c79b-4ee5-6c194efff73a)
+        )
+    )
+    (device
+        (vbd
+            (uname phy:/dev/sda8)
+            (driver paravirtualised)
+            (mode w)
+            (dev hda:disk)
+            (uuid df6969ad-d312-deb1-8d36-223d71e4ce95)
+        )
+    )
+    (device
+        (vbd
+            (driver paravirtualised)
+            (mode r)
+            (dev hdc:cdrom)
+            (uuid fca3aa85-1d1f-2c31-2f0b-f2dff2311f7e)
+        )
+    )
+)

diff --git a/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml
new file mode 100644 (file)
index 0000000..5b9c7f6
--- /dev/null
+++ b/tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml
@@ -0,0 +1,36 @@
+<domain type='xen' id='6'>
+  <name>test</name>
+  <uuid>cc2315e7d26a307a438c6d188ec4c09c</uuid>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='hd'/>
+  </os>
+  <memory>391168</memory>
+  <currentMemory>358400</currentMemory>
+  <vcpu>1</vcpu>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>destroy</on_reboot>
+  <on_crash>destroy</on_crash>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <interface type='ethernet'>
+      <mac address='00:16:3e:0a:7b:39'/>
+    </interface>
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/dev/sda8'/>
+      <target dev='hda:disk'/>
+    </disk>
+    <disk device='cdrom'>
+      <target dev='hdc'/>
+      <readonly/>
+    </disk>
+    <graphics type='vnc' port='5906'/>
+  </devices>
+</domain>

diff --git a/tests/sexpr2xmltest.c b/tests/sexpr2xmltest.c
index 80403bddf5282052f6f1eaf2d77df17ef5ef7b86..2c567aaf8baa576d1befbad45748c9bb4753a9e1 100644 (file)
--- a/tests/sexpr2xmltest.c
+++ b/tests/sexpr2xmltest.c
@@ -124,6 +124,12 @@ static int testCompareNetBridged(void *data ATTRIBUTE_UNUSED) {
                          1);
 }
 
+static int testCompareNoSourceCDRom(void *data ATTRIBUTE_UNUSED) {
+  return testCompareFiles("sexpr2xmldata/sexpr2xml-no-source-cdrom.xml",
+                         "sexpr2xmldata/sexpr2xml-no-source-cdrom.sexpr",
+                         1);
+}
+
 
 int
 main(int argc, char **argv)
@@ -189,6 +195,10 @@ main(int argc, char **argv)
                    1, testCompareNetBridged, NULL) != 0)
        ret = -1;
 
+    if (virtTestRun("SEXPR-2-XML no source CDRom",
+                   1, testCompareNoSourceCDRom, NULL) != 0)
+       ret = -1;
+
     exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }
 #else /* WITHOUT_XEN */

diff --git a/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr b/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr
new file mode 100644 (file)
index 0000000..a027cd0
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr
@@ -0,0 +1 @@
+(vm (name 'test')(memory 350)(maxmem 382)(vcpus 1)(uuid 'cc2315e7d26a307a438c6d188ec4c09c')(on_poweroff 'destroy')(on_reboot 'destroy')(on_crash 'destroy')(image (hvm (kernel '/usr/lib/xen/boot/hvmloader')(device_model '/usr/lib/xen/bin/qemu-dm')(vcpus 1)(boot c)(acpi 1)(apic 1)(pae 1)(vnc 1)(vncdisplay 6)))(device (vbd (dev 'hda:disk:disk')(uname 'phy:/dev/sda8')(mode 'w')))(device (vbd (dev 'hdc:cdrom')(mode 'r')))(device (vif (mac '00:16:3e:0a:7b:39')(type ioemu))))
\ No newline at end of file

diff --git a/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml b/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml
new file mode 100644 (file)
index 0000000..5b9c7f6
--- /dev/null
+++ b/tests/xml2sexprdata/xml2sexpr-no-source-cdrom.xml
@@ -0,0 +1,36 @@
+<domain type='xen' id='6'>
+  <name>test</name>
+  <uuid>cc2315e7d26a307a438c6d188ec4c09c</uuid>
+  <os>
+    <type>hvm</type>
+    <loader>/usr/lib/xen/boot/hvmloader</loader>
+    <boot dev='hd'/>
+  </os>
+  <memory>391168</memory>
+  <currentMemory>358400</currentMemory>
+  <vcpu>1</vcpu>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>destroy</on_reboot>
+  <on_crash>destroy</on_crash>
+  <features>
+    <acpi/>
+    <apic/>
+    <pae/>
+  </features>
+  <devices>
+    <emulator>/usr/lib/xen/bin/qemu-dm</emulator>
+    <interface type='ethernet'>
+      <mac address='00:16:3e:0a:7b:39'/>
+    </interface>
+    <disk type='block' device='disk'>
+      <driver name='phy'/>
+      <source dev='/dev/sda8'/>
+      <target dev='hda:disk'/>
+    </disk>
+    <disk device='cdrom'>
+      <target dev='hdc'/>
+      <readonly/>
+    </disk>
+    <graphics type='vnc' port='5906'/>
+  </devices>
+</domain>

diff --git a/tests/xml2sexprtest.c b/tests/xml2sexprtest.c
index b5657ec6bed000eaa98bdafd76761590b6594297..069d65f063b538195330be1709cd6fe1bbd38dba 100644 (file)
--- a/tests/xml2sexprtest.c
+++ b/tests/xml2sexprtest.c
@@ -1,6 +1,9 @@
 #include <stdio.h>
 #include <string.h>
 
+#include <sys/types.h>
+#include <fcntl.h>
+
 #include "xml.h"
 #include "testutils.h"
 #include "internal.h"
@@ -31,11 +34,14 @@ static int testCompareFiles(const char *xml, const char *sexpr, const char *name
       printf("Expect %d '%s'\n", (int)strlen(sexprData), sexprData);
       printf("Actual %d '%s'\n", (int)strlen(gotsexpr), gotsexpr);
   }
-  if (strcmp(sexprData, gotsexpr))
-    goto fail;
+  if (strcmp(sexprData, gotsexpr)) {
+      goto fail;
+  }
 
-  if (strcmp(name, gotname))
-    goto fail;
+  if (strcmp(name, gotname)) {
+      printf("Got wrong name: expected %s, got %s\n", name, gotname);
+      goto fail;
+  }
 
   ret = 0;
 
@@ -167,6 +173,13 @@ static int testCompareNetBridged(void *data ATTRIBUTE_UNUSED) {
                          2);
 }
 
+static int testCompareNoSourceCDRom(void *data ATTRIBUTE_UNUSED) {
+  return testCompareFiles("xml2sexprdata/xml2sexpr-no-source-cdrom.xml",
+                         "xml2sexprdata/xml2sexpr-no-source-cdrom.sexpr",
+                         "test",
+                         2);
+}
+
 
 int
 main(int argc, char **argv)
@@ -248,5 +261,9 @@ main(int argc, char **argv)
                    1, testCompareNetBridged, NULL) != 0)
        ret = -1;
 
+    if (virtTestRun("XML-2-SEXPR No Source CDRom",
+                   1, testCompareNoSourceCDRom, NULL) != 0)
+       ret = -1;
+
     exit(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }