Pavel Zhukov has posted comments on this change.

Change subject: hooks: Add fcoe hook
......................................................................


Patch Set 13: Verified+1

(5 comments)

https://gerrit.ovirt.org/#/c/55029/3/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

Line 13: 
Line 14: from shutil import copyfile
Line 15: from vdsm import utils
Line 16: from vdsm.netconfpersistence import RunningConfig
Line 17: 
> we do not need this section, as we would backport this only to 3.6.
done
Line 18: 
Line 19: FCOE_CONFIG_DIR = '/etc/fcoe/'
Line 20: # TODO this is default for EL7, Fedora.
Line 21: # Debian, EL6 etc should be checked


Line 21: # Debian, EL6 etc should be checked
Line 22: FCOE_DEFAULT_CONFIG = 'cfg-ethx'
Line 23: 
Line 24: 
Line 25: def _configure(interface):
> file is a python keyword. please use another name. such as filename.
Switched to use RunningConfig().
Done
Line 26:     """
Line 27:     Enable FCoE on specified interface by coping default configuration
Line 28:     """
Line 29:     # IOError can be raised here is FCOE_DEFAULT_CONFIG doesn't exist


https://gerrit.ovirt.org/#/c/55029/12/vdsm_hooks/fcoe/fcoe_before_network_setup.py
File vdsm_hooks/fcoe/fcoe_before_network_setup.py:

PS12, Line 24: 
> please use triple doublequotes, as pep8 prefers.
Fixed


PS12, Line 81: 
             :     for (net, net_nic) in six.iteritems(removed_networks):
             :         if net in configured:
             :             _unconfigure(config.networks[net].get('nic'))
             : 
             :     for (net, net_nic) in six.iteritems(changed_non_fcoe):
             :  
> please use hooking.log if you must, and avoid trailing spaces.
Sorry. I forgot to remove this again. Not sure if we need this logged.


PS12, Line 88:     
> using
Fixed


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iad2faed7205ca08801132df072b469dbe781318c
Gerrit-PatchSet: 13
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Pavel Zhukov <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Edward Haas <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Pavel Zhukov <[email protected]>
Gerrit-Reviewer: Yaniv Kaul <[email protected]>
Gerrit-Reviewer: gerrit-hooks <[email protected]>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to