2010/12/2 Jean-Baptiste Rouault <jean-baptiste.rouault@diateam.net>:
--- configure.ac | 7 + include/libvirt/virterror.h | 1 + po/POTFILES.in | 2 + src/Makefile.am | 24 +- src/driver.h | 3 +- src/libvirt.c | 15 + src/util/virterror.c | 3 + src/vmware/vmware_conf.c | 480 +++++++++++++++ src/vmware/vmware_conf.h | 82 +++ src/vmware/vmware_driver.c | 1372 +++++++++++++++++++++++++++++++++++++++++++ src/vmware/vmware_driver.h | 25 + 11 files changed, 2010 insertions(+), 4 deletions(-) create mode 100644 src/vmware/vmware_conf.c create mode 100644 src/vmware/vmware_conf.h create mode 100644 src/vmware/vmware_driver.c create mode 100644 src/vmware/vmware_driver.h
diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c new file mode 100644 index 0000000..5e873e1 --- /dev/null +++ b/src/vmware/vmware_conf.c @@ -0,0 +1,480 @@ +/*---------------------------------------------------------------------------*/ +/* Copyright 2010, diateam (www.diateam.net) + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +/*---------------------------------------------------------------------------*/ + +#include <config.h> + +#include <sys/utsname.h> + +#include "cpu/cpu.h" +#include "memory.h" +#include "nodeinfo.h" +#include "util/files.h" +#include "uuid.h" +#include "virterror_internal.h" +#include "../esx/esx_vmx.h" + +#include "vmware_conf.h" + +#define VMWARE_MAX_ARG 20 + +/* Free all memory associated with a vmware_driver structure */ +void +vmwareFreeDriver(struct vmware_driver *driver) +{ + if (!driver) + return;
virMutexDestroy(&driver->lock) is missing here.
+ virDomainObjListDeinit(&driver->domains); + virCapabilitiesFree(driver->caps); + VIR_FREE(driver); +} +
+ +int +vmwareLoadDomains(struct vmware_driver *driver) +{ + FILE *fp; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; + char vmxPath[1024]; + char * vmx = NULL; + vmwareDomainPtr pDomain; + char *directoryName = NULL; + char *fileName = NULL; + int ret = -1; + esxVMX_Context ctx; + + ctx.parseFileName = esxCopyVMXFileName; + + fp = driver->type == + TYPE_PLAYER ? popen(VMRUN " -T " "player" " list 2>/dev/null", + "r") : popen(VMRUN " -T " "ws" + " list 2>/dev/null", "r"); + if (fp == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); + return -1; + } + + while (!feof(fp)) { + if (fgets(vmxPath, 1024, fp) == NULL) {
Use fgets(vmxPath, sizeof(vmxPath), fp) here instead.
+ if (feof(fp)) + break; + + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to parse vmrun list output")); + goto cleanup; + } + if (vmxPath[0] != '/') + continue; + + /* remove trailing newline */ + int len = strlen(vmxPath);
strlen returns size_t, not int. Also move size_t len to the block of other variables at the beginning of the function.
+ if (len && vmxPath[len-1] == '\n') + vmxPath[len-1] = '\0'; + + if (virFileReadAll(vmxPath, 10000, &vmx) == -1) { + perror("error reading vmx file"); + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("failed to read vmx file %s"), vmxPath); + goto cleanup; + } + + if ((vmdef = + esxVMX_ParseConfig(&ctx, driver->caps, vmx, + esxVI_ProductVersion_ESX4x)) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse config file")); + goto cleanup; + } + + if (!(vm = virDomainAssignDef(driver->caps, + &driver->domains, vmdef, false))) + goto cleanup; + + pDomain = vm->privateData; + + pDomain->vmxPath = strdup(vmxPath); + if (pDomain->vmxPath == NULL) { + virReportOOMError(); + goto no_memory;
Just goto cleanup here, as you already reported the OOM error. Now you can remove the no_memory label and avoid the jump backwards to the cleanup label.
+ } + + vmwareDomainConfigDisplay(pDomain, vmdef); + + //vmrun list only reports running vms + vm->state = VIR_DOMAIN_RUNNING; + vm->def->id = driver->nextvmid++; + vm->persistent = 1; + + virDomainObjUnlock(vm); + + vmdef = NULL; + vm = NULL; + } + + ret = 0; + +cleanup: + virDomainDefFree(vmdef); + VIR_FORCE_FCLOSE(fp); + VIR_FREE(directoryName); + VIR_FREE(fileName); + VIR_FREE(vmx); + if (vm) + virDomainObjUnref(vm); + return ret; + +no_memory: + virReportOOMError(); + goto cleanup; +} +
+ +int +vmwareExtractVersion(struct vmware_driver *driver) +{ + unsigned long version = 0; + FILE *fp = NULL; + char str[50]; + char *tmp; + int ret = -1; + + if (driver->type == TYPE_PLAYER) { + if ((fp = popen("vmplayer -v 2>/dev/null", "r")) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); + goto cleanup; + } + if (!feof(fp) && fgets(str, 49, fp)) { + if ((tmp = STRSKIP(str, "VMware Player ")) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmplayer -v parsing error")); + goto cleanup; + } + if (virParseVersionString(tmp, &version) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("version parsing error")); + goto cleanup; + } + driver->version = version; + ret = 0; + } else { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmplayer -v reading error")); + } + } else if (driver->type == TYPE_WORKSTATION) { + if ((fp = popen("vmware -v 2>/dev/null", "r")) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); + goto cleanup; + } + if (!feof(fp) && fgets(str, 49, fp)) { + if ((tmp = STRSKIP(str, "VMware Workstation ")) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmware -v parsing error")); + goto cleanup; + } + if (virParseVersionString(tmp, &version) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("version parsing error")); + goto cleanup; + } + driver->version = version; + ret = 0; + } else { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmware -v reading error")); + } + } + +cleanup: + VIR_FORCE_FCLOSE(fp); + return ret; +} +
+ +int +vmwareParsePath(char *path, char **directory, char **filename) +{ + char *separator; + + separator = strrchr(path, '/'); + + if (separator != NULL) { + *separator++ = '\0'; + + if (*separator == '\0') { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("path '%s' doesn't reference a file"), path); + return -1; + } + + if ((*directory = strdup(path)) == NULL) + goto no_memory; + if ((*filename = strdup(separator)) == NULL) + goto no_memory; + + } else { + if ((*filename = strdup(separator)) == NULL) + goto no_memory; + } + + return 0; + +no_memory: + virReportOOMError(); + return -1; +} +
+int +vmwareVmxPath(virDomainDefPtr vmdef, char **vmxPath) +{ + virDomainDiskDefPtr disk = NULL; + char *directoryName = NULL; + char *fileName = NULL; + int ret = -1; + int i = 0; + + /* + * Build VMX URL. Use the source of the first file-based harddisk + * to deduce the path for the VMX file. Don't just use the + * first disk, because it may be CDROM disk and ISO images are normaly not + * located in the virtual machine's directory. This approach + * isn't perfect but should work in the majority of cases. + */ + if (vmdef->ndisks < 1) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Domain XML doesn't contain any disks, " + "cannot deduce datastore and path for VMX file")); + goto cleanup; + } + + for (i = 0; i < vmdef->ndisks; ++i) { + if (vmdef->disks[i]->device == VIR_DOMAIN_DISK_DEVICE_DISK && + vmdef->disks[i]->type == VIR_DOMAIN_DISK_TYPE_FILE) { + disk = vmdef->disks[i]; + break; + } + } + + if (disk == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Domain XML doesn't contain any file-based harddisks, " + "cannot deduce datastore and path for VMX file")); + goto cleanup; + } + + if (disk->src == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("First file-based harddisk has no source, cannot " + "deduce datastore and path for VMX file")); + goto cleanup; + } + + if (vmwareParsePath(disk->src, &directoryName, &fileName) < 0) { + goto cleanup; + } + + if (!virFileHasSuffix(fileName, ".vmdk")) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("Expecting source '%s' of first file-based harddisk " + "to be a VMDK image"), disk->src); + goto cleanup; + } + + if (vmwareConstructVmxPath(directoryName, vmdef->name, vmxPath) < 0) { + virReportOOMError(); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(directoryName); + VIR_FREE(fileName); + return ret; +} + +int +vmwareMoveFile(char *srcFile, char *dstFile) +{ + const char *cmdmv[] = + { "mv", PROGRAM_SENTINAL, PROGRAM_SENTINAL, NULL }; + + if (!virFileExists(srcFile)) { + vmwareError(VIR_ERR_INTERNAL_ERROR, _("file %s does not exist"), + srcFile); + return -1; + } + + if (STREQ(srcFile, dstFile)) + return 0; + + vmwareSetSentinal(cmdmv, srcFile); + vmwareSetSentinal(cmdmv, dstFile); + if (virRun(cmdmv, NULL) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("failed to move file to %s "), dstFile); + return -1; + } + + return 0; +} + +int +vmwareMakePath(char *srcDir, char *srcName, char *srcExt, char **outpath) +{ + if (virAsprintf(outpath, "%s/%s.%s", srcDir, srcName, srcExt) < 0) { + virReportOOMError(); + return -1; + } + return 0; +} +
diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c new file mode 100644 index 0000000..5615bef --- /dev/null +++ b/src/vmware/vmware_driver.c
+static virDrvOpenStatus +vmwareOpen(virConnectPtr conn, + virConnectAuthPtr auth ATTRIBUTE_UNUSED, + int flags ATTRIBUTE_UNUSED) +{ + struct vmware_driver *driver; + + if (conn->uri == NULL) { + /* @TODO accept */ + return VIR_DRV_OPEN_DECLINED; + } else { + if (conn->uri->scheme == NULL || + (STRNEQ(conn->uri->scheme, "vmwareplayer") && + STRNEQ(conn->uri->scheme, "vmwarews"))) + return VIR_DRV_OPEN_DECLINED; + + /* If server name is given, its for remote driver */ + if (conn->uri->server != NULL) + return VIR_DRV_OPEN_DECLINED; + + /* If path isn't /session, then they typoed, so tell them correct path */ + if (conn->uri->path == NULL || STRNEQ(conn->uri->path, "/session")) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + ("unexpected VMware URI path '%s', try vmwareplayer:///session or vmwarews:///session"), + conn->uri->path);
Should be _() instead of (). Add vmwareError to the list of msg_gen_function's in cfg.mk to let "make syntax-check" verify this. Also you might feed a NULL string for %s, that's not save. Use NULLSTR(conn->uri->path) here in the vmwareError call, or reorder the testing logic.
+ return VIR_DRV_OPEN_ERROR; + } + } + + /* We now know the URI is definitely for this driver, so beyond + * here, don't return DECLINED, always use ERROR */
You should probably check early if vmrun is available at all, For example like this. char *vmrun = virFindFileInPath(VMRUN); if (vmrun == NULL) { vmwareError(VIR_ERR_INTERNAL_ERROR, _("%s utility is missing"), VMRUN); return VIR_DRV_OPEN_ERROR; } else { VIR_FREE(vmrun); }
+ if (VIR_ALLOC(driver) < 0) { + virReportOOMError(); + return VIR_DRV_OPEN_ERROR; + }
+static const char * +vmwareGetType(virConnectPtr conn ATTRIBUTE_UNUSED)
conn is not unused here, so you don't need to mark it as such.
+{ + struct vmware_driver *driver = conn->privateData; + int type; + + vmwareDriverLock(driver); + type = driver->type; + vmwareDriverUnlock(driver); + return type == TYPE_PLAYER ? "vmware player" : "vmware workstation"; +}
+static virDomainPtr +vmwareDomainDefineXML(virConnectPtr conn, const char *xml) +{ + struct vmware_driver *driver = conn->privateData; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; + virDomainPtr dom = NULL; + char *vmx = NULL; + char *directoryName = NULL; + char *fileName = NULL; + char *vmxPath = NULL; + vmwareDomainPtr pDomain = NULL; + esxVMX_Context ctx; + + ctx.formatFileName = esxCopyVMXFileName; + + vmwareDriverLock(driver); + if ((vmdef = virDomainDefParseString(driver->caps, xml, + VIR_DOMAIN_XML_INACTIVE)) == NULL) + goto cleanup; + + vm = virDomainFindByName(&driver->domains, vmdef->name); + if (vm) { + vmwareError(VIR_ERR_OPERATION_FAILED, + _("Already an VMWARE VM active with the id '%s'"), + vmdef->name); + goto cleanup; + } + + /* generate vmx file */ + vmx = esxVMX_FormatConfig(&ctx, driver->caps, vmdef, + esxVI_ProductVersion_ESX4x); + if (vmx == NULL) { + vmwareError(VIR_ERR_OPERATION_FAILED, _("cannot create xml"));
Don't report your own error here, esxVMX_FormatConfig did this already when it returns NULL. Your error message will overwrite the more detailed error message reported by esxVMX_FormatConfig.
+ goto cleanup; + } + + if (vmwareVmxPath(vmdef, &vmxPath) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("retrieve vmx path.. failed")); + goto cleanup; + } + + /* create vmx file */ + if (virFileWriteStr(vmxPath, vmx) < 0) {
This will need the 3. argument for the extended virFileWriteStr function.
+ vmwareError(VIR_ERR_INTERNAL_ERROR, + _("Failed to write vmx file '%s'"), vmxPath); + goto cleanup; + }
+static int +vmwareDomainSuspend(virDomainPtr dom) +{ + struct vmware_driver *driver = dom->conn->privateData; + + virDomainObjPtr vm; + const char *cmd[] = { + VMRUN, "-T", PROGRAM_SENTINAL, "pause", + PROGRAM_SENTINAL, NULL + }; + int ret = -1; + + if (driver->type == TYPE_PLAYER) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmplayer does not support libvirt suspend/resume" + " (vmware pause/unpause) operation ")); + return ret; + } + + vmwareDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + vmwareDriverUnlock(driver); + + if (!vm) { + vmwareError(VIR_ERR_INVALID_DOMAIN, "%s", + _("no domain with matching uuid")); + goto cleanup; + } + + vmwareSetSentinal(cmd, vmw_types[driver->type]);
All access to the driver struct is made with the driver lock held, expect this one. Actually accessing driver->type unlocked should be okay, as driver->type is set in the open function and then only read. In some functions you take the lock to access driver->type in some you don't. Choose one way and then be consistent.
+static int +vmwareDomainReboot(virDomainPtr dom, unsigned int flags ATTRIBUTE_UNUSED) +{ + struct vmware_driver *driver = dom->conn->privateData; + + virDomainObjPtr vm; + const char *cmd[] = { + VMRUN, "-T", PROGRAM_SENTINAL, + "reset", PROGRAM_SENTINAL, NULL + };
virDomainReboot is meant to reboot the guest OS as in executing the reboot command from inside the guest OS. It's not meant to do a hardware level reset as in pressing the reset button of an actual computer. I'm not sure "vmrun reset" is the right thing here.
+static int +vmwareDomainSave(virDomainPtr dom, const char *path) +{ + struct vmware_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + const char *cmdsuspend[] = { + VMRUN, "-T", PROGRAM_SENTINAL, + "suspend", PROGRAM_SENTINAL, NULL + }; + int ret = -1; + char *fDirectoryName = NULL; + char *fFileName = NULL; + char *tDirectoryName = NULL; + char *tFileName = NULL; + char *copyPath = NULL; + char *copyvmxPath = NULL; + char *fvmss = NULL; + char *tvmss = NULL; + char *fvmem = NULL; + char *tvmem = NULL; + char *fvmx = NULL; + char *tvmx = NULL; + + vmwareDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + vmwareDriverUnlock(driver); + + if (!vm) { + vmwareError(VIR_ERR_INVALID_DOMAIN, "%s", + _("no domain with matching uuid")); + goto cleanup; + } + + /* vmware suspend */ + vmwareSetSentinal(cmdsuspend, vmw_types[driver->type]); + vmwareSetSentinal(cmdsuspend, + ((vmwareDomainPtr) vm->privateData)->vmxPath); + + if (vm->state != VIR_DOMAIN_RUNNING) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("domain is not in running state")); + goto cleanup; + } + + if (virRun(cmdsuspend, NULL) < 0) + goto cleanup; + + /* create files path */ + copyvmxPath = strdup(((vmwareDomainPtr) vm->privateData)->vmxPath); + if(copyvmxPath == NULL) { + virReportOOMError(); + goto cleanup; + } + + if((copyPath = strdup(path)) == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (vmwareParsePath(copyvmxPath, &fDirectoryName, &fFileName) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse vmxPath"));
Don't report and error here, vmwareParsePath does this already.
+ goto cleanup; + } + if (vmwareParsePath(copyPath, &tDirectoryName, &tFileName) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse path"));
Don't report and error here, vmwareParsePath does this already.
+ goto cleanup; + } + + /* dir */ + if (virFileMakePath(tDirectoryName) != 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", _("make path error")); + goto cleanup; + } + + vmwareMakePath(fDirectoryName, vm->def->name, (char *) "vmss", &fvmss); + vmwareMakePath(tDirectoryName, vm->def->name, (char *) "vmss", &tvmss); + vmwareMakePath(fDirectoryName, vm->def->name, (char *) "vmem", &fvmem); + vmwareMakePath(tDirectoryName, vm->def->name, (char *) "vmem", &tvmem); + vmwareMakePath(fDirectoryName, vm->def->name, (char *) "vmx", &fvmx); + vmwareMakePath(tDirectoryName, vm->def->name, (char *) "vmx", &tvmx);
The the return values of all this vmwareMakePath calls.
+ /* create linker file */ + int fd; + + if ((fd = + open(path, O_CREAT | O_WRONLY | O_TRUNC, + S_IRUSR | S_IWUSR)) == -1) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open linker file '%s'"), path); + goto cleanup; + } + if (safewrite + (fd, ((vmwareDomainPtr) vm->privateData)->vmxPath, + strlen(((vmwareDomainPtr) vm->privateData)->vmxPath)) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create linker file")); + goto cleanup; + } + + /* we want to let saved VM inside default directory */ + if (STREQ(fDirectoryName, tDirectoryName)) + goto end; + + /* move {vmx,vmss,vmem} files */ + if ((vmwareMoveFile(fvmss, tvmss) < 0) + || (vmwareMoveFile(fvmem, tvmem) < 0) + || (vmwareMoveFile(fvmx, tvmx) < 0) + ) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to move file"));
Don't report and error here, vmwareMoveFile does this already.
+ goto cleanup; + } + + end: + vm->def->id = -1; + vm->state = VIR_DOMAIN_SHUTOFF; + ret = 0; + + cleanup: + VIR_FREE(fDirectoryName); + VIR_FREE(fFileName); + VIR_FREE(tDirectoryName); + VIR_FREE(tFileName); + VIR_FREE(copyPath); + VIR_FREE(copyvmxPath); + + VIR_FREE(fvmss); + VIR_FREE(tvmss); + VIR_FREE(fvmem); + VIR_FREE(tvmem); + VIR_FREE(fvmx); + VIR_FREE(tvmx); + if (vm) + virDomainObjUnlock(vm); + return ret; +} + +static int +vmwareDomainRestore(virConnectPtr conn, const char *path) +{ + struct vmware_driver *driver = conn->privateData; + virDomainDefPtr vmdef = NULL; + virDomainPtr dom = NULL; + virDomainObjPtr vm = NULL; + int ret = -1; + char *vmxPath = NULL; + char *copypath = NULL; + char *copyvmxPath = NULL; + char *tDirectoryName = NULL; + char *tFileName = NULL; + char *fDirectoryName = NULL; + char *fFileName = NULL; + char *vmx = NULL; + char *xml = NULL; + char *sep = NULL; + char *baseVmss; + char *fvmss = NULL; + char *tvmss = NULL; + char *fvmem = NULL; + char *tvmem = NULL; + char *fvmx = NULL; + char *tvmx = NULL; + esxVMX_Context ctx; + + ctx.parseFileName = esxCopyVMXFileName; + + if (!virFileExists(path)) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("file %s does not exist"), path); + goto cleanup; + } + if (virFileHasSuffix(path, ".vmx")) { //we want to restore a vm saved in its default directory + if((tvmx = strdup(path)) == NULL) { + virReportOOMError(); + goto cleanup; + } + + if((copypath = strdup(path)) == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (vmwareParsePath(copypath, &fDirectoryName, &fFileName) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse path"));
Don't report and error here, vmwareParsePath does this already.
+ goto cleanup; + } + sep = strrchr(fFileName, '.'); + if (sep != NULL) + *sep = '\0'; + vmwareMakePath(fFileName, fFileName, (char *) "vmss", &fvmss);
You should check the return value of vmwareMakePath
+ baseVmss = basename(fvmss); + } else { //we want to restore a vm saved elsewhere + if (virFileReadAll(path, 1024, &vmxPath) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to read vmxPath")); + goto cleanup; + } + + if (!virFileHasSuffix(vmxPath, ".vmx")) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("%s is not a .vmx file"), vmxPath); + goto cleanup; + } + + /* move files */ + if((copyvmxPath = strdup(vmxPath)) == NULL) { + virReportOOMError(); + goto cleanup; + } + + if((copypath = strdup(path)) == NULL) { + virReportOOMError(); + goto cleanup; + } + + if (vmwareParsePath(copypath, &fDirectoryName, &fFileName) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse path"));
Don't report and error here, vmwareMoveFile does this already.
+ goto cleanup; + } + if (vmwareParsePath(copyvmxPath, &tDirectoryName, &tFileName) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to parse vmxPath"));
Don't report and error here, vmwareMoveFile does this already.
+ goto cleanup; + } + + sep = strrchr(tFileName, '.'); + if (sep != NULL) + *sep = '\0'; + + if (virFileMakePath(tDirectoryName) != 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("make path error")); + goto cleanup; + } + + vmwareMakePath(fDirectoryName, tFileName, (char *) "vmss", &fvmss); + vmwareMakePath(tDirectoryName, tFileName, (char *) "vmss", &tvmss); + vmwareMakePath(fDirectoryName, tFileName, (char *) "vmem", &fvmem); + vmwareMakePath(tDirectoryName, tFileName, (char *) "vmem", &tvmem); + vmwareMakePath(fDirectoryName, tFileName, (char *) "vmx", &fvmx); + vmwareMakePath(tDirectoryName, tFileName, (char *) "vmx", &tvmx); + + if ((vmwareMoveFile(fvmss, tvmss) < 0) + || (vmwareMoveFile(fvmem, tvmem) < 0) + || (vmwareMoveFile(fvmx, tvmx) < 0) + ) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to move files"));
Don't report and error here, vmwareMoveFile does this already.
+ goto cleanup; + } + + baseVmss = basename(tvmss); + } + + if (virFileReadAll(tvmx, 10000, &vmx) == -1) { + perror("error reading vmx file"); + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("failed to read vmx file %s"), tvmx); + goto cleanup; + } + + vmwareDriverLock(driver); + if ((vmdef = + esxVMX_ParseConfig(&ctx, driver->caps, vmx, + esxVI_ProductVersion_ESX4x)) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse config file"));
Don't report and error here, esxVMX_ParseConfig does this already.
+ goto cleanup; + } + vmwareDriverUnlock(driver); + + xml = virDomainDefFormat(vmdef, VIR_DOMAIN_XML_SECURE); + + if ((dom = vmwareDomainDefineXML(conn, xml)) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to define domain"));
Don't report an error here, vmwareDomainDefineXML does this already.
+ goto cleanup; + } + + /* esxVMX_ParseConfig don't care about vmx checkpoint property for now, so we add it here + * TODO + */ + FILE *pFile = NULL; + + if ((pFile = fopen(tvmx, "a+")) == NULL) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to reopen vmx file")); + goto cleanup; + } + + fputs("checkpoint.vmState = \"", pFile); + fputs(baseVmss, pFile); + fputs("\"", pFile); + fclose(pFile); + + vmwareDriverLock(driver); + vm = virDomainFindByName(&driver->domains, dom->name); + + if (!vm) { + vmwareError(VIR_ERR_INVALID_DOMAIN, "%s", + _("no domain with matching id")); + goto cleanup; + } + + /* start */ + if (vmwareStartVM(driver, vm) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, "%s", + _("failed to create domain"));
Don't report an error here, vmwareStartVM does this already.
+ goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(vmxPath); + VIR_FREE(copypath); + VIR_FREE(copyvmxPath); + VIR_FREE(tDirectoryName); + VIR_FREE(tFileName); + VIR_FREE(fDirectoryName); + VIR_FREE(fFileName); + VIR_FREE(vmx); + VIR_FREE(xml); + + VIR_FREE(fvmss); + VIR_FREE(tvmss); + VIR_FREE(fvmem); + VIR_FREE(tvmem); + VIR_FREE(fvmx); + VIR_FREE(tvmx); + + if (dom) + virUnrefDomain(dom); + if (vm) + virDomainObjUnlock(vm); + vmwareDriverUnlock(driver); + return ret; +} + +static virDomainPtr +vmwareDomainCreateXML(virConnectPtr conn, const char *xml, + unsigned int flags ATTRIBUTE_UNUSED) +{ + struct vmware_driver *driver = conn->privateData; + virDomainDefPtr vmdef = NULL; + virDomainObjPtr vm = NULL; + virDomainPtr dom = NULL; + char *vmx = NULL; + char *vmxPath = NULL; + vmwareDomainPtr pDomain = NULL; + esxVMX_Context ctx; + + ctx.formatFileName = esxCopyVMXFileName; + + vmwareDriverLock(driver); + + if ((vmdef = virDomainDefParseString(driver->caps, xml, + VIR_DOMAIN_XML_INACTIVE)) == NULL) + goto cleanup; + + vm = virDomainFindByName(&driver->domains, vmdef->name); + if (vm) { + vmwareError(VIR_ERR_OPERATION_FAILED, + _("Already an VMWARE VM active with the id '%s'"), + vmdef->name); + goto cleanup; + } + + /* generate vmx file */ + vmx = esxVMX_FormatConfig(&ctx, driver->caps, vmdef, + esxVI_ProductVersion_ESX4x); + if (vmx == NULL) { + vmwareError(VIR_ERR_OPERATION_FAILED, _("cannot create xml")); + goto cleanup; + } + + if (vmwareVmxPath(vmdef, &vmxPath) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("retrieve vmx path.. failed")); + goto cleanup; + } + + /* create vmx file */ + if (virFileWriteStr(vmxPath, vmx) < 0) { + vmwareError(VIR_ERR_INTERNAL_ERROR, + _("Failed to write vmx file '%s'"), vmxPath); + goto cleanup; + } + + /* assign def */ + if (!(vm = virDomainAssignDef(driver->caps, + &driver->domains, vmdef, false))) + goto cleanup; + + pDomain = vm->privateData; + pDomain->vmxPath = strdup(vmxPath); + + vmwareDomainConfigDisplay(pDomain, vmdef); + vmdef = NULL; + + if (vmwareStartVM(driver, vm) < 0) { + virDomainRemoveInactive(&driver->domains, vm); + vm = NULL; + goto cleanup; + } + + dom = virGetDomain(conn, vm->def->name, vm->def->uuid); + if (dom) + dom->id = vm->def->id; + +cleanup: + virDomainDefFree(vmdef); + VIR_FREE(vmx); + VIR_FREE(vmxPath); + if(vm) + virDomainObjUnlock(vm); + vmwareDriverUnlock(driver); + return dom; +}
+static int +vmwareDomainUndefine(virDomainPtr dom) +{ + struct vmware_driver *driver = dom->conn->privateData; + virDomainObjPtr vm; + int ret = -1; + + vmwareDriverLock(driver); + vm = virDomainFindByUUID(&driver->domains, dom->uuid); + + if (!vm) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(dom->uuid, uuidstr); + vmwareError(VIR_ERR_NO_DOMAIN, + _("no domain with matching uuid '%s'"), uuidstr); + goto cleanup; + } + + if (virDomainObjIsActive(vm)) { + vmwareError(VIR_ERR_OPERATION_INVALID, + "%s", _("cannot delete active domain"));
s/delete/undefine/ At the end of my review I noticed that you overwrite already reported errors in many places. I only commented on some of them so you should check for this in the whole driver. I think the next version of this driver will be ready to be added to the official codebase :) Matthias