Federico Simoncelli has posted comments on this change.
Change subject: Fix parent volume rollback after failure to create snapshot
......................................................................
Patch Set 6:
(1 comment)
....................................................
File vdsm/storage/volume.py
Line 262: parent = self.getVolumePath()
Line 263: parent_format = fmt2str(self.getFormat())
Line 264: # We should use parent's relative path instead of full
path
Line 265: parent =
os.path.join(os.path.basename(os.path.dirname(parent)),
Line 266: os.path.basename(parent))
Anything could raise an exception there (KeyError in fmt2str, etc.). I'm not a
fan of exceptions translation/masking. The CannotCloneVolume exception is not
helping the engine: what difference it makes at what step we failed? We weren't
able to create a volume (which required cloning, but that's not interesting).
Even more, if we translate it to CannotCloneVolume it is even misleading as the
real error would be a metadata corruption (as the size is not a base-10 integer
or the format is unknown).
For as much as I know the exception translation could be removed:
def clone(...):
(prepare the needed variables)
self.prepare(rw=False)
try:
createVolume(...)
finally:
self.teardown(...)
if self.isLeaf():
...
Line 267: createVolume(parent, parent_format, dst_path,
Line 268: size, volFormat, preallocate)
Line 269: except Exception as e:
Line 270: self.log.error("Volume.clone: can't clone: %s to %s" %
--
To view, visit http://gerrit.ovirt.org/18205
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I11948aacad21008ffbb4cb23013bf363e60bdb9b
Gerrit-PatchSet: 6
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Ayal Baron <aba...@redhat.com>
Gerrit-Reviewer: Eduardo <ewars...@redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com>
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgot...@redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykap...@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
