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

Add a new create_options argument to new() and setup() #848

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Add a new create_options argument to new() and setup() #848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
03f9f8b
Add 'create_options' to device new() and setup() methods
bmr-cymru Apr 6, 2023
c9139b0
Allow clippy::too_many_arguments for CacheDev new() and setup()
bmr-cymru Apr 6, 2023
3f78077
Add an additional Option<DmOptions> parameter to resume()
bmr-cymru Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 27 additions & 14 deletions src/cachedev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,7 @@ 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.
#[allow(clippy::too_many_arguments)]
pub fn new(
dm: &DM,
name: &DmName,
Expand All @@ -566,14 +567,19 @@ impl CacheDev {
cache: LinearDev,
origin: LinearDev,
cache_block_size: Sectors,
create_options: Option<DmOptions>,
) -> DmResult<CacheDev> {
if device_exists(dm, name)? {
let err_msg = format!("cachedev {name} already exists");
return Err(DmError::Dm(ErrorEnum::Invalid, err_msg));
}

let options = match create_options {
Some(options) => options,
None => DmOptions::private(),
};
let table = CacheDev::gen_default_table(&meta, &cache, &origin, cache_block_size);
let dev_info = device_create(dm, name, uuid, &table, DmOptions::private())?;
let dev_info = device_create(dm, name, uuid, &table, options)?;

Ok(CacheDev {
dev_info: Box::new(dev_info),
Expand All @@ -585,6 +591,7 @@ impl CacheDev {
}

/// Set up a cache device from the given metadata and data devices.
#[allow(clippy::too_many_arguments)]
pub fn setup(
dm: &DM,
name: &DmName,
Expand All @@ -593,6 +600,7 @@ impl CacheDev {
cache: LinearDev,
origin: LinearDev,
cache_block_size: Sectors,
create_options: Option<DmOptions>,
) -> DmResult<CacheDev> {
let table = CacheDev::gen_default_table(&meta, &cache, &origin, cache_block_size);
let dev = if device_exists(dm, name)? {
Expand All @@ -607,7 +615,11 @@ impl CacheDev {
device_match(dm, &dev, uuid)?;
dev
} else {
let dev_info = device_create(dm, name, uuid, &table, DmOptions::private())?;
let options = match create_options {
Some(options) => options,
None => DmOptions::private(),
};
let dev_info = device_create(dm, name, uuid, &table, options)?;
CacheDev {
dev_info: Box::new(dev_info),
meta_dev: meta,
Expand All @@ -634,7 +646,7 @@ impl CacheDev {
self.suspend(dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))?;

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

let mut table = self.table.clone();
table.table.length = self.origin_dev.size();
Expand All @@ -658,7 +670,7 @@ impl CacheDev {
) -> DmResult<()> {
self.suspend(dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))?;
self.cache_dev.set_table(dm, table)?;
self.cache_dev.resume(dm)?;
self.cache_dev.resume(dm, None)?;

// Reload the table, even though it is unchanged. Otherwise, we
// suffer from whacky smq bug documented in the following PR:
Expand All @@ -681,7 +693,7 @@ impl CacheDev {
) -> DmResult<()> {
self.suspend(dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))?;
self.meta_dev.set_table(dm, table)?;
self.meta_dev.resume(dm)?;
self.meta_dev.resume(dm, None)?;

// Reload the table, even though it is unchanged. Otherwise, we
// suffer from whacky smq bug documented in the following PR:
Expand Down Expand Up @@ -774,7 +786,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
meta_length,
LinearDevTargetParams::Linear(meta_params),
)];
let meta = LinearDev::setup(dm, &meta_name, None, meta_table).unwrap();
let meta = LinearDev::setup(dm, &meta_name, None, meta_table, None).unwrap();

let cache_name = test_name("cache-cache").expect("valid format");
let cache_offset = meta_length;
Expand All @@ -785,7 +797,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
cache_length,
LinearDevTargetParams::Linear(cache_params),
)];
let cache = LinearDev::setup(dm, &cache_name, None, cache_table).unwrap();
let cache = LinearDev::setup(dm, &cache_name, None, cache_table, None).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 @@ -797,7 +809,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
dev2_size,
LinearDevTargetParams::Linear(origin_params),
)];
let origin = LinearDev::setup(dm, &origin_name, None, origin_table).unwrap();
let origin = LinearDev::setup(dm, &origin_name, None, origin_table, None).unwrap();

CacheDev::new(
dm,
Expand All @@ -807,6 +819,7 @@ pub fn minimal_cachedev(dm: &DM, paths: &[&Path]) -> CacheDev {
cache,
origin,
MIN_CACHE_BLOCK_SIZE,
None,
)
.unwrap()
}
Expand Down Expand Up @@ -920,7 +933,7 @@ mod tests {
LinearDevTargetParams::Linear(cache_params),
));
assert_matches!(cache.set_meta_table(&dm, table), Ok(_));
cache.resume(&dm).unwrap();
cache.resume(&dm, None).unwrap();

match cache.status(&dm, DmOptions::default()).unwrap() {
CacheDevStatus::Working(ref status) => {
Expand Down Expand Up @@ -974,7 +987,7 @@ mod tests {
LinearDevTargetParams::Linear(cache_params),
));
assert_matches!(cache.set_cache_table(&dm, cache_table.clone()), Ok(_));
cache.resume(&dm).unwrap();
cache.resume(&dm, None).unwrap();

match cache.status(&dm, DmOptions::default()).unwrap() {
CacheDevStatus::Working(ref status) => {
Expand All @@ -991,7 +1004,7 @@ mod tests {
cache_table.pop();

assert_matches!(cache.set_cache_table(&dm, cache_table), Ok(_));
cache.resume(&dm).unwrap();
cache.resume(&dm, None).unwrap();

match cache.status(&dm, DmOptions::default()).unwrap() {
CacheDevStatus::Working(ref status) => {
Expand Down Expand Up @@ -1034,7 +1047,7 @@ mod tests {
));

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

let origin_size = origin_size + dev3_size;
assert_eq!(cache.origin_dev.size(), origin_size);
Expand All @@ -1060,8 +1073,8 @@ mod tests {
cache
.suspend(&dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))
.unwrap();
cache.resume(&dm).unwrap();
cache.resume(&dm).unwrap();
cache.resume(&dm, None).unwrap();
cache.resume(&dm, None).unwrap();
cache.teardown(&dm).unwrap();
}

Expand Down
36 changes: 21 additions & 15 deletions src/lineardev.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ impl LinearDev {
name: &DmName,
uuid: Option<&DmUuid>,
table: Vec<TargetLine<LinearDevTargetParams>>,
create_options: Option<DmOptions>,
) -> DmResult<LinearDev> {
let table = LinearDevTargetTable::new(table);
let dev = if device_exists(dm, name)? {
Expand All @@ -539,7 +540,11 @@ impl LinearDev {
device_match(dm, &dev, uuid)?;
dev
} else {
let dev_info = device_create(dm, name, uuid, &table, DmOptions::private())?;
let options = match create_options {
Some(options) => options,
None => DmOptions::private(),
};
let dev_info = device_create(dm, name, uuid, &table, options)?;
LinearDev {
dev_info: Box::new(dev_info),
table,
Expand Down Expand Up @@ -596,6 +601,7 @@ mod tests {
&test_name("new").expect("valid format"),
None,
vec![],
None,
),
Err(_)
);
Expand All @@ -614,10 +620,10 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table, None).unwrap();

assert_matches!(ld.set_table(&dm, vec![]), Err(_));
ld.resume(&dm).unwrap();
ld.resume(&dm, None).unwrap();
ld.teardown(&dm).unwrap();
}

Expand All @@ -634,7 +640,7 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table, None).unwrap();

ld.set_name(&dm, &name).unwrap();
assert_eq!(ld.name(), &*name);
Expand All @@ -655,7 +661,7 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table, None).unwrap();

let new_name = test_name("new_name").expect("valid format");
ld.set_name(&dm, &new_name).unwrap();
Expand Down Expand Up @@ -689,7 +695,7 @@ mod tests {
];
let range: Sectors = table.iter().map(|s| s.length).sum();
let count = table.len();
let mut ld = LinearDev::setup(&dm, &name, None, table).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table, None).unwrap();

let table = LinearDev::read_kernel_table(&dm, &DevId::Name(ld.name()))
.unwrap()
Expand Down Expand Up @@ -723,7 +729,7 @@ mod tests {
)
})
.collect::<Vec<_>>();
let mut ld = LinearDev::setup(&dm, &name, None, table.clone()).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table.clone(), None).unwrap();

let loaded_table = LinearDev::read_kernel_table(&dm, &DevId::Name(ld.name())).unwrap();
assert!(
Expand All @@ -747,15 +753,15 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table.clone()).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table.clone(), None).unwrap();
let params2 = LinearTargetParams::new(dev, Sectors(1));
let table2 = vec![TargetLine::new(
Sectors(0),
Sectors(1),
LinearDevTargetParams::Linear(params2),
)];
assert_matches!(LinearDev::setup(&dm, &name, None, table2), Err(_));
assert_matches!(LinearDev::setup(&dm, &name, None, table), Ok(_));
assert_matches!(LinearDev::setup(&dm, &name, None, table2, None), Err(_));
assert_matches!(LinearDev::setup(&dm, &name, None, table, None), Ok(_));
ld.teardown(&dm).unwrap();
}

