Dan Kenigsberg has posted comments on this change.
Change subject: storage: Don't ignore nfs_mount_options in vdsm.conf
......................................................................
Patch Set 13: I would prefer that you didn't submit this
(4 inline comments)
I have only one comment about set comparison, and one observations about the suggested
implementation which requires closer scrutiny by someone familiar with nfs/posixfs
differentiation.
....................................................
File vdsm/storage/alerts.py
Line 28: alerts = []
Line 29:
Line 30: # FIXME: Remove when nfs_mount_options is no longer supported
Line 31: nfs_config = config.get('irs',
'nfs_mount_options').replace(' ', '')
Line 32: if len(frozenset(nfs_config.split(',')) ^
the equality sign could have been clearer.
Line 33: frozenset(NFSConnection.DEFAULT_OPTIONS)) > 0:
Line 34: alert = se.MiscDeprecatedNFSOptions(nfs_config)
Line 35: alerts.append({'code': alert.code,
Line 36: 'message': alert.message,
....................................................
File vdsm/storage/hsm.py
Line 159: conDict.get('retrans', None),
Line 160: conDict.get('timeout', None)) == (None, None, None):
Line 161: conf_options = (config.get('irs',
'nfs_mount_options')
Line 162: .replace(' ', ''))
Line 163: if len(frozenset(conf_options.split(',')) ^ frozenset(
==
Line 164: storageServer.NFSConnection.DEFAULT_OPTIONS)) > 0:
Line 165: logging.warning("Using deprecated nfs_mount_options
from"
Line 166: " vdsm.conf to mount %s: %s",
Line 167: conDict.get('connection', '(unknown)'),
conf_options)
Line 177: conDict.get('connection', None))
Line 178: elif typeName == 'nfs':
Line 179: params = tryDeprecatedNfsParams(conDict)
Line 180: if params is not None:
Line 181: # Hack to support vdsm.conf nfs_mount_options
oh, so using nfs with nfs_mount_options, is interpreted as posixfs. do we really want
that? if I recalled correctly we had ideas that nfs would have specific stuff beyond
posixfs.
Line 182: typeName = 'posixfs'
Line 183: else:
Line 184: version = conDict.get('protocol_version', "3")
Line 185: version = str(version)
....................................................
File vdsm/storage/storage_exception.py
Line 173: code = 2008
Line 174: message = "Directory cleanup failure"
Line 175:
Line 176:
Line 177: class MiscDeprecatedNFSOptions(StorageException):
Shu Ming, I believe that your comment was not correct. Misc* is related to exceptions
raised in the misc.py module, unrelated to the error code.
I personally do not care much about the name, though.
Line 178: def __init__(self, options):
Line 179: self.value = "options=%s" % (options,)
Line 180: code = 2009
Line 181: message = "Using deprecated nfs_mount_options parameter from
vdsm.conf."\
--
To view, visit
http://gerrit.ovirt.org/5368
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c1d13bfd98a9b41b9728bdcb7f01b3161c26bd8
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi(a)redhat.com>
Gerrit-Reviewer: Shu Ming <shuming(a)linux.vnet.ibm.com>
Gerrit-Reviewer: oVirt Jenkins CI Server