Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Do not pass DM reference as parameter #288

Closed
wants to merge 2 commits into from
Closed

Do not pass DM reference as parameter #288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c65403f
Add a function to obtain a reference to the devicemapper context
mulkieran Mar 15, 2018
f2c0f8e
Do not pass dm parameter to devicemapper methods
mulkieran Mar 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
115 changes: 54 additions & 61 deletions src/cachedev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::str::FromStr;

use super::device::Device;
use super::deviceinfo::DeviceInfo;
use super::dm::DM;
use super::dm::get_dm_context;
use super::dm_flags::DmFlags;
use super::lineardev::{LinearDev, LinearDevTargetParams};
use super::result::{DmError, DmResult, ErrorEnum};
Expand Down Expand Up @@ -413,11 +413,12 @@ impl DmDevice<CacheDevTargetTable> for CacheDev {
table!(self)
}

fn teardown(self, dm: &DM) -> DmResult<()> {
dm.device_remove(&DevId::Name(self.name()), DmFlags::empty())?;
self.cache_dev.teardown(dm)?;
self.origin_dev.teardown(dm)?;
self.meta_dev.teardown(dm)?;
fn teardown(self) -> DmResult<()> {
get_dm_context()
.device_remove(&DevId::Name(self.name()), DmFlags::empty())?;
self.cache_dev.teardown()?;
self.origin_dev.teardown()?;
self.meta_dev.teardown()?;
Ok(())
}

Expand All @@ -431,21 +432,20 @@ impl DmDevice<CacheDevTargetTable> for CacheDev {
impl CacheDev {
/// Construct a new CacheDev with the given data and meta devs.
/// Returns an error if the device is already known to the kernel.
pub fn new(dm: &DM,
name: &DmName,
pub fn new(name: &DmName,
uuid: Option<&DmUuid>,
meta: LinearDev,
cache: LinearDev,
origin: LinearDev,
cache_block_size: Sectors)
-> DmResult<CacheDev> {
if device_exists(dm, name)? {
if device_exists(name)? {
let err_msg = format!("cachedev {} already exists", name);
return Err(DmError::Dm(ErrorEnum::Invalid, err_msg));
}

let table = CacheDev::gen_default_table(&meta, &cache, &origin, cache_block_size);
let dev_info = device_create(dm, name, uuid, &table)?;
let dev_info = device_create(name, uuid, &table)?;

Ok(CacheDev {
dev_info: Box::new(dev_info),
Expand All @@ -457,28 +457,27 @@ impl CacheDev {
}

/// Set up a cache device from the given metadata and data devices.
pub fn setup(dm: &DM,
name: &DmName,
pub fn setup(name: &DmName,
uuid: Option<&DmUuid>,
meta: LinearDev,
cache: LinearDev,
origin: LinearDev,
cache_block_size: Sectors)
-> DmResult<CacheDev> {
let table = CacheDev::gen_default_table(&meta, &cache, &origin, cache_block_size);
let dev = if device_exists(dm, name)? {
let dev_info = dm.device_info(&DevId::Name(name))?;
let dev = if device_exists(name)? {
let dev_info = get_dm_context().device_info(&DevId::Name(name))?;
let dev = CacheDev {
dev_info: Box::new(dev_info),
meta_dev: meta,
cache_dev: cache,
origin_dev: origin,
table,
};
device_match(dm, &dev, uuid)?;
device_match(&dev, uuid)?;
dev
} else {
let dev_info = device_create(dm, name, uuid, &table)?;
let dev_info = device_create(name, uuid, &table)?;
CacheDev {
dev_info: Box::new(dev_info),
meta_dev: meta,
Expand All @@ -498,17 +497,16 @@ impl CacheDev {
/// If not, this function will still succeed, but some kind of
/// data corruption will be the inevitable result.
pub fn set_origin_table(&mut self,
dm: &DM,
table: Vec<TargetLine<LinearDevTargetParams>>)
-> DmResult<()> {
self.suspend(dm)?;
self.suspend()?;

self.origin_dev.set_table(dm, table)?;
self.origin_dev.resume(dm)?;
self.origin_dev.set_table(table)?;
self.origin_dev.resume()?;

let mut table = self.table.clone();
table.table.length = self.origin_dev.size();
self.table_load(dm, &table)?;
self.table_load(&table)?;

self.table = table;

Expand All @@ -522,17 +520,16 @@ impl CacheDev {
/// If not, this function will still succeed, but some kind of
/// data corruption will be the inevitable result.
pub fn set_cache_table(&mut self,
dm: &DM,
table: Vec<TargetLine<LinearDevTargetParams>>)
-> DmResult<()> {
self.suspend(dm)?;
self.cache_dev.set_table(dm, table)?;
self.cache_dev.resume(dm)?;
self.suspend()?;
self.cache_dev.set_table(table)?;
self.cache_dev.resume()?;

// Reload the table, even though it is unchanged. Otherwise, we
// suffer from whacky smq bug documented in the following PR:
// https://github.com/stratis-storage/devicemapper-rs/pull/279.
self.table_load(dm, self.table())?;
self.table_load(self.table())?;

Ok(())
}
Expand Down Expand Up @@ -582,8 +579,9 @@ impl CacheDev {
/// Get the current status of the cache device.
// Note: This method is not entirely complete. In particular, *_args values
// may require more or better checking or processing.
pub fn status(&self, dm: &DM) -> DmResult<CacheDevStatus> {
let (_, status) = dm.table_status(&DevId::Name(self.name()), DmFlags::empty())?;
pub fn status(&self) -> DmResult<CacheDevStatus> {
let (_, status) = get_dm_context()
.table_status(&DevId::Name(self.name()), DmFlags::empty())?;

assert_eq!(status.len(),
1,
Expand Down Expand Up @@ -720,7 +718,7 @@ pub const MAX_CACHE_BLOCK_SIZE: Sectors = Sectors(2_097_152); // 1 GiB
// Make a minimal cachedev. Put the meta and cache on one device, and put
// the origin on a separate device. paths.len() must be at least 2 or the
// method will fail.
pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
pub fn minimal_cachedev(paths: &[&Path]) -> CacheDev {
assert!(paths.len() >= 2);
let dev1 = Device::from(devnode_to_devno(paths[0]).unwrap().unwrap());

Expand All @@ -732,7 +730,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
let meta_table = vec![TargetLine::new(Sectors(0),
meta_length,
LinearDevTargetParams::Linear(meta_params))];
let meta = LinearDev::setup(&dm, meta_name, None, meta_table).unwrap();
let meta = LinearDev::setup(meta_name, None, meta_table).unwrap();

let cache_name = DmName::new("cache-cache").expect("valid format");
let cache_offset = meta_length;
Expand All @@ -741,7 +739,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
let cache_table = vec![TargetLine::new(Sectors(0),
cache_length,
LinearDevTargetParams::Linear(cache_params))];
let cache = LinearDev::setup(&dm, cache_name, None, cache_table).unwrap();
let cache = LinearDev::setup(cache_name, None, cache_table).unwrap();

let dev2_size = blkdev_size(&OpenOptions::new().read(true).open(paths[1]).unwrap()).sectors();
let dev2 = Device::from(devnode_to_devno(paths[1]).unwrap().unwrap());
Expand All @@ -751,10 +749,9 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
let origin_table = vec![TargetLine::new(Sectors(0),
dev2_size,
LinearDevTargetParams::Linear(origin_params))];
let origin = LinearDev::setup(&dm, origin_name, None, origin_table).unwrap();
let origin = LinearDev::setup(origin_name, None, origin_table).unwrap();

CacheDev::new(&dm,
DmName::new("cache").expect("valid format"),
CacheDev::new(DmName::new("cache").expect("valid format"),
None,
meta,
cache,
Expand All @@ -776,10 +773,9 @@ mod tests {
// Verify that status method executes and gives reasonable values.
fn test_minimal_cache_dev(paths: &[&Path]) -> () {
assert!(paths.len() >= 2);
let dm = DM::new().unwrap();
let cache = minimal_cachedev(&dm, paths);
let cache = minimal_cachedev(paths);

match cache.status(&dm).unwrap() {
match cache.status().unwrap() {
CacheDevStatus::Working(ref status) => {
let usage = &status.usage;

Expand Down Expand Up @@ -823,7 +819,7 @@ mod tests {
_ => assert!(false),
}

let table = CacheDev::read_kernel_table(&dm, &DevId::Name(cache.name()))
let table = CacheDev::read_kernel_table(&DevId::Name(cache.name()))
.unwrap()
.table;

Expand All @@ -835,7 +831,7 @@ mod tests {
.collect::<HashSet<_>>());
assert_eq!(params.policy, "default");

cache.teardown(&dm).unwrap();
cache.teardown().unwrap();
}

#[test]
Expand All @@ -851,8 +847,7 @@ mod tests {
fn test_cache_size_change(paths: &[&Path]) {
assert!(paths.len() >= 3);

let dm = DM::new().unwrap();
let mut cache = minimal_cachedev(&dm, paths);
let mut cache = minimal_cachedev(paths);

let mut cache_table = cache.cache_dev.table().table.clone();
let dev3 = Device::from(devnode_to_devno(paths[2]).unwrap().unwrap());
Expand All @@ -861,7 +856,7 @@ mod tests {
let cache_params = LinearTargetParams::new(dev3, Sectors(0));
let current_length = cache.cache_dev.size();

match cache.status(&dm).unwrap() {
match cache.status().unwrap() {
CacheDevStatus::Working(ref status) => {
let usage = &status.usage;
assert_eq!(*usage.total_cache * usage.cache_block_size, current_length);
Expand All @@ -872,10 +867,10 @@ mod tests {
cache_table.push(TargetLine::new(current_length,
extra_length,
LinearDevTargetParams::Linear(cache_params)));
assert!(cache.set_cache_table(&dm, cache_table.clone()).is_ok());
cache.resume(&dm).unwrap();
assert!(cache.set_cache_table(cache_table.clone()).is_ok());
cache.resume().unwrap();

match cache.status(&dm).unwrap() {
match cache.status().unwrap() {
CacheDevStatus::Working(ref status) => {
let usage = &status.usage;
assert_eq!(*usage.total_cache * usage.cache_block_size,
Expand All @@ -886,18 +881,18 @@ mod tests {

cache_table.pop();

assert!(cache.set_cache_table(&dm, cache_table).is_ok());
cache.resume(&dm).unwrap();
assert!(cache.set_cache_table(cache_table).is_ok());
cache.resume().unwrap();

match cache.status(&dm).unwrap() {
match cache.status().unwrap() {
CacheDevStatus::Working(ref status) => {
let usage = &status.usage;
assert_eq!(*usage.total_cache * usage.cache_block_size, current_length);
}
CacheDevStatus::Fail => panic!("cache should not have failed"),
}

cache.teardown(&dm).unwrap();
cache.teardown().unwrap();
}

#[test]
Expand All @@ -911,8 +906,7 @@ mod tests {
fn test_origin_size_change(paths: &[&Path]) {
assert!(paths.len() >= 3);

let dm = DM::new().unwrap();
let mut cache = minimal_cachedev(&dm, paths);
let mut cache = minimal_cachedev(paths);

let mut origin_table = cache.origin_dev.table().table.clone();
let origin_size = cache.origin_dev.size();
Expand All @@ -926,14 +920,14 @@ mod tests {
dev3_size,
LinearDevTargetParams::Linear(origin_params)));

cache.set_origin_table(&dm, origin_table).unwrap();
cache.resume(&dm).unwrap();
cache.set_origin_table(origin_table).unwrap();
cache.resume().unwrap();

let origin_size = origin_size + dev3_size;
assert_eq!(cache.origin_dev.size(), origin_size);
assert_eq!(cache.size(), origin_size);

cache.teardown(&dm).unwrap();
cache.teardown().unwrap();
}

#[test]
Expand All @@ -945,13 +939,12 @@ mod tests {
fn test_suspend(paths: &[&Path]) {
assert!(paths.len() >= 2);

let dm = DM::new().unwrap();
let mut cache = minimal_cachedev(&dm, paths);
cache.suspend(&dm).unwrap();
cache.suspend(&dm).unwrap();
cache.resume(&dm).unwrap();
cache.resume(&dm).unwrap();
cache.teardown(&dm).unwrap();
let mut cache = minimal_cachedev(paths);
cache.suspend().unwrap();
cache.suspend().unwrap();
cache.resume().unwrap();
cache.resume().unwrap();
cache.teardown().unwrap();
}

#[test]
Expand Down
19 changes: 19 additions & 0 deletions src/dm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use std::{cmp, io, slice};
use std::fs::File;
use std::mem::{size_of, transmute};
use std::os::unix::io::AsRawFd;
use std::sync::{Once, ONCE_INIT};

use nix::libc::ioctl as nix_ioctl;
use nix::libc::c_ulong;
Expand Down Expand Up @@ -749,6 +750,24 @@ impl DM {
}
}

static INIT: Once = ONCE_INIT;
static mut DM_CONTEXT: Option<DM> = None;

/// Obtain a reference to a devicemapper context
/// The first time this is invoked, the devicemapper context is obtained,
/// and the method may panic. On all subsequent calls, the context has
/// already been obtained, and the method can not fail.
pub fn get_dm_context() -> &'static DM {
unsafe {
INIT.call_once(|| DM_CONTEXT = Some(DM::new().unwrap()));
match DM_CONTEXT {
Some(ref context) => context,
_ => panic!("DM_CONTEXT.is_some()"),
}
}
}


#[cfg(test)]
mod tests {

Expand Down
2 changes: 1 addition & 1 deletion src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ pub use cachedev::{CacheDev, CacheDevPerformance, CacheDevStatus, CacheDevUsage,
CacheDevWorkingStatus, MAX_CACHE_BLOCK_SIZE, MIN_CACHE_BLOCK_SIZE};
pub use consts::{IEC, SECTOR_SIZE};
pub use device::{Device, devnode_to_devno};
pub use dm::DM;
pub use dm::{DM, get_dm_context};
pub use dm_flags::DmFlags;
pub use lineardev::{FlakeyTargetParams, LinearDev, LinearDevTargetParams, LinearDevTargetTable,
LinearTargetParams};
Expand Down