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