Saggi Mizrahi has posted comments on this change.
Change subject: Made spm securing logic more generic
......................................................................
Patch Set 2: (5 inline comments)
....................................................
File vdsm/storage/securable.py
Line 0
Line 1: from threading import Event
Line 2: from functools import wraps
Line 3:
Line 4: class SecureError(RuntimeError): pass
There is no need for tight Coupling. I'll fix it in hsm.
Line 5:
Line 6: class Securable(type):
Line 7: def __new__(mcs, name, bases, fdict):
Line 8:
Line 27: def secured(f):
Line 28: @wraps(f)
Line 29: def wrapper(*args, **kwargs):
Line 30: if not hasattr(args[0], "__securable__"):
Line 31: raise RuntimeError("Secured object is not a securable")
The object being secured is not a 'Securable'?
Line 32:
Line 33: override = kwargs.get(OVERRIDE_ARG, False)
Line 34: try:
Line 35: del kwargs[OVERRIDE_ARG]
Line 29: def wrapper(*args, **kwargs):
Line 30: if not hasattr(args[0], "__securable__"):
Line 31: raise RuntimeError("Secured object is not a securable")
Line 32:
Line 33: override = kwargs.get(OVERRIDE_ARG, False)
Either the value that the user put in the kwarg or the default, "false". I
don't see what's wrong
Line 34: try:
Line 35: del kwargs[OVERRIDE_ARG]
Line 36: except KeyError:
Line 37: pass
Line 35: del kwargs[OVERRIDE_ARG]
Line 36: except KeyError:
Line 37: pass
Line 38:
Line 39: if not (args[0]._isSafe() or override):
graa
Line 40: raise SecureError()
Line 41:
Line 42: return f(*args, **kwargs)
Line 43: return wrapper
....................................................
File vdsm/storage/spm.py
Line 263: spUUID, self.spmRole, -1, -1)
Line 264: return dict(spm_st=st)
Line 265:
Line 266:
Line 267: @secured
The logic can be easily reversed. But I don't think this is the correct thing
to do. It's a lot more expressive that a method is secured (different from a
normal method). Nothing can prevent a developer from making a mistake anyway
because the Image class is not protected. I'll change it in a later patch for
the +1.
Line 268: @logged()
Line 269: def public_upgradeStoragePool(self, spUUID, targetDomVersion):
Line 270: targetDomVersion = int(targetDomVersion)
Line 271: self._upgradePool(spUUID, targetDomVersion)
--
To view, visit http://gerrit.usersys.redhat.com/991
To unsubscribe, visit http://gerrit.usersys.redhat.com/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e16feafc03d6efe5c904b18d4d27177dd14b905
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Ayal Baron
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://fedorahosted.org/mailman/listinfo/vdsm-patches
