[pbs-devel] [PATCH proxmox-backup 0/5] fix zfs/disk listings with special pool names

[pbs-devel] [PATCH proxmox-backup 0/5] fix zfs/disk listings with special pool names

[Dominik Csapak]

this series adds functionality to allow zpools that have
special characters in the name, such as ':','.','_', etc.
(spaces are still disallowed)

also fixed disk list when zfsutils-linux is not installed
(the user still gets an error on the ZFS panel)

also add some tests

Dominik Csapak (5):
  api: add ZPOOL_NAME_SCHEMA and regex
  zpool_list: add tests for special pool names
  disks/zpool_list: allow some more characters for pool list
  disks/zpool_status: add test for pool with special character
  get_disks: don't fail on zfs_devices

 src/api2/node/disks/zfs.rs      |  5 ++++-
 src/api2/types.rs               |  2 ++
 src/tools/disks.rs              |  5 ++++-
 src/tools/disks/zpool_list.rs   | 32 +++++++++++++++++++++++++++---
 src/tools/disks/zpool_status.rs | 35 +++++++++++++++++++++++++++++++++
 5 files changed, 74 insertions(+), 5 deletions(-)

-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 1/5] api: add ZPOOL_NAME_SCHEMA and regex

[Dominik Csapak]

poolnames can containe spaces and some other special characters

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/node/disks/zfs.rs | 5 ++++-
 src/api2/types.rs          | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/api2/node/disks/zfs.rs b/src/api2/node/disks/zfs.rs
index 3184a76..be2a163 100644
--- a/src/api2/node/disks/zfs.rs
+++ b/src/api2/node/disks/zfs.rs
@@ -41,6 +41,9 @@ pub const ZFS_ASHIFT_SCHEMA: Schema = IntegerSchema::new(
     .default(12)
     .schema();
 
+pub const ZPOOL_NAME_SCHEMA: Schema =StringSchema::new("ZFS Pool Name")
+    .format(&ApiStringFormat::Pattern(&ZPOOL_NAME_REGEX))
+    .schema();
 
 #[api(
     default: "On",
@@ -157,7 +160,7 @@ pub fn list_zpools() -> Result<Vec<ZpoolListItem>, Error> {
                 schema: NODE_SCHEMA,
             },
             name: {
-                schema: DATASTORE_SCHEMA,
+                schema: ZPOOL_NAME_SCHEMA,
             },
         },
     },
diff --git a/src/api2/types.rs b/src/api2/types.rs
index 7b90270..0d0fab3 100644
--- a/src/api2/types.rs
+++ b/src/api2/types.rs
@@ -78,6 +78,8 @@ const_regex!{
     pub ACL_PATH_REGEX = concat!(r"^(?:/|", r"(?:/", PROXMOX_SAFE_ID_REGEX_STR!(), ")+", r")$");
 
     pub BLOCKDEVICE_NAME_REGEX = r"^(:?(:?h|s|x?v)d[a-z]+)|(:?nvme\d+n\d+)$";
+
+    pub ZPOOL_NAME_REGEX = r"^[a-zA-Z][a-z0-9A-Z\-_.:]+$";
 }
 
 pub const SYSTEMD_DATETIME_FORMAT: ApiStringFormat =
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/5] zpool_list: add tests for special pool names

[Dominik Csapak]

those names are allowed for zpools

these will fail for now, but it will be fixed in the next commit

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/disks/zpool_list.rs | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/src/tools/disks/zpool_list.rs b/src/tools/disks/zpool_list.rs
index 3c65280..343ac0d 100644
--- a/src/tools/disks/zpool_list.rs
+++ b/src/tools/disks/zpool_list.rs
@@ -221,7 +221,7 @@ logs
     assert_eq!(data, expect);
 
     let output = "\
-btest	427349245952	761856	427348484096	-	-	0	0	1.00	ONLINE	-
+b-test	427349245952	761856	427348484096	-	-	0	0	1.00	ONLINE	-
 	mirror	213674622976	438272	213674184704	-	-	0	0	-	ONLINE
 	/dev/sda1	-	-	-	-	-	-	-	-	ONLINE
 	/dev/sda2	-	-	-	-	-	-	-	-	ONLINE
