esx: Fix regression in absolute file name handling

Before commit 145d6cb05c45f4 (in August 2010) absolute file names
in VMX and domain XML configs were handled correctly. But this got
lost during the refactoring. The test cases didn't highlight this
problem because they have their own set of file name handling
functions. The actual ones require a real connection to an ESX
server. Also the test case functions always worked correctly.

Fix the regression and add a new in-the-wild VMX file that contains
such a problematic absolute path. Even though this test case won't
protect against new regressions.

Reported by lofic (IRC nick)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 845dd4a319eec4b7bb9b1632d6c0b82f01b63dc4..809afeb55530deab33cbb9956c5197c5257bd577 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -74,8 +74,8 @@ esxFreePrivate(esxPrivate **priv)
 
 
 /*
- * Parse a file name from a .vmx file and convert it to datastore path format.
- * A .vmx file can contain file names in various formats:
+ * Parse a file name from a .vmx file and convert it to datastore path format
+ * if possbile. A .vmx file can contain file names in various formats:
  *
  * - A single name referencing a file in the same directory as the .vmx file:
  *
@@ -97,6 +97,14 @@ esxFreePrivate(esxPrivate **priv)
  *     C:\Virtual Machines\test1\test1.vmdk
  *     \\nas1\storage1\test1\test1.vmdk
  *
+ * - There might also be absolute file names referencing files outside of a
+ *   datastore:
+ *
+ *     /usr/lib/vmware/isoimages/linux.iso
+ *
+ *   Such file names are left as is and are not converted to datastore path
+ *   format because this is not possible.
+ *
  * The datastore path format typically looks like this:
  *
  *  [datastore1] test1/test1.vmdk
@@ -117,7 +125,7 @@ esxFreePrivate(esxPrivate **priv)
 static char *
 esxParseVMXFileName(const char *fileName, void *opaque)
 {
-    char *datastorePath = NULL;
+    char *result = NULL;
     esxVMX_Data *data = opaque;
     esxVI_String *propertyNameList = NULL;
     esxVI_ObjectContent *datastoreList = NULL;
@@ -132,7 +140,7 @@ esxParseVMXFileName(const char *fileName, void *opaque)
 
     if (strchr(fileName, '/') == NULL && strchr(fileName, '\\') == NULL) {
         /* Plain file name, use same directory as for the .vmx file */
