Martin Mucha has uploaded a new change for review. Change subject: core: refactor of HostNetworkAttachmentPersister ......................................................................
core: refactor of HostNetworkAttachmentPersister • added few javadoc • typo • extracted some conditions to give them name • extracted part of constructor code out to method to make constructor shorter/simpler. • renamed method 'removeNetworkAttachments' to make it clearer why are they removed. Change-Id: Ibecf02fe47f42c2184c1f010f794592944c55e62 Signed-off-by: Martin Mucha <[email protected]> --- M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java M backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java 2 files changed, 32 insertions(+), 9 deletions(-) git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/41/36141/1 diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java index e47ee0d..ffad017 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkAttachmentsPersister.java @@ -18,12 +18,20 @@ private final NetworkAttachmentDao networkAttachmentDao; private final Guid hostId; private final List<NetworkAttachment> userNetworkAttachments; - private List<NetworkAttachment> dbNetworkAttachments; private final Map<String, VdsNetworkInterface> nicsByName; + + /** + * map networkID-->NIC for each network assigned to one of given nics. Value of mapping is such mapped NIC. + */ private final Map<Guid, VdsNetworkInterface> reportedNicsByNetworkId; + + /** + * map networkID-->Network for each network assigned to one of given nics. + */ private final Map<Guid, Network> reportedNetworksById; private final BusinessEntityMap<Network> clusterNetworks; private final List<NetworkAttachment> configuredNetworkAttachments; + private List<NetworkAttachment> dbNetworkAttachments; public HostNetworkAttachmentsPersister(final NetworkAttachmentDao networkAttachmentDao, final Guid hostId, @@ -39,9 +47,13 @@ reportedNetworksById = new HashMap<>(); reportedNicsByNetworkId = new HashMap<>(); + initReportedNetworksAndNics(nics); + } + + private void initReportedNetworksAndNics(List<VdsNetworkInterface> nics) { for (VdsNetworkInterface nic : nics) { if (nic.getNetworkName() != null) { - Network network = this.clusterNetworks.get(nic.getNetworkName()); + Network network = clusterNetworks.get(nic.getNetworkName()); if (network != null) { reportedNetworksById.put(network.getId(), network); reportedNicsByNetworkId.put(network.getId(), nic); @@ -50,8 +62,8 @@ } } - public void persistNetworkAttchments() { - removeNetworkAttachments(); + public void persistNetworkAttachments() { + removeInvalidNetworkAttachmentsFromDb(); saveUserNetworkAttachments(); addNewNetworkAttachments(); } @@ -96,11 +108,16 @@ */ private void saveUserNetworkAttachments() { Map<Guid, NetworkAttachment> dbAttachmentsById = Entities.businessEntitiesById(getDbNetworkAttachments()); + //TODO MM: what should happen if there are more attachments referencing same network? Should it insert multiple records to db? Or should it update to last version? Or fail? + //TODO MM: what if attachment has nicID already assigned? for (NetworkAttachment attachment : userNetworkAttachments) { - if (networkConfiguredOnHost(attachment.getNetworkId())) { + boolean userAttachmentReferencesNetworkBoundToNic = networkConfiguredOnHost(attachment.getNetworkId()); + if (userAttachmentReferencesNetworkBoundToNic) { VdsNetworkInterface nic = reportedNicsByNetworkId.get(attachment.getNetworkId()); attachment.setNicId(nic.getId()); - if (dbAttachmentsById.containsKey(attachment.getId())) { + boolean alreadyExistingAttachment = dbAttachmentsById.containsKey(attachment.getId()); + + if (alreadyExistingAttachment) { networkAttachmentDao.update(attachment); } else { attachment.setId(Guid.newGuid()); @@ -112,9 +129,11 @@ } /** - * Removes attachments for networks which weren't reported from host + * Removes {@link org.ovirt.engine.core.common.businessentities.network.NetworkAttachment NetworkAttachments} + * which network aren't associated with any of given {@link #nicsByName nics} + * */ - private void removeNetworkAttachments() { + private void removeInvalidNetworkAttachmentsFromDb() { for (NetworkAttachment attachment : getDbNetworkAttachments()) { if (!networkConfiguredOnHost(attachment.getNetworkId())) { networkAttachmentDao.remove(attachment.getId()); @@ -124,6 +143,10 @@ } } + /** + * @param networkId network ID. + * @return true if given network ID describes network bound to any given NIC. + */ private boolean networkConfiguredOnHost(Guid networkId) { return reportedNetworksById.containsKey(networkId); } diff --git a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java index 7542f68..3ba5d87 100644 --- a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java +++ b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/HostNetworkTopologyPersisterImpl.java @@ -249,7 +249,7 @@ userConfiguredData.getNetworkAttachments(), clusterNetworks); - networkAttachmentPersister.persistNetworkAttchments(); + networkAttachmentPersister.persistNetworkAttachments(); } private String getVmNetworksImplementedAsBridgeless(VDS host, List<Network> clusterNetworks) { -- To view, visit http://gerrit.ovirt.org/36141 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ibecf02fe47f42c2184c1f010f794592944c55e62 Gerrit-PatchSet: 1 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Mucha <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
