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 5930D1FF136 for ; Mon, 20 Apr 2026 16:50:02 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 577FF5412; Mon, 20 Apr 2026 16:50:00 +0200 (CEST) Date: Mon, 20 Apr 2026 16:49:53 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= Subject: Re: [PATCH proxmox-backup v7 5/9] api: add POST endpoint for move-group To: Hannes Laimer , pbs-devel@lists.proxmox.com References: <20260416171830.266553-1-h.laimer@proxmox.com> <20260416171830.266553-6-h.laimer@proxmox.com> In-Reply-To: <20260416171830.266553-6-h.laimer@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.17.0 (https://github.com/astroidmail/astroid) Message-Id: <1776689393.b4yb3h91kk.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1776696512451 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.054 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 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: ADQZV3E77ZRIOKQINPOHS4I6U2B26XQ4 X-Message-ID-Hash: ADQZV3E77ZRIOKQINPOHS4I6U2B26XQ4 X-MailFrom: f.gruenbichler@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 April 16, 2026 7:18 pm, Hannes Laimer wrote: > Add a dedicated /move-group endpoint for moving backup groups between > namespaces within the same datastore. >=20 > The permission model allows users with DATASTORE_PRUNE on the source > and DATASTORE_BACKUP on the target namespace to move groups they own, > without requiring full DATASTORE_MODIFY on both sides. >=20 > Signed-off-by: Hannes Laimer > --- > src/api2/admin/datastore.rs | 105 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 105 insertions(+) >=20 > diff --git a/src/api2/admin/datastore.rs b/src/api2/admin/datastore.rs > index fcb81ec5..54895f2b 100644 > --- a/src/api2/admin/datastore.rs > +++ b/src/api2/admin/datastore.rs > @@ -280,6 +280,110 @@ pub async fn delete_group( > .await? > } > =20 > +#[api( > + input: { > + properties: { > + store: { schema: DATASTORE_SCHEMA }, > + ns: { > + type: BackupNamespace, > + optional: true, > + }, > + group: { > + type: pbs_api_types::BackupGroup, > + flatten: true, > + }, > + "target-ns": { > + type: BackupNamespace, > + optional: true, > + }, > + "merge-group": { > + type: bool, > + optional: true, > + default: true, > + description: "If the group already exists in the target = namespace, merge \ > + snapshots into it. Requires matching ownership and n= on-overlapping \ > + snapshot times.", > + }, > + }, > + }, > + returns: { > + schema: UPID_SCHEMA, > + }, > + access: { > + permission: &Permission::Anybody, > + description: "Requires DATASTORE_MODIFY or DATASTORE_PRUNE (+ gr= oup ownership) on the \ > + source namespace and DATASTORE_MODIFY or DATASTORE_BACKUP (+= group ownership) on \ > + the target namespace.", > + }, > +)] > +/// Move a backup group to a different namespace within the same datasto= re. > +pub fn move_group( > + store: String, > + ns: Option, > + group: pbs_api_types::BackupGroup, > + target_ns: Option, > + merge_group: bool, > + rpcenv: &mut dyn RpcEnvironment, > +) -> Result { > + let auth_id: Authid =3D rpcenv.get_auth_id().unwrap().parse()?; > + let ns =3D ns.unwrap_or_default(); > + let target_ns =3D target_ns.unwrap_or_default(); > + > + let source_limited =3D check_ns_privs_full( > + &store, > + &ns, > + &auth_id, > + PRIV_DATASTORE_MODIFY, > + PRIV_DATASTORE_PRUNE, > + )?; > + let target_limited =3D check_ns_privs_full( > + &store, > + &target_ns, > + &auth_id, > + PRIV_DATASTORE_MODIFY, > + PRIV_DATASTORE_BACKUP, > + )?; > + > + let datastore =3D DataStore::lookup_datastore(lookup_with(&store, Op= eration::Write))?; > + > + if source_limited || target_limited { > + let owner =3D datastore.get_owner(&ns, &group)?; > + check_backup_owner(&owner, &auth_id)?; > + } > + > + // Best-effort pre-checks for a fast synchronous error before spawni= ng a worker. > + if ns =3D=3D target_ns { > + bail!("source and target namespace must be different"); > + } this check > + if !datastore.namespace_exists(&target_ns) { > + bail!("target namespace '{target_ns}' does not exist"); > + } this check > + let source_group =3D datastore.backup_group(ns.clone(), group.clone(= )); > + if !source_group.exists() { > + bail!("group '{group}' does not exist in namespace '{ns}'"); > + } and this check are all done right away again after forking the worker (before locking), but not again after locking.. should we maybe have a helper fn? > + let target_group =3D datastore.backup_group(target_ns.clone(), group= .clone()); > + if target_group.exists() && !merge_group { > + bail!( > + "group '{group}' already exists in target namespace '{target= _ns}' \ > + and merge-group is disabled" > + ); > + } this one is checked again after locking only.. > + > + let worker_id =3D format!("{store}:{ns}/{group}:{target_ns}"); > + let to_stdout =3D rpcenv.env_type() =3D=3D RpcEnvironmentType::CLI; > + > + let upid_str =3D WorkerTask::new_thread( > + "move-group", > + Some(worker_id), > + auth_id.to_string(), > + to_stdout, > + move |_worker| datastore.move_group(&ns, &group, &target_ns, mer= ge_group), > + )?; > + > + Ok(json!(upid_str)) > +} > + > #[api( > input: { > properties: { > @@ -2852,6 +2956,7 @@ const DATASTORE_INFO_SUBDIRS: SubdirMap =3D &[ > .delete(&API_METHOD_DELETE_GROUP), > ), > ("mount", &Router::new().post(&API_METHOD_MOUNT)), > + ("move-group", &Router::new().post(&API_METHOD_MOVE_GROUP)), > ( > "namespace", > // FIXME: move into datastore:: sub-module?! > --=20 > 2.47.3 >=20 >=20 >=20 >=20 >=20 >=20