Skip to content

Commit 72d733e

Browse files
fkm3crosvm LUCI
authored and
crosvm LUCI
committed
snapshot: serialize snapshots using CBOR
The main motivation for this change is support more efficient snapshotting of binary data for virtio-gpu 3d backends. It isn't expected to make much of a difference for existing snapshot support. Even though this adds a new dependency, the overall crosvm binary size goes down a bit (-30kb). From some quick experimentation, I noticed that only switching to `ciborium::Value` (from `serde_json::Value`) netted a bigger binary size drop, so it is probably the conversion to/from `AnySnapshot` that is responsible. Excluding the RAM snapshot file, the total snapshot size goes from 108K to 88K. Quick and not completely trustworthy benchmarking results on Cuttlefish (with virtio-gpu 2D backend): snapshot time (seconds): before: 4.275 4.259 4.265 4.271 after: 4.212 4.152 4.162 4.153 restore time (seconds): before: 35.843 25.404 35.778 25.467 after: 25.121 25.401 25.361 25.331 Ignoring the 35 second restore times (probably from some interference), there is a tiny latency improvement. Bug: 369615058 Change-Id: I904bb7fb50951b727fc90541ffce10652b580705 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/6161914 Reviewed-by: Daniel Verkamp <[email protected]> Reviewed-by: Elie Kheirallah <[email protected]> Commit-Queue: Frederick Mayle <[email protected]> Auto-Submit: Frederick Mayle <[email protected]>
1 parent 35bd243 commit 72d733e

File tree

9 files changed

+73
-21
lines changed

9 files changed

+73
-21
lines changed

Cargo.lock

+32-2
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ exclude = [
117117
]
118118

119119
[workspace.dependencies]
120+
ciborium = "0.2.2"
120121
tokio = { version = "1.29.1", features = ["net", "rt-multi-thread", "time", "sync", "macros"] }
121122
snapshot = { path = "snapshot" }
122123

devices/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ virtio-media = { version = "0.0.7", optional = true }
103103
vm_control = { path = "../vm_control" }
104104
vm_memory = { path = "../vm_memory" }
105105
zerocopy = { version = "0.8.13", features = ["derive"] }
106+
ciborium = { workspace = true }
106107

107108
[target.'cfg(any(target_os = "android", target_os = "linux"))'.dependencies]
108109
android_audio = { path = "../android_audio" }

devices/src/proxy.rs

+4-8
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
77
use std::fs;
88
use std::fs::File;
9-
use std::io::BufReader;
109
use std::io::BufWriter;
1110
use std::io::Seek;
1211
use std::io::Write;
@@ -82,12 +81,9 @@ impl SnapshotFile {
8281
}
8382