-        if (virAsprintf(&datastorePath, "%s/%s",
+        if (virAsprintf(&result, "%s/%s",
                         data->datastorePathWithoutFileName, fileName) < 0) {
             virReportOOMError();
             goto cleanup;
@@ -184,7 +192,7 @@ esxParseVMXFileName(const char *fileName, void *opaque)
                 ++tmp;
             }
 
-            if (virAsprintf(&datastorePath, "[%s] %s", datastoreName,
+            if (virAsprintf(&result, "[%s] %s", datastoreName,
                             strippedFileName) < 0) {
                 virReportOOMError();
                 goto cleanup;
@@ -194,7 +202,7 @@ esxParseVMXFileName(const char *fileName, void *opaque)
         }
 
         /* Fallback to direct datastore name match */
-        if (datastorePath == NULL && STRPREFIX(fileName, "/vmfs/volumes/")) {
+        if (result == NULL && STRPREFIX(fileName, "/vmfs/volumes/")) {
             if (esxVI_String_DeepCopyValue(&copyOfFileName, fileName) < 0) {
                 goto cleanup;
             }
@@ -224,16 +232,24 @@ esxParseVMXFileName(const char *fileName, void *opaque)
                 goto cleanup;
             }
 
-            if (virAsprintf(&datastorePath, "[%s] %s", datastoreName,
+            if (virAsprintf(&result, "[%s] %s", datastoreName,
                             directoryAndFileName) < 0) {
                 virReportOOMError();
                 goto cleanup;
             }
         }
 
-        if (datastorePath == NULL) {
+        /* If it's an absolute path outside of a datastore just use it as is */
+        if (result == NULL && *fileName == '/') {
+            /* FIXME: need to deal with Windows paths here too */
+            if (esxVI_String_DeepCopyValue(&result, fileName) < 0) {
+                goto cleanup;
+            }
+        }
+
+        if (result == NULL) {
             ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
-                      _("Could not find datastore for '%s'"), fileName);
+                      _("Could not handle file name '%s'"), fileName);
             goto cleanup;
         }
     }
@@ -245,15 +261,15 @@ esxParseVMXFileName(const char *fileName, void *opaque)
     VIR_FREE(strippedFileName);
     VIR_FREE(copyOfFileName);
 
-    return datastorePath;
+    return result;
 }
 
 
 
 /*
  * This function does the inverse of esxParseVMXFileName. It takes an file name
- * in datastore path format and converts it to a file name that can be used in
- * a .vmx file.
+ * in datastore path format or in absolute format and converts it to a file
+ * name that can be used in a .vmx file.
  *
  * The datastore path format and the formats found in a .vmx file are described
  * in the documentation of esxParseVMXFileName.
@@ -264,9 +280,10 @@ esxParseVMXFileName(const char *fileName, void *opaque)
  * based on the mount path.
  */
 static char *
-esxFormatVMXFileName(const char *datastorePath, void *opaque)
+esxFormatVMXFileName(const char *fileName, void *opaque)
 {
     bool success = false;
+    char *result = NULL;
     esxVMX_Data *data = opaque;
     char *datastoreName = NULL;
     char *directoryAndFileName = NULL;
@@ -276,58 +293,68 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque)
     virBuffer buffer = VIR_BUFFER_INITIALIZER;
     char *tmp;
     size_t length;
-    char *absolutePath = NULL;
 
-    /* Parse datastore path and lookup datastore */
-    if (esxUtil_ParseDatastorePath(datastorePath, &datastoreName, NULL,
-                                   &directoryAndFileName) < 0) {
-        goto cleanup;
-    }
+    if (*fileName == '[') {
+        /* Parse datastore path and lookup datastore */
+        if (esxUtil_ParseDatastorePath(fileName, &datastoreName, NULL,
+                                       &directoryAndFileName) < 0) {
+            goto cleanup;
+        }
 
-    if (esxVI_LookupDatastoreByName(data->ctx, datastoreName, NULL, &datastore,
-                                    esxVI_Occurrence_RequiredItem) < 0 ||
-        esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
-                                       &hostMount) < 0) {
-        goto cleanup;
-    }
+        if (esxVI_LookupDatastoreByName(data->ctx, datastoreName, NULL, &datastore,
+                                        esxVI_Occurrence_RequiredItem) < 0 ||
+            esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
+                                           &hostMount) < 0) {
+            goto cleanup;
+        }
 
-    /* Detect separator type */
-    if (strchr(hostMount->mountInfo->path, '\\') != NULL) {
-        separator = '\\';
-    }
+        /* Detect separator type */
+        if (strchr(hostMount->mountInfo->path, '\\') != NULL) {
+            separator = '\\';
+        }
 
-    /* Strip trailing separators */
-    length = strlen(hostMount->mountInfo->path);
+        /* Strip trailing separators */
+        length = strlen(hostMount->mountInfo->path);
 
-    while (length > 0 && hostMount->mountInfo->path[length - 1] == separator) {
-        --length;
-    }
+        while (length > 0 && hostMount->mountInfo->path[length - 1] == separator) {
+            --length;
+        }
 
-    /* Format as <mount>[/<directory>]/<file>, convert / to \ when necessary */
-    virBufferAdd(&buffer, hostMount->mountInfo->path, length);
+        /* Format as <mount>[/<directory>]/<file>, convert / to \ when necessary */
+        virBufferAdd(&buffer, hostMount->mountInfo->path, length);
 
-    if (separator != '/') {
-        tmp = directoryAndFileName;
+        if (separator != '/') {
+            tmp = directoryAndFileName;
 
-        while (*tmp != '\0') {
-            if (*tmp == '/') {
-                *tmp = separator;
-            }
+            while (*tmp != '\0') {
+                if (*tmp == '/') {
+                    *tmp = separator;
+                }
 
-            ++tmp;
+                ++tmp;
+            }
         }
-    }
 
-    virBufferAddChar(&buffer, separator);
-    virBufferAdd(&buffer, directoryAndFileName, -1);
+        virBufferAddChar(&buffer, separator);
+        virBufferAdd(&buffer, directoryAndFileName, -1);
 
-    if (virBufferError(&buffer)) {
-        virReportOOMError();
+        if (virBufferError(&buffer)) {
+            virReportOOMError();
+            goto cleanup;
+        }
+
+        result = virBufferContentAndReset(&buffer);
+    } else if (*fileName == '/') {
+        /* FIXME: need to deal with Windows paths here too */
+        if (esxVI_String_DeepCopyValue(&result, fileName) < 0) {
+            goto cleanup;
+        }
+    } else {
+        ESX_ERROR(VIR_ERR_INTERNAL_ERROR,
+                  _("Could not handle file name '%s'"), fileName);
         goto cleanup;
     }
 
-    absolutePath = virBufferContentAndReset(&buffer);
-
     /* FIXME: Check if referenced path/file really exists */
 
     success = true;
@@ -335,7 +362,7 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque)
   cleanup:
     if (! success) {
         virBufferFreeAndReset(&buffer);
-        VIR_FREE(absolutePath);
+        VIR_FREE(result);
     }
 
     VIR_FREE(datastoreName);
@@ -343,7 +370,7 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque)
     esxVI_ObjectContent_Free(&datastore);
     esxVI_DatastoreHostMount_Free(&hostMount);
 
-    return absolutePath;
+    return result;
 }
 
 

diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.vmx b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.vmx
new file mode 100644 (file)
index 0000000..b795e80
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.vmx
@@ -0,0 +1,80 @@
+.encoding = "UTF-8"
+config.version = "8"
+virtualHW.version = "7"
+pciBridge0.present = "TRUE"
+pciBridge4.present = "TRUE"
+pciBridge4.virtualDev = "pcieRootPort"
+pciBridge4.functions = "8"
+pciBridge5.present = "TRUE"
+pciBridge5.virtualDev = "pcieRootPort"
+pciBridge5.functions = "8"
+pciBridge6.present = "TRUE"
+pciBridge6.virtualDev = "pcieRootPort"
+pciBridge6.functions = "8"
+pciBridge7.present = "TRUE"
+pciBridge7.virtualDev = "pcieRootPort"
+pciBridge7.functions = "8"
+vmci0.present = "TRUE"
+nvram = "el6-test.nvram"
+virtualHW.productCompatibility = "hosted"
+powerType.powerOff = "soft"
+powerType.powerOn = "hard"
+powerType.suspend = "hard"
+powerType.reset = "soft"
+displayName = "el6-test"
+extendedConfigFile = "el6-test.vmxf"
+floppy0.present = "TRUE"
+scsi0.present = "TRUE"
+scsi0.sharedBus = "none"
+scsi0.virtualDev = "pvscsi"
+memsize = "1024"
+scsi0:0.present = "TRUE"
+scsi0:0.fileName = "el6-test-000001.vmdk"
+scsi0:0.deviceType = "scsi-hardDisk"
+ide1:0.present = "TRUE"
+ide1:0.clientDevice = "FALSE"
+ide1:0.deviceType = "cdrom-image"
+ide1:0.startConnected = "FALSE"
+floppy0.startConnected = "FALSE"
+floppy0.fileName = ""
+floppy0.clientDevice = "TRUE"
+ethernet0.present = "TRUE"
+ethernet0.virtualDev = "vmxnet3"
+ethernet0.networkName = "VM Network"
+ethernet0.addressType = "generated"
+guestOS = "rhel6-64"
+uuid.location = "56 4d 15 d4 d0 62 fe 9a-80 f5 eb 8e 1a 2c 3a fc"
+uuid.bios = "56 4d 15 d4 d0 62 fe 9a-80 f5 eb 8e 1a 2c 3a fc"
+vc.uuid = "52 00 b6 9b 8d 88 7b df-a1 4a 02 70 5d 65 37 72"
+ethernet0.generatedAddress = "00:0c:29:2c:3a:fc"
+svga.vramSize = "8388608"
+vmci0.id = "439106300"
+cleanShutdown = "TRUE"
+replay.supported = "TRUE"
+sched.swap.derivedName = "/vmfs/volumes/4dd68884-f4586c0e-8223-00215aaab842/el6-test/el6-test-4475e3f0.vswp"
+replay.filename = ""
+scsi0:0.redo = ""
+pciBridge0.pciSlotNumber = "17"
+pciBridge4.pciSlotNumber = "21"
+pciBridge5.pciSlotNumber = "22"
+pciBridge6.pciSlotNumber = "23"
+pciBridge7.pciSlotNumber = "24"
+scsi0.pciSlotNumber = "160"
+ethernet0.pciSlotNumber = "192"
+vmci0.pciSlotNumber = "32"
+scsi0.sasWWID = "50 05 05 64 d0 62 fe 90"
+vmotion.checkpointFBSize = "8388608"
+ethernet0.generatedAddressOffset = "0"
+tools.remindInstall = "FALSE"
+hostCPUID.0 = "0000000a756e65476c65746e49656e69"
+hostCPUID.1 = "0001067600040800000ce33dbfebfbff"
+hostCPUID.80000001 = "00000000000000000000000120100800"
+guestCPUID.0 = "0000000a756e65476c65746e49656e69"
+guestCPUID.1 = "0001067600010800800822010febfbff"
+guestCPUID.80000001 = "00000000000000000000000120100800"
+userCPUID.0 = "0000000a756e65476c65746e49656e69"
+userCPUID.1 = "0001067600040800000822010febfbff"
+userCPUID.80000001 = "00000000000000000000000120100800"
+evcCompatibilityMode = "FALSE"
+ide1:0.fileName = "/usr/lib/vmware/isoimages/linux.iso"
+tools.syncTime = "FALSE"

