Change in vdsm[master]: FakeLVM: support single LV and array in activateLVs

nsoffer at redhat.com nsoffer at redhat.com
Wed Apr 6 07:04:46 UTC 2016


Nir Soffer has posted comments on this change.

Change subject: FakeLVM: support single LV and array in activateLVs
......................................................................


Patch Set 2: Code-Review-1

(2 comments)

https://gerrit.ovirt.org/#/c/49547/2/tests/storagefakelib.py
File tests/storagefakelib.py:

Line 132:         self.lvmd[(vgName, lvName)] = lv_md
Line 133:         self.vgmd[vgName]['lv_count'] = str(lv_count)
Line 134: 
Line 135:     def activateLVs(self, vgName, lvNames):
Line 136:         lvNames = real_lvm._normalizeargs(lvNames)
We should not support this usage. Instead of adding this in the tests, we should eliminate this in the original code.

The pythonic way to support single and multiple argument is using *lvNames - you always get a tuple with 0 or more items.
Line 137:         for lv in lvNames:
Line 138:             try:
Line 139:                 lv_md = self.lvmd[(vgName, lv)]
Line 140:             except KeyError as e:


https://gerrit.ovirt.org/#/c/49547/2/tests/storagefakelibTests.py
File tests/storagefakelibTests.py:

Line 289:         with self.base_config() as lvm:
Line 290:             lvm.createLV(self.VG_NAME, self.LV_NAME, str(self.LV_SIZE_MB),
Line 291:                          activate=False)
Line 292:             lvm.activateLVs(self.VG_NAME, lv_arg)
Line 293:             for lv_name in real_lvm._normalizeargs(lv_arg):
Please don't use this private helper here, instead test that the result is having the lv LV_NAME active.
Line 294:                 lv = lvm.getLV(self.VG_NAME, lv_name)
Line 295:                 self.assertTrue(lv.active)
Line 296:                 self.assertEqual('a', lv.attr.state)
Line 297: 


-- 
To view, visit https://gerrit.ovirt.org/49547
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I091aab3c9ccab16e8d4d9693957a061afac1366d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list