Hello Liron Ar,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/19527

to review the following change.

Change subject: core: InitVdsOnUp - proceed regardless of problematic 
iso/export domain
......................................................................

core: InitVdsOnUp - proceed regardless of problematic iso/export domain

Prior to this patch, when host reported iso/export domain as problematic
during the InitVdsOnUp flow it didn't move to status UP.

As currently when ISO/Export domains are reported as problematic by some
of the hosts those hosts remain UP and doesn't move to NonOperational,
the behaviour between those two flows should be unified - therefore it
was decided that when hosts reports iso/export domain as in problem
it won't stop it from moving to UP.

Change-Id: I32372aa80d7193a814f2500231d4545d2b1a5fc3
Bug-Url: https://bugzilla.redhat.com/967604
Signed-off-by: Liron Aravot <lara...@redhat.com>
Signed-off-by: Tal Nisan <tni...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
2 files changed, 22 insertions(+), 14 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/27/19527/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
index fa56c63..37c9dcd 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
@@ -17,10 +17,12 @@
 import org.ovirt.engine.core.common.action.HostStoragePoolParametersBase;
 import org.ovirt.engine.core.common.action.SetNonOperationalVdsParameters;
 import org.ovirt.engine.core.common.action.VdcActionType;
+import org.ovirt.engine.core.common.businessentities.AttestationResultEnum;
 import org.ovirt.engine.core.common.businessentities.FenceActionType;
 import org.ovirt.engine.core.common.businessentities.FenceStatusReturnValue;
 import org.ovirt.engine.core.common.businessentities.NonOperationalReason;
 import org.ovirt.engine.core.common.businessentities.StorageDomain;
+import org.ovirt.engine.core.common.businessentities.StorageDomainStatic;
 import org.ovirt.engine.core.common.businessentities.StorageDomainStatus;
 import org.ovirt.engine.core.common.businessentities.StorageDomainType;
 import org.ovirt.engine.core.common.businessentities.StoragePool;
@@ -60,7 +62,6 @@
 import org.ovirt.engine.core.vdsbroker.attestation.AttestationService;
 import org.ovirt.engine.core.vdsbroker.attestation.AttestationValue;
 import org.ovirt.engine.core.vdsbroker.irsbroker.IrsBrokerCommand;
-import org.ovirt.engine.core.common.businessentities.AttestationResultEnum;
 
 /**
  * Initialize Vds on its loading. For storages: First connect all storage
@@ -266,12 +267,16 @@
         boolean returnValue = true;
         try {
             runVdsCommand(VDSCommandType.GetStats, new 
VdsIdAndVdsVDSCommandParametersBase(getVds()));
-            if 
(IrsBrokerCommand.isDomainsReportedAsProblematic(getVds().getStoragePoolId(), 
getVds().getDomains())) {
-                log.errorFormat("One of the Storage Domains of host {0} in 
pool {1} is problematic",
-                        getVds().getName(),
-                        getStoragePool()
-                                .getName());
-                returnValue = false;
+            List<Guid> problematicDomainsIds = 
IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getStoragePoolId(), 
getVds().getDomains());
+            for (Guid domainId : problematicDomainsIds) {
+                StorageDomainStatic domainInfo = 
getStorageDomainStaticDAO().get(domainId);
+                log.errorFormat("Storage Domain {0} of pool {1} is in problem 
in host {2}",
+                        domainInfo != null ? domainInfo.getStorageName() : 
domainId,
+                        getStoragePool().getName(),
+                        getVds().getName());
+                if (domainInfo == null || 
domainInfo.getStorageDomainType().isDataDomain()) {
+                    returnValue = false;
+                }
             }
         } catch (VdcBLLException e) {
             log.errorFormat("Could not get Host statistics for Host {0}, Error 
is {1}",
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
index 0279371..b889f7d 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
@@ -5,9 +5,11 @@
 import java.text.MessageFormat;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -94,12 +96,12 @@
         }
     }
 
-    public static boolean isDomainsReportedAsProblematic(Guid storagePoolId, 
List<VDSDomainsData> vdsDomainsData) {
+    public static List<Guid> fetchDomainsReportedAsProblematic(Guid 
storagePoolId, List<VDSDomainsData> vdsDomainsData) {
         IrsProxyData proxy = _irsProxyData.get(storagePoolId);
         if (proxy != null) {
-            return proxy.isDomainsReportedAsProblematic(vdsDomainsData);
+            return proxy.checkIfDomainsReportedAsProblematic(vdsDomainsData);
         }
-        return false;
+        return Collections.emptyList();
     }
 
     @Override
@@ -1104,7 +1106,8 @@
                     AuditLogType.VDS_DOMAIN_DELAY_INTERVAL);
         }
 
-        public boolean isDomainsReportedAsProblematic(List<VDSDomainsData> 
vdsDomainsData) {
+        public List<Guid> 
checkIfDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData) {
+            List<Guid> domainsInProblem = new LinkedList<>();
             Set<Guid> domainsInPool = new HashSet<Guid>(
                     
DbFacade.getInstance().getStorageDomainStaticDao().getAllIds(
                             _storagePoolId, StorageDomainStatus.Active));
@@ -1114,7 +1117,7 @@
             for (VDSDomainsData vdsDomainData : vdsDomainsData) {
                 if (domainsInPool.contains(vdsDomainData.getDomainId())) {
                     if (isDomainReportedAsProblematic(vdsDomainData, true)) {
-                        return true;
+                        domainsInProblem.add(vdsDomainData.getDomainId());
                     }
                     domainWhicWereSeen.add(vdsDomainData.getDomainId());
                 }
@@ -1124,9 +1127,9 @@
                 for (Guid domainId : domainsInPool) {
                     log.errorFormat("Domain {0} is not seen by Host", 
domainId);
                 }
-                return true;
+                domainsInProblem.addAll(domainsInPool);
             }
-            return false;
+            return domainsInProblem;
         }
 
         private boolean isDomainReportedAsProblematic(VDSDomainsData tempData, 
boolean isLog) {


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I32372aa80d7193a814f2500231d4545d2b1a5fc3
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: ovirt-engine-3.3
Gerrit-Owner: Tal Nisan <tni...@redhat.com>
Gerrit-Reviewer: Liron Ar <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to