Alon Bar-Lev has posted comments on this change. Change subject: oVirt Node Upgrade: Support N configuration ......................................................................
Patch Set 23: Code-Review+1 (4 comments) this is something I want to see gone in ovirt 4.0... but for this we need to re-write the entire node upgrade which we should anyway. I would have removed this function from VdsHandler as it is not used by any other module. but this is not that important. I truly hope this code works as I cannot execute it in mind as it is way to complex because of the incompatibility between data structures and requirements. http://gerrit.ovirt.org/#/c/14756/23/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java: Line 85 Line 86 Line 87 Line 88 Line 89 yes, I am being annoying... but you do not need to define shouldAdd here nor assign false. Line 100 Line 101 Line 102 Line 103 Line 104 or best remove it at all and use: if ( isoData.getVdsmCompatibilityVersion() != null && isIsoCompatibleForUpgradeByClusterVersion(isoData) || vdsOsVersion != null && VdsHandler.isIsoVersionCompatibleForUpgrade(vdsOsVersion, isoVersion) ) { } but this is just /me/ hate temporary one use vars... :) Line 84: Line 85: boolean shouldAdd = false; Line 86: RpmVersion isoVersion = new RpmVersion(file.getName()); Line 87: Line 88: shouldAdd = ( you can add boolean here Line 89: isoData.getVdsmCompatibilityVersion() != null && isIsoCompatibleForUpgradeByClusterVersion(isoData) || Line 90: vdsOsVersion != null && VdsHandler.isIsoVersionCompatibleForUpgrade(vdsOsVersion, isoVersion) Line 91: ); Line 92: Line 118: "cluster major {0} minor {1} iso major {2} minor {3}", Line 119: vdsClusterVersion.getMajor(), Line 120: vdsClusterVersion.getMinor(), Line 121: isoClusterVersion.getMajor(), Line 122: isoClusterVersion.getMinor() I really want to see toString() within these classes, even for debug, calling all these functions each time is something that should be avoided. Line 123: ); Line 124: return (vdsClusterVersion.getMajor() == isoClusterVersion.getMajor() && vdsClusterVersion.getMinor() <= isoClusterVersion.getMinor()); Line 125: } Line 126: -- To view, visit http://gerrit.ovirt.org/14756 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ibfb9dc5d0dc8780b519107acbe0ae866831f782c Gerrit-PatchSet: 23 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Barak Azulay <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Douglas Schilling Landgraf <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Ravi Nori <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[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
