Alon Bar-Lev has posted comments on this change. Change subject: kdump: Add kdump plugin ......................................................................
Patch Set 3: (15 comments) http://gerrit.ovirt.org/#/c/28385/3/src/plugins/ovirt-host-deploy/core/offlinepackager.py File src/plugins/ovirt-host-deploy/core/offlinepackager.py: Line 67: 'epoch': '0', Line 68: 'arch': 'noarch', Line 69: }, Line 70: ] Line 71: elif tuple(patterns) == ('kexec-tools',): per what you wrote, this is to be returned only for fedora>=20 and rhel=6, as in rhel-7 we do not have support, so ovirt-node of these will require to report unsupported. Line 72: return [ Line 73: { Line 74: 'operation': 'installed', Line 75: 'display_name': 'kexec-tools', http://gerrit.ovirt.org/#/c/28385/3/src/plugins/ovirt-host-deploy/kdump/packages.py File src/plugins/ovirt-host-deploy/kdump/packages.py: Line 24: import gettext Line 25: import platform Line 26: import re Line 27: Line 28: from distutils.version import LooseVersion this is fedora/rhel specific, it cannot be imported here. I would also try to avoid using it anyway... if possible. but otherwise, import it only if you are fedora/rhel distro Line 29: Line 30: from otopi import util Line 31: from otopi import plugin Line 32: from otopi import constants as otopicons Line 118: # line with fence_kdump config backup begin/end Line 119: if fk_bck_m.group('value') == 'begin': Line 120: inside_backup = True Line 121: else: Line 122: inside_backup = False why do we need to know if we are inside? all we need to know is that we back it up it sometime in past, as these comments, they will not match the key value pattern. we just need to copy-it-out as is. Line 123: else: Line 124: if inside_backup: Line 125: backup.append(line) Line 126: else: Line 138: new_content.append( Line 139: 'fence_kdump_args -p %s -i %s' % ( Line 140: port, Line 141: interval, Line 142: ) try: new_content.extend( 'xxxxx' % ( ), 'xxxxxx' % ( ), ) Line 143: ) Line 144: return new_content Line 145: Line 146: @plugin.event( Line 146: @plugin.event( Line 147: stage=plugin.Stages.STAGE_INIT, Line 148: ) Line 149: def _setup(self): Line 150: self.environment[odeploycons.KdumpEnv.ENABLE] = False set the supported as well. Line 151: Line 152: @plugin.event( Line 153: stage=plugin.Stages.STAGE_CUSTOMIZATION, Line 154: priority=plugin.Stages.PRIORITY_HIGH, Line 152: @plugin.event( Line 153: stage=plugin.Stages.STAGE_CUSTOMIZATION, Line 154: priority=plugin.Stages.PRIORITY_HIGH, Line 155: ) Line 156: def _validation(self): _customization :) Line 157: fk_supported = True Line 158: Line 159: result = self.packager.queryPackages( Line 160: patterns=(self._KEXEC_TOOLS_PKG,), Line 153: stage=plugin.Stages.STAGE_CUSTOMIZATION, Line 154: priority=plugin.Stages.PRIORITY_HIGH, Line 155: ) Line 156: def _validation(self): Line 157: fk_supported = True always assume unsupported/failure. Line 158: Line 159: result = self.packager.queryPackages( Line 160: patterns=(self._KEXEC_TOOLS_PKG,), Line 161: ) Line 162: if not result: Line 163: self.logger.debug('Package kexec-tools not found') Line 164: fk_supported = False Line 165: else: Line 166: entry = result[0] go over all results, so that we do not miss multiple. Line 167: self.logger.debug('Found kexec-tools %s', entry) Line 168: Line 169: cur_version = '%s-%s' % ( Line 170: entry['version'], Line 187: cur_version, Line 188: ) Line 189: fk_supported = False Line 190: Line 191: self.environment.setdefault( it is not set default... in this case it is real assignment... to reduce logic... minver = self._get_min_kexec_tools_version() if minver is not None: from distutils.version import LooseVersion result = self.packager.queryPackages(...) self.logger.debug("minver: %s, result=%s", minver, result); for package in result: if package.version >= minver: self.environment[odeploycons.KdumpEnv.SUPPORTED] = True break Line 192: odeploycons.KdumpEnv.SUPPORTED, Line 193: fk_supported Line 194: ) Line 195: Line 196: @plugin.event( Line 197: stage=plugin.Stages.STAGE_PACKAGES, Line 198: condition=lambda self: ( Line 199: self.environment[odeploycons.KdumpEnv.ENABLE] and Line 200: self.environment[odeploycons.KdumpEnv.SUPPORTED] enabled is enough Line 201: ), Line 202: ) Line 203: def _packages(self): Line 204: self.packager.installUpdate( Line 208: @plugin.event( Line 209: stage=plugin.Stages.STAGE_MISC, Line 210: condition=lambda self: ( Line 211: self.environment[odeploycons.KdumpEnv.ENABLE] and Line 212: self.environment[odeploycons.KdumpEnv.SUPPORTED] enabled is enough Line 213: ), Line 214: ) Line 215: def _reconfigure(self): Line 216: with open(odeploycons.FileLocations.KDUMP_CONFIG_FILE, 'r') as f: Line 220: content=content, Line 221: engine_node=self.environment[ Line 222: odeploycons.KdumpEnv.DESTINATION_ADDRESS Line 223: ], Line 224: port=str( conversion, if any, should be done in function Line 225: self.environment[ Line 226: odeploycons.KdumpEnv.DESTINATION_PORT Line 227: ] Line 228: ), Line 225: self.environment[ Line 226: odeploycons.KdumpEnv.DESTINATION_PORT Line 227: ] Line 228: ), Line 229: interval=str( conversion, if any, should be done in function Line 230: self.environment[ Line 231: odeploycons.KdumpEnv.MESSAGE_INTERVAL Line 232: ] Line 233: ), Line 248: @plugin.event( Line 249: stage=plugin.Stages.STAGE_CLOSEUP, Line 250: condition=lambda self: ( Line 251: self.environment[odeploycons.KdumpEnv.ENABLE] and Line 252: self.environment[odeploycons.KdumpEnv.SUPPORTED] enabled is enough Line 253: ), Line 254: ) Line 255: def _closeup(self): Line 256: self.logger.info(_('Restarting kdump')) Line 256: self.logger.info(_('Restarting kdump')) Line 257: self.services.startup('kdump', True) Line 258: for state in (False, True): Line 259: self.services.state('kdump', state) Line 260: one more space :) -- To view, visit http://gerrit.ovirt.org/28385 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idda48cb053c7e8747de5434c3681403f739c06b1 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-host-deploy Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
