[pbs-devel] [PATCH proxmox-backup v2 0/6] fix #3690: wipe disk

[pbs-devel] [PATCH proxmox-backup v2 0/6] fix #3690: wipe disk

[Markus Frank]

Wipe a disk/partition like we do in PVE. (wipefs, dd, change_parttype to 0x83
with sgdisk)

Markus Frank (6):
  fix #3690: pbs_api_types: add regex, format & schema for partition
    names to pbs-api-types
  fix #3690: tools: add wipe_blockdev & change_parttype rust equivalent
  fix #3690: api: add function wipe_disk
  fix #3690: cli: add function wipe_disk
  fix #3690: ui: enable wipe disk in StorageAndDisks
  tools: prohibit disk wipe of EFI partition

 pbs-api-types/src/lib.rs               |  10 ++
 src/api2/node/disks/mod.rs             |  53 +++++++++-
 src/bin/proxmox_backup_manager/disk.rs |  44 ++++++--
 src/tools/disks/mod.rs                 | 133 ++++++++++++++++++++++++-
 www/Utils.js                           |   1 +
 www/panel/StorageAndDisks.js           |   1 +
 6 files changed, 232 insertions(+), 10 deletions(-)

-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 1/6] fix #3690: pbs_api_types: add regex, format & schema for partition names to pbs-api-types

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 pbs-api-types/src/lib.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/pbs-api-types/src/lib.rs b/pbs-api-types/src/lib.rs
index 4306eca3..a771a6b2 100644
--- a/pbs-api-types/src/lib.rs
+++ b/pbs-api-types/src/lib.rs
@@ -191,6 +191,7 @@ const_regex! {
     );
 
     pub BLOCKDEVICE_NAME_REGEX = r"^(?:(?:h|s|x?v)d[a-z]+)|(?:nvme\d+n\d+)$";
+    pub BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX = r"^(?:(?:h|s|x?v)d[a-z]+\d*)|(?:nvme\d+n\d+(p\d+)?)$";
     pub SUBSCRIPTION_KEY_REGEX = concat!(r"^pbs(?:[cbsp])-[0-9a-f]{10}$");
 }
 
@@ -205,6 +206,8 @@ pub const PASSWORD_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&PASSWORD_
 pub const UUID_FORMAT: ApiStringFormat = ApiStringFormat::Pattern(&UUID_REGEX);
 pub const BLOCKDEVICE_NAME_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&BLOCKDEVICE_NAME_REGEX);
+pub const BLOCKDEVICE_DISK_AND_PARTITION_NAME_FORMAT: ApiStringFormat =
+    ApiStringFormat::Pattern(&BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX);
 pub const SUBSCRIPTION_KEY_FORMAT: ApiStringFormat =
     ApiStringFormat::Pattern(&SUBSCRIPTION_KEY_REGEX);
 pub const SYSTEMD_DATETIME_FORMAT: ApiStringFormat =
@@ -285,6 +288,13 @@ pub const BLOCKDEVICE_NAME_SCHEMA: Schema =
         .max_length(64)
         .schema();
 
+pub const BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA: Schema =
+    StringSchema::new("(Partition) block device name (/sys/class/block/<name>).")
+        .format(&BLOCKDEVICE_DISK_AND_PARTITION_NAME_FORMAT)
+        .min_length(3)
+        .max_length(64)
+        .schema();
+
 pub const DISK_ARRAY_SCHEMA: Schema =
     ArraySchema::new("Disk name list.", &BLOCKDEVICE_NAME_SCHEMA).schema();
 
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 2/6] fix #3690: tools: add wipe_blockdev & change_parttype rust equivalent

[Markus Frank]

The wipe_blockdev & change_parttype functions are similar to
PVE::Diskmanage's wipe_blockdev & change_parttype functions.

The partition_by_name & complete_partition_name functions are
modified disk_by_name & complete_disk_name functions for partitions.

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 src/tools/disks/mod.rs | 125 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 1 deletion(-)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 72e374ca..51919f9e 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -16,10 +16,12 @@ use ::serde::{Deserialize, Serialize};
 
 use proxmox_lang::error::io_err_other;
 use proxmox_lang::{io_bail, io_format_err};
+use proxmox_rest_server::WorkerTask;
 use proxmox_schema::api;
 use proxmox_sys::linux::procfs::{mountinfo::Device, MountInfo};
+use proxmox_sys::task_log;
 
-use pbs_api_types::BLOCKDEVICE_NAME_REGEX;
+use pbs_api_types::{BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX, BLOCKDEVICE_NAME_REGEX};
 
 mod zfs;
 pub use zfs::*;
@@ -111,6 +113,12 @@ impl DiskManage {
         self.disk_by_sys_path(syspath)
     }
 
+    /// Get a `Disk` for a name in `/sys/class/block/<name>`.
+    pub fn partition_by_name(self: Arc<Self>, name: &str) -> io::Result<Disk> {
+        let syspath = format!("/sys/class/block/{}", name);
+        self.disk_by_sys_path(syspath)
+    }
+
     /// Gather information about mounted disks:
     fn mounted_devices(&self) -> Result<&HashSet<dev_t>, Error> {
         self.mounted_devices
@@ -1056,6 +1064,105 @@ pub fn inititialize_gpt_disk(disk: &Disk, uuid: Option<&str>) -> Result<(), Erro
     Ok(())
 }
 
+/// Wipes all labels and the first 200 MiB of a disk/partition (or the whole if it is smaller).
+/// If called with a partition, also sets the partition type to 0x83 'Linux filesystem'.
+pub fn wipe_blockdev(disk: &Disk, worker: Arc<WorkerTask>) -> Result<(), Error> {
+    let disk_path = match disk.device_path() {
+        Some(path) => path,
+        None => bail!("disk {:?} has no node in /dev", disk.syspath()),
+    };
+    let disk_path_str = match disk_path.to_str() {
+        Some(path) => path,
+        None => bail!("disk {:?} could not transform into a str", disk.syspath()),
+    };
+
+    let mut is_partition = false;
+    for disk_info in get_lsblk_info()?.iter() {
+        if disk_info.path == disk_path_str && disk_info.partition_type.is_some() {
+            is_partition = true;
+        }
+    }
+
+    let mut to_wipe: Vec<PathBuf> = Vec::new();
+
+    let partitions_map = disk.partitions()?;
+    for part_disk in partitions_map.values() {
+        let part_path = match part_disk.device_path() {
+            Some(path) => path,
+            None => bail!("disk {:?} has no node in /dev", part_disk.syspath()),
+        };
+        to_wipe.push(part_path.to_path_buf());
+    }
+
+    to_wipe.push(disk_path.to_path_buf());
+
+    task_log!(worker, "Wiping block device {}", disk_path.display());
+
+    let mut wipefs_command = std::process::Command::new("wipefs");
+    wipefs_command.arg("--all").args(&to_wipe);
+
+    let wipefs_output = proxmox_sys::command::run_command(wipefs_command, None)?;
+    task_log!(worker, "wipefs output: {}", wipefs_output);
+
+    let size = disk.size().map(|size| size / 1024 / 1024)?;
+    let count = size.min(200);
+
+    let mut dd_command = std::process::Command::new("dd");
+    let mut of_path = OsString::from("of=");
+    of_path.push(disk_path);
+    let mut count_str = OsString::from("count=");
+    count_str.push(count.to_string());
+    let args = [
+        "if=/dev/zero".into(),
+        of_path,
+        "bs=1M".into(),
+        "conv=fdatasync".into(),
+        count_str.into(),
+    ];
+    dd_command.args(args);
+
+    let dd_output = proxmox_sys::command::run_command(dd_command, None)?;
+    task_log!(worker, "dd output: {}", dd_output);
+
+    if is_partition {
+        // set the partition type to 0x83 'Linux filesystem'
+        change_parttype(&disk, "8300", worker)?;
+    }
+
+    Ok(())
+}
+
+pub fn change_parttype(
+    part_disk: &Disk,
+    part_type: &str,
+    worker: Arc<WorkerTask>,
+) -> Result<(), Error> {
+    let part_path = match part_disk.device_path() {
+        Some(path) => path,
+        None => bail!("disk {:?} has no node in /dev", part_disk.syspath()),
+    };
+    if let Ok(stat) = nix::sys::stat::stat(part_path) {
+        let mut sgdisk_command = std::process::Command::new("sgdisk");
+        let major = unsafe { libc::major(stat.st_rdev) };
+        let minor = unsafe { libc::minor(stat.st_rdev) };
+        let partnum_path = &format!("/sys/dev/block/{}:{}/partition", major, minor);
+        let partnum: u32 = std::fs::read_to_string(partnum_path)?.trim_end().parse()?;
+        sgdisk_command.arg(&format!("-t{}:{}", partnum, part_type));
+        let part_disk_parent = match part_disk.parent() {
+            Some(disk) => disk,
+            None => bail!("disk {:?} has no node in /dev", part_disk.syspath()),
+        };
+        let part_disk_parent_path = match part_disk_parent.device_path() {
+            Some(path) => path,
+            None => bail!("disk {:?} has no node in /dev", part_disk.syspath()),
+        };
+        sgdisk_command.arg(part_disk_parent_path);
+        let sgdisk_output = proxmox_sys::command::run_command(sgdisk_command, None)?;
+        task_log!(worker, "sgdisk output: {}", sgdisk_output);
+    }
+    Ok(())
+}
+
 /// Create a single linux partition using the whole available space
 pub fn create_single_linux_partition(disk: &Disk) -> Result<Disk, Error> {
     let disk_path = match disk.device_path() {
@@ -1136,6 +1243,22 @@ pub fn complete_disk_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<S
         .collect()
 }
 
+/// Block device partition name completion helper
+pub fn complete_partition_name(_arg: &str, _param: &HashMap<String, String>) -> Vec<String> {
+    let dir = match proxmox_sys::fs::scan_subdir(
+        libc::AT_FDCWD,
+        "/sys/class/block",
+        &BLOCKDEVICE_DISK_AND_PARTITION_NAME_REGEX,
+    ) {
+        Ok(dir) => dir,
+        Err(_) => return vec![],
+    };
+
+    dir.flatten()
+        .map(|item| item.file_name().to_str().unwrap().to_string())
+        .collect()
+}
+
 /// Read the FS UUID (parse blkid output)
 ///
 /// Note: Calling blkid is more reliable than using the udev ID_FS_UUID property.
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 3/6] fix #3690: api: add function wipe_disk

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 src/api2/node/disks/mod.rs | 53 +++++++++++++++++++++++++++++++++++---
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/src/api2/node/disks/mod.rs b/src/api2/node/disks/mod.rs
index 5ee959cd..711dae7b 100644
--- a/src/api2/node/disks/mod.rs
+++ b/src/api2/node/disks/mod.rs
@@ -9,12 +9,13 @@ use proxmox_sortable_macro::sortable;
 use proxmox_sys::task_log;
 
 use pbs_api_types::{
-    BLOCKDEVICE_NAME_SCHEMA, NODE_SCHEMA, PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, UPID_SCHEMA,
+    BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA, BLOCKDEVICE_NAME_SCHEMA, NODE_SCHEMA,
+    PRIV_SYS_AUDIT, PRIV_SYS_MODIFY, UPID_SCHEMA,
 };
 
 use crate::tools::disks::{
-    get_smart_data, inititialize_gpt_disk, DiskManage, DiskUsageInfo, DiskUsageQuery,
-    DiskUsageType, SmartData,
+    get_smart_data, inititialize_gpt_disk, wipe_blockdev, DiskManage, DiskUsageInfo,
+    DiskUsageQuery, DiskUsageType, SmartData,
 };
 use proxmox_rest_server::WorkerTask;
 
@@ -178,6 +179,51 @@ pub fn initialize_disk(
     Ok(json!(upid_str))
 }
 
+#[api(
+    protected: true,
+    input: {
+        properties: {
+            node: {
+                schema: NODE_SCHEMA,
+            },
+            disk: {
+                schema: BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA,
+            },
+        },
+    },
+    returns: {
+        schema: UPID_SCHEMA,
+    },
+    access: {
+        permission: &Permission::Privilege(&["system", "disks"], PRIV_SYS_MODIFY, false),
+    },
+)]
+/// wipe disk
+pub fn wipe_disk(disk: String, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    let to_stdout = rpcenv.env_type() == RpcEnvironmentType::CLI;
+
+    let auth_id = rpcenv.get_auth_id().unwrap();
+
+    let upid_str = WorkerTask::new_thread(
+        "wipedisk",
+        Some(disk.clone()),
+        auth_id,
+        to_stdout,
+        move |worker| {
+            task_log!(worker, "wipe disk {}", disk);
+
+            let disk_manager = DiskManage::new();
+            let disk_info = disk_manager.partition_by_name(&disk)?;
+
+            wipe_blockdev(&disk_info, worker)?;
+
+            Ok(())
+        },
+    )?;
+
+    Ok(json!(upid_str))
+}
+
 #[sortable]
 const SUBDIRS: SubdirMap = &sorted!([
     //    ("lvm", &lvm::ROUTER),
@@ -186,6 +232,7 @@ const SUBDIRS: SubdirMap = &sorted!([
     ("initgpt", &Router::new().post(&API_METHOD_INITIALIZE_DISK)),
     ("list", &Router::new().get(&API_METHOD_LIST_DISKS)),
     ("smart", &Router::new().get(&API_METHOD_SMART_STATUS)),
+    ("wipedisk", &Router::new().put(&API_METHOD_WIPE_DISK)),
 ]);
 
 pub const ROUTER: Router = Router::new()
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 4/6] fix #3690: cli: add function wipe_disk

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 src/bin/proxmox_backup_manager/disk.rs | 44 ++++++++++++++++++++++----
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs
index 73cb95e6..5514da4b 100644
--- a/src/bin/proxmox_backup_manager/disk.rs
+++ b/src/bin/proxmox_backup_manager/disk.rs
@@ -5,10 +5,12 @@ use proxmox_router::{cli::*, ApiHandler, RpcEnvironment};
 use proxmox_schema::api;
 
 use pbs_api_types::{
-    ZfsCompressionType, ZfsRaidLevel, BLOCKDEVICE_NAME_SCHEMA, DATASTORE_SCHEMA, DISK_LIST_SCHEMA,
-    ZFS_ASHIFT_SCHEMA,
+    ZfsCompressionType, ZfsRaidLevel, BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA,
+    BLOCKDEVICE_NAME_SCHEMA, DATASTORE_SCHEMA, DISK_LIST_SCHEMA, ZFS_ASHIFT_SCHEMA,
+};
+use proxmox_backup::tools::disks::{
+    complete_disk_name, complete_partition_name, FileSystemType, SmartAttribute,
 };
-use proxmox_backup::tools::disks::{complete_disk_name, FileSystemType, SmartAttribute};
 
 use proxmox_backup::api2;
 
@@ -137,6 +139,30 @@ async fn initialize_disk(
     Ok(Value::Null)
 }
 
+#[api(
+   input: {
+        properties: {
+            disk: {
+                schema: BLOCKDEVICE_DISK_AND_PARTITION_NAME_SCHEMA,
+            },
+        },
+   },
+)]
+/// wipe disk
+async fn wipe_disk(mut param: Value, rpcenv: &mut dyn RpcEnvironment) -> Result<Value, Error> {
+    param["node"] = "localhost".into();
+
+    let info = &api2::node::disks::API_METHOD_WIPE_DISK;
+    let result = match info.handler {
+        ApiHandler::Sync(handler) => (handler)(param, info, rpcenv)?,
+        _ => unreachable!(),
+    };
+
+    crate::wait_for_local_worker(result.as_str().unwrap()).await?;
+
+    Ok(Value::Null)
+}
+
 #[api(
    input: {
         properties: {
@@ -350,10 +376,10 @@ pub fn filesystem_commands() -> CommandLineInterface {
             CliCommand::new(&API_METHOD_CREATE_DATASTORE_DISK)
                 .arg_param(&["name"])
                 .completion_cb("disk", complete_disk_name),
-        ).insert(
+        )
+        .insert(
             "delete",
-            CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK)
-                .arg_param(&["name"]),
+            CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK).arg_param(&["name"]),
         );
 
     cmd_def.into()
