Liron Aravot has uploaded a new change for review. Change subject: core: errors during preview of diskless snapshots ......................................................................
core: errors during preview of diskless snapshots 1. When creating a diskless snapshot, then adding a disk to the vm and later on try to preview that snapshots an NPE occurs within ovf writer. The exception occurs because of outdated disk information contained within the vm - the problem wasn't fixed locally but globally to prevent it from occuring in other flows as well. VmHandler.updateDisksFromDb() should clear the current disks of the vm otherwise we might get corrupted disk information for the vm (by adding the disks information from the DB while not clearing the present information). 2. When trying to run diskless snapshots exception can occur for various reason in VmHandler.updateVmInSpm(). When such exception occurs the operation should be rolled back (in the bug scenario - it wasn't as there were db updates that were executed with no transaction which led to partially rollback). Change-Id: I6e8edc74bc34676f526dfd24d2f89eb60d8acf2e Bug-Url: https://bugzilla.redhat.com/873595 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java 2 files changed, 15 insertions(+), 13 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/28/9128/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java index 19ce1e0..a1bdec2 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java @@ -120,9 +120,13 @@ protected void executeVmCommand() { final Guid newActiveSnapshotId = Guid.NewGuid(); - Guid previousActiveSnapshotId = TransactionSupport.executeInNewTransaction(new TransactionMethod<Guid>() { + final List<DiskImage> images = DbFacade + .getInstance() + .getDiskImageDao() + .getAllSnapshotsForVmSnapshot(getParameters().getDstSnapshotId()); + TransactionSupport.executeInNewTransaction(new TransactionMethod<Void>() { @Override - public Guid runInTransaction() { + public Void runInTransaction() { Snapshot previousActiveSnapshot = getSnapshotDao().get(getVmId(), SnapshotType.ACTIVE); getCompensationContext().snapshotEntity(previousActiveSnapshot); Guid previousActiveSnapshotId = previousActiveSnapshot.getId(); @@ -134,15 +138,16 @@ getCompensationContext()); snapshotsManager.addActiveSnapshot(newActiveSnapshotId, getVm(), getCompensationContext()); snapshotsManager.removeAllIllegalDisks(previousActiveSnapshotId, getVm().getId()); - getCompensationContext().stateChanged(); - return previousActiveSnapshotId; + // if there are no images we can should restore the config now so it'll be executed within the transaction. + if (!images.isEmpty()) { + getCompensationContext().stateChanged(); + } else { + restoreVmConfigFromSnapshot(); + } + return null; } }); - final List<DiskImage> images = DbFacade - .getInstance() - .getDiskImageDao() - .getAllSnapshotsForVmSnapshot(getParameters().getDstSnapshotId()); if (images.size() > 0) { VmHandler.LockVm(getVm().getDynamicData(), getCompensationContext()); freeLock(); @@ -162,7 +167,6 @@ p, ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); getParameters().getImagesParameters().add(p); - if (vdcReturnValue.getSucceeded()) { getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList()); } else if (vdcReturnValue.getFault() != null) { @@ -177,11 +181,7 @@ return null; } }); - } else { - restoreVmConfigFromSnapshot(); - freeLock(); } - setSucceeded(true); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java index e847747..f633e0c 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java @@ -251,6 +251,8 @@ public static void updateDisksFromDb(VM vm) { List<Disk> imageList = DbFacade.getInstance().getDiskDao().getAllForVm(vm.getId()); + vm.getDiskList().clear(); + vm.getDiskMap().clear(); updateDisksForVm(vm, imageList); } -- To view, visit http://gerrit.ovirt.org/9128 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I6e8edc74bc34676f526dfd24d2f89eb60d8acf2e Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liron Aravot <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
