Daniel Erez has posted comments on this change. Change subject: core,restapi: DirectLUN disk - validate LUN visibilty ......................................................................
Patch Set 1: (2 comments) http://gerrit.ovirt.org/#/c/31676/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AbstractDiskVmCommand.java: Line 279: } Line 280: Line 281: @Override Line 282: public VDS getVds() { Line 283: setVdsId(getParameters().getVdsId()); > This should be done in the ctor, IMHO. Done Line 284: return super.getVds(); Line 285: } Line 286: Line 287: @Override http://gerrit.ovirt.org/#/c/31676/1/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/validator/DiskValidator.java: Line 157: public ValidationResult isLunDiskVisible(final LUNs lun, VDS vds) { Line 158: GetDeviceListVDSCommandParameters parameters = Line 159: new GetDeviceListVDSCommandParameters(vds.getId(), lun.getLunType()); Line 160: List<LUNs> luns = (List<LUNs>) getVdsBroker().RunVdsCommand( Line 161: VDSCommandType.GetDeviceList, parameters).getReturnValue(); > Can you do this without running ConnectStorageServer first? Yes, I think it's redundant to call connect here as the flow of adding a DirectLUN disk requires connecting the storage server in order to retrieve the LUN's details. Hence, getting the device list without connecting is actually part of the validation of determining whether the LUN is valid and been retrieved using the correct procedure. Line 162: Line 163: // Search LUN in the device list Line 164: boolean lunExists = CollectionUtils.exists(luns, new Predicate() { Line 165: @Override -- To view, visit http://gerrit.ovirt.org/31676 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I339c999ebdfa69b8e2cbe4f55f8fe12e455810e4 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Daniel Erez <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Daniel Erez <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ori Liel <[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
