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