Change in vdsm[master]: Related to 846307 - Simplifying clientIF._cleanOldFiles().
smizrahi at redhat.com
smizrahi at redhat.com
Thu Jan 3 21:42:37 UTC 2013
Saggi Mizrahi has posted comments on this change.
Change subject: Related to 846307 - Simplifying clientIF._cleanOldFiles().
......................................................................
Patch Set 10: I would prefer that you didn't submit this
(1 inline comment)
....................................................
File vdsm/clientIF.py
Line 466: orphanVMFile = lambda vmFileName: vmFileName.split('.', 1)[0] \
Line 467: not in self.vmContainer
Line 468: dontDel = ("vdsmd.pid", "respawn.pid", "svdsm.pid")
Line 469:
Line 470: fNames = tuple(fName for fName in os.listdir(constants.P_VDSM_RUN)
This is much simpler and faster
# Get all potential files
toDel = set(os.listdir(constants.P_VDSM_RUN))
# Don't include files that are in the whitelist
toDel.difference_update(("vdsmd.pid", "respawn.pid", "svdsm.pid"))
# Don't include files related to running VMs
for vm in self.vmContainer:
for suffix in ['.guest.socket', 'pid', 'recovery']:
toDel.discard(vm + suffix)
# Delete all remaining files.
for fName in toDel:
....
This is better because:
- no lambdas
- It's clear that when you add a file not related to VMs it should be added to
the whitelist
- It's clear that when adding a new file related to VMs it should be named
<VM>.<suffix> and it's clear how to add the suffix
- Because all running VMs have files on the directory just running over that list is
simpler and faster.
Line 471: if fName not in dontDel)
Line 472: toDel = [f for f in fnmatch.filter(fNames, "*.guest.socket")
Line 473: if orphanVMFile(f)]
Line 474: toDel.extend(f for f in fnmatch.filter(fNames, "*.pid")
--
To view, visit http://gerrit.ovirt.org/8626
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifd63a1f4b1c64f84d5734da033a2420b97ce0ada
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Adam Litke <agl at us.ibm.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
More information about the vdsm-patches
mailing list