Arik Hadas has uploaded a new change for review.

Change subject: core: refactor ProcessDownVmCommand
......................................................................

core: refactor ProcessDownVmCommand

Minor refactoring in the newly introduced command

Change-Id: I0325e208aa9a76ad859ffff502d02e89f0001caa
Bug-Url: https://bugzilla.redhat.com/1108675
Signed-off-by: Arik Hadas <[email protected]>
---
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java
M 
backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
2 files changed, 105 insertions(+), 76 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/13/28813/1

diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java
index c2e351f..924c302 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/ProcessDownVmCommand.java
@@ -3,7 +3,6 @@
 import java.util.Collections;
 import java.util.List;
 
-import org.ovirt.engine.core.bll.context.CommandContext;
 import org.ovirt.engine.core.bll.job.ExecutionHandler;
 import org.ovirt.engine.core.bll.quota.QuotaManager;
 import org.ovirt.engine.core.bll.utils.PermissionSubject;
@@ -13,11 +12,17 @@
 import org.ovirt.engine.core.common.action.VmPoolSimpleUserParameters;
 import org.ovirt.engine.core.common.businessentities.DbUser;
 import org.ovirt.engine.core.common.businessentities.Snapshot.SnapshotType;
+import org.ovirt.engine.core.common.businessentities.VmDevice;
+import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
 import org.ovirt.engine.core.common.businessentities.VmPool;
-import org.ovirt.engine.core.common.businessentities.VmPoolMap;
 import org.ovirt.engine.core.common.businessentities.VmPoolType;
+import org.ovirt.engine.core.common.errors.VdcBllMessages;
+import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils;
+import org.ovirt.engine.core.common.utils.VmDeviceType;
 import org.ovirt.engine.core.compat.Guid;
 import org.ovirt.engine.core.dal.dbbroker.DbFacade;
+import org.ovirt.engine.core.dao.SnapshotDao;
+import org.ovirt.engine.core.dao.VmPoolDAO;
 import org.ovirt.engine.core.utils.log.Log;
 import org.ovirt.engine.core.utils.log.LogFactory;
 
@@ -33,33 +38,64 @@
 
     public ProcessDownVmCommand(T parameters) {
         super(parameters);
+        setVmId(getParameters().getId());
+    }
+
+    @Override
+    protected boolean canDoAction() {
+        if (getVm() == null) {
+            return 
failCanDoAction(VdcBllMessages.ACTION_TYPE_FAILED_VM_NOT_FOUND);
+        }
+
+        return true;
     }
 
     @Override
     protected void executeCommand() {
-        Guid vmId = getParameters().getId();
-        VmPoolMap map = 
DbFacade.getInstance().getVmPoolDao().getVmPoolMapByVmGuid(vmId);
-        List<DbUser> users = 
DbFacade.getInstance().getDbUserDao().getAllForVm(vmId);
-        // Check if this is a Vm from a Vm pool, and is attached to a user
-        if (map != null && users != null && !users.isEmpty()) {
-            VmPool pool = 
DbFacade.getInstance().getVmPoolDao().get(map.getvm_pool_id());
-            if (pool != null && pool.getVmPoolType() == VmPoolType.Automatic) {
-                // should be only one user in the collection
-                for (DbUser dbUser : users) {
-                    
Backend.getInstance().runInternalAction(VdcActionType.DetachUserFromVmFromPool,
-                            new 
VmPoolSimpleUserParameters(map.getvm_pool_id(), dbUser.getId(), vmId),
-                            
ExecutionHandler.createDefaultContexForTasks(getExecutionContext(), getLock()));
-                }
-            }
-        } else {
+        boolean removedStatelessSnapshot = detachUsers();
+        if (!removedStatelessSnapshot) {
             // If we are dealing with a prestarted Vm or a regular Vm - clean 
stateless images
             // Otherwise this was already done in 
DetachUserFromVmFromPoolCommand
-            removeVmStatelessImages(vmId,
-                    
ExecutionHandler.createDefaultContexForTasks(getExecutionContext(), getLock()));
+            removeVmStatelessImages();
         }
 
-        QuotaManager.getInstance().rollbackQuotaByVmId(vmId);
-        VmHandler.removeStatelessVmUnmanagedDevices(vmId);
+        QuotaManager.getInstance().rollbackQuotaByVmId(getVmId());
+        removeStatelessVmUnmanagedDevices();
+    }
+
+    private boolean detachUsers() {
+        // check if this is a VM from a VM pool
+        if (getVm().getVmPoolId() == null) {
+            return false;
+        }
+
+        List<DbUser> users = getDbUserDAO().getAllForVm(getVmId());
+        // check if this VM is attached to a user
+        if (users == null || users.isEmpty()) {
+            return false;
+        }
+
+        VmPool pool = getVmPoolDAO().get(getVm().getVmPoolId());
+        if (pool != null && pool.getVmPoolType() == VmPoolType.Automatic) {
+            // should be only one user in the collection
+            for (DbUser dbUser : users) {
+                
Backend.getInstance().runInternalAction(VdcActionType.DetachUserFromVmFromPool,
+                        new VmPoolSimpleUserParameters(getVm().getVmPoolId(), 
dbUser.getId(), getVmId()),
+                        
ExecutionHandler.createDefaultContexForTasks(getExecutionContext(), getLock()));
+            }
+
+            return true;
+        }
+
+        return false;
+    }
+
+    private VmPoolDAO getVmPoolDAO() {
+        return DbFacade.getInstance().getVmPoolDao();
+    }
+
+    private SnapshotDao getSnapshotDAO() {
+        return DbFacade.getInstance().getSnapshotDao();
     }
 
     @Override
@@ -67,12 +103,55 @@
         return Collections.emptyList();
     }
 
