Re: [pbs-devel] [PATCH v3 proxmox-backup 0/5] fix: #3847 pipe from STDIN to proxmox-backup-client

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

On January 9, 2026 6:35 pm, Robert Obkircher wrote:
> Add support for commands like:
>     ssh host cmd | proxmox-backup-client backup data.img:/dev/stdin
>     proxmox-backup-client backup a.img:<(mysqldump) b.img:<(pgdump)

this is already shaping up nicely, some comments on individual patches,
and some feedback for the organization of the series:

the actual fix for the referenced bug is the last two patches - the rest
is just preparation work but has no effect yet until it is wired up in
the client (AFAICT?). that means that the "fix .." prefix should only go
into those two patches (or technically even just #4, since #5 is just
syntactic sugar on top).

please ensure each commit builds when applied in series. the first patch
breaks the backup API and should include changes making it compatible
(i.e., wrapping things in Some(..) as needed). the third patch would
then replace those compat changes with making it optional in the API and
handling that correctly.

thanks!

> 
> Changes since v2:
> 
> client: 
> - Renamed ChunkSize to IndexType as suggested.
> 
> datastore:
> - Introduced explicit `index_capacity` field.
>     - Previously I (ab)used `index_length` for the capacity and 
>       computed the actual index length from `size`.
>     - This removes the assert mentioned in the review.
> - Renamed INITIAL_CHUNKS_IF_UNKNOWN to INITIAL_CAPACITY and increased 
>   it to 128 blocks which is enough for 512 MiB of content.
> - Only remap when capacity increases, not every time size increases.
> - Increase capacity to next power of 2 instead of by 1.5x.
>     - This is less code and file systems probably don't do in place
>       updates where 1.5x is theoretically better.
> - Ensure that add_digest and close fail after remap errors.
> - Keep seek+write_all instead of write_all_at to minimize changes.
> - Imroved error messages in clone_data_from.
> - Refuse creation of empty files because the original also did that.
> - Added tests for FixedIndexWriter:
>   - Is it okay to to write to a directory in the cwd?
>   - This was inspired by `chunk_store::test_chunk_store1`
> - Removed test script.
> 
> Changes since v1:
> - use mremap+ftruncate instead of write_all_at
> - make the size API parameter optional instead of using 0
> - use an enum to represent fixed/dynamic chunk size in UploadOptions
> - alias "-" to "/dev/stdin"
> - split changes into separate commits
> 
> Robert Obkircher (5):
>   fix #3847: datastore: support writing fidx files of unknown size
>   fix #3847: datastore: test FixedIndexWriter
>   fix #3847: api: backup: make fixed index file size optional
>   fix #3847: client: support fifo pipe inputs for images
>   fix #3847: client: treat minus sign as stdin
> 
>  pbs-client/src/backup_writer.rs   |  38 ++--
>  pbs-datastore/src/datastore.rs    |   2 +-
>  pbs-datastore/src/fixed_index.rs  | 304 +++++++++++++++++++++++++++++-
>  proxmox-backup-client/src/main.rs |  37 ++--
>  src/api2/backup/environment.rs    |   8 +-
>  src/api2/backup/mod.rs            |   4 +-
>  src/server/push.rs                |  11 +-
>  7 files changed, 366 insertions(+), 38 deletions(-)
> 
> -- 
> 2.47.3
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 


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