fix(migration): support flags defined as an integer

Domain::Migrate made an assumption that the passed in flags param of the migration options were an array of integers. This patch adds support for interpreting the value as a plain integer. closes #86
hooklift · Jan 9, 2017 · c81e718 · c81e718
1 parent c9f159e
commit c81e718
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 16 deletions.
diff --git a/src/domain.cc b/src/domain.cc
@@ -1518,23 +1518,27 @@ NAN_METHOD(Domain::Migrate)
   std::string dest_uri(*Nan::Utf8String(info_->Get(Nan::New("dest_uri").ToLocalChecked())));
   std::string dest_name(*Nan::Utf8String(info_->Get(Nan::New("dest_name").ToLocalChecked())));
 
-  if(info_->Has(Nan::New("flags").ToLocalChecked())) {
-    Local<Array> flags_ = info_->Get(Nan::New("flags").ToLocalChecked()).As<Array>();
-    unsigned int length = flags_->Length();
-    for (unsigned int i = 0; i < length; i++)
-      flags |= flags_->Get(Nan::New<Integer>(i))->Int32Value();
+  if (info_->Has(Nan::New("flags").ToLocalChecked())) {
+    Local<Value> flagsValue = info_->Get(Nan::New("flags").ToLocalChecked());
+    if (flagsValue->IsArray()) {
+      Local<Array> flags_ = flagsValue.As<Array>();
+      unsigned int length = flags_->Length();
+      for (unsigned int i = 0; i < length; i++)
+        flags |= flags_->Get(Nan::New<Integer>(i))->Int32Value();
+    } else if (flagsValue->IsNumber()) {
+      flags = flagsValue->IntegerValue();
+    }
   }
 
-  if(info_->Has(Nan::New("bandwidth").ToLocalChecked())) {
+  if (info_->Has(Nan::New("bandwidth").ToLocalChecked())) {
     bandwidth = info_->Get(Nan::New("bandwidth").ToLocalChecked())->Int32Value();
   }
 
   Domain *domain = Domain::Unwrap(info.This());
-
   Nan::Callback *callback = new Nan::Callback(info[1].As<Function>());
   MigrateWorker *worker;
 
-  if(info_->Has(Nan::New("dest_hypervisor").ToLocalChecked())) {
+  if (info_->Has(Nan::New("dest_hypervisor").ToLocalChecked())) {
     Local<Object> hyp_obj = info_->Get(Nan::New("dest_hypervisor").ToLocalChecked())->ToObject();
     if(!Nan::New(Hypervisor::constructor_template)->HasInstance(hyp_obj)) {
       Nan::ThrowTypeError("You must specify a Hypervisor object instance");

diff --git a/test/domain.test.js b/test/domain.test.js
@@ -302,14 +302,47 @@ describe('Domain', function() {
     // });
 
     it('should migrate a domain to another hypervisor through an uri', function(done) {
-      var flags = [
-        test.domain.VIR_MIGRATE_LIVE,
-        test.domain.VIR_MIGRATE_PEER2PEER,
-        test.domain.VIR_MIGRATE_PAUSED,
-        test.domain.VIR_MIGRATE_PERSIST_DEST
-      ];
-
-      test.domain.migrate({ dest_uri: 'test:///default', dest_name: 'test2', bandwidth: 100, flags: flags }, function(err) {
+      var options = {
+        dest_uri: 'test:///default',
+        dest_name: 'test2',
+        bandwidth: 100,
+        flags: [
+          test.domain.VIR_MIGRATE_LIVE,
+          test.domain.VIR_MIGRATE_PEER2PEER,
+          test.domain.VIR_MIGRATE_PAUSED,
+          test.domain.VIR_MIGRATE_PERSIST_DEST
+        ]
+      };
+
+      test.domain.migrate(options, function(err) {
+        expect(err).to.exist;
+        // some libvirt versions report different error codes.
+        var possibleErrors = [
+          libvirt.VIR_ERR_OPERATION_INVALID,
+          libvirt.VIR_ERR_NO_SUPPORT,
+          libvirt.VIR_ERR_ARGUMENT_UNSUPPORTED
+        ];
+
+        expect(possibleErrors).to.include(err.code);
+        // NOTE: not supported by test driver
+        // expect(err).to.not.exist;
+
+        done();
+      });
+    });
+
+    it('should migrate a domain to another hypervisor through an uri (int flags)', function(done) {
+      var options = {
+        dest_uri: 'qemu://192.168.1.202/system',
+        dest_name: 'test1',
+        flags: test.domain.VIR_MIGRATE_LIVE |
+               test.domain.VIR_MIGRATE_PEER2PEER |
+               test.domain.VIR_MIGRATE_TUNNELLED |
+               test.domain.VIR_MIGRATE_PERSIST_DEST |
+               test.domain.VIR_MIGRATE_UNDEFINE_SOURCE
+      };
+
+      test.domain.migrate(options, function(err) {
         expect(err).to.exist;
         // some libvirt versions report different error codes.
         var possibleErrors = [