Expand All @@ -773,8 +779,8 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table.clone()).unwrap();
let ld2 = LinearDev::setup(&dm, &ersatz, None, table);
let mut ld = LinearDev::setup(&dm, &name, None, table.clone(), None).unwrap();
let ld2 = LinearDev::setup(&dm, &ersatz, None, table, None);
assert_matches!(ld2, Ok(_));

ld2.unwrap().teardown(&dm).unwrap();
Expand All @@ -794,14 +800,14 @@ mod tests {
Sectors(1),
LinearDevTargetParams::Linear(params),
)];
let mut ld = LinearDev::setup(&dm, &name, None, table).unwrap();
let mut ld = LinearDev::setup(&dm, &name, None, table, None).unwrap();

ld.suspend(&dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))
.unwrap();
ld.suspend(&dm, DmOptions::default().set_flags(DmFlags::DM_NOFLUSH))
.unwrap();
ld.resume(&dm).unwrap();
ld.resume(&dm).unwrap();
ld.resume(&dm, None).unwrap();
ld.resume(&dm, None).unwrap();

ld.teardown(&dm).unwrap();
}
Expand Down
8 changes: 6 additions & 2 deletions src/shared.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,12 @@ pub trait DmDevice<T: TargetTable> {
fn name(&self) -> &DmName;

/// Resume I/O on the device.
fn resume(&mut self, dm: &DM) -> DmResult<()> {
dm.device_suspend(&DevId::Name(self.name()), DmOptions::private())?;
fn resume(&mut self, dm: &DM, options: Option<DmOptions>) -> DmResult<()> {
let options = match options {
Some(options) => options,
None => DmOptions::private(),
};
dm.device_suspend(&DevId::Name(self.name()), options)?;
Ok(())
}

Expand Down
Loading