Skip to content

Commit

Permalink
Worked around some possible runtime exceptions introduced in PR 780.
Browse files Browse the repository at this point in the history
  • Loading branch information
@rafaelweingartner
rafaelweingartner committed Apr 29, 2016
1 parent 7ad3c5e commit 6f659e6
Show file tree
Hide file tree
Showing 3 changed files with 84 additions and 44 deletions.
43 changes: 24 additions & 19 deletions plugins/hypervisors/kvm/src/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,17 @@
import org.apache.cloudstack.utils.qemu.QemuImg.PhysicalDiskFormat;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.math.NumberUtils;
import org.apache.log4j.Logger;
import org.libvirt.Connect;
import org.libvirt.Domain;
import org.libvirt.DomainBlockStats;
import org.libvirt.DomainInfo;
import org.libvirt.MemoryStatistic;
import org.libvirt.DomainInfo.DomainState;
import org.libvirt.DomainInterfaceStats;
import org.libvirt.LibvirtException;
import org.libvirt.MemoryStatistic;
import org.libvirt.NodeInfo;

import com.cloud.agent.api.Answer;
Expand Down Expand Up @@ -189,7 +191,6 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
private String _clusterId;

private long _hvVersion;
private long _kernelVersion;
private int _timeout;
private static final int NUMMEMSTATS =2;

Expand Down Expand Up @@ -958,13 +959,6 @@ public boolean configure(final String name, final Map<String, Object> params) th
storageProcessor.configure(name, params);
storageHandler = new StorageSubsystemCommandHandlerBase(storageProcessor);

final String unameKernelVersion = Script.runSimpleBashScript("uname -r");
final String[] kernelVersions = unameKernelVersion.split("[\\.\\-]");
_kernelVersion = Integer.parseInt(kernelVersions[0]) * 1000 * 1000 + (long)Integer.parseInt(kernelVersions[1]) * 1000 + Integer.parseInt(kernelVersions[2]);

/* Disable this, the code using this is pretty bad and non portable
* getOsVersion();
*/
return true;
}

Expand Down Expand Up @@ -3020,27 +3014,26 @@ private class VmStats {
long _ioWrote;
long _bytesRead;
long _bytesWrote;
long _intmemfree;
long _memory;
long _maxmemory;
Calendar _timestamp;
}

public VmStatsEntry getVmStat(final Connect conn, final String vmName) throws LibvirtException {
Domain dm = null;
try {
dm = getDomain(conn, vmName);
final DomainInfo info = dm.getInfo();
final MemoryStatistic[] mems = dm.memoryStats(NUMMEMSTATS); //number of memory statistics required.

if (dm == null) {
return null;
}
DomainInfo info = dm.getInfo();
final VmStatsEntry stats = new VmStatsEntry();

stats.setNumCPUs(info.nrVirtCpu);
stats.setEntityType("vm");

stats.setMemoryKBs(info.maxMem);
stats.setTargetMemoryKBs(info.memory);
stats.setIntFreeMemoryKBs((double) mems[0].getValue());
stats.setIntFreeMemoryKBs(getMemoryFreeInKBs(dm));

/* get cpu utilization */
VmStats oldStats = null;

Expand Down Expand Up @@ -3125,9 +3118,6 @@ public VmStatsEntry getVmStat(final Connect conn, final String vmName) throws Li
newStat._bytesRead = bytes_rd;
newStat._bytesWrote = bytes_wr;
newStat._timestamp = now;
newStat._intmemfree = mems[0].getValue();
newStat._memory = info.memory;
newStat._maxmemory = info.maxMem;
_vmStats.put(vmName, newStat);
return stats;
} finally {
Expand All @@ -3137,6 +3127,21 @@ public VmStatsEntry getVmStat(final Connect conn, final String vmName) throws Li
}
}

/**
* This method retrieves the memory statistics from the domain given as parameters.
* If no memory statistic is found, it will return {@link NumberUtils#LONG_ZERO} as the value of free memory in the domain.
* If it can retrieve the domain memory statistics, it will return the free memory statistic; that means, it returns the value at the first position of the array returned by {@link Domain#memoryStats(int)}.
*
* @return the amount of free memory in KBs
*/
protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException {
MemoryStatistic[] mems = dm.memoryStats(NUMMEMSTATS);
if (ArrayUtils.isEmpty(mems)) {
return NumberUtils.LONG_ZERO;
}
return mems[0].getValue();
}

private boolean canBridgeFirewall(final String prvNic) {
final Script cmd = new Script(_securityGroupPath, _timeout, s_logger);
cmd.add("can_bridge_firewall");
Expand Down
69 changes: 51 additions & 18 deletions .../hypervisors/kvm/test/com/cloud/hypervisor/kvm/resource/LibvirtComputingResourceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,10 @@
import org.libvirt.DomainInfo.DomainState;
import org.libvirt.DomainInterfaceStats;
import org.libvirt.LibvirtException;
import org.libvirt.MemoryStatistic;
import org.libvirt.NodeInfo;
import org.libvirt.StorageVol;
import org.libvirt.MemoryStatistic;

import org.libvirt.jna.virDomainMemoryStats;
import org.mockito.Matchers;
import org.mockito.Mock;
import org.mockito.Mockito;
Expand Down Expand Up @@ -182,8 +182,8 @@ public class LibvirtComputingResourceTest {
@Mock
private LibvirtComputingResource libvirtComputingResource;

String _hyperVisorType = "kvm";
Random _random = new Random();
String hyperVisorType = "kvm";
Random random = new Random();

/**
This test tests if the Agent can handle a vmSpec coming
Expand All @@ -194,10 +194,10 @@ public class LibvirtComputingResourceTest {
*/
@Test
public void testCreateVMFromSpecLegacy() {
final int id = _random.nextInt(65534);
final int id = random.nextInt(65534);
final String name = "test-instance-1";

final int cpus = _random.nextInt(2) + 1;
final int cpus = random.nextInt(2) + 1;
final int speed = 1024;
final int minRam = 256 * 1024;
final int maxRam = 512 * 1024;
Expand All @@ -213,7 +213,7 @@ public void testCreateVMFromSpecLegacy() {
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");

final LibvirtVMDef vm = lcr.createVMFromSpec(to);
vm.setHvsType(_hyperVisorType);
vm.setHvsType(hyperVisorType);

verifyVm(to, vm);
}
Expand All @@ -223,7 +223,7 @@ public void testCreateVMFromSpecLegacy() {
*/
@Test
public void testCreateVMFromSpecWithTopology6() {
final int id = _random.nextInt(65534);
final int id = random.nextInt(65534);
final String name = "test-instance-1";

final int cpus = 12;
Expand All @@ -243,7 +243,7 @@ public void testCreateVMFromSpecWithTopology6() {
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");

final LibvirtVMDef vm = lcr.createVMFromSpec(to);
vm.setHvsType(_hyperVisorType);
vm.setHvsType(hyperVisorType);

verifyVm(to, vm);
}
Expand All @@ -253,7 +253,7 @@ public void testCreateVMFromSpecWithTopology6() {
*/
@Test
public void testCreateVMFromSpecWithTopology4() {
final int id = _random.nextInt(65534);
final int id = random.nextInt(65534);
final String name = "test-instance-1";

final int cpus = 8;
Expand All @@ -273,7 +273,7 @@ public void testCreateVMFromSpecWithTopology4() {
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");

final LibvirtVMDef vm = lcr.createVMFromSpec(to);
vm.setHvsType(_hyperVisorType);
vm.setHvsType(hyperVisorType);

verifyVm(to, vm);
}
Expand All @@ -287,10 +287,10 @@ public void testCreateVMFromSpecWithTopology4() {
*/
@Test
public void testCreateVMFromSpec() {
final int id = _random.nextInt(65534);
final int id = random.nextInt(65534);
final String name = "test-instance-1";

final int cpus = _random.nextInt(2) + 1;
final int cpus = random.nextInt(2) + 1;
final int minSpeed = 1024;
final int maxSpeed = 2048;
final int minRam = 256 * 1024;
Expand All @@ -308,7 +308,7 @@ public void testCreateVMFromSpec() {
to.setUuid("b0f0a72d-7efb-3cad-a8ff-70ebf30b3af9");

final LibvirtVMDef vm = lcr.createVMFromSpec(to);
vm.setHvsType(_hyperVisorType);
vm.setHvsType(hyperVisorType);

verifyVm(to, vm);
}
Expand Down Expand Up @@ -718,10 +718,9 @@ public void testGetVmDiskStatsCommand() {
}
}

@SuppressWarnings("unchecked")
@Test
@SuppressWarnings("unchecked")
public void testGetVmDiskStatsCommandException() {
Mockito.mock(Connect.class);
final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class);

final String vmName = "Test";
Expand Down Expand Up @@ -941,7 +940,6 @@ public void testRebootRouterCommandConnect() {
public void testGetHostStatsCommand() {
// A bit difficult to 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.
Mockito.mock(LibvirtUtilitiesHelper.class);
final CPUStat cpuStat = Mockito.mock(CPUStat.class);
final MemStat memStat = Mockito.mock(MemStat.class);

Expand Down Expand Up @@ -3531,7 +3529,6 @@ public void testNetworkUsageCommandVpcNoOption() {
verify(libvirtComputingResource, times(1)).configureVPCNetworkUsage(command.getPrivateIP(), command.getGatewayIP(), command.getOption(), command.getVpcCIDR());
}

@SuppressWarnings("unchecked")
@Test
public void testCreatePrivateTemplateFromVolumeCommand() {
//Simple test used to make sure the flow (LibvirtComputingResource => Request => CommandWrapper) is working.
Expand Down Expand Up @@ -5040,4 +5037,40 @@ public void testIsInterface () {
}
}
}

@Test
public void testMemoryFreeInKBsDomainReturningOfSomeMemoryStatistics() throws LibvirtException {
LibvirtComputingResource libvirtComputingResource = new LibvirtComputingResource();

MemoryStatistic[] mem = createMemoryStatisticFreeMemory100();
Domain domainMock = getDomainConfiguredToReturnMemoryStatistic(mem);
long memoryFreeInKBs = libvirtComputingResource.getMemoryFreeInKBs(domainMock);

Assert.assertEquals(100, memoryFreeInKBs);
}

@Test
public void testMemoryFreeInKBsDomainReturningNoMemoryStatistics() throws LibvirtException {
LibvirtComputingResource libvirtComputingResource = new LibvirtComputingResource();

Domain domainMock = getDomainConfiguredToReturnMemoryStatistic(null);
long memoryFreeInKBs = libvirtComputingResource.getMemoryFreeInKBs(domainMock);

Assert.assertEquals(0, memoryFreeInKBs);
}

private MemoryStatistic[] createMemoryStatisticFreeMemory100() {
virDomainMemoryStats stat = new virDomainMemoryStats();
stat.val = 100;

MemoryStatistic[] mem = new MemoryStatistic[2];
mem[0] = new MemoryStatistic(stat);
return mem;
}

private Domain getDomainConfiguredToReturnMemoryStatistic(MemoryStatistic[] mem) throws LibvirtException {
Domain domainMock = Mockito.mock(Domain.class);
when(domainMock.memoryStats(2)).thenReturn(mem);
return domainMock;
}
}
16 changes: 9 additions & 7 deletions ...hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,8 @@ public String toString() {
}
}

private final static int BASE_TO_CONVERT_BYTES_INTO_KILOBYTES = 1024;

protected static final XenServerConnectionPool ConnPool = XenServerConnectionPool.getInstance();
// static min values for guests on xenserver
private static final long mem_128m = 134217728L;
Expand Down Expand Up @@ -3323,19 +3325,19 @@ public HashMap<String, VmStatsEntry> getVmStats(final Connection conn, final Get
vmStatsAnswer.setNumCPUs(vmStatsAnswer.getNumCPUs() + 1);
vmStatsAnswer.setCPUUtilization(vmStatsAnswer.getCPUUtilization() + getDataAverage(dataNode, col, numRows));
} else if (param.matches("vif_\\d*_rx")) {
vmStatsAnswer.setNetworkReadKBs(vmStatsAnswer.getNetworkReadKBs() + getDataAverage(dataNode, col, numRows) / 1000);
vmStatsAnswer.setNetworkReadKBs(vmStatsAnswer.getNetworkReadKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.matches("vif_\\d*_tx")) {
vmStatsAnswer.setNetworkWriteKBs(vmStatsAnswer.getNetworkWriteKBs() + getDataAverage(dataNode, col, numRows) / 1000);
vmStatsAnswer.setNetworkWriteKBs(vmStatsAnswer.getNetworkWriteKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.matches("vbd_.*_read")) {
vmStatsAnswer.setDiskReadKBs(vmStatsAnswer.getDiskReadKBs() + getDataAverage(dataNode, col, numRows) / 1000);
vmStatsAnswer.setDiskReadKBs(vmStatsAnswer.getDiskReadKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.matches("vbd_.*_write")) {
vmStatsAnswer.setDiskWriteKBs(vmStatsAnswer.getDiskWriteKBs() + getDataAverage(dataNode, col, numRows) / 1000);
vmStatsAnswer.setDiskWriteKBs(vmStatsAnswer.getDiskWriteKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.contains("memory_internal_free")) {
vmStatsAnswer.setIntFreeMemoryKBs(vmStatsAnswer.getIntFreeMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
vmStatsAnswer.setIntFreeMemoryKBs(vmStatsAnswer.getIntFreeMemoryKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.contains("memory_target")) {
vmStatsAnswer.setTargetMemoryKBs(vmStatsAnswer.getTargetMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
vmStatsAnswer.setTargetMemoryKBs(vmStatsAnswer.getTargetMemoryKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.contains("memory")) {
vmStatsAnswer.setMemoryKBs(vmStatsAnswer.getMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
vmStatsAnswer.setMemoryKBs(vmStatsAnswer.getMemoryKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
}

}
Expand Down

0 comments on commit 6f659e6

Please sign in to comment.