From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 37C021FF0B3 for ; Wed, 11 Nov 2026 11:21:56 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id BF9952162C; Wed, 11 Nov 2026 11:21:55 +0100 (CET) Message-ID: <8f133607-84d3-4d13-b88b-20db5d0d2353@proxmox.com> Date: Wed, 11 Nov 2026 11:21:51 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup] tape: virtual drive: fix endless recursion in changer status To: Jakob-Tobias Winter , tobias@linuxdingsda.de, pbs-devel@lists.proxmox.com References: <20261109215950.903491-1-tobias@stuttgart.de> Content-Language: en-US, de-DE From: Christian Ebner In-Reply-To: <20261109215950.903491-1-tobias@stuttgart.de> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794392511816 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.422 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: HJMFQ5K24ZREZ5ADQSJLEYHR5TGSR5SC X-Message-ID-Hash: HJMFQ5K24ZREZ5ADQSJLEYHR5TGSR5SC X-MailFrom: c.ebner@proxmox.com 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 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: On 11/9/26 11:07 PM, Jakob-Tobias Winter wrote: > 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 asked for the list of online media, > but got the generic variant of that list, which is derived from the > changer status. The two called each other until the stack overflowed, so > every operation that needs the changer status of a virtual drive, like > loading or unloading a tape, crashed the process. > > Use the list of tapes in the directory of the drive instead, as it was > intended, for the status and for loading a tape from a slot. Thanks for the patch, I would however prefer a different approach for fixing this: The issue is that VirtualTapeHandle does not implement the MediaChange traits online_media_label_texts() method with signature fn online_media_label_texts(&mut self) -> Result, Error>> but has it's own impl with a slightly different method signature fn online_media_label_texts(&self) -> Result, Error>> which leads to the incorrect method being called and therefore the recursion in the first place. So the better fix would be to move the VirtualTapeHandle's custom impl to actually override the MediaChange trait method. > Signed-off-by: Jakob-Tobias Winter > --- > The recursion has been there since 46a1863f8 ("tape: improve MediaChange > trait"). nit: please include this as Fixes: ... trailer with the patch. > 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 debug builds of current master and of 4.2.8, calling the API > in-process (PROXMOX_DEBUG_API_CODE=1): the call aborts with a stack > overflow without the patch and loads the tape with it. > > src/tape/drive/virtual_tape.rs | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs > index a17916bab..b2febaed5 100644 > --- a/src/tape/drive/virtual_tape.rs > +++ b/src/tape/drive/virtual_tape.rs > @@ -499,7 +499,7 @@ impl MediaChange for VirtualTapeHandle { > // slot-assignment here. > > let mut slots = Vec::new(); > - let label_texts = self.online_media_label_texts()?; > + let label_texts = VirtualTapeHandle::online_media_label_texts(self)?; > let max_slots = label_texts.len().div_ceil(8) * 8; > > for i in 0..max_slots { > @@ -535,7 +535,7 @@ impl MediaChange for VirtualTapeHandle { > bail!("invalid slot ID {}", slot); > } > > - let label_texts = self.online_media_label_texts()?; > + let label_texts = VirtualTapeHandle::online_media_label_texts(self)?; > > if slot > label_texts.len() as u64 { > bail!("slot {} is empty", slot);