Liron Aravot has uploaded a new change for review.

Change subject: core: determine wether domain monitoring result is actual
......................................................................

core: determine wether domain monitoring result is actual

When the domain monitoring results are reported back from vdsm, the
first monitor run may haven't finished yet. In that case the returned
status from vdsm is a dummy status and not an actual result.
The non actual status could lead to a wrong decision making on the engine
status (for example - decide that an inactive domain is now active).

This patch uses the logic added on vdsm side (change I1fea5) to inspect
whether the reported status is actual or not.

Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab
Signed-off-by: lara...@redhat.com <lara...@redhat.com>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/InitVdsOnUpCommand.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsBrokerCommand.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
M 
backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java
8 files changed, 54 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/37/38037/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 ae9ac38..a196eb7 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
@@ -275,7 +275,7 @@
         }
 
         if (result.isSuccess()) {
-            Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = 
proceedVdsStats(!masterDomainInactiveOrUnknown);
+            Pair<Boolean, List<StorageDomainStatic>> vdsStatsResults = 
proceedVdsStats(!masterDomainInactiveOrUnknown, storagePool);
             result.setSuccess(vdsStatsResults.getFirst());
             if (!result.isSuccess()) {
                 result.setResultData(vdsStatsResults.getSecond());
@@ -302,14 +302,13 @@
         return returnValue;
     }
 
-    private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean 
shouldCheckReportedDomains) {
+    private Pair<Boolean, List<StorageDomainStatic>> proceedVdsStats(boolean 
shouldCheckReportedDomains, StoragePool storagePool) {
         Pair<Boolean, List<StorageDomainStatic>> returnValue = new 
Pair<>(true, null);
         try {
             runVdsCommand(VDSCommandType.GetStats, new 
VdsIdAndVdsVDSCommandParametersBase(getVds()));
             if (shouldCheckReportedDomains) {
                 List<Guid> problematicDomainsIds =
-                        
IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getStoragePoolId(),
-                                getVds().getDomains());
+                        
IrsBrokerCommand.fetchDomainsReportedAsProblematic(getVds().getDomains(), 
storagePool);
                 for (Guid domainId : problematicDomainsIds) {
                     StorageDomainStatic domainInfo = 
getStorageDomainStaticDAO().get(domainId);
                     log.error("Storage Domain '{}' of pool '{}' is in problem 
in host '{}'",
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
index a85ab54..dad7682 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/FeatureSupported.java
@@ -471,4 +471,12 @@
         return supportedInConfig(ConfigValues.GraphicsDeviceEnabled, version);
     }
 
+    /**
+     * @param version
+     *            Compatibility version to check for.
+     * @return <code>true</code> if data center without spm is supported for 
the given version
+     */
+    public static boolean reportWhetherDomainMonitoringResultIsActual(Version 
version) {
+        return 
supportedInConfig(ConfigValues.ReportWhetherDomainMonitoringResultIsActual, 
version);
+    }
 }
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java
index e72b64c..fc44a86 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/VDSDomainsData.java
@@ -9,6 +9,7 @@
     private Guid privateDomainId;
     private double lastCheck;
     private double delay;
+    private boolean actual;
 
     public Guid getDomainId() {
         return privateDomainId;
@@ -44,6 +45,14 @@
         return delay;
     }
 
+    public boolean isActual() {
+        return actual;
+    }
+
+    public void setActual(boolean actual) {
+        this.actual = actual;
+    }
+
     public VDSDomainsData() {
         privateDomainId = Guid.Empty;
     }
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
index a254905..0e75b73 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
@@ -2035,6 +2035,10 @@
     @DefaultValueAttribute("true")
     GraphicsDeviceEnabled,
 
+    @TypeConverterAttribute(Boolean.class)
+    @DefaultValueAttribute("true")
+    ReportWhetherDomainMonitoringResultIsActual,
+
     Invalid
 
 }
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 9a5ae6d..5ffd786 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
@@ -68,10 +68,10 @@
         }
     }
 
-    public static List<Guid> fetchDomainsReportedAsProblematic(Guid 
storagePoolId, List<VDSDomainsData> vdsDomainsData) {
-        IrsProxyData proxy = _irsProxyData.get(storagePoolId);
+    public static List<Guid> 
fetchDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, 
StoragePool storagePool) {
+        IrsProxyData proxy = _irsProxyData.get(storagePool.getId());
         if (proxy != null) {
-            return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData);
+            return proxy.obtainDomainsReportedAsProblematic(vdsDomainsData, 
storagePool);
         }
         return Collections.emptyList();
     }
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
index 95b9e92..617768b 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/irsbroker/IrsProxyData.java
@@ -17,6 +17,7 @@
 
 import org.apache.commons.lang.StringUtils;
 import org.ovirt.engine.core.common.AuditLogType;
+import org.ovirt.engine.core.common.FeatureSupported;
 import org.ovirt.engine.core.common.businessentities.AsyncTaskStatus;
 import org.ovirt.engine.core.common.businessentities.AsyncTaskStatusEnum;
 import 
