Alon Bar-Lev has posted comments on this change.

Change subject: Validate installed host by requesting for vdsm uuid
......................................................................


Patch Set 1: (7 inline comments)

Sorry for all comments...

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVdsCommand.java
Line 377
Line 378
Line 379
Line 380
Line 381
you should add a call above this one, and implement the logic here, it has 
nothing to do with VdsDeploy...

please follow the gluster hint... it reuse the same sshclient, so we use the 
same session, same authentication.


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsDeploy.java
Line 886:             _iptables = _getIpTables();
Line 887:         }
Line 888:     }
Line 889: 
Line 890:     static public String getInstalledVdsIdIfExists(String hostname, 
String password) {
This code does not belong here.
Line 891:         try {
Line 892:             final int SSH_PORT = 22;
Line 893:             final String USER = "root";
Line 894:             final String SUCCESS_RESULT = "RESULT=0";


Line 892:             final int SSH_PORT = 22;
Line 893:             final String USER = "root";
Line 894:             final String SUCCESS_RESULT = "RESULT=0";
Line 895:             final String ERR_OUTPUT = "RESULT=1";
Line 896:             final String VDSM_TOOL = "/usr/bin/vdsm-tool";
This should be in database, see gluster example.
Line 897: 
Line 898:             SSHClient client = new SSHClient();
Line 899:             Integer timeout = Config.<Integer> 
GetValue(ConfigValues.ConnectToServerTimeoutInSeconds) * 1000;
Line 900:             client.setHardTimeout(timeout);


Line 894:             final String SUCCESS_RESULT = "RESULT=0";
Line 895:             final String ERR_OUTPUT = "RESULT=1";
Line 896:             final String VDSM_TOOL = "/usr/bin/vdsm-tool";
Line 897: 
Line 898:             SSHClient client = new SSHClient();
No need to create new client.
Line 899:             Integer timeout = Config.<Integer> 
GetValue(ConfigValues.ConnectToServerTimeoutInSeconds) * 1000;
Line 900:             client.setHardTimeout(timeout);
Line 901:             client.setSoftTimeout(timeout);
Line 902:             client.setHost(hostname, SSH_PORT);


Line 904:             client.setUser(USER);
Line 905:             client.connect();
Line 906:             client.authenticate();
Line 907:             ByteArrayOutputStream out = new 
ConstraintByteArrayOutputStream(50);
Line 908:             ByteArrayOutputStream err = new 
ConstraintByteArrayOutputStream(50);
50 is not enough... I would have put here at least 256.
Line 909:             client.executeCommand(
Line 910:                     "TOOL=" + VDSM_TOOL + "; " +
Line 911:                     "if [ -x $TOOL ]; then " +
Line 912:                         "$TOOL vdsm-id ; echo RESULT=$?; " +


Line 905:             client.connect();
Line 906:             client.authenticate();
Line 907:             ByteArrayOutputStream out = new 
ConstraintByteArrayOutputStream(50);
Line 908:             ByteArrayOutputStream err = new 
ConstraintByteArrayOutputStream(50);
Line 909:             client.executeCommand(
I thought we agreed to just call the tool and fail if not exist...
Line 910:                     "TOOL=" + VDSM_TOOL + "; " +
Line 911:                     "if [ -x $TOOL ]; then " +
Line 912:                         "$TOOL vdsm-id ; echo RESULT=$?; " +
Line 913:                     " else echo " + ERR_OUTPUT + "; fi",


Line 914:                     null,
Line 915:                     out,
Line 916:                     err
Line 917:                 );
Line 918:             client.disconnect();
Not that it is relevant now... but this should have been in finally block.
Line 919:             String[] ret = new String(out.toByteArray(), 
"UTF-8").split("\n");
Line 920:             if (ret.length > 1 && ret[1].equals(SUCCESS_RESULT)) {
Line 921:                 return ret[0];
Line 922:             }


--
To view, visit http://gerrit.ovirt.org/14905
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I5c6c0a85daba47fabb9253963ff187a670f28ae6
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Shireesh Anjal <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to