Re: [pve-devel] [PATCH installer v3 4/4] low-level: install: check for already-existing `rpool` on install

From: Christoph Heiss <c.heiss@proxmox.com>
To: Aaron Lauterer <a.lauterer@proxmox.com>
Cc: Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH installer v3 4/4] low-level: install: check for already-existing `rpool` on install
Date: Mon, 15 Jul 2024 16:49:00 +0200	[thread overview]
Message-ID: <4i5cm53dazdtkjcbcjmnec6fp5sk2k3a62ec4k2lzpm2bzp4iv@uo2iajfidx62> (raw)
In-Reply-To: <0d63d9ef-d6cd-4d4a-a68a-ae8ac5f2f9cb@proxmox.com>

Thanks for the review!

On Fri, Jul 12, 2024 at 11:36:56AM GMT, Aaron Lauterer wrote:
> only one small nit inline
>
> On  2024-07-11  13:57, 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-$_->{id}";
> > +
> > +	my $response_ok = Proxmox::Install::Config::get_existing_storage_auto_rename();
> maybe we want to name this differently to avoid confusion?
> response_ok -> do_rename
> or something in that regard?
>
> but that could be done in a follow up patch as well if we want to

Makes sense - I'll spin a new revision of this series and address this
directly.

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