Omer Frenkel has posted comments on this change.

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


Patch Set 15: (4 inline comments)

also missing dao test for new dao method, and update to fixtures.xml

....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/attestationbroker/AttestThread.java
Line 73:         vds.setNonOperationalReason(null);
Line 74:         
DbFacade.getInstance().getVdsDynamicDao().update(vds.getDynamicData());
Line 75: 
Line 76:         HostStoragePoolParametersBase tempVar = new 
HostStoragePoolParametersBase(vds);
Line 77:         
Backend.getInstance().runInternalAction(VdcActionType.InitVdsOnUp, tempVar);
calling this command will not move the host to up.
need to think of a way to activate the host and keep a flag it already been 
attested
i thought using the non-operational reason but i think it is cleared in 
ActivateVds command
Line 78:     }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/Backend.java
Line 246:         if (vdsGroups == null || vdsGroups.size() == 0) {
Line 247:             return;
Line 248:         }
Line 249:         for (VDSGroup vdsGroup : vdsGroups) {
Line 250:             List<VDS> vdssInGroup = 
DbFacade.getInstance().getVdsDao().getAllForVdsGroup(vdsGroup.getId());
all hosts? not only UP hosts?
i dont see a reason to change hosts in non-responsive,non-operational and 
maintenance...
Line 251:             if (vdssInGroup != null) {
Line 252:                 trustedVdsList.addAll(vdssInGroup);
Line 253:             }
Line 254:         }


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
Line 76:     private boolean initTrustedService() {
Line 77:         List <String> hosts = new ArrayList<String> ();
Line 78: 
Line 79:         if (getVds().getStatus() != VDSStatus.NonOperational
Line 80:                 || getVds().getNonOperationalReason() != 
NonOperationalReason.UNTRUSTED) {
i think something is wrong here, when host moves to up this command is called, 
status shouldnt be non operational, also the reason, so this always returns 
true?
Line 81:             return true;
Line 82:         }
Line 83: 
Line 84:         hosts.add(getVds().getHostName());


....................................................
File 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/FenceVdsManualyCommand.java
Line 118:             // Remove all alerts except NOT CONFIG alert
Line 119:             AlertDirector.RemoveAllVdsAlerts(_problematicVds.getId(), 
false);
Line 120:         }
Line 121: 
Line 122:         if 
(getVdsGroupDAO().get(_problematicVds.getVdsGroupId()).supportsTrustedService())
 {
can you please explain why attestation is needed here?
Line 123:             attestHost();
Line 124:         }
Line 125:     }
Line 126: 


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