diff --git a/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml
new file mode 100644 (file)
index 0000000..0c4e4d5
--- /dev/null
+++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml
@@ -0,0 +1,36 @@
+<domain type='vmware'>
+  <name>el6-test</name>
+  <uuid>564d15d4-d062-fe9a-80f5-eb8e1a2c3afc</uuid>
+  <memory>1048576</memory>
+  <currentMemory>1048576</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='x86_64'>hvm</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <disk type='file' device='disk'>
+      <source file='[datastore] directory/el6-test-000001.vmdk'/>
+      <target dev='sda' bus='scsi'/>
+      <address type='drive' controller='0' bus='0' unit='0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <source file='/usr/lib/vmware/isoimages/linux.iso'/>
+      <target dev='hdc' bus='ide'/>
+      <address type='drive' controller='0' bus='1' unit='0'/>
+    </disk>
+    <controller type='scsi' index='0' model='vmpvscsi'/>
+    <controller type='ide' index='0'/>
+    <interface type='bridge'>
+      <mac address='00:0c:29:2c:3a:fc'/>
+      <source bridge='VM Network'/>
+      <model type='vmxnet3'/>
+    </interface>
+    <video>
+      <model type='vmvga' vram='8192'/>
+    </video>
+  </devices>
+</domain>

diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c
index e01e8ad03ec0c8623dcb822c16bb19a6bed11588..7396f39bb3d4fb2eaffd8ec01f26d9fc256492e3 100644 (file)
--- a/tests/vmx2xmltest.c
+++ b/tests/vmx2xmltest.c
@@ -261,6 +261,7 @@ mymain(void)
     DO_TEST("esx-in-the-wild-3", "esx-in-the-wild-3");
     DO_TEST("esx-in-the-wild-4", "esx-in-the-wild-4");
     DO_TEST("esx-in-the-wild-5", "esx-in-the-wild-5");
