Deepak C Shetty has posted comments on this change.
Change subject: [WIP] Add new repository management code ......................................................................
Patch Set 16: (4 inline comments)
few Qs.
.................................................... File vdsm/storage/imageRepository/imageManipulator.py Line 132: code = 400 Line 133: message = "Base has wrong mutablility state for requested operation" Line 134: Line 135: Line 136: class WrokerLostError(ImageManipulatorError): s/WrokerLostError/WorkerLostError Line 137: """Means we lost communication with the worker. The end state of the Line 138: operation is unknown. The only way to know is to inspect the repo and see Line 139: what is there and what isn't""" Line 140: code = 410
Line 150: code = 400 Line 151: message = "Internal communication with the worker failed" Line 152: Line 153: Line 154: class UnhandeledExceptionError(ImageManipulatorError): s/UnhandeledExceptionError/UnhandledExceptionError Line 155: code = 500 Line 156: Line 157: def __init__(self, e): Line 158: self.message = str(e)
.................................................... File vdsm/storage/storageRepositoryManager.py Line 36: def createStorageRepository(self, repoFormat, creationParameters={}): Line 37: engine = self._getEngine(repoFormat) Line 38: Line 39: try: Line 40: engine.create(**creationParameters) Why use engine.create here ? IIUC ImageRepository is supposed to encapsulate the engine and its low level details for create\delete StorageRepository, we should go via ImageRepository itself.
In other words, why expose the engine to SRM ? Curently engine is used by ImageRepo as well SRM.. 2 places! Line 41: except Exception as e: Line 42: self.log.error("Storage repository creation failed", exc_info=True) Line 43: raise se.StorageDomainCreationError(str(e)) Line 44:
Line 45: def deleteStorageRepository(self, repoFormat, deletionParameters={}): Line 46: engine = self._getEngine(repoFormat) Line 47: Line 48: try: Line 49: engine.delete(**deletionParameters) The same comment as applicable to create above, applies here. Line 50: except Exception as e: Line 51: self.log.error("Storage repository deletion failed", exc_info=True) Line 52: raise se.StorageDomainFormatError(str(e)) Line 53:
-- To view, visit http://gerrit.ovirt.org/6247 To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment Gerrit-Change-Id: Ib09c89cf982b475f45d26b2428fe05e2f4565dab Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Adam Litke agl@us.ibm.com Gerrit-Reviewer: Ayal Baron abaron@redhat.com Gerrit-Reviewer: Dan Kenigsberg danken@redhat.com Gerrit-Reviewer: Deepak C Shetty deepakcs@linux.vnet.ibm.com Gerrit-Reviewer: Eduardo ewarszaw@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimonce@redhat.com Gerrit-Reviewer: Igor Lvovsky ilvovsky@redhat.com Gerrit-Reviewer: Royce Lv lvroyce@linux.vnet.ibm.com Gerrit-Reviewer: Saggi Mizrahi smizrahi@redhat.com Gerrit-Reviewer: Shu Ming shuming@linux.vnet.ibm.com Gerrit-Reviewer: oVirt Jenkins CI Server