Mike Kolesnik has posted comments on this change.

Change subject: core: Validate update disk for shared disk
......................................................................


Patch Set 3: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java
Line 119:         if (retValue && getParameters().getDiskInfo().isShareable()) {
You can optimize here to check only when old disk was not shareable

Line 122:             for (DiskImage diskImage : diskImageList) {
I think check should be if there is more than one image for the disk. No need 
to check the images themselves.

Also you need to check (probably different patch) if disk is raw?

Line 124:                     
addCanDoActionMessage(VdcBllMessages.SHAREABLE_DISK_IS_NOT_SUPPORTED_FOR_DISK);
The code name doesn't seem to represent this case.. Is this on purpose?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I00dcd0dd30dd36ef7e03d17a3a360173720ab0ac
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]>
Gerrit-Reviewer: Mike Kolesnik <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to