[libvirt] Connection release is not correct in libvirt and libvrt java

[Eric Blake]

On 12/18/2012 08:14 AM, Benjamin Wang (gendwang) wrote:
> Now if the cable is unplugged and the application call virConnectClose to release connection, the code will enter into the error procedure, the connection
> Can't be released. I have changed the following two parts to fix this issue. Please give your comments:
> Changed Code1:
> int
> virConnectClose(virConnectPtr conn)
> {
> ...
> 
> +    if(NULL == conn) {
> +        return 0;
> +    }
> 
> ...
> 
> 
> -    if (!VIR_IS_CONNECT(conn)) {
> -        virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
> -        goto error;
> -    }
> ...


It's much easier to read patches that are produced by 'git diff' than by
open-coding the differences into your email.  In particular, missing
context makes it hard to see where you are intending your patch to
apply.  However, I will NACK this patch.  NULL is never a valid
connection, and passing NULL must make the API return failure, not
success.  The existing 'if (!VIR_IS_CONNECT(conn))' is sufficient to
check for an invalid passing of a NULL pointer.


> 
> For libvirt java, there are similar issue. I have changed code as following in Collect.java. Please also give your comments.
>     public int close() throws LibvirtException {
>         int success = 0;
>         if (VCP != null) {
> +            try {
>              success = libvirt.virConnectClose(VCP);
>                 processError();
> +            }
> +            finally {
>                 // If leave an invalid pointer dangling around JVM crashes and burns
>                 // if someone tries to call a method on us
>                 // We rely on the underlying libvirt error handling to detect that
>                 // it's called with a null virConnectPointer
>                 VCP = null;
>  +           }

Unfortunately, I'm not familiar enough with libvirt-java to know if this
patch makes sense.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 619 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20121219/fa1ea1b1/attachment-0001.sig>