[libvirt] [vmware] Fix VMware driver version checking on Linux
Recent changes to the VMware driver allow for VMware Fusion version checking on OS X. It looks like the version checking on Linux for VMware Workstation and Player could possibly be broken by the changes. The version check in vmware_conf.c bundled STDOUT and STDERR into a common buffer; when/if something is printed to STDERR, at least on Player, version checking fails. This example output causes the version check to fail on a RHEL6-based Linux install: ** [ryan@os-sl6-controller ~]$ vmplayer -v Gtk-Message: Failed to load module "canberra-gtk-module": libcanberra-gtk-module.so: cannot open shared object file: No such file or directory VMware Player 6.0.1 build-1379776 [ryan@os-sl6-controller ~]$ virsh -c vmwareplayer:///session error: failed to connect to the hypervisor error: internal error: failed to parse VMware Player version ** This patch bundles STDERR into the checked buffer only with the VMware Fusion driver, allowing the VMware Player version check to succeed on Linux even with the above warning. It looks like there's still an issue (see below) but virsh now starts at least. ** [ryan@os-sl6-controller ~]$ virsh -c vmwareplayer:///session Welcome to virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # version Compiled against library: libvirt 1.2.1 Using library: libvirt 1.2.1 Using API: VMware 1.2.1 Cannot extract running VMware hypervisor version ** This is virsh running against VMware Player 6.0.1 on Linux x86_64. I have not tested on OS X since I don't have a license for VMware Fusion. Thanks everyone! -r diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..502d5c9 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -293,7 +293,8 @@ vmwareExtractVersion(struct vmware_driver *driver) cmd = virCommandNewArgList(bin, "-v", NULL); virCommandSetOutputBuffer(cmd, &outbuf); - virCommandSetErrorBuffer(cmd, &outbuf); + if (driver->type == VMWARE_DRIVER_FUSION) + virCommandSetErrorBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) goto cleanup;
On Mon, Jan 20, 2014 at 02:19:20PM -0600, ryan woodsmall wrote:
Recent changes to the VMware driver allow for VMware Fusion version checking on OS X. It looks like the version checking on Linux for VMware Workstation and Player could possibly be broken by the changes. The version check in vmware_conf.c bundled STDOUT and STDERR into a common buffer; when/if something is printed to STDERR, at least on Player, version checking fails.
This example output causes the version check to fail on a RHEL6-based Linux install: ** [ryan@os-sl6-controller ~]$ vmplayer -v Gtk-Message: Failed to load module "canberra-gtk-module": libcanberra-gtk-module.so: cannot open shared object file: No such file or directory VMware Player 6.0.1 build-1379776
[ryan@os-sl6-controller ~]$ virsh -c vmwareplayer:///session error: failed to connect to the hypervisor error: internal error: failed to parse VMware Player version **
This patch bundles STDERR into the checked buffer only with the VMware Fusion driver, allowing the VMware Player version check to succeed on Linux even with the above warning. It looks like there's still an issue (see below) but virsh now starts at least.
I think that rather than skipping STDERR, we should try to make the parsing code more robust. eg skip over lines in the output buffer until we see a line starting with the string 'VMware'. Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Daniel
Excellent point, Daniel. Try two is below. The verbuf string is tokenized on newline, then iterated over with blank lines ignored. First match of the VMware driver pattern will break out of the loop with tmp set correctly or the default NULL if it's not found. This should allow vmwareExtractVersion to work as expected without change and VMware Fusion/Player/Workstation to continue working on OS X and Linux. Tested successfully with VMware Player 5.0.3 and 6.0.1 on an SL6 box. Would appreciate a Fusion test on OS X if possible. -r (sorry if this reply shows up incorrectly, my git skills are... lacking.) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..1f5b4ae 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -222,8 +222,10 @@ vmwareSetSentinal(const char **prog, const char *key) int vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) { + int tok; const char *pattern; - const char *tmp; + const char *tmp = NULL; + char **verbuftok = NULL; switch (type) { case VMWARE_DRIVER_PLAYER: @@ -241,7 +243,18 @@ vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) return -1; } - if ((tmp = STRSKIP(verbuf, pattern)) == NULL) { + verbuftok = virStringSplit(verbuf, "\n", 0); + for(tok = 0; verbuftok[tok] != NULL; tok++){ + if (strlen(verbuftok[tok]) > 0) { + tmp = STRSKIP(verbuftok[tok], pattern); + if (tmp == NULL) + continue; + else + break; + } + } + + if (tmp == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse %sversion"), pattern); return -1;
Below works, but it still special cases Fusion. I'm not sure it's better, but I think the VMWARE_DRIVER_FUSION pattern string could be changed to just "VMware Fusion " then tmp checked against "Information:" and discarded if a match is found using the tokenized output of virStringSplit. Something like this in the loop over the tokenized driver command check output: ** if ((tmp == NULL) || (strncmp(tmp, "Information:", strlen("Information:")) == 0)) continue; ** This could potentially be extensible with an array of discard substrings for each VMware driver that are iteratable. That may be a bit outside my grasp... Am I completely barking up the wrong tree here? Heading to bed, sorry for the bad patches! -r diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index c96bd62..a2ad7bd 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -222,8 +222,10 @@ vmwareSetSentinal(const char **prog, const char *key) int vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) { + int tok = 0; const char *pattern; - const char *tmp; + const char *tmp = NULL; + char **verbuftok = NULL; switch (type) { case VMWARE_DRIVER_PLAYER: @@ -241,7 +243,22 @@ vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) return -1; } - if ((tmp = STRSKIP(verbuf, pattern)) == NULL) { + verbuftok = virStringSplit(verbuf, "\n", 0); + if (type == VMWARE_DRIVER_FUSION) { + tmp = STRSKIP(verbuf, pattern); + }else{ + for (; verbuftok[tok] != NULL; tok++) { + if (strlen(verbuftok[tok]) > 0) { + tmp = STRSKIP(verbuftok[tok], pattern); + if (tmp == NULL) + continue; + else + break; + } + } + } + + if (tmp == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, _("failed to parse %sversion"), pattern); return -1;
participants (2)
-
Daniel P. Berrange -
ryan woodsmall