Federico Simoncelli has posted comments on this change.

Change subject: lvm: Add an option to replace locking type 4
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.ovirt.org/#/c/23645/7/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 256:         else:
Line 257:             conf = self._getCachedExtraCfg()
Line 258: 
Line 259:         if rw:
Line 260:             conf = conf.replace("locking_type=4", "locking_type=1")
It seems that you rebased on top of "Towards a more (block) secure HSM" which 
is already providing RE_LOCKING_TYPE and setROMD/setRWMD. We probably want to 
have just one way to substituting/set the locking_type.

I was wondering, instead of using string replacements wouldn't it be possible 
to use something similar to the device filters? E.g.:

 LVMCONF_TEMPLATE = """
 ...
 devices {
  %(filters)s
 }
 ...
 global {
 ...
  locking_type=%(lockingType)s
 ...
 }
 """

 def _buildConfig(devList, lockingType):
     flt = _buildFilter(chain(devList, USER_DEV_LIST))
     conf = _current_lvmconf % {'filters': flt, 'lockingType': lockingType}
     return conf.replace("\n", " ")
Line 261:         newcmd += ["--config", conf]
Line 262: 
Line 263:         if len(cmd) > 1:
Line 264:             newcmd += cmd[1:]


-- 
To view, visit http://gerrit.ovirt.org/23645
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9a67a7fa20145763d8ab5cdbf293a9c3eb070067
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Barak Azulay <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to