all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Maximiliano Sandoval <m.sandoval@proxmox.com>
To: Proxmox Backup Server development discussion
	<pbs-devel@lists.proxmox.com>
Subject: Re: [pbs-devel] [PATCH backup] verify: print number of chunks failing verification
Date: Tue, 15 Apr 2025 13:13:45 +0200	[thread overview]
Message-ID: <s8oa58hekuq.fsf@proxmox.com> (raw)
In-Reply-To: <1744712693.r64hp93gpu.astroid@yuna.none>


Fabian Grünbichler <f.gruenbichler@proxmox.com> writes:

> On April 14, 2025 10:28 am, Maximiliano Sandoval wrote:
>> We also re-use the error_count variable defined right above.
>> 
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>>  src/backup/verify.rs | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/backup/verify.rs b/src/backup/verify.rs
>> index 3d2cba8ac..1344b3b22 100644
>> --- a/src/backup/verify.rs
>> +++ b/src/backup/verify.rs
>> @@ -231,8 +231,8 @@ fn verify_index_chunks(
>>          "  verified {read_bytes_mib:.2}/{decoded_bytes_mib:.2} MiB in {elapsed:.2} seconds, speed {read_speed:.2}/{decode_speed:.2} MiB/s ({error_count} errors)"
>>      );
>>  
>> -    if errors.load(Ordering::SeqCst) > 0 {
>> -        bail!("chunks could not be verified");
>> +    if error_count > 0 {
>> +        bail!("{error_count} chunks could not be verified");
>
> but the error count is already included in the log message right before
> it?

You are right, would it make more sense to print it here? The "number of
errors" is a bit unclear and that line is already a bit too long.

It is not clear that "errors" in this context means how many chunks were
not verified.

> if we do this, then the message before it should be changed to just
> indicate success/failure, logging the error count twice doesn't make
> much sense..
>
>>      }
>>  
>>      Ok(())
>> -- 
>> 2.39.5
>> 
>> 
>> 
>> _______________________________________________
>> pbs-devel mailing list
>> pbs-devel@lists.proxmox.com
>> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
>> 
>> 
>> 
>
>
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

  reply	other threads:[~2025-04-15 11:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-14  8:28 Maximiliano Sandoval
2025-04-15 10:25 ` Fabian Grünbichler
2025-04-15 11:13   ` Maximiliano Sandoval [this message]
2025-04-15 11:32 ` Wolfgang Bumiller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s8oa58hekuq.fsf@proxmox.com \
    --to=m.sandoval@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal