Change in vdsm[master]: janitorial: vm: remove _reportError
by fromani＠redhat.com
Francesco Romani has uploaded a new change for review.
Change subject: janitorial: vm: remove _reportError
......................................................................
janitorial: vm: remove _reportError
Vm._reportError was an early attempt to build what
we have now with response.error().
With the new code in place there is no more reason
to use it, so this patch removes it.
Change-Id: Ie23167c0289bdb0326dcfe48903cd1bae905e9b5
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
---
M vdsm/virt/vm.py
1 file changed, 15 insertions(+), 27 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/69/38269/1
diff --git a/vdsm/virt/vm.py b/vdsm/virt/vm.py
index cd392c7..4be63ce 100644
--- a/vdsm/virt/vm.py
+++ b/vdsm/virt/vm.py
@@ -42,7 +42,7 @@
from vdsm import utils
from vdsm.compat import pickle
from vdsm.config import config
-from vdsm.define import ERROR, NORMAL, doneCode, errCode
+from vdsm.define import ERROR, NORMAL, doneCode
from vdsm.netinfo import DUMMY_BRIDGE
from storage import outOfProcess as oop
from storage import sd
@@ -2650,15 +2650,15 @@
if not params:
self.log.error("updateVmPolicy got an empty policy.")
- return self._reportError(key='MissParam',
- msg="updateVmPolicy got an empty policy.")
+ return response.error('MissParam',
+ 'updateVmPolicy got an empty policy.')
#
# Get the current QoS block
metadata_modified = False
qos = self._getVmPolicy()
if qos is None:
- return self._reportError(key='updateVmPolicyErr')
+ return response.error('updateVmPolicyErr')
#
# Process provided properties, remove property after it is processed
@@ -2712,8 +2712,7 @@
if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
return response.error('noVM')
else:
- return self._reportError(key='updateVmPolicyErr',
- msg=e.message)
+ return response.error('updateVmPolicyErr', e.message)
return {'status': doneCode}
@@ -2771,9 +2770,9 @@
found_device = self._findDeviceByNameOrPath(device_name,
device_path)
if found_device is None:
- return self._reportError(
- key='updateIoTuneErr',
- msg="Device {} not found".format(device_name))
+ return response.error(
+ 'updateIoTuneErr',
+ "Device {} not found".format(device_name))
# Merge the update with current values
dom = found_device.getXML()
@@ -2799,8 +2798,7 @@
if e.get_error_code() == libvirt.VIR_ERR_NO_DOMAIN:
return response.error('noVM')
else:
- return self._reportError(key='updateIoTuneErr',
- msg=e.message)
+ return response.error('updateIoTuneErr', e.message)
# Update both the ioTune arguments and device xml DOM
# so we are still up-to-date
@@ -3956,7 +3954,7 @@
def setBalloonTarget(self, target):
if self._dom is None:
- return self._reportError(key='balloonErr')
+ return response.error('balloonErr')
try:
target = int(target)
self._dom.setMemory(target)
@@ -3980,8 +3978,8 @@
try:
self._dom.setSchedulerParameters({'vcpu_quota': int(quota)})
except ValueError:
- return self._reportError(key='cpuTuneErr',
- msg='an integer is required for period')
+ return response.error('cpuTuneErr',
+ 'an integer is required for period')
except libvirt.libvirtError as e:
return self._reportException(key='cpuTuneErr', msg=e.message)
return {'status': doneCode}
@@ -3990,21 +3988,11 @@
try:
self._dom.setSchedulerParameters({'vcpu_period': int(period)})
except ValueError:
- return self._reportError(key='cpuTuneErr',
- msg='an integer is required for period')
+ return response.error('cpuTuneErr',
+ 'an integer is required for period')
except libvirt.libvirtError as e:
return self._reportException(key='cpuTuneErr', msg=e.message)
return {'status': doneCode}
-
- def _reportError(self, key, msg=None):
- """
- Produce an error status.
- """
- if msg is None:
- error = errCode[key]
- else:
- error = response.error(key, msg)
- return error
def _reportException(self, key, msg=None):
"""
@@ -4012,7 +4000,7 @@
This method should be called only within exception-handling context.
"""
self.log.exception("Operation failed")
- return self._reportError(key, msg)
+ return response.error(key, msg)
def _getUnderlyingDeviceAddress(self, devXml):
"""
--
To view, visit https://gerrit.ovirt.org/38269
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie23167c0289bdb0326dcfe48903cd1bae905e9b5
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Francesco Romani <fromani(a)redhat.com>
7 years, 11 months
Change in vdsm[master]: virt: add test to allow custom XML parsing test
by Martin Polednik
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>
7 years, 11 months
Change in vdsm[master]: Don't prepare image with illegal volumes
by alitke＠redhat.com
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>
7 years, 11 months
Change in vdsm[master]: tests: increase timeout to 30 seconds
by ykaplan＠redhat.com
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>
7 years, 11 months
Change in vdsm[master]: configfile: remove unneeded methods.
by mtayer＠redhat.com
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>
7 years, 11 months
Change in vdsm[ovirt-3.5]: tests: Enable again utils.memoized in the tests
by fromani＠redhat.com
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>
7 years, 11 months
Change in vdsm[ovirt-3.5]: utils: Add memoized invalidation
by fromani＠redhat.com
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>
7 years, 11 months
Change in vdsm[ovirt-3.5]: tests: disable memoization in testrunner
by fromani＠redhat.com
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>
7 years, 11 months
Change in vdsm[ovirt-3.5]: tests: clear libvirtconnection cache
by fromani＠redhat.com
Hello Nir Soffer, Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/39948
to review the following change.
Change subject: tests: clear libvirtconnection cache
......................................................................
tests: clear libvirtconnection cache
the libvirtconnection has a cache to reuse connections,
and this caching doesn't play well with the monkeypatching
which we do in tests.
In libvirtconnectionTests, we create fake libvirt objects,
most notably fake virConnect-s. Unfortunately, the test fails
to clean up properly the libvirtconnection cache.
So, the test runs just fine in isolation, but if run in batch
like in 'make check', later tests which expect the real object
become broken.
The failure presents itself like the following:
AttributeError: 'virConnect' object has no attribute 'getMemoryStats'
This patch fixes this error adding a facility to clear
the libvirtconnection cache, and clearing it after each
test.
Change-Id: If4d64920e5f26c276accf26b7a532461d04f02df
Signed-off-by: Francesco Romani <fromani(a)redhat.com>
Reviewed-on: http://gerrit.ovirt.org/37747
Reviewed-by: Nir Soffer <nsoffer(a)redhat.com>
Reviewed-by: Dan Kenigsberg <danken(a)redhat.com>
---
M lib/vdsm/libvirtconnection.py
M tests/libvirtconnectionTests.py
2 files changed, 12 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/48/39948/1
diff --git a/lib/vdsm/libvirtconnection.py b/lib/vdsm/libvirtconnection.py
index b1332db..33fdf11 100644
--- a/lib/vdsm/libvirtconnection.py
+++ b/lib/vdsm/libvirtconnection.py
@@ -95,6 +95,14 @@
return utils.retry(libvirtOpen, timeout=10, sleep=0.2)
+def _clear():
+ """
+ For clearing connections during the tests.
+ """
+ with __connectionLock:
+ __connections.clear()
+
+
def get(target=None, killOnFailure=True):
"""Return current connection to libvirt or open a new one.
Use target to get/create the connection object linked to that object.
diff --git a/tests/libvirtconnectionTests.py b/tests/libvirtconnectionTests.py
index 1ac60a0..18e1984 100644
--- a/tests/libvirtconnectionTests.py
+++ b/tests/libvirtconnectionTests.py
@@ -103,6 +103,10 @@
class testLibvirtconnection(TestCaseBase):
+
+ def tearDown(self):
+ libvirtconnection._clear()
+
@MonkeyPatch(libvirtconnection, 'libvirt', LibvirtMock())
def testCallSucceeded(self):
"""Positive test - libvirtMock does not raise any errors"""
--
To view, visit https://gerrit.ovirt.org/39948
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If4d64920e5f26c276accf26b7a532461d04f02df
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>
7 years, 11 months
Change in vdsm[ovirt-3.5]: vdsm: Fix memory leak in netlink.py
by jtt77777＠yahoo.com
John Taylor has uploaded a new change for review.
Change subject: vdsm: Fix memory leak in netlink.py
......................................................................
vdsm: Fix memory leak in netlink.py
The documentation for libnl says that calls to rtnl_link_get_by_name
increment the reference counter and should be released by a call to
rtnl_link_put(). That's not done in the get_link(name) function where
it is used and causes a memory leak.
Add a C function prototype for the libnl rtnl_link_put function,
construct the link_info, and call rtnl_link_put before returning from
get_link(name).
Change-Id: I6e4dc72bd77e46ba4f95dd8a622f5c3288115315
Signed-off-by: John Taylor <jtt77777(a)yahoo.com>
---
M lib/vdsm/netlink.py
1 file changed, 5 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/72/39372/1
diff --git a/lib/vdsm/netlink.py b/lib/vdsm/netlink.py
index afae5ce..d822944 100644
--- a/lib/vdsm/netlink.py
+++ b/lib/vdsm/netlink.py
@@ -64,8 +64,9 @@
if not link:
raise IOError(errno.ENODEV, '%s is not present in the system' %
name)
- return _link_info(cache, link)
-
+ link_info = _link_info(cache, link)
+ _rtnl_link_put(link)
+ return link_info
class NLSocketPool(object):
"""Pool of netlink sockets."""
@@ -332,5 +333,7 @@
_nl_af2str = _int_char_proto(('nl_af2str', LIBNL))
_rtnl_scope2str = _int_char_proto(('rtnl_scope2str', LIBNL_ROUTE))
+_rtnl_link_put = _none_proto(('rtnl_link_put', LIBNL_ROUTE))
+
_nl_link_cache = partial(_cache_manager, _rtnl_link_alloc_cache)
_nl_addr_cache = partial(_cache_manager, _rtnl_addr_alloc_cache)
--
To view, visit https://gerrit.ovirt.org/39372
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e4dc72bd77e46ba4f95dd8a622f5c3288115315
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: John Taylor <jtt77777(a)yahoo.com>
7 years, 11 months