* [pbs-devel] [PATCH proxmox-backup 0/2] fix 2885 and a division by zero @ 2020-07-23 16:16 Stoiko Ivanov 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target Stoiko Ivanov 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero Stoiko Ivanov 0 siblings, 2 replies; 6+ messages in thread From: Stoiko Ivanov @ 2020-07-23 16:16 UTC (permalink / raw) To: pbs-devel This patchset fixes #2885 (clobbering of the didx file if the same target is used multiple times). The second patch fixes a division by zero panic in case the backup was too fast :) (as_secs() returns 0). Tested on my machine (where I ran into the division by zero issue) Stoiko Ivanov (2): fix 2885: bail on duplicate backup target fix division by zero src/bin/proxmox-backup-client.rs | 6 ++++++ src/client/backup_writer.rs | 11 ++++++++--- 2 files changed, 14 insertions(+), 3 deletions(-) -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target 2020-07-23 16:16 [pbs-devel] [PATCH proxmox-backup 0/2] fix 2885 and a division by zero Stoiko Ivanov @ 2020-07-23 16:16 ` Stoiko Ivanov 2020-07-24 9:12 ` [pbs-devel] applied: " Thomas Lamprecht 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero Stoiko Ivanov 1 sibling, 1 reply; 6+ messages in thread From: Stoiko Ivanov @ 2020-07-23 16:16 UTC (permalink / raw) To: pbs-devel Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> --- src/bin/proxmox-backup-client.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs index 32483c00..fc2d77ce 100644 --- a/src/bin/proxmox-backup-client.rs +++ b/src/bin/proxmox-backup-client.rs @@ -935,12 +935,18 @@ async fn create_backup( } let mut upload_list = vec![]; + let mut target_set = HashSet::new(); for backupspec in backupspec_list { let spec = parse_backup_specification(backupspec.as_str().unwrap())?; let filename = &spec.config_string; let target = &spec.archive_name; + if target_set.contains(target) { + bail!("got target twice: '{}'", target); + } + target_set.insert(target.to_string()); + use std::os::unix::fs::FileTypeExt; let metadata = std::fs::metadata(filename) -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] applied: [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target Stoiko Ivanov @ 2020-07-24 9:12 ` Thomas Lamprecht 0 siblings, 0 replies; 6+ messages in thread From: Thomas Lamprecht @ 2020-07-24 9:12 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Stoiko Ivanov Am 7/23/20 um 6:16 PM schrieb Stoiko Ivanov: > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> > --- > src/bin/proxmox-backup-client.rs | 6 ++++++ > 1 file changed, 6 insertions(+) > applied, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero 2020-07-23 16:16 [pbs-devel] [PATCH proxmox-backup 0/2] fix 2885 and a division by zero Stoiko Ivanov 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target Stoiko Ivanov @ 2020-07-23 16:16 ` Stoiko Ivanov 2020-07-24 5:52 ` Fabian Grünbichler 2020-07-24 7:15 ` Dietmar Maurer 1 sibling, 2 replies; 6+ messages in thread From: Stoiko Ivanov @ 2020-07-23 16:16 UTC (permalink / raw) To: pbs-devel in case the backup duration gets rounded to 0 seconds Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> --- src/client/backup_writer.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs index 7e5adb3c..c2db2e76 100644 --- a/src/client/backup_writer.rs +++ b/src/client/backup_writer.rs @@ -264,9 +264,14 @@ impl BackupWriter { crate::tools::format::strip_server_file_expenstion(archive_name.clone()) }; if archive_name != CATALOG_NAME { - let speed: HumanByte = (uploaded / (duration.as_secs() as usize)).into(); - let uploaded: HumanByte = uploaded.into(); - println!("{}: had to upload {} from {} in {}s, avgerage speed {}/s).", archive, uploaded, vsize_h, duration.as_secs(), speed); + let hb_uploaded: HumanByte = uploaded.into(); + match duration.as_secs() { + 0 => println!("{}: had to upload {} from {} in {}s", archive, hb_uploaded, vsize_h, duration.as_secs()), + dur => { + let speed: HumanByte = (uploaded / (dur as usize)).into(); + println!("{}: had to upload {} from {} in {}s, (average speed {}/s).", archive, hb_uploaded, vsize_h, duration.as_secs(), speed); + } + } } else { println!("Uploaded backup catalog ({})", vsize_h); } -- 2.20.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero Stoiko Ivanov @ 2020-07-24 5:52 ` Fabian Grünbichler 2020-07-24 7:15 ` Dietmar Maurer 1 sibling, 0 replies; 6+ messages in thread From: Fabian Grünbichler @ 2020-07-24 5:52 UTC (permalink / raw) To: Proxmox Backup Server development discussion On July 23, 2020 6:16 pm, Stoiko Ivanov wrote: > in case the backup duration gets rounded to 0 seconds is this the only place where we calculate speed/throughput like this? if not, would it make sense to have a helper that does it in a uniform fashion? > > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> > --- > src/client/backup_writer.rs | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs > index 7e5adb3c..c2db2e76 100644 > --- a/src/client/backup_writer.rs > +++ b/src/client/backup_writer.rs > @@ -264,9 +264,14 @@ impl BackupWriter { > crate::tools::format::strip_server_file_expenstion(archive_name.clone()) > }; > if archive_name != CATALOG_NAME { > - let speed: HumanByte = (uploaded / (duration.as_secs() as usize)).into(); > - let uploaded: HumanByte = uploaded.into(); > - println!("{}: had to upload {} from {} in {}s, avgerage speed {}/s).", archive, uploaded, vsize_h, duration.as_secs(), speed); > + let hb_uploaded: HumanByte = uploaded.into(); > + match duration.as_secs() { > + 0 => println!("{}: had to upload {} from {} in {}s", archive, hb_uploaded, vsize_h, duration.as_secs()), s/from/of or s/from/out of also in this case, it would make sense to print 'in <1s' instead of 0s? or do we want to calculate speed based on the actual duration in that case? or in any case? if we switch to as_millis() it's already rather unlikely to return 0 (although it would still need to be handled for very very fast systems or broken time keeping) > + dur => { > + let speed: HumanByte = (uploaded / (dur as usize)).into(); > + println!("{}: had to upload {} from {} in {}s, (average speed {}/s).", archive, hb_uploaded, vsize_h, duration.as_secs(), speed); 'from' is wrong here as well > + } > + } > } else { > println!("Uploaded backup catalog ({})", vsize_h); > } > -- > 2.20.1 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel > > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero Stoiko Ivanov 2020-07-24 5:52 ` Fabian Grünbichler @ 2020-07-24 7:15 ` Dietmar Maurer 1 sibling, 0 replies; 6+ messages in thread From: Dietmar Maurer @ 2020-07-24 7:15 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Stoiko Ivanov Why do we use duration.as_secs() here? We can do better by using floating point values using duration.as_secs_f64() to calculate the speed more exactly. > On 07/23/2020 6:16 PM Stoiko Ivanov <s.ivanov@proxmox.com> wrote: > > > in case the backup duration gets rounded to 0 seconds > > Signed-off-by: Stoiko Ivanov <s.ivanov@proxmox.com> > --- > src/client/backup_writer.rs | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/src/client/backup_writer.rs b/src/client/backup_writer.rs > index 7e5adb3c..c2db2e76 100644 > --- a/src/client/backup_writer.rs > +++ b/src/client/backup_writer.rs > @@ -264,9 +264,14 @@ impl BackupWriter { > crate::tools::format::strip_server_file_expenstion(archive_name.clone()) > }; > if archive_name != CATALOG_NAME { > - let speed: HumanByte = (uploaded / (duration.as_secs() as usize)).into(); > - let uploaded: HumanByte = uploaded.into(); > - println!("{}: had to upload {} from {} in {}s, avgerage speed {}/s).", archive, uploaded, vsize_h, duration.as_secs(), speed); > + let hb_uploaded: HumanByte = uploaded.into(); > + match duration.as_secs() { > + 0 => println!("{}: had to upload {} from {} in {}s", archive, hb_uploaded, vsize_h, duration.as_secs()), > + dur => { > + let speed: HumanByte = (uploaded / (dur as usize)).into(); > + println!("{}: had to upload {} from {} in {}s, (average speed {}/s).", archive, hb_uploaded, vsize_h, duration.as_secs(), speed); > + } > + } > } else { > println!("Uploaded backup catalog ({})", vsize_h); > } > -- > 2.20.1 > > > > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-24 9:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-23 16:16 [pbs-devel] [PATCH proxmox-backup 0/2] fix 2885 and a division by zero Stoiko Ivanov 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 1/2] fix 2885: bail on duplicate backup target Stoiko Ivanov 2020-07-24 9:12 ` [pbs-devel] applied: " Thomas Lamprecht 2020-07-23 16:16 ` [pbs-devel] [PATCH proxmox-backup 2/2] fix division by zero Stoiko Ivanov 2020-07-24 5:52 ` Fabian Grünbichler 2020-07-24 7:15 ` Dietmar Maurer
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox