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