Arik Hadas has uploaded a new change for review.

Change subject: core: prevent possible race on add vm
......................................................................

core: prevent possible race on add vm

We insert the VM to the DB in Down state when adding VM and only after
we check if the VM should contain disks, we lock it (DB-lock, i.e
ImageLocked status).

If another command will start in between, the user will get wrong error
message. For example:

1. Add VM
2. Try to run the VM right after the VM was inserted to the DB
   (while it is in Down state)

-> If the VM is configured to boot from network and there's network
interface defined for it, the VM will boot while it doesn't have the
disks it is supposed to have. If the VM is configured to boot only from
disk, the error message will say that the VM can't boot instead of
saying that the VM is being added.

Change-Id: Id32f1939815b2cdf0da639f3e81bcdadc1940c5f
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/AddVmCommand.java
1 file changed, 5 insertions(+), 6 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/94/32494/1

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 45d44a7..3d59845 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
@@ -785,6 +785,10 @@
                     return null;
                 }
             });
+
+            if (getTaskIdList().isEmpty()) {
+                unlockVm();
+            }
         } else {
             log.errorFormat("Failed to add vm . The reasons are: {0}", 
StringUtils.join(errorMessages, ','));
         }
@@ -953,7 +957,7 @@
     void addVmDynamic() {
         VmDynamic vmDynamic = new VmDynamic();
         vmDynamic.setId(getVmId());
-        vmDynamic.setStatus(VMStatus.Down);
+        vmDynamic.setStatus(VMStatus.ImageLocked);
         vmDynamic.setVmHost("");
         vmDynamic.setVmIp("");
         vmDynamic.setVmFQDN("");
@@ -972,11 +976,6 @@
 
     protected void addVmImages() {
         if (!vmDisksSource.getDiskTemplateMap().isEmpty()) {
-            if (getVm().getStatus() != VMStatus.Down) {
-                log.error("Cannot add images. VM is not Down");
-                throw new 
VdcBLLException(VdcBllErrors.IRS_IMAGE_STATUS_ILLEGAL);
-            }
-            VmHandler.lockVm(getVmId());
             for (DiskImage image : 
getImagesToCheckDestinationStorageDomains()) {
                 VdcReturnValueBase result = runInternalActionWithTasksContext(
                         VdcActionType.CreateSnapshotFromTemplate,


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id32f1939815b2cdf0da639f3e81bcdadc1940c5f
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