From 8357d91b652930440f11c205c6d510da2e854c87 Mon Sep 17 00:00:00 2001 From: Matthias Bolte Date: Thu, 26 May 2011 17:40:51 +0200 Subject: [PATCH] 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) --- src/esx/esx_driver.c | 147 +++++++++++------- .../vmx2xmldata/vmx2xml-esx-in-the-wild-6.vmx | 80 ++++++++++ .../vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml | 36 +++++ tests/vmx2xmltest.c | 1 + .../xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx | 26 ++++ .../xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml | 36 +++++ tests/xml2vmxtest.c | 1 + 7 files changed, 267 insertions(+), 60 deletions(-) create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.vmx create mode 100644 tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.vmx create mode 100644 tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 845dd4a319..809afeb555 100644 --- 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(©OfFileName, 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 [/]/, 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 [/]/, 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; } 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 index 0000000000..b795e80a0d --- /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 index 0000000000..0c4e4d56cb --- /dev/null +++ b/tests/vmx2xmldata/vmx2xml-esx-in-the-wild-6.xml @@ -0,0 +1,36 @@ + + el6-test + 564d15d4-d062-fe9a-80f5-eb8e1a2c3afc + 1048576 + 1048576 + 1 + + hvm + + + destroy + restart + destroy + + + + +
+ + + + +
+ + + + + + + + + + + diff --git a/tests/vmx2xmltest.c b/tests/vmx2xmltest.c index e01e8ad03e..7396f39bb3 100644 --- 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 index 0000000000..1f29ae5490 --- /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 index 0000000000..0c4e4d56cb --- /dev/null +++ b/tests/xml2vmxdata/xml2vmx-esx-in-the-wild-6.xml @@ -0,0 +1,36 @@ + + el6-test + 564d15d4-d062-fe9a-80f5-eb8e1a2c3afc + 1048576 + 1048576 + 1 + + hvm + + + destroy + restart + destroy + + + + +
+ + + + +
+ + + + + + + + + + + diff --git a/tests/xml2vmxtest.c b/tests/xml2vmxtest.c index efd4d74930..66ed53d696 100644 --- 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);