+    DO_TEST("esx-in-the-wild-6", "esx-in-the-wild-6");
 
     DO_TEST("gsx-in-the-wild-1", "gsx-in-the-wild-1");
     DO_TEST("gsx-in-the-wild-2", "gsx-in-the-wild-2");

diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx
new file mode 100644 (file)
index 0000000..1f29ae5
--- /dev/null
+++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx
@@ -0,0 +1,26 @@
+.encoding = "UTF-8"
+config.version = "8"
+virtualHW.version = "4"
+guestOS = "other-64"
+uuid.bios = "56 4d 15 d4 d0 62 fe 9a-80 f5 eb 8e 1a 2c 3a fc"
+displayName = "el6-test"
+memsize = "1024"
+numvcpus = "1"
+scsi0.present = "true"
+scsi0.virtualDev = "pvscsi"
+scsi0:0.present = "true"
+scsi0:0.deviceType = "scsi-hardDisk"
+scsi0:0.fileName = "/vmfs/volumes/datastore/directory/el6-test-000001.vmdk"
+ide1:0.present = "true"
+ide1:0.deviceType = "cdrom-image"
+ide1:0.fileName = "/usr/lib/vmware/isoimages/linux.iso"
+floppy0.present = "false"
+floppy1.present = "false"
+ethernet0.present = "true"
+ethernet0.virtualDev = "vmxnet3"
+ethernet0.networkName = "VM Network"
+ethernet0.connectionType = "bridged"
+ethernet0.addressType = "generated"
+ethernet0.generatedAddress = "00:0C:29:2C:3A:FC"
+ethernet0.generatedAddressOffset = "0"
+svga.vramSize = "8388608"

diff --git a/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml
new file mode 100644 (file)
index 0000000..0c4e4d5
--- /dev/null
+++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml
@@ -0,0 +1,36 @@
+<domain type='vmware'>
+  <name>el6-test</name>
+  <uuid>564d15d4-d062-fe9a-80f5-eb8e1a2c3afc</uuid>
+  <memory>1048576</memory>
+  <currentMemory>1048576</currentMemory>
+  <vcpu>1</vcpu>
+  <os>
+    <type arch='x86_64'>hvm</type>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <disk type='file' device='disk'>
+      <source file='[datastore] directory/el6-test-000001.vmdk'/>
+      <target dev='sda' bus='scsi'/>
+      <address type='drive' controller='0' bus='0' unit='0'/>
+    </disk>
+    <disk type='file' device='cdrom'>
+      <source file='/usr/lib/vmware/isoimages/linux.iso'/>
+      <target dev='hdc' bus='ide'/>
+      <address type='drive' controller='0' bus='1' unit='0'/>
+    </disk>
+    <controller type='scsi' index='0' model='vmpvscsi'/>
+    <controller type='ide' index='0'/>
+    <interface type='bridge'>
+      <mac address='00:0c:29:2c:3a:fc'/>
+      <source bridge='VM Network'/>
+      <model type='vmxnet3'/>
+    </interface>
+    <video>
+      <model type='vmvga' vram='8192'/>
+    </video>
+  </devices>
+</domain>

diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c
index efd4d749306be46f4bf2b39d62cd2a9e35b335a3..66ed53d6960a36198fa611b7fd3095621177bdf6 100644 (file)
--- a/tests/xml2vmxtest.c
+++ b/tests/xml2vmxtest.c
@@ -272,6 +272,7 @@ mymain(void)
     DO_TEST("esx-in-the-wild-3", "esx-in-the-wild-3", 4);
     DO_TEST("esx-in-the-wild-4", "esx-in-the-wild-4", 4);
     DO_TEST("esx-in-the-wild-5", "esx-in-the-wild-5", 4);
+    DO_TEST("esx-in-the-wild-6", "esx-in-the-wild-6", 4);
 
     DO_TEST("gsx-in-the-wild-1", "gsx-in-the-wild-1", 4);
     DO_TEST("gsx-in-the-wild-2", "gsx-in-the-wild-2", 4);