Repository navigation
feat(snapshot): support full dumps to non-seekable files - #6232
coffeeinit wants to merge 5 commits into
Conversation
| } | ||
|
|
||
| fn dump_non_seekable<T: WriteVolatile>(&self, writer: &mut T) -> Result<(), MemoryError> { | ||
| let zeroes = &mut [0u8; 4096]; |
There was a problem hiding this comment.
Writing each 4KiB separately might be terribly inefficient since each write to the FIFO results in a syscall.
I'd argue that using a bigger buffer would yield more performance. E.g. using a vec![0u8; chunk], where chunk is the length to "seek" limited to let's say 2MiB.
Could you also check whether a vec![0u8; 2MiB] call on a normal system without hugepages/THP actually consumes RSS from Firecracker, or is Rust allocator+kernel able to optimize this to use the shared zero page as long as we don't write to it? I think both glibc and musl should do it, but please double check me.
There was a problem hiding this comment.
Removed this dump_non_seekable + 4K stack. Now reused dump(writer, HoleStrategy) with shared write_zeros() using [0u8; 64K] stack, chunked by min(remaining,64K).
On RSS: confirmed your guess — vec![0u8;2M] read-only would stay on shared zero-page COW on both glibc and musl without THP/hugetlb, only page-tables. Used 64K stack instead to get same benefit with no heap and pipe-aligned writes.
There was a problem hiding this comment.
The optimization I was mentioning only happens: on the heap, and (2) if the allocation is at least 128KiB (on glibc at least)
a397c0c to
1029943
Compare
| if len == 0 { | ||
| return Ok(()); | ||
| } | ||
| let mut zeroes = [0u8; ZERO_FILL_CHUNK_SIZE]; |
There was a problem hiding this comment.
Does it need to be mutable?
Also, like I mentioned in the other comment, this won't optimize down to re-use the shared zero page from the kernel. In this form, it will actually memset.
1029943 to
220c76e
Compare
Signed-off-by: Janarthanan <janasuba2307@gmail.com>
Signed-off-by: Janarthanan <janasuba2307@gmail.com>
f9322ea to
4a73034
Compare
Signed-off-by: Janarthanan <janasuba2307@gmail.com>
Changes
Full snapshots (
SnapshotType::Full) can now write guest memory tonon-seekable files such as FIFOs, so memory can be streamed to another
process (compression, encryption, upload) without landing on local disk.
snapshot_memory_to_file()detects non-regular files viametadata().file_type().is_file()and skipsset_len()/truncate-sizehandling/
sync_all()for them (previously failed with EINVAL).GuestMemoryExtension::dump_non_seekable(): writes unplugged slotsas zeroes in 4K chunks instead of
seek()ing over them (previouslypanicked with ESPIPE via
.unwrap()).dump_dirty()(diff path) intentionally untouched: seeked-means-cleanis load-bearing there.
test_full_snapshot_to_fifo(integration, FIFO drained on areader thread, bytes equal to a regular snapshot) and
test_dump_non_seekable_matches_dump(unit,Vec<u8>as thenon-seekable writer). Implements PR-1 of [Feature Request] Support non-seekable file descriptors (e.g. FIFOs) for snapshot memory output #6030 as agreed with
marco-marangoni.Reason
Support pointing
mem_file_pathat a FIFO to stream snapshot memory(#6030). Follow-up efficient sparse wire format intentionally left for
PR-2 per maintainer direction.
License Acceptance
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license. For more information on
following Developer Certificate of Origin and signing off your commits,
please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures. (runs in Buildkite)
tools/devtool checkstyle --no-clippyto verify that the PRpasses the automated style checks. (added lines verified nightly-fmt
clean; full checkstyle runs in Buildkite)
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.PR-1 of [Feature Request] Support non-seekable file descriptors (e.g. FIFOs) for snapshot memory output #6030; wire-format follow-up remains)
Runbook for Firecracker API changes. (no API surface change)
integration tests.
TODO. (no new TODOs)rust-vmm. (Firecrackersnapshot-path specific: plugged-slot handling and
KvmVmbacking-filelogic)