From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 55CEB1FF141 for ; Tue, 05 May 2026 14:49:28 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 7187ED96; Tue, 5 May 2026 14:49:27 +0200 (CEST) Message-ID: <56de5717-2aae-4e94-b1ff-6e0b10372311@proxmox.com> Date: Tue, 5 May 2026 14:48:50 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser To: =?UTF-8?Q?Fabian_Gr=C3=BCnbichler?= , pbs-devel@lists.proxmox.com References: <20260430100506.159160-1-n.frey@proxmox.com> <1777900823.7usgya04qb.astroid@yuna.none> Content-Language: en-US From: Nicolas Frey In-Reply-To: <1777900823.7usgya04qb.astroid@yuna.none> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1777985224882 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.777 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_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] Message-ID-Hash: 7SSM72M7AFCG33EANTKJUEJWBQ4747EY X-Message-ID-Hash: 7SSM72M7AFCG33EANTKJUEJWBQ4747EY X-MailFrom: n.frey@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 5/4/26 3:28 PM, Fabian Grünbichler wrote: > On April 30, 2026 12:05 pm, Nicolas Frey wrote: >> so spares that have the status `INUSE` parse correctly. >> >> adds a line to the mock config in the `test_zpool_status_parser_spares` >> test containing an example of a vdev that would be `INUSE`. >> >> Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=7541 >> Signed-off-by: Nicolas Frey >> --- >> src/tools/disks/zpool_status.rs | 33 +++++++++++++++++++-------------- >> 1 file changed, 19 insertions(+), 14 deletions(-) >> >> diff --git a/src/tools/disks/zpool_status.rs b/src/tools/disks/zpool_status.rs >> index 05ef5d80..071431f2 100644 >> --- a/src/tools/disks/zpool_status.rs >> +++ b/src/tools/disks/zpool_status.rs >> @@ -14,7 +14,7 @@ use nom::{ >> character::complete::line_ending, >> combinator::opt, >> multi::{many0, many1}, >> - sequence::preceded, >> + sequence::{preceded, tuple}, >> }; >> >> #[derive(Debug, Serialize, Deserialize)] >> @@ -66,33 +66,37 @@ fn parse_zpool_status_vdev(i: &str) -> IResult<&str, ZFSPoolVDevState> { >> } >> >> let (i, state) = preceded(multispace1, notspace1)(i)?; >> - if let Ok((n, _)) = preceded(multispace0, line_ending)(i) { >> - // spares >> + if let Ok((n, (read, write, cksum, msg, _))) = tuple(( >> + preceded(multispace1, parse_u64), >> + preceded(multispace1, parse_u64), >> + preceded(multispace1, parse_u64), >> + opt(preceded(multispace1, take_while1(|c: char| c != '\n'))), >> + line_ending, >> + ))(i) >> + { >> let vdev = ZFSPoolVDevState { >> name: vdev_name.to_string(), >> lvl: indent_level, >> state: Some(state.to_string()), >> - read: None, >> - write: None, >> - cksum: None, >> - msg: None, >> + read: Some(read), >> + write: Some(write), >> + cksum: Some(cksum), >> + msg: msg.map(String::from), >> }; >> return Ok((n, vdev)); > > I think for following along, it would actually be easier to make > read/write/cksum an optional tuple? would also drop a bit of duplicated > code AFAICT.. > Makes a lot of sense here, will add as separate commit in v2. Thanks! >> } >> >> - let (i, read) = preceded(multispace1, parse_u64)(i)?; >> - let (i, write) = preceded(multispace1, parse_u64)(i)?; >> - let (i, cksum) = preceded(multispace1, parse_u64)(i)?; >> - let (i, msg) = opt(preceded(multispace1, take_while(|c| c != '\n')))(i)?; >> + // spares >> + let (i, msg) = opt(preceded(multispace1, take_while1(|c: char| c != '\n')))(i)?; >> let (i, _) = line_ending(i)?; >> >> let vdev = ZFSPoolVDevState { >> name: vdev_name.to_string(), >> lvl: indent_level, >> state: Some(state.to_string()), >> - read: Some(read), >> - write: Some(write), >> - cksum: Some(cksum), >> + read: None, >> + write: None, >> + cksum: None, >> msg: msg.map(String::from), >> }; >> >> @@ -490,6 +494,7 @@ config: >> spares >> /dev/sdb AVAIL >> /dev/sdc AVAIL >> + /dev/sdd INUSE currently in use >> >> errors: No known data errors >> "###; >> -- >> 2.47.3 >> >> >> >>