From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 15B617A802; Tue, 5 Jul 2022 13:40:13 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 09E652E58C; Tue, 5 Jul 2022 13:39:43 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS; Tue, 5 Jul 2022 13:39:42 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 3469040F01; Tue, 5 Jul 2022 13:39:42 +0200 (CEST) Date: Tue, 5 Jul 2022 13:39:41 +0200 From: Wolfgang Bumiller To: Dominik Csapak Cc: pve-devel@lists.proxmox.com, pbs-devel@lists.proxmox.com Message-ID: <20220705113941.kj4qawt6v63owiqm@casey.proxmox.com> References: <20220531111726.2972022-1-d.csapak@proxmox.com> <20220531111726.2972022-4-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220531111726.2972022-4-d.csapak@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.295 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% 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 T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [api.rs] Subject: Re: [pve-devel] [pbs-devel] [PATCH proxmox-backup 1/2] restore-daemon: add 'format' parameter to the 'extract' handler X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Jul 2022 11:40:13 -0000 On Tue, May 31, 2022 at 01:17:22PM +0200, Dominik Csapak wrote: > this can be 'plain', 'pxar', 'zip' or 'tar.zst', and it returns the > content in the given format (with fallback to the old behaviour if not > given) > > Signed-off-by: Dominik Csapak > --- > note: needs a bumped 'proxmox-compression' in the Cargo.toml to build > > .../src/proxmox_restore_daemon/api.rs | 49 ++++++++++++++++--- > 1 file changed, 43 insertions(+), 6 deletions(-) > > diff --git a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > index aeb5a71d..c4977ce6 100644 > --- a/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > +++ b/proxmox-restore-daemon/src/proxmox_restore_daemon/api.rs > @@ -13,7 +13,7 @@ use serde_json::Value; > use tokio::sync::Semaphore; > > use pathpatterns::{MatchEntry, MatchPattern, MatchType, Pattern}; > -use proxmox_compression::zip::zip_directory; > +use proxmox_compression::{tar::tar_directory, zip::zip_directory, zstd::ZstdEncoder}; > use proxmox_router::{ > list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture, Permission, Router, > RpcEnvironment, SubdirMap, > @@ -236,11 +236,24 @@ pub const API_METHOD_EXTRACT: ApiMethod = ApiMethod::new( > true, > &BooleanSchema::new(concat!( > "if true, return a pxar archive, otherwise either the ", > - "file content or the directory as a zip file" > + "file content or the directory as a zip file. DEPRECATED: use 'format' instead." > )) > .default(true) > .schema() > - ) > + ), > + ( > + "format", > + true, > + &StringSchema::new("The desired format of the result.") > + .format(&ApiStringFormat::Enum(&[ > + EnumEntry::new("plain", "Plain file (only works for single files)"), > + EnumEntry::new("pxar", "PXAR archive"), > + EnumEntry::new("zip", "ZIP archive"), > + EnumEntry::new("tar.zst", "Zstd compressed TAR archive"), > + ])) > + .default("pxar") > + .schema() If you make an `#[api] enum RestoreFormat {}` and `use proxmox_schema::ApiType` you can pass the enum's `RestoreFormat::API_SCHEMA` in here. Would be nicer than using strings... (you can use `#[serde(rename)]` for the "tar.zst" value). On the other hand... We actually use a *tar* encoder, not a *tar.zstd* encoder (obviously, given the nature of how this works), which makes me wonder, shouldn't `zstd` maybe be a separate boolean? I do think for a large single file "plain+zstd" makes sense, also zstd-compressing a pxar might be worthwhile... (And while zip+zstd doesn't make *that* much sense... I wouldn't really care much either ;-) ) > + ), > ]), > ), > ) > @@ -271,6 +284,11 @@ fn extract( > let path = Path::new(OsStr::from_bytes(&path[..])); > > let pxar = param["pxar"].as_bool().unwrap_or(true); > + let format = match param["format"].as_str() { > + Some(format) => format.to_string(), Maybe consider erroring if both `pxar` and `format` are set here? Otherwise move the pxar binding into the None scope so it's gone afterwards, it's a long function ;-) > + // FIXME: old default was plain or zip, remove that with 3.0 > + None => if pxar { "pxar".to_string() } else { String::new() } > + }; > > let query_result = proxmox_async::runtime::block_in_place(move || { > let mut disk_state = crate::DISK_STATE.lock().unwrap();