Moti Asayag has uploaded a new change for review.

Change subject: engine: Change backward compatibility logic for add/update vnic
......................................................................

engine: Change backward compatibility logic for add/update vnic

The current backward compatibility logic designed for
resolving a vnic profile id for a given network should
be modified, since REST clients which provides both
network name and vnic profile id might face issues, such
ignoring the vnic profile id.

For that purpose the engine will attempt to find a change
in the network name provided by the user to what is already
configured on the vnic:

1. If network name is provided, and unchanged - the vnic
    profile id from the parameters is used.
2. If the network name is provided and changed, use it
    instead of the vnic profile id.

Change-Id: If1611087bad26b0bfa5a42780c5362e3413fbb75
Bug-Url: https://bugzilla.redhat.com/1047887
Signed-off-by: Moti Asayag <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/VmTemplateInterfaceCommandBase.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
5 files changed, 55 insertions(+), 11 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/48/24348/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
index f3fbf87..342c8bf 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/UpdateVmTemplateInterfaceCommand.java
@@ -46,10 +46,6 @@
             return false;
         }
 
-        if (!updateVnicForBackwardCompatibility()) {
-            return false;
-        }
-
         // Interface oldIface = interfaces.First(i => i.id ==
         // AddVmInterfaceParameters.Interface.id);
         VmNic oldIface = LinqUtils.firstOrNull(interfaces, new 
Predicate<VmNic>() {
@@ -59,6 +55,15 @@
             }
         });
 
+        if (oldIface == null) {
+            addCanDoActionMessage(VdcBllMessages.VM_INTERFACE_NOT_EXIST);
+            return false;
+        }
+
+        if (!updateVnicForBackwardCompatibility(oldIface)) {
+            return false;
+        }
+
         Version clusterCompatibilityVersion = 
getVdsGroup().getcompatibility_version();
         VmNicValidator nicValidator = new 
VmNicValidator(getParameters().getInterface(), clusterCompatibilityVersion, 
getVmTemplate().getOsId());
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/VmTemplateInterfaceCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/VmTemplateInterfaceCommandBase.java
index 0582dbc..5e79677 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/VmTemplateInterfaceCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/template/VmTemplateInterfaceCommandBase.java
@@ -47,8 +47,9 @@
                 : new 
ValidationResult(VdcBllMessages.NETWORK_INTERFACE_VM_CANNOT_BE_SET);
     }
 
-    protected boolean updateVnicForBackwardCompatibility() {
+    protected boolean updateVnicForBackwardCompatibility(VmNic oldNic) {
         if 
(!validate(VnicProfileHelper.updateNicForBackwardCompatibility(getParameters().getInterface(),
+                oldNic,
                 getParameters().getNetworkName(),
                 getParameters().isPortMirroring(),
                 getVmTemplate(),
@@ -59,4 +60,7 @@
         return true;
     }
 
+    protected boolean updateVnicForBackwardCompatibility() {
+        return updateVnicForBackwardCompatibility(null);
+    }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java
index 9bdd494..3c2f3ed 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/AbstractVmInterfaceCommand.java
@@ -94,8 +94,9 @@
                 : ValidationResult.VALID;
     }
 
-    protected boolean updateVnicForBackwardCompatibility() {
+    protected boolean updateVnicForBackwardCompatibility(VmNic oldNic) {
         if 
(!validate(VnicProfileHelper.updateNicForBackwardCompatibility(getParameters().getInterface(),
+                oldNic,
                 getParameters().getNetworkName(),
                 getParameters().isPortMirroring(),
                 getVm().getStaticData(),
@@ -106,6 +107,10 @@
         return true;
     }
 
+    protected boolean updateVnicForBackwardCompatibility() {
+        return updateVnicForBackwardCompatibility(null);
+    }
+
     protected ValidationResult vmStatusLegal(VMStatus status) {
         return status == VMStatus.Up || status == VMStatus.Down || status == 
VMStatus.ImageLocked
                 ? ValidationResult.VALID
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
index d9d4a2c..9bb9306 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/UpdateVmInterfaceCommand.java
@@ -152,10 +152,6 @@
             return false;
         }
 
-        if (!updateVnicForBackwardCompatibility()) {
-            return false;
-        }
-
         if (getVm() == null) {
             
addCanDoActionMessage(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_EXIST);
             return false;
@@ -183,6 +179,10 @@
             return false;
         }
 
+        if (!updateVnicForBackwardCompatibility(oldIface)) {
+            return false;
+        }
+
         if (!StringUtils.equals(oldIface.getName(), getInterfaceName()) && 
!uniqueInterfaceName(interfaces)) {
             return false;
         }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
index b8cc7df..e18391f 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/network/vm/VnicProfileHelper.java
@@ -154,17 +154,46 @@
         }
     }
 
+    /**
+     * Since the network name and port mirroring attributed were replaced on 
the {@link VmNic} with the vnic profile id,
+     * a certain logic should be applied to translate a given usage of the 
former api to the expected one.
+     *
+     * @param nic
+     *            the candidate nic to apply the logic for
+     * @param oldNic
+     *            the existing nic
+     * @param networkName
+     *            the network name to be configured for the nic
+     * @param portMirroring
+     *            indicator if port mirroring should be configured for the 
network
+     * @param vm
+     *            the vm which contains the nic
+     * @param user
+     *            the user which execute the action
+     * @return
+     */
     public static ValidationResult updateNicForBackwardCompatibility(VmNic nic,
+            VmNic oldNic,
             String networkName,
             boolean portMirroring,
             VmBase vm,
             VdcUser user) {
 
+        // if network wasn't provided, no need for backward compatibility logic
         if (networkName == null) {
             return ValidationResult.VALID;
         }
 
-        // empty network name is considered as an empty network
+        // if the network was provided but unchanged, use the provided vnic 
profile id
+        if (oldNic != null && oldNic.getVnicProfileId() != null) {
+            VnicProfile oldProfile = 
getVnicProfileDao().get(oldNic.getVnicProfileId());
+            Network oldNetwork = 
getNetworkDao().get(oldProfile.getNetworkId());
+            if (StringUtils.equals(networkName, oldNetwork.getName())) {
+                return ValidationResult.VALID;
+            }
+        }
+
+        // empty network name is considered as an empty (unlinked) network
         if ("".equals(networkName)) {
             if (portMirroring) {
                 return new 
ValidationResult(VdcBllMessages.PORT_MIRRORING_REQUIRES_NETWORK);
@@ -174,6 +203,7 @@
             }
         }
 
+        // if the network was provided with changed name, resolve a suitable 
profile for it
         Network network = getNetworkDao().getByNameAndCluster(networkName, 
vm.getVdsGroupId());
         if (network == null) {
             return new 
ValidationResult(VdcBllMessages.NETWORK_NOT_EXISTS_IN_CLUSTER);


-- 
To view, visit http://gerrit.ovirt.org/24348
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: If1611087bad26b0bfa5a42780c5362e3413fbb75
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Moti Asayag <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to