Skip to content

Commit

Permalink
mgmt: Fix delete snapshot
Browse files Browse the repository at this point in the history 
The snapshots were deleted from CS but the main record in the
`snapshots` DB table was not marked as deleted
  • Loading branch information
@slavkap
slavkap committed Nov 13, 2024
1 parent 165d8d9 commit 446a535
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 41 deletions.
3 changes: 3 additions & 0 deletions ...ume/storpool/src/main/java/org/apache/cloudstack/storage/datastore/util/StorPoolUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ public static void spLog(String fmt, Object... args) {
public static final String DELAY_DELETE = "delayDelete";

public static final String SP_TIER = "SP_QOSCLASS";

public static final String OBJECT_DOES_NOT_EXIST = "objectDoesNotExist";

public static enum StorpoolRights {
RO("ro"), RW("rw"), DETACH("detach");

Expand Down
110 changes: 69 additions & 41 deletions ...orpool/src/main/java/org/apache/cloudstack/storage/snapshot/StorPoolSnapshotStrategy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,21 @@
// under the License.
package org.apache.cloudstack.storage.snapshot;

import java.util.ArrayList;
import java.util.List;

import javax.inject.Inject;
import com.cloud.exception.InvalidParameterValueException;
import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.Snapshot;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.SnapshotZoneDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.utils.db.SearchBuilder;
import com.cloud.utils.db.SearchCriteria;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;

import org.apache.cloudstack.engine.subsystem.api.storage.DataStore;
import org.apache.cloudstack.engine.subsystem.api.storage.DataStoreManager;
Expand All @@ -40,27 +51,19 @@
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpApiResponse;
import org.apache.cloudstack.storage.datastore.util.StorPoolUtil.SpConnectionDesc;
import org.apache.commons.collections.CollectionUtils;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;

import org.springframework.stereotype.Component;

import com.cloud.exception.InvalidParameterValueException;
import com.cloud.hypervisor.kvm.storage.StorPoolStorageAdaptor;
import com.cloud.storage.DataStoreRole;
import com.cloud.storage.Snapshot;
import com.cloud.storage.SnapshotVO;
import com.cloud.storage.VolumeVO;
import com.cloud.storage.dao.SnapshotDao;
import com.cloud.storage.dao.SnapshotDetailsDao;
import com.cloud.storage.dao.SnapshotDetailsVO;
import com.cloud.storage.dao.SnapshotZoneDao;
import com.cloud.storage.dao.VolumeDao;
import com.cloud.utils.exception.CloudRuntimeException;
import com.cloud.utils.fsm.NoTransitionException;
import javax.inject.Inject;
import java.util.ArrayList;
import java.util.List;


@Component
public class StorPoolSnapshotStrategy implements SnapshotStrategy {
private static final Logger log = Logger.getLogger(StorPoolSnapshotStrategy.class);
protected Logger logger = LogManager.getLogger(getClass());

@Inject
private SnapshotDao _snapshotDao;
Expand Down Expand Up @@ -90,11 +93,11 @@ public SnapshotInfo backupSnapshot(SnapshotInfo snapshotInfo) {
snapshotObj.processEvent(Snapshot.Event.BackupToSecondary);
snapshotObj.processEvent(Snapshot.Event.OperationSucceeded);
} catch (NoTransitionException ex) {
log.debug("Failed to change state: " + ex.toString());
logger.debug("Failed to change state: " + ex.toString());
try {
snapshotObj.processEvent(Snapshot.Event.OperationFailed);
} catch (NoTransitionException ex2) {
log.debug("Failed to change state: " + ex2.toString());
logger.debug("Failed to change state: " + ex2.toString());
}
}
return snapshotInfo;
Expand All @@ -115,12 +118,11 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
SpApiResponse resp = StorPoolUtil.snapshotDelete(name, conn);
if (resp.getError() != null) {
final String err = String.format("Failed to clean-up Storpool snapshot %s. Error: %s", name, resp.getError());
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp);
StorPoolUtil.spLog(err);
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp);
throw new CloudRuntimeException(err);
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId, resp.getError().getName().equals(StorPoolUtil.OBJECT_DOES_NOT_EXIST));
} else {
res = deleteSnapshotFromDbIfNeeded(snapshotVO, zoneId);
markSnapshotAsDestroyedIfAlreadyRemoved(snapshotId,true);
StorPoolUtil.spLog("StorpoolSnapshotStrategy.deleteSnapshot: executed successfully=%s, snapshot uuid=%s, name=%s", res, snapshotVO.getUuid(), name);
}
} catch (Exception e) {
Expand All @@ -129,28 +131,41 @@ public boolean deleteSnapshot(Long snapshotId, Long zoneId) {
}
}

