2010/12/2 Jean-Baptiste Rouault <jean-baptiste.rouault@xxxxxxxxxxx>:
> ---
> Â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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list