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 8256B7E4F6 for ; Wed, 10 Nov 2021 14:32:12 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 757A31D091 for ; Wed, 10 Nov 2021 14:31:42 +0100 (CET) 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 0A8E11D085 for ; Wed, 10 Nov 2021 14:31:41 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id D69F444B98 for ; Wed, 10 Nov 2021 14:31:40 +0100 (CET) Date: Wed, 10 Nov 2021 14:31:34 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox VE development discussion References: <20211105102945.185988-1-f.ebner@proxmox.com> <20211105102945.185988-2-f.ebner@proxmox.com> <6457c3b2-78c9-bca9-690e-0bf78a846bc3@proxmox.com> In-Reply-To: <6457c3b2-78c9-bca9-690e-0bf78a846bc3@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.15.0 (https://github.com/astroidmail/astroid) Message-Id: <1636551047.wh5nb02p22.astroid@nora.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.284 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, lvmplugin.pm, lvmthinplugin.pm] Subject: [pve-devel] applied-series: [PATCH storage 2/2] lvm thin: don't assume that a thin pool and its volumes are active 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: Wed, 10 Nov 2021 13:32:12 -0000 On November 8, 2021 8:49 am, Fabian Ebner wrote: > Am 05.11.21 um 11:29 schrieb Fabian Ebner: >> There are cases where autoactivation can fail, as reported in the >> community forum [0]. And it could also be that a volume was >> deactivated by something outside of our control. >>=20 >> It doesn't seem strictly necessary to activate the thin pool itself >> (creating/removing/activating LVs within the pool still works if it's >> not active), but it does not report usage information as long as >> neither the pool nor any of its LVs are active. Activate the pool for >> that, for being able to use the flag in status(), and it should also >> serve as a good indicator that there's a problem with the pool if it >> can't be activated. >=20 > For the user reporting the issue, the autoactivation manages to activate=20 > the _tdata _tmeta component LVs [0] and then crashes (or at least fails=20 > without reverting the partial activation). But as long as the component=20 > LVs are activate, neither the pool LV nor any LVs in the pool can be=20 > activated. Meaning that this patch does not help for that scenario. applied anyway, since the changes do make sense for other situations=20 where volumes are not activated (anymore/automatically/..) >=20 > [0]:=20 > https://forum.proxmox.com/threads/local-lvm-not-available-after-kernel-up= date-on-pve-7.97406/post-428298 >=20 >>=20 >> Before activating, check the (cached) lv_state from lvm_list_volumes. >> It's necessary to update the cache in activate_storage, because the >> flag is re-used in status(). Also update it for other (de)activations >> to be more future-proof. >>=20 >> [0]: https://forum.proxmox.com/threads/local-lvm-not-available-after-ker= nel-update-on-pve-7.97406 >>=20 >> Signed-off-by: Fabian Ebner >> --- >> PVE/Storage/LVMPlugin.pm | 1 + >> PVE/Storage/LvmThinPlugin.pm | 64 +++++++++++++++++++++++------------- >> 2 files changed, 42 insertions(+), 23 deletions(-) >>=20 >> diff --git a/PVE/Storage/LVMPlugin.pm b/PVE/Storage/LVMPlugin.pm >> index 139d391..b44aeb8 100644 >> --- a/PVE/Storage/LVMPlugin.pm >> +++ b/PVE/Storage/LVMPlugin.pm >> @@ -173,6 +173,7 @@ sub lvm_list_volumes { >> =20 >> my $d =3D { >> lv_size =3D> int($lv_size), >> + lv_state =3D> substr($lv_attr, 4, 1), >> lv_type =3D> $lv_type, >> }; >> $d->{pool_lv} =3D $pool_lv if $pool_lv; >> diff --git a/PVE/Storage/LvmThinPlugin.pm b/PVE/Storage/LvmThinPlugin.pm >> index 34e57b2..ea7b9b3 100644 >> --- a/PVE/Storage/LvmThinPlugin.pm >> +++ b/PVE/Storage/LvmThinPlugin.pm >> @@ -202,43 +202,61 @@ sub status { >> =20 >> return if !$info || $info->{lv_type} ne 't' || !$info->{lv_size}; >> =20 >> - return ($info->{lv_size}, $info->{lv_size} - $info->{used}, $info->= {used}, 1); >> + return ( >> + $info->{lv_size}, >> + $info->{lv_size} - $info->{used}, >> + $info->{used}, >> + $info->{lv_state} eq 'a' ? 1 : 0, >> + ); >> +} >> + >> +my $activate_lv =3D sub { >> + my ($vg, $lv, $cache) =3D @_; >> + >> + my $lvs =3D $cache->{lvs} ||=3D PVE::Storage::LVMPlugin::lvm_list_v= olumes(); >> + >> + die "no such logical volume $vg/$lv" if !$lvs->{$vg} || !$lvs->{$vg= }->{$lv}; >> + >> + return if $lvs->{$vg}->{$lv}->{lv_state} eq 'a'; >> + >> + run_command(['lvchange', '-ay', '-K', "$vg/$lv"], errmsg =3D> "acti= vating LV '$vg/$lv' failed"); >> + >> + $lvs->{$vg}->{$lv}->{lv_state} =3D 'a'; # update cache >> + >> + return; >> +}; >> + >> +sub activate_storage { >> + my ($class, $storeid, $scfg, $cache) =3D @_; >> + >> + $class->SUPER::activate_storage($storeid, $scfg, $cache); >> + >> + $activate_lv->($scfg->{vgname}, $scfg->{thinpool}, $cache); >> } >> =20 >> sub activate_volume { >> my ($class, $storeid, $scfg, $volname, $snapname, $cache) =3D @_; >> =20 >> my $vg =3D $scfg->{vgname}; >> + my $lv =3D $snapname ? "snap_${volname}_$snapname" : $volname; >> =20 >> - # only snapshot volumes needs activation >> - if ($snapname) { >> - my $snapvol =3D "snap_${volname}_$snapname"; >> - my $cmd =3D ['/sbin/lvchange', '-ay', '-K', "$vg/$snapvol"]; >> - run_command($cmd, errmsg =3D> "activate_volume '$vg/$snapvol' error"); >> - } elsif ($volname =3D~ /^base-/) { >> - my $cmd =3D ['/sbin/lvchange', '-ay', '-K', "$vg/$volname"]; >> - run_command($cmd, errmsg =3D> "activate_volume '$vg/$volname' error"); >> - } else { >> - # other volumes are active by default >> - } >> + $activate_lv->($vg, $lv, $cache); >> } >> =20 >> sub deactivate_volume { >> my ($class, $storeid, $scfg, $volname, $snapname, $cache) =3D @_; >> =20 >> + return if !$snapname && $volname !~ /^base-/; # other volumes are k= ept active >> + >> my $vg =3D $scfg->{vgname}; >> + my $lv =3D $snapname ? "snap_${volname}_$snapname" : $volname; >> =20 >> - # we only deactivate snapshot volumes >> - if ($snapname) { >> - my $snapvol =3D "snap_${volname}_$snapname"; >> - my $cmd =3D ['/sbin/lvchange', '-an', "$vg/$snapvol"]; >> - run_command($cmd, errmsg =3D> "deactivate_volume '$vg/$snapvol' error"= ); >> - } elsif ($volname =3D~ /^base-/) { >> - my $cmd =3D ['/sbin/lvchange', '-an', "$vg/$volname"]; >> - run_command($cmd, errmsg =3D> "deactivate_volume '$vg/$volname' error"= ); >> - } else { >> - # other volumes are kept active >> - } >> + run_command(['lvchange', '-an', "$vg/$lv"], errmsg =3D> "deactivate= _volume '$vg/$lv' error"); >> + >> + $cache->{lvs}->{$vg}->{$lv}->{lv_state} =3D '-' # update cache >> + if $cache->{lvs} && $cache->{lvs}->{$vg} && $cache->{lvs}->{$vg}->{$lv= }; >> + >> + return; >> } >> =20 >> sub clone_image { >>=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