From 55b05cfd80b02f7e7744412b03aefda73a2f2fe4 Mon Sep 17 00:00:00 2001 From: bunnie Date: Tue, 29 Mar 2022 01:47:28 +0800 Subject: [PATCH] add debugging routines; fix zero-length key creation case - add a "dangerous debug" command that only exists when the feature "pddbtest" is enabled. The feature can't work on real hardware, but still, it enables things like dumping of the database and storing the keys for inspection, so it's isolated in a feature that prevents the code from being compiled in the first place when it's not selected. - fix a bug where keys that have zero length aren't being created correctly. this is a common use case for reserving some space for data that isn't yet available. --- services/pddb/src/api.rs | 17 ++++++++-- services/pddb/src/backend/dictionary.rs | 1 - services/pddb/src/lib.rs | 27 ++++++++++++--- services/pddb/src/main.rs | 45 +++++++++++++++++++++---- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/services/pddb/src/api.rs b/services/pddb/src/api.rs index 6e03d4dac..c5140e0e5 100644 --- a/services/pddb/src/api.rs +++ b/services/pddb/src/api.rs @@ -68,7 +68,7 @@ pub(crate) const FSCB_FILL_COEFFICIENT: f32 = 0.5; pub(crate) const FSCB_FILL_UNCERTAINTY: f32 = 0.1; #[allow(dead_code)] -pub(crate) const PDDB_DEFAULT_SYSTEM_BASIS: &'static str = ".System"; +pub const PDDB_DEFAULT_SYSTEM_BASIS: &'static str = ".System"; // this isn't an "official" basis, but it is used for the AAD for encrypting the FastSpace structure #[allow(dead_code)] pub(crate) const PDDB_FAST_SPACE_SYSTEM_BASIS: &'static str = ".FastSpace"; @@ -120,7 +120,7 @@ pub(crate) enum Opcode { Quit, /// Write debug dump (only available in hosted mode) #[cfg(not(any(target_os = "none", target_os = "xous")))] - DbgDump, + DangerousDebug, } #[derive(num_derive::FromPrimitive, num_derive::ToPrimitive, Debug)] pub(crate) enum PollOp { @@ -320,6 +320,19 @@ impl PddbKeyAttrIpc { } } +/// Debugging commands, available only in hosted mode +#[cfg(not(any(target_os = "none", target_os = "xous")))] +#[derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize)] +pub struct PddbDangerousDebug { + pub request: DebugRequest, + pub dump_name: xous_ipc::String::<128>, +} +#[cfg(not(any(target_os = "none", target_os = "xous")))] +#[derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize)] +pub enum DebugRequest { + Dump, + Remount, +} #[cfg(test)] mod tests { diff --git a/services/pddb/src/backend/dictionary.rs b/services/pddb/src/backend/dictionary.rs index b4875f39a..7f22f9297 100644 --- a/services/pddb/src/backend/dictionary.rs +++ b/services/pddb/src/backend/dictionary.rs @@ -419,7 +419,6 @@ impl DictCacheEntry { if let KeyCacheData::Small(cache_data) = kcache.data.as_mut().expect("small pool should all have their data 'hot' if the index entry is also in cache") { cache_data.clean = false; // grow the data cache to accommodate the necessary length; this should be efficient because we reserved space when the vector was allocated - assert!(cache_data.data.len() != 0, "(temporary assert - expected data in the cache for a particular test. Remove this assert if you're hitting this on an actual zero-lenth data set."); while cache_data.data.len() < data.len() + offset { cache_data.data.push(0); } diff --git a/services/pddb/src/lib.rs b/services/pddb/src/lib.rs index a7495b089..b8ff671a0 100644 --- a/services/pddb/src/lib.rs +++ b/services/pddb/src/lib.rs @@ -583,11 +583,30 @@ impl Pddb { } Ok(dict_list) } + /// Triggers a dump of the PDDB to host disk #[cfg(not(any(target_os = "none", target_os = "xous")))] pub fn dbg_dump(&self, name: &str) -> Result<()> { - let ipc_str = xous_ipc::String::<128>::from_str(name); - ipc_str.send(self.conn).expect("couldn't initiate debug dump"); - Ok(()) + let ipc = PddbDangerousDebug { + request: DebugRequest::Dump, + dump_name: xous_ipc::String::from_str(name), + }; + let buf = Buffer::into_buf(ipc) + .or(Err(Error::new(ErrorKind::Other, "Xous internal error")))?; + buf.lend(self.conn, Opcode::DangerousDebug.to_u32().unwrap()) + .or(Err(Error::new(ErrorKind::Other, "Xous internal error"))).map(|_| ()) + } + /// Triggers an umount/remount, forcing a read of the PDDB from disk back into the cache structures. + /// It's a cheesy way to test a power cycle, without having to power cycle. + #[cfg(not(any(target_os = "none", target_os = "xous")))] + pub fn dbg_remount(&self) -> Result<()> { + let ipc = PddbDangerousDebug { + request: DebugRequest::Remount, + dump_name: xous_ipc::String::new(), + }; + let buf = Buffer::into_buf(ipc) + .or(Err(Error::new(ErrorKind::Other, "Xous internal error")))?; + buf.lend(self.conn, Opcode::DangerousDebug.to_u32().unwrap()) + .or(Err(Error::new(ErrorKind::Other, "Xous internal error"))).map(|_| ()) } } @@ -595,7 +614,7 @@ impl Drop for Pddb { fn drop(&mut self) { let cid = xous::connect(self.cb_sid).unwrap(); send_message(cid, Message::new_blocking_scalar(CbOp::Quit.to_usize().unwrap(), 0, 0, 0, 0)).unwrap(); - unsafe{xous::disconnect(cid).unwrap();} + unsafe{xous::disconnect(cid).ok();} if let Some(handle) = self.cb_handle.take() { handle.join().expect("couldn't terminate callback helper thread"); } diff --git a/services/pddb/src/main.rs b/services/pddb/src/main.rs index 459343039..d024ad322 100644 --- a/services/pddb/src/main.rs +++ b/services/pddb/src/main.rs @@ -730,13 +730,29 @@ fn xmain() -> ! { buffer.replace(req).unwrap(); continue } } + let alloc_hint = if let Some(hint) = req.alloc_hint {Some(hint as usize)} else {None}; if basis_cache.key_attributes(&mut pddb_os, dict, key, bname).is_err() { if !req.create_key { req.result = PddbRequestCode::NotFound; buffer.replace(req).unwrap(); continue + } else { + // create an empty key placeholder + let empty: [u8; 0] = []; + match basis_cache.key_update(&mut pddb_os, + dict, key, &empty, None, alloc_hint, bname, true + ) { + Ok(_) => {}, + Err(e) => { + log::error!("Couldn't allocate key: {:?}", e); + match e.kind() { + std::io::ErrorKind::NotFound => req.result = PddbRequestCode::NotMounted, + std::io::ErrorKind::OutOfMemory => req.result = PddbRequestCode::NoFreeSpace, + _ => req.result = PddbRequestCode::InternalError, + } + buffer.replace(req).unwrap(); continue + } + } } - // by default keys are created when they are written on the first try. - // ...do need to remember to create an "empty" key if we try to read a key that doesn't already exist, tho... } // at this point, we have established a basis/dict/key tuple. let token: ApiToken = [pddb_os.trng_u32(), pddb_os.trng_u32(), pddb_os.trng_u32()]; @@ -746,7 +762,7 @@ fn xmain() -> ! { key: String::from(key), basis: if let Some(name) = bname {Some(String::from(name))} else {None}, conn: cid, - alloc_hint: if let Some(hint) = req.alloc_hint {Some(hint as usize)} else {None}, + alloc_hint, }; token_dict.insert(token, token_record); req.token = Some(token); @@ -1089,10 +1105,25 @@ fn xmain() -> ! { modals.show_notification(¬e).expect("couldn't show basis list"); }, #[cfg(not(any(target_os = "none", target_os = "xous")))] - Some(Opcode::DbgDump) => { - let dumpname = xous_ipc::String::<128>::from_message(msg.body.memory_message().unwrap()).unwrap(); - log::info!("dumping {}", dumpname); - pddb_os.dbg_dump(Some(dumpname.as_str().unwrap().to_string()), None); + Some(Opcode::DangerousDebug) => { + let buffer = unsafe { Buffer::from_memory_message(msg.body.memory_message().unwrap()) }; + let dbg = buffer.to_original::().unwrap(); + match dbg.request { + DebugRequest::Dump => { + log::info!("dumping pddb to {}", dbg.dump_name.as_str().unwrap()); + pddb_os.dbg_dump(Some(dbg.dump_name.as_str().unwrap().to_string()), None); + }, + DebugRequest::Remount => { + log::info!("attempting remount"); + basis_cache = BasisCache::new(); // this effectively erases the PDDB from memory + if let Some(sys_basis) = pddb_os.pddb_mount() { + log::info!("remount successful"); + basis_cache.basis_add(sys_basis); + } else { + log::info!("remount failed"); + } + } + } } Some(Opcode::Quit) => { log::warn!("quitting the PDDB server");