[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 00/15] [libvirt-java] some improvements and bug fixes (resend



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]