[libvirt] [PATCH 00/15] [libvirt-java] some improvements and bug fixes (resend
Claudio Bley
cbley at av-test.de
Wed Nov 28 09:51:56 UTC 2012
ping?
Anyone care to review these patches, before someone redoes this work again...?
At Fri, 12 Oct 2012 12:33:01 +0200,
Claudio Bley wrote:
>
> Hi.
>
> / something went not quite right the last time, I'm resending this /
> / series. Please ignore my previous mails. /
>
> Just sending off a few patches that have piled up during the last few
> months.
>
> Patch #1 to #7 and #9 to #10 and #12 contain mostly cosmetic changes
> resp. changes to the build system.
>
> Patch #8 changes the handling of errors. It is unnecessary trying to
> retrieve an error when the libvirt function called did not signal any.
> Also, some functions never fail (return void).
>
> Patch #11 changes how errors are checked. This avoids copying and
> freeing an error object and hence a few JVM to native calls.
>
> Patch #13 fixes all the memory leaks I encountered by using my GDB
> memcheck.py script. Running the junit tests under GDB's supervision
> yielded 750 places of (potential) memory leaks, pointing to calls of
> these libvirt functions:
>
> virDomainDefineXML, virInterfaceGetXMLDesc,
> virConnectListDefinedDomains, virStoragePoolLookupByName,
> virDomainLookupByName, virNetworkGetBridgeName,
> virNetworkLookupByUUIDString, virDomainLookupByUUID,
> virDomainLookupByUUIDString, virConnectListInterfaces,
> virDomainGetOSType, virConnectListDefinedNetworks,
> virNetworkCreateXML, virStoragePoolDefineXML, virConnectOpen,
> virDomainCreateXML, virNetworkDefineXML, virNetworkLookupByName,
> virCopyLastError, virNetworkCreate, virNetworkLookupByUUID,
> virInterfaceLookupByName, virConnectGetHostname,
> virDomainGetXMLDesc, virConnectListNetworks, virNetworkGetXMLDesc
>
> After applying patch #13, the script reports this:
>
> 1. @140737220948192
> #1 __GI___strdup __GI___strdup strdup.c:45
> #2 ... [1]
> #3 Java_java_util_zip_ZipFile_open ? ?:0
> #4 ... [5]
> 2. @140737221175744
> #1 virAlloc virAlloc /build/buildd/libvirt-0.9.12/./src/util/memory.c:103
> #2 virLastErrorObject virLastErrorObject /build/buildd/libvirt-0.9.12/./src/util/virterror.c:277
> #3 virGetLastError virGetLastError /build/buildd/libvirt-0.9.12/./src/util/virterror.c:299
> #4 ffi_call_unix64 ? ?:0
> #5 ffi_call ? ?:0
> #6 ... [1]
> #7 Java_com_sun_jna_Native_invokePointer ? ?:0
> #8 ... [5]
> 3. @140737220736736
> #1 __libc_res_nsend __libc_res_nsend res_send.c:442
> #2 __libc_res_nquery __libc_res_nquery res_query.c:226
> #3 __libc_res_nquerydomain __libc_res_nquerydomain res_query.c:578
> #4 __libc_res_nsearch __libc_res_nsearch res_query.c:416
> #5 _nss_dns_gethostbyname3_r _nss_dns_gethostbyname3_r nss_dns/dns-host.c:197
> #6 gaih_inet gaih_inet ../sysdeps/posix/getaddrinfo.c:940
> #7 __GI_getaddrinfo __GI_getaddrinfo ../sysdeps/posix/getaddrinfo.c:2423
> #8 virGetHostname virGetHostname /build/buildd/libvirt-0.9.12/./src/util/util.c:2105
> #9 virConnectGetHostname virConnectGetHostname /build/buildd/libvirt-0.9.12/./src/libvirt.c:1724
> #10 ffi_call_unix64 ? ?:0
> #11 ffi_call ? ?:0
> #12 ... [1]
> #13 Java_com_sun_jna_Native_invokePointer ? ?:0
> #14 ... [4]
> 4. @140737018598944
> #1 virAlloc virAlloc /build/buildd/libvirt-0.9.12/./src/util/memory.c:103
> #2 virLastErrorObject virLastErrorObject /build/buildd/libvirt-0.9.12/./src/util/virterror.c:277
> #3 virResetLastError virResetLastError /build/buildd/libvirt-0.9.12/./src/util/virterror.c:428
> #4 virNetworkFree virNetworkFree /build/buildd/libvirt-0.9.12/./src/libvirt.c:10151
> #5 ffi_call_unix64 ? ?:0
> #6 ffi_call ? ?:0
> #7 ... [1]
> #8 Java_com_sun_jna_Native_invokeInt ? ?:0
> #9 ... [2]
> 5. @140737221440624
> #1 __libc_res_nsend __libc_res_nsend res_send.c:442
> #2 __libc_res_nquery __libc_res_nquery res_query.c:226
> #3 __libc_res_nquerydomain __libc_res_nquerydomain res_query.c:578
> #4 __libc_res_nsearch __libc_res_nsearch res_query.c:416
> #5 _nss_dns_gethostbyname3_r _nss_dns_gethostbyname3_r nss_dns/dns-host.c:197
> #6 gaih_inet gaih_inet ../sysdeps/posix/getaddrinfo.c:940
> #7 __GI_getaddrinfo __GI_getaddrinfo ../sysdeps/posix/getaddrinfo.c:2423
> #8 virGetHostname virGetHostname /build/buildd/libvirt-0.9.12/./src/util/util.c:2105
> #9 virConnectGetHostname virConnectGetHostname /build/buildd/libvirt-0.9.12/./src/libvirt.c:1724
> #10 ffi_call_unix64 ? ?:0
> #11 ffi_call ? ?:0
> #12 ... [1]
> #13 Java_com_sun_jna_Native_invokePointer ? ?:0
> #14 ... [4]
>
> only pointing to the following function calls:
>
> virConnectGetHostname, virNetworkFree, virGetLastError
>
> AFAICS, these are neglectable. Probably just some global memory which
> is not freed on exit.
>
> (for the record: I'm on Ubuntu Precise, using libvirt 0.9.12. And yes,
> I did call __libc_freeres on exit.)
>
> Patch #14 removes functions that should have never been wrapped. These
> functions do not increase the ref count and hence create Java objects
> that actually do not represent "real" instances on the libvirt side.
>
> I have tested with JNA 3.3.0, 3.4.{0, 1, 2}. I had JVM crashes
> repeatedly with all JNA versions before 3.4.2. So, I strongly
> recommend to use JNA 3.4.2.
>
> Claudio Bley (15):
> Explicitly set includeAntRuntime to false for javac tasks.
> Introduce a javac.debug property.
> Add findbugs build file for ant.
> Make finalize() methods protected.
> Remove redundant public modifier from Libvirt interface methods.
> Change visibility of class members to private to enforce
> encapsulation.
> Mark virConnCopyLastError and virConnGetLastError as deprecated.
> Call processError only if a libvirt function indicates an error.
> Split JUnit tests and use a fixture for Connect.
> Split "build" target and automatically rebuild out of date files.
> Avoid unnecessary copying and calling virResetLastError.
> Remove the libvirt instance attribute from all classes.
> Fix memory leaks for libvirt functions returning newly allocated
> memory.
> Remove functions not intended to be used by libvirt bindings.
> Explicitely define the order of a struct's fields.
>
> build.xml | 37 +-
> findbugs.xml | 36 ++
> src/main/java/org/libvirt/Connect.java | 572 +++++++-------------
> src/main/java/org/libvirt/Device.java | 9 +-
> src/main/java/org/libvirt/Domain.java | 33 +-
> src/main/java/org/libvirt/DomainSnapshot.java | 9 +-
> src/main/java/org/libvirt/Error.java | 24 +-
> src/main/java/org/libvirt/ErrorHandler.java | 9 +-
> src/main/java/org/libvirt/Interface.java | 19 +-
> src/main/java/org/libvirt/Library.java | 88 +++
> src/main/java/org/libvirt/LibvirtException.java | 2 +-
> src/main/java/org/libvirt/Network.java | 26 +-
> src/main/java/org/libvirt/NetworkFilter.java | 9 +-
> src/main/java/org/libvirt/Secret.java | 9 +-
> src/main/java/org/libvirt/StoragePool.java | 9 +-
> src/main/java/org/libvirt/StorageVol.java | 9 +-
> src/main/java/org/libvirt/Stream.java | 11 +-
> src/main/java/org/libvirt/jna/Libvirt.java | 502 ++++++++---------
> src/main/java/org/libvirt/jna/virConnectAuth.java | 10 +
> .../java/org/libvirt/jna/virConnectCredential.java | 12 +
> .../java/org/libvirt/jna/virDomainBlockInfo.java | 8 +
> .../java/org/libvirt/jna/virDomainBlockStats.java | 11 +
> src/main/java/org/libvirt/jna/virDomainInfo.java | 11 +
> .../org/libvirt/jna/virDomainInterfaceStats.java | 13 +
> .../java/org/libvirt/jna/virDomainJobInfo.java | 18 +
> .../java/org/libvirt/jna/virDomainMemoryStats.java | 7 +
> src/main/java/org/libvirt/jna/virError.java | 19 +
> src/main/java/org/libvirt/jna/virNodeInfo.java | 15 +
> .../java/org/libvirt/jna/virSchedParameter.java | 9 +
> .../java/org/libvirt/jna/virStoragePoolInfo.java | 10 +
> .../java/org/libvirt/jna/virStorageVolInfo.java | 8 +
> src/main/java/org/libvirt/jna/virVcpuInfo.java | 9 +
> src/test/java/org/libvirt/TestJavaBindings.java | 50 +-
> src/test/java/org/libvirt/TestLibvirtGlobals.java | 21 +
> 34 files changed, 883 insertions(+), 761 deletions(-)
> create mode 100644 findbugs.xml
> create mode 100644 src/main/java/org/libvirt/Library.java
> create mode 100644 src/test/java/org/libvirt/TestLibvirtGlobals.java
>
> --
> 1.7.9.5
>
> --
> AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
> Phone: +49 341 265 310 19
> Web:<http://www.av-test.org>
>
> Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
> Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
>
--
AV-Test GmbH, Henricistraße 20, 04155 Leipzig, Germany
Phone: +49 341 265 310 19
Web:<http://www.av-test.org>
Eingetragen am / Registered at: Amtsgericht Stendal (HRB 114076)
Geschaeftsfuehrer (CEO): Andreas Marx, Guido Habicht, Maik Morgenstern
More information about the libvir-list
mailing list