JoaoJandre commented on code in PR #8511:
URL: https://github.com/apache/cloudstack/pull/8511#discussion_r1471756588
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -4401,127 +4403,193 @@ protected String getDiskPathFromDiskDef(DiskDef disk)
{
return null;
}
- private class VmStats {
- long usedTime;
- long tx;
- long rx;
- long ioRead;
- long ioWrote;
- long bytesRead;
- long bytesWrote;
- Calendar timestamp;
+ private String vmToString(Domain dm) throws LibvirtException {
+ return String.format("{\"name\":\"%s\",\"uuid\":\"%s\"}",
dm.getName(), dm.getUUIDString());
}
+ /**
+ * Returns metrics for the period since this function was last called for
the specified VM.
+ * @param conn the Libvirt connection.
+ * @param vmName name of the VM.
+ * @return metrics for the period since last time this function was called
for the VM.
+ * @throws LibvirtException
+ */
public VmStatsEntry getVmStat(final Connect conn, final String vmName)
throws LibvirtException {
Domain dm = null;
try {
+ s_logger.debug(String.format("Trying to get VM with name [%s].",
vmName));
dm = getDomain(conn, vmName);
if (dm == null) {
+ s_logger.warn(String.format("Could not get VM with name
[%s].", vmName));
return null;
}
- DomainInfo info = dm.getInfo();
- final VmStatsEntry stats = new VmStatsEntry();
- stats.setNumCPUs(info.nrVirtCpu);
- stats.setEntityType("vm");
+ LibvirtExtendedVmStatsEntry newStats = getVmCurrentStats(dm);
+ LibvirtExtendedVmStatsEntry oldStats = vmStats.get(vmName);
- stats.setMemoryKBs(info.maxMem);
- stats.setTargetMemoryKBs(info.memory);
- stats.setIntFreeMemoryKBs(getMemoryFreeInKBs(dm));
+ VmStatsEntry metrics = calculateVmMetrics(dm, oldStats, newStats);
- /* get cpu utilization */
- VmStats oldStats = null;
+ String vmAsString = vmToString(dm);
+ s_logger.info(String.format("Saving stats for VM [%s].",
vmAsString));
+ vmStats.put(vmName, newStats);
+ s_logger.debug(String.format("Saved stats for VM [%s]: [%s].",
vmAsString, newStats));
Review Comment:
Can't we merge these two logs into one? I think we could just remove the
info one.
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java:
##########
@@ -4401,127 +4403,193 @@ protected String getDiskPathFromDiskDef(DiskDef disk)
{
return null;
}
- private class VmStats {
- long usedTime;
- long tx;
- long rx;
- long ioRead;
- long ioWrote;
- long bytesRead;
- long bytesWrote;
- Calendar timestamp;
+ private String vmToString(Domain dm) throws LibvirtException {
+ return String.format("{\"name\":\"%s\",\"uuid\":\"%s\"}",
dm.getName(), dm.getUUIDString());
}
+ /**
+ * Returns metrics for the period since this function was last called for
the specified VM.
+ * @param conn the Libvirt connection.
+ * @param vmName name of the VM.
+ * @return metrics for the period since last time this function was called
for the VM.
+ * @throws LibvirtException
+ */
public VmStatsEntry getVmStat(final Connect conn, final String vmName)
throws LibvirtException {
Domain dm = null;
try {
+ s_logger.debug(String.format("Trying to get VM with name [%s].",
vmName));
dm = getDomain(conn, vmName);
if (dm == null) {
+ s_logger.warn(String.format("Could not get VM with name
[%s].", vmName));
return null;
}
- DomainInfo info = dm.getInfo();
- final VmStatsEntry stats = new VmStatsEntry();
- stats.setNumCPUs(info.nrVirtCpu);
- stats.setEntityType("vm");
+ LibvirtExtendedVmStatsEntry newStats = getVmCurrentStats(dm);
+ LibvirtExtendedVmStatsEntry oldStats = vmStats.get(vmName);
- stats.setMemoryKBs(info.maxMem);
- stats.setTargetMemoryKBs(info.memory);
- stats.setIntFreeMemoryKBs(getMemoryFreeInKBs(dm));
+ VmStatsEntry metrics = calculateVmMetrics(dm, oldStats, newStats);
- /* get cpu utilization */
- VmStats oldStats = null;
+ String vmAsString = vmToString(dm);
+ s_logger.info(String.format("Saving stats for VM [%s].",
vmAsString));
+ vmStats.put(vmName, newStats);
+ s_logger.debug(String.format("Saved stats for VM [%s]: [%s].",
vmAsString, newStats));
- final Calendar now = Calendar.getInstance();
+ return metrics;
+ } finally {
+ if (dm != null) {
+ dm.free();
+ }
+ }
+ }
- oldStats = vmStats.get(vmName);
+ /**
+ * Returns a VM's current statistics.
+ * @param dm domain of the VM.
+ * @return current statistics of the VM.
+ * @throws LibvirtException
+ */
+ protected LibvirtExtendedVmStatsEntry getVmCurrentStats(final Domain dm)
throws LibvirtException {
+ final LibvirtExtendedVmStatsEntry stats = new
LibvirtExtendedVmStatsEntry();
- long elapsedTime = 0;
- if (oldStats != null) {
- elapsedTime = now.getTimeInMillis() -
oldStats.timestamp.getTimeInMillis();
- double utilization = (info.cpuTime - oldStats.usedTime) /
((double)elapsedTime * 1000000);
+ getVmCurrentCpuStats(dm, stats);
+ getVmCurrentNetworkStats(dm, stats);
+ getVmCurrentDiskStats(dm, stats);
- utilization = utilization / info.nrVirtCpu;
- if (utilization > 0) {
- stats.setCPUUtilization(utilization * 100);
- }
- }
+ s_logger.debug(String.format("Retrieved statistics for VM [%s]:
[%s].", vmToString(dm), stats));
+ stats.setTimestamp(Calendar.getInstance());
+ return stats;
+ }
- /* get network stats */
+ /**
+ * Passes a VM's current CPU statistics into the provided
LibvirtExtendedVmStatsEntry.
+ * @param dm domain of the VM.
+ * @param stats LibvirtExtendedVmStatsEntry that will receive the current
CPU statistics.
+ * @throws LibvirtException
+ */
+ protected void getVmCurrentCpuStats(final Domain dm, final
LibvirtExtendedVmStatsEntry stats) throws LibvirtException {
+ s_logger.debug(String.format("Getting CPU stats for VM [%s].",
vmToString(dm)));
+ stats.setCpuTime(dm.getInfo().cpuTime);
+ }
- final List<InterfaceDef> vifs = getInterfaces(conn, vmName);
- long rx = 0;
- long tx = 0;
- for (final InterfaceDef vif : vifs) {
- final DomainInterfaceStats ifStats =
dm.interfaceStats(vif.getDevName());
- rx += ifStats.rx_bytes;
- tx += ifStats.tx_bytes;
- }
+ /**
+ * Passes a VM's current network statistics into the provided
LibvirtExtendedVmStatsEntry.
+ * @param dm domain of the VM.
+ * @param stats LibvirtExtendedVmStatsEntry that will receive the current
network statistics.
+ * @throws LibvirtException
+ */
+ protected void getVmCurrentNetworkStats(final Domain dm, final
LibvirtExtendedVmStatsEntry stats) throws LibvirtException {
+ final String vmAsString = vmToString(dm);
+ s_logger.debug(String.format("Getting network stats for VM [%s].",
vmAsString));
+ final List<InterfaceDef> vifs = getInterfaces(dm.getConnect(),
dm.getName());
+ s_logger.debug(String.format("Found [%d] network interface(s) for VM
[%s].", vifs.size(), vmAsString));
+ double rx = 0;
+ double tx = 0;
+ for (final InterfaceDef vif : vifs) {
+ final DomainInterfaceStats ifStats =
dm.interfaceStats(vif.getDevName());
+ rx += ifStats.rx_bytes;
+ tx += ifStats.tx_bytes;
+ }
+ stats.setNetworkReadKBs(rx / 1024);
+ stats.setNetworkWriteKBs(tx / 1024);
+ }
- if (oldStats != null) {
- final double deltarx = rx - oldStats.rx;
- if (deltarx > 0) {
- stats.setNetworkReadKBs(deltarx / 1024);
- }
- final double deltatx = tx - oldStats.tx;
- if (deltatx > 0) {
- stats.setNetworkWriteKBs(deltatx / 1024);
- }
+ /**
+ * Passes a VM's current disk statistics into the provided
LibvirtExtendedVmStatsEntry.
+ * @param dm domain of the VM.
+ * @param stats LibvirtExtendedVmStatsEntry that will receive the current
disk statistics.
+ * @throws LibvirtException
+ */
+ protected void getVmCurrentDiskStats(final Domain dm, final
LibvirtExtendedVmStatsEntry stats) throws LibvirtException {
+ final String vmAsString = vmToString(dm);
+ s_logger.debug(String.format("Getting disk stats for VM [%s].",
vmAsString));
+ final List<DiskDef> disks = getDisks(dm.getConnect(), dm.getName());
+ s_logger.debug(String.format("Found [%d] disk(s) for VM [%s].",
disks.size(), vmAsString));
+ long io_rd = 0;
+ long io_wr = 0;
+ double bytes_rd = 0;
+ double bytes_wr = 0;
+ for (final DiskDef disk : disks) {
+ if (disk.getDeviceType() == DeviceType.CDROM ||
disk.getDeviceType() == DeviceType.FLOPPY) {
+ s_logger.debug(String.format("Ignoring disk [%s] in VM [%s]'s
stats since its deviceType is [%s].", disk.toString().replace("\n", ""),
vmAsString, disk.getDeviceType()));
+ continue;
}
+ final DomainBlockStats blockStats =
dm.blockStats(disk.getDiskLabel());
+ io_rd += blockStats.rd_req;
+ io_wr += blockStats.wr_req;
+ bytes_rd += blockStats.rd_bytes;
+ bytes_wr += blockStats.wr_bytes;
+ }
+ stats.setDiskReadIOs(io_rd);
+ stats.setDiskWriteIOs(io_wr);
+ stats.setDiskReadKBs(bytes_rd / 1024);
+ stats.setDiskWriteKBs(bytes_wr / 1024);
+ }
- /* get disk stats */
- final List<DiskDef> disks = getDisks(conn, vmName);
- long io_rd = 0;
- long io_wr = 0;
- long bytes_rd = 0;
- long bytes_wr = 0;
- for (final DiskDef disk : disks) {
- if (disk.getDeviceType() == DeviceType.CDROM ||
disk.getDeviceType() == DeviceType.FLOPPY) {
- continue;
- }
- final DomainBlockStats blockStats =
dm.blockStats(disk.getDiskLabel());
- io_rd += blockStats.rd_req;
- io_wr += blockStats.wr_req;
- bytes_rd += blockStats.rd_bytes;
- bytes_wr += blockStats.wr_bytes;
+ /**
+ * Calculates a VM's metrics for the period between the two statistics
given as parameters.
+ * @param dm domain of the VM.
+ * @param oldStats old statistics. If null, the CPU, network and disk
utilization won't be calculated.
+ * @param newStats new statistics.
+ * @return metrics for the period between the two statistics.
+ * @throws LibvirtException
+ */
+ protected VmStatsEntry calculateVmMetrics(final Domain dm, final
LibvirtExtendedVmStatsEntry oldStats, final LibvirtExtendedVmStatsEntry
newStats) throws LibvirtException {
+ final VmStatsEntry metrics = new VmStatsEntry();
+ final DomainInfo info = dm.getInfo();
+ final String vmAsString = vmToString(dm);
+
+ metrics.setEntityType("vm");
+ s_logger.debug(String.format("Writing VM [%s]'s CPU and memory
information into the metrics.", vmAsString));
+ metrics.setNumCPUs(info.nrVirtCpu);
+ metrics.setMemoryKBs(info.maxMem);
+ metrics.setTargetMemoryKBs(info.memory);
+ s_logger.debug(String.format("Trying to get free memory for VM [%s].",
vmAsString));
+ metrics.setIntFreeMemoryKBs(getMemoryFreeInKBs(dm));
+
+ if (oldStats != null) {
+ s_logger.debug(String.format("Old stats exist for VM [%s];
thefore, the utilization will be calculated.", vmAsString));
Review Comment:
```suggestion
s_logger.debug(String.format("Old stats exist for VM [%s];
therefore, the utilization will be calculated.", vmAsString));
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
