-
Notifications
You must be signed in to change notification settings - Fork 62
Add two features for local storage support #9386
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.
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
Changes from all commits
26b7d31
7b7d7c1
1ffd876
f340e58
634ee2a
f7c53ab
78468e6
8724549
79c0bf3
551a528
c6110fc
bf71ddb
7e3d484
b985534
d63ff69
c85bf91
504581b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -220,6 +220,76 @@ pub struct DestroySnapshotError { | |
| err: crate::ExecutionError, | ||
| } | ||
|
|
||
| #[derive(thiserror::Error, Debug)] | ||
| pub enum EnsureDatasetVolumeErrorInner { | ||
| #[error(transparent)] | ||
| Execution(#[from] crate::ExecutionError), | ||
|
|
||
| #[error(transparent)] | ||
| GetValue(#[from] GetValueError), | ||
|
|
||
| #[error("value {value_name} parse error: {value} not a number!")] | ||
| ValueParseError { value_name: String, value: String }, | ||
|
|
||
| #[error("expected {value_name} to be {expected}, but saw {actual}")] | ||
| ValueMismatch { value_name: String, expected: u64, actual: u64 }, | ||
| } | ||
|
|
||
| /// Error returned by [`Zfs::ensure_dataset_volume`]. | ||
| #[derive(thiserror::Error, Debug)] | ||
| #[error("Failed to ensure volume '{name}': {err}")] | ||
| pub struct EnsureDatasetVolumeError { | ||
| name: String, | ||
| #[source] | ||
| err: EnsureDatasetVolumeErrorInner, | ||
| } | ||
|
|
||
| impl EnsureDatasetVolumeError { | ||
| pub fn execution(name: String, err: crate::ExecutionError) -> Self { | ||
| EnsureDatasetVolumeError { | ||
| name, | ||
| err: EnsureDatasetVolumeErrorInner::Execution(err), | ||
| } | ||
| } | ||
|
|
||
| pub fn get_value(name: String, err: GetValueError) -> Self { | ||
| EnsureDatasetVolumeError { | ||
| name, | ||
| err: EnsureDatasetVolumeErrorInner::GetValue(err), | ||
| } | ||
| } | ||
|
|
||
| pub fn value_parse( | ||
| name: String, | ||
| value_name: String, | ||
| value: String, | ||
| ) -> Self { | ||
| EnsureDatasetVolumeError { | ||
| name, | ||
| err: EnsureDatasetVolumeErrorInner::ValueParseError { | ||
| value_name, | ||
| value, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| pub fn value_mismatch( | ||
| name: String, | ||
| value_name: String, | ||
| expected: u64, | ||
| actual: u64, | ||
| ) -> Self { | ||
| EnsureDatasetVolumeError { | ||
| name, | ||
| err: EnsureDatasetVolumeErrorInner::ValueMismatch { | ||
| value_name, | ||
| expected, | ||
| actual, | ||
| }, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// Wraps commands for interacting with ZFS. | ||
| pub struct Zfs {} | ||
|
|
||
|
|
@@ -1339,6 +1409,88 @@ impl Zfs { | |
| } | ||
| Ok(result) | ||
| } | ||
|
|
||
| pub async fn ensure_dataset_volume( | ||
| name: String, | ||
| size: ByteCount, | ||
| block_size: u32, | ||
| ) -> Result<(), EnsureDatasetVolumeError> { | ||
| let mut command = Command::new(PFEXEC); | ||
| let cmd = command.args(&[ZFS, "create"]); | ||
|
|
||
| cmd.args(&[ | ||
| "-V", | ||
| &size.to_bytes().to_string(), | ||
| "-o", | ||
| &format!("volblocksize={}", block_size), | ||
| &name, | ||
| ]); | ||
|
|
||
| // The command to create a dataset is not idempotent and will fail with | ||
| // "dataset already exists" if the volume is created already. Eat this | ||
| // and return Ok instead. | ||
|
|
||
| match execute_async(cmd).await { | ||
| Ok(_) => Ok(()), | ||
|
|
||
| Err(crate::ExecutionError::CommandFailure(info)) | ||
| if info.stderr.contains("dataset already exists") => | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to check it has the expected size and block size?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 8724549 does this - importantly we can't change these after the fact so calling with different parameters is an error. |
||
| { | ||
| // Validate that the total size and volblocksize are what is | ||
| // being requested: these cannot be changed once the volume is | ||
| // created. | ||
|
|
||
| let [actual_size, actual_block_size] = | ||
| Self::get_values(&name, &["volsize", "volblocksize"], None) | ||
| .await | ||
| .map_err(|err| { | ||
| EnsureDatasetVolumeError::get_value( | ||
| name.clone(), | ||
| err, | ||
| ) | ||
| })?; | ||
|
|
||
| let actual_size: u64 = actual_size.parse().map_err(|_| { | ||
| EnsureDatasetVolumeError::value_parse( | ||
| name.clone(), | ||
| String::from("volsize"), | ||
| actual_size, | ||
| ) | ||
| })?; | ||
|
|
||
| let actual_block_size: u32 = | ||
| actual_block_size.parse().map_err(|_| { | ||
| EnsureDatasetVolumeError::value_parse( | ||
| name.clone(), | ||
| String::from("volblocksize"), | ||
| actual_block_size, | ||
| ) | ||
| })?; | ||
|
|
||
| if actual_size != size.to_bytes() { | ||
| return Err(EnsureDatasetVolumeError::value_mismatch( | ||
| name.clone(), | ||
| String::from("volsize"), | ||
| size.to_bytes(), | ||
| actual_size, | ||
| )); | ||
| } | ||
|
|
||
| if actual_block_size != block_size { | ||
| return Err(EnsureDatasetVolumeError::value_mismatch( | ||
| name.clone(), | ||
| String::from("volblocksize"), | ||
| u64::from(block_size), | ||
| u64::from(actual_block_size), | ||
| )); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
|
|
||
| Err(err) => Err(EnsureDatasetVolumeError::execution(name, err)), | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// A read-only snapshot of a ZFS filesystem. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question about newtypes - is any
u32valid for a block size, or should this be aBlockSize(u32)(or even aBlockSize::*enum if there are only a few choices that we allow)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the block size argument from the ensure request in 79c0bf3 - these should always only ever be 4096 byte blocks.