Omer Frenkel has posted comments on this change. Change subject: core: save memory state on live snapshot with memory ......................................................................
Patch Set 17: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/CreateAllSnapshotsFromVmCommand.java Line 118: Line 119: MemoryImageBuilder memoryImageBuilder = createMemoryImageBuilder(); Line 120: addSnapshotToDB(createdSnapshotId, memoryImageBuilder); Line 121: createSnapshotsForDisks(newActiveSnapshotId); Line 122: memoryImageBuilder.build(); not exactly, internal errors usually happen on exception in can do action, and on execute if there is no error set, in case of error from vdsm, error should be inside the exception, no handling is needed, all in command base Line 123: Line 124: if (getTaskIdList().isEmpty()) { Line 125: getParameters().setTaskGroupSuccess(true); Line 126: incrementVmGeneration(); Line 222: Line 223: VDSReturnValue vdsRetValue1 = runVdsCommand( Line 224: VDSCommandType.DeleteImageGroup, Line 225: new DeleteImageGroupVDSCommandParameters(guids[1], guids[0], guids[2], Line 226: false, false, getVm().getVdsGroupCompatibilityVersion().toString())); shouldn't be 2 deletes here? Line 227: Line 228: if (!vdsRetValue1.getSucceeded()) { Line 229: // log.. Line 230: } Line 307: Line 308: private SnapshotVDSCommandParameters buildLiveSnapshotParameters(Snapshot snapshot) { Line 309: List<Disk> pluggedDisks = getDiskDao().getAllForVm(getVm().getId(), true); Line 310: List<DiskImage> filteredPluggedDisks = ImagesHandler.filterImageDisks(pluggedDisks, false, true); Line 311: if (FeatureSupported.memorySnapshot(ClusterUtils.getCompatilibilyVersion(getVm()))) { in case isSaveMemory()==false this will cause snpashot verb to be sent to vdsm with empty string, is this ok? Line 312: return new SnapshotVDSCommandParameters(getVm().getRunOnVds().getValue(), Line 313: getVm().getId(), Line 314: filteredPluggedDisks, Line 315: snapshot.getMemoryVolume()); Line 391: return false; Line 392: } Line 393: } Line 394: Line 395: if (getStorageDomainId().equals(Guid.Empty)) { this might fail create snapshot of diskless vm with isSaveMemory()==false or some other weird scenarios Line 396: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_NO_SUITABLE_DOMAIN_FOUND); Line 397: } Line 398: Line 399: return true; Line 524: VDSReturnValue retVal = Line 525: Backend Line 526: .getInstance() Line 527: .getResourceManager() Line 528: .RunVdsCommand( not sure its possible if it doesn't extend CommandBase.. ? Line 529: VDSCommandType.CreateImage, Line 530: new CreateImageVDSCommandParameters(getVm().getStoragePoolId(), Line 531: getStorageDomainId() Line 532: .getValue(), -- To view, visit http://gerrit.ovirt.org/14291 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ifbb28efda1b63e506233a88399488a256e6ab1c8 Gerrit-PatchSet: 17 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Arik Hadas <aha...@redhat.com> Gerrit-Reviewer: Liron Ar <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: Roy Golan <rgo...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches