[pbs-devel] [PATCH proxmox-backup 1/2] tape/drive: add 'set_file_offset' to TapeDriver Trait

[pbs-devel] [PATCH proxmox-backup 1/2] tape/drive: add 'set_file_offset' to TapeDriver Trait

[Dominik Csapak]

it seems the LOCATE command behaves slightly different across vendors
e.g. for IBM drives, LOCATE 1 moves to File #2, but
for HP drives, LOCATE 1 move to File #1

since we only want to move forward when correcting, we keep the default
behaviour (for IBM drives), and have an optional offset
that we can set when we detect a discrepancy in target file number and
the actual file number

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/tape/drive/lto/mod.rs      |  4 ++++
 src/tape/drive/lto/sg_tape.rs  | 13 ++++++++++++-
 src/tape/drive/mod.rs          |  4 ++++
 src/tape/drive/virtual_tape.rs | 12 +++++++++++-
 4 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/tape/drive/lto/mod.rs b/src/tape/drive/lto/mod.rs
index 8c319b07..6db5dc1a 100644
--- a/src/tape/drive/lto/mod.rs
+++ b/src/tape/drive/lto/mod.rs
@@ -286,6 +286,10 @@ impl TapeDriver for LtoTapeHandle {
         Ok(())
     }
 
+    fn set_file_offset(&mut self, offset: i64) {
+        self.sg_tape.set_file_offset(offset);
+    }
+
     fn move_to_file(&mut self, file: u64) -> Result<(), Error> {
         self.locate_file(file)
     }
