The VMware driver supports multiple backends for the VMware Player and VMware Workstation, convert this logic into enum and use VIR_ENUM_IMPL() to provide conversions to and from strings. --- src/vmware/vmware_conf.c | 11 ++++++++--- src/vmware/vmware_conf.h | 11 +++++++++-- src/vmware/vmware_driver.c | 21 ++++++++++----------- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index e00c66c..3bb5610 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -35,6 +35,10 @@ #include "vmware_conf.h" #include "virstring.h" +VIR_ENUM_IMPL(vmwareDriver, VMWARE_DRIVER_LAST, + "player", + "ws"); + /* Free all memory associated with a vmware_driver structure */ void vmwareFreeDriver(struct vmware_driver *driver) @@ -141,7 +145,7 @@ vmwareLoadDomains(struct vmware_driver *driver) ctx.parseFileName = vmwareCopyVMXFileName; cmd = virCommandNewArgList(VMRUN, "-T", - driver->type == TYPE_PLAYER ? "player" : "ws", + vmwareDriverTypeToString(driver->type), "list", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -221,8 +225,9 @@ vmwareExtractVersion(struct vmware_driver *driver) int ret = -1; virCommandPtr cmd; char * outbuf = NULL; - const char * bin = (driver->type == TYPE_PLAYER) ? "vmplayer" : "vmware"; - const char * pattern = (driver->type == TYPE_PLAYER) ? + const char * bin = (driver->type == VMWARE_DRIVER_PLAYER) ? + "vmplayer" : "vmware"; + const char * pattern = (driver->type == VMWARE_DRIVER_PLAYER) ? "VMware Player " : "VMware Workstation "; cmd = virCommandNewArgList(bin, "-v", NULL); diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index 6666eaf..978f22b 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -1,5 +1,6 @@ /*---------------------------------------------------------------------------*/ /* Copyright 2010, diateam (www.diateam.net) + * Copyright (c) 2013, Doug Goldstein (cardoe@cardoe.com) * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -30,8 +31,14 @@ # define VIR_FROM_THIS VIR_FROM_VMWARE # define PROGRAM_SENTINEL ((char *)0x1) -# define TYPE_PLAYER 0 -# define TYPE_WORKSTATION 1 +enum vmwareDriverType { + VMWARE_DRIVER_PLAYER = 0, /* VMware Player */ + VMWARE_DRIVER_WORKSTATION = 1, /* VMware Workstation */ + + VMWARE_DRIVER_LAST, /* required last item */ +}; + +VIR_ENUM_DECL(vmwareDriver) struct vmware_driver { virMutex lock; diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 74591d6..9d674b6 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -35,8 +35,6 @@ #include "vmware_driver.h" #include "virstring.h" -static const char *vmw_types[] = { "player", "ws" }; - static void vmwareDriverLock(struct vmware_driver *driver) { @@ -133,7 +131,7 @@ vmwareConnectOpen(virConnectPtr conn, goto cleanup; driver->type = STRNEQ(conn->uri->scheme, "vmwareplayer") ? - TYPE_WORKSTATION : TYPE_PLAYER; + VMWARE_DRIVER_WORKSTATION : VMWARE_DRIVER_PLAYER; if (!(driver->domains = virDomainObjListNew())) goto cleanup; @@ -202,7 +200,8 @@ vmwareUpdateVMStatus(struct vmware_driver *driver, virDomainObjPtr vm) int newState; int ret = -1; - cmd = virCommandNewArgList(VMRUN, "-T", vmw_types[driver->type], + cmd = virCommandNewArgList(VMRUN, "-T", + vmwareDriverTypeToString(driver->type), "list", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) @@ -256,7 +255,7 @@ vmwareStopVM(struct vmware_driver *driver, PROGRAM_SENTINEL, "soft", NULL }; - vmwareSetSentinal(cmd, vmw_types[driver->type]); + vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); if (virRun(cmd, NULL) < 0) { @@ -284,7 +283,7 @@ vmwareStartVM(struct vmware_driver *driver, virDomainObjPtr vm) return -1; } - vmwareSetSentinal(cmd, vmw_types[driver->type]); + vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); vmwareSetSentinal(cmd, vmxPath); if (!((vmwareDomainPtr) vm->privateData)->gui) vmwareSetSentinal(cmd, NOGUI); @@ -451,7 +450,7 @@ vmwareDomainSuspend(virDomainPtr dom) }; int ret = -1; - if (driver->type == TYPE_PLAYER) { + if (driver->type == VMWARE_DRIVER_PLAYER) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vmplayer does not support libvirt suspend/resume" " (vmware pause/unpause) operation ")); @@ -468,7 +467,7 @@ vmwareDomainSuspend(virDomainPtr dom) goto cleanup; } - vmwareSetSentinal(cmd, vmw_types[driver->type]); + vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -500,7 +499,7 @@ vmwareDomainResume(virDomainPtr dom) }; int ret = -1; - if (driver->type == TYPE_PLAYER) { + if (driver->type == VMWARE_DRIVER_PLAYER) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("vmplayer does not support libvirt suspend/resume " "(vmware pause/unpause) operation ")); @@ -517,7 +516,7 @@ vmwareDomainResume(virDomainPtr dom) goto cleanup; } - vmwareSetSentinal(cmd, vmw_types[driver->type]); + vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); vmwareSetSentinal(cmd, ((vmwareDomainPtr) vm->privateData)->vmxPath); if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_PAUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -562,7 +561,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) } vmxPath = ((vmwareDomainPtr) vm->privateData)->vmxPath; - vmwareSetSentinal(cmd, vmw_types[driver->type]); + vmwareSetSentinal(cmd, vmwareDriverTypeToString(driver->type)); vmwareSetSentinal(cmd, vmxPath); if (vmwareUpdateVMStatus(driver, vm) < 0) -- 1.8.1.5

