From 77fc3be9db81de063e7989f3829d7bd019eac26d Mon Sep 17 00:00:00 2001 From: sureshanaparti <12028987+sureshanaparti@users.noreply.github.com> Date: Fri, 8 Oct 2021 12:50:54 +0530 Subject: [PATCH] ScaleIO/PowerFlex volume migration improvements (#119) * Report the PowerFlex/ScaleIO disk copy failure during volume migration and fail the migration. * Add the source format to conversion cmd 'qemu-img convert' when specified explicitly. * Ignore the file encryption for now, to specify source format --- .../kvm/storage/KVMStorageProcessor.java | 20 ++++-- .../kvm/storage/ScaleIOStorageAdaptor.java | 19 ++++-- .../apache/cloudstack/utils/qemu/QemuImg.java | 68 ++++++++++++++++--- 3 files changed, 86 insertions(+), 21 deletions(-) diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java index 4f1d35a0301..b8af03cde77 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java @@ -1866,14 +1866,20 @@ public class KVMStorageProcessor implements StorageProcessor { } destPool = storagePoolMgr.getStoragePool(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid()); - storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds()); + try { + storagePoolMgr.copyPhysicalDisk(volume, destVolumeName, destPool, cmd.getWaitInMillSeconds()); + } catch (Exception e) { // Any exceptions while copying the disk, should send failed answer with the error message + String errMsg = String.format("Failed to copy volume: %s to dest storage: %s, due to %s", srcVol.getName(), destPrimaryStore.getName(), e.toString()); + s_logger.debug(errMsg, e); + throw new CloudRuntimeException(errMsg); + } finally { + if (srcPrimaryStore.isManaged()) { + storagePoolMgr.disconnectPhysicalDisk(srcPrimaryStore.getPoolType(), srcPrimaryStore.getUuid(), srcVolumePath); + } - if (srcPrimaryStore.isManaged()) { - storagePoolMgr.disconnectPhysicalDisk(srcPrimaryStore.getPoolType(), srcPrimaryStore.getUuid(), srcVolumePath); - } - - if (destPrimaryStore.isManaged()) { - storagePoolMgr.disconnectPhysicalDisk(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid(), destVolumePath); + if (destPrimaryStore.isManaged()) { + storagePoolMgr.disconnectPhysicalDisk(destPrimaryStore.getPoolType(), destPrimaryStore.getUuid(), destVolumePath); + } } final VolumeObjectTO newVol = new VolumeObjectTO(); diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java index 62eb5440468..0446d06991a 100644 --- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java +++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/ScaleIOStorageAdaptor.java @@ -19,6 +19,7 @@ package com.cloud.hypervisor.kvm.storage; import java.io.File; import java.io.FileFilter; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -267,12 +268,20 @@ public class ScaleIOStorageAdaptor implements StorageAdaptor { srcFile = new QemuImgFile(disk.getPath(), disk.getFormat()); destFile = new QemuImgFile(destDisk.getPath(), destDisk.getFormat()); - LOGGER.debug("Starting copy from source image " + srcFile.getFileName() + " to PowerFlex volume: " + destDisk.getPath()); - qemu.convert(srcFile, destFile); - LOGGER.debug("Succesfully converted source image " + srcFile.getFileName() + " to PowerFlex volume: " + destDisk.getPath()); + LOGGER.debug("Starting copy from source disk image " + srcFile.getFileName() + " to PowerFlex volume: " + destDisk.getPath()); + qemu.convert(srcFile, destFile, true); + LOGGER.debug("Succesfully converted source disk image " + srcFile.getFileName() + " to PowerFlex volume: " + destDisk.getPath()); } catch (QemuImgException e) { - LOGGER.error("Failed to convert from " + srcFile.getFileName() + " to " + destFile.getFileName() + " the error was: " + e.getMessage()); - destDisk = null; + try { + Map srcInfo = qemu.info(srcFile); + LOGGER.debug("Source disk info: " + Arrays.asList(srcInfo)); + } catch (Exception ignored) { + LOGGER.warn("Unable to get info from source disk: " + disk.getName()); + } + + String errMsg = String.format("Unable to convert/copy from %s to %s, due to: %s", disk.getName(), name, ((Strings.isNullOrEmpty(e.getMessage())) ? "an unknown error" : e.getMessage())); + LOGGER.error(errMsg); + throw new CloudRuntimeException(errMsg, e); } return destDisk; diff --git a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java index 481dcdcd406..e25ce00b18b 100644 --- a/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java +++ b/plugins/hypervisors/kvm/src/main/java/org/apache/cloudstack/utils/qemu/QemuImg.java @@ -20,8 +20,8 @@ import java.util.HashMap; import java.util.Iterator; import java.util.Map; -import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.NotImplementedException; +import org.apache.commons.lang.StringUtils; import com.cloud.storage.Storage; import com.cloud.utils.script.OutputInterpreter; @@ -216,7 +216,7 @@ public class QemuImg { /** * Convert a image from source to destination * - * This method calls 'qemu-img convert' and takes two objects + * This method calls 'qemu-img convert' and takes five objects * as an argument. * * @@ -228,11 +228,13 @@ public class QemuImg { * Options for the convert. Takes a Map with key value * pairs which are passed on to qemu-img without validation. * @param snapshotName - * If it is provided, convertion uses it as parameter + * If it is provided, conversion uses it as parameter + * @param forceSourceFormat + * If true, specifies the source format in the conversion cmd * @return void */ public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, - final Map options, final String snapshotName) throws QemuImgException { + final Map options, final String snapshotName, final boolean forceSourceFormat) throws QemuImgException { Script script = new Script(_qemuImgPath, timeout); if (StringUtils.isNotBlank(snapshotName)) { String qemuPath = Script.runSimpleBashScript(getQemuImgPathScript); @@ -240,9 +242,13 @@ public class QemuImg { } script.add("convert"); - // autodetect source format. Sometime int he future we may teach KVMPhysicalDisk about more formats, then we can explicitly pass them if necessary - //s.add("-f"); - //s.add(srcFile.getFormat().toString()); + + // autodetect source format unless specified explicitly + if (forceSourceFormat) { + script.add("-f"); + script.add(srcFile.getFormat().toString()); + } + script.add("-O"); script.add(destFile.getFormat().toString()); @@ -258,8 +264,10 @@ public class QemuImg { } if (StringUtils.isNotBlank(snapshotName)) { - script.add("-f"); - script.add(srcFile.getFormat().toString()); + if (!forceSourceFormat) { + script.add("-f"); + script.add(srcFile.getFormat().toString()); + } script.add("-s"); script.add(snapshotName); } @@ -277,6 +285,29 @@ public class QemuImg { } } + /** + * Convert a image from source to destination + * + * This method calls 'qemu-img convert' and takes four objects + * as an argument. + * + * + * @param srcFile + * The source file + * @param destFile + * The destination file + * @param options + * Options for the convert. Takes a Map with key value + * pairs which are passed on to qemu-img without validation. + * @param snapshotName + * If it is provided, convertion uses it as parameter + * @return void + */ + public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, + final Map options, final String snapshotName) throws QemuImgException { + this.convert(srcFile, destFile, options, snapshotName, false); + } + /** * Convert a image from source to destination * @@ -294,6 +325,25 @@ public class QemuImg { this.convert(srcFile, destFile, null, null); } + /** + * Convert a image from source to destination + * + * This method calls 'qemu-img convert' and takes three objects + * as an argument. + * + * + * @param srcFile + * The source file + * @param destFile + * The destination file + * @param forceSourceFormat + * If true, specifies the source format in the conversion cmd + * @return void + */ + public void convert(final QemuImgFile srcFile, final QemuImgFile destFile, final boolean forceSourceFormat) throws QemuImgException { + this.convert(srcFile, destFile, null, null, forceSourceFormat); + } + /** * Convert a image from source to destination *