all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser
@ 2026-04-30 10:05 Nicolas Frey
  2026-05-04 13:29 ` Fabian Grünbichler
  2026-05-05 14:17 ` superseded: " Nicolas Frey
  0 siblings, 2 replies; 4+ messages in thread
From: Nicolas Frey @ 2026-04-30 10:05 UTC (permalink / raw)
  To: pbs-devel

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));
     }

-    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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser
  2026-04-30 10:05 [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser Nicolas Frey
@ 2026-05-04 13:29 ` Fabian Grünbichler
  2026-05-05 12:48   ` Nicolas Frey
  2026-05-05 14:17 ` superseded: " Nicolas Frey
  1 sibling, 1 reply; 4+ messages in thread
From: Fabian Grünbichler @ 2026-05-04 13:29 UTC (permalink / raw)
  To: Nicolas Frey, pbs-devel

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..

>      }
> 
> -    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
> 
> 
> 
> 




^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser
  2026-05-04 13:29 ` Fabian Grünbichler
@ 2026-05-05 12:48   ` Nicolas Frey
  0 siblings, 0 replies; 4+ messages in thread
From: Nicolas Frey @ 2026-05-05 12:48 UTC (permalink / raw)
  To: Fabian Grünbichler, pbs-devel

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
>>
>>
>>
>>





^ permalink raw reply	[flat|nested] 4+ messages in thread

* superseded: [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser
  2026-04-30 10:05 [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser Nicolas Frey
  2026-05-04 13:29 ` Fabian Grünbichler
@ 2026-05-05 14:17 ` Nicolas Frey
  1 sibling, 0 replies; 4+ messages in thread
From: Nicolas Frey @ 2026-05-05 14:17 UTC (permalink / raw)
  To: pbs-devel

Superseded-by: https://lore.proxmox.com/pbs-devel/20260505141606.438908-1-n.frey@proxmox.com/T/#t




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-05 14:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 10:05 [PATCH proxmox-backup 1/1] fix #7541: zfs status: account for msg directly after status in vdev parser Nicolas Frey
2026-05-04 13:29 ` Fabian Grünbichler
2026-05-05 12:48   ` Nicolas Frey
2026-05-05 14:17 ` superseded: " Nicolas Frey

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal