[libvirt] [PATCH 2/3] qemuxml2argvtest: Don't initialize qemuCaps twice

Michal Privoznik posted 3 patches 7 years, 6 months ago
[libvirt] [PATCH 2/3] qemuxml2argvtest: Don't initialize qemuCaps twice
Posted by Michal Privoznik 7 years, 6 months ago
There's no point in calling testInitQEMUCaps() (which sets
info.qemuCaps) only to overwrite (and leak) it on the very next
line.

==12962== 296 (208 direct, 88 indirect) bytes in 1 blocks are definitely lost in loss record 265 of 331
==12962==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
==12962==    by 0x5D28D9F: virAllocVar (viralloc.c:560)
==12962==    by 0x5D96AB4: virObjectNew (virobject.c:239)
==12962==    by 0x56DB7C7: virQEMUCapsNew (qemu_capabilities.c:1480)
==12962==    by 0x112A5B: testInitQEMUCaps (qemuxml2argvtest.c:361)
==12962==    by 0x1371C8: mymain (qemuxml2argvtest.c:2871)
==12962==    by 0x13AD0B: virTestMain (testutils.c:1120)
==12962==    by 0x1372FD: main (qemuxml2argvtest.c:2883)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 tests/qemuxml2argvtest.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ddd2b88c0a..6f421ce8f5 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -699,8 +699,6 @@ mymain(void)
             (flags), parseFlags, false, NULL \
         }; \
         info.skipLegacyCPUs = skipLegacyCPUs; \
-        if (testInitQEMUCaps(&info, gic) < 0) \
-            return EXIT_FAILURE; \
         if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
                                                             capsfile))) \
             return EXIT_FAILURE; \
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemuxml2argvtest: Don't initialize qemuCaps twice
Posted by Peter Krempa 7 years, 6 months ago
On Wed, May 30, 2018 at 18:04:28 +0200, Michal Privoznik wrote:
> There's no point in calling testInitQEMUCaps() (which sets
> info.qemuCaps) only to overwrite (and leak) it on the very next
> line.
> 
> ==12962== 296 (208 direct, 88 indirect) bytes in 1 blocks are definitely lost in loss record 265 of 331
> ==12962==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
> ==12962==    by 0x5D28D9F: virAllocVar (viralloc.c:560)
> ==12962==    by 0x5D96AB4: virObjectNew (virobject.c:239)
> ==12962==    by 0x56DB7C7: virQEMUCapsNew (qemu_capabilities.c:1480)
> ==12962==    by 0x112A5B: testInitQEMUCaps (qemuxml2argvtest.c:361)
> ==12962==    by 0x1371C8: mymain (qemuxml2argvtest.c:2871)
> ==12962==    by 0x13AD0B: virTestMain (testutils.c:1120)
> ==12962==    by 0x1372FD: main (qemuxml2argvtest.c:2883)
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  tests/qemuxml2argvtest.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index ddd2b88c0a..6f421ce8f5 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -699,8 +699,6 @@ mymain(void)
>              (flags), parseFlags, false, NULL \
>          }; \
>          info.skipLegacyCPUs = skipLegacyCPUs; \
> -        if (testInitQEMUCaps(&info, gic) < 0) \
> -            return EXIT_FAILURE; \


This makes the 'gic' macro argument unused. You probably need to replace
it with testQemuCapsSetGIC after the caps are parsed if the parser does
not do that.

