Skip to content

[WIP] Remove allocated snapshots / vm snapshots on start #8452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: 4.19
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ public interface VMSnapshotDao extends GenericDao<VMSnapshotVO, Long>, StateDao<
VMSnapshotVO findByName(Long vmId, String name);

List<VMSnapshotVO> listByAccountId(Long accountId);

List<VMSnapshotVO> listAllByStatus(VMSnapshot.State... status);
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import java.util.Date;
import java.util.List;


import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;

Expand Down Expand Up @@ -125,6 +124,13 @@ public List<VMSnapshotVO> listByAccountId(Long accountId) {
return listBy(sc, null);
}

@Override
public List<VMSnapshotVO> listAllByStatus(VMSnapshot.State... status) {
SearchCriteria<VMSnapshotVO> sc = SnapshotStatusSearch.create();
sc.setParameters("status", (Object[])status);
return listBy(sc, null);
}

@Override
public boolean updateState(State currentState, Event event, State nextState, VMSnapshot vo, Object data) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,11 @@

@Override
public boolean start() {
//remove snapshots in allocated state
List<SnapshotVO> allocatedSnapshots = _snapshotDao.listAllByStatus(Snapshot.State.Allocated);
for (SnapshotVO snapshot : allocatedSnapshots) {
_snapshotDao.remove(snapshot.getId());
}

Check warning on line 1523 in server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/storage/snapshot/SnapshotManagerImpl.java#L1522-L1523

Added lines #L1522 - L1523 were not covered by tests
Comment on lines +1519 to +1523
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be a single call to DB - removeAllByStatus?

//destroy snapshots in destroying state
List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Destroying);
for (SnapshotVO snapshotVO : snapshots) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@

@Override
public boolean start() {
//Remove VM Snapshots in allocated state
List<VMSnapshotVO> allocatedVMSnapshots = _vmSnapshotDao.listAllByStatus(VMSnapshot.State.Allocated);
for (VMSnapshotVO vmSnapshot : allocatedVMSnapshots) {
_vmSnapshotDao.remove(vmSnapshot.getId());
}

Check warning on line 207 in server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java

View check run for this annotation

Codecov / codecov/patch

server/src/main/java/com/cloud/vm/snapshot/VMSnapshotManagerImpl.java#L206-L207

Added lines #L206 - L207 were not covered by tests
Comment on lines +203 to +207
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be a single call to DB - removeAllByStatus?

return true;
}

Expand Down