Adam Litke has posted comments on this change.
Change subject: tests: FakeLVM test command failure ......................................................................
Patch Set 5:
(8 comments)
https://gerrit.ovirt.org/#/c/47864/5/tests/storagefakelib.py File tests/storagefakelib.py:
Line 114: opened=False, Line 115: active=bool(activate)) Line 116: Line 117: try: Line 118: vg_dict = self.lvmd.setdefault(vgName, {})
This line cannot raise KeyError.
ok, moved above. Line 119: lv_count = int(self.vgmd[vgName]['lv_count']) + 1 Line 120: except KeyError: Line 121: raise se.CannotCreateLogicalVolume(vgName, lvName) Line 122:
Line 115: active=bool(activate)) Line 116: Line 117: try: Line 118: vg_dict = self.lvmd.setdefault(vgName, {}) Line 119: lv_count = int(self.vgmd[vgName]['lv_count']) + 1
We have both vgmd[vgname] and lvmd[vgname]? This is error prone.
hmm... Maybe for another patch. Line 120: except KeyError: Line 121: raise se.CannotCreateLogicalVolume(vgName, lvName) Line 122: Line 123: if lvName in vg_dict:
Line 128: Line 129: def activateLVs(self, vgName, lvNames): Line 130: for lv in lvNames: Line 131: try: Line 132: self.lvmd[vgName][lv]['active'] = True
This can fail because of bug in fake lvm (missing 'active' key).
fixed. Line 133: self.lvmd[vgName][lv]['attr']['state'] = 'a' Line 134: except KeyError as e: Line 135: raise se.CannotActivateLogicalVolume(str(e)) Line 136:
Line 129: def activateLVs(self, vgName, lvNames): Line 130: for lv in lvNames: Line 131: try: Line 132: self.lvmd[vgName][lv]['active'] = True Line 133: self.lvmd[vgName][lv]['attr']['state'] = 'a'
If this line fails, this is a bug in fake lvm, not caller issue (missing 'a
yep. Fixed. Line 134: except KeyError as e: Line 135: raise se.CannotActivateLogicalVolume(str(e)) Line 136: Line 137: def addtag(self, vg, lv, tag):
Line 131: try: Line 132: self.lvmd[vgName][lv]['active'] = True Line 133: self.lvmd[vgName][lv]['attr']['state'] = 'a' Line 134: except KeyError as e: Line 135: raise se.CannotActivateLogicalVolume(str(e))
Lets do this:
Done Line 136: Line 137: def addtag(self, vg, lv, tag): Line 138: try: Line 139: self.lvmd[vg][lv]['tags'] += (tag,)
Line 135: raise se.CannotActivateLogicalVolume(str(e)) Line 136: Line 137: def addtag(self, vg, lv, tag): Line 138: try: Line 139: self.lvmd[vg][lv]['tags'] += (tag,)
Same, missing tags is hidden
Done Line 140: except KeyError: Line 141: raise se.MissingTagOnLogicalVolume("%s/%s" % (vg, lv), tag) Line 142: Line 143: def lvPath(self, vgName, lvName):
Line 144: return os.path.join(self.root, "dev", vgName, lvName) Line 145: Line 146: def getPV(self, pvName): Line 147: try: Line 148: md = deepcopy(self.pvmd[pvName])
Probably works, but better do:
Done Line 149: except KeyError: Line 150: raise se.InaccessiblePhysDev((pvName,)) Line 151: return real_lvm.PV(**md) Line 152:
Line 159: return real_lvm.VG(**vg_md) Line 160: Line 161: def _getLV(self, vgName, lvName): Line 162: try: Line 163: lv_md = deepcopy(self.lvmd[vgName][lvName])
Probably works, but better keep deepcopy outside the try.
Done Line 164: except KeyError: Line 165: raise se.LogicalVolumeDoesNotExistError("%s/%s" % (vgName, lvName)) Line 166: lv_attr = real_lvm.LV_ATTR(**lv_md['attr']) Line 167: lv_md['attr'] = lv_attr