Yaniv Bronhaim has posted comments on this change.
Change subject: make misc.pgrep for general usage ......................................................................
Patch Set 3: I would prefer that you didn't submit this
(3 inline comments)
.................................................... File tests/utilsTests.py Line 52: sproc = misc.execCmd(args, sync=False, sudo=False) Line 53: stats = utils.pidStat(sproc.pid) Line 54: pid = int(stats.pid) Line 55: # procName comes in the format of (procname) Line 56: name = stats.comm you should add tests for more fields that we use to be sure pidStat works as we want, and i would add the change of pidStat to the patch comment. Line 57: self.assertEquals(pid, sproc.pid) Line 58: self.assertEquals(name, args[0]) Line 59: sproc.kill()
.................................................... File vdsm/storage/misc.py Line 1108: pid = os.path.basename(path) Line 1109: yield int(pid) Line 1110: Line 1111: Line 1112: def pgrep(value, field): As I understand pgrep just returns pid of processes with specific name.. you can add more restrictions as owners but you don't use pgrep to match other fields except the name of process..
You can add another misc\util function for this purpose Line 1113: """ Line 1114: grep processes whose given field's value matches specified value Line 1115: see man 5 proc '/proc/[pid]/stat' for fields description Line 1116: """
Line 1287: def killall(name, signum, group=False): Line 1288: exception = None Line 1289: knownPgs = set() Line 1290: pidList = pgrep(name) Line 1291: if len(pidList) == 0: what about this call? Line 1292: raise OSError(errno.ESRCH, Line 1293: "Could not find processes named `%s`" % name) Line 1294: Line 1295: for pid in pidList:
-- To view, visit http://gerrit.ovirt.org/11033 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: I2034b04a4787d2f222cb98dfe3ffcdbb2a94ebb8 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Mark Wu wudxw@linux.vnet.ibm.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: Yaniv Bronhaim ybronhei@redhat.com Gerrit-Reviewer: oVirt Jenkins CI Server