kvm: fix RBD exclusive-lock leak that breaks revertSnapshot on Ceph

[calvix]

Summary

On KVM + Ceph/RBD, revertSnapshot fails intermittently with:

com.ceph.rbd.RbdException: Failed to rollback snapshot <uuid>
  at com.ceph.rbd.RbdImage.snapRollBack(RbdImage.java:139)
  at LibvirtRevertSnapshotCommandWrapper.execute(LibvirtRevertSnapshotCommandWrapper.java:111)

The root cause is not in the revert path at all — it is a leaked RBD
exclusive-lock left behind by snapshot creation.

What is wrong

KVMStorageProcessor.takeRbdVolumeSnapshotOfStoppedVm() calls
image.snapCreate(snapshotName) twice:

logger.debug("Attempting to create RBD snapshot {}@{}", disk.getName(), snapshotName);
image.snapCreate(snapshotName);      // creates the snapshot

image.snapCreate(snapshotName);      // always throws: snapshot already exists
long rbdSnapshotSize = getRbdSnapshotSize(...);   // unreachable
...
rbd.close(image);                    // unreachable
r.ioCtxDestroy(io);                  // unreachable
} catch (final Exception e) {
    logger.error("A RBD snapshot operation on [{}] failed. ...");   // swallowed
}

The duplicate is a merge artifact. 4.20 grew a snapCreate next to the new
getRbdSnapshotSize(), 4.22 already had one, and the merge kept both:

commit 30d306622a90ac43f2a6c35ee999110ad1bc5194  "Merge branch '4.20' into 4.22"
  parent ef60aa5601  -> image.snapCreate(snapshotName) x1
  parent 6bed3d4e64  -> image.snapCreate(snapshotName) x1
  result             -> image.snapCreate(snapshotName) x2

4.19 and 4.20 have one call; 4.22 and main have two.

Because there is no finally, the exception from the second call skips
rbd.close(image) / r.ioCtxDestroy(io), so the agent keeps the image open
and holds its RBD exclusive-lock indefinitely. The exception is only logged,
so the snapshot job still reports success and nothing looks wrong.

Note this method runs for running VMs too — createSnapshot() branches on
RUNNING && !primaryPool.isExternalSnapshot(), and RBD is an
external-snapshot pool, so every RBD volume snapshot takes this path.

Observed consequences

Reproduced on CloudStack 4.22 with KVM + Ceph/RBD primary storage:

1. revertSnapshot fails with EROFS. A live peer holds the
   exclusive-lock, so librbd refuses snap_rollback. It only succeeds once
   that client dies and librbd can break the lock — which is why the failure
   looks intermittent (~50% in my testing). Removing the lock by hand makes the
   very same rollback succeed immediately:

   # rbd snap rollback cloudstack/<vol>@<snap>
   Rolling back to snapshot: 0% complete...failed.
   rbd: rollback failed: (30) Read-only file system
   
   # rbd lock ls cloudstack/<vol>
   There is 1 exclusive lock on this image.
   Locker           ID                    Address
   client.21303777  auto 128935052455264  10.2.107.135:0/1367644037
   
   # rbd lock rm cloudstack/<vol> "auto 128935052455264" client.21303777
   # rbd snap rollback cloudstack/<vol>@<snap>
   Rolling back to snapshot: 100% complete...done.
   

   After clearing the lock, the CloudStack revertSnapshot API job also
   returns success.

2. Snapshots report physicalsize: 0 when snapshot.backup.to.secondary
   is false, because getRbdSnapshotSize() is never reached.

3. Volumes get stuck in state Destroy. The leaked watchers keep the image
   busy, rbd rm fails, and the volume can never be expunged.

The agent log shows the swallowed exception on every snapshot:

ERROR [kvm.storage.KVMStorageProcessor] A RBD snapshot operation on [<vol-uuid>] failed.
The error was: Failed to create snapshot <snap-uuid>
  at com.ceph.rbd.RbdImage.snapCreate(RbdImage.java:111)
  at KVMStorageProcessor.takeRbdVolumeSnapshotOfStoppedVm(KVMStorageProcessor.java:2392)
  at KVMStorageProcessor.createSnapshot(KVMStorageProcessor.java:1907)

<snap-uuid> there is exactly the snapshot the next revertSnapshot then
failed to roll back.

The fix

Commit 1 — takeRbdVolumeSnapshotOfStoppedVm

• remove the duplicated image.snapCreate(snapshotName);
• move rbd.close(image) and r.ioCtxDestroy(io) into a finally so the
  exclusive-lock is released even when the snapshot itself fails.

Commit 2 — createRBDvolumeFromRBDSnapshot (same class of defect)

This method released its handles only on the success path. Two paths escaped
cleanup: the early Could not find snapshot ... on RBD return, and any
exception from clone() / resize() / flatten() (caught and turned into a
null disk). Both leak the same lock. Worse, the failure paths after
snapProtect() leave the snapshot protected, and a protected snapshot can
be deleted neither on its own nor with its volume.

Cleanup moved into a finally, tracking whether the snapshot was actually
protected so snapUnprotect() runs exactly when it should. Cleanup failures
are logged and never mask the original outcome; a failed snapUnprotect is
logged at ERROR because it needs manual intervention.

No behaviour change on the success path in either method.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• Build/CI

How Has This Been Tested?

On a KVM + Ceph/RBD cluster (CloudStack 4.22.1.0, Ceph 20.2.2, single RBD pool,
snapshot.backup.to.secondary tested both true and false):

• Reproduced the failure end to end: createSnapshot on a DATADISK, stop the
  VM, revertSnapshot → RbdException: Failed to rollback snapshot.
• Confirmed the RBD snapshot itself exists and is correctly named, so the
  rollback target was never the problem:
  rbd snap ls → 505162dd-8fa5-4262-acfc-bce540cf36cd, the exact name passed
  to snapRollBack.
• Correlated success/failure with the lock owner: when the lock holder still
  appears in rbd status (live client) the revert fails; when it does not
  (dead client, librbd breaks the lock) it succeeds. Outcomes were predicted
  from rbd status before each attempt.
• Verified that clearing the lock makes both rbd snap rollback and the
  CloudStack revertSnapshot job succeed (see output above).
• Confirmed the duplicated call is reached on every snapshot via the agent log
  stack trace quoted above.

Not compile-tested locally — no Maven toolchain on the machine used for the
investigation — so CI is the first build of these commits.

[calvix]

Superseded — reopening with corrected commit authorship.