Change in vdsm[master]: vm: unit test for vm._waitForDriveRemoval
mmirecki at redhat.com
mmirecki at redhat.com
Mon Nov 23 15:00:30 UTC 2015
Marcin Mirecki has posted comments on this change.
Change subject: vm: unit test for vm._waitForDriveRemoval
......................................................................
Patch Set 1:
(8 comments)
https://gerrit.ovirt.org/#/c/48880/1/tests/vmTests.py
File tests/vmTests.py:
Line 1256: pass
Line 1257: drive = Drive()
Line 1258: drive.name = ''
Line 1259: drive.diskType = DISK_TYPE.FILE
Line 1260: drive.path = 'test_path'
> Why not use a real drive object? See vmStorageTests.py to see how to use re
Done
Line 1261: with fake.VM() as testvm:
Line 1262: testvm._dom = FakeVmDom(999)
Line 1263: testvm._hotunplug_timeout = 1
Line 1264: try:
Line 1257: drive = Drive()
Line 1258: drive.name = ''
Line 1259: drive.diskType = DISK_TYPE.FILE
Line 1260: drive.path = 'test_path'
Line 1261: with fake.VM() as testvm:
> fake.VM is too complex, we cannot use such monster in the tests. I think a
Done
Line 1262: testvm._dom = FakeVmDom(999)
Line 1263: testvm._hotunplug_timeout = 1
Line 1264: try:
Line 1265: testvm._waitForDriveRemoval(drive)
Line 1261: with fake.VM() as testvm:
Line 1262: testvm._dom = FakeVmDom(999)
Line 1263: testvm._hotunplug_timeout = 1
Line 1264: try:
Line 1265: testvm._waitForDriveRemoval(drive)
> Yes, but 2 seconds is too much.
With both values changed to 0, utils.monotonic_time() >= deadline is true after about calling sleep(0) about 2 to 4 times.
Line 1266: raise BaseException("HotunplugTimeout should be raised")
Line 1267: except HotunplugTimeout:
Line 1268: pass
Line 1269:
Line 1264: try:
Line 1265: testvm._waitForDriveRemoval(drive)
Line 1266: raise BaseException("HotunplugTimeout should be raised")
Line 1267: except HotunplugTimeout:
Line 1268: pass
> Use self.assertRaises instead.
Done
Line 1269:
Line 1270: testvm._dom = FakeVmDom()
Line 1271: testvm._hotunplug_timeout = 1
Line 1272: testvm._wait_for_sleep = self._wait_for_sleep
Line 1265: testvm._waitForDriveRemoval(drive)
Line 1266: raise BaseException("HotunplugTimeout should be raised")
Line 1267: except HotunplugTimeout:
Line 1268: pass
Line 1269:
> The code bellow should be in another test - we want to test one scenario pe
Done
Line 1270: testvm._dom = FakeVmDom()
Line 1271: testvm._hotunplug_timeout = 1
Line 1272: testvm._wait_for_sleep = self._wait_for_sleep
Line 1273: testvm._waitForDriveRemoval(drive)
Line 1298: </disk>
Line 1299: </devices>
Line 1300: </domain>
Line 1301: """
Line 1302: return '<does_not_match/>'
> This is good only for drive test, but we need it also for the nic in the ne
Done
Line 1303:
Line 1304:
Line 1305: class ChangingSchedulerDomain(object):
Line 1306:
https://gerrit.ovirt.org/#/c/48880/1/vdsm/virt/vm.py
File vdsm/virt/vm.py:
Line 325: self._vcpuTuneInfo = {}
Line 326: self._numaInfo = {}
Line 327: self._vmJobs = None
Line 328: self._clientPort = ''
Line 329: self._hotunplug_timeout = config.getint('vars', 'hotunplug_timeout')
> Vm is too big, we cannot add more state.
Done
Line 330:
Line 331: def _get_lastStatus(self):
Line 332: # note that we don't use _statusLock here. One of the reasons is the
Line 333: # non-obvious recursive locking in the following flow:
Line 2689: if utils.monotonic_time() > deadline:
Line 2690: raise HotunplugTimeout("Timeout detaching drive %s"
Line 2691: % drive.name)
Line 2692:
Line 2693: def _wait_for_sleep(self):
> Yes, but again VM is too big, we cannot add such methods for testing. The b
Done
Line 2694: time.sleep(1)
Line 2695:
Line 2696: def _isDriveAttached(self, drive):
Line 2697: root = ET.fromstring(self._dom.XMLDesc(0))
--
To view, visit https://gerrit.ovirt.org/48880
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4d1f86e17132a7099f109a5e33407c56dde40df2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: Amit Aviram <aaviram at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Vinzenz Feenstra <vfeenstr at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list