From: Filip Schauer <f.schauer@proxmox.com>
To: Max Carrara <m.carrara@proxmox.com>,
Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH v4 vma-to-pbs 5/6] Add support for streaming the VMA file via stdin
Date: Tue, 12 Mar 2024 15:04:39 +0100 [thread overview]
Message-ID: <8f97a7af-af75-4414-b40e-a77b51a23c10@proxmox.com> (raw)
In-Reply-To: <9a4d9e4c-3175-44a4-aa02-61e68b2c4b2a@proxmox.com>
On 06/03/2024 16:49, Max Carrara wrote:
> Regarding `F: Fn(u8, u64, Option<Vec<u8>>) -> Result<()>`:
> * Why `Fn` and not `FnOnce`? You call this with a closure later on.
It is called multiple times in the for loop in restore_extent.
>> + let is_zero = blockinfo.mask == 0;
> I'm usually in favour of assigning the result of conditional checks
> to variables first, but is that really necessary here?
>
>>
>> - while file_offset < vma_file_size {
>> - self.vma_file.seek(SeekFrom::Start(file_offset))?;
>> - let vma_extent_header = Self::read_extent_header(&mut self.vma_file)?;
>> - file_offset += size_of::<VmaExtentHeader>() as u64;
>> + let image_chunk_buffer = if is_zero {
> It's only used here after all, and inlining it wouldn't make the code
> more complex at all.
Not necessary but it improves readability in my opinion.
> Also,
> `Vec<u8>` could probably be `&[u8]`, couldn't it?
No, because then the image_chunk_buffer would not live long enough.
The chunk needs to live past func so it can be stored in the
images_chunks HashMap in vma2pbs.rs. This HashMap can hold multiple
chunks before they are sent off as one big chunk to the PBS.
next prev parent reply other threads:[~2024-03-12 14:05 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-05 13:56 [pbs-devel] [PATCH v4 vma-to-pbs 0/6] Implement vma-to-pbs tool Filip Schauer
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 1/6] Initial commit Filip Schauer
2024-03-06 15:49 ` Max Carrara
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 2/6] cargo fmt Filip Schauer
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 3/6] bump proxmox-backup-qemu Filip Schauer
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 4/6] Add the ability to provide credentials via files Filip Schauer
2024-03-06 15:49 ` Max Carrara
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 5/6] Add support for streaming the VMA file via stdin Filip Schauer
2024-03-06 15:49 ` Max Carrara
2024-03-12 14:04 ` Filip Schauer [this message]
2024-03-05 13:56 ` [pbs-devel] [PATCH v4 vma-to-pbs 6/6] Add a fallback for the --fingerprint argument Filip Schauer
2024-03-06 15:49 ` [pbs-devel] [PATCH v4 vma-to-pbs 0/6] Implement vma-to-pbs tool Max Carrara
2024-03-06 15:57 ` Wolfgang Bumiller
2024-03-20 14:18 ` Filip Schauer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8f97a7af-af75-4414-b40e-a77b51a23c10@proxmox.com \
--to=f.schauer@proxmox.com \
--cc=m.carrara@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox