etan39
/
cloudstack
mirror of https://github.com/apache/cloudstack.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Refactoring the LibvirtComputingResource 

- Adding LibvirtNetworkElementCommandWrapper and LibvirtStorageSubSystemCommandWrapper
  - 2 unit tests added
  - KVM hypervisor plugin with 22.2% coverage

I also refactored the StorageSubSystemCommand interface into an abstract class
  - Remove the pseudo-multiple-inheritance implementation
    - The StorageSubSystemCommand was an interface, not related to the Command class
      and its implementation were extending the Command class anyway. The whole structure is better now.
This commit is contained in:
wilderrodrigues 2015-05-06 14:31:02 +02:00
parent 08106e34d0
commit 09656ca84e
16 changed files with 182 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
core/src/org/apache/cloudstack/storage/command/AttachCommand.java
View File

@ -19,15 +19,14 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DiskTO;
public final class AttachCommand extends Command implements StorageSubSystemCommand {
public final class AttachCommand extends StorageSubSystemCommand {
private DiskTO disk;
private String vmName;
private boolean inSeq = false;
private boolean inSeq;
public AttachCommand(DiskTO disk, String vmName) {
public AttachCommand(final DiskTO disk, final String vmName) {
super();
this.disk = disk;
this.vmName = vmName;
@ -42,7 +41,7 @@ public final class AttachCommand extends Command implements StorageSubSystemComm
return disk;
}
public void setDisk(DiskTO disk) {
public void setDisk(final DiskTO disk) {
this.disk = disk;
}
@ -50,12 +49,12 @@ public final class AttachCommand extends Command implements StorageSubSystemComm
return vmName;
}
public void setVmName(String vmName) {
public void setVmName(final String vmName) {
this.vmName = vmName;
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
this.inSeq = inSeq;
}
}

7
core/src/org/apache/cloudstack/storage/command/AttachPrimaryDataStoreCmd.java
View File

@ -19,17 +19,16 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
public final class AttachPrimaryDataStoreCmd extends Command implements StorageSubSystemCommand {
public final class AttachPrimaryDataStoreCmd extends StorageSubSystemCommand {
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
private final String dataStore;
public AttachPrimaryDataStoreCmd(String uri) {
public AttachPrimaryDataStoreCmd(final String uri) {
super();
dataStore = uri;
}

31
core/src/org/apache/cloudstack/storage/command/CopyCommand.java
View File

@ -22,10 +22,9 @@ package org.apache.cloudstack.storage.command;
import java.util.HashMap;
import java.util.Map;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataTO;
public final class CopyCommand extends Command implements StorageSubSystemCommand {
public final class CopyCommand extends StorageSubSystemCommand {
private DataTO srcTO;
private DataTO destTO;
private DataTO cacheTO;
@ -33,28 +32,28 @@ public final class CopyCommand extends Command implements StorageSubSystemComman
private Map<String, String> options = new HashMap<String, String>();
private Map<String, String> options2 = new HashMap<String, String>();
public CopyCommand(DataTO srcData, DataTO destData, int timeout, boolean executeInSequence) {
public CopyCommand(final DataTO srcData, final DataTO destData, final int timeout, final boolean executeInSequence) {
super();
this.srcTO = srcData;
this.destTO = destData;
this.setWait(timeout);
srcTO = srcData;
destTO = destData;
setWait(timeout);
this.executeInSequence = executeInSequence;
}
public DataTO getDestTO() {
return this.destTO;
return destTO;
}
public void setSrcTO(DataTO srcTO) {
public void setSrcTO(final DataTO srcTO) {
this.srcTO = srcTO;
}
public void setDestTO(DataTO destTO) {
public void setDestTO(final DataTO destTO) {
this.destTO = destTO;
}
public DataTO getSrcTO() {
return this.srcTO;
return srcTO;
}
@Override
@ -66,15 +65,15 @@ public final class CopyCommand extends Command implements StorageSubSystemComman
return cacheTO;
}
public void setCacheTO(DataTO cacheTO) {
public void setCacheTO(final DataTO cacheTO) {
this.cacheTO = cacheTO;
}
public int getWaitInMillSeconds() {
return this.getWait() * 1000;
return getWait() * 1000;
}
public void setOptions(Map<String, String> options) {
public void setOptions(final Map<String, String> options) {
this.options = options;
}
@ -82,7 +81,7 @@ public final class CopyCommand extends Command implements StorageSubSystemComman
return options;
}
public void setOptions2(Map<String, String> options2) {
public void setOptions2(final Map<String, String> options2) {
this.options2 = options2;
}
@ -91,7 +90,7 @@ public final class CopyCommand extends Command implements StorageSubSystemComman
}
@Override
public void setExecuteInSequence(boolean inSeq) {
this.executeInSequence = inSeq;
public void setExecuteInSequence(final boolean inSeq) {
executeInSequence = inSeq;
}
}

7
core/src/org/apache/cloudstack/storage/command/CreateObjectCommand.java
View File

@ -19,13 +19,12 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataTO;
public final class CreateObjectCommand extends Command implements StorageSubSystemCommand {
public final class CreateObjectCommand extends StorageSubSystemCommand {
private DataTO data;
public CreateObjectCommand(DataTO obj) {
public CreateObjectCommand(final DataTO obj) {
super();
data = obj;
}
@ -44,7 +43,7 @@ public final class CreateObjectCommand extends Command implements StorageSubSyst
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

11
core/src/org/apache/cloudstack/storage/command/CreatePrimaryDataStoreCmd.java
View File

@ -19,18 +19,17 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
public final class CreatePrimaryDataStoreCmd extends Command implements StorageSubSystemCommand {
public final class CreatePrimaryDataStoreCmd extends StorageSubSystemCommand {
private final String dataStore;
public CreatePrimaryDataStoreCmd(String uri) {
public CreatePrimaryDataStoreCmd(final String uri) {
super();
this.dataStore = uri;
dataStore = uri;
}
public String getDataStore() {
return this.dataStore;
return dataStore;
}
@Override
@ -39,7 +38,7 @@ public final class CreatePrimaryDataStoreCmd extends Command implements StorageS
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

7
core/src/org/apache/cloudstack/storage/command/DeleteCommand.java
View File

@ -19,13 +19,12 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataTO;
public final class DeleteCommand extends Command implements StorageSubSystemCommand {
public final class DeleteCommand extends StorageSubSystemCommand {
private DataTO data;
public DeleteCommand(DataTO data) {
public DeleteCommand(final DataTO data) {
super();
this.data = data;
}
@ -44,7 +43,7 @@ public final class DeleteCommand extends Command implements StorageSubSystemComm
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

19
core/src/org/apache/cloudstack/storage/command/DettachCommand.java
View File

@ -19,10 +19,9 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DiskTO;
public class DettachCommand extends Command implements StorageSubSystemCommand {
public class DettachCommand extends StorageSubSystemCommand {
private DiskTO disk;
private String vmName;
private boolean _managed;
@ -30,7 +29,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
private String _storageHost;
private int _storagePort;
public DettachCommand(DiskTO disk, String vmName) {
public DettachCommand(final DiskTO disk, final String vmName) {
super();
this.disk = disk;
this.vmName = vmName;
@ -45,7 +44,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
return disk;
}
public void setDisk(DiskTO disk) {
public void setDisk(final DiskTO disk) {
this.disk = disk;
}
@ -53,11 +52,11 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
return vmName;
}
public void setVmName(String vmName) {
public void setVmName(final String vmName) {
this.vmName = vmName;
}
public void setManaged(boolean managed) {
public void setManaged(final boolean managed) {
_managed = managed;
}
@ -65,7 +64,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
return _managed;
}
public void set_iScsiName(String iScsiName) {
public void set_iScsiName(final String iScsiName) {
_iScsiName = iScsiName;
}
@ -73,7 +72,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
return _iScsiName;
}
public void setStorageHost(String storageHost) {
public void setStorageHost(final String storageHost) {
_storageHost = storageHost;
}
@ -81,7 +80,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
return _storageHost;
}
public void setStoragePort(int storagePort) {
public void setStoragePort(final int storagePort) {
_storagePort = storagePort;
}
@ -90,7 +89,7 @@ public class DettachCommand extends Command implements StorageSubSystemCommand {
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

9
core/src/org/apache/cloudstack/storage/command/ForgetObjectCmd.java
View File

@ -19,13 +19,12 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataTO;
public class ForgetObjectCmd extends Command implements StorageSubSystemCommand {
private DataTO dataTO;
public class ForgetObjectCmd extends StorageSubSystemCommand {
private final DataTO dataTO;
public ForgetObjectCmd(DataTO data) {
public ForgetObjectCmd(final DataTO data) {
dataTO = data;
}
@ -39,7 +38,7 @@ public class ForgetObjectCmd extends Command implements StorageSubSystemCommand
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

9
core/src/org/apache/cloudstack/storage/command/IntroduceObjectCmd.java
View File

@ -19,13 +19,12 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import com.cloud.agent.api.to.DataTO;
public class IntroduceObjectCmd extends Command implements StorageSubSystemCommand {
private DataTO dataTO;
public class IntroduceObjectCmd extends StorageSubSystemCommand {
private final DataTO dataTO;
public IntroduceObjectCmd(DataTO dataTO) {
public IntroduceObjectCmd(final DataTO dataTO) {
this.dataTO = dataTO;
}
@ -39,7 +38,7 @@ public class IntroduceObjectCmd extends Command implements StorageSubSystemComma
}
@Override
public void setExecuteInSequence(boolean inSeq) {
public void setExecuteInSequence(final boolean inSeq) {
}
}

14
core/src/org/apache/cloudstack/storage/command/SnapshotAndCopyCommand.java
View File

@ -19,18 +19,16 @@
package org.apache.cloudstack.storage.command;
import com.cloud.agent.api.Command;
import java.util.Map;
public final class SnapshotAndCopyCommand extends Command implements StorageSubSystemCommand {
private String _uuidOfSourceVdi;
private Map<String, String> _sourceDetails;
private Map<String, String> _destDetails;
public final class SnapshotAndCopyCommand extends StorageSubSystemCommand {
private final String _uuidOfSourceVdi;
private final Map<String, String> _sourceDetails;
private final Map<String, String> _destDetails;
private boolean _executeInSequence = true;
public SnapshotAndCopyCommand(String uuidOfSourceVdi, Map<String, String> sourceDetails, Map<String, String> destDetails) {
public SnapshotAndCopyCommand(final String uuidOfSourceVdi, final Map<String, String> sourceDetails, final Map<String, String> destDetails) {
_uuidOfSourceVdi = uuidOfSourceVdi;
_sourceDetails = sourceDetails;
_destDetails = destDetails;
@ -49,7 +47,7 @@ public final class SnapshotAndCopyCommand extends Command implements StorageSubS
}
@Override
public void setExecuteInSequence(boolean executeInSequence) {
public void setExecuteInSequence(final boolean executeInSequence) {
_executeInSequence = executeInSequence;
}

8
core/src/org/apache/cloudstack/storage/command/StorageSubSystemCommand.java
View File

@ -19,6 +19,8 @@
package org.apache.cloudstack.storage.command;
public interface StorageSubSystemCommand {
void setExecuteInSequence(boolean inSeq);
}
import com.cloud.agent.api.Command;
public abstract class StorageSubSystemCommand extends Command {
abstract void setExecuteInSequence(boolean inSeq);
}

9
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View File

@ -45,7 +45,6 @@ import javax.ejb.Local;
import javax.inject.Inject;
import javax.naming.ConfigurationException;
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
import org.apache.cloudstack.storage.to.PrimaryDataStoreTO;
import org.apache.cloudstack.storage.to.VolumeObjectTO;
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
@ -408,6 +407,10 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
return _resizeVolumePath;
}
public StorageSubsystemCommandHandler getStorageHandler() {
return storageHandler;
}
private static final class KeyValueInterpreter extends OutputInterpreter {
private final Map<String, String> map = new HashMap<String, String>();
@ -1257,10 +1260,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
try {
if (cmd instanceof StartCommand) {
return execute((StartCommand)cmd);
} else if (cmd instanceof NetworkElementCommand) {
return _virtRouterResource.executeRequest((NetworkElementCommand)cmd);
} else if (cmd instanceof StorageSubSystemCommand) {
return storageHandler.handleStorageCommands((StorageSubSystemCommand)cmd);
} else {
s_logger.warn("Unsupported command ");
return Answer.createUnsupportedCommandAnswer(cmd);

35
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtNetworkElementCommandWrapper.java Normal file
View File

@ -0,0 +1,35 @@
//
// 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.routing.NetworkElementCommand;
import com.cloud.agent.resource.virtualnetwork.VirtualRoutingResource;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
public final class LibvirtNetworkElementCommandWrapper extends CommandWrapper<NetworkElementCommand, Answer, LibvirtComputingResource> {
@Override
public Answer execute(final NetworkElementCommand command, final LibvirtComputingResource libvirtComputingResource) {
final VirtualRoutingResource virtRouterResource = libvirtComputingResource.getVirtRouterResource();
return virtRouterResource.executeRequest(command);
}
}

5
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtRequestWrapper.java
View File

@ -20,6 +20,8 @@ package com.cloud.hypervisor.kvm.resource.wrapper;
import java.util.Hashtable;
import org.apache.cloudstack.storage.command.StorageSubSystemCommand;
import com.cloud.agent.api.Answer;
import com.cloud.agent.api.AttachIsoCommand;
import com.cloud.agent.api.AttachVolumeCommand;
@ -70,6 +72,7 @@ import com.cloud.agent.api.UpgradeSnapshotCommand;
import com.cloud.agent.api.check.CheckSshCommand;
import com.cloud.agent.api.proxy.CheckConsoleProxyLoadCommand;
import com.cloud.agent.api.proxy.WatchConsoleProxyLoadCommand;
import com.cloud.agent.api.routing.NetworkElementCommand;
import com.cloud.agent.api.storage.CopyVolumeCommand;
import com.cloud.agent.api.storage.CreateCommand;
import com.cloud.agent.api.storage.DestroyCommand;
@ -150,6 +153,8 @@ public class LibvirtRequestWrapper extends RequestWrapper {
linbvirtCommands.put(CopyVolumeCommand.class, new LibvirtCopyVolumeCommandWrapper());
linbvirtCommands.put(PvlanSetupCommand.class, new LibvirtPvlanSetupCommandWrapper());
linbvirtCommands.put(ResizeVolumeCommand.class, new LibvirtResizeVolumeCommandWrapper());
linbvirtCommands.put(NetworkElementCommand.class, new LibvirtNetworkElementCommandWrapper());
linbvirtCommands.put(StorageSubSystemCommand.class, new LibvirtStorageSubSystemCommandWrapper());
resources.put(LibvirtComputingResource.class, linbvirtCommands);
}

36
plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtStorageSubSystemCommandWrapper.java Normal file
View File

@ -0,0 +1,36 @@
//
// 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 org.apache.cloudstack.storage.command.StorageSubSystemCommand;
import com.cloud.agent.api.Answer;
import com.cloud.hypervisor.kvm.resource.LibvirtComputingResource;
import com.cloud.resource.CommandWrapper;
import com.cloud.storage.resource.StorageSubsystemCommandHandler;
public final class LibvirtStorageSubSystemCommandWrapper extends CommandWrapper<StorageSubSystemCommand, Answer, LibvirtComputingResource> {
@Override
public Answer execute(final StorageSubSystemCommand command, final LibvirtComputingResource libvirtComputingResource) {
final StorageSubsystemCommandHandler handler = libvirtComputingResource.getStorageHandler();
return handler.handleStorageCommands(command);
}
}

39
plugins/hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
View File

@ -46,6 +46,8 @@ import javax.xml.xpath.XPathConstants;
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
import org.apache.cloudstack.storage.command.AttachAnswer;
import org.apache.cloudstack.storage.command.AttachCommand;
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
import org.apache.commons.lang.SystemUtils;
import org.junit.Assert;
@ -76,6 +78,8 @@ import com.cloud.agent.api.BackupSnapshotCommand;
import com.cloud.agent.api.CheckHealthCommand;
import com.cloud.agent.api.CheckNetworkCommand;
import com.cloud.agent.api.CheckOnHostCommand;
import com.cloud.agent.api.CheckRouterAnswer;
import com.cloud.agent.api.CheckRouterCommand;
import com.cloud.agent.api.CheckVirtualMachineCommand;
import com.cloud.agent.api.CleanupNetworkRulesCmd;
import com.cloud.agent.api.CreatePrivateTemplateFromSnapshotCommand;
@ -152,6 +156,7 @@ import com.cloud.storage.Storage.StoragePoolType;
import com.cloud.storage.StorageLayer;
import com.cloud.storage.StoragePool;
import com.cloud.storage.Volume;
import com.cloud.storage.resource.StorageSubsystemCommandHandler;
import com.cloud.storage.template.Processor;
import com.cloud.storage.template.Processor.FormatInfo;
import com.cloud.storage.template.TemplateLocation;
@ -4506,4 +4511,38 @@ public class LibvirtComputingResourceTest {
verify(libvirtComputingResource, times(1)).getStoragePoolMgr();
}
@Test
public void testNetworkElementCommand() {
final CheckRouterCommand command = new CheckRouterCommand();
final VirtualRoutingResource virtRouterResource = Mockito.mock(VirtualRoutingResource.class);
when(libvirtComputingResource.getVirtRouterResource()).thenReturn(virtRouterResource);
when(virtRouterResource.executeRequest(command)).thenReturn(new CheckRouterAnswer(command, "mock_resource"));
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertFalse(answer.getResult());
}
@Test
public void testStorageSubSystemCommand() {
final DiskTO disk = Mockito.mock(DiskTO.class);
final String vmName = "Test";
final AttachCommand command = new AttachCommand(disk, vmName);
final StorageSubsystemCommandHandler handler = Mockito.mock(StorageSubsystemCommandHandler.class);
when(libvirtComputingResource.getStorageHandler()).thenReturn(handler);
when(handler.handleStorageCommands(command)).thenReturn(new AttachAnswer(disk));
final LibvirtRequestWrapper wrapper = LibvirtRequestWrapper.getInstance();
assertNotNull(wrapper);
final Answer answer = wrapper.execute(command, libvirtComputingResource);
assertTrue(answer.getResult());
}
}