@@ -375,6 +401,12 @@ pub fn disk_commands() -> CommandLineInterface {
             CliCommand::new(&API_METHOD_INITIALIZE_DISK)
                 .arg_param(&["disk"])
                 .completion_cb("disk", complete_disk_name),
+        )
+        .insert(
+            "wipe",
+            CliCommand::new(&API_METHOD_WIPE_DISK)
+                .arg_param(&["disk"])
+                .completion_cb("disk", complete_partition_name),
         );
 
     cmd_def.into()
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 5/6] fix #3690: ui: enable wipe disk in StorageAndDisks

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
 www/Utils.js                 | 1 +
 www/panel/StorageAndDisks.js | 1 +
 2 files changed, 2 insertions(+)

diff --git a/www/Utils.js b/www/Utils.js
index 2eca600e..51fd7746 100644
--- a/www/Utils.js
+++ b/www/Utils.js
@@ -425,6 +425,7 @@ Ext.define('PBS.Utils', {
 	    verify: ['Datastore', gettext('Verification')],
 	    verify_group: ['Group', gettext('Verification')],
 	    verify_snapshot: ['Snapshot', gettext('Verification')],
+	    wipedisk: ['Device', gettext('Wipe Disk')],
 	    zfscreate: [gettext('ZFS Storage'), gettext('Create')],
 	});
 