>          if (!(info.qemuCaps = qemuTestParseCapabilitiesArch(virArchFromString(arch), \
>                                                              capsfile))) \
>              return EXIT_FAILURE; \
> -- 
> 2.16.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemuxml2argvtest: Don't initialize qemuCaps twice
Posted by Michal Privoznik 7 years, 6 months ago
On 05/30/2018 06:46 PM, Peter Krempa wrote:
> On Wed, May 30, 2018 at 18:04:28 +0200, Michal Privoznik wrote:
>> There's no point in calling testInitQEMUCaps() (which sets
>> info.qemuCaps) only to overwrite (and leak) it on the very next
>> line.
>>
>> ==12962== 296 (208 direct, 88 indirect) bytes in 1 blocks are definitely lost in loss record 265 of 331
>> ==12962==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
>> ==12962==    by 0x5D28D9F: virAllocVar (viralloc.c:560)
>> ==12962==    by 0x5D96AB4: virObjectNew (virobject.c:239)
>> ==12962==    by 0x56DB7C7: virQEMUCapsNew (qemu_capabilities.c:1480)
>> ==12962==    by 0x112A5B: testInitQEMUCaps (qemuxml2argvtest.c:361)
>> ==12962==    by 0x1371C8: mymain (qemuxml2argvtest.c:2871)
>> ==12962==    by 0x13AD0B: virTestMain (testutils.c:1120)
>> ==12962==    by 0x1372FD: main (qemuxml2argvtest.c:2883)
>>
>> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
>> ---
>>  tests/qemuxml2argvtest.c | 2 --
>>  1 file changed, 2 deletions(-)
>>
>> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
>> index ddd2b88c0a..6f421ce8f5 100644
>> --- a/tests/qemuxml2argvtest.c
>> +++ b/tests/qemuxml2argvtest.c
>> @@ -699,8 +699,6 @@ mymain(void)
>>              (flags), parseFlags, false, NULL \
>>          }; \
>>          info.skipLegacyCPUs = skipLegacyCPUs; \
>> -        if (testInitQEMUCaps(&info, gic) < 0) \
>> -            return EXIT_FAILURE; \
> 
> 
> This makes the 'gic' macro argument unused. You probably need to replace
> it with testQemuCapsSetGIC after the caps are parsed if the parser does
> not do that.

Ah good point. However, since all callers of this macro pass GIC_NONE
(if anything) the argument is not needed. The qemuCaps code is perfectly
capable of handling no gic set (seevirQEMUCapsSupportsGICVersion()).
Because of this I'm going to remove the argument in v2.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemuxml2argvtest: Don't initialize qemuCaps twice
Posted by Peter Krempa 7 years, 6 months ago
On Thu, May 31, 2018 at 10:06:38 +0200, Michal Privoznik wrote:
> On 05/30/2018 06:46 PM, Peter Krempa wrote:
> > On Wed, May 30, 2018 at 18:04:28 +0200, Michal Privoznik wrote:
> >> There's no point in calling testInitQEMUCaps() (which sets
> >> info.qemuCaps) only to overwrite (and leak) it on the very next
> >> line.
> >>
> >> ==12962== 296 (208 direct, 88 indirect) bytes in 1 blocks are definitely lost in loss record 265 of 331
> >> ==12962==    at 0x4C2CF26: calloc (vg_replace_malloc.c:711)
> >> ==12962==    by 0x5D28D9F: virAllocVar (viralloc.c:560)
> >> ==12962==    by 0x5D96AB4: virObjectNew (virobject.c:239)
> >> ==12962==    by 0x56DB7C7: virQEMUCapsNew (qemu_capabilities.c:1480)
> >> ==12962==    by 0x112A5B: testInitQEMUCaps (qemuxml2argvtest.c:361)
> >> ==12962==    by 0x1371C8: mymain (qemuxml2argvtest.c:2871)
> >> ==12962==    by 0x13AD0B: virTestMain (testutils.c:1120)
> >> ==12962==    by 0x1372FD: main (qemuxml2argvtest.c:2883)
> >>
> >> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> >> ---
> >>  tests/qemuxml2argvtest.c | 2 --
> >>  1 file changed, 2 deletions(-)
> >>
> >> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> >> index ddd2b88c0a..6f421ce8f5 100644
> >> --- a/tests/qemuxml2argvtest.c
> >> +++ b/tests/qemuxml2argvtest.c
> >> @@ -699,8 +699,6 @@ mymain(void)
> >>              (flags), parseFlags, false, NULL \
> >>          }; \
> >>          info.skipLegacyCPUs = skipLegacyCPUs; \
> >> -        if (testInitQEMUCaps(&info, gic) < 0) \
> >> -            return EXIT_FAILURE; \
> > 
> > 
> > This makes the 'gic' macro argument unused. You probably need to replace
> > it with testQemuCapsSetGIC after the caps are parsed if the parser does
> > not do that.
> 
> Ah good point. However, since all callers of this macro pass GIC_NONE
> (if anything) the argument is not needed. The qemuCaps code is perfectly
> capable of handling no gic set (seevirQEMUCapsSupportsGICVersion()).
> Because of this I'm going to remove the argument in v2.

Well, that was so that we could later add macros for ARM, but as we
parse in capabilities, it should perhaps be encoded in them rather than
provided externally.

pre-emptive ACK and obvious-safe-for-freeze for the version with the GIC
argument removed.

> 
> Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list