From ca84fd4ffd5d80aef9c7624365d29d7b2aeb3225 Mon Sep 17 00:00:00 2001 From: Anshul Gangwar Date: Fri, 24 Jul 2015 14:45:20 +0530 Subject: [PATCH] CLOUDSTACK-8663: Fixed various issues to allow VM snapshots and volume snapshots to exist together Reverting VM to disk only snapshot in Xenserver corrupts VM Stale NFS secondary storage on XS leads to volume creation failure from snapshot --- .../resource/CitrixResourceBase.java | 30 +++++++++++++++++++ .../resource/XenServerStorageProcessor.java | 2 +- .../Xenserver625StorageProcessor.java | 2 +- .../cloud/storage/VolumeApiServiceImpl.java | 7 ----- .../vm/snapshot/VMSnapshotManagerImpl.java | 5 ---- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java index 2865e56fd30..da465639507 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/CitrixResourceBase.java @@ -29,6 +29,8 @@ import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; import java.nio.charset.Charset; + +import org.apache.commons.collections.MapUtils; import org.joda.time.Duration; import java.util.ArrayList; import java.util.Date; @@ -1415,6 +1417,10 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe vbdr.userdevice = "autodetect"; vbdr.mode = Types.VbdMode.RW; vbdr.type = Types.VbdType.DISK; + Long deviceId = volumeTO.getDeviceId(); + if (deviceId != null && (!isDeviceUsed(conn, vm, deviceId) || deviceId > 3)) { + vbdr.userdevice = deviceId.toString(); + } VBD.create(conn, vbdr); } return vm; @@ -4350,6 +4356,30 @@ public abstract class CitrixResourceBase implements ServerResource, HypervisorRe } } + protected void skipOrRemoveSR(Connection conn, SR sr) { + if (sr == null) { + return; + } + if (s_logger.isDebugEnabled()) { + s_logger.debug(logX(sr, "Removing SR")); + } + try { + Set vdis = sr.getVDIs(conn); + for (VDI vdi : vdis) { + if (MapUtils.isEmpty(vdi.getCurrentOperations(conn))) { + continue; + } + return; + } + removeSR(conn, sr); + return; + } catch (XenAPIException | XmlRpcException e) { + s_logger.warn(logX(sr, "Unable to get current opertions " + e.toString()), e); + } + String msg = "Remove SR failed"; + s_logger.warn(msg); + } + public void removeSR(final Connection conn, final SR sr) { if (sr == null) { return; diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java index bcf997fe2ca..1144276f711 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/XenServerStorageProcessor.java @@ -1199,7 +1199,7 @@ public class XenServerStorageProcessor implements StorageProcessor { final Set snapshots = volume.getSnapshots(conn); for (final VDI snapshot : snapshots) { try { - if (!snapshot.getUuid(conn).equals(avoidSnapshotUuid) && snapshot.getSnapshotTime(conn).before(avoidSnapshot.getSnapshotTime(conn))) { + if (!snapshot.getUuid(conn).equals(avoidSnapshotUuid) && snapshot.getSnapshotTime(conn).before(avoidSnapshot.getSnapshotTime(conn)) && snapshot.getVBDs(conn).isEmpty()) { snapshot.destroy(conn); } } catch (final Exception e) { diff --git a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java index e3c3beb01bb..b70057dda9c 100644 --- a/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java +++ b/plugins/hypervisors/xenserver/src/com/cloud/hypervisor/xenserver/resource/Xenserver625StorageProcessor.java @@ -738,7 +738,7 @@ public class Xenserver625StorageProcessor extends XenServerStorageProcessor { s_logger.warn(details, e); } finally { if (srcSr != null) { - hypervisorResource.removeSR(conn, srcSr); + hypervisorResource.skipOrRemoveSR(conn, srcSr); } if (!result && destVdi != null) { try { diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index 77ecc2a0406..2a9f02d7b5c 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/server/src/com/cloud/storage/VolumeApiServiceImpl.java @@ -2146,13 +2146,6 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic throw new InvalidParameterValueException("Can't find zone by id " + volume.getDataCenterId()); } - if (volume.getInstanceId() != null) { - // Check that Vm to which this volume is attached does not have VM Snapshots - if (_vmSnapshotDao.findByVm(volume.getInstanceId()).size() > 0) { - throw new InvalidParameterValueException("Volume snapshot is not allowed, please detach it from VM with VM Snapshots"); - } - } - if (Grouping.AllocationState.Disabled == zone.getAllocationState() && !_accountMgr.isRootAdmin(caller.getId())) { throw new PermissionDeniedException("Cannot perform this operation, Zone is currently disabled: " + zone.getName()); } diff --git a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java index bb1536d2810..5e6ec1c5a7d 100644 --- a/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java +++ b/server/src/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java @@ -263,11 +263,6 @@ public class VMSnapshotManagerImpl extends MutualExclusiveIdsManagerBase impleme throw new InvalidParameterValueException("Creating VM snapshot failed due to VM:" + vmId + " is a system VM or does not exist"); } - if (_snapshotDao.listByInstanceId(vmId, Snapshot.State.BackedUp).size() > 0) { - throw new InvalidParameterValueException( - "VM snapshot for this VM is not allowed. This VM has volumes attached which has snapshots, please remove all snapshots before taking VM snapshot"); - } - // VM snapshot with memory is not supported for VGPU Vms if (snapshotMemory && _serviceOfferingDetailsDao.findDetail(userVmVo.getServiceOfferingId(), GPU.Keys.vgpuType.toString()) != null) { throw new InvalidParameterValueException("VM snapshot with MEMORY is not supported for vGPU enabled VMs.");