fix(Hypervisor): close connection on disconnect

hooklift · Aug 1, 2015 · c843ed1 · c843ed1
1 parent 8c53948
commit c843ed1
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 18 deletions.
diff --git a/src/hypervisor.cc b/src/hypervisor.cc
@@ -147,10 +147,10 @@ void Hypervisor::Initialize(Handle<Object> exports)
   NODE_DEFINE_CONSTANT(exports, VIR_DOMAIN_EVENT_ID_IO_ERROR);
   NODE_DEFINE_CONSTANT(exports, VIR_DOMAIN_EVENT_ID_GRAPHICS);
   NODE_DEFINE_CONSTANT(exports, VIR_DOMAIN_EVENT_ID_IO_ERROR_REASON);
-  
+
   // virMemory
   NODE_DEFINE_CONSTANT(exports, VIR_NODE_MEMORY_STATS_ALL_CELLS);
-  
+
 #ifdef VIR_ENUM_SENTINELS
   NODE_DEFINE_CONSTANT(exports, VIR_DOMAIN_EVENT_ID_LAST);
 #endif
@@ -172,6 +172,15 @@ Hypervisor::Hypervisor(string uri, string username, string password, bool readon
 
 Hypervisor::~Hypervisor()
 {
+  if (handle_ != NULL) {
+    int result = virConnectClose(handle_);
+    if (result == -1) {
+      fprintf(stderr, "unable to free connection handle\n");
+      return;
+    }
+
+    handle_ = NULL;
+  }
 }
 
 virConnectPtr Hypervisor::Connection() const
@@ -294,13 +303,30 @@ NLV_WORKER_EXECUTE(Hypervisor, Connect)
     SetVirError(virGetLastError());
 }
 
-NLV_WORKER_METHOD_NO_ARGS(Hypervisor, Disconnect)
+NAN_METHOD(Hypervisor::Disconnect)
+{
+  NanScope();
+  if (args.Length() == 1 && !args[0]->IsFunction()) {
+    NanThrowTypeError("You must specify a function as first argument");
+    NanReturnUndefined();
+  }
+
+  NanCallback *callback = new NanCallback(args[0].As<Function>());
+  Hypervisor *hv = ObjectWrap::Unwrap<Hypervisor>(args.This());
+  NanAsyncQueueWorker(new DisconnectWorker(callback, hv));
+  NanReturnUndefined();
+}
+
 NLV_WORKER_EXECUTE(Hypervisor, Disconnect)
 {
   NLV_WORKER_ASSERT_CONNECTION();
-  if (Handle().ToConnection() != NULL) {
-    Handle().Clear();
+  int result = virConnectClose(Handle().ToConnection());
+  if (result == -1) {
+    SetVirError(virGetLastError());
+    return;
   }
+
+  Handle().Clear();
 }
 
 #define HYPERVISOR_STRING_RETURN_EXECUTE(MethodName, Accessor)  \

diff --git a/src/hypervisor.h b/src/hypervisor.h
@@ -3,6 +3,7 @@
 #define SRC_HYPERVISOR_H_
 
 #include "node_libvirt.h"
+#include "domain.h"
 
 #include "worker_macros.h"
 #include "worker.h"
@@ -107,9 +108,11 @@ class Hypervisor : public ObjectWrap
 
   class DisconnectWorker : public LibVirtWorker {
   public:
-    DisconnectWorker(NanCallback *callback, const LibVirtHandle &handle)
-      : LibVirtWorker(callback, handle) {}
+    DisconnectWorker(NanCallback *callback, Hypervisor *hypervisor)
+      : LibVirtWorker(callback, hypervisor->handle_), hypervisor_(hypervisor) {}
     void Execute();
+  private:
+    Hypervisor *hypervisor_;
   };
 
   class CompareCPUWorker : public PrimitiveReturnWorker<int> {
@@ -146,7 +149,7 @@ class Hypervisor : public ObjectWrap
   NLV_LIST_RETURN_WORKER(ListActiveNetworks, std::string, v8::String);
   NLV_LIST_RETURN_WORKER(ListSecrets, std::string, v8::String);
   NLV_LIST_RETURN_WORKER(ListActiveStoragePools, std::string, v8::String);
-  
+
   NLV_PRIMITIVE_RETURN_WORKER(GetNumberOfDefinedDomains, int);
   NLV_PRIMITIVE_RETURN_WORKER(GetNumberOfDefinedInterfaces, int);
   NLV_PRIMITIVE_RETURN_WORKER(GetNumberOfDefinedNetworks, int);

diff --git a/test/hypervisor.test.js b/test/hypervisor.test.js
@@ -25,27 +25,23 @@ describe('Hypervisor', function() {
       var connection = new Hypervisor('test:///default');
       connection.connect(function(err) {
         expect(err).to.not.exist;
-        done();
-      });
-    });
 
-    it('should open a read-only connection', function(done) {
-      var connection = new Hypervisor('test:///default', true);
-      connection.connect(function(err) {
-        expect(err).to.not.exist;
-        done();
+        connection.disconnect(function(err) {
+          expect(err).to.not.exist;
+          done();
+        });
       });
     });
 
-    it('should open and close a connection', function(done) {
+    it('should open a read-only connection', function(done) {
       var connection = new Hypervisor('test:///default', true);
       connection.connect(function(err) {
         expect(err).to.not.exist;
 
         connection.disconnect(function(err) {
           expect(err).to.not.exist;
           done();
-        });
+        })
       });
     });