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
next prev parent reply other threads:[~2024-07-15 14:48 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-11 11:57 [pve-devel] [PATCH installer v3 0/4] add check/rename for already-existing ZFS rpool Christoph Heiss
2024-07-11 11:57 ` [pve-devel] [PATCH installer v3 1/4] proxmox: add zfs module for retrieving importable zpool info Christoph Heiss
2024-07-11 11:57 ` [pve-devel] [PATCH installer v3 2/4] test: add test cases for new zfs module Christoph Heiss
2024-07-11 11:57 ` [pve-devel] [PATCH installer v3 3/4] install: config: rename option lvm_auto_rename -> existing_storage_auto_rename Christoph Heiss
2024-07-11 11:57 ` [pve-devel] [PATCH installer v3 4/4] low-level: install: check for already-existing `rpool` on install Christoph Heiss
2024-07-12 9:36 ` Aaron Lauterer
2024-07-15 14:49 ` Christoph Heiss [this message]
2024-07-12 9:42 ` [pve-devel] [PATCH installer v3 0/4] add check/rename for already-existing ZFS rpool Aaron Lauterer
2024-07-15 14:52 ` Christoph Heiss
2024-07-16 8:33 ` 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=4i5cm53dazdtkjcbcjmnec6fp5sk2k3a62ec4k2lzpm2bzp4iv@uo2iajfidx62 \
--to=c.heiss@proxmox.com \
--cc=a.lauterer@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox