Liron Aravot has uploaded a new change for review. Change subject: core: live snapshot failes in case of a deactivated disk (#845020) ......................................................................
core: live snapshot failes in case of a deactivated disk (#845020) https://bugzilla.redhat.com/845020 when creating live snapshot for a VM, a SnapshotVdsCommand was executed for all disks, plugged and unplugged. in case of an unplugged disks, the command had failed as the disk didn't exist in the storage. this fix filters out the unplugged disks from the list of disks sent to the SnapshotVdsCommand. Change-Id: I27c32525794b4ec3c9d8c78f033a3c9d39da72b9 Signed-off-by: Liron Aravot <[email protected]> --- M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java M backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java 2 files changed, 32 insertions(+), 16 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/30/7330/1 diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java index 8ff9c11..17bb147 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java @@ -18,6 +18,7 @@ import org.ovirt.engine.core.common.action.ImagesActionsParametersBase; import org.ovirt.engine.core.common.action.VdcActionType; import org.ovirt.engine.core.common.action.VdcReturnValueBase; +import org.ovirt.engine.core.common.businessentities.Disk; import org.ovirt.engine.core.common.businessentities.DiskImage; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotStatus; import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType; @@ -39,7 +40,6 @@ import org.ovirt.engine.core.utils.transaction.TransactionMethod; import org.ovirt.engine.core.utils.transaction.TransactionSupport; - @LockIdNameAttribute public class CreateAllSnapshotsFromVmCommand<T extends CreateAllSnapshotsFromVmParameters> extends VmCommand<T> implements Quotable { @@ -59,13 +59,15 @@ /** * Filter all allowed snapshot disks. + * * @return list of disks to be snapshot. */ private List<DiskImage> getDisksList() { if (selectedActiveDisks == null) { - selectedActiveDisks = ImagesHandler.filterImageDisks(DbFacade.getInstance().getDiskDao().getAllForVm(getVmId()), - false, - true); + selectedActiveDisks = + ImagesHandler.filterImageDisks(DbFacade.getInstance().getDiskDao().getAllForVm(getVmId()), + false, + true); } return selectedActiveDisks; } @@ -98,15 +100,16 @@ tempVar.setVmSnapshotId(newActiveSnapshotId); tempVar.setEntityId(getParameters().getEntityId()); tempVar.setParentCommand(getParameters().getParentCommand() != VdcActionType.Unknown ? getParameters() - .getParentCommand() : VdcActionType.CreateAllSnapshotsFromVm); + .getParentCommand() + : VdcActionType.CreateAllSnapshotsFromVm); ImagesActionsParametersBase p = tempVar; // ParametersCurrentUser = CurrentUser, getParameters().getImagesParameters().add(p); p.setParentParemeters(getParameters()); VdcReturnValueBase vdcReturnValue = Backend.getInstance().runInternalAction( - VdcActionType.CreateSnapshot, - p, - ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); + VdcActionType.CreateSnapshot, + p, + ExecutionHandler.createDefaultContexForTasks(getExecutionContext())); if (vdcReturnValue.getSucceeded()) { getTaskIdList().addAll(vdcReturnValue.getInternalTaskIdList()); @@ -164,12 +167,13 @@ @Override public Void runInTransaction() { + List<Disk> pluggedDisks = new VmRunHandler().getPluggedDisks(getVm(), DbFacade.getInstance() + .getDiskDao() + .getAllForVm(getVm().getId())); runVdsCommand(VDSCommandType.Snapshot, new SnapshotVDSCommandParameters(getVm().getrun_on_vds().getValue(), getVm().getId(), - ImagesHandler.filterImageDisks(DbFacade.getInstance() - .getDiskDao() - .getAllForVm(getVm().getId()), false, true))); + ImagesHandler.filterImageDisks(pluggedDisks, false, true))); return null; } }); @@ -186,8 +190,8 @@ } /** - * Return the given snapshot ID's snapshot to be the active snapshot. The snapshot with the given ID is removed - * in the process. + * Return the given snapshot ID's snapshot to be the active snapshot. The snapshot with the given ID is removed in + * the process. * * @param createdSnapshotId * The snapshot ID to return to being active. @@ -267,7 +271,7 @@ private ValidationResult vmNotRunningStateless() { if (getSnapshotDao().exists(getVm().getId(), SnapshotType.STATELESS)) { VdcBllMessages message = getVm().isStatusUp() ? VdcBllMessages.ACTION_TYPE_FAILED_VM_RUNNING_STATELESS : - VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_STATELESS_SNAPSHOT_LEFTOVER; + VdcBllMessages.ACTION_TYPE_FAILED_VM_HAS_STATELESS_SNAPSHOT_LEFTOVER; return new ValidationResult(message); } @@ -308,7 +312,7 @@ for (DiskImage disk : getDisksList()) { list.add(new StorageQuotaValidationParameter(disk.getQuotaId() != null ? disk.getQuotaId() : getVm().getQuotaId(), - //TODO: shared disk? + // TODO: shared disk? disk.getstorage_ids().get(0), disk.getSizeInGigabytes())); } diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java index 92c5ee9..7803622 100644 --- a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java +++ b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmRunHandler.java @@ -193,7 +193,19 @@ * @return */ protected List<Disk> getPluggedDisks(VM vm) { - List<Disk> diskImages = getDiskDAO().getAllForVm(vm.getId()); + return getPluggedDisks(vm, null); + } + + /** + * The following method should return only plugged images which are attached to VM, + * only those images are relevant for the RunVmCommand + * @param vm + * @return + */ + protected List<Disk> getPluggedDisks(VM vm, List<? extends Disk> diskImages) { + if (diskImages == null) { + diskImages = getDiskDAO().getAllForVm(vm.getId()); + } List<VmDevice> diskVmDevices = getVmDeviceDAO().getVmDeviceByVmIdTypeAndDevice(vm.getId(), VmDeviceType.DISK.getName(), VmDeviceType.DISK.getName()); -- To view, visit http://gerrit.ovirt.org/7330 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I27c32525794b4ec3c9d8c78f033a3c9d39da72b9 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
