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

mkolesni at redhat.com mkolesni at redhat.com
Sun Jun 23 08:49:43 UTC 2013


Mike Kolesnik has posted comments on this change.

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


Patch Set 5: (6 inline comments)

....................................................
File configure.ac
Line 227: 	vdsm_hooks/pincpu/Makefile
Line 228: 	vdsm_hooks/promisc/Makefile
Line 229: 	vdsm_hooks/qemucmdline/Makefile
Line 230: 	vdsm_hooks/qos/Makefile
Line 231: 	vdsm_hooks/openstacknet/Makefile
Done
Line 232: 	vdsm_hooks/scratchpad/Makefile
Line 233: 	vdsm_hooks/smbios/Makefile
Line 234: 	vdsm_hooks/sriov/Makefile
Line 235: 	vdsm_hooks/vhostmd/Makefile


....................................................
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
I guess this should be changed in hooking.py, but as you said it's not part of this patch.

I will send a different patch for this.
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):
>From what I see, in hooks stderr is always used both for regular "log" messages, and for error messages..
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'm not sure, since we need sudo to the command so it would be one of the possible two commands in the sudoers file.

I could also just as well use '/usr/bin/env brctl' for this
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"
Yes but in general you'd like to exit the hook so I'm not sure why you think it's a bad name.

If someone were to use it to exit gracefully, I wouldn't want to prevent it with a weird name, especially since you can use return_code = 0 and then it isn't really an error..
Line 51:                           % (tapName, DUMMY_BRIDGE, err))
Line 52: 
Line 53: 
Line 54: def main():


....................................................
File vdsm_hooks/openstacknet/before_device_create.py
Line 18: 
Line 19: import os
Line 20: import sys
Line 21: import traceback
Line 22: import argparse
Done
Line 23: from xml.dom import minidom
Line 24: 
Line 25: import hooking
Line 26: from vdsm.netinfo import DUMMY_BRIDGE


-- 
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