vdsm-patches July 2015

vdsm-patches@lists.fedorahosted.org

44 participants
919 discussions

Change in vdsm[master]: virt: add test to allow custom XML parsing test
by Martin Polednik 27 Oct '15

27 Oct '15

Martin Polednik has uploaded a new change for review. Change subject: virt: add test to allow custom XML parsing test ...................................................................... virt: add test to allow custom XML parsing test Tests should run against explicit device specification, but it may be useful to have a means of simply testing that parse of given XML succeeds. This patch implements such functionality, allowing user to supply XML in tests/devices/data and modify tests/parsing/custom_vm_tests.py to see if the parsing conforms general device specification. Change-Id: I3bb24e2854e6f7b93c5108eca8b6f79f17353e85 Signed-off-by: Martin Polednik <mpolednik(a)redhat.com> --- M tests/Makefile.am M tests/devices/data/Makefile.am A tests/devices/data/testSriovVm.xml M tests/devices/parsing/Makefile.am A tests/devices/parsing/custom_vm_tests.py 5 files changed, 181 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/09/39909/1 diff --git a/tests/Makefile.am b/tests/Makefile.am index b8a8964..2bc9018 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -27,6 +27,7 @@ device_modules = \ devices/parsing/complex_vm_tests.py \ + devices/parsing/custom_vm_tests.py \ $(NULL) test_modules = \ diff --git a/tests/devices/data/Makefile.am b/tests/devices/data/Makefile.am index 0ead0e2..6a6288c 100644 --- a/tests/devices/data/Makefile.am +++ b/tests/devices/data/Makefile.am @@ -22,4 +22,5 @@ dist_vdsmdevdatatests_DATA = \ testComplexVm.xml \ + testSriovVm.xml \ $(NULL) diff --git a/tests/devices/data/testSriovVm.xml b/tests/devices/data/testSriovVm.xml new file mode 100644 index 0000000..2980c7f --- /dev/null +++ b/tests/devices/data/testSriovVm.xml @@ -0,0 +1,144 @@ +<domain type='kvm' id='2'> + <name>vm1_Copy</name> + <uuid>78144ebf-7894-456e-997f-9fc96083341e</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static' current='1'>16</vcpu> + <cputune> + <shares>1020</shares> + <period>12500</period> + <quota>100000</quota> + </cputune> + <resource> + <partition>/machine</partition> + </resource> + <sysinfo type='smbios'> + <system> + <entry name='manufacturer'>oVirt</entry> + <entry name='product'>oVirt Node</entry> + <entry name='version'>7.1-0.1.el7</entry> + <entry name='serial'>38373035-3536-4247-3830-333334344139</entry> + <entry name='uuid'>78144ebf-7894-456e-997f-9fc96083341e</entry> + </system> + </sysinfo> + <os> + <type arch='x86_64' machine='pc-i440fx-rhel7.0.0'>hvm</type> + <smbios mode='sysinfo'/> + </os> + <features> + <acpi/> + </features> + <cpu mode='custom' match='exact'> + <model fallback='allow'>Conroe</model> + <topology sockets='16' cores='1' threads='1'/> + </cpu> + <clock offset='variable' adjustment='0' basis='utc'> + <timer name='rtc' tickpolicy='catchup'/> + <timer name='pit' tickpolicy='delay'/> + <timer name='hpet' present='no'/> + </clock> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <disk type='file' device='cdrom'> + <driver name='qemu' type='raw'/> + <source startupPolicy='optional'/> + <backingStore/> + <target dev='hdc' bus='ide'/> + <readonly/> + <serial></serial> + <alias name='ide0-1-0'/> + <address type='drive' controller='0' bus='1' target='0' unit='0'/> + </disk> + <disk type='file' device='disk' snapshot='no'> + <driver name='qemu' type='raw' cache='none' error_policy='stop' io='threads'/> + <source file='/rhev/data-center/bd4ba8d0-024e-412b-aa6e-b22a1654f53e/9a8980dc-b533-4085-884e-daa9f3753ce7/images/369a5d94-05bc-41c0-84c5-ed3b1b8d2d89/79d06c74-8a95-4e4b-afba-8d09975c5f8d'> + <seclabel model='selinux' labelskip='yes'/> + </source> + <backingStore/> + <target dev='vda' bus='virtio'/> + <serial>369a5d94-05bc-41c0-84c5-ed3b1b8d2d89</serial> + <boot order='1'/> + <alias name='virtio-disk0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/> + </disk> + <controller type='scsi' index='0' model='virtio-scsi'> + <alias name='scsi0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + </controller> + <controller type='virtio-serial' index='0' ports='16'> + <alias name='virtio-serial0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </controller> + <controller type='usb' index='0'> + <alias name='usb0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'> + <alias name='pci.0'/> + </controller> + <controller type='ide' index='0'> + <alias name='ide0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <interface type='hostdev'> + <mac address='00:1a:4a:16:01:53'/> + <driver name='vfio'/> + <source> + <address type='pci' domain='0x0000' bus='0x02' slot='0x10' function='0x0'/> + </source> + <alias name='hostdev0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/> + </interface> + <interface type='bridge'> + <mac address='00:1a:4a:16:01:54'/> + <source bridge='ovirtmgmt'/> + <bandwidth> + </bandwidth> + <target dev='vnet0'/> + <model type='virtio'/> + <filterref filter='vdsm-no-mac-spoofing'/> + <link state='down'/> + <alias name='net1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </interface> + <channel type='unix'> + <source mode='bind' path='/var/lib/libvirt/qemu/channels/78144ebf-7894-456e-997f-9fc96083341e.com.redhat.rhevm.vdsm'/> + <target type='virtio' name='com.redhat.rhevm.vdsm'/> + <alias name='channel0'/> + <address type='virtio-serial' controller='0' bus='0' port='1'/> + </channel> + <channel type='unix'> + <source mode='bind' path='/var/lib/libvirt/qemu/channels/78144ebf-7894-456e-997f-9fc96083341e.org.qemu.guest_agent.0'/> + <target type='virtio' name='org.qemu.guest_agent.0'/> + <alias name='channel1'/> + <address type='virtio-serial' controller='0' bus='0' port='2'/> + </channel> + <channel type='spicevmc'> + <target type='virtio' name='com.redhat.spice.0'/> + <alias name='channel2'/> + <address type='virtio-serial' controller='0' bus='0' port='3'/> + </channel> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='spice' port='5900' tlsPort='5901' autoport='yes' listen='0' passwdValidTo='1970-01-01T00:00:01'> + <listen type='address' address='0'/> + </graphics> + <video> + <model type='qxl' ram='65536' vram='32768' vgamem='16384' heads='1'/> + <alias name='video0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </video> + <memballoon model='virtio'> + <alias name='balloon0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/> + </memballoon> + </devices> + <seclabel type='dynamic' model='selinux' relabel='yes'> + <label>system_u:system_r:svirt_t:s0:c799,c978</label> + <imagelabel>system_u:object_r:svirt_image_t:s0:c799,c978</imagelabel> + </seclabel> +</domain> + diff --git a/tests/devices/parsing/Makefile.am b/tests/devices/parsing/Makefile.am index 978e9bd..26edbd4 100644 --- a/tests/devices/parsing/Makefile.am +++ b/tests/devices/parsing/Makefile.am @@ -23,4 +23,5 @@ dist_vdsmdevparsingtests_PYTHON = \ __init__.py \ complex_vm_tests.py \ + custom_vm_tests.py \ $(NULL) diff --git a/tests/devices/parsing/custom_vm_tests.py b/tests/devices/parsing/custom_vm_tests.py new file mode 100644 index 0000000..eca8db9 --- /dev/null +++ b/tests/devices/parsing/custom_vm_tests.py @@ -0,0 +1,34 @@ +import os + +from testlib import permutations, expandPermutations +from testlib import XMLTestCase + +from virt import domain_descriptor +import vmfakelib as fake + +import verify + + +@expandPermutations +class TestVmDevicesXmlParsing(XMLTestCase, verify.DeviceMixin): + + @permutations([['testComplexVm.xml'], ['testSriovVm.xml']]) + def test_custom_vm(self, domain_xml): + params = {'name': 'complexVm', 'displaySecurePort': '-1', + 'memSize': '256', 'displayPort': '-1', 'display': 'qxl'} + + devices = [{'device': 'spice', 'type': 'graphics'}] + + test_path = os.path.realpath(__file__) + dir_name = os.path.split(test_path)[0] + api_path = os.path.join( + dir_name, '..', 'data', domain_xml) + + domain = None + with open(api_path, 'r') as domxml: + domain = domxml.read() + + with fake.VM(params=params, devices=devices) as vm: + vm._domain = domain_descriptor.DomainDescriptor(domain) + vm._getUnderlyingVmDevicesInfo() + self.verifyDevicesConf(vm.conf['devices']) -- To view, visit https://gerrit.ovirt.org/39909 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3bb24e2854e6f7b93c5108eca8b6f79f17353e85 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Martin Polednik <mpolednik(a)redhat.com>