diff --git a/www/panel/StorageAndDisks.js b/www/panel/StorageAndDisks.js
index 7bd7042c..e9d7ed19 100644
--- a/www/panel/StorageAndDisks.js
+++ b/www/panel/StorageAndDisks.js
@@ -17,6 +17,7 @@ Ext.define('PBS.StorageAndDiskPanel', {
 	    xtype: 'pmxDiskList',
 	    title: gettext('Disks'),
 	    includePartitions: true,
+	    supportsWipeDisk: true,
 	    itemId: 'disks',
 	    iconCls: 'fa fa-hdd-o',
 	},
-- 
2.39.2

[pbs-devel] [PATCH proxmox-backup v2 6/6] tools: prohibit disk wipe of EFI partition

[Markus Frank]

Signed-off-by: Markus Frank <m.frank@proxmox.com>
---
This patch is based on a suggestion by Dominik.
I am not so sure if we should prohibit wiping EFI partitions.
Any opinions on this?

 src/tools/disks/mod.rs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
index 51919f9e..34ace7d6 100644
--- a/src/tools/disks/mod.rs
+++ b/src/tools/disks/mod.rs
@@ -39,6 +39,8 @@ lazy_static::lazy_static! {
         regex::Regex::new(r"host[^/]*/session[^/]*").unwrap();
 }
 
+const EFI_PARTITION_TYPE: &str = "c12a7328-f81f-11d2-ba4b-00a0c93ec93b";
+
 /// Disk management context.
 ///
 /// This provides access to disk information with some caching for faster querying of multiple
@@ -1080,6 +1082,12 @@ pub fn wipe_blockdev(disk: &Disk, worker: Arc<WorkerTask>) -> Result<(), Error>
     for disk_info in get_lsblk_info()?.iter() {
         if disk_info.path == disk_path_str && disk_info.partition_type.is_some() {
             is_partition = true;
+            if matches!(
+                disk_info.partition_type.as_deref(),
+                Some(EFI_PARTITION_TYPE)
+            ) {
+                bail!("You will not be able to boot if you wipe the EFI partition.");
+            }
         }
     }
 
-- 
2.39.2

Re: [pbs-devel] [PATCH proxmox-backup v2 4/6] fix #3690: cli: add function wipe_disk

[Philipp Hufnagl]

Hello

Maybe I am doing something wrong but I get when I try to apply the patch:

error: patch failed: src/bin/proxmox_backup_manager/disk.rs:350
error: src/bin/proxmox_backup_manager/disk.rs: patch does not apply

Re: [pbs-devel] [PATCH proxmox-backup v2 4/6] fix #3690: cli: add function wipe_disk

[Max Carrara]

This patch doesn't seem to apply on master (as of commit 78fc7b0e94b5fb840432075cafcd07efd2873f81).

The remaining patches all apply if the rejected hunk is ignored.

Here's the rejected hunk in question:

diff a/src/bin/proxmox_backup_manager/disk.rs b/src/bin/proxmox_backup_manager/disk.rs	(rejected hunks)
@@ -350,10 +376,10 @@ pub fn filesystem_commands() -> CommandLineInterface {
             CliCommand::new(&API_METHOD_CREATE_DATASTORE_DISK)
                 .arg_param(&["name"])
                 .completion_cb("disk", complete_disk_name),
-        ).insert(
+        )
+        .insert(
             "delete",
-            CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK)
-                .arg_param(&["name"]),
+            CliCommand::new(&API_METHOD_DELETE_DATASTORE_DISK).arg_param(&["name"]),
         );

     cmd_def.into()

Re: [pbs-devel] [PATCH proxmox-backup v2 4/6] fix #3690: cli: add function wipe_disk

[Max Carrara]

On 11/28/23 12:18, Philipp Hufnagl wrote:
> Hello
> 
> Maybe I am doing something wrong but I get when I try to apply the patch:
> 
> error: patch failed: src/bin/proxmox_backup_manager/disk.rs:350
> error: src/bin/proxmox_backup_manager/disk.rs: patch does not apply

You were a little faster than me ;) You're correct, the patch doesn't fully
apply. See my response for the rejected hunk.

> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
>