8483
fn read(&mut self) -> anyhow::Result<AnySnapshot> {
85-
let data: AnySnapshot = {
86-
let mut reader = BufReader::new(&self.file);
87-
88-
serde_json::from_reader(&mut reader)
89-
.context("failed to read snapshot data from snapshot temp file")?
90-
};
84+
// NOTE: No BufReader because ciborium::from_reader has an internal buffer.
85+
let data: AnySnapshot = ciborium::from_reader(&mut self.file)
86+
.context("failed to read snapshot data from snapshot temp file")?;
9187

9288
self.file
9389
.rewind()
@@ -100,7 +96,7 @@ impl SnapshotFile {
10096
{
10197
let mut writer = BufWriter::new(&self.file);
10298

103-
serde_json::to_writer(&mut writer, &data)
99+
ciborium::into_writer(&data, &mut writer)
104100
.context("failed to write data to snasphot temp file")?;
105101

106102
writer

devices/src/virtio/vhost/user/device/handler.rs

+5-4
Original file line numberDiff line numberDiff line change
@@ -294,8 +294,8 @@ pub struct DeviceRequestHandler<T: VhostUserDevice> {
294294
}
295295

296296
enum DeviceStateThread {
297-
Save(WorkerThread<serde_json::Result<()>>),
298-
Load(WorkerThread<serde_json::Result<DeviceRequestHandlerSnapshot>>),
297+
Save(WorkerThread<Result<(), ciborium::ser::Error<std::io::Error>>>),
298+
Load(WorkerThread<Result<DeviceRequestHandlerSnapshot, ciborium::de::Error<std::io::Error>>>),
299299
}
300300

301301
#[derive(Serialize, Deserialize)]
@@ -747,7 +747,8 @@ impl<T: VhostUserDevice> vmm_vhost::Backend for DeviceRequestHandler<T> {
747747
// Spawn thread to write the serialized bytes.
748748
self.device_state_thread = Some(DeviceStateThread::Save(WorkerThread::start(
749749
"device_state_save",
750-
move |_kill_event| serde_json::to_writer(&mut fd, &snapshot),
750+
// TODO: should probably use BufWriter
751+
move |_kill_event| ciborium::into_writer(&snapshot, &mut fd),
751752
)));
752753
Ok(None)
753754
}
@@ -756,7 +757,7 @@ impl<T: VhostUserDevice> vmm_vhost::Backend for DeviceRequestHandler<T> {
756757
// `check_device_state`.
757758
self.device_state_thread = Some(DeviceStateThread::Load(WorkerThread::start(
758759
"device_state_load",
759-
move |_kill_event| serde_json::from_reader(&mut fd),
760+
move |_kill_event| ciborium::from_reader(&mut fd),
760761
)));
761762
Ok(None)
762763
}

docs/book/src/architecture/snapshotting.md

+22
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,28 @@ there is no way to fetch this state atomically, we have to freeze the guest (VCP
1212
backends. Similarly, on restore we must freeze in the same way to prevent partially restored state
1313
from being modified.
1414

15+
## Snapshot format
16+
17+
The snapshot format is not stable. Currently, the output is a directory, where most VM components
18+
are snapshotted to separate files using CBOR encoding.
19+
20+
When debugging snapshots, you may want to inspect the CBOR files. One tool available is
21+
[cbor-cli](https://docs.rs/crate/cbor-cli/latest). You can run `cargo install cbor-cli`, then use it
22+
to view a file as JSON, e.g.
23+
24+
```
25+
$ cbor export --format=json /tmp/crosvm-snapshot/irqchip | jq
26+
{
27+
"mp_state": [
28+
"Halted",
29+
"Halted",
30+
"Halted",
31+
"Halted"
32+
],
33+
"pit_state": {
34+
...
35+
```
36+
1537
## Snapshotting a running VM
1638

1739
In code, this is implemented by

snapshot/Cargo.toml

+1
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,4 @@ base = { path = "../base" }
1212
crypto = { path = "../vendor/generic/crypto", package = "crypto_generic" }
1313
serde = { version = "1", features = ["derive"] }
1414
serde_json = "1"
15+
ciborium = { workspace = true }

snapshot/src/any_snapshot.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@
1414
/// it will technically work, but the result might not match what you'd get if you directly
1515
/// serialized the original value. That should be OK as long as the serialization and
1616
/// deserialization paths make symmetric use of `AnySnapshot`.
17-
#[derive(Debug, Clone, PartialEq, Eq, serde::Serialize, serde::Deserialize)]
18-
pub struct AnySnapshot(serde_json::Value);
17+
#[derive(Debug, Clone, PartialEq, serde::Serialize, serde::Deserialize)]
18+
pub struct AnySnapshot(ciborium::Value);
1919

2020
impl AnySnapshot {
2121
pub fn to_any(x: impl serde::Serialize) -> anyhow::Result<Self> {
22-
Ok(AnySnapshot(serde_json::to_value(x)?))
22+
Ok(AnySnapshot(ciborium::Value::serialized(&x)?))
2323
}
2424

2525
pub fn from_any<T: serde::de::DeserializeOwned>(x: Self) -> anyhow::Result<T> {
26-
Ok(serde_json::from_value(x.0)?)
26+
Ok(x.0.deserialized()?)
2727
}
2828
}

snapshot/src/lib.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ impl SnapshotWriter {
117117
/// Creates a snapshot fragment from a serialized representation of `v`.
118118
pub fn write_fragment<T: serde::Serialize>(&self, name: &str, v: &T) -> Result<()> {
119119
let mut w = std::io::BufWriter::new(self.raw_fragment(name)?);
120-
serde_json::to_writer(&mut w, v)?;
120+
ciborium::into_writer(v, &mut w)?;
121121
w.flush()?;
122122
Ok(())
123123
}
@@ -199,8 +199,8 @@ impl SnapshotReader {
199199

200200
/// Reads a fragment.
201201
pub fn read_fragment<T: serde::de::DeserializeOwned>(&self, name: &str) -> Result<T> {
202-
serde_json::from_reader(std::io::BufReader::new(self.raw_fragment(name)?))
203-
.with_context(|| format!("failed to parse json from snapshot fragment named {}", name))
202+
// NOTE: No BufReader because ciborium::from_reader has an internal buffer.
203+
Ok(ciborium::from_reader(self.raw_fragment(name)?)?)
204204
}
205205

206206
/// Reads the names of all fragments in this namespace.

0 commit comments

Comments
 (0)