4 18

Change in vdsm[master]: Don't prepare image with illegal volumes
by alitke＠redhat.com 25 Oct '15

25 Oct '15

Adam Litke has uploaded a new change for review. Change subject: Don't prepare image with illegal volumes ...................................................................... Don't prepare image with illegal volumes The irs verb 'prepareImage' is used only by clientIF when preparing the host to start a VM. Currently, this operation succeeds even if one or more of the volumes in the image is illegal. Since we should never permit a VM to start with an illegal disk, check for this and report an error if any illegal volumes are found. The pivot stage of a live merge operation depends on this behavior to ensure that a VM is not accidentally started using a stale leaf volume. Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1206722 Change-Id: Ie543aeb8bdb52305419613ab6297681817124308 Signed-off-by: Adam Litke <alitke(a)redhat.com> --- M vdsm/storage/hsm.py 1 file changed, 5 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/02/39302/1 diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py index 8f07586..dc8ca77 100644 --- a/vdsm/storage/hsm.py +++ b/vdsm/storage/hsm.py @@ -3225,6 +3225,11 @@ if leafUUID not in imgVolumes: raise se.VolumeDoesNotExist(leafUUID) + for volUUID in imgVolumes: + legality = dom.produceVolume(imgUUID, volUUID).getLegality() + if legality == volume.ILLEGAL_VOL: + raise se.prepareIllegalVolumeError(volUUID) + imgPath = dom.activateVolumes(imgUUID, imgVolumes) if spUUID and spUUID != sd.BLANK_UUID: runImgPath = dom.linkBCImage(imgPath, imgUUID) -- To view, visit https://gerrit.ovirt.org/39302 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ie543aeb8bdb52305419613ab6297681817124308 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Adam Litke <alitke(a)redhat.com>

8 29

Change in vdsm[master]: tests: increase timeout to 30 seconds
by ykaplan＠redhat.com 25 Oct '15

25 Oct '15

Yeela Kaplan has uploaded a new change for review. Change subject: tests: increase timeout to 30 seconds ...................................................................... tests: increase timeout to 30 seconds Increase maximum timeout to 30 seconds instead of only 1. On automation testing gc is a bit slower, so we want to be safe and avoid unit tests failing. Change-Id: I62cca38db73a0344d927e58cf79ca14e323278bf Signed-off-by: Yeela Kaplan <ykaplan(a)redhat.com> --- M tests/outOfProcessTests.py 1 file changed, 14 insertions(+), 10 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/67/36267/1 diff --git a/tests/outOfProcessTests.py b/tests/outOfProcessTests.py index 8cb89a7..18c97e9 100644 --- a/tests/outOfProcessTests.py +++ b/tests/outOfProcessTests.py @@ -63,6 +63,7 @@ def testAmountOfInstancesPerPoolName(self): idle = oop.IOPROC_IDLE_TIME + maxSleep = 30 try: oop.IOPROC_IDLE_TIME = 5 poolA = "A" @@ -74,16 +75,19 @@ oop.getProcessPool(poolB) self.assertEquals(wrapper(), None) gc.collect() - time.sleep(1) - gc.collect() - try: - self.assertEquals(ioproc(), None) - except AssertionError: - logging.info("GARBAGE: %s", gc.garbage) - refs = gc.get_referrers(ioproc()) - logging.info(refs) - logging.info(gc.get_referrers(*refs)) - raise + for i in xrange(1, maxSleep+1): + time.sleep(1) + gc.collect() + try: + self.assertEquals(ioproc(), None) + break + except AssertionError: + if (i == maxSleep): + logging.info("GARBAGE: %s", gc.garbage) + refs = gc.get_referrers(ioproc()) + logging.info(refs) + logging.info(gc.get_referrers(*refs)) + raise finally: oop.IOPROC_IDLE_TIME = idle -- To view, visit http://gerrit.ovirt.org/36267 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I62cca38db73a0344d927e58cf79ca14e323278bf Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>

4 10

Change in vdsm[master]: Added standard build skeleton
by dcaroest＠redhat.com 21 Oct '15

21 Oct '15

David Caro has uploaded a new change for review. Change subject: Added standard build skeleton ...................................................................... Added standard build skeleton This will replace all/any other vdsm jobs being run now. Change-Id: I24b446d3ae38c944b5a28d56ed0965d0f2edc206 Signed-off-by: David Caro <dcaroest(a)redhat.com> --- A automation/build-artifacts.sh A automation/check-merged.sh A automation/check-patch.sh 3 files changed, 9 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/28/41928/1 diff --git a/automation/build-artifacts.sh b/automation/build-artifacts.sh new file mode 100644 index 0000000..231f390 --- /dev/null +++ b/automation/build-artifacts.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +echo "TODO" diff --git a/automation/check-merged.sh b/automation/check-merged.sh new file mode 100644 index 0000000..231f390 --- /dev/null +++ b/automation/check-merged.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +echo "TODO" diff --git a/automation/check-patch.sh b/automation/check-patch.sh new file mode 100644 index 0000000..231f390 --- /dev/null +++ b/automation/check-patch.sh @@ -0,0 +1,3 @@ +#!/usr/bin/env bash + +echo "TODO" -- To view, visit https://gerrit.ovirt.org/41928 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I24b446d3ae38c944b5a28d56ed0965d0f2edc206 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: David Caro <dcaroest(a)redhat.com>

6 21

Change in vdsm[master]: ImageUpload-api
by aaviram＠redhat.com 21 Oct '15

21 Oct '15

Amit Aviram has uploaded a new change for review. Change subject: ImageUpload-api ...................................................................... ImageUpload-api Change-Id: Iddf676fb15c88e126811a25e5e97609c6d937422 Bug-Url: https://bugzilla.redhat.com/?????? Signed-off-by: Amit Aviram <aaviram(a)redhat.com> --- M client/vdsClient.py M vdsm.spec.in M vdsm/API.py M vdsm/rpc/BindingXMLRPC.py M vdsm/rpc/vdsmapi-schema.json 5 files changed, 60 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/70/41870/1 diff --git a/client/vdsClient.py b/client/vdsClient.py index 375fa1a..fe03aa7 100755 --- a/client/vdsClient.py +++ b/client/vdsClient.py @@ -1585,6 +1585,19 @@ return stats['status']['code'], stats['status']['message'] return 0, '' + def do_startImageTransferSession(self, args): + ticket = args[0] + op = args[1] + spId = args[2] + sdId = args[3] + imgId = args[4] + volId = args[5] + stats = self.s.startImageTransferSession(ticket, op, spId, sdId, + imgId, volId) + if stats['status']['code']: + return stats['status']['code'], stats['status']['message'] + return 0, '' + def do_getVmsInfo(self, args): spUUID = args[0] if len(args) >= 2: @@ -2535,6 +2548,11 @@ ' <enabled = true/false>', 'Enable or disable Hosted Engine HA' ' maintenance')), + 'startImageTransferSession': (serv.do_startImageTransferSession, + ('<ticketUUID> <op>', + '<spUUID> <sdUUID> ', + '<imgUUID> <volUUID> ', + 'Start an image transfer session.')), 'deleteImage': (serv.deleteImage, ('<sdUUID> <spUUID> <imgUUID> [<postZero>] [<force>]', 'Delete Image folder with all volumes.', diff --git a/vdsm.spec.in b/vdsm.spec.in index 456a2d9..82fbe9b 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -293,7 +293,7 @@ # GlusterFS client-side RPMs needed for Gluster SD %if 0%{?with_gluster} -Requires: glusterfs >= 3.6.999 +Requires: glusterfs >= 3.5 Requires: glusterfs-cli Requires: glusterfs-api Requires: glusterfs-fuse diff --git a/vdsm/API.py b/vdsm/API.py index 3cdc465..bb41d39 100644 --- a/vdsm/API.py +++ b/vdsm/API.py @@ -1662,6 +1662,14 @@ return errCode['haErr'] return {'status': doneCode} + def startImageTransferSession(self, ticket, op, spId, sdId, + imgId, volId): + self.log.info("ImageUploadSessionStart!!: \nticket:%s\nop:%s\n" + "spId:%s\nsdId:%s\nimgId:%s\nvolId:%s", ticket, + op, spId, sdId, imgId, volId) + + return {'status': doneCode} + # take a rough estimate on how much free mem is available for new vm # memTotal = memFree + memCached + mem_used_by_non_qemu + resident . # simply returning (memFree + memCached) is not good enough, as the diff --git a/vdsm/rpc/BindingXMLRPC.py b/vdsm/rpc/BindingXMLRPC.py index 9a1da99..3e92879 100644 --- a/vdsm/rpc/BindingXMLRPC.py +++ b/vdsm/rpc/BindingXMLRPC.py @@ -603,6 +603,12 @@ api = API.Global() return api.setHaMaintenanceMode(mode, enabled) + def startImageTransferSession(self, ticket, op, spId, sdId, + imgId, volId): + api = API.Global() + return api.startImageTransferSession(ticket, op, spId, + sdId, imgId, volId) + def domainActivate(self, sdUUID, spUUID, options=None): domain = API.StorageDomain(sdUUID) return domain.activate(spUUID) @@ -1025,6 +1031,7 @@ (self.setMOMPolicy, 'setMOMPolicy'), (self.setMOMPolicyParameters, 'setMOMPolicyParameters'), (self.setHaMaintenanceMode, 'setHaMaintenanceMode'), + (self.startImageTransferSession, 'startImageTransferSession'), (self.vmHotplugDisk, 'hotplugDisk'), (self.vmHotunplugDisk, 'hotunplugDisk'), (self.vmHotplugNic, 'hotplugNic'), diff --git a/vdsm/rpc/vdsmapi-schema.json b/vdsm/rpc/vdsmapi-schema.json index 0c73078..959c24d 100644 --- a/vdsm/rpc/vdsmapi-schema.json +++ b/vdsm/rpc/vdsmapi-schema.json @@ -574,6 +574,8 @@ {'enum': 'HaMaintenanceMode', 'data': ['GLOBAL', 'LOCAL']} + + ## # @Host.setHaMaintenanceMode: # @@ -589,6 +591,30 @@ 'data': {'mode': 'HaMaintenanceMode', 'enabled': 'bool'}} ## +# @Host.startImageTransferSession: +# +# Start an image upload session +# +# @ticket: ticket +# +# @op: op +# +# @spId: op +# +# @sdId: op +# +# @imgId: op +# +# @volId: op +# +# Since: 4.14.0 +## +{'command': {'class': 'Host', 'name': 'startImageTransferSession'}, + 'data': {'ticket': 'UUID', 'op': 'str', 'spId': 'UUID', + 'sdId': 'UUID', 'imgId': 'UUID', 'volId': 'UUID'}} + + +## # @TaskDetails: # # A collection of information about a task. -- To view, visit https://gerrit.ovirt.org/41870 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Iddf676fb15c88e126811a25e5e97609c6d937422 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Amit Aviram <aaviram(a)redhat.com>

2 9

Change in vdsm[master]: configfile: remove unneeded methods.
by mtayer＠redhat.com 20 Oct '15

20 Oct '15

mooli tayer has uploaded a new change for review. Change subject: configfile: remove unneeded methods. ...................................................................... configfile: remove unneeded methods. Change-Id: Icf08bfebc83a9af5eb3c7de48f9a51d2263766fd Signed-off-by: Mooli Tayer <mtayer(a)redhat.com> --- M lib/vdsm/tool/configurators/configfile.py M tests/toolTests.py 2 files changed, 2 insertions(+), 106 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/36324/1 diff --git a/lib/vdsm/tool/configurators/configfile.py b/lib/vdsm/tool/configurators/configfile.py index d8b7fdd..d1e0472 100644 --- a/lib/vdsm/tool/configurators/configfile.py +++ b/lib/vdsm/tool/configurators/configfile.py @@ -83,7 +83,6 @@ version, sectionStart='## beginning of configuration section by vdsm', sectionEnd='## end of configuration section by vdsm', - prefix='# VDSM backup', lineComment='by vdsm'): if not os.path.exists(filename): raise OSError( @@ -94,7 +93,6 @@ self._context = False self._sectionStart = sectionStart self._sectionEnd = sectionEnd - self._prefix = prefix # remove 'lineComment' at 4.0. see 'Backward compatibility' self._lineComment = lineComment self._version = version @@ -104,9 +102,6 @@ raise RuntimeError("can only enter once") self._entries = {} self._context = True - self._prefixRemove = None - self._prefixAdd = None - self._section = None self._oldmod = os.stat(self._filename).st_mode self._remove = None self._rmstate = BEFORE @@ -131,11 +126,6 @@ continue if not self._remove or self._rmstate != WITHIN: - if self._prefixRemove: - if line.startswith(self._prefix): - line = line[len(self._prefix):] - if self._prefixAdd: - line = self._prefix + line m = confpat.match(line.rstrip()) if m: oldentries.add(m.group('key')) @@ -149,11 +139,6 @@ def _end(self): return "%s-%s\n" % (self._sectionEnd, self._version) - - def _writeSection(self, f): - f.write(self._start()) - f.write(self._section) - f.write(self._end()) def _writeEntries(self, f, oldentries): f.write(self._start()) @@ -170,8 +155,6 @@ try: oldlines, oldentries = self._getOldContent() with os.fdopen(fd, 'w', ) as f: - if self._section: - self._writeSection(f) f.writelines(oldlines) if self._entries: self._writeEntries(f, oldentries) @@ -199,32 +182,6 @@ all pairs are added in a comment wrapped section. """ self._entries[key] = val - - @context - def prependSection(self, section): - """ - add 'section' in the beginning of the file. - section is added in a comment wrapped section. - - Only one section is currently supported. - """ - self._section = section - - @context - def prefixLines(self): - """ - Add self.prefix to the beginning of each line. - No editing is done on new content added by this config file. - """ - self._prefixAdd = True - - @context - def unprefixLines(self): - """ - Remove self.prefix from each line starting with it. - No editing is done on new content added by this config file. - """ - self._prefixRemove = True @context def removeConf(self): diff --git a/tests/toolTests.py b/tests/toolTests.py index 82a2668..89a93bf 100644 --- a/tests/toolTests.py +++ b/tests/toolTests.py @@ -452,66 +452,6 @@ "key4=val\n" "# end conf-3.4.4\n") - def testPrefixAndPrepend(self): - self._writeConf("/var/log/libvirt/libvirtd.log {\n" - " weekly\n" - "}\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conf: - conf.prefixLines() - conf.prependSection("Some text to\n" - "add at the top\n") - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), - "# start conf-3.4.4\n" - "Some text to\n" - "add at the top\n" - "# end conf-3.4.4\n" - "# comment /var/log/libvirt/libvirtd.log {\n" - "# comment weekly\n" - "# comment }\n") - - def testPrefixIdempotencey(self): - original = ( - "/var/log/libvirt/libvirtd.log {\n" - " weekly\n" - "}\n" - ) - self._writeConf(original) - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conf: - conf.prefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), - "# comment /var/log/libvirt/libvirtd.log {\n" - "# comment weekly\n" - "# comment }\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment ") as conff: - conff.unprefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), original) - - def testRemoveEntireLinePrefix(self): - self._writeConf("# comment\n") - with ConfigFile(self.tname, - version='3.4.4', - sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment") as conf: - conf.unprefixLines() - with open(self.tname, 'r') as f: - self.assertEqual(f.read(), "\n") - def testRemoveConfSection(self): self._writeConf("key=val\n" "remove me!(see 'Backward compatibility')# by vdsm\n" @@ -523,8 +463,7 @@ with ConfigFile(self.tname, version='3.4.4', sectionStart="# start conf", - sectionEnd="# end conf", - prefix="# comment") as conf: + sectionEnd="# end conf") as conf: conf.removeConf() with open(self.tname, 'r') as f: self.assertEqual(f.read(), "key=val\n" @@ -536,7 +475,7 @@ version='3.4.4', sectionStart="# start conf", sectionEnd="# end conf") - self.assertRaises(RuntimeError, conff.prefixLines) + self.assertRaises(RuntimeError, conff.addEntry, 'key', 'val') self.assertRaises(RuntimeError, conff.removeConf) def testHasConf(self): -- To view, visit http://gerrit.ovirt.org/36324 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icf08bfebc83a9af5eb3c7de48f9a51d2263766fd Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: mooli tayer <mtayer(a)redhat.com>

5 13

Change in vdsm[master]: sampling: move translation code into hoststats.py
by fromani＠redhat.com 20 Oct '15

20 Oct '15

Francesco Romani has uploaded a new change for review. Change subject: sampling: move translation code into hoststats.py ...................................................................... sampling: move translation code into hoststats.py Move all the functions that produces host stats from HostSamples into hoststats.py, like we did for vmstats.py. Note: getBootTime() and get_boot_time() were moved as well, even though they aren't translation functions. Should they stay into sampling.py? Change-Id: Icce6acbd596d087491678d039282d5d37d761904 Signed-off-by: Francesco Romani <fromani(a)redhat.com> --- M debian/vdsm.install M tests/samplingTests.py M vdsm.spec.in M vdsm/virt/Makefile.am A vdsm/virt/hoststats.py M vdsm/virt/sampling.py 6 files changed, 222 insertions(+), 185 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/34/42034/1 diff --git a/debian/vdsm.install b/debian/vdsm.install index ae35369..5d01712 100644 --- a/debian/vdsm.install +++ b/debian/vdsm.install @@ -148,6 +148,7 @@ ./usr/share/vdsm/virt/__init__.py ./usr/share/vdsm/virt/domain_descriptor.py ./usr/share/vdsm/virt/guestagent.py +./usr/share/vdsm/virt/hoststats.py ./usr/share/vdsm/virt/migration.py ./usr/share/vdsm/virt/periodic.py ./usr/share/vdsm/virt/sampling.py diff --git a/tests/samplingTests.py b/tests/samplingTests.py index 229d3d4..ab85834 100644 --- a/tests/samplingTests.py +++ b/tests/samplingTests.py @@ -28,7 +28,9 @@ from vdsm import ipwrapper from vdsm.password import ProtectedPassword +from virt import hoststats import virt.sampling as sampling + import caps @@ -80,39 +82,39 @@ shutil.rmtree(self._tmpDir) def testBootTimeOk(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', self._good_path)]): - self.assertEquals(sampling.getBootTime(), + self.assertEquals(hoststats.getBootTime(), 1395249141) def testBootTimeEmpty(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', '/dev/null')]): with self.assertRaises(ValueError): - sampling.getBootTime() + hoststats.getBootTime() def testBootTimeMissing(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', self._missing_path)]): with self.assertRaises(ValueError): - sampling.getBootTime() + hoststats.getBootTime() def testBootTimeMalformed(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', self._malformed_path)]): with self.assertRaises(ValueError): - sampling.getBootTime() + hoststats.getBootTime() def testBootTimeNonExistantFile(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', '/i/do/not/exist/1234567890')]): with self.assertRaises(IOError): - sampling.getBootTime() + hoststats.getBootTime() def testBootTimeExtra(self): - with MonkeyPatchScope([(sampling, '_PROC_STAT_PATH', + with MonkeyPatchScope([(hoststats, '_PROC_STAT_PATH', self._extra_path)]): - self.assertEquals(sampling.getBootTime(), 1395249141) + self.assertEquals(hoststats.getBootTime(), 1395249141) @contextmanager @@ -336,7 +338,7 @@ with MonkeyPatchScope([(caps, 'getNumaTopology', fakeNumaTopology)]): self.assertEqual( - sampling._get_cpu_core_stats(first_sample, last_sample), + hoststats._get_cpu_core_stats(first_sample, last_sample), expected) diff --git a/vdsm.spec.in b/vdsm.spec.in index 5c9061d..7d7abd2 100644 --- a/vdsm.spec.in +++ b/vdsm.spec.in @@ -1000,6 +1000,7 @@ %{_datadir}/%{vdsm_name}/virt/__init__.py* %{_datadir}/%{vdsm_name}/virt/domain_descriptor.py* %{_datadir}/%{vdsm_name}/virt/guestagent.py* +%{_datadir}/%{vdsm_name}/virt/hoststats.py* %{_datadir}/%{vdsm_name}/virt/migration.py* %{_datadir}/%{vdsm_name}/virt/periodic.py* %{_datadir}/%{vdsm_name}/virt/sampling.py* diff --git a/vdsm/virt/Makefile.am b/vdsm/virt/Makefile.am index 376f973..88025be 100644 --- a/vdsm/virt/Makefile.am +++ b/vdsm/virt/Makefile.am @@ -27,6 +27,7 @@ __init__.py \ domain_descriptor.py \ guestagent.py \ + hoststats.py \ migration.py \ periodic.py \ sampling.py \ diff --git a/vdsm/virt/hoststats.py b/vdsm/virt/hoststats.py new file mode 100644 index 0000000..57c6954 --- /dev/null +++ b/vdsm/virt/hoststats.py @@ -0,0 +1,202 @@ +# +# Copyright 2008-2015 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +# +# Refer to the README and COPYING files for full details of the license +# + +import logging + +from vdsm import utils + +import caps +import v2v + + +def produce(ncpus, first_sample, last_sample): + stats = _empty_stats() + + if first_sample is None: + return stats + + stats.update(_get_interfaces_stats(first_sample, last_sample)) + + interval = last_sample.timestamp - first_sample.timestamp + + jiffies = ( + last_sample.pidcpu.user - first_sample.pidcpu.user) % (2 ** 32) + stats['cpuUserVdsmd'] = jiffies / interval + jiffies = ( + last_sample.pidcpu.sys - first_sample.pidcpu.sys) % (2 ** 32) + stats['cpuSysVdsmd'] = jiffies / interval + + jiffies = ( + last_sample.totcpu.user - first_sample.totcpu.user) % (2 ** 32) + stats['cpuUser'] = jiffies / interval / ncpus + jiffies = ( + last_sample.totcpu.sys - first_sample.totcpu.sys) % (2 ** 32) + stats['cpuSys'] = jiffies / interval / ncpus + stats['cpuIdle'] = max(0.0, + 100.0 - stats['cpuUser'] - stats['cpuSys']) + stats['memUsed'] = last_sample.memUsed + stats['anonHugePages'] = last_sample.anonHugePages + stats['cpuLoad'] = last_sample.cpuLoad + + stats['diskStats'] = last_sample.diskStats + stats['thpState'] = last_sample.thpState + + if _boot_time(): + stats['bootTime'] = _boot_time() + + stats['numaNodeMemFree'] = last_sample.numaNodeMem.nodesMemSample + stats['cpuStatistics'] = _get_cpu_core_stats( + first_sample, last_sample) + + stats['v2vJobs'] = v2v.get_jobs_status() + return stats + + +_PROC_STAT_PATH = '/proc/stat' + + +def getBootTime(): + """ + Returns the boot time of the machine in seconds since epoch. + + Raises IOError if file access fails, or ValueError if boot time not + present in file. + """ + with open(_PROC_STAT_PATH) as proc_stat: + for line in proc_stat: + if line.startswith('btime'): + parts = line.split() + if len(parts) > 1: + return int(parts[1]) + else: + break + raise ValueError('Boot time not present') + + +(a)utils.memoized +def _boot_time(): + # Try to get boot time only once, if N/A just log the error and never + # include it in the response. + try: + return getBootTime() + except (IOError, ValueError): + logging.exception('Failed to get boot time') + return None + + +def _empty_stats(): + return { + 'cpuUser': 0.0, + 'cpuSys': 0.0, + 'cpuIdle': 100.0, + 'rxRate': 0.0, # REQUIRED_FOR: engine < 3.6 + 'txRate': 0.0, # REQUIRED_FOR: engine < 3.6 + 'cpuSysVdsmd': 0.0, + 'cpuUserVdsmd': 0.0, + } + + +def _get_cpu_core_stats(first_sample, last_sample): + interval = last_sample.timestamp - first_sample.timestamp + + def compute_cpu_usage(cpu_core, mode): + jiffies = ( + last_sample.cpuCores.getCoreSample(cpu_core)[mode] - + first_sample.cpuCores.getCoreSample(cpu_core)[mode] + ) % (2 ** 32) + return ("%.2f" % (jiffies / interval)) + + cpu_core_stats = {} + for node_index, numa_node in caps.getNumaTopology().iteritems(): + cpu_cores = numa_node['cpus'] + for cpu_core in cpu_cores: + core_stat = { + 'nodeIndex': int(node_index), + 'cpuUser': compute_cpu_usage(cpu_core, 'user'), + 'cpuSys': compute_cpu_usage(cpu_core, 'sys'), + } + core_stat['cpuIdle'] = ( + "%.2f" % max(0.0, + 100.0 - + float(core_stat['cpuUser']) - + float(core_stat['cpuSys']))) + cpu_core_stats[str(cpu_core)] = core_stat + return cpu_core_stats + + +def _get_interfaces_stats(first_sample, last_sample): + interval = last_sample.timestamp - first_sample.timestamp + + rx = tx = rxDropped = txDropped = 0 + stats = {'network': {}} + total_rate = 0 + for ifid in last_sample.interfaces: + # it skips hot-plugged devices if we haven't enough information + # to count stats from it + if ifid not in first_sample.interfaces: + continue + + ifrate = last_sample.interfaces[ifid].speed or 1000 + Mbps2Bps = (10 ** 6) / 8 + thisRx = ( + last_sample.interfaces[ifid].rx - + first_sample.interfaces[ifid].rx + ) % (2 ** 32) + thisTx = ( + last_sample.interfaces[ifid].tx - + first_sample.interfaces[ifid].tx + ) % (2 ** 32) + rxRate = 100.0 * thisRx / interval / ifrate / Mbps2Bps + txRate = 100.0 * thisTx / interval / ifrate / Mbps2Bps + if txRate > 100 or rxRate > 100: + txRate = min(txRate, 100.0) + rxRate = min(rxRate, 100.0) + logging.debug('Rate above 100%%.') + iface = last_sample.interfaces[ifid] + stats['network'][ifid] = { + 'name': ifid, 'speed': str(ifrate), + 'rxDropped': str(iface.rxDropped), + 'txDropped': str(iface.txDropped), + 'rxErrors': str(iface.rxErrors), + 'txErrors': str(iface.txErrors), + 'state': iface.operstate, + 'rxRate': '%.1f' % rxRate, + 'txRate': '%.1f' % txRate, + 'rx': str(iface.rx), + 'tx': str(iface.tx), + 'sampleTime': last_sample.timestamp, + } + rx += thisRx + tx += thisTx + rxDropped += last_sample.interfaces[ifid].rxDropped + txDropped += last_sample.interfaces[ifid].txDropped + total_rate += ifrate + + total_bytes_per_sec = (total_rate or 1000) * (10 ** 6) / 8 + stats['rxRate'] = 100.0 * rx / interval / total_bytes_per_sec + stats['txRate'] = 100.0 * tx / interval / total_bytes_per_sec + if stats['txRate'] > 100 or stats['rxRate'] > 100: + stats['txRate'] = min(stats['txRate'], 100.0) + stats['rxRate'] = min(stats['rxRate'], 100.0) + logging.debug(stats) + stats['rxDropped'] = rxDropped + stats['txDropped'] = txDropped + + return stats diff --git a/vdsm/virt/sampling.py b/vdsm/virt/sampling.py index 38c5892..5729a54 100644 --- a/vdsm/virt/sampling.py +++ b/vdsm/virt/sampling.py @@ -35,9 +35,9 @@ from vdsm import netinfo from vdsm import utils from vdsm.config import config -import v2v from .utils import ExpiringCache +from . import hoststats import caps @@ -196,27 +196,6 @@ class TimedSample(object): def __init__(self): self.timestamp = time.time() - - -_PROC_STAT_PATH = '/proc/stat' - - -def getBootTime(): - """ - Returns the boot time of the machine in seconds since epoch. - - Raises IOError if file access fails, or ValueError if boot time not - present in file. - """ - with open(_PROC_STAT_PATH) as proc_stat: - for line in proc_stat: - if line.startswith('btime'): - parts = line.split() - if len(parts) > 1: - return int(parts[1]) - else: - break - raise ValueError('Boot time not present') def _get_interfaces_and_samples(): @@ -590,158 +569,9 @@ if not self._stopEvent.isSet(): self._log.exception("Error while sampling stats") - @utils.memoized - def _boot_time(self): - # Try to get boot time only once, if N/A just log the error and never - # include it in the response. - try: - return getBootTime() - except (IOError, ValueError): - self._log.exception('Failed to get boot time') - return None - def get(self): - stats = self._empty_stats() - first_sample, last_sample, _ = self._samples.stats() - if first_sample is None: - return stats - - stats.update(_get_interfaces_stats(first_sample, last_sample)) - - interval = last_sample.timestamp - first_sample.timestamp - - jiffies = ( - last_sample.pidcpu.user - first_sample.pidcpu.user) % (2 ** 32) - stats['cpuUserVdsmd'] = jiffies / interval - jiffies = ( - last_sample.pidcpu.sys - first_sample.pidcpu.sys) % (2 ** 32) - stats['cpuSysVdsmd'] = jiffies / interval - - jiffies = ( - last_sample.totcpu.user - first_sample.totcpu.user) % (2 ** 32) - stats['cpuUser'] = jiffies / interval / self._ncpus - jiffies = ( - last_sample.totcpu.sys - first_sample.totcpu.sys) % (2 ** 32) - stats['cpuSys'] = jiffies / interval / self._ncpus - stats['cpuIdle'] = max(0.0, - 100.0 - stats['cpuUser'] - stats['cpuSys']) - stats['memUsed'] = last_sample.memUsed - stats['anonHugePages'] = last_sample.anonHugePages - stats['cpuLoad'] = last_sample.cpuLoad - - stats['diskStats'] = last_sample.diskStats - stats['thpState'] = last_sample.thpState - - if self._boot_time(): - stats['bootTime'] = self._boot_time() - - stats['numaNodeMemFree'] = last_sample.numaNodeMem.nodesMemSample - stats['cpuStatistics'] = _get_cpu_core_stats( - first_sample, last_sample) - - stats['v2vJobs'] = v2v.get_jobs_status() - return stats - - def _empty_stats(self): - return { - 'cpuUser': 0.0, - 'cpuSys': 0.0, - 'cpuIdle': 100.0, - 'rxRate': 0.0, # REQUIRED_FOR: engine < 3.6 - 'txRate': 0.0, # REQUIRED_FOR: engine < 3.6 - 'cpuSysVdsmd': 0.0, - 'cpuUserVdsmd': 0.0, - } - - -def _get_cpu_core_stats(first_sample, last_sample): - interval = last_sample.timestamp - first_sample.timestamp - - def compute_cpu_usage(cpu_core, mode): - jiffies = ( - last_sample.cpuCores.getCoreSample(cpu_core)[mode] - - first_sample.cpuCores.getCoreSample(cpu_core)[mode] - ) % (2 ** 32) - return ("%.2f" % (jiffies / interval)) - - cpu_core_stats = {} - for node_index, numa_node in caps.getNumaTopology().iteritems(): - cpu_cores = numa_node['cpus'] - for cpu_core in cpu_cores: - core_stat = { - 'nodeIndex': int(node_index), - 'cpuUser': compute_cpu_usage(cpu_core, 'user'), - 'cpuSys': compute_cpu_usage(cpu_core, 'sys'), - } - core_stat['cpuIdle'] = ( - "%.2f" % max(0.0, - 100.0 - - float(core_stat['cpuUser']) - - float(core_stat['cpuSys']))) - cpu_core_stats[str(cpu_core)] = core_stat - return cpu_core_stats - - -def _get_interfaces_stats(first_sample, last_sample): - interval = last_sample.timestamp - first_sample.timestamp - - rx = tx = rxDropped = txDropped = 0 - stats = {'network': {}} - total_rate = 0 - for ifid in last_sample.interfaces: - # it skips hot-plugged devices if we haven't enough information - # to count stats from it - if ifid not in first_sample.interfaces: - continue - - ifrate = last_sample.interfaces[ifid].speed or 1000 - Mbps2Bps = (10 ** 6) / 8 - thisRx = ( - last_sample.interfaces[ifid].rx - - first_sample.interfaces[ifid].rx - ) % (2 ** 32) - thisTx = ( - last_sample.interfaces[ifid].tx - - first_sample.interfaces[ifid].tx - ) % (2 ** 32) - rxRate = 100.0 * thisRx / interval / ifrate / Mbps2Bps - txRate = 100.0 * thisTx / interval / ifrate / Mbps2Bps - if txRate > 100 or rxRate > 100: - txRate = min(txRate, 100.0) - rxRate = min(rxRate, 100.0) - logging.debug('Rate above 100%%.') - iface = last_sample.interfaces[ifid] - stats['network'][ifid] = { - 'name': ifid, 'speed': str(ifrate), - 'rxDropped': str(iface.rxDropped), - 'txDropped': str(iface.txDropped), - 'rxErrors': str(iface.rxErrors), - 'txErrors': str(iface.txErrors), - 'state': iface.operstate, - 'rxRate': '%.1f' % rxRate, - 'txRate': '%.1f' % txRate, - 'rx': str(iface.rx), - 'tx': str(iface.tx), - 'sampleTime': last_sample.timestamp, - } - rx += thisRx - tx += thisTx - rxDropped += last_sample.interfaces[ifid].rxDropped - txDropped += last_sample.interfaces[ifid].txDropped - total_rate += ifrate - - total_bytes_per_sec = (total_rate or 1000) * (10 ** 6) / 8 - stats['rxRate'] = 100.0 * rx / interval / total_bytes_per_sec - stats['txRate'] = 100.0 * tx / interval / total_bytes_per_sec - if stats['txRate'] > 100 or stats['rxRate'] > 100: - stats['txRate'] = min(stats['txRate'], 100.0) - stats['rxRate'] = min(stats['rxRate'], 100.0) - logging.debug(stats) - stats['rxDropped'] = rxDropped - stats['txDropped'] = txDropped - - return stats + return hoststats.produce(self._ncpus, first_sample, last_sample) def _getLinkSpeed(dev): -- To view, visit https://gerrit.ovirt.org/42034 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Icce6acbd596d087491678d039282d5d37d761904 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>

4 24

Change in vdsm[ovirt-3.5]: tests: Enable again utils.memoized in the tests
by fromani＠redhat.com 20 Oct '15

20 Oct '15

Hello Nir Soffer, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/40030 to review the following change. Change subject: tests: Enable again utils.memoized in the tests ...................................................................... tests: Enable again utils.memoized in the tests Tests that may leave dirt in memoized functions should invalidate the functions during teardown. Since we disabled memoizing becuase of pulluted memoized functions in caps module, caps tests invalidate all memoized functions now. Change-Id: I019e2a2ad75973511cccd195a7e9eaa105d8154f Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-on: http://gerrit.ovirt.org/37841 Reviewed-by: Francesco Romani <fromani(a)redhat.com> Reviewed-by: Dan Kenigsberg <danken(a)redhat.com> Tested-by: Dan Kenigsberg <danken(a)redhat.com> --- M tests/capsTests.py M tests/testrunner.py 2 files changed, 7 insertions(+), 6 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/30/40030/1 diff --git a/tests/capsTests.py b/tests/capsTests.py index e3f7d6a..d0ac3d5 100644 --- a/tests/capsTests.py +++ b/tests/capsTests.py @@ -41,6 +41,13 @@ class TestCaps(TestCaseBase): + + def tearDown(self): + for name in dir(caps): + obj = getattr(caps, name) + if isinstance(obj, utils.memoized): + obj.invalidate() + def _readCaps(self, fileName): testPath = os.path.realpath(__file__) dirName = os.path.split(testPath)[0] diff --git a/tests/testrunner.py b/tests/testrunner.py index 13fe55e..37b4c95 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -411,10 +411,6 @@ raise AssertionError(msg) -def fakeMemoized(func): - return func - - if __name__ == '__main__': if "--help" in sys.argv: print("testrunner options:\n" @@ -426,6 +422,4 @@ # Mock panic() calls for tests utils.panic = panicMock - # Memoization during tests is a bad idea. - utils.memoized = fakeMemoized run() -- To view, visit https://gerrit.ovirt.org/40030 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I019e2a2ad75973511cccd195a7e9eaa105d8154f Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>

4 12

Change in vdsm[ovirt-3.5]: utils: Add memoized invalidation
by fromani＠redhat.com 20 Oct '15

20 Oct '15

Hello Piotr Kliczewski, Nir Soffer, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/40029 to review the following change. Change subject: utils: Add memoized invalidation ...................................................................... utils: Add memoized invalidation We learned in the hard way that memoizing and monkey-patching do work nicely together. However, disabling memoizing during the tests means that we do not test the same code in the application. This patch adds invalidate() method to memoized functions, so tests can clean up properly after they modify memozied data. This can also be used by application code to invalidate memoized functions in certain conditions. Change-Id: I3053b8ecc567f7c3154f7473dfd6b587823313ae Signed-off-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-on: http://gerrit.ovirt.org/37788 Reviewed-by: Francesco Romani <fromani(a)redhat.com> Reviewed-by: Piotr Kliczewski <piotr.kliczewski(a)gmail.com> Reviewed-by: Dan Kenigsberg <danken(a)redhat.com> --- M lib/vdsm/utils.py M tests/utilsTests.py 2 files changed, 67 insertions(+), 1 deletion(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/29/40029/1 diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 4619b74..944cf4d 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -971,9 +971,14 @@ self.cache[args] = value return value + def invalidate(self): + self.cache.clear() + def __get__(self, obj, objtype): """Support instance methods.""" - return functools.partial(self.__call__, obj) + wrapper = functools.partial(self.__call__, obj) + wrapper.invalidate = self.cache.clear + return wrapper def validateMinimalKeySet(dictionary, reqParams): diff --git a/tests/utilsTests.py b/tests/utilsTests.py index 5b3614d..2a0b8ef 100644 --- a/tests/utilsTests.py +++ b/tests/utilsTests.py @@ -18,6 +18,7 @@ # Refer to the README and COPYING files for full details of the license # +import collections import os.path import contextlib import errno @@ -662,3 +663,63 @@ self.assertTrue( hack < base/2, "picklecopy [%f] not faster than deepcopy [%f]" % (hack, base)) + + +@expandPermutations +class MemoizedTests(TestCaseBase): + + def setUp(self): + self.values = {} + self.accessed = collections.defaultdict(int) + + @permutations([[()], [("a",)], [("a", "b")]]) + def test_memoized_method(self, args): + self.values[args] = 42 + self.assertEqual(self.accessed[args], 0) + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 1) + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 1) + + @permutations([[()], [("a",)], [("a", "b")]]) + def test_memoized_function(self, args): + self.values[args] = 42 + self.assertEqual(self.accessed[args], 0) + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 1) + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 1) + + def test_key_error(self): + self.assertRaises(KeyError, self.memoized_method) + self.assertRaises(KeyError, self.memoized_method, "a") + self.assertRaises(KeyError, self.memoized_method, "a", "b") + + def test_invalidate_method(self): + args = ("a",) + self.values[args] = 42 + self.assertEqual(self.memoized_method(*args), 42) + self.memoized_method.invalidate() + self.assertEqual(self.memoized_method(*args), 42) + self.assertEqual(self.accessed[args], 2) + + def test_invalidate_function(self): + args = ("a",) + self.values[args] = 42 + self.assertEqual(memoized_function(self, *args), 42) + memoized_function.invalidate() + self.assertEqual(memoized_function(self, *args), 42) + self.assertEqual(self.accessed[args], 2) + + @utils.memoized + def memoized_method(self, *args): + return self.get(args) + + def get(self, key): + self.accessed[key] += 1 + return self.values[key] + + +(a)utils.memoized +def memoized_function(test, *args): + return test.get(args) -- To view, visit https://gerrit.ovirt.org/40029 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I3053b8ecc567f7c3154f7473dfd6b587823313ae Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski(a)gmail.com>

5 14

Change in vdsm[ovirt-3.5]: tests: disable memoization in testrunner
by fromani＠redhat.com 20 Oct '15

20 Oct '15

Hello Nir Soffer, Dan Kenigsberg, I'd like you to do a code review. Please visit https://gerrit.ovirt.org/39949 to review the following change. Change subject: tests: disable memoization in testrunner ...................................................................... tests: disable memoization in testrunner memoization during tests is harmful, because it adds another one hidden layer of global state shared across test runs. This may lead -and actually did- to hard to debug failures in apparently unrelated code. The net result is waste of developer's time, and decreased trust in automated tests because of mysterious failures. To fix that, we now monkeypatch in testrunner only utils.memoized with a fake which avoids caching. Change-Id: Ibd986403a76268995b83c9e6ac400e942ed24cb6 Signed-off-by: Francesco Romani <fromani(a)redhat.com> Reviewed-on: http://gerrit.ovirt.org/37748 Reviewed-by: Nir Soffer <nsoffer(a)redhat.com> Tested-by: Nir Soffer <nsoffer(a)redhat.com> Reviewed-by: Dan Kenigsberg <danken(a)redhat.com> --- M tests/testrunner.py 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/49/39949/1 diff --git a/tests/testrunner.py b/tests/testrunner.py index 37b4c95..13fe55e 100644 --- a/tests/testrunner.py +++ b/tests/testrunner.py @@ -411,6 +411,10 @@ raise AssertionError(msg) +def fakeMemoized(func): + return func + + if __name__ == '__main__': if "--help" in sys.argv: print("testrunner options:\n" @@ -422,4 +426,6 @@ # Mock panic() calls for tests utils.panic = panicMock + # Memoization during tests is a bad idea. + utils.memoized = fakeMemoized run() -- To view, visit https://gerrit.ovirt.org/39949 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibd986403a76268995b83c9e6ac400e942ed24cb6 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: ovirt-3.5 Gerrit-Owner: Francesco Romani <fromani(a)redhat.com> Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com> Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>

4 15

← Newer
1
...
7
8
9
10
11
12
13
...
92
Older →

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

vdsm-patches July 2015