Re: [pbs-devel] [PATCH backup] use BufReader/Writer for Files passed to serde_json::from_reader/writer

[Mark Schouten <mark@tuxis.nl>]

Hi Wolfgang,

Thanks for the quick response (even though I somehow missed it). Reported-by is fine with me!

— 
Mark Schouten, CTO
Tuxis B.V.
mark@tuxis.nl



> On 6 Apr 2022, at 14:51, Wolfgang Bumiller <w.bumiller@proxmox.com> wrote:
> 
> As serde_json will otherwise read files 1 byte at a time.
> Writing is a bit better, but syntacitcal elements (quotes, braces,
> commas) still often show up as single write syscalls, so use BufWriter
> there as well.
> 
> Note that while we do store the file in the resulting objects, we do not
> need to keep the buffered read/writers as we always `seek` to the
> beginning on further file operations.
> 
> Reported-by: Mark Schouten <mark@tuxis.nl>
> Link: https://lists.proxmox.com/pipermail/pbs-devel/2022-April/004909.html
> Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
> ---
> @Mark: Hope the Reported-by tag is fine with you.
> 
> proxmox-file-restore/src/block_driver_qemu.rs | 8 ++++----
> src/config/tfa.rs                             | 6 +++---
> 2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/proxmox-file-restore/src/block_driver_qemu.rs b/proxmox-file-restore/src/block_driver_qemu.rs
> index 0176f7f1..ac61f917 100644
> --- a/proxmox-file-restore/src/block_driver_qemu.rs
> +++ b/proxmox-file-restore/src/block_driver_qemu.rs
> @@ -1,7 +1,7 @@
> //! Block file access via a small QEMU restore VM using the PBS block driver in QEMU
> use std::collections::HashMap;
> use std::fs::{File, OpenOptions};
> -use std::io::{prelude::*, SeekFrom};
> +use std::io::{prelude::*, BufReader, BufWriter, SeekFrom};
> 
> use anyhow::{bail, Error};
> use futures::FutureExt;
> @@ -51,7 +51,7 @@ impl VMStateMap {
>     fn load() -> Result<Self, Error> {
>         let mut file = Self::open_file_raw(true)?;
>         lock_file(&mut file, true, Some(std::time::Duration::from_secs(120)))?;
> -        let map = serde_json::from_reader(&file).unwrap_or_default();
> +        let map = serde_json::from_reader(BufReader::new(&mut file)).unwrap_or_default();
>         Ok(Self { map, file })
>     }
> 
> @@ -59,14 +59,14 @@ impl VMStateMap {
>     /// shell auto-completion, for anything requiring consistency use load() !
>     fn load_read_only() -> Result<HashMap<String, VMState>, Error> {
>         let file = Self::open_file_raw(false)?;
> -        Ok(serde_json::from_reader(&file).unwrap_or_default())
> +        Ok(serde_json::from_reader(BufReader::new(file)).unwrap_or_default())
>     }
> 
>     /// Write back a potentially modified state map, consuming the held lock
>     fn write(mut self) -> Result<(), Error> {
>         self.file.seek(SeekFrom::Start(0))?;
>         self.file.set_len(0)?;
> -        serde_json::to_writer(self.file, &self.map)?;
> +        serde_json::to_writer(BufWriter::new(&mut self.file), &self.map)?;
> 
>         // drop ourselves including file lock
>         Ok(())
> diff --git a/src/config/tfa.rs b/src/config/tfa.rs
> index 790e0960..7120553c 100644
> --- a/src/config/tfa.rs
> +++ b/src/config/tfa.rs
> @@ -40,7 +40,7 @@ pub fn read() -> Result<TfaConfig, Error> {
>         Err(err) => return Err(err.into()),
>     };
> 
> -    Ok(serde_json::from_reader(file)?)
> +    Ok(serde_json::from_reader(io::BufReader::new(file))?)
> }
> 
> pub(crate) fn webauthn_config_digest(config: &WebauthnConfig) -> Result<[u8; 32], Error> {
> @@ -116,7 +116,7 @@ impl TfaUserChallengeData {
>     fn save(mut self) -> Result<(), Error> {
>         self.rewind()?;
> 
> -        serde_json::to_writer(&mut &self.lock, &self.inner).map_err(|err| {
> +        serde_json::to_writer(io::BufWriter::new(&mut &self.lock), &self.inner).map_err(|err| {
>             format_err!("failed to update challenge file {:?}: {}", self.path, err)
>         })?;
> 
> @@ -293,7 +293,7 @@ impl proxmox_tfa::api::OpenUserChallengeData for UserAccess {
> 
>         proxmox_sys::fs::lock_file(&mut file, true, None)?;
> 
> -        let inner = serde_json::from_reader(&mut file).map_err(|err| {
> +        let inner = serde_json::from_reader(io::BufReader::new(&mut file)).map_err(|err| {
>             format_err!("failed to read challenge data for user {}: {}", userid, err)
>         })?;
> 
> -- 
> 2.30.2
> 
>