Re: [pbs-devel] [PATCH proxmox-backup 2/7] backup: only allow finished backups as base snapshot

[Stefan Reiter <s.reiter@proxmox.com>]

On 8/6/20 6:45 AM, Dietmar Maurer wrote:
> This break the assumption that the current backup only reference junks from
> the previous one.
> 

Why do we need that assumption?

> Instead, we can simply fail if the previous backup is not complete yet?
> 

"Not complete *yet*" can't happen, since only one backup can run at a 
time. So if this case occurs, it's because the server has broken backups.

>> On 08/04/2020 12:42 PM Stefan Reiter <s.reiter@proxmox.com> wrote:
>>
>>   
>> If the datastore holds broken backups for some reason, do not attempt to
>> base following snapshots on those. This would lead to an error on
>> /previous, leaving the client no choice but to upload all chunks, even
>> though there might be potential for incremental savings.
>>
>> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
>> ---
>>   src/api2/backup.rs        | 2 +-
>>   src/backup/backup_info.rs | 8 ++++++--
>>   2 files changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/api2/backup.rs b/src/api2/backup.rs
>> index ad13faa5..aafea8fa 100644
>> --- a/src/api2/backup.rs
>> +++ b/src/api2/backup.rs
>> @@ -97,7 +97,7 @@ async move {
>>           bail!("backup owner check failed ({} != {})", username, owner);
>>       }
>>   
>> -    let last_backup = BackupInfo::last_backup(&datastore.base_path(), &backup_group).unwrap_or(None);
>> +    let last_backup = BackupInfo::last_backup(&datastore.base_path(), &backup_group, true).unwrap_or(None);
>>       let backup_dir = BackupDir::new_with_group(backup_group.clone(), backup_time);
>>   
>>       if let Some(last) = &last_backup {
>> diff --git a/src/backup/backup_info.rs b/src/backup/backup_info.rs
>> index 37dc7aa1..ea917d3c 100644
>> --- a/src/backup/backup_info.rs
>> +++ b/src/backup/backup_info.rs
>> @@ -313,9 +313,13 @@ impl BackupInfo {
>>       }
>>   
>>       /// Finds the latest backup inside a backup group
>> -    pub fn last_backup(base_path: &Path, group: &BackupGroup) -> Result<Option<BackupInfo>, Error> {
>> +    pub fn last_backup(base_path: &Path, group: &BackupGroup, only_finished: bool)
>> +        -> Result<Option<BackupInfo>, Error>
>> +    {
>>           let backups = group.list_backups(base_path)?;
>> -        Ok(backups.into_iter().max_by_key(|item| item.backup_dir.backup_time()))
>> +        Ok(backups.into_iter()
>> +            .filter(|item| !only_finished || item.is_finished())
>> +            .max_by_key(|item| item.backup_dir.backup_time()))
>>       }
>>   
>>       pub fn sort_list(list: &mut Vec<BackupInfo>, ascendending: bool) {
>> -- 
>> 2.20.1
>>
>>
>>
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel