engine: Block GetCaps and SetupNetworks running in parallel

- The monitoring-lock was expanded to the whole SetupNetworks execution. All the refreshCaps calls already have monitoring lock. - Since the monitoring lock is a waiting lock, added the exlusive lock of SetupNetworks (with non-waiting manner) to the getCaps event mechanism. Change-Id: Iea864d9159358cc67d5fd305f2ff8e5ac796dc6e Bug-Url: https://bugzilla.redhat.com/1324479 Signed-off-by: Alona Kaplan <alkaplan@redhat.com>
oVirt · Jun 30, 2016 · 28f34a4 · 28f34a4
1 parent 05e6eb1
commit 28f34a4
Show file tree

Hide file tree

Showing 12 changed files with 55 additions and 31 deletions.
diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HostLocking.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/HostLocking.java
@@ -6,6 +6,7 @@
 import javax.inject.Singleton;
 
 import org.ovirt.engine.core.common.businessentities.VDS;
+import org.ovirt.engine.core.common.errors.EngineMessage;
 import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
@@ -43,6 +44,12 @@ private static String calculateClosingMessage(String commandName, VDS host) {
                 host.getStoragePoolName());
     }
 
+    public Map<String, Pair<String, String>> getSetupNetworksLock(Guid hostId) {
+        return Collections.singletonMap(LockingGroup.HOST_NETWORK.name() + hostId.toString(),
+                LockMessagesMatchUtil.makeLockingPair(LockingGroup.HOST_NETWORK,
+                        EngineMessage.ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS));
+    }
+
     private static class HostEngineLock extends EngineLock implements AutoCloseable {
         public final String closingMessage;
         private final Logger log;

diff --git a/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/RefreshHostCapabilitiesCommand.java b/...r/modules/bll/src/main/java/org/ovirt/engine/core/bll/RefreshHostCapabilitiesCommand.java
@@ -1,6 +1,6 @@
 package org.ovirt.engine.core.bll;
 
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
 
 import javax.inject.Inject;
@@ -13,6 +13,7 @@
 import org.ovirt.engine.core.common.errors.EngineMessage;
 import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.common.utils.Pair;
+import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.utils.lock.EngineLock;
 import org.ovirt.engine.core.vdsbroker.ResourceManager;
 
@@ -22,6 +23,9 @@ public class RefreshHostCapabilitiesCommand<T extends VdsActionParameters> exten
     @Inject
     private ResourceManager resourceManager;
 
+    @Inject
+    private HostLocking hostLocking;
+
     public RefreshHostCapabilitiesCommand(T parameters, CommandContext commandContext) {
         super(parameters, commandContext);
     }
@@ -41,8 +45,15 @@ protected void executeCommand() {
 
     @Override
     protected Map<String, Pair<String, String>> getExclusiveLocks() {
-        return Collections.singletonMap(getParameters().getVdsId().toString(),
-                LockMessagesMatchUtil.makeLockingPair(LockingGroup.VDS, EngineMessage.ACTION_TYPE_FAILED_OBJECT_LOCKED));
+        Guid hostId = getParameters().getVdsId();
+
+        Map<String, Pair<String, String>> exclusiveLocks = new HashMap<>();
+
+        exclusiveLocks.put(hostId.toString(),
+                LockMessagesMatchUtil.makeLockingPair(LockingGroup.VDS,
+                        EngineMessage.ACTION_TYPE_FAILED_OBJECT_LOCKED));
+        exclusiveLocks.putAll(hostLocking.getSetupNetworksLock(hostId));
+        return exclusiveLocks;
     }
 
     @Override

diff --git a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VdsEventListener.java
@@ -548,6 +548,11 @@ public void refreshHostIfAnyVmHasHostDevices(final List<Guid> vmIds, final Guid
         });
     }
 
+    @Override
+    public void refreshHostCapabilities(Guid hostId) {
+        backend.runInternalAction(VdcActionType.RefreshHostCapabilities, new VdsActionParameters(hostId));
+    }
+
     @Override
     public boolean isUpdateAvailable(VDS host) {
         return availableUpdatesFinder.isUpdateAvailable(host);

diff --git a/...es/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksCommand.java b/...es/bll/src/main/java/org/ovirt/engine/core/bll/network/host/HostSetupNetworksCommand.java
@@ -3,7 +3,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -21,7 +20,7 @@
 
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.Validate;
-import org.ovirt.engine.core.bll.LockMessagesMatchUtil;
+import org.ovirt.engine.core.bll.HostLocking;
 import org.ovirt.engine.core.bll.NonTransactiveCommandAttribute;
 import org.ovirt.engine.core.bll.ValidationResult;
 import org.ovirt.engine.core.bll.VdsCommand;
@@ -57,7 +56,6 @@
 import org.ovirt.engine.core.common.errors.EngineException;
 import org.ovirt.engine.core.common.errors.EngineMessage;
 import org.ovirt.engine.core.common.interfaces.FutureVDSCall;
-import org.ovirt.engine.core.common.locks.LockingGroup;
 import org.ovirt.engine.core.common.network.SwitchType;
 import org.ovirt.engine.core.common.queries.IdQueryParameters;
 import org.ovirt.engine.core.common.queries.VdcQueryReturnValue;
@@ -168,6 +166,9 @@ public class HostSetupNetworksCommand<T extends HostSetupNetworksParameters> ext
     @Inject
     private UnmanagedNetworkValidator unmanagedNetworkValidator;
 
+    @Inject
+    private HostLocking hostLocking;
+
     public HostSetupNetworksCommand(T parameters) {
         this(parameters, null);
     }
@@ -190,9 +191,7 @@ protected LockProperties applyLockProperties(LockProperties lockProperties) {
 
     @Override
     protected Map<String, Pair<String, String>> getExclusiveLocks() {
-        return Collections.singletonMap(getParameters().getVdsId().toString(),
-                LockMessagesMatchUtil.makeLockingPair(LockingGroup.HOST_NETWORK,
-                        EngineMessage.ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS));
+        return hostLocking.getSetupNetworksLock(getParameters().getVdsId());
     }
 
     @Override
@@ -303,32 +302,33 @@ protected void executeCommand() {
             return;
         }
 
-        int timeout = getSetupNetworksTimeout();
-        FutureVDSCall<VDSReturnValue> setupNetworksTask = invokeSetupNetworksCommand(timeout);
+        try (EngineLock monitoringLock = acquireMonitorLock("Host setup networks")) {
+            int timeout = getSetupNetworksTimeout();
+            FutureVDSCall<VDSReturnValue> setupNetworksTask = invokeSetupNetworksCommand(timeout);
 
-        try {
-            VDSReturnValue retVal = setupNetworksTask.get(timeout, TimeUnit.SECONDS);
-            if (retVal != null) {
-                if (!retVal.getSucceeded() && retVal.getVdsError() == null && getParameters().rollbackOnFailure()) {
-                    throw new EngineException(EngineError.SETUP_NETWORKS_ROLLBACK, retVal.getExceptionString());
-                }
+            try {
+                VDSReturnValue retVal = setupNetworksTask.get(timeout, TimeUnit.SECONDS);
+                if (retVal != null) {
+                    if (!retVal.getSucceeded() && retVal.getVdsError() == null && getParameters().rollbackOnFailure()) {
+                        throw new EngineException(EngineError.SETUP_NETWORKS_ROLLBACK, retVal.getExceptionString());
+                    }
+
+                    VdsHandler.handleVdsResult(retVal);
 
-                VdsHandler.handleVdsResult(retVal);
+                    if (retVal.getSucceeded()) {
 
-                if (retVal.getSucceeded()) {
-                    try (EngineLock monitoringLock = acquireMonitorLock("Host setup networks")) {
                         VDSReturnValue returnValue =
-                            runVdsCommand(VDSCommandType.GetCapabilities,
-                                new VdsIdAndVdsVDSCommandParametersBase(getVds()));
+                                runVdsCommand(VDSCommandType.GetCapabilities,
+                                        new VdsIdAndVdsVDSCommandParametersBase(getVds()));
                         VDS updatedHost = (VDS) returnValue.getReturnValue();
                         persistNetworkChanges(updatedHost);
                     }
 
                     setSucceeded(true);
                 }
+            } catch (TimeoutException e) {
+                log.debug("Host Setup networks command timed out for {} seconds", timeout);
             }
-        } catch (TimeoutException e) {
-            log.debug("Host Setup networks command timed out for {} seconds", timeout);
         }
     }
 

diff --git a/...common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java b/...common/src/main/java/org/ovirt/engine/core/common/businessentities/IVdsEventListener.java
@@ -75,6 +75,8 @@ void storagePoolStatusChange(Guid storagePoolId, StoragePoolStatus status, Audit
 
     void refreshHostIfAnyVmHasHostDevices(List<Guid> vmIds, Guid hostId);
 
+    void refreshHostCapabilities(Guid hostId);
+
     boolean isUpdateAvailable(VDS host);
 
     void importHostedEngineVm(VM vm);

diff --git a/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java b/...nager/modules/common/src/main/java/org/ovirt/engine/core/common/errors/EngineMessage.java
@@ -574,7 +574,7 @@ public enum EngineMessage {
     VALIDATION_CLUSTER_NAME_INVALID(ErrorType.BAD_PARAMETERS),
     VALIDATION_CLUSTER_MIGRATE_ON_ERROR_NOT_NULL(ErrorType.BAD_PARAMETERS),
     VALIDATION_CLUSTER_SPICE_PROXY_HOSTNAME_OR_IP(ErrorType.BAD_PARAMETERS),
-    ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS(ErrorType.CONFLICT),
+    ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_NETWORK_NAME_IN_USE(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_NETWORK_IN_ONE_USE(ErrorType.CONFLICT),
     ACTION_TYPE_FAILED_NETWORK_IN_MANY_USES(ErrorType.CONFLICT),

diff --git a/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties b/backend/manager/modules/dal/src/main/resources/bundles/AppErrors.properties
@@ -592,7 +592,7 @@ ERROR_CANNOT_ADD_STORAGE_DOMAIN_WITH_ATTACHED_DATA_DOMAIN=Cannot ${action} ${typ
 ERROR_CANNOT_REMOVE_POOL_WITH_ACTIVE_DOMAINS=Cannot remove Data Center which contains active/locked Storage Domains.\n\
     -Please deactivate all domains and wait for tasks to finish before removing the Data Center.
 CLUSTER_CANNOT_CHANGE_STORAGE_POOL=Cannot change Data Center association when editing a Cluster.
-ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks process in progress on the host. Please try later after refreshing the host network capabilities.
+ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks or Host Refresh process in progress on the host. Please try later.
 ACTION_TYPE_FAILED_NETWORK_NAME_IN_USE=Cannot ${action} ${type}. The name of the logical network '${NetworkName}' is already used by an existing logical network in the same data-center.\n\
         -Please choose a different name.
 ACTION_TYPE_FAILED_NETWORK_IN_MANY_USES=Cannot ${action} ${type}. This logical network is used by ${entities}: (${ACTION_TYPE_FAILED_NETWORK_IN_ONE_USE_LIST_COUNTER}): ${ACTION_TYPE_FAILED_NETWORK_IN_ONE_USE_LIST}\n - Please detach ${entities} using this logical network and try again.

diff --git a/...ules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/HostConnectionRefresher.java b/...ules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/HostConnectionRefresher.java
@@ -44,7 +44,7 @@ public void onSubscribe(Subscription sub) {
         @Override
         public void onNext(Map<String, Object> map) {
             try {
-                vdsManager.refreshHost();
+                resourceManager.getEventListener().refreshHostCapabilities(vdsManager.getVdsId());
             } finally {
                 subscription.request(1);
             }

diff --git a/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java b/frontend/webadmin/modules/frontend/src/main/java/org/ovirt/engine/ui/frontend/AppErrors.java
@@ -1097,7 +1097,7 @@ public interface AppErrors extends ConstantsWithLookup {
 
     String CLUSTER_CANNOT_CHANGE_STORAGE_POOL();
 
-    String ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS();
+    String ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS();
 
     String ACTION_TYPE_FAILED_NETWORK_NAME_IN_USE();
 

diff --git a/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/...min/modules/frontend/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -404,7 +404,7 @@ ACTION_TYPE_FAILED_ROLE_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} ${type}. D
 ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
 ACTION_TYPE_FAILED_SERVER_STATUS_NOT_UP=Cannot ${action} ${type}. The server ${VdsName} is not UP.
 ACTION_TYPE_FAILED_SETTING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Setting default ${type} is not supported.
-ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks process in progress on the host. Please try later after refreshing the host network capabilities.
+ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks or Host Refresh process in progress on the host. Please try later.
 ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
 ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN=Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains.
 ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type} (${diskAliases}). This operation is not supported.

diff --git a/...ules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/...ules/userportal-gwtp/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -406,7 +406,6 @@ ACTION_TYPE_FAILED_ROLE_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} ${type}. D
 ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
 ACTION_TYPE_FAILED_SERVER_STATUS_NOT_UP=Cannot ${action} ${type}. The server ${VdsName} is not UP.
 ACTION_TYPE_FAILED_SETTING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Setting default ${type} is not supported.
-ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks process in progress on the host. Please try later after refreshing the host network capabilities.
 ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
 ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type}. This operation is not supported.
 ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN=Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains.

diff --git a/...min/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties b/...min/modules/webadmin/src/main/resources/org/ovirt/engine/ui/frontend/AppErrors.properties
@@ -408,7 +408,7 @@ ACTION_TYPE_FAILED_ROLE_NETWORK_HAS_NO_BOOT_PROTOCOL=Cannot ${action} ${type}. R
 ACTION_TYPE_FAILED_SCSI_RESERVATION_NOT_VALID_FOR_FLOATING_DISK=Cannot ${action} ${type}. SCSI reservation cannot be set when adding floating disks.
 ACTION_TYPE_FAILED_SERVER_STATUS_NOT_UP=Cannot ${action} ${type}. The server ${VdsName} is not UP.
 ACTION_TYPE_FAILED_SETTING_DEFAULT_MAC_POOL_IS_NOT_SUPPORTED=Cannot ${action} ${type}. Setting default ${type} is not supported.
-ACTION_TYPE_FAILED_SETUP_NETWORKS_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks process in progress on the host. Please try later after refreshing the host network capabilities.
+ACTION_TYPE_FAILED_SETUP_NETWORKS_OR_REFRESH_IN_PROGRESS=Cannot ${action} ${type}. Another Setup Networks or Host Refresh process in progress on the host. Please try later.
 ACTION_TYPE_FAILED_SGIO_IS_FILTERED=Cannot ${action} ${type}. SCSI reservation can be set only when SGIO is unfiltered.
 ACTION_TYPE_FAILED_SHAREABLE_DISK_NOT_SUPPORTED=Cannot ${action} a shareable ${type} (${diskAliases}). This operation is not supported.
 ACTION_TYPE_FAILED_SHAREABLE_DISKS_NOT_SUPPORTED_ON_GLUSTER_DOMAIN=Cannot ${action} ${type}. Shareable disks are not supported on Gluster domains.