You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by ek...@apache.org on 2015/05/13 11:35:49 UTC

[07/41] git commit: updated refs/heads/master to 45c0fa2

Refactoring the LibvirtComputingResource
  - Adding LibvirtCheckHealthCommandWrapper and LibvirtPrepareForMigrationCommandWrapper
  - 2 unit tests added
  - KVM hypervisor plugin with 10.8% coverage


Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo
Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/d730efb8
Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/d730efb8
Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/d730efb8

Branch: refs/heads/master
Commit: d730efb86681b2206b82cbacc828e77e1204367b
Parents: 54f68f5
Author: wilderrodrigues <wr...@schubergphilis.com>
Authored: Thu Apr 23 11:36:38 2015 +0200
Committer: wilderrodrigues <wr...@schubergphilis.com>
Committed: Wed May 6 19:20:41 2015 +0200

----------------------------------------------------------------------
 .../kvm/resource/LibvirtComputingResource.java  | 68 ++-------------
 .../LibvirtCheckHealthCommandWrapper.java       | 34 ++++++++
 ...ibvirtPrepareForMigrationCommandWrapper.java | 89 ++++++++++++++++++++
 .../resource/wrapper/LibvirtRequestWrapper.java |  4 +
 .../resource/LibvirtComputingResourceTest.java  | 72 ++++++++++++++++
 5 files changed, 206 insertions(+), 61 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d730efb8/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
index b90b28b..69f291e 100644
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
@@ -95,8 +95,6 @@ import com.cloud.agent.api.AttachVolumeAnswer;
 import com.cloud.agent.api.AttachVolumeCommand;
 import com.cloud.agent.api.BackupSnapshotAnswer;
 import com.cloud.agent.api.BackupSnapshotCommand;
-import com.cloud.agent.api.CheckHealthAnswer;
-import com.cloud.agent.api.CheckHealthCommand;
 import com.cloud.agent.api.CheckNetworkAnswer;
 import com.cloud.agent.api.CheckNetworkCommand;
 import com.cloud.agent.api.CheckOnHostCommand;
@@ -145,8 +143,6 @@ import com.cloud.agent.api.PingRoutingWithNwGroupsCommand;
 import com.cloud.agent.api.PingTestCommand;
 import com.cloud.agent.api.PlugNicAnswer;
 import com.cloud.agent.api.PlugNicCommand;
-import com.cloud.agent.api.PrepareForMigrationAnswer;
-import com.cloud.agent.api.PrepareForMigrationCommand;
 import com.cloud.agent.api.PvlanSetupCommand;
 import com.cloud.agent.api.ReadyAnswer;
 import com.cloud.agent.api.ReadyCommand;
@@ -397,6 +393,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
         return _publicBridgeName;
     }
 
+    public KVMStoragePoolManager getStoragePoolMgr() {
+        return _storagePoolMgr;
+    }
+
     private static final class KeyValueInterpreter extends OutputInterpreter {
         private final Map<String, String> map = new HashMap<String, String>();
 
@@ -1044,7 +1044,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
         return vifDriver;
     }
 