org.ovirt.engine.core.common.businessentities.BusinessEntitiesDefinitions;
@@ -1111,14 +1112,14 @@
                 // Unknown domains in pool
                 for (VDSDomainsData tempData : data) {
                     if (domainsInPool.contains(tempData.getDomainId())) {
-                        DomainMonitoringResult domainMonitoringResult = 
analyzeDomainReport(tempData, false);
+                        DomainMonitoringResult domainMonitoringResult = 
analyzeDomainReport(tempData, storagePool, false);
                         if (domainMonitoringResult.invalid()) {
                             
domainsProblematicReportInfo.put(tempData.getDomainId(), 
domainMonitoringResult);
                         } else if (tempData.getDelay() > Config.<Double> 
getValue(ConfigValues.MaxStorageVdsDelayCheckSec)) {
                             logDelayedDomain(vdsId, tempData);
                         }
                     } else if 
(inActiveDomainsInPool.contains(tempData.getDomainId())
-                            && analyzeDomainReport(tempData, false).valid()) {
+                            && analyzeDomainReport(tempData, storagePool, 
false).valid()) {
                         log.warn("Storage Domain '{}' was reported by Host 
'{}' as Active in Pool '{}', moving to active status",
                                 getDomainIdTuple(tempData.getDomainId()),
                                 vdsName,
@@ -1185,7 +1186,7 @@
                 AuditLogType.VDS_DOMAIN_DELAY_INTERVAL);
     }
 
-    protected List<Guid> 
obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData) {
+    protected List<Guid> 
obtainDomainsReportedAsProblematic(List<VDSDomainsData> vdsDomainsData, 
StoragePool storagePool) {
         List<Guid> domainsInProblem = new LinkedList<>();
         Set<Guid> domainsInPool = new HashSet<Guid>(
                 DbFacade.getInstance().getStorageDomainStaticDao().getAllIds(
@@ -1195,7 +1196,7 @@
         List<Guid> domainWhichWereSeen = new ArrayList<Guid>();
         for (VDSDomainsData vdsDomainData : vdsDomainsData) {
             if (domainsInPool.contains(vdsDomainData.getDomainId())) {
-                if (analyzeDomainReport(vdsDomainData, true).invalid()) {
+                if (analyzeDomainReport(vdsDomainData, storagePool, 
true).invalid()) {
                     domainsInProblem.add(vdsDomainData.getDomainId());
                 }
                 domainWhichWereSeen.add(vdsDomainData.getDomainId());
@@ -1212,24 +1213,34 @@
     }
 
     private enum DomainMonitoringResult {
-        PROBLEMATIC(false), STORAGE_ACCCESS_ERROR(false), OK(true), 
NOT_REPORTED(false);
+        PROBLEMATIC(Boolean.FALSE), STORAGE_ACCCESS_ERROR(Boolean.FALSE), 
OK(Boolean.TRUE), NOT_REPORTED(Boolean.FALSE), NOT_ACTUAL(null);
 
-        private boolean valid;
+        private Boolean valid;
 
-        private DomainMonitoringResult(boolean valid) {
+        private DomainMonitoringResult(Boolean valid) {
             this.valid = valid;
         }
 
         public boolean valid() {
-            return valid;
+            return actual() && valid;
         }
 
         public boolean invalid() {
-            return !valid;
+            return actual() && !valid;
         }
+
+        public boolean actual() {return this != NOT_ACTUAL;}
     }
 
-    private DomainMonitoringResult analyzeDomainReport(VDSDomainsData 
tempData, boolean isLog) {
+    private DomainMonitoringResult analyzeDomainReport(VDSDomainsData 
tempData, StoragePool storagePool, boolean isLog) {
+        if (!tempData.isActual() &&
+                
FeatureSupported.reportWhetherDomainMonitoringResultIsActual(storagePool.getCompatibilityVersion()))
 {
+            log.error("Domain '{}' report isn't an actual report '{}'",
+                    getDomainIdTuple(tempData.getDomainId()),
+                    tempData.getCode());
+            return DomainMonitoringResult.NOT_ACTUAL;
+        }
+
         if (tempData.getCode() != 0) {
             if (isLog) {
                 log.error("Domain '{}' was reported with error code '{}'",
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
index 05268a8..3a76230 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsBrokerObjectsBuilder.java
@@ -1019,6 +1019,11 @@
                         delay = Double.parseDouble((String) 
internalValue.get(VdsProperties.delay));
                     }
                     data.setDelay(delay);
+                    boolean actual = true;
+                    if (internalValue.containsKey(VdsProperties.actual)) {
+                        actual = Boolean.parseBoolean((String) 
internalValue.get(VdsProperties.actual));
+                    }
+                    data.setActual(actual);
                     domainsData.add(data);
                 } catch (Exception e) {
                     log.error("failed building domains: {}", e.getMessage());
diff --git 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java
 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java
index c92b83b..0c7b25c 100644
--- 
a/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java
+++ 
b/backend/manager/modules/vdsbroker/src/main/java/org/ovirt/engine/core/vdsbroker/vdsbroker/VdsProperties.java
@@ -362,6 +362,7 @@
     public static final String code = "code";
     public static final String lastCheck = "lastCheck";
     public static final String delay = "delay";
+    public static final String actual = "actual";
 
     public static final String DISK_STATS = "diskStats";
     public static final String DISK_STATS_FREE = "free";


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Iddbba94d81694bd6cb1ce3648872eb81182a50ab
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Liron Aravot <lara...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to