Martin Sivák has posted comments on this change. Change subject: Encapsulate the storage metadata and add support for blockSD ......................................................................
Patch Set 8: (4 comments) http://gerrit.ovirt.org/#/c/25797/8/ovirt_hosted_engine_ha/lib/storage_backends.py File ovirt_hosted_engine_ha/lib/storage_backends.py: Line 7: Line 8: logger = logging.getLogger(__name__) Line 9: Line 10: Line 11: class StorageBackend(object): > Why not defining this as abstract with abc module? This is an abstract clas Done Line 12: """ Line 13: The base template for Storage backend classes. Line 14: """ Line 15: Line 50: raise NotImplementedError() Line 51: Line 52: Line 53: class BackendFailureException(Exception): Line 54: pass > Maybe a doc line here about when this is raised would help. Done Line 55: Line 56: Line 57: class FilesystemBackend(StorageBackend): Line 58: """ Line 107: Line 108: # strip the prefix and use the rest as symlink name Line 109: service = lv.split(constants.SD_METADATA_DIR + "-", 1)[-1] Line 110: service_link = os.path.join(self._storage_path, service) Line 111: try: > if os.path.exists(service_link): I do not like the race and OSError catches the case as well. Line 112: os.unlink(service_link) Line 113: logger.info("Cleaning up stale LV link %s", service_link) Line 114: except OSError: Line 115: pass Line 111: try: Line 112: os.unlink(service_link) Line 113: logger.info("Cleaning up stale LV link %s", service_link) Line 114: except OSError: Line 115: pass > if file exists and we have OSError it's worth to be logged why it failed. No logging here, this is library function and as such I want to raise the exception. But I updated it to ignore the ENOENT only. Line 116: os.symlink(os.path.join("/dev", uuid, lv), service_link) Line 117: Line 118: def disconnect(self): Line 119: pass -- To view, visit http://gerrit.ovirt.org/25797 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaced4ac08936fc9314ff821343ce33d29a5897cf Gerrit-PatchSet: 8 Gerrit-Project: ovirt-hosted-engine-ha Gerrit-Branch: master Gerrit-Owner: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Jiří Moskovčák <jmosk...@redhat.com> Gerrit-Reviewer: Martin Sivák <msi...@redhat.com> Gerrit-Reviewer: Sandro Bonazzola <sbona...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
