Michael Kublin has posted comments on this change.

Change subject: core: Use allow snapshot when Adding a disk to VM.
......................................................................


Patch Set 3: (5 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
Line 89:             ImagesHandler.setDiskAlias(getParameters().getDiskInfo(), 
getVm());
If returnValue == false, why we need to run that code? Also same question 
regards diskAlis, which u also inserted here

Line 142:     private void setAllowSnapshotForDisk() {
why we need to set such logic on params that passed from client, maybe it can 
be done at execute?

Line 335:     private void setVmSnapshotIdForDisk(AddImageFromScratchParameters 
parameters) {
if the value of isAllowSnapshot calculated on the fly, why it was added to disk 
entity?

Line 338:         } else {
it is already null or Guid.Empty by default

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddImageFromScratchCommand.java
Line 49:             
mNewCreatedDiskImage.setShareable(getParameters().getDiskInfo().isShareable());
I already asked about that field

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I135669c5f7bfb85fda561a47258834cd35d7688b
Gerrit-PatchSet: 3
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[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

Reply via email to