From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id C7FA31FF13C for ; Thu, 02 Apr 2026 11:22:20 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 2F4691025A; Thu, 2 Apr 2026 11:22:50 +0200 (CEST) Date: Thu, 2 Apr 2026 11:22:44 +0200 From: Arthur Bied-Charreton To: Hannes Laimer Subject: Re: [PATCH proxmox-backup v6 8/8] cli: add move-namespace and move-group commands Message-ID: References: <20260331123409.198353-1-h.laimer@proxmox.com> <20260331123409.198353-9-h.laimer@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260331123409.198353-9-h.laimer@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775121707165 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.710 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: WTZV6RKC5HT4JNJMBIUMEKE4I7JA5VJS X-Message-ID-Hash: WTZV6RKC5HT4JNJMBIUMEKE4I7JA5VJS 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:09PM +0200, Hannes Laimer wrote: > Add 'move-namespace' and 'move-group' subcommands to > proxmox-backup-manager datastore. Both call the corresponding API > handler and wait for the worker task to complete. > > Signed-off-by: Hannes Laimer Hey, one comment & one nit inline > --- > src/bin/proxmox_backup_manager/datastore.rs | 84 ++++++++++++++++++++- > 1 file changed, 83 insertions(+), 1 deletion(-) > > diff --git a/src/bin/proxmox_backup_manager/datastore.rs b/src/bin/proxmox_backup_manager/datastore.rs > index 5c65c5ec..2ee56fba 100644 > --- a/src/bin/proxmox_backup_manager/datastore.rs > +++ b/src/bin/proxmox_backup_manager/datastore.rs > @@ -1,5 +1,6 @@ > use pbs_api_types::{ > - DataStoreConfig, DataStoreConfigUpdater, DATASTORE_SCHEMA, PROXMOX_CONFIG_DIGEST_SCHEMA, > + BackupNamespace, DataStoreConfig, DataStoreConfigUpdater, DATASTORE_SCHEMA, > + PROXMOX_CONFIG_DIGEST_SCHEMA, > }; > use pbs_client::view_task_result; > use proxmox_router::{cli::*, ApiHandler, RpcEnvironment}; > @@ -323,6 +324,75 @@ async fn uuid_mount(mut param: Value, _rpcenv: &mut dyn RpcEnvironment) -> Resul > Ok(Value::Null) > } > > +#[api( > + protected: true, > + input: { > + properties: { > + store: { > + schema: DATASTORE_SCHEMA, > + }, > + ns: { > + type: BackupNamespace, > + }, > + "new-ns": { > + type: BackupNamespace, > + }, > + }, > + }, > +)] > +/// Move a backup namespace to a new location within the same datastore. > +async fn cli_move_namespace( > + mut param: Value, > + rpcenv: &mut dyn RpcEnvironment, > +) -> Result<(), Error> { > + param["node"] = "localhost".into(); > + > + let info = &api2::admin::namespace::API_METHOD_MOVE_NAMESPACE; > + let result = match info.handler { > + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, > + _ => unreachable!(), > + }; > + > + crate::wait_for_local_worker(result.as_str().unwrap()).await?; > + Ok(()) > +} nit: The `move-namespace` command outputs a log message from `DataStore::with_store_and_config` ("using datastore cache ..."). That's unrelated to these changes, but it's a little confusing since that ends up being the only output you get when moving a namespace. IMO this would benefit from some kind of success message to make it clear that the move happened. > + > +#[api( > + protected: true, > + input: { > + properties: { > + store: { > + schema: DATASTORE_SCHEMA, > + }, > + ns: { > + type: BackupNamespace, > + optional: true, > + }, > + group: { > + type: pbs_api_types::BackupGroup, > + flatten: true, > + }, > + "new-ns": { > + type: BackupNamespace, > + optional: true, > + }, > + }, > + }, > +)] > +/// Move a backup group to a different namespace within the same datastore. > +async fn cli_move_group(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<(), Error> { > + param["node"] = "localhost".into(); > + > + let info = &api2::admin::datastore::API_METHOD_MOVE_GROUP; > + let result = match info.handler { > + ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?, > + _ => unreachable!(), > + }; > + > + crate::wait_for_local_worker(result.as_str().unwrap()).await?; > + Ok(()) > +} It is not clear to me why not setting a source/target operand should automatically imply the root namespace. I assume this is to reflect the actual namespace structure, where root is actually stored as '', but I think it would be cleaner if there was a way to make it explicit, or if that's not possible (not sure if there is even a keyword we can reserve for this), at least document it. > #[api( > protected: true, > input: { > @@ -407,6 +477,18 @@ pub fn datastore_commands() -> CommandLineInterface { > CliCommand::new(&API_METHOD_DELETE_DATASTORE) > .arg_param(&["name"]) > .completion_cb("name", pbs_config::datastore::complete_datastore_name), > + ) > + .insert( > + "move-namespace", > + CliCommand::new(&API_METHOD_CLI_MOVE_NAMESPACE) > + .arg_param(&["store"]) > + .completion_cb("store", pbs_config::datastore::complete_datastore_name), > + ) > + .insert( > + "move-group", > + CliCommand::new(&API_METHOD_CLI_MOVE_GROUP) > + .arg_param(&["store"]) > + .completion_cb("store", pbs_config::datastore::complete_datastore_name), > ); > > cmd_def.into() > -- > 2.47.3 > > > > >