Rather than looking up the path to vmrun each time we call it, look it up once and save it. This sets up the ability for us to detect where the path is on Mac OS X and not have to look it up each time we execute it. --- src/vmware/vmware_conf.c | 3 ++- src/vmware/vmware_conf.h | 2 +- src/vmware/vmware_driver.c | 46 ++++++++++++++++++++++++++++++---------------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 3bb5610..b33bddc 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -50,6 +50,7 @@ vmwareFreeDriver(struct vmware_driver *driver) virObjectUnref(driver->domains); virObjectUnref(driver->caps); virObjectUnref(driver->xmlopt); + VIR_FREE(driver->vmrun); VIR_FREE(driver); } @@ -144,7 +145,7 @@ vmwareLoadDomains(struct vmware_driver *driver) ctx.parseFileName = vmwareCopyVMXFileName; - cmd = virCommandNewArgList(VMRUN, "-T", + cmd = virCommandNewArgList(driver->vmrun, "-T", vmwareDriverTypeToString(driver->type), "list", NULL); virCommandSetOutputBuffer(cmd, &outbuf); diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index 978f22b..4b99d8e 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -21,7 +21,6 @@ #ifndef VMWARE_CONF_H # define VMWARE_CONF_H -# define VMRUN "vmrun" # define NOGUI "nogui" # include "internal.h" @@ -48,6 +47,7 @@ struct vmware_driver { virDomainObjListPtr domains; int version; int type; + char *vmrun; }; typedef struct _vmwareDomain { diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 9d674b6..4e56971 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -35,6 +35,13 @@ #include "vmware_driver.h" #include "virstring.h" +/* Various places we may find the "vmrun" binary, + * without a leading / it will be searched in PATH + */ +static const char * const vmrun_candidates[] = { + "vmrun", +}; + static void vmwareDriverLock(struct vmware_driver *driver) { @@ -85,7 +92,7 @@ vmwareConnectOpen(virConnectPtr conn, unsigned int flags) { struct vmware_driver *driver; - char * vmrun = NULL; + size_t i; virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR); @@ -114,18 +121,25 @@ vmwareConnectOpen(virConnectPtr conn, /* We now know the URI is definitely for this driver, so beyond * here, don't return DECLINED, always use ERROR */ - vmrun = virFindFileInPath(VMRUN); - - if (vmrun == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("%s utility is missing"), VMRUN); + if (VIR_ALLOC(driver) < 0) return VIR_DRV_OPEN_ERROR; - } else { - VIR_FREE(vmrun); + + /* Find vmrun, which is what this driver uses to communicate to + * the VMware hypervisor. We look this up first since we use it + * for auto detection of the backend + */ + for (i = 0; i < ARRAY_CARDINALITY(vmrun_candidates); i++) { + driver->vmrun = virFindFileInPath(vmrun_candidates[i]); + /* If we found one, we can stop looking */ + if (driver->vmrun) + break; } - if (VIR_ALLOC(driver) < 0) - return VIR_DRV_OPEN_ERROR; + if (driver->vmrun == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vmrun utility is missing")); + goto cleanup; + } if (virMutexInit(&driver->lock) < 0) goto cleanup; @@ -200,7 +214,7 @@ vmwareUpdateVMStatus(struct vmware_driver *driver, virDomainObjPtr vm) int newState; int ret = -1; - cmd = virCommandNewArgList(VMRUN, "-T", + cmd = virCommandNewArgList(driver->vmrun, "-T", vmwareDriverTypeToString(driver->type), "list", NULL); virCommandSetOutputBuffer(cmd, &outbuf); @@ -251,7 +265,7 @@ vmwareStopVM(struct vmware_driver *driver, virDomainShutoffReason reason) { const char *cmd[] = { - VMRUN, "-T", PROGRAM_SENTINEL, "stop", + driver->vmrun, "-T", PROGRAM_SENTINEL, "stop", PROGRAM_SENTINEL, "soft", NULL }; @@ -272,7 +286,7 @@ static int vmwareStartVM(struct vmware_driver *driver, virDomainObjPtr vm) { const char *cmd[] = { - VMRUN, "-T", PROGRAM_SENTINEL, "start", + driver->vmrun, "-T", PROGRAM_SENTINEL, "start", PROGRAM_SENTINEL, PROGRAM_SENTINEL, NULL }; const char *vmxPath = ((vmwareDomainPtr) vm->privateData)->vmxPath; @@ -445,7 +459,7 @@ vmwareDomainSuspend(virDomainPtr dom) virDomainObjPtr vm; const char *cmd[] = { - VMRUN, "-T", PROGRAM_SENTINEL, "pause", + driver->vmrun, "-T", PROGRAM_SENTINEL, "pause", PROGRAM_SENTINEL, NULL }; int ret = -1; @@ -494,7 +508,7 @@ vmwareDomainResume(virDomainPtr dom) virDomainObjPtr vm; const char *cmd[] = { - VMRUN, "-T", PROGRAM_SENTINEL, "unpause", PROGRAM_SENTINEL, + driver->vmrun, "-T", PROGRAM_SENTINEL, "unpause", PROGRAM_SENTINEL, NULL }; int ret = -1; @@ -543,7 +557,7 @@ vmwareDomainReboot(virDomainPtr dom, unsigned int flags) const char * vmxPath = NULL; virDomainObjPtr vm; const char *cmd[] = { - VMRUN, "-T", PROGRAM_SENTINEL, + driver->vmrun, "-T", PROGRAM_SENTINEL, "reset", PROGRAM_SENTINEL, "soft", NULL }; int ret = -1; -- 1.8.1.5

This splits up the version parsing code into a callable API like QEMU help/version string parsing so that we can test it as we need to add additional patterns for newer versions/products. --- .gitignore | 1 + src/Makefile.am | 6 ++ src/libvirt_vmware.syms | 12 ++++ src/vmware/vmware_conf.c | 49 ++++++++++---- src/vmware/vmware_conf.h | 2 + tests/Makefile.am | 14 ++++ tests/vmwareverdata/workstation-7.0.0.txt | 1 + tests/vmwarevertest.c | 105 ++++++++++++++++++++++++++++++ 8 files changed, 176 insertions(+), 14 deletions(-) create mode 100644 src/libvirt_vmware.syms create mode 100644 tests/vmwareverdata/workstation-7.0.0.txt create mode 100644 tests/vmwarevertest.c diff --git a/.gitignore b/.gitignore index d70d0dd..edd5d26 100644 --- a/.gitignore +++ b/.gitignore @@ -217,6 +217,7 @@ /tests/virsystemdtest /tests/virtimetest /tests/viruritest +/tests/vmwarevertest /tests/vmx2xmltest /tests/xencapstest /tests/xmconfigtest diff --git a/src/Makefile.am b/src/Makefile.am index 1388c5f..4375ef7 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1768,6 +1768,12 @@ else ! WITH_VMX SYM_FILES += $(srcdir)/libvirt_vmx.syms endif ! WITH_VMX +if WITH_VMWARE +USED_SYM_FILES += $(srcdir)/libvirt_vmware.syms +else ! WITH_VMWARE +SYM_FILES += $(srcdir)/libvirt_vmware.syms +endif ! WITH_VMWARE + if WITH_XENXS USED_SYM_FILES += $(srcdir)/libvirt_xenxs.syms else ! WITH_XENXS diff --git a/src/libvirt_vmware.syms b/src/libvirt_vmware.syms new file mode 100644 index 0000000..c64a7d5 --- /dev/null +++ b/src/libvirt_vmware.syms @@ -0,0 +1,12 @@ +# +# These symbols are dependent upon --with-vmware via WITH_VMWARE. +# + +# vmware/vmware_conf.h +vmwareDriverTypeFromString; +vmwareParseVersionStr; + +# Let emacs know we want case-insensitive sorting +# Local Variables: +# sort-fold-case: t +# End: diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index b33bddc..261a4f6 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -219,17 +219,48 @@ vmwareSetSentinal(const char **prog, const char *key) } int +vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) +{ + const char *pattern; + const char *tmp; + + switch (type) { + case VMWARE_DRIVER_PLAYER: + pattern = "VMware Player "; + break; + case VMWARE_DRIVER_WORKSTATION: + pattern = "VMware Workstation "; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid driver type: %d"), type); + return -1; + } + + if ((tmp = STRSKIP(verbuf, pattern)) == NULL) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("failed to parse %sversion"), pattern); + return -1; + } + + if (virParseVersionString(tmp, version, false) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("version parsing error")); + return -1; + } + + return 0; +} + +int vmwareExtractVersion(struct vmware_driver *driver) { unsigned long version = 0; - char *tmp; int ret = -1; virCommandPtr cmd; char * outbuf = NULL; const char * bin = (driver->type == VMWARE_DRIVER_PLAYER) ? "vmplayer" : "vmware"; - const char * pattern = (driver->type == VMWARE_DRIVER_PLAYER) ? - "VMware Player " : "VMware Workstation "; cmd = virCommandNewArgList(bin, "-v", NULL); virCommandSetOutputBuffer(cmd, &outbuf); @@ -237,19 +268,9 @@ vmwareExtractVersion(struct vmware_driver *driver) if (virCommandRun(cmd, NULL) < 0) goto cleanup; - if ((tmp = STRSKIP(outbuf, pattern)) == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("failed to parse %s version"), bin); + if (vmwareParseVersionStr(driver->type, outbuf, &version) < 0) goto cleanup; - } - - if (virParseVersionString(tmp, &version, false) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("version parsing error")); - goto cleanup; - } - driver->version = version; ret = 0; cleanup: diff --git a/src/vmware/vmware_conf.h b/src/vmware/vmware_conf.h index 4b99d8e..a4fd855 100644 --- a/src/vmware/vmware_conf.h +++ b/src/vmware/vmware_conf.h @@ -66,6 +66,8 @@ void vmwareSetSentinal(const char **prog, const char *key); int vmwareExtractVersion(struct vmware_driver *driver); +int vmwareParseVersionStr(int type, const char *buf, unsigned long *version); + int vmwareDomainConfigDisplay(vmwareDomainPtr domain, virDomainDefPtr vmdef); int vmwareParsePath(char *path, char **directory, char **filename); diff --git a/tests/Makefile.am b/tests/Makefile.am index b1d6dab..fe36810 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -107,6 +107,7 @@ EXTRA_DIST = \ xmconfigdata \ xml2sexprdata \ xml2vmxdata \ + vmwareverdata \ .valgrind.supp test_helpers = commandhelper ssh test_conf @@ -188,6 +189,10 @@ if WITH_VMX test_programs += vmx2xmltest xml2vmxtest endif WITH_VMX +if WITH_VMWARE +test_programs += vmwarevertest +endif WITH_VMWARE + if WITH_CIL test_programs += object-locking endif WITH_CIL @@ -507,6 +512,15 @@ else ! WITH_VMX EXTRA_DIST += vmx2xmltest.c xml2vmxtest.c endif ! WITH_VMX +if WITH_VMWARE +vmwarevertest_SOURCES = \ + vmwarevertest.c \ + testutils.c testutils.h +vmwarevertest_LDADD = $(LDADDS) +else ! WITH_VMWARE +EXTRA_DIST += vmwarevertest.c +endif ! WITH_VMWARE + networkxml2xmltest_SOURCES = \ networkxml2xmltest.c \ testutils.c testutils.h diff --git a/tests/vmwareverdata/workstation-7.0.0.txt b/tests/vmwareverdata/workstation-7.0.0.txt new file mode 100644 index 0000000..207b71c --- /dev/null +++ b/tests/vmwareverdata/workstation-7.0.0.txt @@ -0,0 +1 @@ +VMware Workstation 7.0.0 build-203739 Release diff --git a/tests/vmwarevertest.c b/tests/vmwarevertest.c new file mode 100644 index 0000000..f5ccb06 --- /dev/null +++ b/tests/vmwarevertest.c @@ -0,0 +1,105 @@ +/* + * Copyright (c) 2013. Doug Goldstein <cardoe@cardoe.com> + * + * 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, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "testutils.h" + +#ifdef WITH_VMWARE + +# include <stdio.h> +# include <stdlib.h> + +# include "vmware/vmware_conf.h" + +//# define VIR_FROM_THIS VIR_FROM_NONE + +struct testInfo { + const char *vmware_type; + const char *name; + unsigned long version; +}; + +static int +testVerStrParse(const void *data) +{ + const struct testInfo *info = data; + int ret = -1; + char *path = NULL; + char *databuf = NULL; + unsigned long version; + int vmware_type; + + if (virAsprintf(&path, "%s/vmwareverdata/%s.txt", abs_srcdir, + info->name) < 0) + return -1; + + if (virtTestLoadFile(path, &databuf) < 0) + goto cleanup; + + if ((vmware_type = vmwareDriverTypeFromString(info->vmware_type)) < 0) + goto cleanup; + + if (vmwareParseVersionStr(vmware_type, databuf, &version) < 0) + goto cleanup; + + if (version != info->version) { + fprintf(stderr, "%s: parsed versions do not match: got %lu, " + "expected %lu\n", info->name, version, info->version); + goto cleanup; + } + + ret = 0; + +cleanup: + VIR_FREE(path); + VIR_FREE(databuf); + return ret; +} + +static int +mymain(void) +{ + int ret = 0; + +# define DO_TEST(vmware_type, name, version) \ + do { \ + struct testInfo info = { \ + vmware_type, name, version \ + }; \ + if (virtTestRun("VMware Version String Parsing " name, \ + 1, testVerStrParse, &info) < 0) \ + ret = -1; \ + } while (0) + + DO_TEST("ws", "workstation-7.0.0", 7000000); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) + +#else + +int +main(void) +{ + return EXIT_AM_SKIP; +} + +#endif /* WITH_VMWARE */ -- 1.8.1.5