Change in vdsm[master]: createImageLinks: recreate if the link exists but it's broken

stirabos at redhat.com stirabos at redhat.com
Fri Feb 12 15:37:18 UTC 2016


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 <stirabos at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sandro Bonazzola <sbonazzo at redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list