Dan Kenigsberg has posted comments on this change.
Change subject: hooks: Add OpenStack Network vNIC hook
......................................................................
Patch Set 6: I would prefer that you didn't submit this
(2 inline comments)
minor comment.
....................................................
File vdsm_hooks/openstacknet/after_device_create.py
Line 38:
Line 39:
Line 40: def disconnectVnic(portId):
Line 41: tapName = ('tap' + portId)[:DEV_MAX_LENGTH]
Line 42: command = ['/usr/sbin/brctl', 'delif', DUMMY_BRIDGE, tapName]
that's constants.EXT_BRCTL. With it, you could skip the /sbin/brctl step.
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)
Line 46: if retcode == 127:
....................................................
File vdsm_hooks/openstacknet/sudoers.vdsm_hook_openstacknet
Line 1: vdsm ALL=(ALL) NOPASSWD: /usr/sbin/brctl, /sbin/brctl
regarding this file's name: it is not your fault, since you're following the
lead of other hooks, but I do not see the point of having such a long file
name.
"vdsm_hooks/openstacknet/sudoers" carries all the needed information.
Anyway, this can wait for a future cleanup.
--
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: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Giuseppe Vallarelli <[email protected]>
Gerrit-Reviewer: Livnat Peer <[email protected]>
Gerrit-Reviewer: Mark Wu <[email protected]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
