waterWang opened a new pull request, #13976:
URL: https://github.com/apache/cloudstack/pull/13976

   ## Fix: Handle failed storage VM snapshot cleanup in 
StorageVMSnapshotStrategy
   
   ### Problem
   When a KVM disk-only VM snapshot (created via `StorageVMSnapshotStrategy`) 
fails during creation — for example, when the QEMU guest agent is not connected 
and the freeze operation fails — the snapshot enters the `Error` state. The 
`STORAGE_SNAPSHOT` detail is removed during the creation rollback.
   
   When the user later tries to delete this failed snapshot, 
`StorageVMSnapshotStrategy.canHandle()` returns `CANT_HANDLE` because the 
`STORAGE_SNAPSHOT` detail is absent. The deletion falls through to 
`DefaultVMSnapshotStrategy`, which sends a `DeleteVMSnapshotCommand` to the KVM 
agent. For a stopped VM with RAW/RBD volumes, the libvirt domain does not 
exist, so the command fails with:
   
   ```
   Delete Instance Snapshot failed due to org.libvirt.LibvirtException:
   Domain not found: no domain with matching name 'i-2-43-VM'
   ```
   
   This is a RAW/RBD variant of issue #11673. PR #11687 fixed a similar case 
for QCOW2/stopped VMs, but the fallback only handles QCOW2 volumes.
   
   ### Fix
   Two changes in `StorageVMSnapshotStrategy`:
   
   1. **`canHandle(VMSnapshot)`**: Allow `Error` state snapshots (that would 
otherwise be handled by `StorageVMSnapshotStrategy`) to skip the 
`STORAGE_SNAPSHOT` detail check. Since the detail is removed during the 
creation rollback, failed snapshots would otherwise be rejected.
   
   2. **`deleteVMSnapshot` override**: For `Error` state storage snapshots, 
skip the hypervisor `DeleteVMSnapshotCommand` (which would fail on a stopped 
VM) and clean up the database record directly via `deleteVMSnapshotFromDB`. 
This is safe because the `takeVMSnapshot` rollback already removed any 
underlying storage snapshots that were created before the failure.
   
   ### FSM validation
   The `Error → Event.ExpungeRequested → Expunging` transition is valid 
(confirmed in `VMSnapshot.State`).
   
   ### Testing
   - `Error` state snapshots with KVM + Disk type + `VmStorageSnapshotKvm` 
enabled are now handled by `StorageVMSnapshotStrategy` instead of 
`DefaultVMSnapshotStrategy`
   - For `Error` state, deletion bypasses the hypervisor command and cleans up 
the DB directly
   - `Ready` state snapshots continue to use the inherited `deleteVMSnapshot` 
flow (unchanged behavior)
   


-- 
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]

Reply via email to