Emily Zhang has posted comments on this change.

Change subject: [WIP] engine:Trusted Compute Pools - Open Attestation 
integration with oVirt engine proposal
......................................................................


Patch Set 13: (3 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/attestationbroker/AttestThread.java
Line 34:             values = 
AttestationService.getInstance().attestHosts(trustedHostsNames.subList(0, 
FIRST_STAGE_QUERY_SIZE));
Line 35:             handleValues(values);
Line 36:             for (int i = FIRST_STAGE_QUERY_SIZE; i < 
trustedHostsNames.size(); i+= SECOND_STAGE_QUERY_SIZE) {
Line 37:                 values = AttestationService.getInstance()
Line 38:                         .attestHosts(trustedHostsNames.subList(i, i + 
SECOND_STAGE_QUERY_SIZE));
Ofri, the for loop is to cut the list to few small lists with 
size=attestationSecondStageSize. But here is a case, if 
(trustedHostsNames.size-FIRST_STAGE_QUERY_SIZE)% SECOND_STAGE_QUERY_SIZE !=0, 
and we still cut with the size for the last list, then it will throw NPE.
We can do like this:
for (int i = FIRST_STAGE_QUERY_SIZE, int j =0; i < trustedHostsNames.size();  
i+= SECOND_STAGE_QUER
Y_SIZE){
        j = (i+SECOND_STAGE_QUERY_SIZE < trustedHostsNames.size()) ? i + 
SECOND_STAGE_QUERY_SIZE: trustedHostsNames.size();
       values = AttestationService.getInstance. 
attestHosts(trustedHostsNames.subList(i, j));
}
Line 39:                 handleValues(values);
Line 40:             }
Line 41:         } else {
Line 42:             values = 
AttestationService.getInstance().attestHosts(trustedHostsNames);


Line 66: 
Line 67:     private void moveVdsToUp(VDS vds) {
Line 68:         HostStoragePoolParametersBase tempVar =
Line 69:                 new HostStoragePoolParametersBase(vds);
Line 70:         
Backend.getInstance().runInternalAction(VdcActionType.InitVdsOnUp, tempVar);
Ofri, here we have attested the host, and its result is trusted, so we need to 
move the vds status to up. But we involk InitVdsOnUp process, and in this 
process we attest the host again. How can we avoid the reduplicate attestation?
Line 71:     }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FenceVdsManualyCommand.java
Line 126: 
Line 127:     private void attestHost() {
Line 128:         List<String> name =  new ArrayList<>();
Line 129:         name.add(_problematicVds.getHostName());
Line 130:         AttestThread attestThread = new AttestThread(name);
Ofri, why we need to attest here?
Line 131:         attestThread.run();
Line 132:     }
Line 133: 
Line 134:     @Override


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8ce3448a821c74521d277f92f2c8d63ba0accfed
Gerrit-PatchSet: 13
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Dave Chen <wei.d.c...@intel.com>
Gerrit-Reviewer: Dave Chen <wei.d.c...@intel.com>
Gerrit-Reviewer: Doron Fediuck <dfedi...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Emily Zhang <lijuan.zh...@intel.com>
Gerrit-Reviewer: Gang Wei <gang....@intel.com>
Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com>
Gerrit-Reviewer: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-Reviewer: ofri masad <oma...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to