fix merge witthout events and parent search

Author: JoaoJandre

engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDao.java

@@ -37,6 +37,8 @@ public interface VMSnapshotDao extends GenericDao<VMSnapshotVO, Long>, StateDao<
 
     List<VMSnapshotVO> listByParent(Long vmSnapshotId);
 
+    List<VMSnapshotVO> listByParentAndStateIn(Long vmSnapshotId, VMSnapshot.State... states);
+
     VMSnapshotVO findByName(Long vmId, String name);
 
     List<VMSnapshotVO> listByAccountId(Long accountId);

engine/schema/src/main/java/com/cloud/vm/snapshot/dao/VMSnapshotDaoImpl.java

@@ -42,6 +42,12 @@ public class VMSnapshotDaoImpl extends GenericDaoBase<VMSnapshotVO, Long> implem
     private final SearchBuilder<VMSnapshotVO> SnapshotStatusSearch;
     private final SearchBuilder<VMSnapshotVO> AllFieldsSearch;
 
+    private SearchBuilder<VMSnapshotVO> parentIdEqAndStateIn;
+
+    private static final String PARENT = "parent";
+
+    private static final String STATE = "state";
+
     protected VMSnapshotDaoImpl() {
         AllFieldsSearch = createSearchBuilder();
         AllFieldsSearch.and("state", AllFieldsSearch.entity().getState(), Op.EQ);
@@ -71,6 +77,11 @@ protected VMSnapshotDaoImpl() {
         SnapshotStatusSearch.and("vm_id", SnapshotStatusSearch.entity().getVmId(), SearchCriteria.Op.EQ);
         SnapshotStatusSearch.and("state", SnapshotStatusSearch.entity().getState(), SearchCriteria.Op.IN);
         SnapshotStatusSearch.done();
+
+        parentIdEqAndStateIn = createSearchBuilder();
+        parentIdEqAndStateIn.and(PARENT, parentIdEqAndStateIn.entity().getParent(), Op.EQ);
+        parentIdEqAndStateIn.and(STATE, parentIdEqAndStateIn.entity().getState(), Op.IN);
+        parentIdEqAndStateIn.done();
     }
 
     @Override
@@ -119,6 +130,14 @@ public List<VMSnapshotVO> listByParent(Long vmSnapshotId) {
         return listBy(sc, null);
     }
 
+    @Override
+    public List<VMSnapshotVO> listByParentAndStateIn(Long vmSnapshotId, State... states) {
+        SearchCriteria<VMSnapshotVO> sc = parentIdEqAndStateIn.create();
+        sc.setParameters(PARENT, vmSnapshotId);
+        sc.setParameters(STATE, (Object[])states);
+        return listBy(sc, null);
+    }
+
     @Override
     public VMSnapshotVO findByName(Long vmId, String name) {
         SearchCriteria<VMSnapshotVO> sc = AllFieldsSearch.create();

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/vmsnapshot/KvmFileBasedStorageVmSnapshotStrategy.java

@@ -123,7 +123,7 @@ public boolean deleteVMSnapshot(VMSnapshot vmSnapshot) {
         transitStateWithoutThrow(vmSnapshotBeingDeleted, VMSnapshot.Event.ExpungeRequested);
 
         List<VolumeObjectTO> volumeTOs = vmSnapshotHelper.getVolumeTOList(vmSnapshotBeingDeleted.getVmId());
-        List<VMSnapshotVO> snapshotChildren = vmSnapshotDao.listByParent(vmSnapshotBeingDeleted.getId());
+        List<VMSnapshotVO> snapshotChildren = vmSnapshotDao.listByParentAndStateIn(vmSnapshotBeingDeleted.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
 
         long realSize = getVMSnapshotRealSize(vmSnapshotBeingDeleted);
         int numberOfChildren = snapshotChildren.size();
@@ -248,10 +248,10 @@ private void mergeOldSiblingWithOldParentIfOldParentIsDead(VMSnapshotVO oldParen
         if (oldParent.getCurrent()) {
             snapshotVos = mergeCurrentDeltaOnSnapshot(oldParent, userVm, hostId, volumeTOs);
         } else {
-            List<VMSnapshotVO> oldSiblings = vmSnapshotDao.listByParent(oldParent.getId());
+            List<VMSnapshotVO> oldSiblings = vmSnapshotDao.listByParentAndStateIn(oldParent.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
 
             if (oldSiblings.size() > 1) {
-                logger.debug("The old snapshot [{}] is dead and still has more than one live snapshot. We will keep it on storage still.", oldParent.getUuid());
+                logger.debug("The old snapshot [{}] is dead and still has more than one live child snapshot. We will keep it on storage still.", oldParent.getUuid());
                 return;
             }
 
@@ -366,7 +366,7 @@ private List<SnapshotVO> deleteSnapshot(VMSnapshotVO vmSnapshotVO, Long hostId)
     private List<SnapshotVO> mergeSnapshots(VMSnapshotVO vmSnapshotVO, VMSnapshotVO childSnapshot, UserVmVO userVm, List<VolumeObjectTO> volumeObjectTOS, Long hostId) {
         logger.debug("Merging VM snapshot [{}] with its child [{}].", vmSnapshotVO.getUuid(), childSnapshot.getUuid());
 
-        List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParent(childSnapshot.getId());
+        List<VMSnapshotVO> snapshotGrandChildren = vmSnapshotDao.listByParentAndStateIn(childSnapshot.getId(), VMSnapshot.State.Ready, VMSnapshot.State.Hidden);
 
         if (userVm.getState().equals(VirtualMachine.State.Running) && !snapshotGrandChildren.isEmpty()) {
             logger.debug("Removing VM snapshots that are part of the VM's [{}] current backing chain from the list of snapshots to be rebased.", userVm.getUuid());

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/LibvirtComputingResource.java

@@ -571,9 +571,8 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
      * 1st parameter: VM's name;<br>
      * 2nd parameter: disk's label (target.dev tag from VM's XML);<br>
      * 3rd parameter: the absolute path of the base file;
-     * 4th parameter: the flag '--delete', if Libvirt supports it. Libvirt started to support it on version <b>6.0.0</b>;
      */
-    private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s --active --wait %s --pivot";
+    private static final String COMMAND_MERGE_SNAPSHOT = "virsh blockcommit %s %s --base %s";
 
     public long getHypervisorLibvirtVersion() {
         return hypervisorLibvirtVersion;
@@ -5878,7 +5877,7 @@ public void mergeSnapshotIntoBaseFile(Domain vm, String diskLabel, String baseFi
         if (AgentPropertiesFileHandler.getPropertyValue(AgentProperties.LIBVIRT_EVENTS_ENABLED)) {
             mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(vm, diskLabel, baseFilePath, topFilePath, active, snapshotName, volume, conn);
         } else {
-            mergeSnapshotIntoBaseFileWithoutEvents(vm, diskLabel, baseFilePath, snapshotName, volume, conn);
+            mergeSnapshotIntoBaseFileWithoutEvents(vm, diskLabel, baseFilePath, topFilePath, active, snapshotName, volume, conn);
         }
     }
 
@@ -5949,10 +5948,10 @@ protected void mergeSnapshotIntoBaseFileWithEventsAndConfigurableTimeout(Domain
      * @param snapshotName Name of the snapshot;
      * @throws LibvirtException
      */
-    protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabel, String baseFilePath, String snapshotName, VolumeObjectTO volume, Connect conn) throws LibvirtException {
+    protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabel, String baseFilePath, String topFilePath, boolean active, String snapshotName, VolumeObjectTO volume, Connect conn) throws LibvirtException {
         boolean isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit = LibvirtUtilitiesHelper.isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit(conn);
         String vmName = vm.getName();
-        String mergeCommand = String.format(COMMAND_MERGE_SNAPSHOT, vmName, diskLabel, baseFilePath, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit ? "--delete" : "");
+        String mergeCommand = buildMergeCommand(vmName, diskLabel, baseFilePath, topFilePath, active, isLibvirtSupportingFlagDeleteOnCommandVirshBlockcommit);
         String mergeResult = Script.runSimpleBashScript(mergeCommand);
 
         if (mergeResult == null) {
@@ -5969,6 +5968,22 @@ protected void mergeSnapshotIntoBaseFileWithoutEvents(Domain vm, String diskLabe
         throw new CloudRuntimeException(errorMsg);
     }
 
+    protected String buildMergeCommand(String vmName, String diskLabel, String baseFilePath, String topFilePath, boolean active, boolean delete) {
+        StringBuilder cmd = new StringBuilder(COMMAND_MERGE_SNAPSHOT);
+        if (StringUtils.isNotEmpty(topFilePath)) {
+            cmd.append(" --top ");
+            cmd.append(topFilePath);
+        }
+        if (active) {
+            cmd.append(" --active --pivot");
+        }
+        if (delete) {
+            cmd.append(" --delete");
+        }
+        cmd.append(" --wait");
+        return String.format(cmd.toString(), vmName, diskLabel, baseFilePath);
+    }
+
     /**
      * This was created to facilitate testing.
      * */