From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 88D9273EE1 for ; Thu, 8 Jul 2021 09:29:33 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8023513333 for ; Thu, 8 Jul 2021 09:29:33 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 816B613328 for ; Thu, 8 Jul 2021 09:29:32 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 4F09240EF5 for ; Thu, 8 Jul 2021 09:29:32 +0200 (CEST) Date: Thu, 08 Jul 2021 09:29:19 +0200 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20210707102250.5478-1-f.ebner@proxmox.com> <20210707102250.5478-2-f.ebner@proxmox.com> In-Reply-To: <20210707102250.5478-2-f.ebner@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1625729074.m9x7ql96rs.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.512 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, pve6to7.pm] Subject: Re: [pve-devel] [PATCH manager 2/2] pve6to7: storage content: ignore misconfigured unreferenced volumes X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Jul 2021 07:29:33 -0000 On July 7, 2021 12:22 pm, Fabian Ebner wrote: > If the same local storage is configured twice with content type > separation, migration in PVE 6 would lead to the volumes being > duplicated. As that would happen for every migration, such an issue > would likely be noticed already, and in PVE 7 such configuration is > not problematic for migration anymore. Also, misconfigured > unreferenced volumes are not an issue with respect to the upgrade > itself, just drop the check. but those checks also catch storages that are misconfigured for which no=20 such inverse storage with opposite content type and otherwise identical=20 settings exists? we can't just drop them altogether? we COULD skip them conditionally for storage pairs (same type, same=20 'path'/pool/pool+mons/.., one with images on with rootfs), but such=20 setups are still wrong and not properly separated IMHO. and stuff like=20 dir-storage on-top of other dir-like storage with volumes stored in the=20 same path are not really "detectable" in a reliable and cheap fashion=20 and should really be fixed by the user (e.g., by moving the dir storage=20 into a non-confusable sub-dir of the backing storage). >=20 > It's not necessary to scan storages with either 'images' or 'rootdir' > anymore, as only the log_info() remains. >=20 > Signed-off-by: Fabian Ebner > --- > PVE/CLI/pve6to7.pm | 43 ++++++------------------------------------- > 1 file changed, 6 insertions(+), 37 deletions(-) >=20 > diff --git a/PVE/CLI/pve6to7.pm b/PVE/CLI/pve6to7.pm > index 17da70e8..7d7b09d2 100644 > --- a/PVE/CLI/pve6to7.pm > +++ b/PVE/CLI/pve6to7.pm > @@ -695,15 +695,11 @@ sub check_description_lengths { > sub check_storage_content { > log_info("Checking storage content type configuration.."); > =20 > - my $found_referenced; > - my $found_unreferenced; > + my $found; > my $pass =3D 1; > =20 > my $storage_cfg =3D PVE::Storage::config(); > =20 > - my $potentially_affected =3D {}; > - my $referenced_volids =3D {}; > - > for my $storeid (sort keys $storage_cfg->{ids}->%*) { > my $scfg =3D $storage_cfg->{ids}->{$storeid}; > =20 > @@ -718,7 +714,8 @@ sub check_storage_content { > delete $scfg->{content}->{none}; # scan for guest images below > } > =20 > - next if $scfg->{content}->{images} && $scfg->{content}->{rootdir}; > + next if $scfg->{content}->{images}; > + next if $scfg->{content}->{rootdir}; > =20 > # Skip 'iscsi(direct)' (and foreign plugins with potentially similiar b= ehavior) with 'none', > # because that means "use LUNs directly" and vdisk_list() in PVE 6.x st= ill lists those. > @@ -739,12 +736,8 @@ sub check_storage_content { > } > my @volids =3D map { $_->{volid} } $res->{$storeid}->@*; > =20 > - for my $volid (@volids) { > - $potentially_affected->{$volid} =3D 1; > - } > - > my $number =3D scalar(@volids); > - if ($number > 0 && !$scfg->{content}->{images} && !$scfg->{content}->{r= ootdir}) { > + if ($number > 0) { > log_info("storage '$storeid' - neither content type 'images' nor 'r= ootdir' configured" > .", but found $number guest volume(s)"); > } > @@ -753,8 +746,6 @@ sub check_storage_content { > my $check_volid =3D sub { > my ($volid, $vmid, $vmtype, $reference) =3D @_; > =20 > - $referenced_volids->{$volid} =3D 1 if $reference ne 'unreferenced'; > - > my $guesttext =3D $vmtype eq 'qemu' ? 'VM' : 'CT'; > my $prefix =3D "$guesttext $vmid - volume '$volid' ($reference)"; > =20 > @@ -777,19 +768,14 @@ sub check_storage_content { > } > =20 > if (!$scfg->{content}->{$vtype}) { > - $found_referenced =3D 1 if $reference ne 'unreferenced'; > - $found_unreferenced =3D 1 if $reference eq 'unreferenced'; > + $found =3D 1; > $pass =3D 0; > log_warn("$prefix - storage does not have content type '$vtype' con= figured."); > } > }; > =20 > - my $guests =3D {}; > - > my $cts =3D PVE::LXC::config_list(); > for my $vmid (sort { $a <=3D> $b } keys %$cts) { > - $guests->{$vmid} =3D 'lxc'; > - > my $conf =3D PVE::LXC::Config->load_config($vmid); > =20 > my $volhash =3D {}; > @@ -817,8 +803,6 @@ sub check_storage_content { > =20 > my $vms =3D PVE::QemuServer::config_list(); > for my $vmid (sort { $a <=3D> $b } keys %$vms) { > - $guests->{$vmid} =3D 'qemu'; > - > my $conf =3D PVE::QemuConfig->load_config($vmid); > =20 > my $volhash =3D {}; > @@ -849,26 +833,11 @@ sub check_storage_content { > } > } > =20 > - if ($found_referenced) { > + if ($found) { > log_warn("Proxmox VE 7.0 enforces stricter content type checks. The gue= sts above " . > "might not work until the storage configuration is fixed."); > } > =20 > - for my $volid (sort keys $potentially_affected->%*) { > - next if $referenced_volids->{$volid}; # already checked > - > - my (undef, undef, $vmid) =3D PVE::Storage::parse_volname($storage_cfg, = $volid); > - my $vmtype =3D $guests->{$vmid}; > - next if !$vmtype; > - > - $check_volid->($volid, $vmid, $vmtype, 'unreferenced'); > - } > - > - if ($found_unreferenced) { > - log_warn("When migrating, Proxmox VE 7.0 only scans storages with the a= ppropriate " . > - "content types for unreferenced guest volumes."); > - } > - > if ($pass) { > log_pass("no problems found"); > } > --=20 > 2.20.1 >=20 >=20 >=20 > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >=20 >=20 >=20