Nir Soffer has posted comments on this change. Change subject: utils: move nice ionice sid and sudo out of execCmd ......................................................................
Patch Set 4: (5 comments) http://gerrit.ovirt.org/#/c/33691/4/lib/vdsm/cmdutils.py File lib/vdsm/cmdutils.py: Line 44: command.extend(cmd) Line 45: return command Line 46: Line 47: Line 48: def sudo(cmd): This can be simplified and more similar to other commands: if os.geteuid() == 0: return cmd command = [constants.EXT_SUDO, SUDO_NON_INTERACTIVE_FLAG] command.extend(cmd) return command Line 49: command = [] Line 50: if os.geteuid() != 0: Line 51: command.extend((constants.EXT_SUDO, SUDO_NON_INTERACTIVE_FLAG)) Line 52: command.extend(cmd) http://gerrit.ovirt.org/#/c/33691/4/lib/vdsm/utils.py File lib/vdsm/utils.py: Line 584 Line 585 Line 586 Line 587 Line 588 We can remove this now, as cmdutils.xxx _list2cmdline and Popen can work with both tuples and lists, and there is no reason to support iterators. Line 595 Line 596 Line 597 Line 598 Line 599 This was the reason we had to do convert command to a list. Line 598 Line 599 Line 600 Line 601 Line 602 Same, command doe not have to be a list now Line 602 Line 603 Line 604 Line 605 Line 606 Same, command does not have to be a list now -- To view, visit http://gerrit.ovirt.org/33691 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I442c64e8b9cfb3933dbe9879b1d442d0e96fbe40 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Dima Kuznetsov <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Yoav Kleinberger <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
