Arik Hadas has uploaded a new change for review.

Change subject: core: remove read lock of template being exported - cont
......................................................................

core: remove read lock of template being exported - cont

This is a complementary patch to http://gerrit.ovirt.org/#/c/13109/
which made it possible to create a VM from template, using template
provisioning of type 'clone', while the template is being exported.

This patch makes it possible to create a VM from template, using
template provisioning of type 'thin', while the template is being
exported as well.

The dontCheckTemplateImages field is removed from
VmManagementParameterBase as it was set by all the flows that add VM
besides invoking AddVmCommand directly. now that AddVmCommand doesn't
need to check the template images as well, it becomes redundant.

Instead of checking the template images, the AddVmCommand takes shared
locks for the template and its disks, in a similar way to what was done
in http://gerrit.ovirt.org/#/c/13109/ for AddVmFromTemplateCommand.

Change-Id: I2580bc130e3b9565c8b9f515984c0180a633e3c3
Bug-Url: https://bugzilla.redhat.com/920150
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndAttachToPoolCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmFromSnapshotParameters.java
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
7 files changed, 59 insertions(+), 69 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/60/15960/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndAttachToPoolCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndAttachToPoolCommand.java
index 3449927..02b8f45 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndAttachToPoolCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndAttachToPoolCommand.java
@@ -1,5 +1,7 @@
 package org.ovirt.engine.core.bll;
 
+import java.util.Map;
+
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.action.AddVmAndAttachToPoolParameters;
 import org.ovirt.engine.core.common.action.AddVmFromScratchParameters;
@@ -8,6 +10,7 @@
 import org.ovirt.engine.core.common.action.VdcReturnValueBase;
 import org.ovirt.engine.core.common.action.VmManagementParametersBase;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
+import org.ovirt.engine.core.common.utils.Pair;
 
 @InternalCommandAttribute
 @NonTransactiveCommandAttribute
@@ -36,6 +39,11 @@
         }
     }
 
