-
Notifications
You must be signed in to change notification settings - Fork 97
Enhance VmWareDesktop.pm to support Workstation #1076
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
g-bougard
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Megachip
thank you for your contribution.
I see few little problems with few required changes. Can you check my comments ?
Thank you
| canRun('/Library/Application Support/VMware Fusion/vmrun') || | ||
| canRun('vmrun'); | ||
| canRun('vmrun') || | ||
| canRun('C:\\\\Program Files (x86)\\\\VMware\\\\VMware Workstation\\\\vmrun.exe'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious: does VMware only provide 32-bits version installation ? If a 64-bits version exists, you have to handle it too.
Can you also try by using "/" path separator ? This should be supported by perl, even on windows.
Eventually to simplify code, you may want to use constant this way:
...
use constant WINDOW_VMRUN => 'C:/Program Files (x86)/VMware/VMWare Workstation/vmrun.exe';
sub isEnabled {
...
canRun(WINDOWS_VMRUN);
...
$command = '"'.WINDOWS_VMRUN.'" list';
...Also, can you tell if the installation path can be changed ? In that case, we can't use a static file path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
whyever they are installed in (x86) even on 64 bit systems. Will check custom installation pathes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[debug] module GLPI::Agent::Task::Inventory::Virtualization::VmWareDesktop disabled: failure to load (Bareword "WINDOWS_VMRUN" not allowed while "strict subs" in use at C:/Apps/GAP/nightly/perl/agent/GLPI/Agent/Task/Inventory/Virtualization/VmWareDesktop.pm line 18.
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
Co-authored-by: Guillaume Bougard <gbougard@teclib.com>
| SUBSYSTEM => "VmWare Fusion", | ||
| SUBSYSTEM => $subsystem, | ||
| VMTYPE => "VmWare", | ||
| COMMENT => $info{'annotation'}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the added "COMMENT" field, you must handle the case it is not defined to avoid perl warning when not defined:
| COMMENT => $info{'annotation'}, | |
| COMMENT => $info{'annotation'} // "", |
Generic Windows Support plus setting
VCPUcorrecly.Solving #1075