Alon Bar-Lev has posted comments on this change.
Change subject: oVirt Node Upgrade: Support N configuration
......................................................................
Patch Set 8: (5 inline comments)
OK, much better!
Implementation is still ugly, but knowing the base it is a huge improvement.
Some last tasks.
Thanks!
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/GetoVirtISOsQuery.java
Line 78: info.minimumVersion,
Line 79: file.getName());
Line 80:
Line 81: boolean shouldAdd = false;
Line 82: if (isoVersion != null &&
isIsoVersionSupported(isoVersion, info.minimumVersion)) {
replace with isoVersion.compareTo(new Version(minimumVersion)) >= 0, and remove
this one liner function.
how can it be null? I think it is an error, and should be printed as an error,
to skip the logic of should I upgrade or not.
Line 83: log.debugFormat("isoVersion != null &&
isIsoVersionSupport = true");
Line 84: if (isoData.getVdsmCompatibilityVersion()
!= null) {
Line 85: shouldAdd =
isIsoCompatibleForUpgradeByClusterVersion(isoData);
Line 86:
log.debugFormat("isIsoCompatibleForUpgradeByClusterVersion() [{0}]", shouldAdd);
Line 135
Line 136
Line 137
Line 138
Line 139
it must be oVirtNode... this check should be first think in upgrade, and has
nothing to do with get version.
same for vds == null...
Line 136
Line 137
Line 138
Line 139
Line 140
then remove this one liner and add it to main.
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallVdsCommand.java
Line 52: if (getVds().getStatus() == VDSStatus.NonOperational) {
Line 53:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_STATUS_ILLEGAL);
Line 54: retValue = false;
Line 55: }
Line 56: if (!isIsoFileValid(isoFile, info)) {
you loop over all info here
Line 57:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_MISSING_IMAGE_FILE);
Line 58: retValue = false;
Line 59: } else {
Line 60: RpmVersion ovirtHostOsVersion =
VdsHandler.getOvirtHostOsVersion(getVds());
Line 57:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_MISSING_IMAGE_FILE);
Line 58: retValue = false;
Line 59: } else {
Line 60: RpmVersion ovirtHostOsVersion =
VdsHandler.getOvirtHostOsVersion(getVds());
Line 61: if (ovirtHostOsVersion != null &&
!isIsoVersionCompatible(ovirtHostOsVersion, isoFile, info)) {
and again here, can't we return something from above to eliminate the need to
loop again?
Line 62:
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_UPGRADE_BETWEEN_MAJOR_VERSION);
Line 63: addCanDoActionMessage(String.format("$IsoVersion
%1$s", ovirtHostOsVersion.getMajor()));
Line 64: retValue = false;
Line 65: }
--
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: 8
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Michael Burns <mbu...@redhat.com>
Gerrit-Reviewer: Ravi Nori <rn...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
