Snapshot deletion issues (#3969)

* Fixes snapshot deletion

* Remove legacy '@Component', it is not necessary in this bean/class.

* Fix log message missing %d and remove snapshot on DB

* Remove "dummy" boolean return statement

* Manage snapshot deletion for KVM + NFS (primary storage)

* checkstyle trailing spaces

* rename options strings to *_OPTION

* Fix typo on deleteSnapshotOnSecondaryStorage and enhance log message

* Move the snapshotDao.remove(snapshotId); (#4006)

* Fix deletesnapshot worflow to handle both snapshots created in primary storage and snapshots backed up to secondary storage

* Fix extra space

* refactor out separate handling methods for secondary and primary (reducing returns)

* return false on unexpected error or log when expected

* != instead of ==

* secondary instead of backup storage

* init to null

* Handle snapshot deletion on primary storage. When primary store ref not found for snapshot do not fail the operation.

* Fix debug levels on log messages

Co-authored-by: GabrielBrascher <gabriel@apache.org>
Co-authored-by: Andrija Panic <45762285+andrijapanicsb@users.noreply.github.com>
Co-authored-by: Harikrishna Patnala <harikrishna.patnala@gmail.com>
Co-authored-by: nvazquez <nicovazquez90@gmail.com>
This commit is contained in:
dahn 2020-04-11 16:40:27 +02:00 committed by GitHub
parent e0b67a4c68
commit f18fe5e1da
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 169 additions and 83 deletions

View File

@ -49,7 +49,7 @@
<bean id="dataStoreProviderManagerImpl" class="org.apache.cloudstack.storage.datastore.provider.DataStoreProviderManagerImpl" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
<bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
<bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />

View File

@ -50,7 +50,7 @@
<bean id="ancientDataMotionStrategy" class="org.apache.cloudstack.storage.motion.AncientDataMotionStrategy" />
<bean id="storageCacheManagerImpl" class="org.apache.cloudstack.storage.cache.manager.StorageCacheManagerImpl" />
<bean id="storageCacheRandomAllocator" class="org.apache.cloudstack.storage.cache.allocator.StorageCacheRandomAllocator" />
<bean id="xenserverSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
<bean id="defaultSnapshotStrategy" class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="bAREMETAL" class="org.apache.cloudstack.storage.image.format.BAREMETAL" />
<bean id="dataMotionServiceImpl" class="org.apache.cloudstack.storage.motion.DataMotionServiceImpl" />
<bean id="dataObjectManagerImpl" class="org.apache.cloudstack.storage.datastore.DataObjectManagerImpl" />

View File

@ -21,7 +21,6 @@ import java.util.List;
import javax.inject.Inject;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
import org.apache.cloudstack.engine.subsystem.api.storage.ObjectInDataStoreStateMachine.Event;
@ -34,7 +33,6 @@ import org.apache.cloudstack.engine.subsystem.api.storage.StrategyPriority;
import org.apache.cloudstack.engine.subsystem.api.storage.VolumeInfo;
import org.apache.cloudstack.framework.config.dao.ConfigurationDao;
import org.apache.cloudstack.framework.jobs.AsyncJob;
import org.apache.cloudstack.framework.jobs.dao.SyncQueueItemDao;
import org.apache.cloudstack.storage.command.CreateObjectAnswer;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreDao;
import org.apache.cloudstack.storage.datastore.db.SnapshotDataStoreVO;
@ -71,9 +69,8 @@ import com.cloud.utils.db.DB;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
@Component
public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
private static final Logger s_logger = Logger.getLogger(XenserverSnapshotStrategy.class);
public class DefaultSnapshotStrategy extends SnapshotStrategyBase {
private static final Logger s_logger = Logger.getLogger(DefaultSnapshotStrategy.class);
@Inject
SnapshotService snapshotSvr;
@ -90,12 +87,8 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
@Inject
SnapshotDataFactory snapshotDataFactory;
@Inject
private SnapshotDao _snapshotDao;
@Inject
private SnapshotDetailsDao _snapshotDetailsDao;
@Inject
private SyncQueueItemDao _syncQueueItemDao;
@Inject
VolumeDetailsDao _volumeDetailsDaoImpl;
@Override
@ -264,68 +257,147 @@ public class XenserverSnapshotStrategy extends SnapshotStrategyBase {
return true;
}
if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) && !Snapshot.State.Error.equals(snapshotVO.getState()) &&
if (!Snapshot.State.BackedUp.equals(snapshotVO.getState()) &&
!Snapshot.State.Destroying.equals(snapshotVO.getState())) {
throw new InvalidParameterValueException("Can't delete snapshotshot " + snapshotId + " due to it is in " + snapshotVO.getState() + " Status");
}
// first mark the snapshot as destroyed, so that ui can't see it, but we
// may not destroy the snapshot on the storage, as other snapshots may
// depend on it.
Boolean deletedOnSecondary = deleteOnSecondaryIfNeeded(snapshotId);
boolean deletedOnPrimary = deleteOnPrimaryIfNeeded(snapshotId);
if (deletedOnPrimary) {
s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on primary storage.", snapshotId));
} else {
s_logger.debug(String.format("The snapshot (id: %d) could not be found/deleted on primary storage.", snapshotId));
}
if (null != deletedOnSecondary && deletedOnSecondary) {
s_logger.debug(String.format("Successfully deleted snapshot (id: %d) on secondary storage.", snapshotId));
}
return (deletedOnSecondary != null) && deletedOnSecondary || deletedOnPrimary;
}
private boolean deleteOnPrimaryIfNeeded(Long snapshotId) {
SnapshotVO snapshotVO;
boolean deletedOnPrimary = false;
snapshotVO = snapshotDao.findById(snapshotId);
SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
if (snapshotVO != null && snapshotVO.getState() == Snapshot.State.Destroyed) {
deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId, snapshotOnPrimaryInfo);
} else {
// Here we handle snapshots which are to be deleted but are not marked as destroyed yet.
// This may occur for instance when they are created only on primary because
// snapshot.backup.to.secondary was set to false.
if (snapshotOnPrimaryInfo == null) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Snapshot (id: %d) not found on primary storage, skipping snapshot deletion on primary and marking it destroyed", snapshotId));
}
snapshotVO.setState(Snapshot.State.Destroyed);
snapshotDao.update(snapshotId, snapshotVO);
deletedOnPrimary = true;
} else {
SnapshotObject obj = (SnapshotObject) snapshotOnPrimaryInfo;
try {
obj.processEvent(Snapshot.Event.DestroyRequested);
deletedOnPrimary = deleteSnapshotOnPrimary(snapshotId, snapshotOnPrimaryInfo);
if (!deletedOnPrimary) {
obj.processEvent(Snapshot.Event.OperationFailed);
} else {
obj.processEvent(Snapshot.Event.OperationSucceeded);
}
} catch (NoTransitionException e) {
s_logger.debug("Failed to set the state to destroying: ", e);
deletedOnPrimary = false;
}
}
}
return deletedOnPrimary;
}
private Boolean deleteOnSecondaryIfNeeded(Long snapshotId) {
Boolean deletedOnSecondary = null;
SnapshotInfo snapshotOnImage = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Image);
if (snapshotOnImage == null) {
s_logger.debug("Can't find snapshot on backup storage, delete it in db");
snapshotDao.remove(snapshotId);
return true;
}
SnapshotObject obj = (SnapshotObject)snapshotOnImage;
try {
obj.processEvent(Snapshot.Event.DestroyRequested);
List<VolumeDetailVO> volumesFromSnapshot;
volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
if (volumesFromSnapshot.size() > 0) {
try {
obj.processEvent(Snapshot.Event.OperationFailed);
} catch (NoTransitionException e1) {
s_logger.debug("Failed to change snapshot state: " + e1.toString());
s_logger.debug(String.format("Can't find snapshot [snapshot id: %d] on secondary storage", snapshotId));
} else {
SnapshotObject obj = (SnapshotObject)snapshotOnImage;
try {
deletedOnSecondary = deleteSnapshotOnSecondaryStorage(snapshotId, snapshotOnImage, obj);
if (!deletedOnSecondary) {
s_logger.debug(
String.format("Failed to find/delete snapshot (id: %d) on secondary storage. Still necessary to check and delete snapshot on primary storage.",
snapshotId));
} else {
s_logger.debug(String.format("Snapshot (id: %d) has been deleted on secondary storage.", snapshotId));
}
throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
} catch (NoTransitionException e) {
s_logger.debug("Failed to set the state to destroying: ", e);
// deletedOnSecondary remain null
}
} catch (NoTransitionException e) {
s_logger.debug("Failed to set the state to destroying: ", e);
return false;
}
return deletedOnSecondary;
}
try {
boolean result = deleteSnapshotChain(snapshotOnImage);
obj.processEvent(Snapshot.Event.OperationSucceeded);
if (result) {
//snapshot is deleted on backup storage, need to delete it on primary storage
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
if (snapshotOnPrimary != null) {
SnapshotInfo snapshotOnPrimaryInfo = snapshotDataFactory.getSnapshot(snapshotId, DataStoreRole.Primary);
long volumeId = snapshotOnPrimary.getVolumeId();
VolumeVO volumeVO = volumeDao.findById(volumeId);
if (((PrimaryDataStoreImpl)snapshotOnPrimaryInfo.getDataStore()).getPoolType() == StoragePoolType.RBD && volumeVO != null) {
snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo);
}
snapshotOnPrimary.setState(State.Destroyed);
snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
}
}
} catch (Exception e) {
s_logger.debug("Failed to delete snapshot: ", e);
/**
* Deletes the snapshot on secondary storage.
* It can return false when the snapshot was stored on primary storage and not backed up on secondary; therefore, the snapshot should also be deleted on primary storage even when this method returns false.
*/
private boolean deleteSnapshotOnSecondaryStorage(Long snapshotId, SnapshotInfo snapshotOnImage, SnapshotObject obj) throws NoTransitionException {
obj.processEvent(Snapshot.Event.DestroyRequested);
List<VolumeDetailVO> volumesFromSnapshot;
volumesFromSnapshot = _volumeDetailsDaoImpl.findDetails("SNAPSHOT_ID", String.valueOf(snapshotId), null);
if (volumesFromSnapshot.size() > 0) {
try {
obj.processEvent(Snapshot.Event.OperationFailed);
} catch (NoTransitionException e1) {
s_logger.debug("Failed to change snapshot state: " + e.toString());
s_logger.debug("Failed to change snapshot state: " + e1.toString());
}
return false;
throw new InvalidParameterValueException("Unable to perform delete operation, Snapshot with id: " + snapshotId + " is in use ");
}
return true;
boolean result = deleteSnapshotChain(snapshotOnImage);
obj.processEvent(Snapshot.Event.OperationSucceeded);
return result;
}
/**
* Deletes the snapshot on primary storage. It returns true when the snapshot was not found on primary storage; </br>
* In case of failure while deleting the snapshot, it will throw one of the following exceptions: CloudRuntimeException, InterruptedException, or ExecutionException. </br>
*/
private boolean deleteSnapshotOnPrimary(Long snapshotId, SnapshotInfo snapshotOnPrimaryInfo) {
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
if (isSnapshotOnPrimaryStorage(snapshotId)) {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Snapshot reference is found on primary storage for snapshot id: %d, performing snapshot deletion on primary", snapshotId));
}
if (snapshotSvr.deleteSnapshot(snapshotOnPrimaryInfo)) {
snapshotOnPrimary.setState(State.Destroyed);
snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Successfully deleted snapshot id: %d on primary storage", snapshotId));
}
return true;
}
} else {
if (s_logger.isDebugEnabled()) {
s_logger.debug(String.format("Snapshot reference is not found on primary storage for snapshot id: %d, skipping snapshot deletion on primary", snapshotId));
}
return true;
}
return false;
}
/**
* Returns true if the snapshot volume is on primary storage.
*/
private boolean isSnapshotOnPrimaryStorage(long snapshotId) {
SnapshotDataStoreVO snapshotOnPrimary = snapshotStoreDao.findBySnapshot(snapshotId, DataStoreRole.Primary);
if (snapshotOnPrimary != null) {
long volumeId = snapshotOnPrimary.getVolumeId();
VolumeVO volumeVO = volumeDao.findById(volumeId);
return volumeVO != null && volumeVO.getRemoved() == null;
}
return false;
}
@Override

