Re: [pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install
On Mon, Jul 08, 2024 at 04:16:23PM GMT, Aaron Lauterer wrote:
> On 2024-05-16 12:28, Christoph Heiss wrote:
> > [..]
> > +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?
Good point, I will change that. Only thing I might have to look into is
the maximum pool name length, since the numerical IDs are quite long.
But in the worst case, they should still be unique enough even after
shortening to an appropriate length.
>
> we even have everything in place with $_->{id}.
>
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install
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
---
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(0x);
}
+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
[pve-devel] [PATCH installer 3/3] low-level: install: check for already-existing `rpool` on install
.. 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
---
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(0x);
}
+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();
+
+ 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();
--
2.44.0
___
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
