Juan Hernandez has posted comments on this change.

Change subject: bootstrap: rewrite bootstrap using the new ovirt-host-deploy 
package
......................................................................


Patch Set 11: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsDeploy.java
Line 291:      */
Line 292:     private Callable _customizationDialog[] = new Callable[] {
Line 293:         new Callable<Object>() { public Object call() throws 
Exception {
Line 294:             if (
Line 295:                 (Boolean)_parser.cliEnvironmentGet(
You can put additional logic in the cliEnvironmentGetBoolean and 
cliEnvironmentSetBoolean methods to validate that what you are getting/setting 
is really a boolean (I guess you are using Map<String, Object> to store that), 
then you can generate a nice log message explaining the issue.
Line 296:                     VdsmEnv.OVIRT_NODE
Line 297:                 )
Line 298:             ) {
Line 299:                 _messages.post(


Line 297:                 )
Line 298:             ) {
Line 299:                 _messages.post(
Line 300:                     InstallerMessages.Severity.INFO,
Line 301:                     "Host is hypervisor"
I am not saying that the sentence is not correct, it is, just that it doesn't 
give any information, I already know that a host is a hypervisor. If instead of 
that you say "Host is ovirt-node" or "Host is node" or "Host is bare metal 
hypervisor" then you give some information.
Line 302:                 );
Line 303:                 _setNode();
Line 304:             }
Line 305:             return null;


Line 473:             if (_customizationShouldAbort) {
Line 474:                 _parser.cliAbort();
Line 475:             }
Line 476:             else {
Line 477:                 _customizationDialog[_customizationIndex++].call();
So if the other side of the communication decides to send this unexpected query 
we just crash with an ArrayOutOfBounds exception? Doesn't look very reliable.
Line 478:             }
Line 479:         }
Line 480:         catch (SoftError e) {
Line 481:             log.errorFormat(


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

Gerrit-MessageType: comment
Gerrit-Change-Id: If78c62601231f4729ca95da7653907b37856d672
Gerrit-PatchSet: 11
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com>
Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsl...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Juan Hernandez <juan.hernan...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to