public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Fabio Fantoni via pve-devel <pve-devel@lists.proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Daniel Kral <d.kral@proxmox.com>
Cc: Fabio Fantoni <fabio.fantoni@m2r.biz>
Subject: Re: [pve-devel] [RFC PATCH 2/2] common: btrfs: lower minimum amount of disks for raid10 to 2
Date: Mon, 13 Jan 2025 13:24:06 +0100	[thread overview]
Message-ID: <mailman.254.1736771058.441.pve-devel@lists.proxmox.com> (raw)
In-Reply-To: <20250110170040.201474-2-d.kral@proxmox.com>

[-- Attachment #1: Type: message/rfc822, Size: 9776 bytes --]

From: Fabio Fantoni <fabio.fantoni@m2r.biz>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>, Daniel Kral <d.kral@proxmox.com>
Subject: Re: [pve-devel] [RFC PATCH 2/2] common: btrfs: lower minimum amount of disks for raid10 to 2
Date: Mon, 13 Jan 2025 13:24:06 +0100
Message-ID: <0999b2e1-8b8b-4baf-84d6-32251a675338@m2r.biz>

Il 10/01/2025 18:00, Daniel Kral ha scritto:
> As the installer allows single-disk RAID0 configurations and BTRFS
> allows to create a filesystem with the RAID10 profile with only two
> disks since kernel version 5.15 [0], lower the minimum amount of disks
> the installer requires for a BTRFS RAID10 setup.
>
> The motiviation for this is to allow users to create a BTRFS RAID10
> configuration even though they do not have the necessary disks ready at
> setup time itself without needing to convert the profile afterwards.

btrfs profiles work differently but other hardware or software raids,
many users may not inform themselves well beforehand but even in the
case of informed users even if technically now btrfs allows lower limits
with the creation of raid 0 (and raid10) I think it would be better to
keep them at the base at the creation and then it must be the user who
consciously makes any subsequent conversions.

regarding btrfs profiles at creation, one thing that could be useful is
to always put duplicate metadata (dup with single disk or raid 1 in the
case of raid0), if you don't want it by default maybe put it as an
additional option, and if you don't want that either at least add it to
the documentation (as a suggestion if you want greater resilience of the
filesystem without consuming excessive space)

>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=b2f78e88052bc0bee56bbf646d245fcfb431a873
>
> Signed-off-by: Daniel Kral <d.kral@proxmox.com>
> ---
> Discussion
>
> If this patch seems like something worthwile, I think it would be
> necessary to have some sort of warning popup for 2 <= $diskcount < 4 in
> RAID10, and maybe also the same for $diskcount == 1 in RAID0, that
> there's no advantage to create a degenerate RAID0/10 without planning to
> add at least 1/2 disks later. I would add this in a v2 or followup if
> this gets ACKed.
>
>   Proxmox/Install.pm                          | 2 +-
>   proxmox-installer-common/src/disk_checks.rs | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
> index b72a83e..d52d17b 100644
> --- a/Proxmox/Install.pm
> +++ b/Proxmox/Install.pm
> @@ -365,7 +365,7 @@ sub get_btrfs_raid_setup {
>   	die "btrfs (RAID1) needs at least 2 devices\n" if $diskcount < 2;
>   	$mode = 'raid1';
>       } elsif ($filesys eq 'btrfs (RAID10)') {
> -	die "btrfs (RAID10) needs at least 4 devices\n" if $diskcount < 4;
> +	die "btrfs (RAID10) needs at least 2 devices\n" if $diskcount < 2;
>   	$mode = 'raid10';
>       } else {
>   	die "unknown btrfs mode '$filesys'\n";
> diff --git a/proxmox-installer-common/src/disk_checks.rs b/proxmox-installer-common/src/disk_checks.rs
> index ecc43bd..bd1c54c 100644
> --- a/proxmox-installer-common/src/disk_checks.rs
> +++ b/proxmox-installer-common/src/disk_checks.rs
> @@ -129,7 +129,7 @@ pub fn check_btrfs_raid_config(level: BtrfsRaidLevel, disks: &[Disk]) -> Result<
>       match level {
>           BtrfsRaidLevel::Raid0 => check_raid_min_disks(disks, 1)?,
>           BtrfsRaidLevel::Raid1 => check_raid_min_disks(disks, 2)?,
> -        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 4)?,
> +        BtrfsRaidLevel::Raid10 => check_raid_min_disks(disks, 2)?,
>       }
>
>       Ok(())
> @@ -204,8 +204,8 @@ mod tests {
>           assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid1, &disks).is_ok());
>
>           assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &[]).is_err());
> -        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..3]).is_err());
> -        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..4]).is_ok());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..1]).is_err());
> +        assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks[..2]).is_ok());
>           assert!(check_btrfs_raid_config(BtrfsRaidLevel::Raid10, &disks).is_ok());
>       }
>



--
Questa email è stata esaminata alla ricerca di virus dal software antivirus Avast.
www.avast.com


[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

  reply	other threads:[~2025-01-13 12:24 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-10 17:00 [pve-devel] [RFC PATCH 1/2] install: btrfs: fix raid level falling back to single mode Daniel Kral
2025-01-10 17:00 ` [pve-devel] [RFC PATCH 2/2] common: btrfs: lower minimum amount of disks for raid10 to 2 Daniel Kral
2025-01-13 12:24   ` Fabio Fantoni via pve-devel [this message]
     [not found]   ` <0999b2e1-8b8b-4baf-84d6-32251a675338@m2r.biz>
2025-01-15  9:00     ` Daniel Kral
2025-01-15 16:14       ` Fabio Fantoni via pve-devel
2025-01-13 12:15 ` [pve-devel] [RFC PATCH 1/2] install: btrfs: fix raid level falling back to single mode Fabio Fantoni via pve-devel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=mailman.254.1736771058.441.pve-devel@lists.proxmox.com \
    --to=pve-devel@lists.proxmox.com \
    --cc=d.kral@proxmox.com \
    --cc=fabio.fantoni@m2r.biz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal