Alon Bar-Lev has posted comments on this change.

Change subject: oVirt Node Upgrade: Support N configuration
......................................................................


Patch Set 11: (5 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InstallVdsCommand.java
Line 33:     }
Line 34: 
Line 35:     @Override
Line 36:     protected boolean canDoAction() {
Line 37:         boolean retValue = true;
always assume failure.
Line 38: 
Line 39:         List<OVirtNodeInfo> info = OVirtNodeInfo.get();
Line 40: 
Line 41:         if (getVdsId() == null || getVdsId().equals(Guid.Empty)) {


Line 53:             }
Line 54:             VdsValidator.IsoValidStatus isoStat;
Line 55:             RpmVersion ovirtHostOsVersion = 
VdsHandler.getOvirtHostOsVersion(getVds());
Line 56:             VdsValidator validator = new VdsValidator(getVds());
Line 57:             isoStat = validator.isIsoFileValid(isoFile, info, 
ovirtHostOsVersion);
I think that Yair referred to use something with the validate() method.
Line 58:             switch(isoStat) {
Line 59:             case VALID:
Line 60:                  _isoFullPath = isoFile.toString();
Line 61:                  retValue = true;


Line 62:                  break;
Line 63:             case NOTCOMPATIBLE:
Line 64:                  
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_UPGRADE_BETWEEN_MAJOR_VERSION);
Line 65:                  addCanDoActionMessage(String.format("$IsoVersion 
%1$s", ovirtHostOsVersion.getMajor()));
Line 66:                  retValue = false;
no need to set failure, assume failure.
Line 67:                  break;
Line 68:             case MISSINGFILE:
Line 69:                  
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_MISSING_IMAGE_FILE);
Line 70:                  retValue = true;


Line 66:                  retValue = false;
Line 67:                  break;
Line 68:             case MISSINGFILE:
Line 69:                  
addCanDoActionMessage(VdcBllMessages.VDS_CANNOT_INSTALL_MISSING_IMAGE_FILE);
Line 70:                  retValue = true;
any reason why this is succes?
Line 71:                  break;
Line 72:             }
Line 73:         }
Line 74:         return retValue;


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsValidator.java
Line 101:                             }
Line 102:                         } else {
Line 103:                             result = IsoValidStatus.MISSINGFILE;
Line 104:                         }
Line 105:                         break;
why do you need this break?

or if you break anyway, why do you need the above break?

Why do we need to modify isoFile, shouldn't this function test the file? so it 
actually exist?
Line 106:                     }
Line 107:                 }
Line 108:             }
Line 109:         }


-- 
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: 11
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: Michael Burns <[email protected]>
Gerrit-Reviewer: Ravi Nori <[email protected]>
Gerrit-Reviewer: Yair Zaslavsky <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to