Change in vdsm[master]: tests: FakeLVM test command failure

alitke at redhat.com alitke at redhat.com
Fri Nov 6 21:49:19 UTC 2015


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


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

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


More information about the vdsm-patches mailing list