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

Reply via email to