Maor Lipchuk has posted comments on this change.
Change subject: core: Remove disk from snapshots (#828192)
......................................................................
Patch Set 18: (5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RemoveImageCommand.java
Line 125:
"RemoveImageCommand::RemoveImageFromDB: 'image' (snapshot of image '{0}') is
null, cannot remove it.",
Please remove the prefix RemoveImageCommand::RemoveImageFromDB:. Its redundent
Line 176: protected Snapshot
prepareSnapshotConfigurationsWithoutImageSingleImage(Guid vmSnapshotId, Guid
imageId) {
Please add java doc comment for this method and consider rename the method to
more shorter name (maybe getSnapshotConfWithoutSingleImage)
Line 195: if (imageInList.getImageId().equals(imageId)) {
log here would be a good idea
Line 202: snap.setVmConfiguration(newOvf);
Why not use the ovfManager.ExportVm(vmSnapshot, snapshotImages) directly in
setVmConfiguration. this will save you create a new parameter.
Line 208: return null;
Suggestion: returning null is not so nice, consider use Snapshot snap as a
return parameter for this method
--
To view, visit http://gerrit.ovirt.org/6025
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I97dedb9b8e8cd4dd0cae6c7b01d4b5678ba0a2d0
Gerrit-PatchSet: 18
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Liron Aravot <[email protected]>
Gerrit-Reviewer: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Michael Kublin <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
