Allon Mureinik has posted comments on this change.

Change subject: core: fix of job messages that uses SnapshotName object 
(#844283)
......................................................................


Patch Set 2: I would prefer that you didn't submit this

(3 inline comments)

see inline.

Also, please update the bugzilla ticket to include a fuller description of the 
issue.

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveSnapshotCommand.java
Line 56:     }
Line 57: 
Line 58:     /**
Line 59:      * @return The image snapshots associated with the VM snapshot. 
Note that the first time this method is run it
Line 60:      *         issues DAO call.
?
Line 61:      */
Line 62:     protected List<DiskImage> getSourceImages() {
Line 63:         if (_sourceImages == null) {
Line 64:             _sourceImages = 
getDiskImageDAO().getAllSnapshotsForVmSnapshot(getParameters().getSnapshotId());


Line 197:     }
Line 198: 
Line 199:     // TODO: this is a temporary method used until 
ImagesHandler.PerformImagesChecks will get decoupeld to several tests
Line 200:     // Until then, this method is called and passes hasImages() 
onwards so the VM validations are done even for diskless
Line 201:     // VMs
?
Line 202:     protected boolean validateImagesAndVMStates() {
Line 203:         return ImagesHandler.PerformImagesChecks(getVm(), 
getReturnValue().getCanDoActionMessages(),
Line 204:                 getVm().getstorage_pool_id(), Guid.Empty,
Line 205:                 hasImages(), hasImages(), hasImages(), hasImages(), 
true, true, true, true, null);


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/TryBackToAllSnapshotsOfVmCommand.java
Line 57:         if (jobProperties == null) {
Line 58:             jobProperties = super.getJobMessageProperties();
Line 59:             Snapshot snapshot = 
getSnapshotDao().get(getParameters().getDstSnapshotId());
Line 60:             if (snapshot != null) {
Line 61:                 
jobProperties.put(VdcObjectType.SnapshotName.name().toLowerCase(), 
snapshot.getDescription());
consider extracting a getDestSnapshotDescription() method - this same block is 
repeated here and in RestoreAllSnapshotsCommand.
Line 62:             }
Line 63:         }
Line 64:         return jobProperties;
Line 65:     }


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I43e1de508e53eaf624fd7fb3de455a80003c5cd6
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Tal Nisan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to