Dan Kenigsberg has posted comments on this change. Change subject: network: add ifcfg write hook ......................................................................
Patch Set 5: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/44552/5/vdsm/hooks.py File vdsm/hooks.py: Line 391: hookType=_JSON_HOOK) Line 392: Line 393: Line 394: def before_ifcfg_write(hook_dict): Line 395: return _runHooksDir(hook_dict, 'before_ifcfg_write', raiseError=False, We must use raiseError=True as a failure to generate the new config should spell error in setting up the network. Line 396: hookType=_JSON_HOOK) Line 397: Line 398: Line 399: def after_ifcfg_write(hook_dict): Line 395: return _runHooksDir(hook_dict, 'before_ifcfg_write', raiseError=False, Line 396: hookType=_JSON_HOOK) Line 397: Line 398: Line 399: def after_ifcfg_write(hook_dict): Please drop this hookpoint - unless you provide a use case for it. Line 400: return _runHooksDir(hook_dict, 'after_ifcfg_write', raiseError=False, Line 401: hookType=_JSON_HOOK) Line 402: Line 403: https://gerrit.ovirt.org/#/c/44552/5/vdsm/network/configurators/ifcfg.py File vdsm/network/configurators/ifcfg.py: Line 549: hook_dict = _build_ifcfg_write_hook_dict(name, Line 550: netinfo.NET_CONF_PREF + name, Line 551: cfg) Line 552: hook_return = hooks.before_ifcfg_write(hook_dict) Line 553: if 'ifcfg_file' in hook_return.keys(): > To what extent should the user input be verified? I don't think we should do any. If 'config' is dropped by the hook writer we should explode. What you currently seems too much imo: if the hook script wants to keep former config and ifcfg-name, it should simply not touch them. Line 554: ifcfg_file = hook_return['ifcfg_file'] Line 555: if 'config' in hook_return.keys(): Line 556: cfg = hook_return['config'] Line 557: self.writeConfFile(ifcfg_file, cfg) -- To view, visit https://gerrit.ovirt.org/44552 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2206ccf97b210bd58f03777d4ccd016785b02939 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Marcin Mirecki <[email protected]> Gerrit-Reviewer: Alona Kaplan <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Marcin Mirecki <[email protected]> Gerrit-Reviewer: Petr Horáček <[email protected]> Gerrit-Reviewer: Yevgeny Zaspitsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
