guardmgr: Use a better persistent data format
Previously we stored only one guard sample, in a state file called
"default_guards". That's not future-proof, since we want to have
multiple samples in the future. (guard-spec.txt specifies
separate samples for highly restrictive filters, and for bridge
usage.)
This patch changes our behavior so that we can store multiple samples in a new "guards" file.
I had thought about automatically migrating from the previous file format and location, but I don't think that's necessary given our current (lack of) stability guarantees.
Closes #176 (closed).
Merge request reports
Activity
assigned to @nickm
requested review from @Diziet
233 228 waiting: Vec<PendingRequest>, 234 229 235 230 /// Location in which to store persistent state. 231 storage: DynStorageHandle<GuardMgrPersistState>, 232 } 233 234 /// Persistent state for a guard manager, as serialized to disk. 235 #[derive(Debug, Default, Clone, Serialize, Deserialize)] 236 struct GuardMgrPersistState { - Comment on lines +234 to +236
I think this struct is misnamed and miscommented. It contains both persistent and ephemeral state, because, eventually,
Guarddoes.For example, observe that later you have
/// Update all non-persistent state for the guards in this object with the state in `other`. fn copy_status_from(&mut self, other: &GuardMgrPersistState) { self.default.copy_status_from(&other.default);which is a very strange thing to have written: "update all the non-persistent state in the persistent state".
If you agree and rename this, it might be worth looking at the next nearby the uses, as confusing wording appears elsewhere too eg at least in the
state: GuardMgrPersistStatefield ofGuardMgrInner. changed this line in version 3 of the diff
287 307 /// files. If we have the lock, we only want to save. 288 308 pub fn reload_persistent_state(&self) -> Result<(), GuardMgrError> { 289 309 let mut inner = self.inner.lock().expect("Poisoned lock"); 290 if let Some(new_guards) = inner.default_storage.load()? { 310 if let Some(new_state) = inner.storage.load()? { changed this line in version 3 of the diff
marked this merge request as draft from nickm/arti@d26e10bf
mentioned in issue #282 (closed)
added 1 commit
- 76d9ed24 - fixup! guardmgr: Use a better persistent data format
added 15 commits
-
76d9ed24...e735b3b2 - 13 commits from branch
tpo/core:main - 70a2e2e7 - guardmgr: Use a better persistent data format
- b89ce484 - Remove now-unused GuardSet::new().
-
76d9ed24...e735b3b2 - 13 commits from branch
enabled an automatic merge when the pipeline for b89ce484 succeeds
mentioned in commit 08d3ed97