Yeela Kaplan has uploaded a new change for review.
Change subject: sd: unify lease params ......................................................................
sd: unify lease params
Change-Id: Id65d5dd2a0130a19234d7fb699854a09f5748566 Signed-off-by: Yeela Kaplan ykaplan@redhat.com --- M lib/vdsm/utils.py M vdsm/storage/blockSD.py M vdsm/storage/fileSD.py M vdsm/storage/sd.py 4 files changed, 25 insertions(+), 53 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/18/16318/1
diff --git a/lib/vdsm/utils.py b/lib/vdsm/utils.py index 538498f..1c714ed 100644 --- a/lib/vdsm/utils.py +++ b/lib/vdsm/utils.py @@ -912,3 +912,7 @@ logging.error("Panic: %s", msg, exc_info=True) os.killpg(0, 9) sys.exit(-3) + + +def getRange(d, begin, end): + return dict(e for i, e in enumerate(d.items()) if begin <= i <= end) diff --git a/vdsm/storage/blockSD.py b/vdsm/storage/blockSD.py index 72b3ef0..e8f162a 100644 --- a/vdsm/storage/blockSD.py +++ b/vdsm/storage/blockSD.py @@ -560,19 +560,11 @@ sd.DMDK_ROLE: sd.REGULAR_DOMAIN, sd.DMDK_POOLS: [], sd.DMDK_LOCK_POLICY: '', - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC: sd.DEFAULT_LEASE_PARAMS[ - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[ - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_IO_OP_TIMEOUT_SEC: sd.DEFAULT_LEASE_PARAMS[ - sd.DMDK_IO_OP_TIMEOUT_SEC], - sd.DMDK_LEASE_RETRIES: sd.DEFAULT_LEASE_PARAMS[ - sd.DMDK_LEASE_RETRIES], DMDK_VGUUID: vgUUID, DMDK_LOGBLKSIZE: logBlkSize, DMDK_PHYBLKSIZE: phyBlkSize, } - + initialMetadata.update(sd.SUB_DEFAULT_LEASE_PARAMS) initialMetadata.update(mapping)
md.update(initialMetadata) diff --git a/vdsm/storage/fileSD.py b/vdsm/storage/fileSD.py index dcc6baf..daac802 100644 --- a/vdsm/storage/fileSD.py +++ b/vdsm/storage/fileSD.py @@ -214,7 +214,8 @@ # FIXME : This is 99% like the metadata in block SD # Do we really need to keep the EXPORT_PATH? # no one uses it - md.update({ + meta = {} + meta.update({ sd.DMDK_VERSION: version, sd.DMDK_SDUUID: sdUUID, sd.DMDK_TYPE: storageType, @@ -223,16 +224,10 @@ sd.DMDK_ROLE: sd.REGULAR_DOMAIN, sd.DMDK_POOLS: [], sd.DMDK_LOCK_POLICY: '', - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC: - sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_LEASE_TIME_SEC: sd.DEFAULT_LEASE_PARAMS[ - sd.DMDK_LOCK_RENEWAL_INTERVAL_SEC], - sd.DMDK_IO_OP_TIMEOUT_SEC: - sd.DEFAULT_LEASE_PARAMS[sd.DMDK_IO_OP_TIMEOUT_SEC], - sd.DMDK_LEASE_RETRIES: - sd.DEFAULT_LEASE_PARAMS[sd.DMDK_LEASE_RETRIES], REMOTE_PATH: remotePath }) + meta.update(sd.SUB_DEFAULT_LEASE_PARAMS) + md.update(meta)
def getReadDelay(self): stats = misc.readspeed(self.metafile, 4096) diff --git a/vdsm/storage/sd.py b/vdsm/storage/sd.py index fd79059..630abad 100644 --- a/vdsm/storage/sd.py +++ b/vdsm/storage/sd.py @@ -30,7 +30,7 @@ import resourceFactories from resourceFactories import IMAGE_NAMESPACE, VOLUME_NAMESPACE import resourceManager as rm -from vdsm import constants +from vdsm import constants, utils import clusterlock import outOfProcess as oop from persistentDict import unicodeEncoder, unicodeDecoder @@ -121,11 +121,14 @@ DMDK_IO_OP_TIMEOUT_SEC = 'IOOPTIMEOUTSEC' DMDK_LEASE_RETRIES = 'LEASERETRIES'
-DEFAULT_LEASE_PARAMS = {DMDK_LOCK_POLICY: "ON", - DMDK_LEASE_RETRIES: 3, +# order is significant - DO NOT CHANGE +DEFAULT_LEASE_PARAMS = {DMDK_LEASE_RETRIES: 3, DMDK_LEASE_TIME_SEC: 30, DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, - DMDK_IO_OP_TIMEOUT_SEC: 1} + DMDK_IO_OP_TIMEOUT_SEC: 1, + DMDK_LOCK_POLICY: "ON"} + +SUB_DEFAULT_LEASE_PARAMS = utils.getRange(DEFAULT_LEASE_PARAMS, 0, 3)
MASTER_FS_DIR = 'master' VMS_DIR = 'vms' @@ -268,23 +271,13 @@ DMDK_POOLS: (lambda s: s.split(",") if s else [], lambda poolUUIDs: ",".join(poolUUIDs)), DMDK_LOCK_POLICY: (str, str), - DMDK_LOCK_RENEWAL_INTERVAL_SEC: ( - lambda val: intOrDefault( - DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC], val), - intEncode), - DMDK_LEASE_TIME_SEC: ( - lambda val: intOrDefault( - DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC], val), - intEncode), - DMDK_IO_OP_TIMEOUT_SEC: ( - lambda val: intOrDefault( - DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC], val), - intEncode), - DMDK_LEASE_RETRIES: ( - lambda val: intOrDefault( - DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES], val), - intEncode), } + +for param in SUB_DEFAULT_LEASE_PARAMS: + SD_MD_FIELDS[param] = ( + lambda val: intOrDefault( + DEFAULT_LEASE_PARAMS[param], val), + intEncode)
class StorageDomain: @@ -332,12 +325,7 @@ if not domVersion: domVersion = self.getVersion()
- leaseParams = ( - DEFAULT_LEASE_PARAMS[DMDK_LOCK_RENEWAL_INTERVAL_SEC], - DEFAULT_LEASE_PARAMS[DMDK_LEASE_TIME_SEC], - DEFAULT_LEASE_PARAMS[DMDK_LEASE_RETRIES], - DEFAULT_LEASE_PARAMS[DMDK_IO_OP_TIMEOUT_SEC], - ) + leaseParams = SUB_DEFAULT_LEASE_PARAMS.values()
try: clusterLockClass = self._clusterLockTable[domVersion][0] @@ -478,12 +466,7 @@
def acquireClusterLock(self, hostID): self.refresh() - self._clusterLock.setParams( - self.getMetaParam(DMDK_LOCK_RENEWAL_INTERVAL_SEC), - self.getMetaParam(DMDK_LEASE_TIME_SEC), - self.getMetaParam(DMDK_LEASE_RETRIES), - self.getMetaParam(DMDK_IO_OP_TIMEOUT_SEC) - ) + self._clusterLock.setParams(*self.getLeaseParams()) self._clusterLock.acquire(hostID)
def releaseClusterLock(self): @@ -527,10 +510,8 @@ self.setMetaParams(leaseParamPack)
def getLeaseParams(self): - keys = [DMDK_LOCK_RENEWAL_INTERVAL_SEC, DMDK_LEASE_TIME_SEC, - DMDK_IO_OP_TIMEOUT_SEC, DMDK_LEASE_RETRIES] params = {} - for key in keys: + for key in SUB_DEFAULT_LEASE_PARAMS: params[key] = self.getMetaParam(key) return params
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 1: Fails
Build Failed
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3021/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3099/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2210/ : FAILURE
Ayal Baron has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 1: I would prefer that you didn't submit this
(2 inline comments)
.................................................... File vdsm/storage/sd.py Line 120: DMDK_LEASE_TIME_SEC = 'LEASETIMESEC' Line 121: DMDK_IO_OP_TIMEOUT_SEC = 'IOOPTIMEOUTSEC' Line 122: DMDK_LEASE_RETRIES = 'LEASERETRIES' Line 123: Line 124: # order is significant - DO NOT CHANGE when creating a dictionary you have no guarantee on the order, this looks plain wrong to me. Line 125: DEFAULT_LEASE_PARAMS = {DMDK_LEASE_RETRIES: 3, Line 126: DMDK_LEASE_TIME_SEC: 30, Line 127: DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, Line 128: DMDK_IO_OP_TIMEOUT_SEC: 1,
Line 127: DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, Line 128: DMDK_IO_OP_TIMEOUT_SEC: 1, Line 129: DMDK_LOCK_POLICY: "ON"} Line 130: Line 131: SUB_DEFAULT_LEASE_PARAMS = utils.getRange(DEFAULT_LEASE_PARAMS, 0, 3) why not just define sub_default_lease_params first without lock_policy and then just define default_lease_params as sub_def... + lock_policy? Line 132: Line 133: MASTER_FS_DIR = 'master' Line 134: VMS_DIR = 'vms' Line 135: TASKS_DIR = 'tasks'
Eduardo has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 1: (1 inline comment)
.................................................... File vdsm/storage/sd.py Line 120: DMDK_LEASE_TIME_SEC = 'LEASETIMESEC' Line 121: DMDK_IO_OP_TIMEOUT_SEC = 'IOOPTIMEOUTSEC' Line 122: DMDK_LEASE_RETRIES = 'LEASERETRIES' Line 123: Line 124: # order is significant - DO NOT CHANGE collections.OrderedDict? New in version 2.7 http://code.activestate.com/recipes/576693 Line 125: DEFAULT_LEASE_PARAMS = {DMDK_LEASE_RETRIES: 3, Line 126: DMDK_LEASE_TIME_SEC: 30, Line 127: DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, Line 128: DMDK_IO_OP_TIMEOUT_SEC: 1,
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 2: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4126/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3231/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4047/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4712/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4788/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3903/ : SUCCESS
Ayal Baron has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 3:
(3 comments)
.................................................... File vdsm/storage/fileSD.py Line 214: # initialize domain metadata content Line 215: # FIXME : This is 99% like the metadata in block SD Line 216: # Do we really need to keep the EXPORT_PATH? Line 217: # no one uses it Line 218: meta = {} if you're already changing the name, why not make it consistent with blockSD? Line 219: meta.update({ Line 220: sd.DMDK_VERSION: version, Line 221: sd.DMDK_SDUUID: sdUUID, Line 222: sd.DMDK_TYPE: storageType,
Line 223: sd.DMDK_CLASS: domClass, Line 224: sd.DMDK_DESCRIPTION: domainName, Line 225: sd.DMDK_ROLE: sd.REGULAR_DOMAIN, Line 226: sd.DMDK_POOLS: [], Line 227: sd.DMDK_LOCK_POLICY: '', why not unify all of the above as well? Line 228: REMOTE_PATH: remotePath Line 229: }) Line 230: meta.update(sd.SUB_DEFAULT_LEASE_PARAMS) Line 231: md.update(meta)
.................................................... File vdsm/storage/sd.py Line 120: DMDK_LEASE_TIME_SEC = 'LEASETIMESEC' Line 121: DMDK_IO_OP_TIMEOUT_SEC = 'IOOPTIMEOUTSEC' Line 122: DMDK_LEASE_RETRIES = 'LEASERETRIES' Line 123: Line 124: SUB_DEFAULT_LEASE_PARAMS = {DMDK_LEASE_RETRIES: 3, I don't understand the name of this dict Line 125: DMDK_LEASE_TIME_SEC: 60, Line 126: DMDK_LOCK_RENEWAL_INTERVAL_SEC: 5, Line 127: DMDK_IO_OP_TIMEOUT_SEC: 10} Line 128:
Itamar Heim has posted comments on this change.
Change subject: sd: unify lease params ......................................................................
Patch Set 3:
ping - still relevant?
Itamar Heim has abandoned this change.
Change subject: sd: unify lease params ......................................................................
Abandoned
abandoning - old. no reply. not touched for a while. please restore if relevant
vdsm-patches@lists.fedorahosted.org