@@ -235,7 +235,7 @@ logs               -      -      -        -         -      -      -      -  -
     let data = parse_zpool_list(&output)?;
     let expect = vec![
         ZFSPoolInfo {
-            name: String::from("btest"),
+            name: String::from("b-test"),
             health: String::from("ONLINE"),
             usage: Some(ZFSPoolUsage {
                 size: 427349245952,
@@ -261,5 +261,31 @@ logs               -      -      -        -         -      -      -      -  -
 
     assert_eq!(data, expect);
 
+    let output = "\
+b.test	427349245952	761856	427348484096	-	-	0	0	1.00	ONLINE	-
+	mirror	213674622976	438272	213674184704	-	-	0	0	-	ONLINE
+	/dev/sda1	-	-	-	-	-	-	-	-	ONLINE
+";
+
+    let data = parse_zpool_list(&output)?;
+    let expect = vec![
+        ZFSPoolInfo {
+            name: String::from("b.test"),
+            health: String::from("ONLINE"),
+            usage: Some(ZFSPoolUsage {
+                size: 427349245952,
+                alloc: 761856,
+                free: 427348484096,
+                dedup: 1.0,
+                frag: 0,
+            }),
+            devices: vec![
+                String::from("/dev/sda1"),
+            ]
+        },
+    ];
+
+    assert_eq!(data, expect);
+
     Ok(())
 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 3/5] disks/zpool_list: allow some more characters for pool list

[Dominik Csapak]

not exhaustive of what zfs allows (space is missing), but this
can be done easily without problems

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/disks/zpool_list.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/tools/disks/zpool_list.rs b/src/tools/disks/zpool_list.rs
index 343ac0d..a071037 100644
--- a/src/tools/disks/zpool_list.rs
+++ b/src/tools/disks/zpool_list.rs
@@ -64,7 +64,7 @@ fn parse_zpool_list_header(i: &str) -> IResult<&str, ZFSPoolInfo> {
     let (i, (text, size, alloc, free, _, _,
              frag, _, dedup, health,
              _altroot, _eol)) = tuple((
-        take_while1(|c| char::is_alphanumeric(c)), // name
+        take_while1(|c| char::is_alphanumeric(c) || c == '-' || c == ':' || c == '_' || c == '.'), // name
         preceded(multispace1, parse_optional_u64), // size
         preceded(multispace1, parse_optional_u64), // allocated
         preceded(multispace1, parse_optional_u64), // free
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 4/5] disks/zpool_status: add test for pool with special character

[Dominik Csapak]

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/disks/zpool_status.rs | 35 +++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/src/tools/disks/zpool_status.rs b/src/tools/disks/zpool_status.rs
index f333d53..3b0edc8 100644
--- a/src/tools/disks/zpool_status.rs
+++ b/src/tools/disks/zpool_status.rs
@@ -430,3 +430,38 @@ errors: No known data errors
 
     Ok(())
 }
+
+#[test]
+fn test_zpool_status_parser3() -> Result<(), Error> {
+
+    let output = r###"  pool: bt-est
+ state: ONLINE
+  scan: none requested
+config:
+
+	NAME           STATE     READ WRITE CKSUM
+	bt-est          ONLINE       0     0     0
+	  mirror-0     ONLINE       0     0     0
+	    /dev/sda1  ONLINE       0     0     0
+	    /dev/sda2  ONLINE       0     0     0
+	  mirror-1     ONLINE       0     0     0
+	    /dev/sda3  ONLINE       0     0     0
+	    /dev/sda4  ONLINE       0     0     0
+	logs
+	  /dev/sda5    ONLINE       0     0     0
+
+errors: No known data errors
+"###;
+
+    let key_value_list = parse_zpool_status(&output)?;
+    for (k, v) in key_value_list {
+        println!("{} => {}", k,v);
+        if k == "config" {
+            let vdev_list = parse_zpool_status_config_tree(&v)?;
+            let _tree = vdev_list_to_tree(&vdev_list);
+            //println!("TREE1 {}", serde_json::to_string_pretty(&tree)?);
+        }
+    }
+
+    Ok(())
+}
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 5/5] get_disks: don't fail on zfs_devices

[Dominik Csapak]

zfs does not have to be installed, so simply log an error and
continue, users still get an error when clicking directly on
ZFS

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tools/disks.rs | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/tools/disks.rs b/src/tools/disks.rs
index a4c3e29..73f8370 100644
--- a/src/tools/disks.rs
+++ b/src/tools/disks.rs
@@ -743,7 +743,10 @@ pub fn get_disks(
 
     let partition_type_map = get_partition_type_info()?;
 
-    let zfs_devices = zfs_devices(&partition_type_map, None)?;
+    let zfs_devices = zfs_devices(&partition_type_map, None).or_else(|err| -> Result<HashSet<u64>, Error> {
+        eprintln!("error getting zfs devices: {}", err);
+        Ok(HashSet::new())
+    })?;
 
     let lvm_devices = get_lvm_devices(&partition_type_map)?;
 
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 0/5] fix zfs/disk listings with special pool names

[Dietmar Maurer]

applied 1 to 4

> On 07/08/2020 2:59 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> this series adds functionality to allow zpools that have
> special characters in the name, such as ':','.','_', etc.
> (spaces are still disallowed)
> 
> also fixed disk list when zfsutils-linux is not installed
> (the user still gets an error on the ZFS panel)
> 
> also add some tests
> 
> Dominik Csapak (5):
>   api: add ZPOOL_NAME_SCHEMA and regex
>   zpool_list: add tests for special pool names
>   disks/zpool_list: allow some more characters for pool list
>   disks/zpool_status: add test for pool with special character
>   get_disks: don't fail on zfs_devices
> 
>  src/api2/node/disks/zfs.rs      |  5 ++++-
>  src/api2/types.rs               |  2 ++
>  src/tools/disks.rs              |  5 ++++-
>  src/tools/disks/zpool_list.rs   | 32 +++++++++++++++++++++++++++---
>  src/tools/disks/zpool_status.rs | 35 +++++++++++++++++++++++++++++++++
>  5 files changed, 74 insertions(+), 5 deletions(-)
> 
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox-backup 5/5] get_disks: don't fail on zfs_devices

[Dietmar Maurer]

applied for now ...

> On 07/08/2020 2:59 PM Dominik Csapak <d.csapak@proxmox.com> wrote:
> 
>  
> zfs does not have to be installed, so simply log an error and
> continue, users still get an error when clicking directly on
> ZFS
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  src/tools/disks.rs | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/tools/disks.rs b/src/tools/disks.rs
> index a4c3e29..73f8370 100644
> --- a/src/tools/disks.rs
> +++ b/src/tools/disks.rs
> @@ -743,7 +743,10 @@ pub fn get_disks(
>  
>      let partition_type_map = get_partition_type_info()?;
>  
> -    let zfs_devices = zfs_devices(&partition_type_map, None)?;
> +    let zfs_devices = zfs_devices(&partition_type_map, None).or_else(|err| -> Result<HashSet<u64>, Error> {
> +        eprintln!("error getting zfs devices: {}", err);
> +        Ok(HashSet::new())
> +    })?;
>  
>      let lvm_devices = get_lvm_devices(&partition_type_map)?;
>  
> -- 
> 2.20.1
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel