Nir Soffer has posted comments on this change.

Change subject: clientIF: Teardown volume path only for VDSM images
......................................................................


Patch Set 5:

(2 comments)

....................................................
File vdsm/clientIF.py
Line 326:     def teardownVolumePath(self, drive):
Line 327:         res = {'status': doneCode}
Line 328: 
Line 329:         if vm.isVdsmImage(drive):
Line 330:             res = self.irs.teardownImage(drive['domainID'],
The previous error handler was wrong, checking KeyError and TypeError, but 
assuming that the error is caused by accessing keys when keys are missing, or 
when object does not have a __getitem__ method. Since now we cannot have such 
failures, we can safely remove these checks without changing the behavior of 
this code.

Adding additional error handling is out of the scope of this patch.
Line 331:                                          drive['poolID'], 
drive['imageID'])
Line 332:         return res['status']['code']
Line 333: 
Line 334:     def getDiskAlignment(self, drive):


....................................................
File vdsm/vm.py
Line 81: 
Line 82: 
Line 83: def isVdsmImage(drive):
Line 84:     if hasattr(drive, "isVdsmImage"):
Line 85:         return drive.isVdsmImage()
+1 for moving all checks to vm.isVdsmImage. Having 2 implementations of the 
same thing does not help anybody.

But lets do this in the next patch.
Line 86:     elif type(drive) is dict:
Line 87:         return all(k in drive.keys() for k in ('volumeID', 'domainID',
Line 88:                                                'imageID', 'poolID'))
Line 89:     else:


-- 
To view, visit http://gerrit.ovirt.org/21973
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I041a306636c75a7aa37d4d7c0811366d80fe609c
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Francesco Romani <from...@redhat.com>
Gerrit-Reviewer: Michal Skrivanek <michal.skriva...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to