Bug 1766475
| Summary: | nwfilter: Remove redundant check if object exists | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Red Hat Enterprise Linux 7 | Reporter: | Marian Jankular <mjankula> | ||||
| Component: | libvirt | Assignee: | Pavel Hrdina <phrdina> | ||||
| Status: | CLOSED ERRATA | QA Contact: | Jing Qi <jinqi> | ||||
| Severity: | medium | Docs Contact: | |||||
| Priority: | unspecified | ||||||
| Version: | 7.7 | CC: | dyuan, jdenemar, jsuchane, lhuang, lmen, rbalakri, tborcin, xuzhang, yalzhang | ||||
| Target Milestone: | rc | Keywords: | Triaged | ||||
| Target Release: | 7.8 | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Whiteboard: | |||||||
| Fixed In Version: | libvirt-4.5.0-30.el7 | Doc Type: | If docs needed, set a value | ||||
| Doc Text: | Story Points: | --- | |||||
| Clone Of: | Environment: | ||||||
| Last Closed: | 2020-03-31 19:59:02 UTC | Type: | Feature Request | ||||
| Regression: | --- | Mount Type: | --- | ||||
| Documentation: | --- | CRM: | |||||
| Verified Versions: | Category: | --- | |||||
| oVirt Team: | Network | RHEL 7.3 requirements from Atomic Host: | |||||
| Cloudforms Team: | --- | Target Upstream Version: | |||||
| Embargoed: | |||||||
| Attachments: |
|
||||||
|
Description
Marian Jankular
2019-10-29 08:16:56 UTC
Dominik - as far as I know, we pull all of this directly from libvirt, don't we? (In reply to Ryan Barry from comment #1) > Dominik - as far as I know, we pull all of this directly from libvirt, don't > we? yes, we do Moving to RHEL If RHV 4.3 is requested target, then I believe RHEL-7 should be requested host. By the way, RHEL Advanced virtualization has this patch included since version 8.0.0.
The patch is:
Author: Pavel Hrdina <phrdina>
AuthorDate: Thu Jun 28 09:53:48 2018 +0200
Commit: Pavel Hrdina <phrdina>
CommitDate: Wed Jul 4 07:23:08 2018 +0200
nwfilter: Remove redundant check if object exists
The same check is done by virNWFilterBindingObjListAdd(). The main
issue with the current code is that if the object already exists we
would leak 'def' because 'obj' would be set and the cleanup code frees
'def' only if 'obj' is NULL.
Reviewed-by: Daniel P. Berrangé <berrange>
Signed-off-by: Pavel Hrdina <phrdina>
v4.5.0-36-g3379193f1c
Reproduced this issue with libvirt-4.5.0-10.el7_6.15.x86_64
use "valgrind" to check the memory leak for libvirtd-
Steps:
1. create a vm , binding nwfilter binding for the interface like:
<interface type='network'>
<mac address='54:52:00:54:9e:f4'/>
<source network='default'/>
<model type='e1000'/>
<filterref filter='no-arp-mac-spoofing'>
<parameter name='IP' value='10.0.0.1'/>
</filterref>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
</interface>
2. update the interface with new nwfilter binding
<interface type='network'>
<mac address='54:52:00:54:9e:f4'/>
<source network='default'/>
<model type='e1000'/>
<filterref filter='clean-traffic'>
<parameter name='IP' value='10.0.0.1'/>
</filterref>
</interface>
#virsh update-device vm1 interface_update.xml
3. the leaks happened -
968 bytes in 1 blocks are definitely lost in loss record 2,169 of 2,384
==31221== at 0x4C29E63: malloc (vg_replace_malloc.c:309)
==31221== by 0x6E12EE1: xmlGetGlobalState (in /usr/lib64/libxml2.so.2.9.1)
==31221== by 0x6E124D4: __xmlDefaultSAXHandler (in /usr/lib64/libxml2.so.2.9.1)
==31221== by 0x6E6C468: xmlDefaultSAXHandlerInit (in /usr/lib64/libxml2.so.2.9.1)
==31221== by 0x6D9F601: xmlInitParserCtxt (in /usr/lib64/libxml2.so.2.9.1)
==31221== by 0x6D9FDAB: xmlNewParserCtxt (in /usr/lib64/libxml2.so.2.9.1)
==31221== by 0x5351081: virXMLParseHelper (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x53BDF89: ??? (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x31D181C4: ??? (in /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so)
==31221== by 0x54F5F55: virNWFilterBindingCreateXML (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x12E1EC: ??? (in /usr/sbin/libvirtd)
==31221== by 0x5407B34: virNetServerProgramDispatch (in /usr/lib64/libvirt.so.0.4005.0)
==31221==
==31221== 2,355 (64 direct, 2,291 indirect) bytes in 1 blocks are definitely lost in loss record 2,284 of 2,384
==31221== at 0x4C2BF79: calloc (vg_replace_malloc.c:762)
==31221== by 0x52C37C3: virAlloc (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x53BDB2C: virNWFilterBindingDefParseNode (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x53BDFA4: ??? (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x31D181C4: ??? (in /usr/lib64/libvirt/connection-driver/libvirt_driver_nwfilter.so)
==31221== by 0x54F5F55: virNWFilterBindingCreateXML (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x12E1EC: ??? (in /usr/sbin/libvirtd)
==31221== by 0x5407B34: virNetServerProgramDispatch (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x540E30C: ??? (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x5345710: ??? (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x5344A97: ??? (in /usr/lib64/libvirt.so.0.4005.0)
==31221== by 0x8114DD4: start_thread (in /usr/lib64/libpthread-2.17.so)
Verified the bug with libvirt-4.5.0-30.virtcov.el7.x86_64 & qemu-kvm-rhev-2.12.0-38.el7.x86_64. The steps is in Comment 8. After step 2, there is no more memory leak reported in valgrind. So, it's fixed. Created attachment 1642285 [details]
code coverage file
Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory, and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHBA-2020:1094 |