Change in vdsm[master]: vm: unit test for vm._waitForDriveRemoval
nsoffer at redhat.com
nsoffer at redhat.com
Wed Nov 25 22:05:58 UTC 2015
Nir Soffer has posted comments on this change.
Change subject: vm: unit test for vm._waitForDriveRemoval
......................................................................
Patch Set 5:
(8 comments)
https://gerrit.ovirt.org/#/c/48880/5/tests/vmTests.py
File tests/vmTests.py:
Line 1261: <disk type='file' snapshot='no'>
Line 1262: <source file='test_path'/>
Line 1263: </disk>"""
Line 1264: NETWORK_DRIVE_XML = """
Line 1265: <disk type='file' snapshot='no'>
type="network"
Line 1266: <source name='test_path'/>
Line 1267: </disk>"""
Line 1268: BLOCK_DRIVE_XML = """
Line 1269: <disk type='file' snapshot='no'>
Line 1265: <disk type='file' snapshot='no'>
Line 1266: <source name='test_path'/>
Line 1267: </disk>"""
Line 1268: BLOCK_DRIVE_XML = """
Line 1269: <disk type='file' snapshot='no'>
type="block"
See vmStorageTests for example xmls.
Line 1270: <source dev='/block_path'/>
Line 1271: </disk>"""
Line 1272:
Line 1273: drive_default = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1270: <source dev='/block_path'/>
Line 1271: </disk>"""
Line 1272:
Line 1273: drive_default = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1274: path='test_path', )
We can remove the default, we are not testing the Drive class here.
Line 1275: drive_file = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1276: path='test_path', diskType=DISK_TYPE.FILE)
Line 1277: drive_network = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1278: path='test_path', diskType=DISK_TYPE.NETWORK)
Line 1276: path='test_path', diskType=DISK_TYPE.FILE)
Line 1277: drive_network = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1278: path='test_path', diskType=DISK_TYPE.NETWORK)
Line 1279: drive_block = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1280: path= "/block_path", diskType=DISK_TYPE.BLOCK)
path is always /rhev/data-center/p/d/images/i/v, (p=poolid, d=domainid, i=imageid, v=volumeid) for file and block. For network the path is rbd:pool/volume, but it does not really matter. For the tests, using the same "test_path" is fine.
Line 1281:
Line 1282: @MonkeyPatch(vm, "config", make_config([
Line 1283: ("vars", "hotunplug_timeout", "0.1"),
Line 1284: ("vars", "hotunplug_check_interval", "0.03")
Line 1279: drive_block = Drive({}, log=logging.getLogger(''), index=0, iface="",
Line 1280: path= "/block_path", diskType=DISK_TYPE.BLOCK)
Line 1281:
Line 1282: @MonkeyPatch(vm, "config", make_config([
Line 1283: ("vars", "hotunplug_timeout", "0.1"),
> Made it still shorter.
0.03 is too short for overloaded ci slaves.
Lets use timeout=0.25, check_internal=0.1 will time out after retries, in the worst case after one retry, good enough.
Line 1284: ("vars", "hotunplug_check_interval", "0.03")
Line 1285: ]))
Line 1286: @MonkeyPatch(utils, "isBlockDevice", lambda x: x == "/block_path")
Line 1287: @permutations([[drive_default, FILE_DRIVE_XML],
Line 1282: @MonkeyPatch(vm, "config", make_config([
Line 1283: ("vars", "hotunplug_timeout", "0.1"),
Line 1284: ("vars", "hotunplug_check_interval", "0.03")
Line 1285: ]))
Line 1286: @MonkeyPatch(utils, "isBlockDevice", lambda x: x == "/block_path")
> Drive actually goes to the file system to check if the device is a block de
Looks ok, the actual code is dirty in this case :-)
Line 1287: @permutations([[drive_default, FILE_DRIVE_XML],
Line 1288: [drive_file, FILE_DRIVE_XML],
Line 1289: [drive_network, NETWORK_DRIVE_XML],
Line 1290: [drive_block, BLOCK_DRIVE_XML]])
Line 1338: <source file='invalid_test_path'/>
Line 1339: </disk>
Line 1340: <disk type='file' device='disk' snapshot='no'>
Line 1341: <source file='invalid_test_path'/>
Line 1342: </disk>
Lets update the xml of the devices to look more like the real xml, see vmStorageTests.
Line 1343: <interface>
Line 1344: <mac address='invalid_macAddr'/>
Line 1345: </interface>
Line 1346: </devices>
Line 1356:
Line 1357: def XMLDesc(self, flags):
Line 1358: self.xmldesc_fetched += 1
Line 1359: if self.xmldesc_fetched > self.times_to_match + 1:
Line 1360: raise BaseException(
You should raise an Exception, Base Exception is not for user code (in general).
But I don't think we need it, it will make it less useful, I thin we should just count the fetches and assert about them in the tests.
Line 1361: "_waitForDriveRemoval called XMLDesc too many times.")
Line 1362: if self.xmldesc_fetched > self.times_to_match:
Line 1363: return self.DEFAULT_XML
Line 1364: else:
--
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: 5
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