Ayal Baron has posted comments on this change.
Change subject: Relink template hard links to meta and lease files
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
....................................................
Commit Message
Line 3: AuthorDate: 2013-03-05 11:36:36 +0200
Line 4: Commit: Yeela Kaplan <ykaplan(a)redhat.com>
Line 5: CommitDate: 2013-03-07 17:15:38 +0200
Line 6:
Line 7: Relink template hard links to meta and lease files
why?
Line 8:
Line 9: Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Line 10: Bug-Url:
https://bugzilla.redhat.com/show_bug.cgi?id=864073
....................................................
File vdsm/storage/fileSD.py
Line 526: This function assumes that dom is backup dom and that template image is
Line 527: used by other volumes.
Line 528: """
Line 529: # Avoid relink templates for non-NFS domains
Line 530: if self.getStorageType() not in [sd.NFS_DOMAIN]:
why only NFS and not all file domains? (hard links are supported by all posix compliant
file systems and then some)
Line 531: self.log.debug("Doesn't relink templates non-NFS domain
%s",
Line 532: self.sdUUID)
Line 533: return
Line 534:
Line 545: # This function assumes that all relevant images and template
Line 546: # namespaces are locked.
Line 547: repoPath = self._getRepoPath()
Line 548: tLink = os.path.join(repoPath, self.sdUUID,
Line 549: sd.DOMAIN_IMAGES, rImg, volUUID)
How about:
tVolPath = os.path.join(repoPath, selfSdUUID, sd.DOMAIN_IMAGES, "%s", volUUID)
for ext...
linkName = tVolPath % rImg + ext # or if you prefer: (tvolPath + ext) % rimg
tVolName = tVolPath % templateImage + ext
self.oop.os.unlink(linkName)
self.oop.os.link(tVolName, linkName)
Line 550: for ext in ['', fileVolume.META_FILEEXT]:
Line 551: self.oop.os.unlink(tLink + ext)
Line 552: self.oop.os.link(os.path.join(repoPath, self.sdUUID,
Line 553: sd.DOMAIN_IMAGES, templateImage,
Line 558: self.oop.os.link(os.path.join(
Line 559: repoPath, self.sdUUID, sd.DOMAIN_IMAGES,
Line 560: templateImage, volUUID + fileVolume.LEASE_FILEEXT),
Line 561: tLink + fileVolume.LEASE_FILEEXT)
Line 562: except OSError as e:
I see no reason not to ignore the 'fake' volume / metafile not existing just the
same and unify this into the loop above
i.e.:
for ext in ['', fileVolume.META_FILEEXT, fileVolume.LEASE_FILEEXT]:
try:
...
except OSError as e:
...
Line 563: if e.errno != os.errno.ENOENT:
Line 564: raise
Line 565:
Line 566:
....................................................
File vdsm/storage/sd.py
Line 793: This function assumes that dom is backup dom and that template image is
Line 794: used by other volumes.
Line 795: """
Line 796: # Avoid relink templates for non-NFS domains
Line 797: if self.getStorageType() not in [NFS_DOMAIN]:
since fileSD inherits from sd this 'if' will always be true and if it isn't
then we most certainly want to log that
Line 798: self.log.debug("Doesn't relink templates non-NFS domain
%s",
Line 799: self.sdUUID)
--
To view, visit
http://gerrit.ovirt.org/12837
To unsubscribe, visit
http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idce0f3f1812fdf45efeeeffcccb7dc22b3b0d0f0
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yeela Kaplan <ykaplan(a)redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw(a)redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce(a)redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server