Re: [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser

[Nicolas Frey <n.frey@proxmox.com>]

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 <n.frey@proxmox.com>
>> ---
>>  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
>>
>>
>>
>>