Federico Simoncelli has posted comments on this change. Change subject: backend: prevent live storage migration on live snapshot failure ......................................................................
Patch Set 2: (3 comments) .................................................... Commit Message Line 3: AuthorDate: 2013-10-29 17:14:54 +0100 Line 4: Commit: Federico Simoncelli <[email protected]> Line 5: CommitDate: 2013-10-30 12:56:00 +0100 Line 6: Line 7: backend: prevent live migrate on live snapshot failure Done Line 8: Line 9: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1018876 Line 10: Change-Id: I7c69f663836e74691c968f4a15c5f3012479a8b0 .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java Line 310: TransactionSupport.executeInScope(TransactionScopeOption.Suppress, new TransactionMethod<Void>() { Line 311: @Override Line 312: public Void runInTransaction() { Line 313: runVdsCommand(VDSCommandType.Snapshot, buildLiveSnapshotParameters(snapshot)); Line 314: getParameters().setLiveSnapshotSucceeded(true); Once the volumes are created (vdsm tasks) the live snapshot cannot fail anymore (there's no way to reliable revert the createVolume tasks). So our only correct behavior at the moment is to always succeed also when VDSCommandType.Snapshot fails. Changing the result value to false here (which is basically part of endSuccessfully) is both inconsistent (the command just ended successfully) and useless (luckily otherwise we would try to unreliably revertTask). In general it's really hackish to change the result value in endSuccessfully (if it even works by accident). Line 315: return null; Line 316: } Line 317: }); Line 318: } catch (VdcBLLException e) { .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/lsm/LiveMigrateDisksTaskHandler.java Line 25: } Line 26: Line 27: @Override Line 28: public void execute() { Line 29: if (!enclosingCommand.getParameters().getLiveSnapshotSucceeded()) { SEAT already handles tasks failures compensating the previous (successful) ones. What is not handling is the result being changed in endSuccessfully (which is something unexpected and unsupported). Line 30: throw new VdcBLLException(VdcBllErrors.imageErr, Line 31: "Auto-generated live snapshot for VM " + enclosingCommand.getParameters().getVmId() + " failed"); Line 32: } Line 33: -- To view, visit http://gerrit.ovirt.org/20668 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7c69f663836e74691c968f4a15c5f3012479a8b0 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
