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

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

[Nicolas Frey]

As reported in Enterprise Support, when a spare is "INUSE", it has a
`msg` right after the STATUS, which the parser previously did not
account for.

Changes since v1:
* introduce new struct `VDevStats` to reduce boilerplate

proxmox-backup:

Nicolas Frey (2):
  zfs: status: add `VDevStats` struct instead of 3 optional u64s
  fix #7541: zfs status: account for msg directly after status in vdev
    parser

 src/tools/disks/zpool_status.rs | 51 +++++++++++++++++----------------
 1 file changed, 26 insertions(+), 25 deletions(-)


Summary over all repositories:
  1 files changed, 26 insertions(+), 25 deletions(-)

--
Generated by git-murpp 0.8.1

[PATCH proxmox-backup v2 1/2] zfs: status: add `VDevStats` struct instead of 3 optional u64s

[Nicolas Frey]

in order to reduce a bit of boilerplate. Keeps (de)serialization the
same by flattening `stats`. No functional change intended

Suggested-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Nicolas Frey <n.frey@proxmox.com>
---
New in v2
 src/tools/disks/zpool_status.rs | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/tools/disks/zpool_status.rs b/src/tools/disks/zpool_status.rs
index 05ef5d80..9427f6d4 100644
--- a/src/tools/disks/zpool_status.rs
+++ b/src/tools/disks/zpool_status.rs
@@ -24,15 +24,19 @@ pub struct ZFSPoolVDevState {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub state: Option<String>,
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub read: Option<u64>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub write: Option<u64>,
-    #[serde(skip_serializing_if = "Option::is_none")]
-    pub cksum: Option<u64>,
+    #[serde(flatten)]
+    pub stats: Option<VDevStats>,
     #[serde(skip_serializing_if = "Option::is_none")]
     pub msg: Option<String>,
 }

+#[derive(Debug, Serialize, Deserialize)]
+pub struct VDevStats {
+    read: u64,
+    write: u64,
+    cksum: u64,
+}
+
 fn expand_tab_length(input: &str) -> usize {
     input.chars().map(|c| if c == '\t' { 8 } else { 1 }).sum()
 }
@@ -57,9 +61,7 @@ fn parse_zpool_status_vdev(i: &str) -> IResult<&str, ZFSPoolVDevState> {
             name: vdev_name.to_string(),
             lvl: indent_level,
             state: None,
-            read: None,
-            write: None,
-            cksum: None,
+            stats: None,
             msg: None,
         };
         return Ok((n, vdev));
@@ -72,9 +74,7 @@ fn parse_zpool_status_vdev(i: &str) -> IResult<&str, ZFSPoolVDevState> {
             name: vdev_name.to_string(),
             lvl: indent_level,
             state: Some(state.to_string()),
-            read: None,
-            write: None,
-            cksum: None,
+            stats: None,
             msg: None,
         };
         return Ok((n, vdev));
@@ -90,9 +90,7 @@ fn parse_zpool_status_vdev(i: &str) -> IResult<&str, ZFSPoolVDevState> {
         name: vdev_name.to_string(),
         lvl: indent_level,
         state: Some(state.to_string()),
-        read: Some(read),
-        write: Some(write),
-        cksum: Some(cksum),
+        stats: Some(VDevStats { read, write, cksum }),
         msg: msg.map(String::from),
     };

@@ -283,9 +281,7 @@ fn test_vdev_list_to_tree() {
         name: String::new(),
         lvl: 0,
         state: None,
-        read: None,
-        write: None,
-        cksum: None,
+        stats: None,
         msg: None,
     };

--
2.47.3

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

[Nicolas Frey]

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