[libvirt] [PATCH 1/2] vmware: Fix initialization of VMware Fusion

[Rainer Müller]

On 2018-04-11 14:02, John Ferlan wrote:
> 
> 
> On 04/03/2018 05:32 AM, Rainer Müller wrote:
>> The vmware driver wants to execute vmware-vmx from the same directory in
>> which vmrun was found. However, on VMware Fusion 10 vmrun at
>> /Applications/VMware Fusion.app/Contents/Public/vmrun is a symlink
>> pointing to ../Library/vmrun. vmware-vmx cannot be found, as
>> it is not in PATH, but only in this Library directory.
>>
>> Therefore, follow the vmrun symlink and use the resulting path. Then the
>> assumption that vmware-vmx is right next to it will still work.
>>
>> Signed-off-by: Rainer Müller <raimue at codingfarm.de>
>> ---
>>  src/vmware/vmware_driver.c | 9 ++++++++-
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c
>> index 8b487c4a7..60e1c1abc 100644
>> --- a/src/vmware/vmware_driver.c
>> +++ b/src/vmware/vmware_driver.c
>> @@ -127,6 +127,7 @@ vmwareConnectOpen(virConnectPtr conn,
>>      struct vmware_driver *driver;
>>      size_t i;
>>      char *tmp;
>> +    char *vmrun = NULL;
>>  
>>      virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
>>  
>> @@ -164,7 +165,12 @@ vmwareConnectOpen(virConnectPtr conn,
>>       * for auto detection of the backend
>>       */
>>      for (i = 0; i < ARRAY_CARDINALITY(vmrun_candidates); i++) {
>> -        driver->vmrun = virFindFileInPath(vmrun_candidates[i]);
>> +        vmrun = virFindFileInPath(vmrun_candidates[i]);
> 
> What if this returns NULL?
> 
>> +        if (virFileResolveLink(vmrun, &driver->vmrun) < 0) {
> 
> I doubt this will be very happy...
> 
>> +            virReportSystemError(errno, _("unable to resolve symlink '%s'"), vmrun);
>> +            goto cleanup;
>> +        }
>> +        VIR_FREE(vmrun);
> 
> Prior to this change - if @vmrun wasn't found in the path of
> vmrun_candidates, we'd try the "next one" in the list.>
> With this change if @vmrun returned from virFindFileInPath is NULL, then
> virFileResolveLink would fail, and we wouldn't try the next one in the list.

Thank you for the review. Indeed, this was sloppy testing on my end.
I will prepare a v2.

> There is also virFileIsLink which you may want to consider - as in, what
> we got back is a link, so let's resolve to save it; otherwise, use what
> was found in path.

virFileResolveLink already calls lstat(2) to check whether the given
path is actually link. If it is not, the output is a copy of the input.
So this will already work in the way you describe.

Rainer

> John
> 
>>          /* If we found one, we can stop looking */
>>          if (driver->vmrun)
>>              break;
>> @@ -215,6 +221,7 @@ vmwareConnectOpen(virConnectPtr conn,
>>  
>>   cleanup:
>>      vmwareFreeDriver(driver);
>> +    VIR_FREE(vmrun);
>>      return VIR_DRV_OPEN_ERROR;
>>  };
>>  
>>
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>