Moti Asayag has posted comments on this change. Change subject: engine: Add command to add network on provider ......................................................................
Patch Set 10: (5 inline comments) .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/AddNetworkOnProviderCommand.java Line 11: super(parameters); Line 12: } Line 13: Line 14: @Override Line 15: protected void executeCommand() { Please add can-do-actions for provider existence in addition to AddNetworkCommand validations Line 16: Provider provider = getDbFacade().getProviderDao().get(getNetwork().getProvidedBy().getProviderId()); Line 17: NetworkProviderProxy proxy = ProviderProxyFactory.getInstance().create(provider); Line 18: getNetwork().getProvidedBy().setExternalId(proxy.add(getNetwork())); Line 19: getNetwork().setVlanId(null); Line 14: @Override Line 15: protected void executeCommand() { Line 16: Provider provider = getDbFacade().getProviderDao().get(getNetwork().getProvidedBy().getProviderId()); Line 17: NetworkProviderProxy proxy = ProviderProxyFactory.getInstance().create(provider); Line 18: getNetwork().getProvidedBy().setExternalId(proxy.add(getNetwork())); How long might it take till the action (from provider) side will fail with time out ? Line 19: getNetwork().setVlanId(null); Line 20: Line 21: super.executeCommand(); Line 22: } .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/NetworkForCreate.java Line 3: import org.codehaus.jackson.annotate.JsonProperty; Line 4: import org.codehaus.jackson.map.annotate.JsonRootName; Line 5: Line 6: @SuppressWarnings("serial") Line 7: @JsonRootName(value = "network") Same comment as for PortForCreate - this code should be suggested to openstack to reflect better their properties rather than maintaining another set of business entities. Line 8: public class NetworkForCreate extends org.openstack.quantum.model.NetworkForCreate { Line 9: Line 10: @JsonProperty("provider:network_type") Line 11: private String providerNetworkType; .................................................... File backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/provider/network/openstack/OpenstackNetworkProviderProxy.java Line 49: networkForCreate.setName(network.getName()); Line 50: if (network.getLabel() != null) { Line 51: networkForCreate.setProviderPhysicalNetwork(network.getLabel()); Line 52: if (network.getVlanId() == null) { Line 53: networkForCreate.setProviderNetworkType("flat"); perhaps worth a global constants file for the advanced networking provider Line 54: } else { Line 55: networkForCreate.setProviderNetworkType("vlan"); Line 56: networkForCreate.setProviderSegmentationId(network.getVlanId()); Line 57: } Line 61: org.openstack.quantum.model.Network createdNetwork = Line 62: getQuantumClient().execute(new CreateNetwork(networkForCreate)); Line 63: return createdNetwork.getId(); Line 64: } catch (RuntimeException e) { Line 65: throw new VdcBLLException(VdcBllErrors.PROVIDER_FAILURE, new RuntimeException(e)); new RuntimeException(e) is redundant. Line 66: } Line 67: } Line 68: Line 69: @Override -- To view, visit http://gerrit.ovirt.org/11429 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I08ae38a304cd3cb05ee510f126661e77bd7c0940 Gerrit-PatchSet: 10 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Livnat Peer <lp...@redhat.com> Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com> Gerrit-Reviewer: Moti Asayag <masa...@redhat.com> _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches