Maor Lipchuk has posted comments on this change. Change subject: engine: Add support for Refresh LUNs size ......................................................................
Patch Set 11: (7 comments) https://gerrit.ovirt.org/#/c/39318/11/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RefreshLunsSizeCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RefreshLunsSizeCommand.java: Line 105: for (Map.Entry<String, List<VDS>> entry : lunToFailedVDS.entrySet()) { Line 106: String lunId = entry.getKey(); Line 107: List<VDS> vdsList = entry.getValue(); Line 108: log.error("Failed to refresh device " + lunId + " Not all VDS are seeing the same size " + Line 109: "VDS :" + vdsList); Instead of using the + operator you might want to use the log4j curly brackets pattern Line 110: String vdsListString = StringUtils.join(Entities.objectNames(vdsList), ", "); Line 111: failedVds.add("LUN : " + lunId + "VDS: " + vdsListString); Line 112: } Line 113: Line 107: List<VDS> vdsList = entry.getValue(); Line 108: log.error("Failed to refresh device " + lunId + " Not all VDS are seeing the same size " + Line 109: "VDS :" + vdsList); Line 110: String vdsListString = StringUtils.join(Entities.objectNames(vdsList), ", "); Line 111: failedVds.add("LUN : " + lunId + "VDS: " + vdsListString); same here Line 112: } Line 113: Line 114: throw new VdcBLLException(VdcBllErrors.REFRESH_LUN_ERROR, Line 115: "Failed to refresh LUNs. Not all VDS are seeing the same size: " + failedVds); Line 111: failedVds.add("LUN : " + lunId + "VDS: " + vdsListString); Line 112: } Line 113: Line 114: throw new VdcBLLException(VdcBllErrors.REFRESH_LUN_ERROR, Line 115: "Failed to refresh LUNs. Not all VDS are seeing the same size: " + failedVds); Isn't several failedVds will make the message between one another to be not seperated? Maybe worth to simply use the error message only in the for loop Line 116: } Line 117: Line 118: // Call PVs resize on SPM Line 119: resizePVs(lunsToRefresh); Line 160: vdsList.add(vdsSizePair.getFirst()); Line 161: if (size == -1) { Line 162: size = vdsSizePair.getSecond(); Line 163: } else { Line 164: if (!size.equals(vdsSizePair.getSecond())) { /s/else/elseif Line 165: failed = true; Line 166: } Line 167: } Line 168: } Line 161: if (size == -1) { Line 162: size = vdsSizePair.getSecond(); Line 163: } else { Line 164: if (!size.equals(vdsSizePair.getSecond())) { Line 165: failed = true; why not break here? Line 166: } Line 167: } Line 168: } Line 169: if (failed) { Line 166: } Line 167: } Line 168: } Line 169: if (failed) { Line 170: failedVds.put(entry.getKey(), vdsList); so if there is any failure we will always initiate this with the full list of lunToVds? Line 171: } Line 172: } Line 173: return failedVds; Line 174: } Line 175: Line 176: private void resizePVs(List<String> lunsToRefresh) { Line 177: for (String lun : lunsToRefresh) { Line 178: Long pvSizeInBytes = resizePV(lun); Line 179: log.debug("PV size after resize of LUN " + lun + " :" + pvSizeInBytes + " bytes"); Consider to use carly bracket Line 180: } Line 181: } Line 182: Line 183: private Long resizePV(String lunId) { -- To view, visit https://gerrit.ovirt.org/39318 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6c5c6d59087736466ee5e7eb0c77ee9282199c62 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Ala Hino <ah...@redhat.com> Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com> Gerrit-Reviewer: Amit Aviram <aavi...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Fred Rolland <froll...@redhat.com> Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com> Gerrit-Reviewer: Greg Padgett <gpadg...@redhat.com> Gerrit-Reviewer: Idan Shaby <ish...@redhat.com> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Liron Aravot <lara...@redhat.com> Gerrit-Reviewer: Maor Lipchuk <mlipc...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Ori Liel <ol...@redhat.com> Gerrit-Reviewer: Tal Nisan <tni...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
