Mike Kolesnik has posted comments on this change. Change subject: engine: Bind port to agent when port is being activated ......................................................................
Patch Set 9: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/28792/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/RunVmCommand.java: Line 197: Line 198: private void resumeVm() { Line 199: setVdsId(getVm().getRunOnVds()); Line 200: if (getVds() != null) { Line 201: initParametersForExternalNetworks(); > why you need this on resume? (just a reminder, resume is for paused vm, whi I think he was thinking about hibernate of VM. Paused VM should only "resume" on the host it's on since the process is still running there, so no need to update the port. Line 202: Line 203: try { Line 204: VDSReturnValue result = getBackend().getResourceManager() Line 205: .RunAsyncVdsCommand( Line 529: return parameters; Line 530: } Line 531: Line 532: protected void initParametersForExternalNetworks() { Line 533: if (getVm().getInterfaces().isEmpty()) { This should be pushed in a separate patch since it's not related.. Line 534: return; Line 535: } Line 536: Line 537: Map<VmDeviceId, VmDevice> nicDevices = http://gerrit.ovirt.org/#/c/28792/9/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java: Line 253: portForCreate.setBinding(new Binding()); Line 254: portForCreate.getBinding().setHostId(hostId); Line 255: port = getClient().ports().create(portForCreate).execute(); Line 256: } else { Line 257: boolean securityGroupsChanged = securityGroupsChanged(port.getSecurityGroups(), securityGroups); I think this can be inlined, and remain as "else if" Line 258: if (securityGroupsChanged || hostChanged(port, hostId)) { Line 259: Port portForUpdate = new PortForUpdate(); Line 260: portForUpdate.setId(port.getId()); Line 261: portForUpdate.setSecurityGroups(securityGroupsChanged ? securityGroups : port.getSecurityGroups()); -- To view, visit http://gerrit.ovirt.org/28792 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0cb5bf95054b6d2711f88a09d374a74f156a4055 Gerrit-PatchSet: 9 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> Gerrit-Reviewer: Omer Frenkel <ofren...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
