JoaoJandre commented on code in PR #14502:
URL: https://github.com/apache/cloudstack/pull/14502#discussion_r4388906377
##########
plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtMergeDiskOnlyVMSnapshotCommandWrapper.java:
##########
@@ -113,38 +124,83 @@ protected Answer
mergeDiskOnlySnapshotsForRunningVM(MergeDiskOnlyVmSnapshotComma
Domain domain = resource.getDomain(conn, vmName);
List<LibvirtVMDef.DiskDef> disks = resource.getDisks(conn, vmName);
KVMStoragePoolManager storageManager = resource.getStoragePoolMgr();
- QemuImg qemuImg = new QemuImg(resource.getCmdsTimeout());
+ long timeout = cmd.getWait() * 1000L;
logger.debug("Merging disk-only snapshots for running VM [{}] using
the following Snapshot Merge Trees [{}].", vmName, snapshotMergeTreeTOList);
for (SnapshotMergeTreeTO mergeTreeTO : snapshotMergeTreeTOList) {
+ if (timeout <= 0) {
+ logger.warn("Disk-only VM snapshot for VM [{}] timed out.
Consider increasing the timeout and try again later.", vmName);
+ return new Answer(cmd, false, null);
+ }
+ long startTime = System.currentTimeMillis();
DataTO childTO = mergeTreeTO.getChild();
SnapshotObjectTO parentSnapshotTO = (SnapshotObjectTO)
mergeTreeTO.getParent();
VolumeObjectTO volumeObjectTO = parentSnapshotTO.getVolume();
KVMStoragePool storagePool =
libvirtUtilitiesHelper.getPrimaryPoolFromDataTo(volumeObjectTO, storageManager);
boolean active =
DataObjectType.VOLUME.equals(childTO.getObjectType());
- String label = resource.getDiskWithPathOfVolumeObjectTO(disks,
volumeObjectTO).getDiskLabel();
- String parentSnapshotLocalPath =
storagePool.getLocalPathFor(parentSnapshotTO.getPath());
- String childDeltaPath =
storagePool.getLocalPathFor(childTO.getPath());
- logger.debug("Found label [{}] for [{}]. Will merge delta at [{}]
into delta at [{}].", label, volumeObjectTO, parentSnapshotLocalPath,
childDeltaPath);
+ LibvirtVMDef.DiskDef diskDef = getDiskDef(disks, volumeObjectTO,
childTO, parentSnapshotTO);
+ if (diskDef == null) {
+ logger.warn("Volume [{}] does not match any record we have.
This must be manually normalized.", volumeObjectTO.getUuid());
+ return new Answer(cmd, false, null);
+ }
+ String label = diskDef.getDiskLabel();
+
+ DomainBlockJobInfo jobInfo = domain.getBlockJobInfo(label, 0);
+ if (jobInfo == null || jobInfo.type == 0 && jobInfo.end == 0 &&
jobInfo.cur == 0) {
+ logger.debug("No block job found for disk [{}] of VM [{}].
Continuing the merge process.", label, vmName);
+ } else {
+ logger.debug("Found a block job for disk [{}] of VM [{}]. We
will abort it to continue with the VM snapshot merge process.", label, vmName);
+ domain.blockJobAbort(label, 0);
Review Comment:
A disk move would not be possible while the VM has VM snapshots. And I sure
hope that these types of procedures do not happen concurrently. But I get the
idea, and I agree we should be safe with this. I'll add a check for it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
