Martin Peřina 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, AFAIK, ovirt-node has 2 versions, one is built from rhel6 and one from fedora (I don't know for which version). So at least for rhel6 it should be supported. RHEL 7 (7.0.z or 7.1 at worst) support is currently discussed, but it should be included in RHEVM 3.5 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 LooseVersion is used in the same way in vdsm/packages.py plugin. I really don't understand why on plugin can use it and other cannot :-( 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? Currently all backup lines are inserted between '#ovirt-host-deploy:begin' and '#ovirt-host-deploy:end' lines. If I wouldn't detect those begin/end delimiters, then all changes will generate new backup block with those delimiters 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: Done 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. Done 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 :) Done 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. Done 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. Done 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... Done 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 Done 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 Done 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 Done 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 Done 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 Done 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 :) Done -- 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
