From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 884441FF0B3 for ; Wed, 11 Nov 2026 21:57:22 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D1731215FE; Wed, 11 Nov 2026 21:57:21 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=stuttgart.de; s=s1022; t=1794430150; bh=HQw/dukIDKaCdB5yml/aTydUPQA16NFVsAYhiua/L4o=; h=From:To:Cc:Subject:Date:From; b=DdV8PWPWR8GEdSskJvtPqzqjI20SD0YW5vBc0IJ87QiIvffdAc/5PaaMJQlEkn2wi zegt7ahre1chndatLxz9gAKQCv3VponkyiigRc9zTTRy03mF2GvRBUFs4ZyHevIneH nWuyjEaa9Q1DIvQ9+X8QL+NfYNr/vvynBtvvjAlZmQJUNnmwKRPFrFBm7TQYDl3cfJ M9EnGhgKhyz4UlApy0p+XfUzN+pUzT88rEZ2wgp9iG3Xi3tLhyS2Jh/XTgUpoMTlqW D627TuAn6MpqG32hp3KMQqDtTz7amGXEXF2I5TqeH0dd0UKdnp6uWiPVnZdWkD31Xh HXDedWWeTYFbA== From: Jakob-Tobias Winter To: pbs-devel@lists.proxmox.com Subject: [PATCH proxmox-backup v2] tape: virtual drive: fix endless recursion in changer status Date: Wed, 11 Nov 2026 21:48:54 +0100 Message-ID: <20261111204853.531985-2-tobias@stuttgart.de> X-Mailer: git-send-email 2.53.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.027 Adjusted score from AWL reputation of From: address DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain DMARC_PASS -0.1 DMARC pass policy RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_PASS -0.001 SPF: HELO matches SPF record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: MLZIGCLZ6FJYRVLAR6KL52TBLKYXMPZN X-Message-ID-Hash: MLZIGCLZ6FJYRVLAR6KL52TBLKYXMPZN X-MailFrom: prvs=d437bbd6df=tobias@stuttgart.de X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Jakob-Tobias Winter X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 Signed-off-by: Jakob-Tobias Winter --- 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, 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, 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 { // do nothing self.status() @@ -621,7 +625,7 @@ impl MediaChange for VirtualTapeDrive { } fn online_media_label_texts(&mut self) -> Result, Error> { - let handle = open_virtual_tape_drive(self)?; + let mut handle = open_virtual_tape_drive(self)?; handle.online_media_label_texts() } -- 2.53.0