Sergey Gotliv has posted comments on this change.
Change subject: Teardown only VDSM images when power-off VM
......................................................................
Patch Set 2:
(1 comment)
....................................................
File vdsm/vm.py
Line 2537: "for drive %s", drive,
exc_info=True)
Line 2538: # Skip any exception as we don't want to
interrupt the
Line 2539: # teardown process for any reason.
Line 2540: try:
Line 2541: if drive.isVdsmImage():
Federico, just because you insist to continue that argument after I already
agreed to change it exactly as you want, I have to ask you:
The author of that method designed it that way so it can't be called for
non-vdsm images, right? Do we support non-vdsm images at all?
So why checking if this is vdsm image before calling that method is wrong, how
caller suppose to use this method?
That "if" improves the situation for sure, up until now I explained to 3
different customers that this ugly and particularly long exception is not a
bug. So if this is improvement why you giving it "-1"?
And what you suggests, to make it symmetric to prepare, why?
What is the added value of that symmetry?
Line 2542: self.cif.teardownVolumePath(drive)
Line 2543: except:
Line 2544: self.log.error("Drive teardown failure for %s",
Line 2545: drive, exc_info=True)
--
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: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Sergey Gotliv <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
