Arik Hadas has posted comments on this change.

Change subject: core: remove memory image on remove snapshot
......................................................................


Patch Set 20: (4 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
Line 128:      * volumes should be removed only if the only snapshot that 
points to them is removed
Line 129:      */
Line 130:     private boolean shouldRemoveMemorySnapshotVolumes(String 
memoryVolume) {
Line 131:         return !memoryVolume.isEmpty() &&
Line 132:                 
getSnapshotDao().getNumOfSnapshotsByMemory(memoryVolume) == 1;
the same memory volume is kept on several snapshots in the following cases: 1. 
when preview snapshot, the memory volume is copied from the snapshot that is 
being previewed to the active snapshot 2. when running vm in stateless mode, 
the memory volume is copied to the active snapshot 3. maybe in the future we 
will support cloning a VM with its memory state (clone from snapshot + import 
as clone)
Line 133:     }
Line 134: 
Line 135:     private void removeMemory(final Snapshot snapshot) {
Line 136:         List<Guid> guids = 
GuidUtils.getGuidListFromString(snapshot.getMemoryVolume());


Line 143:                     @Override
Line 144:                     public boolean eval(Disk disk) {
Line 145:                         return disk.isWipeAfterDelete();
Line 146:                     }
Line 147:                 }).size() > 0;
this is just a heuristics to determine if post zero should be set for the 
memory image (which is basically a disk). note that it's the same logic that 
was used for hibernation volumes.
Line 148: 
Line 149:         // delete first image
Line 150:         // the next 'DeleteImageGroup' command should also take care 
of the
Line 151:         // image removal:


Line 152:         VDSReturnValue vdsRetValue = runVdsCommand(
Line 153:                 VDSCommandType.DeleteImageGroup,
Line 154:                 new 
DeleteImageGroupVDSCommandParameters(guids.get(1), guids.get(0), guids.get(2),
Line 155:                         postZero, false, 
getVm().getVdsGroupCompatibilityVersion().toString()));
Line 156: 
this method is replaced at http://gerrit.ovirt.org/#/c/15119/ with a call to 
VmCommand#removeMemoryVolumes. note that it's the same error handling in both 
places (this method was temporarily copied from there)..
Line 157:         if (!vdsRetValue.getSucceeded()) {
Line 158:             log.errorFormat("Cannot remove memory dump volume for 
snapshot {0}", snapshot.getId());
Line 159:         }
Line 160:         else {


Line 168:         vdsRetValue = runVdsCommand(
Line 169:                 VDSCommandType.DeleteImageGroup,
Line 170:                 new 
DeleteImageGroupVDSCommandParameters(guids.get(1), guids.get(0), guids.get(4),
Line 171:                         postZero, false, 
getVm().getVdsGroupCompatibilityVersion().toString()));
Line 172: 
see comment above
Line 173:         if (!vdsRetValue.getSucceeded()) {
Line 174:             log.errorFormat("Cannot remove memory metadata volume for 
snapshot {0}", snapshot.getId());
Line 175:         }
Line 176:         else {


-- 
To view, visit http://gerrit.ovirt.org/14687
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I88a707457b3b1565a887e141d18d9d08a2dbcc69
Gerrit-PatchSet: 20
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: Vered Volansky <vvola...@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