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 Mar 19, 2016
1 parent de173bd commit 6ba07b9
Show file tree
Hide file tree
Showing 3 changed files with 87 additions and 44 deletions.
44 changes: 25 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 @@ -957,14 +958,6 @@ public boolean configure(final String name, final Map<String, Object> params) th
final KVMStorageProcessor storageProcessor = new KVMStorageProcessor(_storagePoolMgr, this);
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 @@ -2210,7 +2203,7 @@ private void createVif(final LibvirtVMDef vm, final NicTO nic, final String nicA

if (s_logger.isDebugEnabled()) {
s_logger.debug("NIC with MAC " + nic.getMac() + " and BroadcastDomainType " + nic.getBroadcastType() + " in network(" + nic.getGateway() + "/" + nic.getNetmask()
+ ") is " + nic.getType() + " traffic type. So, vsp-vr-ip " + vrIp + " is set in the metadata");
+ ") is " + nic.getType() + " traffic type. So, vsp-vr-ip " + vrIp + " is set in the metadata");
}
}

Expand Down Expand Up @@ -3020,18 +3013,17 @@ 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);
if (dm == null) {
return null;
}
final DomainInfo info = dm.getInfo();
final MemoryStatistic[] mems = dm.memoryStats(NUMMEMSTATS); //number of memory statistics required.

final VmStatsEntry stats = new VmStatsEntry();

Expand All @@ -3040,7 +3032,7 @@ public VmStatsEntry getVmStat(final Connect conn, final String vmName) throws Li

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 +3117,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 +3126,23 @@ 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)}.
*
* @param dm
* @return the amount of free memory in KBs
* @throws LibvirtException
*/
protected long getMemoryFreeInKBs(Domain dm) throws LibvirtException {
final 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
45 changes: 39 additions & 6 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 @@ -64,10 +64,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 @@ -717,10 +717,9 @@ public void testGetVmDiskStatsCommand() {
}
}

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

final String vmName = "Test";
Expand Down Expand Up @@ -940,7 +939,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.
final LibvirtUtilitiesHelper libvirtUtilitiesHelper = Mockito.mock(LibvirtUtilitiesHelper.class);
final CPUStat cpuStat = Mockito.mock(CPUStat.class);
final MemStat memStat = Mockito.mock(MemStat.class);

Expand Down Expand Up @@ -3521,7 +3519,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 @@ -5030,4 +5027,40 @@ public void testIsInterface () {
}
}
}

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

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

Assert.assertEquals(0, memoryFreeInKBs);
}

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

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

Assert.assertEquals(100, 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;
}
}
42 changes: 23 additions & 19 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 @@ -167,6 +167,8 @@
@Local(value = ServerResource.class)
public abstract class CitrixResourceBase implements ServerResource, HypervisorResource, VirtualRouterDeployer {

private final static int BASE_TO_CONVERT_BYTES_INTO_KILOBYTES = 1024;

public enum SRType {
EXT, FILE, ISCSI, ISO, LVM, LVMOHBA, LVMOISCSI, NFS;

Expand Down Expand Up @@ -2306,7 +2308,7 @@ public SR getIscsiSR(final Connection conn, final String srNameLabel, final Stri
}
if (target.equals(dc.get("target")) && targetiqn.equals(dc.get("targetIQN")) && lunid.equals(dc.get("lunid"))) {
throw new CloudRuntimeException("There is a SR using the same configuration target:" + dc.get("target") + ", targetIQN:" + dc.get("targetIQN")
+ ", lunid:" + dc.get("lunid") + " for pool " + srNameLabel + "on host:" + _host.getUuid());
+ ", lunid:" + dc.get("lunid") + " for pool " + srNameLabel + "on host:" + _host.getUuid());
}
}
deviceConfig.put("target", target);
Expand Down Expand Up @@ -2617,7 +2619,7 @@ protected XsLocalNetwork getManagementNetwork(final Connection conn) throws XmlR
final Bond bond = mgmtPifRec.bondSlaveOf;
if (!isRefNull(bond)) {
final String msg = "Management interface is on slave(" + mgmtPifRec.uuid + ") of bond(" + bond.getUuid(conn) + ") on host(" + _host.getUuid()
+ "), please move management interface to bond!";
+ "), please move management interface to bond!";
s_logger.warn(msg);
throw new CloudRuntimeException(msg);
}
Expand Down Expand Up @@ -2837,7 +2839,7 @@ public SR getNfsSR(final Connection conn, final String poolid, final String uuid

if (server.equals(dc.get("server")) && serverpath.equals(dc.get("serverpath"))) {
throw new CloudRuntimeException("There is a SR using the same configuration server:" + dc.get("server") + ", serverpath:" + dc.get("serverpath")
+ " for pool " + uuid + " on host:" + _host.getUuid());
+ " for pool " + uuid + " on host:" + _host.getUuid());
}

}
Expand Down Expand Up @@ -3324,20 +3326,22 @@ public HashMap<String, VmStatsEntry> getVmStats(final Connection conn, final Get
if (param.contains("cpu")) {
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);
} else if (param.matches("vif_\\d*_tx")) {
vmStatsAnswer.setNetworkWriteKBs(vmStatsAnswer.getNetworkWriteKBs() + getDataAverage(dataNode, col, numRows) / 1000);
} else if (param.matches("vbd_.*_read")) {
vmStatsAnswer.setDiskReadKBs(vmStatsAnswer.getDiskReadKBs() + getDataAverage(dataNode, col, numRows) / 1000);
} else if (param.matches("vbd_.*_write")) {
vmStatsAnswer.setDiskWriteKBs(vmStatsAnswer.getDiskWriteKBs() + getDataAverage(dataNode, col, numRows) / 1000);
} else if (param.contains("memory_internal_free")) {
vmStatsAnswer.setIntFreeMemoryKBs(vmStatsAnswer.getIntFreeMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
} else if (param.contains("memory_target")) {
vmStatsAnswer.setTargetMemoryKBs(vmStatsAnswer.getTargetMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
} else if (param.contains("memory")) {
vmStatsAnswer.setMemoryKBs(vmStatsAnswer.getMemoryKBs() + getDataAverage(dataNode, col, numRows) / 1024);
} else {
if (param.matches("vif_\\d*_rx")) {
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) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.matches("vbd_.*_read")) {
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) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.contains("memory_internal_free")) {
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) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
} else if (param.contains("memory")) {
vmStatsAnswer.setMemoryKBs(vmStatsAnswer.getMemoryKBs() + getDataAverage(dataNode, col, numRows) / BASE_TO_CONVERT_BYTES_INTO_KILOBYTES);
}
}

}
Expand Down Expand Up @@ -5080,8 +5084,8 @@ public boolean createVmdataFiles(final String vmName, final List<String[]> vmDat
if (result && content != null && !content.isEmpty()) {
File file = new File(folder+"/"+fileName+".txt");
try (OutputStreamWriter fw = new OutputStreamWriter(new FileOutputStream(file.getAbsoluteFile()),"UTF-8");
BufferedWriter bw = new BufferedWriter(fw);
) {
BufferedWriter bw = new BufferedWriter(fw);
) {
bw.write(content);
s_logger.debug("created file: "+ file + " in folder:"+folder);
} catch (final IOException ex) {
Expand Down

0 comments on commit 6ba07b9

Please sign in to comment.