Roy Golan has posted comments on this change.

Change subject: core: introduce osinfo service instead of Config values
......................................................................


Patch Set 1: (1 inline comment)

....................................................
File 
backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/osinfo/LibosinfoClient.java
Line 86: 
Line 87:         String options = "-Djava.rmi.server.hostname=localhost";
Line 88:         String mainClassName = 
"org.ovirt.engine.core.osinfo.server.LibosinfoServer";
Line 89: 
Line 90:         builder.command(java, "-cp", classpath, options, 
mainClassName);
I think we should reconsider having the JNA calls from the backend itsef for 
the work around getting this daemon up with its own life-cycle just doesn't 
seems proportional to its part in the engine.

What about forming a solid test suite for this JNA invocations to give us 
confident of using it instead all of this?

The jvm itself is going native for interning string and many other basic stuff 
we are not aware of and the call behind "going native is out of EE spec" is 
lame for we are not a j2EE application (and don't intend to be)
Line 91:         return builder;
Line 92:     }
Line 93: 
Line 94:     private ProcessBuilder buildShellProcessCmd() {


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ibd75679a1a1af5d5a0925e181b5dfd6e87574a75
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Roy Golan <[email protected]>
Gerrit-Reviewer: Allon Mureinik <[email protected]>
Gerrit-Reviewer: Alon Bar-Lev <[email protected]>
Gerrit-Reviewer: Juan Hernandez <[email protected]>
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
Gerrit-Reviewer: Roy Golan <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to