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

Reply via email to