[pbs-devel] [PATCH proxmox-backup 1/3] file-restore-daemon: disk: ignore "invalid fs" error

[pbs-devel] [PATCH proxmox-backup 1/3] file-restore-daemon: disk: ignore "invalid fs" error

[Stefan Reiter]

Mainly just causes log spam, we print a more useful error in the end if
all mounts fail anyway.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox_restore_daemon/disk.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/bin/proxmox_restore_daemon/disk.rs b/src/bin/proxmox_restore_daemon/disk.rs
index b58a84ff..a59d6360 100644
--- a/src/bin/proxmox_restore_daemon/disk.rs
+++ b/src/bin/proxmox_restore_daemon/disk.rs
@@ -157,6 +157,7 @@ impl Filesystems {
                     info!("mounting '{}' succeeded, fstype: '{}'", source, fs);
                     return Ok(());
                 }
+                Err(nix::Error::Sys(nix::errno::Errno::EINVAL)) => {}
                 Err(err) => {
                     warn!("mount error on '{}' ({}) - {}", source, fs, err);
                 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/3] file-restore-daemon: disk: allow arbitrary component count per bucket

[Stefan Reiter]

A bucket might contain multiple (or 0) layers of components in its path
specification, so allow a mapping between bucket type strings and
expected component depth. For partitions, this is 1, as there is only
the partition number layer below the "part" node.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox_restore_daemon/disk.rs | 83 ++++++++++++++++++--------
 1 file changed, 59 insertions(+), 24 deletions(-)

diff --git a/src/bin/proxmox_restore_daemon/disk.rs b/src/bin/proxmox_restore_daemon/disk.rs
index a59d6360..8ad270fa 100644
--- a/src/bin/proxmox_restore_daemon/disk.rs
+++ b/src/bin/proxmox_restore_daemon/disk.rs
@@ -68,12 +68,17 @@ impl Bucket {
     fn filter_mut<'a, A: AsRef<str>, B: AsRef<str>>(
         haystack: &'a mut Vec<Bucket>,
         ty: A,
-        comp: B,
+        comp: &[B],
     ) -> Option<&'a mut Bucket> {
         let ty = ty.as_ref();
-        let comp = comp.as_ref();
         haystack.iter_mut().find(|b| match b {
-            Bucket::Partition(data) => ty == "part" && comp.parse::<i32>().unwrap() == data.number,
+            Bucket::Partition(data) => {
+                if let Some(comp) = comp.get(0) {
+                    ty == "part" && comp.as_ref().parse::<i32>().unwrap() == data.number
+                } else {
+                    false
+                }
+            }
         })
     }
 
@@ -83,10 +88,26 @@ impl Bucket {
         }
     }
 
-    fn component_string(&self) -> String {
-        match self {
-            Bucket::Partition(data) => data.number.to_string(),
+    fn component_string(&self, idx: usize) -> Result<String, Error> {
+        let max_depth = Self::component_depth(self.type_string())?;
+        if idx >= max_depth {
+            bail!(
+                "internal error: component index out of range {}/{} ({})",
+                idx,
+                max_depth,
+                self.type_string()
+            );
         }
+        Ok(match self {
+            Bucket::Partition(data) => data.number.to_string(),
+        })
+    }
+
+    fn component_depth(type_string: &str) -> Result<usize, Error> {
+        Ok(match type_string {
+            "part" => 1,
+            _ => bail!("invalid bucket type for component depth: {}", type_string),
+        })
     }
 
     fn size(&self) -> u64 {
@@ -299,27 +320,41 @@ impl DiskState {
             }
         };
 
-        let component = match cmp.next() {
-            Some(Component::Normal(x)) => x.to_string_lossy(),
-            Some(c) => bail!("invalid bucket component in path: {:?}", c),
-            None => {
-                // list bucket components available
-                let comps = buckets
-                    .iter()
-                    .filter(|b| b.type_string() == bucket_type)
-                    .map(|b| (b.component_string(), b.size()))
-                    .collect();
-                return Ok(ResolveResult::BucketComponents(comps));
-            }
-        };
+        let mut components = Vec::new();
+        let component_count = Bucket::component_depth(&bucket_type)?;
 
-        let mut bucket = match Bucket::filter_mut(buckets, &bucket_type, &component) {
+        while components.len() < component_count {
+            let component = match cmp.next() {
+                Some(Component::Normal(x)) => x.to_string_lossy(),
+                Some(c) => bail!("invalid bucket component in path: {:?}", c),
+                None => {
+                    // list bucket components available at this level
+                    let comps = buckets
+                        .iter()
+                        .filter_map(|b| {
+                            if b.type_string() != bucket_type {
+                                return None;
+                            }
+                            match b.component_string(components.len()) {
+                                Ok(cs) => Some((cs.to_owned(), b.size())),
+                                Err(_) => None,
+                            }
+                        })
+                        .collect();
+                    return Ok(ResolveResult::BucketComponents(comps));
+                }
+            };
+
+            components.push(component);
+        }
+
+        let mut bucket = match Bucket::filter_mut(buckets, &bucket_type, &components) {
             Some(bucket) => bucket,
             None => bail!(
-                "bucket/component path not found: {}/{}/{}",
+                "bucket/component path not found: {}/{}/{:?}",
                 req_fidx,
                 bucket_type,
-                component
+                components
             ),
         };
 
@@ -329,10 +364,10 @@ impl DiskState {
             .ensure_mounted(&mut bucket)
             .map_err(|err| {
                 format_err!(
-                    "mounting '{}/{}/{}' failed: {}",
+                    "mounting '{}/{}/{:?}' failed: {}",
                     req_fidx,
                     bucket_type,
-                    component,
+                    components,
                     err
                 )
             })?;
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 3/3] file-restore-daemon: disk: add RawFs bucket type

[Stefan Reiter]

Used to specify a filesystem placed directly on a disk, without a
partition table inbetween. Detected by simply attempting to mount the
disk itself.

A helper "make_dev_node" is extracted to avoid code duplication.

Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
 src/bin/proxmox_restore_daemon/disk.rs | 68 +++++++++++++++++++-------
 1 file changed, 50 insertions(+), 18 deletions(-)

diff --git a/src/bin/proxmox_restore_daemon/disk.rs b/src/bin/proxmox_restore_daemon/disk.rs
index 8ad270fa..1ff5468f 100644
--- a/src/bin/proxmox_restore_daemon/disk.rs
+++ b/src/bin/proxmox_restore_daemon/disk.rs
@@ -62,6 +62,7 @@ struct PartitionBucketData {
 /// e.g.: "/drive-scsi0/part/0/etc/passwd"
 enum Bucket {
     Partition(PartitionBucketData),
+    RawFs(PartitionBucketData),
 }
 
 impl Bucket {
@@ -79,12 +80,14 @@ impl Bucket {
                     false
                 }
             }
+            Bucket::RawFs(_) => ty == "raw",
         })
     }
 
     fn type_string(&self) -> &'static str {
         match self {
             Bucket::Partition(_) => "part",
+            Bucket::RawFs(_) => "raw",
         }
     }
 
@@ -100,19 +103,21 @@ impl Bucket {
         }
         Ok(match self {
             Bucket::Partition(data) => data.number.to_string(),
+            Bucket::RawFs(_) => "raw".to_owned(),
         })
     }
 
     fn component_depth(type_string: &str) -> Result<usize, Error> {
         Ok(match type_string {
             "part" => 1,
+            "raw" => 0,
             _ => bail!("invalid bucket type for component depth: {}", type_string),
         })
     }
 
     fn size(&self) -> u64 {
         match self {
-            Bucket::Partition(data) => data.size,
+            Bucket::Partition(data) | Bucket::RawFs(data) => data.size,
         }
     }
 }