-    protected VifDriver getVifDriver(final TrafficType trafficType) {
+    public VifDriver getVifDriver(final TrafficType trafficType) {
         VifDriver vifDriver = _trafficTypeVifDrivers.get(trafficType);
 
         if (vifDriver == null) {
@@ -1299,11 +1299,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
         }
 
         try {
-            if (cmd instanceof CheckHealthCommand) {
-                return execute((CheckHealthCommand)cmd);
-            } else if (cmd instanceof PrepareForMigrationCommand) {
-                return execute((PrepareForMigrationCommand)cmd);
-            } else if (cmd instanceof MigrateCommand) {
+            if (cmd instanceof MigrateCommand) {
                 return execute((MigrateCommand)cmd);
             } else if (cmd instanceof PingTestCommand) {
                 return execute((PingTestCommand)cmd);
@@ -3211,56 +3207,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
         }
     }
 
-    private synchronized Answer execute(final PrepareForMigrationCommand cmd) {
-
-        final VirtualMachineTO vm = cmd.getVirtualMachine();
-        if (s_logger.isDebugEnabled()) {
-            s_logger.debug("Preparing host for migrating " + vm);
-        }
-
-        final NicTO[] nics = vm.getNics();
-
-        boolean skipDisconnect = false;
-
-        try {
-            final Connect conn = LibvirtConnection.getConnectionByVmName(vm.getName());
-            for (final NicTO nic : nics) {
-                getVifDriver(nic.getType()).plug(nic, null, "");
-            }
-
-            /* setup disks, e.g for iso */
-            final DiskTO[] volumes = vm.getDisks();
-            for (final DiskTO volume : volumes) {
-                if (volume.getType() == Volume.Type.ISO) {
-                    getVolumePath(conn, volume);
-                }
-            }
-
-            if (!_storagePoolMgr.connectPhysicalDisksViaVmSpec(vm)) {
-                skipDisconnect = true;
-                return new PrepareForMigrationAnswer(cmd, "failed to connect physical disks to host");
-            }
-
-            skipDisconnect = true;
-
-            return new PrepareForMigrationAnswer(cmd);
-        } catch (final LibvirtException e) {
-            return new PrepareForMigrationAnswer(cmd, e.toString());
-        } catch (final InternalErrorException e) {
-            return new PrepareForMigrationAnswer(cmd, e.toString());
-        } catch (final URISyntaxException e) {
-            return new PrepareForMigrationAnswer(cmd, e.toString());
-        } finally {
-            if (!skipDisconnect) {
-                _storagePoolMgr.disconnectPhysicalDisksViaVmSpec(vm);
-            }
-        }
-    }
-
-    private Answer execute(final CheckHealthCommand cmd) {
-        return new CheckHealthAnswer(cmd, true);
-    }
-
     public String networkUsage(final String privateIpAddress, final String option, final String vif) {
         final Script getUsage = new Script(_routerProxyPath, s_logger);
         getUsage.add("netusage.sh");
@@ -3740,7 +3686,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
         }
     }
 
-    private String getVolumePath(final Connect conn, final DiskTO volume) throws LibvirtException, URISyntaxException {
+    public String getVolumePath(final Connect conn, final DiskTO volume) throws LibvirtException, URISyntaxException {
         final DataTO data = volume.getData();
         final DataStoreTO store = data.getDataStore();
 

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d730efb8/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java
new file mode 100644
index 0000000..c89d031
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtCheckHealthCommandWrapper.java
@@ -0,0 +1,34 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CheckHealthAnswer;
+import com.cloud.agent.api.CheckHealthCommand;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+
+public final class LibvirtCheckHealthCommandWrapper extends CommandWrapper<CheckHealthCommand, Answer, LibvirtComputingResource> {
+
+    @Override
+    public Answer execute(final CheckHealthCommand command, final LibvirtComputingResource libvirtComputingResource) {
+        return new CheckHealthAnswer(command, true);
+    }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d730efb8/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java
new file mode 100644
index 0000000..921d5b8
--- /dev/null
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtPrepareForMigrationCommandWrapper.java
@@ -0,0 +1,89 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
+
+package com.cloud.hypervisor.kvm.resource.wrapper;
+
+import java.net.URISyntaxException;
+
+import org.apache.log4j.Logger;
+import org.libvirt.Connect;
+import org.libvirt.LibvirtException;
+
+import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.PrepareForMigrationAnswer;
+import com.cloud.agent.api.PrepareForMigrationCommand;
+import com.cloud.agent.api.to.DiskTO;
+import com.cloud.agent.api.to.NicTO;
+import com.cloud.agent.api.to.VirtualMachineTO;
+import com.cloud.exception.InternalErrorException;
+import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
+import com.cloud.resource.CommandWrapper;
+import com.cloud.storage.Volume;
+
+public final class LibvirtPrepareForMigrationCommandWrapper extends CommandWrapper<PrepareForMigrationCommand, Answer, LibvirtComputingResource> {
+
+    private static final Logger s_logger = Logger.getLogger(LibvirtPrepareForMigrationCommandWrapper.class);
+
+    @Override
+    public Answer execute(final PrepareForMigrationCommand command, final LibvirtComputingResource libvirtComputingResource) {
+        final VirtualMachineTO vm = command.getVirtualMachine();
+        if (s_logger.isDebugEnabled()) {
+            s_logger.debug("Preparing host for migrating " + vm);
+        }
+
+        final NicTO[] nics = vm.getNics();
+
+        boolean skipDisconnect = false;
+
+        try {
+            final LibvirtConnectionWrapper libvirtConnectionWrapper = libvirtComputingResource.getLibvirtConnectionWrapper();
+
+            final Connect conn = libvirtConnectionWrapper.getConnectionByVmName(vm.getName());
+            for (final NicTO nic : nics) {
+                libvirtComputingResource.getVifDriver(nic.getType()).plug(nic, null, "");
+            }
+
+            /* setup disks, e.g for iso */
+            final DiskTO[] volumes = vm.getDisks();
+            for (final DiskTO volume : volumes) {
+                if (volume.getType() == Volume.Type.ISO) {
+                    libvirtComputingResource.getVolumePath(conn, volume);
+                }
+            }
+
+            skipDisconnect = true;
+
+            if (!libvirtComputingResource.getStoragePoolMgr().connectPhysicalDisksViaVmSpec(vm)) {
+                return new PrepareForMigrationAnswer(command, "failed to connect physical disks to host");
+            }
+
+            return new PrepareForMigrationAnswer(command);
+        } catch (final LibvirtException e) {
+            return new PrepareForMigrationAnswer(command, e.toString());
+        } catch (final InternalErrorException e) {
+            return new PrepareForMigrationAnswer(command, e.toString());
+        } catch (final URISyntaxException e) {
+            return new PrepareForMigrationAnswer(command, e.toString());
+        } finally {
+            if (!skipDisconnect) {
+                libvirtComputingResource.getStoragePoolMgr().disconnectPhysicalDisksViaVmSpec(vm);
+            }
+        }
+    }
+}
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d730efb8/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
index e76a4eb..d8ca27f 100644
--- a/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
+++ b/plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
@@ -21,10 +21,12 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
 import java.util.Hashtable;
 
 import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CheckHealthCommand;
 import com.cloud.agent.api.Command;
 import com.cloud.agent.api.GetHostStatsCommand;
 import com.cloud.agent.api.GetVmDiskStatsCommand;
 import com.cloud.agent.api.GetVmStatsCommand;
+import com.cloud.agent.api.PrepareForMigrationCommand;
 import com.cloud.agent.api.RebootCommand;
 import com.cloud.agent.api.RebootRouterCommand;
 import com.cloud.agent.api.StopCommand;
@@ -56,6 +58,8 @@ public class LibvirtRequestWrapper extends RequestWrapper {
         linbvirtCommands.put(RebootRouterCommand.class, new LibvirtRebootRouterCommandWrapper());
         linbvirtCommands.put(RebootCommand.class, new LibvirtRebootCommandWrapper());
         linbvirtCommands.put(GetHostStatsCommand.class, new LibvirtGetHostStatsCommandWrapper());
+        linbvirtCommands.put(CheckHealthCommand.class, new LibvirtCheckHealthCommandWrapper());
+        linbvirtCommands.put(PrepareForMigrationCommand.class, new LibvirtPrepareForMigrationCommandWrapper());
 
         resources.put(LibvirtComputingResource.class, linbvirtCommands);
     }

http://git-wip-us.apache.org/repos/asf/cloudstack/blob/d730efb8/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
----------------------------------------------------------------------
diff --git a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
index 85f2c74..9b4b421 100644
--- a/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
+++ b/plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
@@ -62,19 +62,26 @@ import org.w3c.dom.Document;
 import org.xml.sax.SAXException;
 
 import com.cloud.agent.api.Answer;
+import com.cloud.agent.api.CheckHealthCommand;
 import com.cloud.agent.api.GetHostStatsCommand;
 import com.cloud.agent.api.GetVmDiskStatsCommand;
 import com.cloud.agent.api.GetVmStatsCommand;
+import com.cloud.agent.api.PrepareForMigrationCommand;
 import com.cloud.agent.api.RebootCommand;
 import com.cloud.agent.api.RebootRouterCommand;
 import com.cloud.agent.api.StopCommand;
 import com.cloud.agent.api.VmStatsEntry;
+import com.cloud.agent.api.to.DiskTO;
+import com.cloud.agent.api.to.NicTO;
 import com.cloud.agent.api.to.VirtualMachineTO;
 import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource;
 import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.DiskDef;
 import com.cloud.hypervisor.kvm.resource.LibvirtVMDef.InterfaceDef;
 import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtConnectionWrapper;
 import com.cloud.hypervisor.kvm.resource.wrapper.LibvirtRequestWrapper;
+import com.cloud.hypervisor.kvm.storage.KVMStoragePoolManager;
+import com.cloud.network.Networks.TrafficType;
+import com.cloud.storage.Volume;
 import com.cloud.template.VirtualMachineTemplate.BootloaderType;
 import com.cloud.utils.Pair;
 import com.cloud.vm.VirtualMachine;
@@ -587,6 +594,9 @@ public class LibvirtComputingResourceTest {
 
     @Test(expected = NumberFormatException.class)
     public void testGetHostStatsCommand() {
+        // A bit difficult top test due to the logger being passed and the parser itself relying on the connection.
+        // Have to spend some more time afterwards in order to refactor the wrapper itself.
+
         final String uuid = "e8d6b4d0-bc6d-4613-b8bb-cb9e0600f3c6";
         final GetHostStatsCommand command = new GetHostStatsCommand(uuid, "summer", 1l);
 
@@ -596,4 +606,66 @@ public class LibvirtComputingResourceTest {
         final Answer answer = wrapper.execute(command, libvirtComputingResource);
         assertFalse(answer.getResult());
     }
+
+    @Test
+    public void testCheckHealthCommand() {
+        // A bit difficult top test due to the logger being passed and the parser itself relying on the connection.
+        // Have to spend some more time afterwards in order to refactor the wrapper itself.
+
+        final CheckHealthCommand command = new CheckHealthCommand();
+
+        final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
+        assertNotNull(wrapper);
+
+        final Answer answer = wrapper.execute(command, libvirtComputingResource);
+        assertTrue(answer.getResult());
+    }
+
+    @Test
+    public void testPrepareForMigrationCommand() {
+        final Connect conn = Mockito.mock(Connect.class);
+        final LibvirtConnectionWrapper libvirtConnectionWrapper = Mockito.mock(LibvirtConnectionWrapper.class);
+
+        final VirtualMachineTO vm = Mockito.mock(VirtualMachineTO.class);
+        final KVMStoragePoolManager storagePoolManager = Mockito.mock(KVMStoragePoolManager.class);
+        final NicTO nicTO = Mockito.mock(NicTO.class);
+        final DiskTO diskTO = Mockito.mock(DiskTO.class);
+        final VifDriver vifDriver = Mockito.mock(VifDriver.class);
+
+        final PrepareForMigrationCommand command = new PrepareForMigrationCommand(vm);
+
+        when(libvirtComputingResource.getLibvirtConnectionWrapper()).thenReturn(libvirtConnectionWrapper);
+        try {
+            when(libvirtConnectionWrapper.getConnectionByVmName(vm.getName())).thenReturn(conn);
+        } catch (final LibvirtException e) {
+            fail(e.getMessage());
+        }
+
+        when(vm.getNics()).thenReturn(new NicTO[]{nicTO});
+        when(vm.getDisks()).thenReturn(new DiskTO[]{diskTO});
+
+        when(nicTO.getType()).thenReturn(TrafficType.Guest);
+        when(diskTO.getType()).thenReturn(Volume.Type.ISO);
+
+        when(libvirtComputingResource.getVifDriver(nicTO.getType())).thenReturn(vifDriver);
+        when(libvirtComputingResource.getStoragePoolMgr()).thenReturn(storagePoolManager);
+
+        final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
+        assertNotNull(wrapper);
+
+        final Answer answer = wrapper.execute(command, libvirtComputingResource);
+        assertFalse(answer.getResult());
+
+        verify(libvirtComputingResource, times(1)).getLibvirtConnectionWrapper();
+        try {
+            verify(libvirtConnectionWrapper, times(1)).getConnectionByVmName(vm.getName());
+        } catch (final LibvirtException e) {
+            fail(e.getMessage());
+        }
+
+        verify(libvirtComputingResource, times(1)).getStoragePoolMgr();
+        verify(vm, times(1)).getNics();
+        verify(vm, times(1)).getDisks();
+        verify(diskTO, times(1)).getType();
+    }
 }
\ No newline at end of file