Change in vdsm[master]: hooks: Add OpenStack Network vNIC hook

gvallare at redhat.com gvallare at redhat.com
Sun Jun 23 09:23:34 UTC 2013


Giuseppe Vallarelli has posted comments on this change.

Change subject: hooks: Add OpenStack Network vNIC hook
......................................................................


Patch Set 5: (4 inline comments)

....................................................
File vdsm/hooking.py
Line 39: import os
Line 40: import sys
Line 41: from xml.dom import minidom
Line 42: 
Line 43: from storage.misc import execCmd
Thanks!
Line 44: from vdsm.utils import tobool
Line 45: 
Line 46: # make pyflakes happy
Line 47: execCmd


Line 55: def write_domxml(domxml):
Line 56:     file(os.environ['_hook_domxml'], 'w').write(domxml.toxml(encoding='utf-8'))
Line 57: 
Line 58: 
Line 59: def exit_hook(message, return_code=2):
I see so it's a convention, thanks for the explanation.
Line 60:     """
Line 61:     Exit the hook with a given message, which will be printed to the standard
Line 62:     error stream. A newline will be printed at the end.
Line 63:     The default return code is 2 for signaling that an error occurred.


....................................................
File vdsm_hooks/openstacknet/after_device_create.py
Line 41:     tapName = ('tap' + portId)[:DEV_MAX_LENGTH]
Line 42:     command = ['/usr/sbin/brctl', 'delif', DUMMY_BRIDGE, tapName]
Line 43:     retcode, out, err = hooking.execCmd(command, sudo=True, raw=True)
Line 44: 
Line 45:     # On some systems, brctl is not at the above location (i.e. Ubuntu)
I see my comment has been triggered by your comment # On some systems... Actually I do not know why in many places we use hardcoded paths or we created an object CommandPath which stores path to binaries for both EL6 and Fedora. But nevermind for me is fine.
Line 46:     if retcode == 127:
Line 47:         command[0] = '/sbin/brctl'
Line 48:         retcode, out, err = hooking.execCmd(command, sudo=True, raw=True)
Line 49:     if retcode != 0:


Line 46:     if retcode == 127:
Line 47:         command[0] = '/sbin/brctl'
Line 48:         retcode, out, err = hooking.execCmd(command, sudo=True, raw=True)
Line 49:     if retcode != 0:
Line 50:         hooking.exit_hook("Can't disconnect %s from %s, due to: %s"
I've been tricked by the use of stderr, then you explained to me that is a common practice. Name is fine, thanks for the explanation.
Line 51:                           % (tapName, DUMMY_BRIDGE, err))
Line 52: 
Line 53: 
Line 54: def main():


-- 
To view, visit http://gerrit.ovirt.org/11108
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I744b83d9c6027bd817e5d4171a77a005611b9818
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <mkolesni at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Giuseppe Vallarelli <gvallare at redhat.com>
Gerrit-Reviewer: Livnat Peer <lpeer at redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw at linux.vnet.ibm.com>
Gerrit-Reviewer: Mike Kolesnik <mkolesni at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list