[libvirt PATCH v2 3/5] vmx: Make virVMXParseFileName return an integer

[Martin Kletzander]

On Tue, Jan 05, 2021 at 10:15:47AM +0100, Michal Privoznik wrote:
>On 1/5/21 12:32 AM, Daniel Henrique Barboza wrote:
>>
>> On 12/21/20 1:19 PM, Martin Kletzander wrote:
>>> And return the actual extracted value in a parameter.  This way we can
>>> later
>>> return success even without any extracted value.
>>>
>>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>>> ---
>>>   src/esx/esx_driver.c     | 31 ++++++++++++++++++-------------
>>>   src/vmware/vmware_conf.c | 10 +++++-----
>>>   src/vmx/vmx.c            | 21 ++++++++++-----------
>>>   src/vmx/vmx.h            |  2 +-
>>>   tests/vmx2xmltest.c      | 19 ++++++++++---------
>>>   5 files changed, 44 insertions(+), 39 deletions(-)
>>>
>
>>> diff --git a/src/vmx/vmx.h b/src/vmx/vmx.h
>>> index df5d39157b99..e5420c970a4b 100644
>>> --- a/src/vmx/vmx.h
>>> +++ b/src/vmx/vmx.h
>>> @@ -36,7 +36,7 @@ virDomainXMLOptionPtr
>>> virVMXDomainXMLConfInit(virCapsPtr caps);
>>>    * Context
>>>    */
>>> -typedef char * (*virVMXParseFileName)(const char *fileName, void
>>> *opaque);
>>> +typedef int (*virVMXParseFileName)(const char *fileName, void
>>> *opaque, char **src);
>>
>>
>>
>> This change is making the build break in my env:
>>
>> ../src/vmware/vmware_conf.c: In function ‘vmwareLoadDomains’:
>> ../src/vmware/vmware_conf.c:142:23: error: assignment to
>> ‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’}
>> from incompatible pointer type ‘char * (*)(const char *, void *)’
>> [-Werror=incompatible-pointer-types]
>>    142 |     ctx.parseFileName = vmwareCopyVMXFileName;
>>        |                       ^
>> ../src/vmware/vmware_conf.c: At top level:
>> ../src/vmware/vmware_conf.c:511:1: error: conflicting types for
>> ‘vmwareCopyVMXFileName’
>>    511 | vmwareCopyVMXFileName(const char *datastorePath, void *opaque
>> G_GNUC_UNUSED,
>>        | ^~~~~~~~~~~~~~~~~~~~~
>> In file included from ../src/vmware/vmware_conf.c:32:
>> ../src/vmware/vmware_conf.h:86:7: note: previous declaration of
>> ‘vmwareCopyVMXFileName’ was here
>>     86 | char *vmwareCopyVMXFileName(const char *datastorePath, void
>> *opaque);
>>        |       ^~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>>
>> (...)
>>
>> ../src/vmware/vmware_driver.c: In function
>> ‘vmwareConnectDomainXMLFromNative’:
>> ../src/vmware/vmware_driver.c:953:23: error: assignment to
>> ‘virVMXParseFileName’ {aka ‘int (*)(const char *, void *, char **)’}
>> from incompatible pointer type ‘char * (*)(const char *, void *)’
>> [-Werror=incompatible-pointer-types]
>>    953 |     ctx.parseFileName = vmwareCopyVMXFileName;
>>        |                       ^
>> cc1: all warnings being treated as errors
>>
>>
>>
>> I believe there are a handful of virVMXParseFileName impl instances that
>> needs
>> to be changed as well.
>
>
>Indeed. I think we will need to change virVMXFormatFileName() too at the
>same time, because of vmwareCopyVMXFileName() which is passed as
>formatFileName callback.
>

Not only that, but vmware_driver uses the smae function for parsing and
formatting.  They are, however, just g_strdup()s, so I can split them no
problem.  Thanks for noticing.

Anyway, just looking at the commits they should be done a bit differently, I had
some cock-ups there, I guess.  I'll send a v3.

>BTW: I've found out that we don't automatically enable vmware driver. I
>had to enable it explicitly:
>
>meson -Dsystem=true -Ddriver_vmware=enabled _build
>
>I'm looking into that.
>

We're not building it downstream even, I think, I managed to fix it for ESX
where the issue was happening actually.  The vmware driver just controls vmware
player/workstation locally.  Are there actually people using that?

>Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210105/a8eaab35/attachment-0001.sig>