bhouse-nexthop commented on PR #14600: URL: https://github.com/apache/cloudstack/pull/14600#issuecomment-6733520336
### Test report: `listSnapshots` with a snapshot whose volume row is gone, on Ceph RBD **Environment** - A CloudStack 24.0 (main) build with this PR applied, alongside some of my other open PRs. - Three KVM hosts on Ubuntu 24.04, nested virtualisation; Ceph RBD primary storage (Ceph 19.2); NFS secondary storage with `snapshot.backup.to.secondary=true`. **Setup.** I made two 5 GB data volumes on RBD, D1 and D2. For each, I attached it to a running VM, took a snapshot (S1 and S2), and detached it. Both snapshots were `BackedUp`, with a Primary and an Image store ref. **1. Control.** `listSnapshots listall=true` succeeds, and both snapshots have `revertable=true`. **2. Volume row deleted** (`DELETE FROM volumes WHERE id=<D1>`, with foreign key checks off; this is how the production error was reproduced) - `listSnapshots listall=true` succeeds and lists both snapshots. - S1 has `revertable=false` and an empty `volumename`. S2 is unchanged. - `listSnapshots id=<S1>` also succeeds. - The management server log has no `because "volumeVO" is null` line. **3. Removed volume with no pool** (`UPDATE volumes SET removed=NOW(), pool_id=NULL, last_pool_id=NULL WHERE id=<D2>`) - `listSnapshots` still succeeds. - S2 now has `revertable=false`. - There is still no NPE in the log. **4. Operations on the orphaned snapshot S1** - `deleteSnapshot` is routed to `DefaultSnapshotStrategy`, as the description says. It fails with `Failed to delete snapshot chain`, because the `SnapshotTO` it sends has `volume=null`. The snapshot keeps its store refs, and the RBD snapshot stays on the image. Cleaning up such an orphan needs a by-hand step. That is outside this PR, which is about strategy selection. - `revertSnapshot` fails with an NPE, but not one this PR covers: `Cannot invoke "com.cloud.storage.VolumeVO.getState()" because "volume" is null` at `SnapshotManagerImpl.revertSnapshot`. That code calls `_volsDao.findById()` and does not check the result for null. It should refuse with "volume not found" instead. That is a small separate fix, and stock 4.22 behaves the same. **Without the PR** (not rerun on a stock build): step 2 is the production failure from #14599. With a snapshot like S1 present, stock fails the whole `listSnapshots` with `Cannot invoke "com.cloud.storage.VolumeVO.getRemoved()" because "volumeVO" is null`. **Result:** pass. - `listSnapshots` succeeds with a missing volume row and with a removed pool-less volume. - Such snapshots are reported as not revertable. - No `volumeVO` NPE was logged. **Not covered:** the ScaleIO and StorageSystem strategies (no such storage in the lab). They are covered by the PR's unit tests. -- 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]
