Skip to content

Commit

Permalink
♻️ ReadOnlyDevices don't track previous values
Browse files Browse the repository at this point in the history
The reason for the previous value was so, when restarting DrMem,
devices could be set back to their previous setting. Read-only devices
get their value from querying their hardware. You don't restore a
read-only device.
  • Loading branch information
rneswold committed Aug 23, 2024
1 parent 495c35b commit 2c4b865
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 44 deletions.
10 changes: 2 additions & 8 deletions drmem-api/src/driver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,7 @@ pub enum Request {
dev_name: device::Name,
dev_units: Option<String>,
max_history: Option<usize>,
rpy_chan:
oneshot::Sender<Result<(ReportReading, Option<device::Value>)>>,
rpy_chan: oneshot::Sender<Result<ReportReading>>,
},

/// Registers a writable device with core.
Expand Down Expand Up @@ -135,12 +134,7 @@ impl RequestChan {

if result.is_ok() {
if let Ok(v) = rx.await {
return v.map(|(rr, prev)| {
ReadOnlyDevice::new(
rr,
prev.and_then(|v| T::try_from(v).ok()),
)
});
return v.map(|rr| ReadOnlyDevice::new(rr));
}
}

Expand Down
15 changes: 4 additions & 11 deletions drmem-api/src/driver/ro_device.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::device;
use std::future::Future;
use std::marker::PhantomData;
use std::pin::Pin;

/// A function that drivers use to report updated values of a device.
Expand All @@ -14,32 +15,24 @@ pub type ReportReading = Box<
/// acceptable.
pub struct ReadOnlyDevice<T: Into<device::Value> + Clone> {
report_chan: ReportReading,
prev_val: Option<T>,
phantom: PhantomData<T>,
}

impl<T> ReadOnlyDevice<T>
where
T: Into<device::Value> + Clone,
{
/// Returns a new `ReadOnlyDevice` type.
pub fn new(report_chan: ReportReading, prev_val: Option<T>) -> Self {
pub fn new(report_chan: ReportReading) -> Self {
ReadOnlyDevice {
report_chan,
prev_val,
phantom: PhantomData,
}
}

/// Saves a new value, returned by the device, to the backend
/// storage.
pub async fn report_update(&mut self, value: T) {
self.prev_val = Some(value.clone());
(self.report_chan)(value.into()).await
}

/// Gets the last value of the device. If DrMem is built with
/// persistent storage, this value will be initialized with the
/// last value saved to storage.
pub fn get_last(&self) -> Option<&T> {
self.prev_val.as_ref()
}
}
2 changes: 1 addition & 1 deletion drmemd/src/backends/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ pub trait Store {
name: &device::Name,
units: Option<&String>,
max_history: Option<usize>,
) -> Result<(driver::ReportReading, Option<device::Value>)>;
) -> Result<driver::ReportReading>;

// Called when a read-write device is to be registered with the
// back-end.
Expand Down
7 changes: 2 additions & 5 deletions drmemd/src/backends/redis/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ impl Store for RedisStore {
name: &device::Name,
units: Option<&String>,
max_history: Option<usize>,
) -> Result<(ReportReading, Option<device::Value>)> {
) -> Result<ReportReading> {
let name = name.to_string();

debug!("registering '{}' as read-only", &name);
Expand All @@ -936,10 +936,7 @@ impl Store for RedisStore {

info!("'{}' has been successfully created", &name);
}
Ok((
self.mk_report_func(&name, max_history),
self.last_value(&name).await.map(|v| v.value),
))
Ok(self.mk_report_func(&name, max_history))
}

async fn register_read_write_device(
Expand Down
28 changes: 9 additions & 19 deletions drmemd/src/backends/simple/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ impl Store for SimpleStore {
name: &device::Name,
units: Option<&String>,
_max_history: Option<usize>,
) -> Result<(ReportReading, Option<device::Value>)> {
) -> Result<ReportReading> {
// Check to see if the device name already exists.

match self.0.entry((*name).clone()) {
Expand All @@ -163,7 +163,7 @@ impl Store for SimpleStore {
// Create and return the closure that the driver will
// use to report updates.

Ok((mk_report_func(di, name), None))
Ok(mk_report_func(di, name))
}

// The device already exists. If it was created from a
Expand All @@ -174,18 +174,8 @@ impl Store for SimpleStore {

if dev_info.owner.as_ref() == driver {
let func = mk_report_func(dev_info, name);
let guard = dev_info.reading.lock();

Ok((
func,
if let Ok(data) = guard {
data.1.as_ref().map(
|device::Reading { value, .. }| value.clone(),
)
} else {
None
},
))
Ok(func)
} else {
Err(Error::InUse)
}
Expand Down Expand Up @@ -483,7 +473,7 @@ mod tests {
let mut db = SimpleStore(HashMap::new());
let name = "test:device".parse::<device::Name>().unwrap();

if let Ok((f, None)) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down Expand Up @@ -567,7 +557,7 @@ mod tests {
let mut db = SimpleStore(HashMap::new());
let name = "test:device".parse::<device::Name>().unwrap();

if let Ok((f, None)) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down Expand Up @@ -654,7 +644,7 @@ mod tests {
let mut db = SimpleStore(HashMap::new());
let name = "test:device".parse::<device::Name>().unwrap();

if let Ok((f, None)) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down Expand Up @@ -698,7 +688,7 @@ mod tests {
let mut db = SimpleStore(HashMap::new());
let name = "test:device".parse::<device::Name>().unwrap();

if let Ok((f, None)) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down Expand Up @@ -887,7 +877,7 @@ mod tests {
// Register a device named "junk" and associate it with the
// driver named "test". We don't define units for this device.

if let Ok((f, None)) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down Expand Up @@ -922,7 +912,7 @@ mod tests {
// Assert that re-registering this device with the same
// driver name is successful.

if let Ok((f, Some(device::Value::Int(1)))) = db
if let Ok(f) = db
.register_read_only_device("test", &name, None, None)
.await
{
Expand Down

0 comments on commit 2c4b865

Please sign in to comment.