View File

@ -27,8 +27,8 @@
http://www.springframework.org/schema/context/spring-context.xsd"
>
<bean id="xenserverSnapshotStrategy"
class="org.apache.cloudstack.storage.snapshot.XenserverSnapshotStrategy" />
<bean id="defaultSnapshotStrategy"
class="org.apache.cloudstack.storage.snapshot.DefaultSnapshotStrategy" />
<bean id="storageSystemSnapshotStrategy"
class="org.apache.cloudstack.storage.snapshot.StorageSystemSnapshotStrategy" />

View File

@ -128,6 +128,14 @@ public class KVMStorageProcessor implements StorageProcessor {
private String _manageSnapshotPath;
private int _cmdsTimeout;
private static final String MANAGE_SNAPSTHOT_CREATE_OPTION = "-c";
private static final String MANAGE_SNAPSTHOT_DESTROY_OPTION = "-d";
private static final String NAME_OPTION = "-n";
private static final String CEPH_MON_HOST = "mon_host";
private static final String CEPH_AUTH_KEY = "key";
private static final String CEPH_CLIENT_MOUNT_TIMEOUT = "client_mount_timeout";
private static final String CEPH_DEFAULT_MOUNT_TIMEOUT = "30";
public KVMStorageProcessor(final KVMStoragePoolManager storagePoolMgr, final LibvirtComputingResource resource) {
this.storagePoolMgr = storagePoolMgr;
this.resource = resource;
@ -563,7 +571,7 @@ public class KVMStorageProcessor implements StorageProcessor {
final Script command = new Script(_createTmplPath, wait, s_logger);
command.add("-f", disk.getPath());
command.add("-t", tmpltPath);
command.add("-n", templateName + ".qcow2");
command.add(NAME_OPTION, templateName + ".qcow2");
final String result = command.execute();
@ -949,7 +957,7 @@ public class KVMStorageProcessor implements StorageProcessor {
} else {
final Script command = new Script(_manageSnapshotPath, cmd.getWaitInMillSeconds(), s_logger);
command.add("-b", snapshotDisk.getPath());
command.add("-n", snapshotName);
command.add(NAME_OPTION, snapshotName);
command.add("-p", snapshotDestPath);
if (isCreatedFromVmSnapshot) {
descName = UUID.randomUUID().toString();
@ -1010,14 +1018,7 @@ public class KVMStorageProcessor implements StorageProcessor {
}
} else {
if (primaryPool.getType() != StoragePoolType.RBD) {
final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
command.add("-d", snapshotDisk.getPath());
command.add("-n", snapshotName);
final String result = command.execute();
if (result != null) {
s_logger.debug("Failed to delete snapshot on primary: " + result);
// return new CopyCmdAnswer("Failed to backup snapshot: " + result);
}
deleteSnapshotViaManageSnapshotScript(snapshotName, snapshotDisk);
}
}
} catch (final Exception ex) {
@ -1035,6 +1036,16 @@ public class KVMStorageProcessor implements StorageProcessor {
}
}
private void deleteSnapshotViaManageSnapshotScript(final String snapshotName, KVMPhysicalDisk snapshotDisk) {
final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
command.add(MANAGE_SNAPSTHOT_DESTROY_OPTION, snapshotDisk.getPath());
command.add(NAME_OPTION, snapshotName);
final String result = command.execute();
if (result != null) {
s_logger.debug("Failed to delete snapshot on primary: " + result);
}
}
protected synchronized String attachOrDetachISO(final Connect conn, final String vmName, String isoPath, final boolean isAttach) throws LibvirtException, URISyntaxException,
InternalErrorException {
String isoXml = null;
@ -1489,12 +1500,7 @@ public class KVMStorageProcessor implements StorageProcessor {
*/
if (primaryPool.getType() == StoragePoolType.RBD) {
try {
final Rados r = new Rados(primaryPool.getAuthUserName());
r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
r.confSet("key", primaryPool.getAuthSecret());
r.confSet("client_mount_timeout", "30");
r.connect();
s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet("mon_host"));
Rados r = radosConnect(primaryPool);
final IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir());
final Rbd rbd = new Rbd(io);
@ -1511,8 +1517,8 @@ public class KVMStorageProcessor implements StorageProcessor {
} else {
/* VM is not running, create a snapshot by ourself */
final Script command = new Script(_manageSnapshotPath, _cmdsTimeout, s_logger);
command.add("-c", disk.getPath());
command.add("-n", snapshotName);
command.add(MANAGE_SNAPSTHOT_CREATE_OPTION, disk.getPath());
command.add(NAME_OPTION, snapshotName);
final String result = command.execute();
if (result != null) {
s_logger.debug("Failed to manage snapshot: " + result);
@ -1531,6 +1537,16 @@ public class KVMStorageProcessor implements StorageProcessor {
}
}
private Rados radosConnect(final KVMStoragePool primaryPool) throws RadosException {
Rados r = new Rados(primaryPool.getAuthUserName());
r.confSet(CEPH_MON_HOST, primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
r.confSet(CEPH_AUTH_KEY, primaryPool.getAuthSecret());
r.confSet(CEPH_CLIENT_MOUNT_TIMEOUT, CEPH_DEFAULT_MOUNT_TIMEOUT);
r.connect();
s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet(CEPH_MON_HOST));
return r;
}
@Override
public Answer deleteVolume(final DeleteCommand cmd) {
final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData();
@ -1619,12 +1635,7 @@ public class KVMStorageProcessor implements StorageProcessor {
String snapshotName = snapshotFullPath.substring(snapshotFullPath.lastIndexOf("/") + 1);
snap_full_name = disk.getName() + "@" + snapshotName;
if (primaryPool.getType() == StoragePoolType.RBD) {
Rados r = new Rados(primaryPool.getAuthUserName());
r.confSet("mon_host", primaryPool.getSourceHost() + ":" + primaryPool.getSourcePort());
r.confSet("key", primaryPool.getAuthSecret());
r.confSet("client_mount_timeout", "30");
r.connect();
s_logger.debug("Succesfully connected to Ceph cluster at " + r.confGet("mon_host"));
Rados r = radosConnect(primaryPool);
IoCTX io = r.ioCtxCreate(primaryPool.getSourceDir());
Rbd rbd = new Rbd(io);
RbdImage image = rbd.open(disk.getName());
@ -1644,6 +1655,9 @@ public class KVMStorageProcessor implements StorageProcessor {
rbd.close(image);
r.ioCtxDestroy(io);
}
} else if (primaryPool.getType() == StoragePoolType.NetworkFilesystem) {
s_logger.info(String.format("Attempting to remove snapshot (id=%s, name=%s, path=%s, storage type=%s) on primary storage", snapshotTO.getId(), snapshotTO.getName(), snapshotTO.getPath(), primaryPool.getType()));
deleteSnapshotViaManageSnapshotScript(snapshotName, disk);
} else {
s_logger.warn("Operation not implemented for storage pool type of " + primaryPool.getType().toString());
throw new InternalErrorException("Operation not implemented for storage pool type of " + primaryPool.getType().toString());