-    public static void removeVmStatelessImages(Guid vmId, CommandContext 
context) {
-        if (DbFacade.getInstance().getSnapshotDao().exists(vmId, 
SnapshotType.STATELESS)) {
-            log.infoFormat("Deleting snapshot for stateless vm {0}", vmId);
+    /**
+     * remove VMs unmanaged devices that are created during run-once or 
stateless run.
+     *
+     * @param vmId
+     */
+    private void removeStatelessVmUnmanagedDevices() {
+        if (getVm().isStateless() || isRunOnce()) {
+            final List<VmDevice> vmDevices =
+                    DbFacade.getInstance()
+                            .getVmDeviceDao()
+                            .getUnmanagedDevicesByVmId(getVmId());
+
+            for (VmDevice device : vmDevices) {
+                // do not remove device if appears in white list
+                if (!VmDeviceCommonUtils.isInWhiteList(device.getType(), 
device.getDevice())) {
+                    
DbFacade.getInstance().getVmDeviceDao().remove(device.getId());
+                }
+            }
+        }
+    }
+
+    /**
+     * This method checks if we are stopping a VM that was started by run-once 
In such case we will may have 2 devices,
+     * one managed and one unmanaged for CD or Floppy This is not supported 
currently by libvirt that allows only one
+     * CD/Floppy This code should be removed if libvirt will support in future 
multiple CD/Floppy
+     */
+    private boolean isRunOnce() {
+        List<VmDevice> cdList =
+                DbFacade.getInstance()
+                        .getVmDeviceDao()
+                        .getVmDeviceByVmIdTypeAndDevice(getVmId(),
+                                VmDeviceGeneralType.DISK,
+                                VmDeviceType.CDROM.getName());
+        List<VmDevice> floppyList =
+                DbFacade.getInstance()
+                        .getVmDeviceDao()
+                        .getVmDeviceByVmIdTypeAndDevice(getVmId(),
+                                VmDeviceGeneralType.DISK,
+                                VmDeviceType.FLOPPY.getName());
+
+        return (cdList.size() > 1 || floppyList.size() > 1);
+    }
+
+    private void removeVmStatelessImages() {
+        if (getSnapshotDAO().exists(getVmId(), SnapshotType.STATELESS)) {
+            log.infoFormat("Deleting snapshot for stateless vm {0}", 
getVmId());
             
Backend.getInstance().runInternalAction(VdcActionType.RestoreStatelessVm,
-                    new VmOperationParameterBase(vmId),
-                    context);
+                    new VmOperationParameterBase(getVmId()),
+                    
ExecutionHandler.createDefaultContexForTasks(getExecutionContext(), getLock()));
         }
     }
 }
