esx: Fix regression in absolute file name handling

Before commit 145d6cb05c (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)

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 (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 = '\\';
-    }
-
-    /* Strip trailing separators */
-    length = strlen(hostMount->mountInfo->path);
-
-    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);
-
-    if (separator != '/') {
-        tmp = directoryAndFileName;
-
-        while (*tmp != '\0') {
-            if (*tmp == '/') {
-                *tmp = separator;
-            }
-
-            ++tmp;
+    if (*fileName == '[') {
+        /* Parse datastore path and lookup datastore */
+        if (esxUtil_ParseDatastorePath(fileName, &datastoreName, NULL,
+                                       &directoryAndFileName) < 0) {
+            goto cleanup;
         }
-    }
 
-    virBufferAddChar(&buffer, separator);
-    virBufferAdd(&buffer, directoryAndFileName, -1);
+        if (esxVI_LookupDatastoreByName(data->ctx, datastoreName, NULL, &datastore,
+                                        esxVI_Occurrence_RequiredItem) < 0 ||
+            esxVI_LookupDatastoreHostMount(data->ctx, datastore->obj,
+                                           &hostMount) < 0) {
+            goto cleanup;
+        }
 
-    if (virBufferError(&buffer)) {
-        virReportOOMError();
+        /* Detect separator type */
+        if (strchr(hostMount->mountInfo->path, '\\') != NULL) {
+            separator = '\\';
+        }
+
+        /* Strip trailing separators */
+        length = strlen(hostMount->mountInfo->path);
+
+        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);
+
+        if (separator != '/') {
+            tmp = directoryAndFileName;
+
+            while (*tmp != '\0') {
+                if (*tmp == '/') {
+                    *tmp = separator;
+                }
+
+                ++tmp;
+            }
+        }
+
+        virBufferAddChar(&buffer, separator);
+        virBufferAdd(&buffer, directoryAndFileName, -1);
+
+        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;
 }
 
 

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"

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>

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");

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"

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>

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);