Re: [pbs-devel] [PATCH v2 proxmox-backup 3/5] fix #3847: client: support fifo pipe inputs for images

[Christian Ebner <c.ebner@proxmox.com>]

some comments inline

On 12/19/25 5:19 PM, Robert Obkircher wrote:
> Accept fifo files as inputs for images and omit the size when
> uploading the fixed index file.
> 
> Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
> ---
>   pbs-client/src/backup_writer.rs   | 37 ++++++++++++++++++++++---------
>   proxmox-backup-client/src/main.rs | 30 ++++++++++++++-----------
>   src/server/push.rs                | 13 ++++++-----
>   3 files changed, 51 insertions(+), 29 deletions(-)
> 
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index dbd177d8..1963b700 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -52,7 +52,16 @@ pub struct UploadOptions {
>       pub previous_manifest: Option<Arc<BackupManifest>>,
>       pub compress: bool,
>       pub encrypt: bool,
> -    pub fixed_size: Option<u64>,
> +    pub chunk_size: ChunkSize,

above is ill-named as this is not the chunk size, but rather the image 
file size.
I suggest to rename this to
```
index_type: IndexType
```
or another even better fitting name and define the IndexType with tuple 
enum variant for the size

> +}
> +
> +#[derive(Default, Clone)]
> +pub enum ChunkSize {
> +    #[default]
> +    Dynamic,
> +    Fixed {
> +        file_size: Option<u64>,
> +    },

/// Index type for upload options
pub enum IndexType {
     #[default]
     /// Dynamic chunking
     Dynamic,
     /// Fixed size chunking with optional image file size
     Fixed(Option<u64>),
}

>   }
>   
>   struct ChunkUploadResponse {
> @@ -292,11 +301,14 @@ impl BackupWriter {
>           options: UploadOptions,
>       ) -> Result<BackupStats, Error> {
>           let mut param = json!({ "archive-name": archive_name });
> -        let prefix = if let Some(size) = options.fixed_size {
> -            param["size"] = size.into();
> -            "fixed"
> -        } else {
> -            "dynamic"
> +        let prefix = match options.chunk_size {
> +            ChunkSize::Fixed { file_size } => {

... above makes this to
```
IndexType::Fixed(file_size) => {
```
> +                if let Some(size) = file_size {
> +                    param["size"] = size.into();
> +                }
> +                "fixed"
> +            }
> +            ChunkSize::Dynamic => "dynamic",

and
```
IndexType::Dynamic => "dynamic",
```
as well as for other occurences.

>           };
>   
>           if options.encrypt && self.crypt_config.is_none() {
> @@ -387,11 +399,14 @@ impl BackupWriter {
>           let known_chunks = Arc::new(Mutex::new(HashSet::new()));
>   
>           let mut param = json!({ "archive-name": archive_name });
> -        let prefix = if let Some(size) = options.fixed_size {
> -            param["size"] = size.into();
> -            "fixed"
> -        } else {
> -            "dynamic"
> +        let prefix = match options.chunk_size {
> +            ChunkSize::Fixed { file_size } => {
> +                if let Some(size) = file_size {
> +                    param["size"] = size.into();
> +                }
> +                "fixed"
> +            }
> +            ChunkSize::Dynamic => "dynamic",
>           };
>   
>           if options.encrypt && self.crypt_config.is_none() {
> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 999e5020..828643da 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -46,7 +46,7 @@ use pbs_client::tools::{
>   use pbs_client::{
>       delete_ticket_info, parse_backup_specification, view_task_result, BackupDetectionMode,
>       BackupReader, BackupRepository, BackupSpecificationType, BackupStats, BackupWriter,
> -    BackupWriterOptions, ChunkStream, FixedChunkStream, HttpClient, InjectionData,
> +    BackupWriterOptions, ChunkSize, ChunkStream, FixedChunkStream, HttpClient, InjectionData,
>       PxarBackupStream, RemoteChunkReader, UploadOptions, BACKUP_SOURCE_SCHEMA,
>   };
>   use pbs_datastore::catalog::{BackupCatalogWriter, CatalogReader, CatalogWriter};
> @@ -205,7 +205,7 @@ async fn backup_directory<P: AsRef<Path>>(
>       pxar_create_options: pbs_client::pxar::PxarCreateOptions,
>       upload_options: UploadOptions,
>   ) -> Result<(BackupStats, Option<BackupStats>), Error> {
> -    if upload_options.fixed_size.is_some() {
> +    if let ChunkSize::Fixed { .. } = upload_options.chunk_size {
>           bail!("cannot backup directory with fixed chunk size!");
>       }
>   
> @@ -295,7 +295,7 @@ async fn backup_image<P: AsRef<Path>>(
>   
>       let stream = FixedChunkStream::new(stream, chunk_size.unwrap_or(4 * 1024 * 1024));
>   
> -    if upload_options.fixed_size.is_none() {
> +    if let ChunkSize::Dynamic = upload_options.chunk_size {
>           bail!("cannot backup image with dynamic chunk size!");
>       }
>   
> @@ -859,15 +859,17 @@ async fn create_backup(
>                   upload_list.push((BackupSpecificationType::PXAR, filename, target, "didx", 0));
>               }
>               BackupSpecificationType::IMAGE => {
> -                if !(file_type.is_file() || file_type.is_block_device()) {
> -                    bail!("got unexpected file type (expected file or block device)");
> -                }
> -
> -                let size = image_size(&PathBuf::from(&filename))?;
> -
> -                if size == 0 {
> -                    bail!("got zero-sized file '{}'", filename);
> -                }
> +                let size = if file_type.is_file() || file_type.is_block_device() {
> +                    let size = image_size(&PathBuf::from(&filename))?;
> +                    if size == 0 {
> +                        bail!("got zero-sized file '{}'", filename);
> +                    }
> +                    size
> +                } else if file_type.is_fifo() {
> +                    0
> +                } else {
> +                    bail!("got unexpected file type (expected file, block device, or fifo");
> +                };
>   
>                   upload_list.push((
>                       BackupSpecificationType::IMAGE,
> @@ -1191,9 +1193,11 @@ async fn create_backup(
>               (BackupSpecificationType::IMAGE, false) => {
>                   log_file("image", &filename, target.as_ref());
>   
> +                // 0 means fifo pipe with unknown size
> +                let file_size = (size != 0).then_some(size);
>                   let upload_options = UploadOptions {
>                       previous_manifest: previous_manifest.clone(),
> -                    fixed_size: Some(size),
> +                    chunk_size: ChunkSize::Fixed { file_size },
>                       compress: true,
>                       encrypt: crypto.mode == CryptMode::Encrypt,
>                   };
> diff --git a/src/server/push.rs b/src/server/push.rs
> index d7884fce..a1216ba9 100644
> --- a/src/server/push.rs
> +++ b/src/server/push.rs
> @@ -17,7 +17,8 @@ use pbs_api_types::{
>       PRIV_REMOTE_DATASTORE_MODIFY, PRIV_REMOTE_DATASTORE_PRUNE,
>   };
>   use pbs_client::{
> -    BackupRepository, BackupWriter, BackupWriterOptions, HttpClient, MergedChunkInfo, UploadOptions,
> +    BackupRepository, BackupWriter, BackupWriterOptions, ChunkSize, HttpClient, MergedChunkInfo,
> +    UploadOptions,
>   };
>   use pbs_config::CachedUserInfo;
>   use pbs_datastore::data_blob::ChunkInfo;
> @@ -917,7 +918,7 @@ pub(crate) async fn push_snapshot(
>                           index,
>                           chunk_reader,
>                           &backup_writer,
> -                        None,
> +                        ChunkSize::Dynamic,
>                           known_chunks.clone(),
>                       )
>                       .await?;
> @@ -944,7 +945,9 @@ pub(crate) async fn push_snapshot(
>                           index,
>                           chunk_reader,
>                           &backup_writer,
> -                        Some(size),
> +                        ChunkSize::Fixed {
> +                            file_size: Some(size),
> +                        },
>                           known_chunks.clone(),
>                       )
>                       .await?;
> @@ -1002,7 +1005,7 @@ async fn push_index(
>       index: impl IndexFile + Send + 'static,
>       chunk_reader: Arc<dyn AsyncReadChunk>,
>       backup_writer: &BackupWriter,
> -    size: Option<u64>,
> +    chunk_size: ChunkSize,
>       known_chunks: Arc<Mutex<HashSet<[u8; 32]>>>,
>   ) -> Result<SyncStats, Error> {
>       let (upload_channel_tx, upload_channel_rx) = mpsc::channel(20);
> @@ -1048,7 +1051,7 @@ async fn push_index(
>       let upload_options = UploadOptions {
>           compress: true,
>           encrypt: false,
> -        fixed_size: size,
> +        chunk_size,
>           ..UploadOptions::default()
>       };
>   



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel