Change in vdsm[ovirt-3.6]: vmstats: handle known-missing stats
fromani at redhat.com
fromani at redhat.com
Wed Feb 24 15:28:09 UTC 2016
Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/53992
to review the following change.
Change subject: vmstats: handle known-missing stats
......................................................................
vmstats: handle known-missing stats
In some well-known and benign causes, libvirt will not
return the complete set of bulk stats.
It is dangerous to blindly ignore KeyErrors when some
fields are missing, because this can hide other bugs.
But it is safe to ignore them in the aforementioned cases.
This patch lays down the basic pieces to safely ignore
the errors, starting with the first loud and known case:
migration destination VM.
Backport-To: 3.6
Change-Id: I04ecbf98d36d576ede3c3cde36586245ea5e4218
Signed-off-by: Francesco Romani <fromani at redhat.com>
Reviewed-on: https://gerrit.ovirt.org/43777
Continuous-Integration: Jenkins CI
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/vmStatsTests.py
M tests/vmTests.py
M vdsm/virt/vm.py
M vdsm/virt/vmstats.py
4 files changed, 81 insertions(+), 53 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/92/53992/1
diff --git a/tests/vmStatsTests.py b/tests/vmStatsTests.py
index f262f94..78e9998 100644
--- a/tests/vmStatsTests.py
+++ b/tests/vmStatsTests.py
@@ -310,8 +310,10 @@
def test_nic_have_all_keys(self):
nic = FakeNic(name='vnet0', model='virtio',
mac_addr='00:1a:4a:16:01:51')
+ testvm = FakeVM(nics=(nic,))
stats = vmstats._nic_traffic(
+ testvm,
nic.name, nic.nicModel, nic.macAddr,
self.bulk_stats, 0,
self.bulk_stats, 0,
diff --git a/tests/vmTests.py b/tests/vmTests.py
index fb8a89d..a0d7d0b 100644
--- a/tests/vmTests.py
+++ b/tests/vmTests.py
@@ -1439,27 +1439,29 @@
GBPS = 10 ** 9 / 8
MAC = '52:54:00:59:F5:3F'
pretime = utils.monotonic_time()
- res = vmstats._nic_traffic(
- name='vnettest', model='virtio', mac=MAC,
- start_sample={'net.0.rx.bytes': 2 ** 64 - 15 * GBPS,
- 'net.0.rx.pkts': 1,
- 'net.0.rx.errs': 2,
- 'net.0.rx.drop': 3,
- 'net.0.tx.bytes': 0,
- 'net.0.tx.pkts': 4,
- 'net.0.tx.errs': 5,
- 'net.0.tx.drop': 6},
- start_index=0,
- end_sample={'net.0.rx.bytes': 0,
- 'net.0.rx.pkts': 7,
- 'net.0.rx.errs': 8,
- 'net.0.rx.drop': 9,
- 'net.0.tx.bytes': 5 * GBPS,
- 'net.0.tx.pkts': 10,
- 'net.0.tx.errs': 11,
- 'net.0.tx.drop': 12},
- end_index=0,
- interval=15.0)
+ with fake.VM(_VM_PARAMS) as testvm:
+ res = vmstats._nic_traffic(
+ testvm,
+ name='vnettest', model='virtio', mac=MAC,
+ start_sample={'net.0.rx.bytes': 2 ** 64 - 15 * GBPS,
+ 'net.0.rx.pkts': 1,
+ 'net.0.rx.errs': 2,
+ 'net.0.rx.drop': 3,
+ 'net.0.tx.bytes': 0,
+ 'net.0.tx.pkts': 4,
+ 'net.0.tx.errs': 5,
+ 'net.0.tx.drop': 6},
+ start_index=0,
+ end_sample={'net.0.rx.bytes': 0,
+ 'net.0.rx.pkts': 7,
+ 'net.0.rx.errs': 8,
+ 'net.0.rx.drop': 9,
+ 'net.0.tx.bytes': 5 * GBPS,
+ 'net.0.tx.pkts': 10,
+ 'net.0.tx.errs': 11,
+ 'net.0.tx.drop': 12},
+ end_index=0,
+ interval=15.0)
posttime = utils.monotonic_time()
self.assertIn('sampleTime', res)
self.assertTrue(pretime <= res['sampleTime'] <= posttime,
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index bb0c31c..890d328 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -759,7 +759,7 @@
self.log.exception("The vm start process failed")
self.setDownStatus(ERROR, vmexitreason.GENERIC_ERROR, str(e))
- def _incomingMigrationPending(self):
+ def incomingMigrationPending(self):
return 'migrationDest' in self.conf or 'restoreState' in self.conf
def stopDisksStatsCollection(self):
diff --git a/vdsm/virt/vmstats.py b/vdsm/virt/vmstats.py
index a816ca4..93dd0e3 100644
--- a/vdsm/virt/vmstats.py
+++ b/vdsm/virt/vmstats.py
@@ -18,6 +18,7 @@
# Refer to the README and COPYING files for full details of the license
#
+import contextlib
import logging
import six
@@ -144,11 +145,16 @@
# MOM will ignore VMs with missing balloon information instead
# using incomplete data and computing wrong balloon targets
if balloon_target is not None and sample is not None:
+
+ balloon_cur = 0
+ with _skip_if_missing_stats(vm):
+ balloon_cur = sample['balloon.current']
+
stats['balloonInfo'].update({
'balloon_max': str(max_mem),
'balloon_min': str(
int(vm.conf.get('memGuaranteedSize', '0')) * 1024),
- 'balloon_cur': str(sample['balloon.current']),
+ 'balloon_cur': str(balloon_cur),
'balloon_target': str(balloon_target)
})
@@ -166,7 +172,7 @@
logging.error('Failed to get VM cpu count')
-def _nic_traffic(name, model, mac,
+def _nic_traffic(vm_obj, name, model, mac,
start_sample, start_index,
end_sample, end_index, interval):
ifSpeed = [100, 1000][model in ('e1000', 'virtio')]
@@ -176,22 +182,25 @@
'speed': str(ifSpeed),
'state': 'unknown'}
- ifStats['rxErrors'] = str(end_sample['net.%d.rx.errs' % end_index])
- ifStats['rxDropped'] = str(end_sample['net.%d.rx.drop' % end_index])
- ifStats['txErrors'] = str(end_sample['net.%d.tx.errs' % end_index])
- ifStats['txDropped'] = str(end_sample['net.%d.tx.drop' % end_index])
+ with _skip_if_missing_stats(vm_obj):
+ ifStats['rxErrors'] = str(end_sample['net.%d.rx.errs' % end_index])
+ ifStats['rxDropped'] = str(end_sample['net.%d.rx.drop' % end_index])
+ ifStats['txErrors'] = str(end_sample['net.%d.tx.errs' % end_index])
+ ifStats['txDropped'] = str(end_sample['net.%d.tx.drop' % end_index])
- rxDelta = (
- end_sample['net.%d.rx.bytes' % end_index] -
- start_sample['net.%d.rx.bytes' % start_index]
- )
+ with _skip_if_missing_stats(vm_obj):
+ rxDelta = (
+ end_sample['net.%d.rx.bytes' % end_index] -
+ start_sample['net.%d.rx.bytes' % start_index]
+ )
+ txDelta = (
+ end_sample['net.%d.tx.bytes' % end_index] -
+ start_sample['net.%d.tx.bytes' % start_index]
+ )
+
ifRxBytes = (100.0 *
(rxDelta % 2 ** 32) /
interval / ifSpeed / _MBPS_TO_BPS)
- txDelta = (
- end_sample['net.%d.tx.bytes' % end_index] -
- start_sample['net.%d.tx.bytes' % start_index]
- )
ifTxBytes = (100.0 *
(txDelta % 2 ** 32) /
interval / ifSpeed / _MBPS_TO_BPS)
@@ -229,7 +238,7 @@
continue
stats['network'][nic.name] = _nic_traffic(
- nic.name, nic.nicModel, nic.macAddr,
+ vm, nic.name, nic.nicModel, nic.macAddr,
first_sample, first_indexes[nic.name],
last_sample, last_indexes[nic.name],
interval)
@@ -263,26 +272,28 @@
if (vm_drive.name in first_indexes and
vm_drive.name in last_indexes):
# will be None if sampled during recovery
- if interval > 0:
- drive_stats.update(
- _disk_rate(
- first_sample, first_indexes[vm_drive.name],
- last_sample, last_indexes[vm_drive.name],
- interval))
- drive_stats.update(
- _disk_latency(
- first_sample, first_indexes[vm_drive.name],
- last_sample, last_indexes[vm_drive.name]))
- else:
+ if interval <= 0:
logging.warning(
'invalid interval %i when calculating '
'stats for vm %s disk %s',
interval, vm.id, vm_drive.name)
-
- drive_stats.update(
- _disk_iops_bytes(
- first_sample, first_indexes[vm_drive.name],
- last_sample, last_indexes[vm_drive.name]))
+ else:
+ with _skip_if_missing_stats(vm):
+ drive_stats.update(
+ _disk_rate(
+ first_sample, first_indexes[vm_drive.name],
+ last_sample, last_indexes[vm_drive.name],
+ interval))
+ with _skip_if_missing_stats(vm):
+ drive_stats.update(
+ _disk_latency(
+ first_sample, first_indexes[vm_drive.name],
+ last_sample, last_indexes[vm_drive.name]))
+ with _skip_if_missing_stats(vm):
+ drive_stats.update(
+ _disk_iops_bytes(
+ first_sample, first_indexes[vm_drive.name],
+ last_sample, last_indexes[vm_drive.name]))
except AttributeError:
logging.exception("Disk %s stats not available",
@@ -359,3 +370,16 @@
else:
name_to_idx[name] = idx
return name_to_idx
+
+
+ at contextlib.contextmanager
+def _skip_if_missing_stats(vm_obj):
+ try:
+ yield
+ except KeyError:
+ if vm_obj.incomingMigrationPending():
+ # If a VM is migration destination,
+ # libvirt doesn't give any disk stat.
+ pass
+ else:
+ raise
--
To view, visit https://gerrit.ovirt.org/53992
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I04ecbf98d36d576ede3c3cde36586245ea5e4218
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
More information about the vdsm-patches
mailing list