+    @Override
+    protected Map<String, Pair<String, String>> getSharedLocks() {
+        return null;
+    }
+
     private VdcReturnValueBase addVmFromScratch(VmStatic vmStatic) {
         AddVmFromScratchParameters parameters = new 
AddVmFromScratchParameters(vmStatic, getParameters()
                 .getDiskInfoList(), getParameters().getStorageDomainId());
@@ -50,7 +58,6 @@
     private VdcReturnValueBase addVm(VmStatic vmStatic) {
         VmManagementParametersBase parameters = new 
VmManagementParametersBase(vmStatic);
         parameters.setSessionId(getParameters().getSessionId());
-        parameters.setDontCheckTemplateImages(true);
         parameters.setDontAttachToDefaultTag(true);
         parameters.setDiskInfoDestinationMap(diskInfoDestinationMap);
 
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
index 48fba43..e296528 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmAndCloneImageCommand.java
@@ -21,6 +21,7 @@
 import org.ovirt.engine.core.common.businessentities.StorageDomain;
 import org.ovirt.engine.core.common.errors.VdcBLLException;
 import org.ovirt.engine.core.common.errors.VdcBllErrors;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
 
@@ -45,6 +46,11 @@
         return true;
     }
 
+    @Override
+    protected Map<String, Pair<String, String>> getSharedLocks() {
+        return null;
+    }
+
     protected void copyDiskImage(
             DiskImage diskImage,
             Guid srcStorageDomainId,
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
index 627f89e..81f3a60 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
@@ -70,7 +70,7 @@
 
 
 @DisableInPrepareMode
-@LockIdNameAttribute
+@LockIdNameAttribute(isReleaseAtEndOfExecute = false)
 @NonTransactiveCommandAttribute
 public class AddVmCommand<T extends VmManagementParametersBase> extends 
VmManagementCommandBase<T>
         implements QuotaStorageDependent, QuotaVdsDependent {
@@ -78,6 +78,7 @@
     protected HashMap<Guid, DiskImage> diskInfoDestinationMap;
     protected Map<Guid, StorageDomain> destStorages = new HashMap<Guid, 
StorageDomain>();
     protected Map<Guid, List<DiskImage>> storageToDisksMap;
+    private String cachedDiskSharedLockMessage;
 
     /**
      * A list of the new disk images which were saved for the VM.
@@ -106,6 +107,33 @@
 
     protected AddVmCommand(Guid commandId) {
         super(commandId);
+    }
+
+    @Override
+    protected Map<String, Pair<String, String>> getSharedLocks() {
+        Map<String, Pair<String, String>> locks = new HashMap<String, 
Pair<String, String>>();
+        locks.put(getVmTemplateId().toString(),
+                LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, 
getTemplateSharedLockMessage()));
+        for (DiskImage image: getImagesToCheckDestinationStorageDomains()) {
+            locks.put(image.getId().toString(),
+                    LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK, 
getDiskSharedLockMessage()));
+        }
+        return locks;
+    }
+
+    private String getTemplateSharedLockMessage() {
+        return new 
StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_IS_USED_FOR_CREATE_VM.name())
+                .append(String.format("$VmName %1$s", getVmName()))
+                .toString();
+    }
+
+    private String getDiskSharedLockMessage() {
+        if (cachedDiskSharedLockMessage == null) {
+            cachedDiskSharedLockMessage = new 
StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM.name())
+            .append(String.format("$VmName %1$s", getVmName()))
+            .toString();
+        }
+        return cachedDiskSharedLockMessage;
     }
 
     protected void initStoragePoolId() {
@@ -446,16 +474,6 @@
 
         if (!verifyAddVM(reasons, vmPriority)) {
             return false;
-        }
-
-        if (!getParameters().getDontCheckTemplateImages()) {
-            boolean checkTemplateLock = getParameters().getParentCommand() != 
VdcActionType.AddVmPoolWithVms;
-            for (StorageDomain storage : destStorages.values()) {
-                if 
(!VmTemplateCommand.isVmTemplateImagesReady(getVmTemplate(), storage.getId(),
-                        reasons, false, checkTemplateLock, true, true, 
storageToDisksMap.get(storage.getId()))) {
-                    return false;
-                }
-            }
         }
 
         return true;
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
index 4862051..420f8e6 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromScratchCommand.java
@@ -2,6 +2,7 @@
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Map;
 
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.bll.utils.PermissionSubject;
@@ -18,6 +19,7 @@
 import org.ovirt.engine.core.common.errors.VdcBLLException;
 import org.ovirt.engine.core.common.errors.VdcBllErrors;
 import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.compat.NGuid;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
@@ -30,7 +32,6 @@
 public class AddVmFromScratchCommand<T extends AddVmFromScratchParameters> 
extends AddVmCommand<T> {
     public AddVmFromScratchCommand(T parameters) {
         super(parameters);
-        getParameters().setDontCheckTemplateImages(true);
     }
 
     protected AddVmFromScratchCommand(Guid commandId) {
@@ -38,6 +39,11 @@
     }
 
     @Override
+    protected Map<String, Pair<String, String>> getSharedLocks() {
+        return null;
+    }
+
+    @Override
     public NGuid getStorageDomainId() {
         NGuid storageDomainId = super.getStorageDomainId();
         if (Guid.Empty.equals(storageDomainId) || storageDomainId == null) {
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
index ffbb71b..185f817 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmFromTemplateCommand.java
@@ -1,8 +1,6 @@
 package org.ovirt.engine.core.bll;
 
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.common.action.AddVmFromTemplateParameters;
@@ -14,20 +12,14 @@
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.errors.VdcBLLException;
 import org.ovirt.engine.core.common.errors.VdcBllErrors;
-import org.ovirt.engine.core.common.errors.VdcBllMessages;
-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.dal.dbbroker.DbFacade;
 
 @LockIdNameAttribute(isReleaseAtEndOfExecute = false)
 public class AddVmFromTemplateCommand<T extends AddVmFromTemplateParameters> 
extends AddVmCommand<T> {
 
-    private String cachedDiskSharedLockMessage;
-
     public AddVmFromTemplateCommand(T parameters) {
         super(parameters);
-        parameters.setDontCheckTemplateImages(true);
     }
 
     protected AddVmFromTemplateCommand(Guid commandId) {
@@ -37,33 +29,6 @@
     @Override
     protected boolean validateIsImagesOnDomains() {
         return true;
-    }
-
-    @Override
-    protected Map<String, Pair<String, String>> getSharedLocks() {
-        Map<String, Pair<String, String>> locks = new HashMap<String, 
Pair<String, String>>();
-        locks.put(getVmTemplateId().toString(),
-                LockMessagesMatchUtil.makeLockingPair(LockingGroup.TEMPLATE, 
getTemplateSharedLockMessage()));
-        for (DiskImage image: getImagesToCheckDestinationStorageDomains()) {
-            locks.put(image.getId().toString(),
-                    LockMessagesMatchUtil.makeLockingPair(LockingGroup.DISK, 
getDiskSharedLockMessage()));
-        }
-        return locks;
-    }
-
-    private String getTemplateSharedLockMessage() {
-        return new 
StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_TEMPLATE_IS_USED_FOR_CREATE_VM.name())
-                .append(String.format("$VmName %1$s", getVmName()))
-                .toString();
-    }
-
-    private String getDiskSharedLockMessage() {
-        if (cachedDiskSharedLockMessage == null) {
-            cachedDiskSharedLockMessage = new 
StringBuilder(VdcBllMessages.ACTION_TYPE_FAILED_DISK_IS_USED_FOR_CREATE_VM.name())
-            .append(String.format("$VmName %1$s", getVmName()))
-            .toString();
-        }
-        return cachedDiskSharedLockMessage;
     }
 
     @Override
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmFromSnapshotParameters.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmFromSnapshotParameters.java
index 1d1a96e..1816009 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmFromSnapshotParameters.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/AddVmFromSnapshotParameters.java
@@ -17,6 +17,15 @@
     @NotNull(message="VALIDATION.SOURCE_SNAPSHOT_ID.NOT_NULL")
     private Guid sourceSnapshotId;
 
+    public AddVmFromSnapshotParameters() {
+    }
+
+    public AddVmFromSnapshotParameters(VmStatic vmStatic, Guid 
sourceSnapshotId) {
+        super(vmStatic);
+        setVmId(Guid.Empty);
+        this.sourceSnapshotId = sourceSnapshotId;
+    }
+
     public Guid getSourceSnapshotId() {
         return sourceSnapshotId;
     }
@@ -24,16 +33,4 @@
     public void setSourceSnapshotId(Guid sourceSnapshotId) {
         this.sourceSnapshotId = sourceSnapshotId;
     }
-
-    public AddVmFromSnapshotParameters() {
-        setDontCheckTemplateImages(true);
-    }
-
-    public AddVmFromSnapshotParameters(VmStatic vmStatic, Guid 
sourceSnapshotId) {
-        super(vmStatic);
-        setVmId(Guid.Empty);
-        this.sourceSnapshotId = sourceSnapshotId;
-        setDontCheckTemplateImages(true);
-    }
-
 }
diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
index 28c53b8..b79a5a7 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/action/VmManagementParametersBase.java
@@ -19,7 +19,6 @@
     private VmStatic _vmStatic;
     private boolean makeCreatorExplicitOwner;
     private Guid privateStorageDomainId = Guid.Empty;
-    private boolean privateDontCheckTemplateImages;
     private HashMap<Guid, DiskImage> diskInfoDestinationMap;
     private VmPayload payload;
     private boolean clearPayload;
@@ -59,14 +58,6 @@
 
     public void setStorageDomainId(Guid value) {
         privateStorageDomainId = value;
-    }
-
-    public boolean getDontCheckTemplateImages() {
-        return privateDontCheckTemplateImages;
-    }
-
-    public void setDontCheckTemplateImages(boolean value) {
-        privateDontCheckTemplateImages = value;
     }
 
     private boolean privateDontAttachToDefaultTag;


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

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

Reply via email to