Simone Tiraboschi has posted comments on this change. Change subject: createImageLinks: recreate if the link exists but it's broken ......................................................................
Patch Set 3: (5 comments) https://gerrit.ovirt.org/#/c/52937/3/vdsm/storage/fileSD.py File vdsm/storage/fileSD.py: Line 520: try: Line 521: os.symlink(srcImgPath, imgRunDir) Line 522: except OSError as e: Line 523: if e.errno == errno.EEXIST: Line 524: prevPath = os.path.realpath(imgRunDir) > This should check link target, see _linkStorageDomain. Done Line 525: if prevPath == srcImgPath: Line 526: self.log.debug( Line 527: "img run dir already exists '%s' and points to the " Line 528: "right target, skipping it" % imgRunDir Line 525: if prevPath == srcImgPath: Line 526: self.log.debug( Line 527: "img run dir already exists '%s' and points to the " Line 528: "right target, skipping it" % imgRunDir Line 529: ) > - The original log is fine, we should use here something like: Done Line 530: else: Line 531: self.log.debug( Line 532: "img run dir already exists '%s' but points to a " Line 533: "different target %s, recreating it" % ( Line 532: "img run dir already exists '%s' but points to a " Line 533: "different target %s, recreating it" % ( Line 534: imgRunDir, Line 535: prevPath, Line 536: ) > Use vdsm style logs, and lets shorten this to: Done Line 537: ) Line 538: try: Line 539: os.remove(imgRunDir) Line 540: os.symlink(srcImgPath, imgRunDir) Line 536: ) Line 537: ) Line 538: try: Line 539: os.remove(imgRunDir) Line 540: os.symlink(srcImgPath, imgRunDir) > The best way to do this is what we do in _linkStorageDomain: Done Line 541: except OSError: Line 542: self.log.error( Line 543: "Failed to re-create img run dir: %s", Line 544: imgRunDir Line 541: except OSError: Line 542: self.log.error( Line 543: "Failed to re-create img run dir: %s", Line 544: imgRunDir Line 545: ) > Do not log and raise. If you cannot handle this, let it fail. It's the same patter that's in use two lines below. Line 546: raise Line 547: else: Line 548: self.log.error("Failed to create img run dir: %s", imgRunDir) Line 549: raise -- To view, visit https://gerrit.ovirt.org/52937 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia0eb50f57cf6f21496f7a8ef70f80693336ae803 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Adam Litke <ali...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches