minor fixes

Author: JoaoJandre

core/src/main/java/com/cloud/agent/api/RemoveBitmapCommand.java

@@ -1,3 +1,21 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
 package com.cloud.agent.api;
 
 import org.apache.cloudstack.storage.to.SnapshotObjectTO;

core/src/main/java/com/cloud/storage/resource/StorageSubsystemCommandHandlerBase.java

@@ -142,7 +142,7 @@ protected Answer execute(CreateObjectCommand cmd) {
             }
             return new CreateObjectAnswer("not supported type");
         } catch (Exception e) {
-            logger.error("Failed to create object [%s] due to [%s].", data.getObjectType(), e.getMessage(), e);
+            logger.error("Failed to create object [{}] due to [{}].", data.getObjectType(), e.getMessage(), e);
             return new CreateObjectAnswer(e.toString());
         }
     }

engine/schema/src/main/java/org/apache/cloudstack/storage/datastore/db/SnapshotDataStoreDaoImpl.java

@@ -341,7 +341,7 @@ public SnapshotDataStoreVO findParent(DataStoreRole role, Long storeId, Long zon
         sc.setParameters(STORE_ROLE, role.toString());
         sc.setParameters(STATE, ObjectInDataStoreStateMachine.State.Ready.name());
         if (storeId != null) {
-            sc.setParameters(STORE_ID, (Object) new Long[]{storeId});
+            sc.setParameters(STORE_ID, new Long[]{storeId});
         } else if (zoneId != null) {
             List<ImageStoreVO> imageStores = imageStoreDao.listStoresByZoneId(zoneId);
             Object[] imageStoreIds = imageStores.stream().map(ImageStoreVO::getId).toArray();

engine/storage/snapshot/src/main/java/org/apache/cloudstack/storage/snapshot/SnapshotServiceImpl.java

@@ -27,6 +27,7 @@
 import com.cloud.agent.api.RemoveBitmapCommand;
 import com.cloud.host.dao.HostDao;
 import com.cloud.hypervisor.Hypervisor;
+import com.cloud.storage.Volume;
 import com.cloud.storage.snapshot.SnapshotManager;
 import com.cloud.vm.VirtualMachine;
 import org.apache.cloudstack.engine.subsystem.api.storage.CopyCommandResult;
@@ -644,11 +645,15 @@ public boolean deleteSnapshot(SnapshotInfo snapInfo) {
     }
 
     protected void deleteBitmap (SnapshotInfo snapshotInfo) {
-        if (snapshotInfo.getBaseVolume() == null) {
+        Volume baseVol = snapshotInfo.getBaseVolume();
+        if (baseVol == null || !Volume.State.Ready.equals(baseVol.getState())) {
             return;
         }
+
+        VirtualMachine attachedVM = snapshotInfo.getBaseVolume().getAttachedVM();
+
         RemoveBitmapCommand cmd = new RemoveBitmapCommand((SnapshotObjectTO) snapshotInfo.getTO(),
-                snapshotInfo.getBaseVolume().getAttachedVM().getState().equals(VirtualMachine.State.Running));
+                attachedVM != null && attachedVM.getState().equals(VirtualMachine.State.Running));
         EndPoint ep = epSelector.select(snapshotInfo, StorageAction.REMOVEBITMAP);
 
         Answer answer = ep.sendMessage(cmd);

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

@@ -335,7 +335,7 @@ public class LibvirtComputingResource extends ServerResourceBase implements Serv
 
     public static final String CHECKPOINT_CREATE_COMMAND = "virsh checkpoint-create --domain %s --xmlfile %s --redefine";
 
-    public static final String CHECKPOINT_DELETE_COMMAND = "virsh checkpoint-delete --domain %s --checkpointname %s";
+    public static final String CHECKPOINT_DELETE_COMMAND = "virsh checkpoint-delete --domain %s --checkpointname %s  --metadata";
 
     private String modifyVlanPath;
     private String versionStringPath;

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

@@ -59,8 +59,9 @@ public Answer execute(ConvertSnapshotCommand command, LibvirtComputingResource s
         String secondaryStoragePoolUrl = nfsImageStore.getUrl();
 
         Set<KVMStoragePool> storagePoolSet = null;
+        KVMStoragePool secondaryStorage = null;
         try {
-            KVMStoragePool secondaryStorage = serverResource.getStoragePoolMgr().getStoragePoolByURI(secondaryStoragePoolUrl);
+            secondaryStorage = serverResource.getStoragePoolMgr().getStoragePoolByURI(secondaryStoragePoolUrl);
             storagePoolSet =  serverResource.connectToAllVolumeSnapshotSecondaryStorages(snapshotObjectTO.getVolume());
 
             String snapshotRelativePath = snapshotObjectTO.getPath();
@@ -94,6 +95,9 @@ public Answer execute(ConvertSnapshotCommand command, LibvirtComputingResource s
             logger.error(String.format("Failed to convert snapshot [%s] due to %s.", snapshotObjectTO, ex.getMessage()), ex);
             return new Answer(command, ex);
         } finally {
+            if (secondaryStorage != null) {
+                serverResource.getStoragePoolMgr().deleteStoragePool(secondaryStorage.getType(), secondaryStorage.getUuid());
+            }
             if (storagePoolSet != null) {
                 serverResource.disconnectAllVolumeSnapshotSecondaryStorages(storagePoolSet);
             }

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

@@ -1,3 +1,21 @@
+//
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+//
 package com.cloud.hypervisor.kvm.resource.wrapper;
 
 import com.cloud.agent.api.Answer;
@@ -71,7 +89,14 @@ protected Answer removeBitmapForStoppedVM(SnapshotObjectTO snapshotObjectTO, Lib
 
         logger.debug("Removing bitmap [{}] for volume [{}].", bitmap, volumeTo.getName());
 
-        qemuImg.bitmap(QemuImg.BitmapOperation.Remove, volume, bitmap);
+        try {
+            qemuImg.bitmap(QemuImg.BitmapOperation.Remove, volume, bitmap);
+        } catch (QemuImgException ex) {
+            if (!(ex.getMessage().contains("Dirty bitmap") || ex.getMessage().contains("not found"))) {
+                throw ex;
+            }
+            logger.warn("Could not delete dirty bitmap [{}] as it was not found. This will happen if the volume was migrated. If it is not the case, this should be reported.", bitmap);
+        }
         return new Answer(cmd);
     }
 

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

@@ -84,7 +84,7 @@ public Answer execute(final RevertSnapshotCommand command, final LibvirtComputin
 
         String volumePath = volume.getPath();
         String snapshotRelPath = snapshot.getPath();
-
+        KVMStoragePool secondaryStoragePool = null;
         try {
             KVMStoragePoolManager storagePoolMgr = libvirtComputingResource.getStoragePoolMgr();
 
@@ -113,7 +113,6 @@ public Answer execute(final RevertSnapshotCommand command, final LibvirtComputin
                 rbd.close(image);
                 rados.ioCtxDestroy(io);
             } else {
-                KVMStoragePool secondaryStoragePool = null;
                 if (snapshotImageStore != null && DataStoreRole.Primary != snapshotImageStore.getRole()) {
                     secondaryStoragePool = storagePoolMgr.getStoragePoolByURI(snapshotImageStore.getUrl());
                 }
@@ -142,7 +141,12 @@ public Answer execute(final RevertSnapshotCommand command, final LibvirtComputin
         } catch (RbdException e) {
             logger.error("Failed to connect to revert snapshot due to RBD exception: ", e);
             return new Answer(command, false, e.toString());
+        } finally {
+            if (secondaryStoragePool != null) {
+                libvirtComputingResource.getStoragePoolMgr().deleteStoragePool(secondaryStoragePool.getType(), secondaryStoragePool.getUuid());
+            }
         }
+
     }
 
     /**

plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java

@@ -200,8 +200,6 @@ public class KVMStorageProcessor implements StorageProcessor {
 
     private static final String DOMJOBABORT_COMMAND = "virsh domjobabort --domain %s";
 
-    private static String CHECKPOINT_DELETE_COMMAND = "virsh checkpoint-delete --domain %s --checkpointname %s";
-
     private static final String DUMMY_VM_XML = "<domain type='qemu'>\n" +
             "  <name>%s</name>\n" +
             "  <memory unit='MiB'>256</memory>\n" +
@@ -2782,6 +2780,7 @@ private KVMPhysicalDisk createVolumeFromSnapshotOnNFS(CopyCommand cmd, PrimaryDa
         KVMPhysicalDisk disk = storagePoolMgr.copyPhysicalDisk(snapshotDisk, path != null ? path : volUuid, primaryPool, cmd.getWaitInMillSeconds());
 
         storagePoolMgr.disconnectPhysicalDisk(pool.getPoolType(), pool.getUuid(), path);
+        secondaryPool.delete();
         return disk;
     }
 
@@ -2922,7 +2921,8 @@ protected void deleteCheckpoint(SnapshotObjectTO snapshotTO) throws IOException
         }
         String checkpointName = snapshotTO.getPath().substring(snapshotTO.getPath().lastIndexOf(File.separator) + 1);
 
-        Script.runSimpleBashScript(String.format(CHECKPOINT_DELETE_COMMAND, vmName, resource.getHypervisorPath(), checkpointName));
+        logger.debug("Deleting checkpoint [{}] of VM [{}].", checkpointName, vmName);
+        Script.runSimpleBashScript(String.format(LibvirtComputingResource.CHECKPOINT_DELETE_COMMAND, vmName, checkpointName));
     }
 
     /**