Dan Kenigsberg has posted comments on this change. Change subject: Added gluster tool support in supervdsm. ......................................................................
Patch Set 2: I would prefer that you didn't submit this (3 inline comments) .................................................... File configure.ac Line 150: AC_PATH_PROG([GLUSTER_CLI_PATH], [gluster], [/usr/sbin/gluster]) please keep sorted .................................................... File vdsm/gluster_super.py Line 24: def execGluster(glusterArgList): I cannot say that I like this wide passthrough of command line. it defies the purpose of having this "python binding" file, and lets vdsm do anything, as root, via gluster cli. I'd rather see an encapsulation of gluster cli functionality, parsing of output, raising errors -- so that whomever uses this module can forget that it is an external tools. .................................................... File vdsm/Makefile.am Line 26: gluster_super.py \ sort? -- To view, visit http://gerrit.ovirt.org/2797 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2faa261a3c44cf84af14102bdf6479287435793b Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Bala.FA <[email protected]> Gerrit-Reviewer: Bala.FA <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Ewoud Kohl van Wijngaarden <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://fedorahosted.org/mailman/listinfo/vdsm-patches
