Federico Simoncelli has uploaded a new change for review.

Change subject: core: move getNewMaster in ReconstructMasterDomain
......................................................................

core: move getNewMaster in ReconstructMasterDomain

Change-Id: I30d342a4c5802cdfbf2c78b50dad4c797c137fe2
Signed-off-by: Federico Simoncelli <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
5 files changed, 50 insertions(+), 41 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/44/28444/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
index 5d6560a..f11d3a6 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/DeactivateStorageDomainCommand.java
@@ -51,25 +51,8 @@
 @NonTransactiveCommandAttribute(forceCompensation = true)
 public class DeactivateStorageDomainCommand<T extends 
StorageDomainPoolParametersBase> extends
         StorageDomainCommandBase<T> {
-    private StorageDomain _newMaster;
+
     protected boolean _isLastMaster;
-    protected boolean canChooseInactiveDomainAsMaster;
-    protected boolean canChooseCurrentMasterAsNewMaster;
-
-    protected StorageDomain getNewMaster(boolean duringReconstruct) {
-        if (_newMaster == null) {
-            _newMaster = electNewMaster(duringReconstruct, 
canChooseInactiveDomainAsMaster, canChooseCurrentMasterAsNewMaster);
-        }
-        return _newMaster;
-    }
-
-    protected Guid getNewMasterId(boolean duringReconstruct) {
-        return (getNewMaster(duringReconstruct) == null) ? Guid.Empty : 
getNewMaster(duringReconstruct).getId();
-    }
-
-    protected void setNewMaster(StorageDomain value) {
-        _newMaster = value;
-    }
 
     public DeactivateStorageDomainCommand(T parameters) {
         super(parameters);
@@ -201,7 +184,11 @@
         map.setStatus(StorageDomainStatus.Unknown);
         changeStorageDomainStatusInTransaction(map,
                 getParameters().isInactive() ? StorageDomainStatus.Locked : 
StorageDomainStatus.PreparingForMaintenance);
-        proceedStorageDomainTreatmentByDomainType(false);
+
+        final StorageDomain newMaster = electNewMaster();
+        final Guid newMasterId = newMaster != null ? newMaster.getId() : 
Guid.Empty;
+
+        proceedStorageDomainTreatmentByDomainType(newMaster, true);
 
         if (_isLastMaster) {
             executeInNewTransaction(new TransactionMethod<Object>() {
@@ -222,9 +209,8 @@
         if (!getParameters().isInactive()) {
             runVdsCommand(VDSCommandType.DeactivateStorageDomain,
                     new 
DeactivateStorageDomainVDSCommandParameters(getStoragePool().getId(),
-                            getStorageDomain()
-                                    .getId(),
-                            getNewMasterId(false),
+                            getStorageDomain().getId(),
+                            newMasterId,
                             getStoragePool().getmaster_domain_version()));
         }
         freeLock();
@@ -262,7 +248,7 @@
                         public EventResult call() {
                             runSynchronizeOperation(new 
AfterDeactivateSingleAsyncOperationFactory(),
                                     _isLastMaster,
-                                    getNewMasterId(false));
+                                    newMasterId);
                             return null;
                         }
                     });
@@ -284,8 +270,8 @@
                             getStorageDomain().getId(), map.getStatus());
                 }
                 getStoragePoolIsoMapDAO().updateStatus(map.getId(), 
map.getStatus());
-                if (!Guid.Empty.equals(getNewMasterId(false))) {
-                    StoragePoolIsoMap mapOfNewMaster = 
getNewMaster(false).getStoragePoolIsoMapData();
+                if (newMaster != null) {
+                    StoragePoolIsoMap mapOfNewMaster = 
newMaster.getStoragePoolIsoMapData();
                     mapOfNewMaster.setStatus(StorageDomainStatus.Active);
                     
getStoragePoolIsoMapDAO().updateStatus(mapOfNewMaster.getId(), 
mapOfNewMaster.getStatus());
                 }
@@ -315,12 +301,11 @@
      * In case of master domain this method decide if to move master to other 
domain or move pool to maintenance (since
      * there is no master domain)
      *
-     * @param duringReconstruct If true storagePool will be saved to DB 
outside of the transaction and master domain
-     * will not be locked
+     * @param lockNewMaster If true the new master domain will not be locked
      */
-    protected void proceedStorageDomainTreatmentByDomainType(final boolean 
duringReconstruct) {
+    protected void proceedStorageDomainTreatmentByDomainType(final 
StorageDomain newMaster,
+                                                             final boolean 
lockNewMaster) {
         if (getStorageDomain().getStorageDomainType() == 
StorageDomainType.Master) {
-            final StorageDomain newMaster = getNewMaster(duringReconstruct);
             if (newMaster != null) {
                 
newMaster.getStorageStaticData().setLastTimeUsedAsMaster(System.currentTimeMillis());
 
@@ -332,7 +317,7 @@
                             StoragePoolIsoMap newMasterMap = 
newMaster.getStoragePoolIsoMapData();
                             
getCompensationContext().snapshotEntityUpdated(newMaster.getStorageStaticData());
                             
newMaster.setStorageDomainType(StorageDomainType.Master);
-                            if (!duringReconstruct) {
+                            if (!lockNewMaster) {
                                 
newMasterMap.setStatus(StorageDomainStatus.Unknown);
                                 
getCompensationContext().snapshotEntityStatus(newMasterMap);
                                 
newMaster.setStatus(StorageDomainStatus.Locked);
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
index a14d310..6a71212 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ForceRemoveStorageDomainCommand.java
@@ -89,7 +89,7 @@
 
         if (returnValue && getStorageDomain().getStorageDomainType() == 
StorageDomainType.Master
                 && getStoragePool() != null) {
-            if (electNewMaster(false) == null) {
+            if (electNewMaster() == null) {
                 returnValue = false;
                 
addCanDoActionMessage(VdcBllMessages.ERROR_CANNOT_DESTROY_LAST_STORAGE_DOMAIN);
             } else if (!initializeVds()) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
index fa8800d..69dcfb1 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/ReconstructMasterDomainCommand.java
@@ -35,6 +35,14 @@
 public class ReconstructMasterDomainCommand<T extends 
ReconstructMasterParameters> extends
         DeactivateStorageDomainCommand<T> {
 
+    protected StorageDomain _newMaster;
+
+    protected Guid _newMasterStorageDomainId = Guid.Empty;
+
+    protected boolean canChooseInactiveDomainAsMaster;
+
+    protected boolean canChooseCurrentMasterAsNewMaster;
+
     /**
      * Constructor for command creation when compensation is applied on startup
      *
@@ -49,6 +57,21 @@
         
setNewMaster(getStorageDomainDAO().get(parameters.getNewMasterDomainId()));
         canChooseInactiveDomainAsMaster = 
parameters.isCanChooseInactiveDomainAsMaster();
         canChooseCurrentMasterAsNewMaster = 
parameters.isCanChooseCurrentMasterAsNewMaster();
+    }
+
+    protected StorageDomain getNewMaster() {
+        if (_newMaster == null) {
+            _newMaster = electNewMaster(true, canChooseInactiveDomainAsMaster, 
canChooseCurrentMasterAsNewMaster);
+        }
+        return _newMaster;
+    }
+
+    protected Guid getNewMasterId() {
+        return (getNewMaster() == null) ? Guid.Empty : getNewMaster().getId();
+    }
+
+    protected void setNewMaster(StorageDomain value) {
+        _newMaster = value;
     }
 
     private boolean checkIsDomainLocked(StoragePoolIsoMap domainMap) {
@@ -93,10 +116,11 @@
 
 
     protected boolean reconstructMaster() {
-        proceedStorageDomainTreatmentByDomainType(true);
+        _newMasterStorageDomainId = getNewMaster().getId();
+        proceedStorageDomainTreatmentByDomainType(getNewMaster(), true);
 
         // To issue a reconstructMaster you need to set the domain inactive 
unless the selected domain is the current master
-        if (getParameters().isInactive() && 
!getStorageDomain().getId().equals(getNewMasterId(true))) {
+        if (getParameters().isInactive() && 
!getStorageDomain().getId().equals(getNewMasterId())) {
             executeInNewTransaction(new TransactionMethod<Void>() {
                 @Override
                 public Void runInTransaction() {
@@ -117,7 +141,7 @@
         final List<String> disconnectPoolFormats = Config.<List<String>> 
getValue(
                 ConfigValues.DisconnectPoolOnReconstruct);
 
-        if (commandSucceeded && 
disconnectPoolFormats.contains(getNewMaster(true).getStorageFormat().getValue()))
 {
+        if (commandSucceeded && 
disconnectPoolFormats.contains(getNewMaster().getStorageFormat().getValue())) {
             commandSucceeded = runVdsCommand(
                     VDSCommandType.DisconnectStoragePool,
                     new 
DisconnectStoragePoolVDSCommandParameters(getVds().getId(),
@@ -139,7 +163,7 @@
         return runVdsCommand(VDSCommandType.ReconstructMaster,
                 new ReconstructMasterVDSCommandParameters(getVds().getId(),
                         getVds().getVdsSpmId(), getStoragePool().getId(),
-                        getStoragePool().getName(), getNewMasterId(true), 
domains,
+                        getStoragePool().getName(), getNewMasterId(), domains,
                         
getStoragePool().getmaster_domain_version())).getSucceeded();
 
     }
@@ -206,7 +230,7 @@
      * @return
      */
     private boolean connectVdsToNewMaster(VDS vds) {
-        StorageDomain masterDomain = getNewMaster(true);
+        StorageDomain masterDomain = getNewMaster();
         if (vds.getId().equals(getVds().getId())
                 || 
StorageHelperDirector.getInstance().getItem(masterDomain.getStorageType())
                         .connectStorageToDomainByVdsId(masterDomain, 
vds.getId())) {
@@ -242,13 +266,13 @@
                             runVdsCommand(
                                     VDSCommandType.ConnectStoragePool,
                                     new 
ConnectStoragePoolVDSCommandParameters(vds, getStoragePool(),
-                                            getNewMasterId(true), 
storagePoolIsoMap, true));
+                                            getNewMasterId(), 
storagePoolIsoMap, true));
                         } catch (VdcBLLException ex) {
                             if (VdcBllErrors.StoragePoolUnknown == 
ex.getVdsError().getCode()) {
                                 VDSReturnValue returnVal = runVdsCommand(
                                         VDSCommandType.ConnectStoragePool,
                                         new 
ConnectStoragePoolVDSCommandParameters(vds, getStoragePool(),
-                                                getNewMasterId(true), 
storagePoolIsoMap));
+                                                getNewMasterId(), 
storagePoolIsoMap));
                                 if (!returnVal.getSucceeded()) {
                                     log.errorFormat("Post reconstruct actions 
(connectPool) did not complete on host {0} in the pool. error {1}",
                                             vds.getId(),
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
index fbed3dd..3335dab 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/RecoveryStoragePoolCommand.java
@@ -61,7 +61,7 @@
                     && getStorageDomain().getStatus() == 
StorageDomainStatus.Active) {
                 addStorageDomainStatusIllegalMessage();
                 returnValue = false;
-            } else if (electNewMaster(false) != null) {
+            } else if (electNewMaster() != null) {
                 getReturnValue().getCanDoActionMessages().add(
                         
VdcBllMessages.STORAGE_POOL_REINITIALIZE_WITH_MORE_THAN_ONE_DATA_DOMAIN.toString());
                 returnValue = false;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
index aedef3f..a20c9f8 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/storage/StorageDomainCommandBase.java
@@ -383,8 +383,8 @@
      * returns new master domain which is in Active/Unknown status
      * @return an elected master domain or null
      */
-    protected StorageDomain electNewMaster(boolean duringReconstruct) {
-        return electNewMaster(duringReconstruct, false, false);
+    protected StorageDomain electNewMaster() {
+        return electNewMaster(false, false, false);
     }
 
     @Override


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

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

Reply via email to