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