Doron Fediuck has posted comments on this change.
Change subject: Trusted Compute Pools - Open Attestation integration with oVirt
engine proposal
......................................................................
Patch Set 1: (4 inline comments)
Hi Dave, I've added a few comments inline, but I'd like to repeat one of the
comments since I think it is common for most comments you have there-
Dave, since this is not a private work, my best suggestion to you is to write
the design you have in mind in the ovirt wiki page you started. Any unclear /
open issues may be marked as such in the wiki. Then, post it to the
engine-devel list so everyone else can share the information and potentially
offer better solutions to performance issues as well as any other issues which
they may discover and we missed.
....................................................
File backend/manager/modules/bll/pom.xml
Line 165:
Line 166: <dependency>
Line 167: <groupId>org.apache.httpcomponents</groupId>
Line 168: <artifactId>httpclient</artifactId>
Line 169: <version>4.1.2</version>
Dave, 2 issues found here (applies to the below code as well):
1. The convention is to define artifact version in the root pom.xml file (the
one you'll find in the root folder of the project) as a maven variable, so
sub-projects can re-use it. In this case you should use ${httpclient.version}.
2. Please reuse the existing artifact version we currently use in the project,
which in this case would be 3.1. If for any reason you find it problematic, you
may suggest a separate patch to upgrade the artifact, after you verified it
will not cause regressions nor new issues with existing code.
Line 170: </dependency>
Line 171:
Line 172: <dependency>
Line 173: <groupId>org.apache.httpcomponents</groupId>
....................................................
File
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsSelector.java
Line 518: vmNICs =
getVmNetworkInterfaceDao().getAllForVm(getVm().getId());
Line 519: }
Line 520: return vmNICs;
Line 521: }
Line 522: private static boolean validateHostIsTrusted(VDS curVds) {
Dave, since this is not a private work, my best suggestion to you is to write
the design you have in mind in the ovirt wiki page you started. Any unclear /
open issues may be marked as such in the wiki. Then, post it to the
engine-devel list so everyone else can share the information and potentially
offer better solutions to performance issues as well as any other issues which
they may discover and we missed.
Line 523: String attestationWSURL, trustStorePath;
Line 524: attestationWSURL = Config.<String>
GetValue(ConfigValues.AttestationWebServicesUrl);
Line 525: trustStorePath=Config.<String>
GetValue(ConfigValues.TrustStore);
Line 526: DefaultHttpClient httpclient = new DefaultHttpClient();
Line 533: try {
Line 534: KeyStore trustStore =
KeyStore.getInstance(KeyStore.getDefaultType());
Line 535: FileInputStream instream = new FileInputStream(new
File(trustStorePath +"/TrustStore.jks"));
Line 536: trustStore.load(instream, null);
Line 537: SSLSocketFactory socketFactory = new
SSLSocketFactory(trustStore);
Sounds like code duplication... I'm sure we can change the original class to
accommodate both cases, or take out the shared code to a utility class.
Line 538: Scheme sch = new Scheme("https", 8443, socketFactory);
Line 539:
httpclient.getConnectionManager().getSchemeRegistry().register(sch);
Line 540: HttpPost httpPost = new HttpPost(attestationWSURL
+"/PollHosts");
Line 541: httpPost.setEntity(new StringEntity("{\"hosts\":[\""
+hostname +"\"]}"));
....................................................
Commit Message
Line 3: AuthorDate: 2013-01-19 05:23:18 -0500
Line 4: Commit: Dave Chen <[email protected]>
Line 5: CommitDate: 2013-01-19 05:23:18 -0500
Line 6:
Line 7: Trusted Compute Pools - Open Attestation integration with oVirt engine
proposal
Thanks Dave.
The wikipedia link is a good reference.
Note that people will expect design contents in the ovirt link in order to be
able to review your patches.
Line 8:
Line 9: Change-Id: I4de780cd46069638433255f3f9c994575f752e55
--
To view, visit http://gerrit.ovirt.org/11237
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4de780cd46069638433255f3f9c994575f752e55
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dave Chen <[email protected]>
Gerrit-Reviewer: Dave Chen <[email protected]>
Gerrit-Reviewer: Doron Fediuck <[email protected]>
Gerrit-Reviewer: Itamar Heim <[email protected]>
Gerrit-Reviewer: Omer Frenkel <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches
