all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Aaron Lauterer <a.lauterer@proxmox.com>
To: Proxmox VE development discussion <pve-devel@lists.proxmox.com>,
	Christoph Heiss <c.heiss@proxmox.com>
Subject: Re: [pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install
Date: Mon, 8 Jul 2024 16:16:23 +0200	[thread overview]
Message-ID: <bd44947a-1a5d-4caf-86c4-45b44aaff40a@proxmox.com> (raw)
In-Reply-To: <20240516102837.422278-4-c.heiss@proxmox.com>



On  2024-05-16  12:28, Christoph Heiss wrote:
> .. in the same manner as the detection for LVM works.
> 
> zpools can only be renamed by importing them with a new name, so
> unfortunaly the import-export dance is needed.
> 
> Signed-off-by: Christoph Heiss <c.heiss@proxmox.com>
> ---
>   Proxmox/Install.pm | 33 +++++++++++++++++++++++++++++++++
>   1 file changed, 33 insertions(+)
> 
> diff --git a/Proxmox/Install.pm b/Proxmox/Install.pm
> index c86a574..531ef1c 100644
> --- a/Proxmox/Install.pm
> +++ b/Proxmox/Install.pm
> @@ -16,6 +16,7 @@ use Proxmox::Install::StorageConfig;
>   use Proxmox::Sys::Block qw(get_cached_disks wipe_disk partition_bootable_disk);
>   use Proxmox::Sys::Command qw(run_command syscmd);
>   use Proxmox::Sys::File qw(file_read_firstline file_write_all);
> +use Proxmox::Sys::ZFS;
>   use Proxmox::UI;
>   
>   # TODO: move somewhere better?
> @@ -176,9 +177,41 @@ sub random_short_uid {
>       return sprintf "%08X", rand(0xffffffff);
>   }
>   
> +sub zfs_ask_existing_zpool_rename {
> +    my ($pool_name) = @_;
> +
> +    # At this point, no pools should be imported/active
> +    my $exported_pools = Proxmox::Sys::ZFS::get_exported_pools();
> +
> +    foreach (@$exported_pools) {
> +	next if $_->{name} ne $pool_name || $_->{state} ne 'ONLINE';
> +	my $renamed_pool = "$_->{name}-OLD-" . random_short_uid();

since the pool already has a unigue numerical id, couln't we use that 
instead of generating a new one?

we even have everything in place with $_->{id}.

> +
> +	my $response_ok = Proxmox::UI::prompt(
> +	    "A ZFS pool named '$_->{name}' (id $_->{id}) already exists on the system.\n\n" .
> +	    "Do you want to rename the pool to '$renamed_pool' before continuing\n" .
> +	    "or cancel the installation?"
> +	);
> +
> +	# Import the pool using its id, as that is unique and works even if there are
> +	# multiple zpools with the same name.
> +	if ($response_ok) {
> +	    syscmd("zpool import -f $_->{id} $renamed_pool") == 0 ||
> +		die "failed to import zfs pool '$_->{name}' with new name '$renamed_pool'\n";
> +	    syscmd("zpool export $renamed_pool") == 0 ||
> +		warn "failed to export renamed zfs pool '$renamed_pool'\n";
> +	} else {
> +	    warn "Canceled installation as requested by user, due to already existing ZFS pool '$pool_name'\n";
> +	    die "\n"; # causes abort without re-showing an error dialogue
> +	}
> +    }
> +}
> +
>   sub zfs_create_rpool {
>       my ($vdev, $pool_name, $root_volume_name) = @_;
>   
> +    zfs_ask_existing_zpool_rename($pool_name);
> +
>       my $iso_env = Proxmox::Install::ISOEnv::get();
>   
>       my $zfs_opts = Proxmox::Install::Config::get_zfs_opt();


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


  reply	other threads:[~2024-07-08 14:16 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-05-16 10:28 [pve-devel] [PATCH installer 0/3] add check/rename for already-existing ZFS rpool Christoph Heiss
2024-05-16 10:28 ` [pve-devel] [PATCH installer 1/3] low-level: add zfs module for retrieving importable zpool info Christoph Heiss
2024-07-08 14:24   ` Aaron Lauterer
2024-07-08 15:13     ` Christoph Heiss
2024-05-16 10:28 ` [pve-devel] [PATCH installer 2/3] low-level: install: split out random disk uid generation Christoph Heiss
2024-05-16 10:28 ` [pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install Christoph Heiss
2024-07-08 14:16   ` Aaron Lauterer [this message]
2024-07-08 15:16     ` Christoph Heiss
2024-07-08 11:27 ` [pve-devel] [PATCH installer 0/3] add check/rename for already-existing ZFS rpool Christoph Heiss
2024-07-11 12:00 ` Christoph Heiss

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=bd44947a-1a5d-4caf-86c4-45b44aaff40a@proxmox.com \
    --to=a.lauterer@proxmox.com \
    --cc=c.heiss@proxmox.com \
    --cc=pve-devel@lists.proxmox.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal