Federico Simoncelli 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 325:
Line 326: def teardownVolumePath(self, drive):
Line 327: res = {'status': doneCode}
Line 328:
Line 329: if vm.isVdsmImage(drive):
If I understand correctly engine wants to start using PDIV in place of full
path for cdroms as well. But I agree with Ayal, we should remove the check from
prepareVolumePath. I just want the checks to be symmetrical.
Line 330: res = self.irs.teardownImage(drive['domainID'],
Line 331: drive['poolID'],
drive['imageID'])
Line 332: return res['status']['code']
Line 333:
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'],
This patch is about correctly calling teardownImage on prepared images (to
avoid logging confusion). So now if the patch is calling teardownImages in
cases that are potentially different from perpareImage... it's not addressing
the issue by definition.
That said, prepareVolumePath and teardownVolumePath are entry/exit points for
any VM image (disks, floppies, cdroms). I like to have them as multi-purpose
hooks for whatever we have (and will have) to do for any image we are about to
start/stop using (e.g. for example who's implementing hotplugDisk just needs to
call prepareVolumePath and teardownVolumePath without bothering about vdsm
images, direct luns etc.).
Now, the fact that teardownVolumePath is (currently) not doing anything for
floppies and cdroms is beyond the point (in fact it is exactly like grouping
and optimizing out the symmetrical checks that in teardown are noop). For the
existing handled teardownVolumePath images I don't see why we should have
different checks in prepare/teardown.
(Question, would you trust a function that is malloc'ing under a certain
condition and it's freeing under a different condition?)
Line 331: drive['poolID'],
drive['imageID'])
Line 332: return res['status']['code']
Line 333:
Line 334: def getDiskAlignment(self, drive):
--
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
