[libvirt] [PATCH v2 32/32] util: Introduce VIR_DEFINE_AUTOPTR_FUNC for virStorageSource

John Ferlan jferlan at redhat.com
Mon Feb 11 14:36:16 UTC 2019



On 2/11/19 8:45 AM, Erik Skultety wrote:
> On Mon, Feb 11, 2019 at 08:33:32AM -0500, John Ferlan wrote:
>>
>>
>> On 2/11/19 7:44 AM, Erik Skultety wrote:
>>> On Fri, Feb 08, 2019 at 01:37:26PM -0500, John Ferlan wrote:
>>>> Let's make use of the auto __cleanup capabilities cleaning up any
>>>> now unnecessary goto paths.
>>>>
>>>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>>>> ---
>>> ...
>>>>              target->backingStore->type = VIR_STORAGE_TYPE_NETWORK;
>>>>              target->backingStore->path = meta->backingStoreRaw;
>>>> @@ -3430,8 +3428,6 @@ storageBackendProbeTarget(virStorageSourcePtr target,
>>>>      target->format = meta->format;
>>>>
>>>>      /* Default to success below this point */
>>>> -    ret = 0;
>>>> -
>>>
>>> Not sure how much sense the comment makes without ^this hunk, I think we can
>>> drop it too
>>
>> 50/50 coin flip, I can remove
>>
>>>
>>> MinGW is still stuborn about inlining in certain VIR_AUTO cases, I'm wondering
>>> why GCC and Clang are okay with that and whether we should drop -Winline or go
>>> without the cases that MinGW is sad about, I tried to lookup something related,
>>> but it doesn't seem to be a recent bug in MinGW.
>>>
>>> I'm okay with the changes, but I don't want to have a failing build for the
>>> next X weeks, so I'd say drop the violators:
>>
>> I don't know which violators exist as I don't build MinGW nor do I have
>> whatever nifty environment generates that type of build as my normal
>> process.
> 
> You can either set up your own environment with Andrea's lcitool or you can

$ grep lcitool docs/*.in

;-)... IOW: Maybe we should document the procedure in our hacking for
"everyone" to use.

> make use of github's travis integration:
> https://travis-ci.org/eskultety/libvirt/jobs/491591878
> 

So from your output it seems:

virStorageFileMetadataNew
virStorageFileGetMetadataFromFD
virStorageSourceCopy
virStorageSourceUpdateCapacity
virStorageSourceNewFromBackingAbsolute
virStorageSourceNewFromBacking
virStorageFileGetMetadataRecurse
virStorageFileGetBackingStoreStr

or IOW everything in src/util/virstoragefile.c other than I think
virStorageSourceNewFromBackingRelative, although there is one pile of
compiler output without an "In function '%s':" before the compiler error
output "inlining failed in call to" before it which I assume is the
*Relative call.  Dropping those means dropping a bunch of patches before
too since the only reason to change was to do this AUTOPTR magic.

As a side note I find it really odd that the order listed in the
compiler output doesn't exactly follow the order in the source. In
particular starting with virStorageSourceUpdateCapacity (line 3832) is
listed in the output before virStorageSourceNewFromBackingAbsolute (line
3613).

Still it makes me wonder what's different in src/util from others that
causes src/util to throw up, while src/conf, src/qemu, tests/, etc.
don't throw up.  Reading m4/extern-inline.m4 doesn't help me.

John




More information about the libvir-list mailing list