Omer Frenkel has posted comments on this change.

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


Patch Set 10: (5 inline comments)

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/attestationbroker/AttestThread.java
Line 65:             i++;
Line 66:         }
Line 67:     }
Line 68: 
Line 69:     public void initVds() {
the part moving hosts to non operational should be before the thread is 
starting so polling will not start for these hosts
also, the status should be saved to the db, or it will not be visible to other 
processes in the system
Line 70:         for (VDS vds : vdss) {
Line 71:             vds.setStatus(VDSStatus.NonOperational);
Line 72:             
vds.setNonOperationalReason(NonOperationalReason.UNINITIALIZED);
Line 73:         }


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/ResourceManager.java
Line 165:                 status = VDSStatus.Unassigned;
Line 166:                 break;
Line 167:             }
Line 168:             if (vds.getTrustedService()){
Line 169:                 status = VDSStatus.Unassigned;
i dont understand this code, if there is code that already move to non 
operational all vds in trusted cluster
Line 170:             }
Line 171:             if (status != vds.getStatus()) {
Line 172:                 vdsManager.setStatus(status, vds);
Line 173:                 
vdsManager.UpdateStatisticsData(vds.getStatisticsData());


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsManager.java
Line 407:                         getVdsId(),
Line 408:                         newStatus);
Line 409:             }
Line 410:             if (vds.getTrustedService()){
Line 411:                 List <String> hosts = new ArrayList<String> ();
why this list is needed?

in general isnt this checked in InitVdsOnUp?
Line 412:                 hosts.add(vds.getHostName());
Line 413:                 AttestationValue value = 
AttestationService.getInstance().attestHosts(hosts).get(0);
Line 414:                 
AttestationCacheManager.getInstance().updateCache(value);
Line 415:                 if (value.getTrustLevel() == 
AttestationResultEnum.TRUSTED) {


....................................................
File 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/VdsUpdateRunTimeInfo.java
Line 524:                 }
Line 525:             } else {
Line 526:                 // refresh dynamic data
Line 527:                 final AtomicBoolean processHardwareNeededAtomic = new 
AtomicBoolean();
Line 528:                 boolean checkOtherParams = _vds.getTrustedService()? 
false: true;
this can be replaced just with:
boolean checkOtherParams = _vds.getTrustedService();
Line 529:                 if (_vds.getTrustedService()){
Line 530:                     AttestationResultEnum trustLevel = 
AttestationService.getInstance().getAttestationValues().remove(_vds.getHostName());
Line 531:                     if (trustLevel != null  && trustLevel != 
AttestationResultEnum.TRUSTED){
Line 532:                         _vds.setStatus(VDSStatus.NonOperational);


Line 526:                 // refresh dynamic data
Line 527:                 final AtomicBoolean processHardwareNeededAtomic = new 
AtomicBoolean();
Line 528:                 boolean checkOtherParams = _vds.getTrustedService()? 
false: true;
Line 529:                 if (_vds.getTrustedService()){
Line 530:                     AttestationResultEnum trustLevel = 
AttestationService.getInstance().getAttestationValues().remove(_vds.getHostName());
its not clear what is done here
Line 531:                     if (trustLevel != null  && trustLevel != 
AttestationResultEnum.TRUSTED){
Line 532:                         _vds.setStatus(VDSStatus.NonOperational);
Line 533:                         
_vds.setNonOperationalReason(NonOperationalReason.UNTRUSTED);
Line 534:                         _saveVdsDynamic = true;


-- 
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: 10
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