From f4ddf95d89e6597121207593b87d6ed5ba480617 Mon Sep 17 00:00:00 2001
From: calvix <7136358+calvix@users.noreply.github.com>
Date: Mon, 10 Aug 2026 18:58:09 +0200
Subject: [PATCH] kvm: extract RBD handle teardown into null-safe helper
 methods

Address review feedback: the nested try-catch teardown blocks in the
finally clauses of takeRbdVolumeSnapshotOfStoppedVm and
createRBDvolumeFromRBDSnapshot are extracted into two reusable,
null-safe helpers that log but never throw:

- closeRbdImage (3 call sites)
- destroyRadosIoCtx (2 call sites)

No behavior change.
---
 .../kvm/storage/KVMStorageProcessor.java      | 75 +++++++++----------
 1 file changed, 34 insertions(+), 41 deletions(-)

diff --git a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
index 836486c219cc..e33fca2d372b 100644
--- a/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
+++ b/plugins/hypervisors/kvm/src/main/java/com/cloud/hypervisor/kvm/storage/KVMStorageProcessor.java
@@ -2362,23 +2362,8 @@ protected Long takeRbdVolumeSnapshotOfStoppedVm(KVMStoragePool primaryPool, KVMP
         } catch (final Exception e) {
             logger.error("A RBD snapshot operation on [{}] failed. The error was: {}", disk.getName(), e.getMessage(), e);
         } finally {
-            // The image MUST be closed on every path. While it stays open this client holds the RBD
-            // exclusive-lock, and a later 'rbd snap rollback' (revertSnapshot) issued from any other host
-            // cannot take a live peer's lock - librbd then fails it with EROFS.
-            if (image != null) {
-                try {
-                    rbd.close(image);
-                } catch (final Exception e) {
-                    logger.warn("Failed to close RBD image [{}] after a snapshot operation. The error was: {}", disk.getName(), e.getMessage(), e);
-                }
-            }
-            if (io != null) {
-                try {
-                    r.ioCtxDestroy(io);
-                } catch (final Exception e) {
-                    logger.warn("Failed to destroy the RADOS IO context used to snapshot [{}]. The error was: {}", disk.getName(), e.getMessage(), e);
-                }
-            }
+            closeRbdImage(rbd, image, disk.getName());
+            destroyRadosIoCtx(r, io, disk.getName());
         }
         return snapshotSize;
     }
@@ -2680,6 +2665,34 @@ protected Rados radosConnect(final KVMStoragePool primaryPool) throws RadosExcep
         return r;
     }
 
+    /**
+     * Closes an RBD image if it was opened; never throws. An image left open keeps this client's RBD
+     * exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and keeps
+     * the image busy so it cannot be removed.
+     */
+    protected void closeRbdImage(Rbd rbd, RbdImage image, String imageName) {
+        if (image == null) {
+            return;
+        }
+        try {
+            rbd.close(image);
+        } catch (final Exception e) {
+            logger.warn("Failed to close RBD image [{}]. The error was: {}", imageName, e.getMessage(), e);
+        }
+    }
+
+    /** Destroys a RADOS IO context if it was created; never throws. */
+    protected void destroyRadosIoCtx(Rados r, IoCTX io, String contextDescription) {
+        if (io == null) {
+            return;
+        }
+        try {
+            r.ioCtxDestroy(io);
+        } catch (final Exception e) {
+            logger.warn("Failed to destroy the RADOS IO context used for [{}]. The error was: {}", contextDescription, e.getMessage(), e);
+        }
+    }
+
     @Override
     public Answer deleteVolume(final DeleteCommand cmd) {
         final VolumeObjectTO vol = (VolumeObjectTO)cmd.getData();
@@ -2877,16 +2890,8 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S
             disk = null;
         } finally {
             // Every handle has to be released on all paths, including the "snapshot not found" return and
-            // any failure of clone/resize/flatten. An image left open keeps this client's RBD
-            // exclusive-lock, which later makes 'rbd snap rollback' (revertSnapshot) fail with EROFS and
-            // keeps the image busy so it cannot be removed.
-            if (diskImage != null) {
-                try {
-                    rbd.close(diskImage);
-                } catch (final Exception e) {
-                    logger.warn(String.format("Failed to close the cloned RBD image %s. The error was: %s", newUuid, e.getMessage()), e);
-                }
-            }
+            // any failure of clone/resize/flatten.
+            closeRbdImage(rbd, diskImage, newUuid);
             // A snapshot left protected cannot be deleted, and neither can its volume.
             if (snapProtected) {
                 try {
@@ -2896,20 +2901,8 @@ private KVMPhysicalDisk createRBDvolumeFromRBDSnapshot(KVMPhysicalDisk volume, S
                             "resolved manually. The error was: %s", snapshotName, e.getMessage()), e);
                 }
             }
-            if (srcImage != null) {
-                try {
-                    rbd.close(srcImage);
-                } catch (final Exception e) {
-                    logger.warn(String.format("Failed to close the source RBD image %s. The error was: %s", volume.getName(), e.getMessage()), e);
-                }
-            }
-            if (io != null) {
-                try {
-                    r.ioCtxDestroy(io);
-                } catch (final Exception e) {
-                    logger.warn(String.format("Failed to destroy the RADOS IO context used to clone %s. The error was: %s", snapshotName, e.getMessage()), e);
-                }
-            }
+            closeRbdImage(rbd, srcImage, volume.getName());
+            destroyRadosIoCtx(r, io, snapshotName);
         }
 
         return disk;