Maor Lipchuk has posted comments on this change. Change subject: core: Fix potential NPE when adding LUN disk ......................................................................
Patch Set 1: (3 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() { It is almost the same, the difference between the two conditions are : 1. We don't fetch the storage pool before checking if we handle disk image (That fixes the NPE) 2. Use of negative condition for DC version. The new condition in the refactored method checks if the DC level is not compatible for image disk. 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 100: } Line 101: Line 102: private boolean isVersionNotSupportedForShareableDisk() { Line 103: return getParameters().getDiskInfo().getDiskStorageType() == DiskStorageType.IMAGE Line 104: && getParameters().getDiskInfo().isShareable() This is negative now Line 105: && !Config.<Boolean> GetValue(ConfigValues.ShareableDiskEnabled, Line 106: getStoragePool().getcompatibility_version().getValue()); Line 107: } Line 108: .................................................... 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. I agree it doesn't sounds that clear, I will try to rephrase it. What I meant was that storage pool is still a field in LUN disk. suggestions are welcome. 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 <mlipc...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches