[pbs-devel] [PATCH vma-to-pbs 6/9] refactor error handling

[Filip Schauer]

Signed-off-by: Filip Schauer <f.schauer at proxmox.com>
---
 src/vma.rs     | 14 +++++++-------
 src/vma2pbs.rs | 33 ++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 22 deletions(-)

diff --git a/src/vma.rs b/src/vma.rs
index 447e8db..f7944cc 100644
--- a/src/vma.rs
+++ b/src/vma.rs
@@ -3,7 +3,7 @@ use std::io::Read;
 use std::mem::size_of;
 use std::str;
 
-use anyhow::{anyhow, bail, Result};
+use anyhow::{bail, Context, Result};
 use bincode::Options;
 use serde::{Deserialize, Serialize};
 use serde_big_array::BigArray;
@@ -135,11 +135,11 @@ impl<T: Read> VmaReader<T> {
         let mut vma_header: VmaHeader = bincode_options.deserialize(&buffer)?;
 
         if vma_header.magic != VMA_HEADER_MAGIC {
-            return Err(anyhow!("Invalid magic number"));
+            bail!("Invalid magic number");
         }
 
         if vma_header.version != 1 {
-            return Err(anyhow!("Invalid VMA version {}", vma_header.version));
+            bail!("Invalid VMA version {}", vma_header.version);
         }
 
         buffer.resize(vma_header.header_size as usize, 0);
@@ -150,7 +150,7 @@ impl<T: Read> VmaReader<T> {
         let computed_md5sum: [u8; 16] = md5::compute(&buffer).into();
 
         if vma_header.md5sum != computed_md5sum {
-            return Err(anyhow!("Wrong VMA header checksum"));
+            bail!("Wrong VMA header checksum");
         }
 
         let blob_buffer = &buffer[VMA_HEADER_SIZE_NO_BLOB_BUFFER..vma_header.header_size as usize];
@@ -233,7 +233,7 @@ impl<T: Read> VmaReader<T> {
         let vma_extent_header: VmaExtentHeader = bincode_options.deserialize(&buffer)?;
 
         if vma_extent_header.magic != VMA_EXTENT_HEADER_MAGIC {
-            return Err(anyhow!("Invalid magic number"));
+            bail!("Invalid magic number");
         }
 
         // Fill the MD5 sum field with zeros to compute the MD5 sum
@@ -241,7 +241,7 @@ impl<T: Read> VmaReader<T> {
         let computed_md5sum: [u8; 16] = md5::compute(&buffer).into();
 
         if vma_extent_header.md5sum != computed_md5sum {
-            return Err(anyhow!("Wrong VMA extent header checksum"));
+            bail!("Wrong VMA extent header checksum");
         }
 
         Ok(vma_extent_header)
@@ -303,7 +303,7 @@ impl<T: Read> VmaReader<T> {
                         if ioerr.kind() == std::io::ErrorKind::UnexpectedEof {
                             break; // Break out of the loop since the end of the file was reached.
                         } else {
-                            return Err(anyhow!("Failed to read VMA file: {}", ioerr));
+                            return Err(anyhow::format_err!(e)).context("Failed to read VMA file");
                         }
                     }
                     _ => {
diff --git a/src/vma2pbs.rs b/src/vma2pbs.rs
index 5b9e20f..9483f6e 100644
--- a/src/vma2pbs.rs
+++ b/src/vma2pbs.rs
@@ -39,6 +39,16 @@ struct BlockDeviceInfo {
     pub device_size: u64,
 }
 
+fn handle_pbs_error(pbs_err: *mut c_char, function_name: &str) -> Result<()> {
+    if pbs_err.is_null() {
+        bail!("{function_name} failed without error message");
+    }
+
+    let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
+    let pbs_err_str = pbs_err_cstr.to_string_lossy();
+    bail!("{function_name} failed: {pbs_err_str}");
+}
+
 fn create_pbs_backup_task(args: BackupVmaToPbsArgs) -> Result<*mut ProxmoxBackupHandle> {
     println!("PBS repository: {}", args.pbs_repository);
     println!("PBS fingerprint: {}", args.fingerprint);
@@ -88,8 +98,7 @@ fn create_pbs_backup_task(args: BackupVmaToPbsArgs) -> Result<*mut ProxmoxBackup
     );
 
     if pbs.is_null() {
-        let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-        bail!("proxmox_backup_new_ns failed: {pbs_err_cstr:?}");
+        handle_pbs_error(pbs_err, "proxmox_backup_new_ns")?;
     }
 
     Ok(pbs)
@@ -117,8 +126,7 @@ where
             &mut pbs_err,
         ) < 0
         {
-            let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-            bail!("proxmox_backup_add_config failed: {pbs_err_cstr:?}");
+            handle_pbs_error(pbs_err, "proxmox_backup_add_config")?;
         }
     }
 
@@ -158,8 +166,7 @@ where
         );
 
         if pbs_device_id < 0 {
-            let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-            bail!("proxmox_backup_register_image failed: {pbs_err_cstr:?}");
+            handle_pbs_error(pbs_err, "proxmox_backup_register_image")?;
         }
 
         let block_device_info = BlockDeviceInfo {
@@ -254,11 +261,10 @@ where
             );
 
             if write_data_result < 0 {
-                let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-                bail!("proxmox_backup_write_data failed: {pbs_err_cstr:?}");
+                handle_pbs_error(pbs_err, "proxmox_backup_write_data")?;
             }
 
-            Ok(())
+            Ok::<(), anyhow::Error>(())
         };
 
         let insert_image_chunk = |image_chunks: &mut HashMap<u64, ImageChunk>,
@@ -325,8 +331,7 @@ where
         let pbs_device_id = block_device_info.pbs_device_id;
 
         if proxmox_backup_close_image(pbs, pbs_device_id, &mut pbs_err) < 0 {
-            let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-            bail!("proxmox_backup_close_image failed: {pbs_err_cstr:?}");
+            handle_pbs_error(pbs_err, "proxmox_backup_close_image")?;
         }
     }
 
@@ -353,8 +358,7 @@ pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<()> {
     let connect_result = proxmox_backup_connect(pbs, &mut pbs_err);
 
     if connect_result < 0 {
-        let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-        bail!("proxmox_backup_connect failed: {pbs_err_cstr:?}");
+        handle_pbs_error(pbs_err, "proxmox_backup_connect")?;
     }
 
     println!("Connected to Proxmox Backup Server");
@@ -365,8 +369,7 @@ pub fn backup_vma_to_pbs(args: BackupVmaToPbsArgs) -> Result<()> {
     upload_block_devices(vma_reader, pbs)?;
 
     if proxmox_backup_finish(pbs, &mut pbs_err) < 0 {
-        let pbs_err_cstr = unsafe { CStr::from_ptr(pbs_err) };
-        bail!("proxmox_backup_finish failed: {pbs_err_cstr:?}");
+        handle_pbs_error(pbs_err, "proxmox_backup_finish")?;
     }
 
     let elapsed_ms = SystemTime::now()
-- 
2.39.2