[PATCH 0/5] blkdiscard: some output fixes

[PATCH 0/5] blkdiscard: some output fixes

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

This is pull request 239 on github
https://github.com/karelzak/util-linux/pull/239

Ruediger Meier (5):
  blkdiscard: fix timeval comparison inclusive usecs
  blkdiscard: fix stats reporting (off by one)
  blkdiscard: don't report "zero range" at the end
  tests: blkdiscard, check return values
  tests: skip BLKDISCARD if unsupported

 sys-utils/blkdiscard.c            | 13 ++++---
 tests/expected/blkdiscard/offsets | 31 ++++++++++++++++
 tests/ts/blkdiscard/offsets       | 78 +++++++++++++++++++++++----------------
 3 files changed, 84 insertions(+), 38 deletions(-)

-- 
1.8.4.5

[PATCH 1/5] blkdiscard: fix timeval comparison inclusive usecs

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

We will see if this makes our "--step" tests reliable.

CC: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/blkdiscard.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
index 150689a..a853a35 100644
--- a/sys-utils/blkdiscard.c
+++ b/sys-utils/blkdiscard.c
@@ -197,10 +197,11 @@ int main(int argc, char **argv)
 				err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path);
 		}
 
-		/* reporting progress */
+		/* reporting progress at most once per second */
 		if (verbose && step) {
 			gettime_monotonic(&now);
-			if (last.tv_sec < now.tv_sec) {
+			if (now.tv_sec > last.tv_sec &&
+			    (now.tv_usec >= last.tv_usec || now.tv_sec > last.tv_sec + 1)) {
 				print_stats(path, stats);
 				stats[0] = range[0], stats[1] = 0;
 				last = now;
-- 
1.8.4.5

[PATCH 2/5] blkdiscard: fix stats reporting (off by one)

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

CC: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/blkdiscard.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
index a853a35..bb4bae6 100644
--- a/sys-utils/blkdiscard.c
+++ b/sys-utils/blkdiscard.c
@@ -197,18 +197,18 @@ int main(int argc, char **argv)
 				err(EXIT_FAILURE, _("%s: BLKDISCARD ioctl failed"), path);
 		}
 
+		stats[1] += range[1];
+
 		/* reporting progress at most once per second */
 		if (verbose && step) {
 			gettime_monotonic(&now);
 			if (now.tv_sec > last.tv_sec &&
 			    (now.tv_usec >= last.tv_usec || now.tv_sec > last.tv_sec + 1)) {
 				print_stats(path, stats);
-				stats[0] = range[0], stats[1] = 0;
+				stats[0] += stats[1], stats[1] = 0;
 				last = now;
 			}
 		}
-
-		stats[1] += range[1];
 	}
 
 	if (verbose)
-- 
1.8.4.5

[PATCH 3/5] blkdiscard: don't report "zero range" at the end

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

We want to address the case that we have printed the last useful
stats line already within the loop. Avoiding an additional line
"Discarded 0 bytes ..." at the end.

Note there is a behavior change now for the edge cases "-v -l 0" and
"-v -o blksize" where we don't print any stats line anymore. But actually
it's correct, we never make any BLKDISCARD syscall with zero range.
Perhaps we should return error in these cases to help people who always
want to parse stats output on success.

CC: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 sys-utils/blkdiscard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sys-utils/blkdiscard.c b/sys-utils/blkdiscard.c
index bb4bae6..f36e504 100644
--- a/sys-utils/blkdiscard.c
+++ b/sys-utils/blkdiscard.c
@@ -211,7 +211,7 @@ int main(int argc, char **argv)
 		}
 	}
 
-	if (verbose)
+	if (verbose && stats[1])
 		print_stats(path, stats);
 
 	close(fd);
-- 
1.8.4.5

[PATCH 4/5] tests: blkdiscard, check return values

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

CC: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 tests/expected/blkdiscard/offsets | 31 +++++++++++++++++
 tests/ts/blkdiscard/offsets       | 73 ++++++++++++++++++++++-----------------
 2 files changed, 72 insertions(+), 32 deletions(-)

diff --git a/tests/expected/blkdiscard/offsets b/tests/expected/blkdiscard/offsets
index 2596fc6..aa90f2f 100644
--- a/tests/expected/blkdiscard/offsets
+++ b/tests/expected/blkdiscard/offsets
@@ -1,39 +1,70 @@
 create loop device from image
 testing offsets with full block size
 Discarded 10485760 bytes from the offset 0
+ret: 0
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 511 is not aligned to sector size 512
+ret: 1
 Discarded 10485248 bytes from the offset 512
+ret: 0
 Discarded 10484736 bytes from the offset 1024
+ret: 0
 testing offsets with specific length
 Discarded 5242880 bytes from the offset 0
+ret: 0
 blkdiscard: length 5242881 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 5243391 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 511 is not aligned to sector size 512
+ret: 1
 Discarded 5242880 bytes from the offset 512
+ret: 0
 Discarded 5242880 bytes from the offset 1024
+ret: 0
 testing aligned steps full device
 Discarded 10485760 bytes from the offset 0
+ret: 0
 Discarded 10485760 bytes from the offset 0
+ret: 0
 testing aligned steps with offsets and length
 Discarded 1024 bytes from the offset 0
+ret: 0
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 511 is not aligned to sector size 512
+ret: 1
 Discarded 1536 bytes from the offset 512