List<SnapshotDataStoreVO> snapshots = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready);
if (res || CollectionUtils.isEmpty(snapshots)) {
updateSnapshotToDestroyed(snapshotVO);
return true;
}
return res;
}

private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, SpApiResponse resp) {
if (resp.getError().getName().equals("objectDoesNotExist")) {
SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findBySourceSnapshot(snapshotId, DataStoreRole.Primary);
if (snapshotOnPrimary != null) {
snapshotOnPrimary.setState(State.Destroyed);
_snapshotStoreDao.update(snapshotOnPrimary.getId(), snapshotOnPrimary);
private void markSnapshotAsDestroyedIfAlreadyRemoved(Long snapshotId, boolean isSnapshotDeleted) {
if (!isSnapshotDeleted) {
return;
}
List<SnapshotDataStoreVO> snapshotsOnStore = _snapshotStoreDao.listBySnapshotIdAndState(snapshotId, State.Ready);
for (SnapshotDataStoreVO snapshot : snapshotsOnStore) {
if (snapshot.getInstallPath() != null && snapshot.getInstallPath().contains(StorPoolUtil.SP_DEV_PATH)) {
snapshot.setState(State.Destroyed);
_snapshotStoreDao.update(snapshot.getId(), snapshot);
}
}
}

@Override
public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperation op) {
log.debug(String.format("StorpoolSnapshotStrategy.canHandle: snapshot=%s, uuid=%s, op=%s", snapshot.getName(), snapshot.getUuid(), op));
logger.debug(String.format("StorpoolSnapshotStrategy.canHandle: snapshot=%s, uuid=%s, op=%s", snapshot.getName(), snapshot.getUuid(), op));

if (op != SnapshotOperation.DELETE) {
return StrategyPriority.CANT_HANDLE;
}

SnapshotDataStoreVO snapshotOnPrimary = _snapshotStoreDao.findOneBySnapshotAndDatastoreRole(snapshot.getId(), DataStoreRole.Primary);
if (snapshotOnPrimary == null) {
List<SnapshotDataStoreVO> snapshotsOnStorPool = listSnapshotsByIdAndPath(snapshot.getId(), StorPoolUtil.SP_DEV_PATH, DataStoreRole.Image);
if (CollectionUtils.isNotEmpty(snapshotsOnStorPool)) {
return StrategyPriority.HIGHEST;
}
return StrategyPriority.CANT_HANDLE;
}
if (zoneId != null) { // If zoneId is present, then it should be same as the zoneId of primary store
Expand All @@ -173,25 +188,25 @@ public StrategyPriority canHandle(Snapshot snapshot, Long zoneId, SnapshotOperat
}

private boolean deleteSnapshotChain(SnapshotInfo snapshot) {
log.debug("delete snapshot chain for snapshot: " + snapshot.getId());
logger.debug("delete snapshot chain for snapshot: " + snapshot.getId());
final SnapshotInfo snapOnImage = snapshot;
boolean result = false;
boolean resultIsSet = false;
try {
while (snapshot != null &&
(snapshot.getState() == Snapshot.State.Destroying || snapshot.getState() == Snapshot.State.Destroyed || snapshot.getState() == Snapshot.State.Error || snapshot.getState() == Snapshot.State.BackedUp)) {
(snapshot.getState() == Snapshot.State.Destroying || snapshot.getState() == Snapshot.State.Destroyed || snapshot.getState() == Snapshot.State.Error || snapshot.getState() == Snapshot.State.BackedUp)) {
SnapshotInfo child = snapshot.getChild();

if (child != null) {
log.debug("the snapshot has child, can't delete it on the storage");
logger.debug("the snapshot has child, can't delete it on the storage");
break;
}
log.debug("Snapshot: " + snapshot.getId() + " doesn't have children, so it's ok to delete it and its parents");
logger.debug("Snapshot: " + snapshot.getId() + " doesn't have children, so it's ok to delete it and its parents");
SnapshotInfo parent = snapshot.getParent();
boolean deleted = false;
if (parent != null) {
if (parent.getPath() != null && parent.getPath().equalsIgnoreCase(snapshot.getPath())) {
log.debug("for empty delta snapshot, only mark it as destroyed in db");
logger.debug("for empty delta snapshot, only mark it as destroyed in db");
snapshot.processEvent(Event.DestroyRequested);
snapshot.processEvent(Event.OperationSuccessed);
deleted = true;
Expand All @@ -208,7 +223,7 @@ private boolean deleteSnapshotChain(SnapshotInfo snapshot) {
if (r) {
List<SnapshotInfo> cacheSnaps = snapshotDataFactory.listSnapshotOnCache(snapshot.getId());
for (SnapshotInfo cacheSnap : cacheSnaps) {
log.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
logger.debug("Delete snapshot " + snapshot.getId() + " from image cache store: " + cacheSnap.getDataStore().getName());
cacheSnap.delete();
}
}
Expand All @@ -217,7 +232,7 @@ private boolean deleteSnapshotChain(SnapshotInfo snapshot) {
resultIsSet = true;
}
} catch (Exception e) {
log.debug("Failed to delete snapshot on storage. ", e);
logger.debug("Failed to delete snapshot on storage. ", e);
}
}
} else {
Expand All @@ -226,7 +241,7 @@ private boolean deleteSnapshotChain(SnapshotInfo snapshot) {
snapshot = parent;
}
} catch (Exception e) {
log.debug("delete snapshot failed: ", e);
logger.debug("delete snapshot failed: ", e);
}
return result;
}
Expand All @@ -248,7 +263,7 @@ protected boolean deleteSnapshotOnImageAndPrimary(long snapshotId, DataStore sto
obj.processEvent(Snapshot.Event.DestroyRequested);
}
} catch (NoTransitionException e) {
log.debug("Failed to set the state to destroying: ", e);
logger.debug("Failed to set the state to destroying: ", e);
return false;
}

Expand All @@ -266,13 +281,13 @@ protected boolean deleteSnapshotOnImageAndPrimary(long snapshotId, DataStore sto
}
}
} catch (Exception e) {
log.debug("Failed to delete snapshot: ", e);
logger.debug("Failed to delete snapshot: ", e);
try {
if (areLastSnapshotRef) {
obj.processEvent(Snapshot.Event.OperationFailed);
}
} catch (NoTransitionException e1) {
log.debug("Failed to change snapshot state: " + e.toString());
logger.debug("Failed to change snapshot state: " + e.toString());
}
return false;
}
Expand Down Expand Up @@ -372,4 +387,17 @@ public boolean revertSnapshot(SnapshotInfo snapshot) {
@Override
public void postSnapshotCreation(SnapshotInfo snapshot) {
}

private List<SnapshotDataStoreVO> listSnapshotsByIdAndPath (Long id, String pathList, DataStoreRole role) {
SearchBuilder<SnapshotDataStoreVO> dataStoreAndInstallPathSearch = _snapshotStoreDao.createSearchBuilder();
dataStoreAndInstallPathSearch.and("id", dataStoreAndInstallPathSearch.entity().getSnapshotId(), SearchCriteria.Op.EQ);
dataStoreAndInstallPathSearch.and("install_pathIN", dataStoreAndInstallPathSearch.entity().getInstallPath(), SearchCriteria.Op.LIKE);
dataStoreAndInstallPathSearch.and("role", dataStoreAndInstallPathSearch.entity().getRole(), SearchCriteria.Op.EQ);
dataStoreAndInstallPathSearch.done();
SearchCriteria<SnapshotDataStoreVO> sc = dataStoreAndInstallPathSearch.create();
sc.setParameters("id", id);
sc.setParameters("install_pathIN", pathList + "%");
sc.setParameters("role", role);
return _snapshotStoreDao.search(sc, null);
}
}

0 comments on commit 446a535

Please sign in to comment.