blkdiscard progress breakes test

blkdiscard progress breakes test

[Ruediger Meier]

Hi,

Commit c472a7e3 introduced progress messages. This randomly
breaks the test-suite like this:


   blkdiscard: offsets                        ... FAILED (blkdiscard/offsets)
....
--- /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/expected/blkdiscard/offsets   2014-10-28 06:53:35.483698330 +0000
+++ /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/output/blkdiscard/offsets     2015-04-22 15:41:45.748155291 +0000
@@ -21,7 +21,8 @@
 blkdiscard: offset 1 is not aligned to sector size 512
 blkdiscard: offset 1 is not aligned to sector size 512
 blkdiscard: offset 511 is not aligned to sector size 512
-Discarded 1536 bytes from the offset 512
+Discarded 1024 bytes from the offset 512
+Discarded 512 bytes from the offset 1536
 Discarded 1024 bytes from the offset 1024
 testing misaligned steps full device
 blkdiscard: length 1 is not aligned to sector size 512


It's because we ignore timeval's tv_usec here:
  if (last.tv_sec < now.tv_sec) {
      print_stats(path, stats);

We could make it better if we also compare tv_usec but IMO it would be fine
to print the stats only one time at the end. Maybe
 -v   prints stats one time only
 -vv  report progress


cu,
Rudi

Re: blkdiscard progress breakes test

[Federico Simoncelli]

----- Original Message -----
> From: "Ruediger Meier" <sweet_f_a@gmx.de>
> To: util-linux@vger.kernel.org
> Cc: "Federico Simoncelli" <fsimonce@redhat.com>
> Sent: Wednesday, April 22, 2015 7:43:06 PM
> Subject: blkdiscard progress breakes test
> 
> Hi,
> 
> Commit c472a7e3 introduced progress messages. This randomly
> breaks the test-suite like this:
> 
> 
>    blkdiscard: offsets                        ... FAILED (blkdiscard/offsets)
> ....
> ---
> /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/expected/blkdiscard/offsets
> 2014-10-28 06:53:35.483698330 +0000
> +++
> /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/output/blkdiscard/offsets
> 2015-04-22 15:41:45.748155291 +0000
> @@ -21,7 +21,8 @@
>  blkdiscard: offset 1 is not aligned to sector size 512
>  blkdiscard: offset 1 is not aligned to sector size 512
>  blkdiscard: offset 511 is not aligned to sector size 512
> -Discarded 1536 bytes from the offset 512
> +Discarded 1024 bytes from the offset 512
> +Discarded 512 bytes from the offset 1536
>  Discarded 1024 bytes from the offset 1024
>  testing misaligned steps full device
>  blkdiscard: length 1 is not aligned to sector size 512
> 
> 
> It's because we ignore timeval's tv_usec here:
>   if (last.tv_sec < now.tv_sec) {
>       print_stats(path, stats);
> 
> We could make it better if we also compare tv_usec but IMO it would be fine

+1 to fix it (rather easy).

> to print the stats only one time at the end. Maybe
>  -v   prints stats one time only
>  -vv  report progress

But then -vv needs to be tested anyway.

-- 
Federico

Re: blkdiscard progress breakes test

[Karel Zak]

On Wed, Apr 22, 2015 at 07:43:06PM +0200, Ruediger Meier wrote:
> Commit c472a7e3 introduced progress messages. This randomly
> breaks the test-suite like this:
> 
> 
>    blkdiscard: offsets                        ... FAILED (blkdiscard/offsets)
> ....
> --- /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/expected/blkdiscard/offsets   2014-10-28 06:53:35.483698330 +0000
> +++ /home/abuild/rpmbuild/BUILD/util-linux-2.26.git233.01aa/tests/output/blkdiscard/offsets     2015-04-22 15:41:45.748155291 +0000
> @@ -21,7 +21,8 @@
>  blkdiscard: offset 1 is not aligned to sector size 512
>  blkdiscard: offset 1 is not aligned to sector size 512
>  blkdiscard: offset 511 is not aligned to sector size 512
> -Discarded 1536 bytes from the offset 512
> +Discarded 1024 bytes from the offset 512
> +Discarded 512 bytes from the offset 1536
>  Discarded 1024 bytes from the offset 1024
>  testing misaligned steps full device
>  blkdiscard: length 1 is not aligned to sector size 512
> 
> 
> It's because we ignore timeval's tv_usec here:
>   if (last.tv_sec < now.tv_sec) {
>       print_stats(path, stats);
> 
> We could make it better if we also compare tv_usec

I have doubts you can fix it, the progress reporting is based fixed
time intervals (60sec), but the number of the discarded chunks within
the interval depends on system and HW performance. IMHO the regression
test will be always fragile.

> but IMO it would be fine
> to print the stats only one time at the end. Maybe
>  -v   prints stats one time only
>  -vv  report progress

-vv is unnecessary, the progress is reported only when you specify 
-v together with -s.

I also guess that "-vs" is already used in applications, it would be
better to not change the behaviors.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com