Allon Mureinik has posted comments on this change.
Change subject: core: Fix potential NPE when adding LUN disk
......................................................................
Patch Set 1: I would prefer that you didn't submit this
(5 inline comments)
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddDiskCommand.java
Line 98: }
Line 99: return returnValue;
Line 100: }
Line 101:
Line 102: private boolean isVersionNotSupportedForShareableDisk() {
I don't get his function.
It looks like you moved stuff around, but didn't change any logic:
Line 103: return getParameters().getDiskInfo().getDiskStorageType() ==
DiskStorageType.IMAGE
Line 104: && getParameters().getDiskInfo().isShareable()
Line 105: && !Config.<Boolean>
GetValue(ConfigValues.ShareableDiskEnabled,
Line 106:
getStoragePool().getcompatibility_version().getValue());
Line 99: return returnValue;
Line 100: }
Line 101:
Line 102: private boolean isVersionNotSupportedForShareableDisk() {
Line 103: return getParameters().getDiskInfo().getDiskStorageType() ==
DiskStorageType.IMAGE
was present in the old isVersionSupportedForShareable function
Line 104: && getParameters().getDiskInfo().isShareable()
Line 105: && !Config.<Boolean>
GetValue(ConfigValues.ShareableDiskEnabled,
Line 106:
getStoragePool().getcompatibility_version().getValue());
Line 107: }
Line 100: }
Line 101:
Line 102: private boolean isVersionNotSupportedForShareableDisk() {
Line 103: return getParameters().getDiskInfo().getDiskStorageType() ==
DiskStorageType.IMAGE
Line 104: && getParameters().getDiskInfo().isShareable()
was present in the old if, line 95
Line 105: && !Config.<Boolean>
GetValue(ConfigValues.ShareableDiskEnabled,
Line 106:
getStoragePool().getcompatibility_version().getValue());
Line 107: }
Line 108:
Line 102: private boolean isVersionNotSupportedForShareableDisk() {
Line 103: return getParameters().getDiskInfo().getDiskStorageType() ==
DiskStorageType.IMAGE
Line 104: && getParameters().getDiskInfo().isShareable()
Line 105: && !Config.<Boolean>
GetValue(ConfigValues.ShareableDiskEnabled,
Line 106:
getStoragePool().getcompatibility_version().getValue());
was present in the old if, line 96-97
Line 107: }
Line 108:
Line 109: private boolean checkIfLunDiskCanBeAdded() {
Line 110: boolean returnValue = true;
....................................................
Commit Message
Line 6:
Line 7: core: Fix potential NPE when adding LUN disk
Line 8:
Line 9: Since LUN disk is not part of storage pool, validation of shareable disk
Line 10: should not use the storage pool that is part of the LUN disk.
what " storage pool that is part of the LUN disk"?
you just stated in the previous sentance that there is no such animal...
Line 11:
Line 12: The proposed fix, only validate the supported DC version on image disk.
Line 13:
Line 14: Change-Id: I6c55b95dac6f70435f7d1ac02a6a2a7cdd3f1e91
--
To view, visit http://gerrit.ovirt.org/7362
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I6c55b95dac6f70435f7d1ac02a6a2a7cdd3f1e91
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Maor Lipchuk <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
