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 33B3F90C48 for ; Tue, 13 Feb 2024 09:34:38 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 14D7732517 for ; Tue, 13 Feb 2024 09:34:38 +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 for ; Tue, 13 Feb 2024 09:34:37 +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 1ABAF47B67 for ; Tue, 13 Feb 2024 09:34:37 +0100 (CET) Message-ID: Date: Tue, 13 Feb 2024 09:34:35 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: pve-devel@lists.proxmox.com References: <20240205175419.1271680-1-m.carrara@proxmox.com> <20240205175419.1271680-6-m.carrara@proxmox.com> <1707741866.lnmb36oztr.astroid@yuna.none> From: Max Carrara In-Reply-To: <1707741866.lnmb36oztr.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.549 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_ASCII_DIVIDERS 0.8 Email that uses ascii formatting dividers and possible spam tricks KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment KAM_LOTSOFHASH 0.25 Emails with lots of hash-like gibberish 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. [cephconfig.pm, ceph.com, proxmox.com] Subject: Re: [pve-devel] [PATCH v2 pve-storage 05/11] cephconfig: align our parser more with Ceph's parser 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: Tue, 13 Feb 2024 08:34:38 -0000 On 2/12/24 14:33, Fabian Grünbichler wrote: > On February 5, 2024 6:54 pm, Max Carrara wrote: >> 1. Comments, irrespective of whether they start with '#' or ';' are >> now treated the same. Otherwise, sections and key-value pairs with >> a trailing comment starting with ';' are still parsed. Consider >> this example: >> >> [some.section] # inline comment after section >> foo = bar ; inline comment after value >> >> The '[some.section]' section in the example above would otherwise >> not be parsed at all, while in the key-value definition 'foo' >> parses as the key, which is correct, but 'bar ; inline comment >> after value' parses as value, which is incorrect according to >> Ceph's grammar [0][1]. >> >> 2. Sections may now contain any character, including whitespace, but >> not '\n' or a comment literal '#' or ';'. The case for comment >> literals is handled in 1. above. > > these seem sensible - what about line continuations? ;) Yeah, those get swallowed. Will be corrected in v3, thanks for pointing this out! > >> >> 3. Instead of treating '-', '_' and ' ' as the same, only '_' and ' ' >> are treated the same, like in Ceph's parser [2]. > > the ceph docs state something else - which is wrong? ;) > > https://docs.ceph.com/en/latest/rados/configuration/ceph-conf/ says: > >> Each of the Ceph configuration options has a unique name that consists >> of words formed with lowercase characters and connected with >> underscore characters (_). > > this would seem to agree > >> When option names are specified on the command line, underscore (_) >> and dash (-) characters can be used interchangeably (for example, >> --mon-host is equivalent to --mon_host). > > okay, this is CLI which might just have its own mapping > >> When option names appear in configuration files, spaces can also be >> used in place of underscores or dashes. However, for the sake of >> clarity and convenience, we suggest that you consistently use >> underscores, as we do throughout this documentation. > > but this now says that dash in config files is OK? The docs and the code don't seem to agree in that regard; at least the INI parser in C++ does *not* treat `-` the same [0]: > /* Normalize a key name. > * > * Normalized key names have no leading or trailing whitespace, and all > * whitespace is stored as underscores. The main reason for selecting this > * normal form is so that in common/config.cc, we can use a macro to stringify > * the field names of md_config_t and get a key in normal form. > */ (The code also actually does what it says on the tin. ;) ) So, I think it would be sensible to stick to how the Code does it, in this case. Or was the original treatment of dashes in our code included in order to guard against user errors, perhaps? [0]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master#l294 > >> [0]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master#l178 >> [1]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master#l194 >> [2]: https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master#l294 >> >> Signed-off-by: Max Carrara >> --- >> Changes v1 --> v2: >> * new >> >> src/PVE/CephConfig.pm | 14 +++++++------- >> 1 file changed, 7 insertions(+), 7 deletions(-) >> >> diff --git a/src/PVE/CephConfig.pm b/src/PVE/CephConfig.pm >> index 6b10d46..77b745f 100644 >> --- a/src/PVE/CephConfig.pm >> +++ b/src/PVE/CephConfig.pm >> @@ -10,6 +10,8 @@ cfs_register_file('ceph.conf', >> \&parse_ceph_config, >> \&write_ceph_config); >> >> +# For more details on how Ceph's config parser works, see: >> +# https://git.proxmox.com/?p=ceph.git;a=blob;f=ceph/src/common/ConfUtils.cc;h=2f78fd02bf9e27467275752e6f3bca0c5e3946ce;hb=refs/heads/master >> sub parse_ceph_config { >> my ($filename, $raw) = @_; >> >> @@ -20,14 +22,13 @@ sub parse_ceph_config { >> >> my $section; >> >> - foreach my $line (@lines) { >> - $line =~ s/#.*$//; >> + for my $line (@lines) { >> + $line =~ s/(#|;).*$//; >> $line =~ s/^\s+//; >> - $line =~ s/^;.*$//; >> $line =~ s/\s+$//; >> next if !$line; >> >> - $section = $1 if $line =~ m/^\[(\S+)\]$/; >> + $section = $1 if $line =~ m/^\[(.+)\]$/; >> if (!$section) { >> warn "no section - skip: $line\n"; >> next; >> @@ -35,11 +36,10 @@ sub parse_ceph_config { >> >> if ($line =~ m/^(.*?\S)\s*=\s*(\S.*)$/) { >> my ($key, $val) = ($1, $2); >> - # ceph treats ' ', '_' and '-' in keys the same, so lets do too >> - $key =~ s/[-\ ]/_/g; >> + # ceph treats ' ' and '_' in keys the same, so lets do too >> + $key =~ s/ /_/g; >> $cfg->{$section}->{$key} = $val; >> } >> - >> } >> >> return $cfg; >> -- >> 2.39.2 >> >> >> >> _______________________________________________ >> pve-devel mailing list >> pve-devel@lists.proxmox.com >> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel >> >> >> > > > _______________________________________________ > pve-devel mailing list > pve-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel > >