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 881741FF145 for ; Thu, 05 Feb 2026 15:25:51 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 9AC0B12AC5; Thu, 5 Feb 2026 15:26:24 +0100 (CET) Content-Type: text/plain; charset=UTF-8 Date: Thu, 05 Feb 2026 15:25:50 +0100 Message-Id: To: "Shan Shaji" , Subject: Re: [PATCH datacenter-manager v2 1/4] cli: admin: make cli handling async From: "Lukas Wagner" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260203175101.457724-1-s.shaji@proxmox.com> <20260203175101.457724-2-s.shaji@proxmox.com> In-Reply-To: <20260203175101.457724-2-s.shaji@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770301473097 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.038 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: GEN5OBWDDGLQJ7YJUFOKOBDCVLWIZMZ4 X-Message-ID-Hash: GEN5OBWDDGLQJ7YJUFOKOBDCVLWIZMZ4 X-MailFrom: l.wagner@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 Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Hi Shan, some comments inline. On Tue Feb 3, 2026 at 6:50 PM CET, Shan Shaji wrote: > The acme API methods internally create workers to process API requests. > An error was thrown if the methods invoked without initializing the > worker tasks. > > Since `init_worker_tasks` needs to be called from an async runtime. > Moved the content of the main function to async `run` function and > wrapped using `proxmox_async::runtime::main`, which creates a Tokio > runtime. > > Signed-off-by: Shan Shaji > --- > changes since v1: > - use `tasklog` function to capture worker task logs to the tasklog > file.=20 > - add `context` method to preserve the error messages.=20 > > cli/admin/Cargo.toml | 3 +++ > cli/admin/src/main.rs | 53 ++++++++++++++++++++++++++++--------------- > 2 files changed, 38 insertions(+), 18 deletions(-) > > diff --git a/cli/admin/Cargo.toml b/cli/admin/Cargo.toml > index 0dec423..e566b39 100644 > --- a/cli/admin/Cargo.toml > +++ b/cli/admin/Cargo.toml > @@ -19,6 +19,9 @@ proxmox-product-config.workspace =3D true > proxmox-router =3D { workspace =3D true, features =3D [ "cli" ], default= -features =3D false } > proxmox-schema =3D { workspace =3D true, features =3D [ "api-macro" ] } > proxmox-access-control.workspace =3D true > +proxmox-rest-server.workspace =3D true > +proxmox-sys.workspace =3D true > +proxmox-daemon.workspace =3D true > =20 > pdm-api-types.workspace =3D true > pdm-config.workspace =3D true > diff --git a/cli/admin/src/main.rs b/cli/admin/src/main.rs > index f698fa2..02148e3 100644 > --- a/cli/admin/src/main.rs > +++ b/cli/admin/src/main.rs > @@ -1,35 +1,33 @@ > +use anyhow::{Context, Error}; > use serde_json::{json, Value}; > =20 > use proxmox_router::cli::{ > - default_table_format_options, format_and_print_result_full, get_outp= ut_format, run_cli_command, > - CliCommand, CliCommandMap, CliEnvironment, ColumnConfig, OUTPUT_FORM= AT, > + default_table_format_options, format_and_print_result_full, get_outp= ut_format, > + run_async_cli_command, CliCommand, CliCommandMap, CliEnvironment, Co= lumnConfig, OUTPUT_FORMAT, > }; > use proxmox_router::RpcEnvironment; > - > use proxmox_schema::api; > +use proxmox_sys::fs::CreateOptions; > =20 > mod remotes; > mod support_status; > =20 > -fn main() { > - //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to pro= xmox-sys and uncomment > - > - let api_user =3D pdm_config::api_user().expect("cannot get api user"= ); > - let priv_user =3D pdm_config::priv_user().expect("cannot get privile= ged user"); > - proxmox_product_config::init(api_user, priv_user); > +async fn run() -> Result<(), Error> { > + let api_user =3D pdm_config::api_user().context("could not get api u= ser")?; > + let priv_user =3D pdm_config::priv_user().context("could not get pri= vileged user")?; > =20 > + proxmox_product_config::init(api_user.clone(), priv_user); > proxmox_access_control::init::init( > &pdm_api_types::AccessControlConfig, > pdm_buildcfg::configdir!("/access"), > ) > - .expect("failed to setup access control config"); > - > + .context("failed to setup access control config")?; > proxmox_log::Logger::from_env("PDM_LOG", proxmox_log::LevelFilter::I= NFO) > .stderr() > .init() > - .expect("failed to set up logger"); > + .context("failed to set-up logger")?; The string here changed, the original version seems to be more correct to me > =20 > - server::context::init().expect("could not set up server context"); > + server::context::init().context("could not set-up server context")?; > =20 > let cmd_def =3D CliCommandMap::new() > .insert("remote", remotes::cli()) > @@ -40,14 +38,33 @@ fn main() { > .insert("support-status", support_status::cli()) > .insert("versions", CliCommand::new(&API_METHOD_GET_VERSIONS)); > =20 > + let args: Vec =3D std::env::args().collect(); > + let avoid_init =3D args.len() >=3D 2 && (args[1] =3D=3D "bashcomplet= e" || args[1] =3D=3D "printdoc"); For what it's worth, this could be let avoid_init =3D matches!( args.get(1).map(String::as_str), Some("bashcomplete") | Some("printdoc") ); or let avoid_init =3D args .get(1) .is_some_and(|c| c =3D=3D "bashcomplete" || c =3D=3D "printdoc"); It's not really shorter, but I think they would be a bit more idiomatic. The original version relies on the short-circuit behavior of the && operator to avoid the panic when accessing the array element via the index operator, which is of course correct, but I think this can be written a bit nicer. no hard feelings, so just pick whichever version you like most. > + > + if !avoid_init { > + let file_opts =3D CreateOptions::new().owner(api_user.uid).group= (api_user.gid); > + proxmox_rest_server::init_worker_tasks(pdm_buildcfg::PDM_LOG_DIR= _M!().into(), file_opts) > + .context("failed to initialize worker tasks")?; > + > + let mut command_sock =3D proxmox_daemon::command_socket::Command= Socket::new(api_user.gid); > + proxmox_rest_server::register_task_control_commands(&mut command= _sock) > + .context("failed to register task control commands")?; > + command_sock > + .spawn(proxmox_rest_server::last_worker_future()) > + .context("failed to activate the socket")?; > + } > + > let mut rpcenv =3D CliEnvironment::new(); > rpcenv.set_auth_id(Some("root@pam".into())); > =20 > - run_cli_command( > - cmd_def, > - rpcenv, > - Some(|future| proxmox_async::runtime::main(future)), > - ); > + run_async_cli_command(cmd_def, rpcenv).await; > + > + Ok(()) > +} > + > +fn main() -> Result<(), Error> { > + //pbs_tools::setup_libc_malloc_opts(); // TODO: move from PBS to pro= xmox-sys and uncomment > + proxmox_async::runtime::main(run()) > } > =20 > #[api(