Amit Aviram has posted comments on this change.

Change subject: core: Preventing moving a shareable disk to a Gluster domain
......................................................................


Patch Set 5:

(4 comments)

http://gerrit.ovirt.org/#/c/35398/5/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommand.java:

Line 98:                 && validateSpaceRequirements()
Line 99:                 && checkCanBeMoveInVm()
Line 100:                 && checkIfNeedToBeOverride()
Line 101:                 && setAndValidateDiskProfiles()
Line 102:                 && verifyShareableDisksMoving();
> /s/verifyShareableDisksMoving/validateShareableNotMovedToGluserDomain()
Done
Line 103:     }
Line 104: 
Line 105:     private boolean verifyShareableDisksMoving() {
Line 106:         if (getParameters().getOperation() == ImageOperation.Move) {


Line 103:     }
Line 104: 
Line 105:     private boolean verifyShareableDisksMoving() {
Line 106:         if (getParameters().getOperation() == ImageOperation.Move) {
Line 107:             DiskImage diskImage = 
getDiskImageDao().get(getParameters().getImageId());
> use getImage() instead, it already caches the disk
Done
Line 108:             if (diskImage.isShareable()) {
Line 109:                 StorageDomain targetDomain = 
getStorageDomainDAO().get(getParameters().getStorageDomainId());
Line 110:                 if 
(targetDomain.getStorageType().equals(StorageType.GLUSTERFS)) {
Line 111:                     return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS);


Line 105:     private boolean verifyShareableDisksMoving() {
Line 106:         if (getParameters().getOperation() == ImageOperation.Move) {
Line 107:             DiskImage diskImage = 
getDiskImageDao().get(getParameters().getImageId());
Line 108:             if (diskImage.isShareable()) {
Line 109:                 StorageDomain targetDomain = 
getStorageDomainDAO().get(getParameters().getStorageDomainId());
> use getStorageDomain()
Done
Line 110:                 if 
(targetDomain.getStorageType().equals(StorageType.GLUSTERFS)) {
Line 111:                     return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_CANT_MOVE_SHAREABLE_DISK_TO_GLUSTERFS);
Line 112:                 }
Line 113:             }


http://gerrit.ovirt.org/#/c/35398/5/backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java
File 
backend/manager/modules/bll/src/test/java/org/ovirt/engine/core/bll/MoveOrCopyDiskCommandTest.java:

Line 92:     public void moveShareableDiskToGlusterDomain() {
Line 93:         initializeCommand(ImageOperation.Move);
Line 94:         initShareableDiskImage();
Line 95:         initGlusterDomain();
Line 96:         assertFalse(command.canDoAction());
> please also check that the correct message is returned..see how it's done i
Done
Line 97:     }
Line 98: 
Line 99:     private void initGlusterDomain() {
Line 100:         StorageDomain destDomain = new StorageDomain();


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I37e500e86d2b6094bb257221a4ca899590aed610
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com>
Gerrit-Reviewer: Daniel Erez <de...@redhat.com>
Gerrit-Reviewer: Idan Shaby <ish...@redhat.com>
Gerrit-Reviewer: Liron Aravot <lara...@redhat.com>
Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to