diff --git a/src/tape/drive/lto/sg_tape.rs b/src/tape/drive/lto/sg_tape.rs
index 25c239a2..c9186830 100644
--- a/src/tape/drive/lto/sg_tape.rs
+++ b/src/tape/drive/lto/sg_tape.rs
@@ -122,6 +122,7 @@ pub struct LtoTapeStatus {
 
 pub struct SgTape {
     file: File,
+    file_offset: i64,
     info: InquiryInfo,
     encryption_key_loaded: bool,
 }
@@ -143,6 +144,7 @@ impl SgTape {
 
         Ok(Self {
             file,
+            file_offset: 0,
             info,
             encryption_key_loaded: false,
         })
@@ -295,12 +297,21 @@ impl SgTape {
         Ok(())
     }
 
+    pub fn set_file_offset(&mut self, offset: i64) {
+        self.file_offset = offset;
+    }
+
     pub fn locate_file(&mut self, position: u64) ->  Result<(), Error> {
         if position == 0 {
             return self.rewind();
         }
 
-        let position = position -1;
+        let position = position - 1;
+        let position = if self.file_offset < 0 {
+            position.saturating_sub((-self.file_offset) as u64)
+        } else {
+            position.saturating_add(self.file_offset as u64)
+        };
 
         let mut sg_raw = SgRaw::new(&mut self.file, 16)?;
         sg_raw.set_timeout(Self::SCSI_TAPE_DEFAULT_TIMEOUT);
diff --git a/src/tape/drive/mod.rs b/src/tape/drive/mod.rs
index 7bc02f9e..c853de90 100644
--- a/src/tape/drive/mod.rs
+++ b/src/tape/drive/mod.rs
@@ -80,9 +80,13 @@ pub trait TapeDriver {
     /// Move to last file
     fn move_to_last_file(&mut self) -> Result<(), Error>;
 
+    /// Set an offset for 'move_to_file'
+    fn set_file_offset(&mut self, offset: i64);
+
     /// Move to given file nr
     fn move_to_file(&mut self, file: u64) -> Result<(), Error>;
 
+
     /// Current file number
     fn current_file_number(&mut self) -> Result<u64, Error>;
 
diff --git a/src/tape/drive/virtual_tape.rs b/src/tape/drive/virtual_tape.rs
index 3c5f9502..30841e66 100644
--- a/src/tape/drive/virtual_tape.rs
+++ b/src/tape/drive/virtual_tape.rs
@@ -56,6 +56,7 @@ impl VirtualTapeDrive {
                 _lock: lock,
                 drive_name: self.name.clone(),
                 max_size: self.max_size.unwrap_or(64*1024*1024),
+                file_offset: 0,
                 path: std::path::PathBuf::from(&self.path),
             })
         }).map_err(|err: Error| format_err!("open drive '{}' ({}) failed - {}", self.name, self.path, err))
@@ -82,6 +83,7 @@ pub struct VirtualTapeHandle {
     drive_name: String,
     path: std::path::PathBuf,
     max_size: usize,
+    file_offset: i64,
     _lock: File,
 }
 
@@ -261,6 +263,10 @@ impl TapeDriver for VirtualTapeHandle {
         Ok(())
     }
 
+    fn set_file_offset(&mut self, offset: i64) {
+        self.file_offset = offset;
+    }
+
     fn move_to_file(&mut self, file: u64) -> Result<(), Error> {
         let mut status = self.load_status()?;
         match status.current_tape {
@@ -273,7 +279,11 @@ impl TapeDriver for VirtualTapeHandle {
                     bail!("invalid file nr");
                 }
 
-                *pos = file as usize;
+                *pos = if self.file_offset < 0 {
+                    file.saturating_sub((-self.file_offset) as u64) as usize
+                } else {
+                    file.saturating_add(self.file_offset as u64) as usize
+                };
 
                 self.store_status(&status)
                     .map_err(|err| io::Error::new(io::ErrorKind::Other, err.to_string()))?;
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] api2/tape/restore: use file offset to compensate for some tape drives

[Dominik Csapak]

by calculating the offset after moving to a file and add the offset
to the drive

assuming the offset is static (e.g. always 1), we only have to do
that for the first file, all others should be correct then

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/api2/tape/restore.rs | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/src/api2/tape/restore.rs b/src/api2/tape/restore.rs
index 14e20ee4..05a2f330 100644
--- a/src/api2/tape/restore.rs
+++ b/src/api2/tape/restore.rs
@@ -685,6 +685,32 @@ fn get_media_set_catalog(
     Ok(catalog)
 }
 
+fn compensate_locate_offset(
+    worker: &WorkerTask,
+    target_file: u64,
+    real_file: u64,
+    drive: &mut Box<dyn TapeDriver>,
+) -> Result<(), Error> {
+    task_log!(worker, "landed on wrong file, adding offset and try again");
+    let offset: i64 =
+        i64::try_from((target_file as i128) - (real_file as i128)).map_err(|err| {
+            format_err!(
+                "offset between {} and {} invalid: {}",
+                target_file,
+                real_file,
+                err
+            )
+        })?;
+    drive.set_file_offset(offset);
+    drive.move_to_file(target_file)?;
+    let current_file = drive.current_file_number()?;
+    if current_file != target_file {
+        bail!("Compensating locate_file with offset did not work, aborting...");
+    }
+
+    Ok(())
+}
+
 fn restore_snapshots_to_tmpdir(
     worker: Arc<WorkerTask>,
     path: &PathBuf,
@@ -727,6 +753,10 @@ fn restore_snapshots_to_tmpdir(
             drive.move_to_file(*file_num)?;
             let current_file_number = drive.current_file_number()?;
             task_log!(worker, "now at file {}", current_file_number);
+            if current_file_number != *file_num {
+                compensate_locate_offset(&worker, *file_num, current_file_number, &mut drive)?;
+                task_log!(worker, "now at file {}", current_file_number);
+            }
         }
         let mut reader = drive.read_next_file()?;
 
@@ -806,6 +836,10 @@ fn restore_file_chunk_map(
             drive.move_to_file(*nr)?;
             let current_file_number = drive.current_file_number()?;
             task_log!(worker, "now at file {}", current_file_number);
+            if current_file_number != *nr {
+                compensate_locate_offset(&worker, *nr, current_file_number, drive)?;
+                task_log!(worker, "now at file {}", current_file_number);
+            }
         }
         let mut reader = drive.read_next_file()?;
         let header: MediaContentHeader = unsafe { reader.read_le_value()? };
-- 
2.20.1

Re: [pbs-devel] [PATCH proxmox-backup 1/2] tape/drive: add 'set_file_offset' to TapeDriver Trait

[Dietmar Maurer]

> +    fn set_file_offset(&mut self, offset: i64) {
> +        self.sg_tape.set_file_offset(offset);
> +    }
> +

I would prefer the name "set_locate_offset".

IMHO, file_offset is something different...