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 5956E74BF4 for ; Thu, 2 Jun 2022 09:15:31 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 4D82D271DF for ; Thu, 2 Jun 2022 09:15:01 +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 BFDD8271D4 for ; Thu, 2 Jun 2022 09:15:00 +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 8FFD742A48 for ; Thu, 2 Jun 2022 09:15:00 +0200 (CEST) Message-ID: <09954bed-0250-8807-2975-eef2af2fdef1@proxmox.com> Date: Thu, 2 Jun 2022 09:14:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:101.0) Gecko/20100101 Thunderbird/101.0 Content-Language: en-GB To: Proxmox VE development discussion , Dominik Csapak References: <20220527123134.3822111-1-d.csapak@proxmox.com> <20220527123134.3822111-2-d.csapak@proxmox.com> From: Thomas Lamprecht Cc: Wolfgang Bumiller In-Reply-To: <20220527123134.3822111-2-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 1.046 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 NICE_REPLY_A -2.575 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [btrfsplugin.pm] URI_NOVOWEL 0.5 URI hostname has long non-vowel sequence Subject: Re: [pve-devel] [PATCH storage v2 2/2] BTRFSPlugin: reuse DirPlugin update/get_volume_attribute 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, 02 Jun 2022 07:15:31 -0000 Am 27/05/2022 um 14:31 schrieb Dominik Csapak: > this allows setting notes+protected for backups on btrfs > > Signed-off-by: Dominik Csapak > --- > PVE/Storage/BTRFSPlugin.pm | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) looks OK, but the whole situation with that and often also btrfs is sometimes a bit weird in subtle ways, so a quick look from you wolfgang would be appreciated. Also, one style nit inline. > > diff --git a/PVE/Storage/BTRFSPlugin.pm b/PVE/Storage/BTRFSPlugin.pm > index be613f4..55f3c05 100644 > --- a/PVE/Storage/BTRFSPlugin.pm > +++ b/PVE/Storage/BTRFSPlugin.pm > @@ -138,9 +138,16 @@ sub status { > return PVE::Storage::DirPlugin::status($class, $storeid, $scfg, $cache); > } > > -# TODO: sub get_volume_attribute {} > +sub get_volume_attribute { > + my ($class, $scfg, $storeid, $volname, $attribute) = @_; > + return PVE::Storage::DirPlugin::get_volume_attribute($class, $scfg, $storeid, $volname, $attribute); > +} > > -# TODO: sub update_volume_attribute {} > +sub update_volume_attribute { > + my ($class, $scfg, $storeid, $volname, $attribute, $value) = @_; > + return PVE::Storage::DirPlugin::update_volume_attribute($class, $scfg, $storeid, $volname, > + $attribute, $value); I'd actually prefer the rustfmt style for wrapping simple lists of parameters, in that case the following should make it happy, well theoretically as it would choke on perl, as lots others do: return PVE::Storage::DirPlugin::update_volume_attribute( $class, $scfg, $storeid, $volname, $attribute, $value); And if that'd still require to much width it'd go one param per line. I'll see that I update the style guide with that. > +} > > # croak would not include the caller from within this module > sub __error {