Verify the apfs volume doesn't already exist before trying to create it (#217)

* Verify the apfs volume doesn't already exist before trying to create it

* Use plist instead of raw parsing

* Remove an unwrap
This commit is contained in:
Ana Hobden 2023-02-01 10:40:42 -08:00 committed by GitHub
parent 6b3a84002e
commit fd149eef47
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 44 additions and 5 deletions

View file

@ -5,6 +5,7 @@ use tracing::{span, Span};
use crate::action::{ActionError, StatefulAction};
use crate::execute_command;
use serde::Deserialize;
use crate::action::{Action, ActionDescription};
@ -22,6 +23,22 @@ impl CreateApfsVolume {
name: String,
case_sensitive: bool,
) -> Result<StatefulAction<Self>, ActionError> {
let output =
execute_command(Command::new("/usr/sbin/diskutil").args(["apfs", "list", "-plist"]))
.await
.map_err(ActionError::Command)?;
let parsed: DiskUtilApfsListOutput = plist::from_bytes(&output.stdout)?;
for container in parsed.containers {
for volume in container.volumes {
if volume.name == name {
return Err(ActionError::Custom(Box::new(
CreateApfsVolumeError::ExistingVolume(name),
)));
}
}
}
Ok(Self {
disk: disk.as_ref().to_path_buf(),
name,
@ -120,7 +137,25 @@ impl Action for CreateApfsVolume {
}
#[derive(Debug, thiserror::Error)]
pub enum CreateVolumeError {
#[error("Failed to execute command")]
Command(#[source] std::io::Error),
pub enum CreateApfsVolumeError {
#[error("Existing volume called `{0}` found in `diskutil apfs list`, delete it with `diskutil apfs deleteVolume \"{0}\"`")]
ExistingVolume(String),
}
#[derive(Deserialize, Clone, Debug)]
#[serde(rename_all = "PascalCase")]
struct DiskUtilApfsListOutput {
containers: Vec<DiskUtilApfsContainer>,
}
#[derive(Deserialize, Clone, Debug)]
#[serde(rename_all = "PascalCase")]
struct DiskUtilApfsContainer {
volumes: Vec<DiskUtilApfsListVolume>,
}
#[derive(Deserialize, Clone, Debug)]
#[serde(rename_all = "PascalCase")]
struct DiskUtilApfsListVolume {
name: String,
}

View file

@ -62,7 +62,7 @@ impl Action for EnableOwnership {
.await
.map_err(ActionError::Command)?
.stdout;
let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf)).unwrap();
let the_plist: DiskUtilOutput = plist::from_reader(Cursor::new(buf))?;
the_plist.global_permissions_enabled
};

View file

@ -12,7 +12,7 @@ pub(crate) mod kickstart_launchctl_service;
pub(crate) mod unmount_apfs_volume;
pub use bootstrap_apfs_volume::{BootstrapApfsVolume, BootstrapVolumeError};
pub use create_apfs_volume::{CreateApfsVolume, CreateVolumeError};
pub use create_apfs_volume::{CreateApfsVolume, CreateApfsVolumeError};
pub use create_nix_volume::{CreateNixVolume, NIX_VOLUME_MOUNTD_DEST};
pub use create_synthetic_objects::{CreateSyntheticObjects, CreateSyntheticObjectsError};
pub use enable_ownership::{EnableOwnership, EnableOwnershipError};

View file

@ -327,6 +327,9 @@ pub enum ActionError {
#[from]
std::string::FromUtf8Error,
),
/// A MacOS (Darwin) plist related error
#[error(transparent)]
Plist(#[from] plist::Error),
}
impl HasExpectedErrors for ActionError {

View file

@ -114,6 +114,7 @@ pub struct CommonSettings {
feature = "cli",
clap(long, env = "NIX_INSTALLER_NIX_BUILD_USER_ID_BASE", global = true)
)]
// Service users on Mac should be between 200-400
#[cfg_attr(all(target_os = "macos", feature = "cli"), clap(default_value_t = 300))]
#[cfg_attr(
all(target_os = "linux", feature = "cli"),