@@ -145,8 +150,8 @@ impl Filesystems {
 
     fn ensure_mounted(&self, bucket: &mut Bucket) -> Result<PathBuf, Error> {
         match bucket {
-            Bucket::Partition(data) => {
-                // regular data partition à la "/dev/vdxN"
+            Bucket::Partition(data) | Bucket::RawFs(data) => {
+                // regular data partition à la "/dev/vdxN" or FS directly on a disk
                 if let Some(mp) = &data.mountpoint {
                     return Ok(mp.clone());
                 }
@@ -197,6 +202,8 @@ pub struct DiskState {
 impl DiskState {
     /// Scan all disks for supported buckets.
     pub fn scan() -> Result<Self, Error> {
+        let filesystems = Filesystems::scan()?;
+
         // create mapping for virtio drives and .fidx files (via serial description)
         // note: disks::DiskManager relies on udev, which we don't have
         let mut disk_map = HashMap::new();
@@ -223,6 +230,25 @@ impl DiskState {
                 }
             };
 
+            // attempt to mount device directly
+            let dev_node = format!("/dev/{}", name);
+            let size = Self::make_dev_node(&dev_node, &sys_path)?;
+            let mut dfs_bucket = Bucket::RawFs(PartitionBucketData {
+                dev_node: dev_node.clone(),
+                number: 0,
+                mountpoint: None,
+                size,
+            });
+            if let Ok(_) = filesystems.ensure_mounted(&mut dfs_bucket) {
+                // mount succeeded, add bucket and skip any other checks for the disk
+                info!(
+                    "drive '{}' ('{}', '{}') contains fs directly ({}B)",
+                    name, fidx, dev_node, size
+                );
+                disk_map.insert(fidx, vec![dfs_bucket]);
+                continue;
+            }
+
             let mut parts = Vec::new();
             for entry in proxmox_backup::tools::fs::scan_subdir(
                 libc::AT_FDCWD,
@@ -232,32 +258,23 @@ impl DiskState {
             .filter_map(Result::ok)
             {
                 let part_name = unsafe { entry.file_name_utf8_unchecked() };
-                let devnode = format!("/dev/{}", part_name);
+                let dev_node = format!("/dev/{}", part_name);
                 let part_path = format!("/sys/block/{}/{}", name, part_name);
 
                 // create partition device node for further use
-                let dev_num_str = fs::file_read_firstline(&format!("{}/dev", part_path))?;
-                let (major, minor) = dev_num_str.split_at(dev_num_str.find(':').unwrap());
-                Self::mknod_blk(&devnode, major.parse()?, minor[1..].trim_end().parse()?)?;
+                let size = Self::make_dev_node(&dev_node, &part_path)?;
 
                 let number = fs::file_read_firstline(&format!("{}/partition", part_path))?
                     .trim()
                     .parse::<i32>()?;
 
-                // this *always* contains the number of 512-byte sectors, regardless of the true
-                // blocksize of this disk - which should always be 512 here anyway
-                let size = fs::file_read_firstline(&format!("{}/size", part_path))?
-                    .trim()
-                    .parse::<u64>()?
-                    * 512;
-
                 info!(
                     "drive '{}' ('{}'): found partition '{}' ({}, {}B)",
-                    name, fidx, devnode, number, size
+                    name, fidx, dev_node, number, size
                 );
 
                 let bucket = Bucket::Partition(PartitionBucketData {
-                    dev_node: devnode,
+                    dev_node,
                     mountpoint: None,
                     number,
                     size,
@@ -266,11 +283,11 @@ impl DiskState {
                 parts.push(bucket);
             }
 
-            disk_map.insert(fidx.to_owned(), parts);
+            disk_map.insert(fidx, parts);
         }
 
         Ok(Self {
-            filesystems: Filesystems::scan()?,
+            filesystems,
             disk_map,
         })
     }
@@ -381,6 +398,21 @@ impl DiskState {
         Ok(ResolveResult::Path(local_path))
     }
 
+    fn make_dev_node(devnode: &str, sys_path: &str) -> Result<u64, Error> {
+        let dev_num_str = fs::file_read_firstline(&format!("{}/dev", sys_path))?;
+        let (major, minor) = dev_num_str.split_at(dev_num_str.find(':').unwrap());
+        Self::mknod_blk(&devnode, major.parse()?, minor[1..].trim_end().parse()?)?;
+
+        // this *always* contains the number of 512-byte sectors, regardless of the true
+        // blocksize of this disk - which should always be 512 here anyway
+        let size = fs::file_read_firstline(&format!("{}/size", sys_path))?
+            .trim()
+            .parse::<u64>()?
+            * 512;
+
+        Ok(size)
+    }
+
     fn mknod_blk(path: &str, maj: u64, min: u64) -> Result<(), Error> {
         use nix::sys::stat;
         let dev = stat::makedev(maj, min);
-- 
2.20.1

[pbs-devel] applied-series: [PATCH proxmox-backup 1/3] file-restore-daemon: disk: ignore "invalid fs" error

[Thomas Lamprecht]

On 17.05.21 14:31, Stefan Reiter wrote:
> Mainly just causes log spam, we print a more useful error in the end if
> all mounts fail anyway.
> 
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
>  src/bin/proxmox_restore_daemon/disk.rs | 1 +
>  1 file changed, 1 insertion(+)
> 
>

applied series, thanks!