[PATCH proxmox-backup v2] tape: virtual drive: fix endless recursion in changer status

[PATCH proxmox-backup v2] tape: virtual drive: fix endless recursion in changer status

[Jakob-Tobias Winter]

The virtual tape drive, which exists for tests and debugging only, also
acts as its own changer and reports every virtual tape in its directory
as a slot. To build that status it asks for the list of online media.
The drive has its own way to get that list, the tapes in its directory,
but that was kept next to the changer interface instead of being part of
it. So the status got the generic variant of the list, which is derived
from the changer status. The two called each other until the stack
overflowed, and every operation that needs the changer status of a
virtual drive, like loading or unloading a tape, crashed the process.

Make the list of tapes in the directory the variant that the changer
interface of the virtual drive provides, so that it replaces the generic
one for the status and for loading a tape from a slot.

Fixes: 46a1863f8 ("tape: improve MediaChange trait")
Suggested-by: Christian Ebner <c.ebner@proxmox.com>
Signed-off-by: Jakob-Tobias Winter <tobias@stuttgart.de>
---
changes since v1, thanks to Christian for the review:
* fix it the suggested way: the list of tapes of the virtual drive now
  is its implementation of the trait method instead of a method next to
  it, so the callers stay as they are
* add the Fixes trailer

v1: https://lore.proxmox.com/pbs-devel/20261109215950.903491-1-tobias@stuttgart.de/

Reproducer, with a virtual drive 'vtape' in tape.cfg and an empty file
'tape-demo1.json' in its directory:

  proxmox-backup-debug api create /tape/drive/vtape/load-slot \
      --source-slot 1

Tested with release builds of current master (68577e654) and
proxmox-backup-api and proxmox-backup-proxy running: without the patch
this call, like 'proxmox-tape load-media', aborts proxmox-backup-proxy
with the stack overflow. With it loading, labeling, unloading and
cleaning work. A tape backup job across three virtual tapes and the
restore of its media set into another datastore end with TASK OK, and
the restored snapshots verify and hold the same data as the source.
cargo test --workspace passes, cargo fmt --check and clippy report
nothing new.

 src/tape/drive/virtual_tape.rs | 42 +++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index a17916bab..326961cdd 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -145,24 +145,6 @@ impl VirtualTapeHandle {
         Ok(())
     }
 
-    fn online_media_label_texts(&self) -> Result<Vec<String>, Error> {
-        let mut list = Vec::new();
-        for entry in std::fs::read_dir(&self.path)? {
-            let entry = entry?;
-            let path = entry.path();
-            if path.is_file() && path.extension() == Some(std::ffi::OsStr::new("json")) {
-                if let Some(name) = path.file_stem() {
-                    if let Some(name) = name.to_str() {
-                        if let Some(label) = name.strip_prefix("tape-") {
-                            list.push(label.to_string());
-                        }
-                    }
-                }
-            }
-        }
-        Ok(list)
-    }
-
     #[allow(dead_code)]
     fn forward_space_count_files(&mut self, count: usize) -> Result<(), Error> {
         let mut status = self.load_status()?;
@@ -575,6 +557,28 @@ impl MediaChange for VirtualTapeHandle {
         self.status()
     }
 
+    /// List the virtual tapes in the directory of the drive
+    ///
+    /// The default implementation must not be used here: it derives the list from the
+    /// status, and the status gets its slots from this list.
+    fn online_media_label_texts(&mut self) -> Result<Vec<String>, Error> {
+        let mut list = Vec::new();
+        for entry in std::fs::read_dir(&self.path)? {
+            let entry = entry?;
+            let path = entry.path();
+            if path.is_file() && path.extension() == Some(std::ffi::OsStr::new("json")) {
+                if let Some(name) = path.file_stem() {
+                    if let Some(name) = name.to_str() {
+                        if let Some(label) = name.strip_prefix("tape-") {
+                            list.push(label.to_string());
+                        }
+                    }
+                }
+            }
+        }
+        Ok(list)
+    }
+
     fn clean_drive(&mut self) -> Result<MtxStatus, Error> {
         // do nothing
         self.status()
@@ -621,7 +625,7 @@ impl MediaChange for VirtualTapeDrive {
     }
 
     fn online_media_label_texts(&mut self) -> Result<Vec<String>, Error> {
-        let handle = open_virtual_tape_drive(self)?;
+        let mut handle = open_virtual_tape_drive(self)?;
         handle.online_media_label_texts()
     }
 
-- 
2.53.0