From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id 453E11FF13C for ; Thu, 02 Apr 2026 11:34:24 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id A2D121071C; Thu, 2 Apr 2026 11:34:53 +0200 (CEST) Date: Thu, 2 Apr 2026 11:34:48 +0200 From: Arthur Bied-Charreton To: Hannes Laimer Subject: Re: [PATCH proxmox-backup v6 0/8] fixes #6195: add support for moving groups and namespaces Message-ID: References: <20260331123409.198353-1-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260331123409.198353-1-h.laimer@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775122430811 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.702 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 1 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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: 6KD3ZDR426DXPWZPW2W6RJMTOTGWQF6R X-Message-ID-Hash: 6KD3ZDR426DXPWZPW2W6RJMTOTGWQF6R X-MailFrom: a.bied-charreton@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 CC: pbs-devel@lists.proxmox.com 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 Tue, Mar 31, 2026 at 02:34:01PM +0200, Hannes Laimer wrote: > Add support for moving backup groups and entire namespace subtrees to > a different location within the same datastore. > > Groups are moved with exclusive per-group and per-snapshot locking. > For S3, objects are copied to the target prefix before deleting the > source. Namespace moves process groups individually, deferring and > retrying lock conflicts once, so partially failed moves can be > completed with move_group. > > > v6, thanks @Fabian and @Dominik!: > - drop ns locks, lock everything directly, like we do for delete. only > difference, we dont do partial group moves, reason being that we > cant move single snapshots, so cleanup would not really be possible > - ui: disable purne for empty groups > - ui: dont render verification status for empty groups > > v5, thanks @Chris!: > - lock dir instead of `.ns-lock` file > - explicitly drop ns lock guards in specific order > - improve cleanup of partially failed s3 moves, we now create the local > empty dir+owner file before we start copying s3 objects, if any of > the s3 ops fail, the dir stays behind and can be deleted through the > UI(which also triggers a prefix cleanup on the s3 storage) > - update parameters for `DataStore::lookup_datastore()` > - ui: re-ordered actions, `move` now next to `verify` > - ui: add move to right-click context menu > - ui: show empty groups in the UI > - add cli commands for both ns and group moves > - add 2s ns lock timeout for worker tasks > > *note*: given the UI change to show empty groups it could make sense to > not auto-delete a group if the last snapshot is deleted. For this series > though that is not relevant since we just need empty groups to be > deletable through the UI for partially failed s3 moves > [...] Hey! I played around with this, I think the UI looks nice and the feature works well. Here's what I tested (on both normal and S3-backed datastores, both in the UI and with the CLI commands): I can move namespaces/groups to other namespaces and to the root namespace without issues. I was also able to rename a namespace by moving it (like `mv`). I was not able to move a namespace/group if it would result in an ID collision in the target namespace, nor could I move a namespace to itself. I also could not move a group during a backup. I did not find any issues while testing and the error messages are helpful. I answered {7,8}/8 with some UX nits, but in general I think this looks nice, so consider this series: Reviewed-by: Arthur Bied-Charreton Tested-by: Arthur Bied-Charreton