Change in vdsm[ovirt-3.6]: network: add ifcfg write hook

mmirecki at redhat.com mmirecki at redhat.com
Mon Sep 21 08:33:48 UTC 2015


Marcin Mirecki has posted comments on this change.

Change subject: network: add ifcfg write hook
......................................................................


Patch Set 1:

(3 comments)

https://gerrit.ovirt.org/#/c/46372/1/vdsm/network/configurators/ifcfg.py
File vdsm/network/configurators/ifcfg.py:

Line 546:                 logging.debug('ignoring variable %s', k)
Line 547: 
Line 548:         ifcfg_file = netinfo.NET_CONF_PREF + name
Line 549:         hook_dict = _build_ifcfg_write_hook_dict(name,
Line 550:                                                  netinfo.NET_CONF_PREF + name,
> why don't you pass just ifcfg_file here?
This is before we write the file, so there is no file yet. Instead we pass the name of the file and its contents. If the user modifies that, the returned values are used to write the file.
Line 551:                                                  cfg)
Line 552:         hook_return = hooks.before_ifcfg_write(hook_dict)
Line 553:         ifcfg_file = hook_return['ifcfg_file']
Line 554:         cfg = hook_return['config']


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:         ifcfg_file = hook_return['ifcfg_file']
> I think we should ignore the returned ifcfg_file path. We can safely assume
The asumption is that if the user uses this at his own risk. If one uses the hook, one has to take care of all the possible consequences.
Line 554:         cfg = hook_return['config']
Line 555:         self.writeConfFile(ifcfg_file, cfg)
Line 556: 
Line 557:     def addBridge(self, bridge, **opts):


Line 552:         hook_return = hooks.before_ifcfg_write(hook_dict)
Line 553:         ifcfg_file = hook_return['ifcfg_file']
Line 554:         cfg = hook_return['config']
Line 555:         self.writeConfFile(ifcfg_file, cfg)
Line 556: 
> shouldn't there be a call to hooks.after_ifcfg_write here?
Right!
We discussed if it should, or should not be for so long that I actually missed this.
Line 557:     def addBridge(self, bridge, **opts):
Line 558:         """ Create ifcfg-* file with proper fields for bridge """
Line 559:         conf = 'TYPE=Bridge\nDELAY=0\n'
Line 560:         opts['hotplug'] = 'no'  # So that udev doesn't trigger an ifup


-- 
To view, visit https://gerrit.ovirt.org/46372
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2206ccf97b210bd58f03777d4ccd016785b02939
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.6
Gerrit-Owner: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Marcin Mirecki <mmirecki at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list