+ret: 0
 Discarded 1024 bytes from the offset 1024
+ret: 0
 testing misaligned steps full device
 blkdiscard: length 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 256 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 511 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 513 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 768 is not aligned to sector size 512
+ret: 1
 testing misaligned steps with offsets and length
 blkdiscard: length 511 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 511 is not aligned to sector size 512
+ret: 1
 blkdiscard: length 511 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 1 is not aligned to sector size 512
+ret: 1
 blkdiscard: offset 511 is not aligned to sector size 512
+ret: 1
 detach loop device from image
diff --git a/tests/ts/blkdiscard/offsets b/tests/ts/blkdiscard/offsets
index eb0097e..29597ad 100755
--- a/tests/ts/blkdiscard/offsets
+++ b/tests/ts/blkdiscard/offsets
@@ -35,50 +35,59 @@ truncate -s 10M $IMAGE_PATH
 ts_log "create loop device from image"
 DEVICE=$($TS_CMD_LOSETUP --show -f $IMAGE_PATH)
 ts_register_loop_device "$DEVICE"
-CMD_SED_DEVICE="sed s#$DEVICE:\s##"
+
+function run_tscmd {
+	local ret
+	"$@" >> $TS_OUTPUT 2>&1
+	ret=$?
+	echo "ret: $ret" >> "$TS_OUTPUT"
+	return $ret
+}
 
 ts_log "testing offsets with full block size"
-$TS_CMD_BLKDISCARD -v $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 1 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 511 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 512 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 1 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 511 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 512 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 1024 $DEVICE
 
 ts_log "testing offsets with specific length"
-$TS_CMD_BLKDISCARD -v -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -l 5242881 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -l 5243391 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 1 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 511 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 512 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -o 1024 -l 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v -l 5242880 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -l 5242881 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -l 5243391 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 1 -l 5242880 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 511 -l 5242880 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 512 -l 5242880 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -o 1024 -l 5242880 $DEVICE
 
 ts_log "testing aligned steps full device"
-$TS_CMD_BLKDISCARD -v -p 5242880 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 1310720 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v -p 5242880 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 1310720 $DEVICE
 
 ts_log "testing aligned steps with offsets and length"
-$TS_CMD_BLKDISCARD -v -p 512 -l 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 512 -o 1 -l 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 512 -o 1 -l 1536 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 512 -o 511 -l 1536 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 512 -o 512 -l 1536 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 512 -o 1024 -l 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -l 1024 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -o 1 -l 1024 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -o 1 -l 1536 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -o 511 -l 1536 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -o 512 -l 1536 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 512 -o 1024 -l 1024 $DEVICE
 
 ts_log "testing misaligned steps full device"
-$TS_CMD_BLKDISCARD -v -p 1 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 256 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 513 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 768 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v -p 1 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 256 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 513 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 768 $DEVICE
 
 ts_log "testing misaligned steps with offsets and length"
-$TS_CMD_BLKDISCARD -v -p 511 -l 1024 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 -o 1 -l 1536 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 -o 511 -l 1536 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 -l 10240 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 -o 1 -l 10240 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
-$TS_CMD_BLKDISCARD -v -p 511 -o 511 -l 10240 $DEVICE 2>&1 | $CMD_SED_DEVICE >> $TS_OUTPUT
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -l 1024 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -o 1 -l 1536 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -o 511 -l 1536 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -l 10240 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -o 1 -l 10240 $DEVICE
+run_tscmd $TS_CMD_BLKDISCARD -v -p 511 -o 511 -l 10240 $DEVICE
+
+sed -i "s#$DEVICE:\s##" $TS_OUTPUT
 
 ts_log "detach loop device from image"
 
-- 
1.8.4.5

[PATCH 5/5] tests: skip BLKDISCARD if unsupported

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

CC: Federico Simoncelli <fsimonce@redhat.com>
Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 tests/ts/blkdiscard/offsets | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/ts/blkdiscard/offsets b/tests/ts/blkdiscard/offsets
index 29597ad..2888411 100755
--- a/tests/ts/blkdiscard/offsets
+++ b/tests/ts/blkdiscard/offsets
@@ -46,6 +46,11 @@ function run_tscmd {
 
 ts_log "testing offsets with full block size"
 run_tscmd $TS_CMD_BLKDISCARD -v $DEVICE
+if [ "$?" != "0" ]; then
+	# Skip the rest? For example loop backing files on NFS seem unsupported.
+	grep -q "BLKDISCARD ioctl failed: Operation not supported" "$TS_OUTPUT" \
+		&& ts_skip "BLKDISCARD not supported"
+fi
 run_tscmd $TS_CMD_BLKDISCARD -v -o 1 $DEVICE
 run_tscmd $TS_CMD_BLKDISCARD -v -o 511 $DEVICE
 run_tscmd $TS_CMD_BLKDISCARD -v -o 512 $DEVICE
-- 
1.8.4.5

Re: [PATCH 0/5] blkdiscard: some output fixes

[Karel Zak]

On Wed, Dec 09, 2015 at 06:43:09PM +0100, Ruediger Meier wrote:
>   blkdiscard: fix timeval comparison inclusive usecs
>   blkdiscard: fix stats reporting (off by one)
>   blkdiscard: don't report "zero range" at the end
>   tests: blkdiscard, check return values
>   tests: skip BLKDISCARD if unsupported

Merged, thanks!

    Karel

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