Nir Soffer has posted comments on this change. Change subject: Core: Prevent VM migration when SCSI reservation is used ......................................................................
Patch Set 2: (5 comments) https://gerrit.ovirt.org/#/c/37664/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java: Line 172: Line 173: // verify that scsi reservation is enabled only when scsi passthrough is enabled Line 174: LunDisk lunDisk = ((LunDisk) getParameters().getDiskInfo()); Line 175: if (lunDisk.isUsingScsiReservation() && !lunDisk.isScsiPassthrough()) { Line 176: // it is illegal to set scsi reservation is used if scsi passthrough is disabled This repeats the comment in line 173, and the code. This should explain why we must ignore the using-scsi-reservation flag when scsi passthrough is disabled. Adding to the original comment in line 173 will be better. Line 177: return failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_PASSTHROUGH_IS_DISABLED); Line 178: } Line 179: Line 180: return true; https://gerrit.ovirt.org/#/c/37664/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/UpdateVmDiskCommand.java: Line 385: updateQuota(diskImage); Line 386: updateDiskProfile(); Line 387: } Line 388: Line 389: if (disk.getDiskStorageType() == DiskStorageType.LUN) { else if? Line 390: updateLunProperties((LunDisk)getNewDisk()); Line 391: } Line 392: Line 393: reloadDisks(); Line 387: } Line 388: Line 389: if (disk.getDiskStorageType() == DiskStorageType.LUN) { Line 390: updateLunProperties((LunDisk)getNewDisk()); Line 391: } else? Isn't this an error? Line 392: Line 393: reloadDisks(); Line 394: updateBootOrder(); Line 395: https://gerrit.ovirt.org/#/c/37664/2/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/utils/VmDeviceUtils.java: Line 1044: private static void addEmptyCD(Guid dstId) { Line 1045: VmDeviceUtils.addManagedDevice(new VmDeviceId(Guid.newGuid(), dstId), Line 1046: VmDeviceGeneralType.DISK, Line 1047: VmDeviceType.CDROM, Line 1048: Collections.<String, Object>singletonMap(VdsProperties.Path, ""), Is this change related? Line 1049: true, Line 1050: true, Line 1051: null); Line 1052: } https://gerrit.ovirt.org/#/c/37664/2/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java File backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VmDevice.java: Line 351: return isScsiReservationUsed; Line 352: } Line 353: Line 354: public void setScsiReservationUsed(boolean flag) { Line 355: this.isScsiReservationUsed = flag; Why not unify with LunDisk and use isUsingScsiReservation? Line 356: } -- To view, visit https://gerrit.ovirt.org/37664 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ia38c03dae04c9dbb30c882941391b1909f5af416 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Ala Hino <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Candace Sheremeta <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Freddy Rolland <[email protected]> Gerrit-Reviewer: Greg Padgett <[email protected]> Gerrit-Reviewer: Idan Shaby <[email protected]> Gerrit-Reviewer: Liron Aravot <[email protected]> Gerrit-Reviewer: Maor Lipchuk <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Vered Volansky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
