public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Nicolas Frey <n.frey@proxmox.com>, pbs-devel@lists.proxmox.com
Subject: Re: [PATCH proxmox-backup v2 2/2] fix #7541: zfs status: account for msg directly after status in vdev parser
Date: Thu, 07 May 2026 10:00:27 +0200	[thread overview]
Message-ID: <1778140573.9y7e3sqjfm.astroid@yuna.none> (raw)
In-Reply-To: <20260505141606.438908-3-n.frey@proxmox.com>

On May 5, 2026 4:16 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 <n.frey@proxmox.com>
> ---
> No functional changes since v1
>  src/tools/disks/zpool_status.rs | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/src/tools/disks/zpool_status.rs b/src/tools/disks/zpool_status.rs
> index 9427f6d4..267fb916 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)]
> @@ -68,29 +68,33 @@ 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()),
> -            stats: None,
> -            msg: None,
> +            stats: Some(VDevStats { read, write, cksum }),
> +            msg: msg.map(String::from),

you misunderstood what I wrote as reply to v1.

we can actually parse the stats optionally first.

and then as next step parse the message.

and then as final step parse the line ending

and then have a single return statement :)

basically

// spares don't have stats
let stats = opt_parse_stats();

let msg = opt_parse_msg();
line_ending();
return

instead of

if (stats and msg and line_ending) {
   return
}

msg = ..
line_ending
return

the goal is to avoid the duplication of parsing the message and line
ending, and making the flow more natural, as that is how lines are
structured:

vdev state [stat stat stat] [msg]\n


>          };
>          return Ok((n, vdev));
>      }
> 
> -    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()),
> -        stats: Some(VDevStats { read, write, cksum }),
> +        stats: None,
>          msg: msg.map(String::from),
>      };
> 
> @@ -486,6 +490,7 @@ config:
>          spares
>            /dev/sdb     AVAIL
>            /dev/sdc     AVAIL
> +          /dev/sdd     INUSE    currently in use
> 
>  errors: No known data errors
>  "###;
> --
> 2.47.3
> 
> 
> 
> 




  reply	other threads:[~2026-05-07  8:00 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-05 14:16 [PATCH proxmox-backup v2 0/2] fix #7541: zfs status: account for msg directly after status in vdev parser Nicolas Frey
2026-05-05 14:16 ` [PATCH proxmox-backup v2 1/2] zfs: status: add `VDevStats` struct instead of 3 optional u64s Nicolas Frey
2026-05-07  8:00   ` Fabian Grünbichler
2026-05-07  9:16     ` Nicolas Frey
2026-05-07  9:19       ` Thomas Lamprecht
2026-05-05 14:16 ` [PATCH proxmox-backup v2 2/2] fix #7541: zfs status: account for msg directly after status in vdev parser Nicolas Frey
2026-05-07  8:00   ` Fabian Grünbichler [this message]
2026-05-07  9:03     ` Nicolas Frey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1778140573.9y7e3sqjfm.astroid@yuna.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=n.frey@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal