Dan Kenigsberg has posted comments on this change.
Change subject: storage: Don't ignore nfs_mount_options in vdsm.conf
......................................................................
Patch Set 12: I would prefer that you didn't submit this
(6 inline comments)
only minor comments
....................................................
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')
Line 32: if nfs_config != ",".join(NFSConnection.DEFAULT_OPTIONS):
nit: I do not think the order of options matters, so a better approach would be
to split nfs_config, cast into a frozenset, and then compare.
Line 33: alert = se.MiscDeprecatedNFSOptions(nfs_config)
Line 34: alerts.append({'code': alert.code,
Line 35: 'message': alert.message,
Line 36: 'value': alert.value})
Line 33: alert = se.MiscDeprecatedNFSOptions(nfs_config)
Line 34: alerts.append({'code': alert.code,
Line 35: 'message': alert.message,
Line 36: 'value': alert.value})
Line 37: logging.warning("%s (%s)" % (alert.message, alert.value))
we try to avoid formatting if the log level is not low enough by
logging.warning("%s (%s)", alert.message, alert.value)
Line 38:
....................................................
File vdsm/storage/storageServer.py
Line 34: import fileSD
Line 35: import iscsi
Line 36: from sync import asyncmethod, AsyncCallStub
Line 37: from mount import MountError
Line 38: from vdsm.config import config
Saggi is going to be very angry about inroducing a new short-circuit into
vdsm.config.
I suppose that I'm fine with this, as this is here only for backward
compatibility of manually-configured hosts.
Line 39: import storage_exception as se
Line 40:
Line 41:
Line 42: class AliasAlreadyRegisteredError(RuntimeError):
Line 292: if ((timeout, retrans, version) == (None, None, None) and
Line 293: conf_options != ",".join(options)):
Line 294: # Overwrite defaults with config, then parse them and
store in
Line 295: # the opts dict so that they're stored as properties
below.
Line 296: self._log.warning("Using deprecated nfs_mount_options
from"\
redundant trailing backslash (f18's pep8 hates this)
Line 297: " vdsm.conf to mount %s: %s", export,
conf_options)
Line 298: options = ("".join(conf_options.split())).split(",")
Line 299: for opt in options:
Line 300: parts = opt.split("=")
Line 294: # Overwrite defaults with config, then parse them and
store in
Line 295: # the opts dict so that they're stored as properties
below.
Line 296: self._log.warning("Using deprecated nfs_mount_options
from"\
Line 297: " vdsm.conf to mount %s: %s", export,
conf_options)
Line 298: options = ("".join(conf_options.split())).split(",")
I find
conf_options.replace(' ', '')
or
opt = opt.strip()
as less confusing ways of removing spaces.
Line 299: for opt in options:
Line 300: parts = opt.split("=")
Line 301: if parts[0] in opts:
Line 302: opts[parts[0]] = parts[1]
Line 312: _addIntegerOption(options, "timeo", opts['timeo'])
Line 313: _addIntegerOption(options, "retrans", opts['retrans'])
Line 314: if opts['vers'] != 'auto':
Line 315: _addIntegerOption(options, "vers", opts['vers'])
Line 316: self._deprecatedNFSOptions = None
is anyone using this "_deprecatedNFSOptions"?
Line 317:
Line 318: self._remotePath = normpath(export)
Line 319: self._timeout = opts['timeo']
Line 320: self._retrans = opts['retrans']
--
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: 12
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Greg Padgett <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Eduardo <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Greg Padgett <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Shu Ming <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
