Yair Zaslavsky has posted comments on this change.
Change subject: core : Refactoring of disks related commands
......................................................................
Patch Set 3: Looks good to me, approved
(2 inline comments)
Minor cleanup issues which can be sent later in a clean up patch.
Looks good to me as well.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java
Line 156: protected boolean isOsSupported(VM vm) {
Minor comment - maybe this should change to
isOsSupportingPluggableDisks?
I suggest this, as maybe in the future we would like to use other disk related
features from OS that not all OS supports, so IMHO, this name is too general.
....................................................
File
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddDiskToVmParameters.java
Line 8: private Guid privateVmSnapshotId = Guid.Empty;
Can you use just "vmSnapshotId" (same goes for the next field).
I know we have lots of places of privateXXX fields , IMHO, we should clean them.
--
To view, visit http://gerrit.ovirt.org/2731
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I460986b12f96f0fe3511e28265bd8d71d678f223
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Michael Kublin <mkub...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
