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

[Roland]

>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?

i'm a systems admin and following pbs/pve dev a little bit and i just
stumbled across this, raising an eyebrow

i'd second what thomas writes - why shoud a disk with efi partitions be
protected from wiping? it could be ANY important disk with ANY important
data on it, no matter of there is a efi partition or not.

i we add a disk we want to use and want to wipe - we just want to wipe,
no matter what's on it.

>- such special partitions get an extra, specific warning in the UI about the
>  potential impact

yes, that would be probably good.

when wiping a disk, you always should think twice and double check,
anyway...

roland

Am 28.11.23 um 18:48 schrieb Thomas Lamprecht:

> Am 28/11/2023 um 14:23 schrieb Markus Frank:
>> If the GUID is c12a7328-f81f-11d2-ba4b-00a0c93ec93b the partition is an
>> EFI Partition and should not be wiped.
>>
>> Since this GUID is used multiple times, a constant for the EFI & BIOS
>> GUID is useful.
>>
>> Signed-off-by: Markus Frank <m.frank at proxmox.com>
>> ---
>>   src/tools/disks/mod.rs | 13 +++++++++++--
>>   1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/tools/disks/mod.rs b/src/tools/disks/mod.rs
>> index beb8178b..0fe57d23 100644
>> --- a/src/tools/disks/mod.rs
>> +++ b/src/tools/disks/mod.rs
>> @@ -39,6 +39,9 @@ lazy_static::lazy_static! {
>>           regex::Regex::new(r"host[^/]*/session[^/]*").unwrap();
>>   }
>>
>> +const EFI_PARTITION_TYPE: &str = "c12a7328-f81f-11d2-ba4b-00a0c93ec93b";
>> +const BIOS_PARTITION_TYPE: &str = "21686148-6449-6e6f-744e-656564454649";
>> +
>>   /// Disk management context.
>>   ///
>>   /// This provides access to disk information with some caching for faster querying of multiple
>> @@ -844,8 +847,8 @@ fn get_partitions_info(
>>               if let (Some(devpath), Some(infos)) = (devpath.as_ref(), lsblk_infos.as_ref()) {
>>                   for info in infos.iter().filter(|i| i.path.eq(devpath)) {
>>                       used = match info.partition_type.as_deref() {
>> -                        Some("21686148-6449-6e6f-744e-656564454649") => PartitionUsageType::BIOS,
>> -                        Some("c12a7328-f81f-11d2-ba4b-00a0c93ec93b") => PartitionUsageType::EFI,
>> +                        Some(BIOS_PARTITION_TYPE) => PartitionUsageType::BIOS,
>> +                        Some(EFI_PARTITION_TYPE) => PartitionUsageType::EFI,
>>                           Some("6a945a3b-1dd2-11b2-99a6-080020736631") => {
>>                               PartitionUsageType::ZfsReserved
>>                           }
>> @@ -1080,6 +1083,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.");
>> +            }
>>           }
>>       }
>>
> I skipped this one for now, see no real sense in singling this out, and why should
> one be forbidden to wipe the paritions of a hard-disk that one moved over from another
> system, where it was previously used for booting, or if one has to switch EFI partition
> and wants to wipe the old one afterwards?
>
> I'd rather see:
> - such special partitions get an extra, specific warning in the UI about the
>    potential impact
> - a force flag added, which then also passes that to wipedisk to be able to
>    actually clear a disk that was in use sometimes
> - the same then implemented for Proxmox VE
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel at lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>