diff --git 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
index d44769d..f2b7632 100644
--- 
a/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
+++ 
b/backend/manager/modules/bll/src/main/java/org/ovirt/engine/core/bll/VmHandler.java
@@ -34,8 +34,6 @@
 import org.ovirt.engine.core.common.businessentities.VM;
 import org.ovirt.engine.core.common.businessentities.VMStatus;
 import org.ovirt.engine.core.common.businessentities.VmBase;
-import org.ovirt.engine.core.common.businessentities.VmDevice;
-import org.ovirt.engine.core.common.businessentities.VmDeviceGeneralType;
 import org.ovirt.engine.core.common.businessentities.VmDynamic;
 import org.ovirt.engine.core.common.businessentities.VmInit;
 import org.ovirt.engine.core.common.businessentities.VmStatic;
@@ -49,8 +47,6 @@
 import org.ovirt.engine.core.common.osinfo.OsRepository;
 import org.ovirt.engine.core.common.utils.Pair;
 import org.ovirt.engine.core.common.utils.SimpleDependecyInjector;
-import org.ovirt.engine.core.common.utils.VmDeviceCommonUtils;
-import org.ovirt.engine.core.common.utils.VmDeviceType;
 import 
org.ovirt.engine.core.common.vdscommands.SetVmStatusVDSCommandParameters;
 import org.ovirt.engine.core.common.vdscommands.VDSCommandType;
 import org.ovirt.engine.core.compat.Guid;
@@ -627,52 +623,6 @@
         if (osRepository.isLinux(vmBase.getOsId()) && 
vmBase.getUsbPolicy().equals(UsbPolicy.ENABLED_LEGACY)) {
             vmBase.setUsbPolicy(UsbPolicy.DISABLED);
         }
-    }
-
-    /**
-     * remove VMs unmanaged devices that are created during run-once or 
stateless run.
-     *
-     * @param vmId
-     */
-    public static void removeStatelessVmUnmanagedDevices(Guid vmId) {
-        VM vm = DbFacade.getInstance().getVmDao().get(vmId);
-
-        if (vm != null && vm.isStateless() || isRunOnce(vmId)) {
-
-            final List<VmDevice> vmDevices =
-                    DbFacade.getInstance()
-                            .getVmDeviceDao()
-                            .getUnmanagedDevicesByVmId(vmId);
-
-            for (VmDevice device : vmDevices) {
-                // do not remove device if appears in white list
-                if (!VmDeviceCommonUtils.isInWhiteList(device.getType(), 
device.getDevice())) {
-                    
DbFacade.getInstance().getVmDeviceDao().remove(device.getId());
-                }
-            }
-        }
-    }
-
-    /**
-     * This method checks if we are stopping a VM that was started by run-once 
In such case we will may have 2 devices,
-     * one managed and one unmanaged for CD or Floppy This is not supported 
currently by libvirt that allows only one
-     * CD/Floppy This code should be removed if libvirt will support in future 
multiple CD/Floppy
-     */
-    private static boolean isRunOnce(Guid vmId) {
-        List<VmDevice> cdList =
-                DbFacade.getInstance()
-                        .getVmDeviceDao()
-                        .getVmDeviceByVmIdTypeAndDevice(vmId,
-                                VmDeviceGeneralType.DISK,
-                                VmDeviceType.CDROM.getName());
-        List<VmDevice> floppyList =
-                DbFacade.getInstance()
-                        .getVmDeviceDao()
-                        .getVmDeviceByVmIdTypeAndDevice(vmId,
-                                VmDeviceGeneralType.DISK,
-                                VmDeviceType.FLOPPY.getName());
-
-        return (cdList.size() > 1 || floppyList.size() > 1);
     }
 
     /**


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

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

Reply via email to