tests: hwclock questions

tests: hwclock questions

[Ruediger Meier]

Hi,

The test hwclock/systohc needs some fixes because of the following 
issues:
 - ntpdate return value checks ("$?" == "1") are wrong
 - actually ntpdate is deprecated and we should go with "ntpd -q"
 - variable OFFSET should be checked for being numeric to protect "bc"
   and to avoid follow-up errors
 - hwclock loop should break and ts_failed if any hwclock call fails
 - We need to skip case "Cannot access the Hardware Clock via any
   known method"
 - I think we have to protect the user from setting his clock to
   un-wanted time zone (local or UTC).

On travis build server this test succeds allthough neither ntpdate nor 
hwclock works :)


But before doing this I have two questions.

1. I wonder what is the original purpose of this test. Are we really 
testing hwclock or the kernel or hardware? I mean setting and reading 
cock 10 times ... could this really discover a bug in our hwclock code?

2. Why do we _set_ the time from ntp server at the beginning? Wouldnt it 
be enough to check whether the offset to ntp is the same before and 
after the test?

cu,
Rudi

Re: tests: hwclock questions

[Karel Zak]

On Thu, Jun 05, 2014 at 12:32:24AM +0200, Ruediger Meier wrote:
> But before doing this I have two questions.

 Maybe we can remove the test at all (or use --force to enable the test). It 
 was always very problematic test and I have doubts it's still necessary as 
 relevant hwclock code is completely different and more robust now.

> 1. I wonder what is the original purpose of this test. Are we really 
> testing hwclock or the kernel or hardware? I mean setting and reading

 The purpose is to test how precisely is able hwclock to set HW time.
 Long time ago we had problem that each iteration added extra 0.5s to
 the time.

> cock 10 times ... could this really discover a bug in our hwclock code?

 Yes.

> 2. Why do we _set_ the time from ntp server at the beginning? Wouldnt it 
> be enough to check whether the offset to ntp is the same before and 
> after the test?

 Probably yes.

    Karel

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

Re: tests: hwclock questions

[Ruediger Meier]

On Thursday 05 June 2014, Karel Zak wrote:
> On Thu, Jun 05, 2014 at 12:32:24AM +0200, Ruediger Meier wrote:
> > But before doing this I have two questions.
>
>  Maybe we can remove the test at all (or use --force to enable the
> test). It was always very problematic test 

I will try to make it a bit more robust and then see if we still should 
make it optional. IMO these time sync problems are some kind of 
science. Could be too much for being solved within one simple shell 
script.

> and I have doubts it's
> still necessary as relevant hwclock code is completely different and
> more robust now.

Ok, then I would do --systohc and --hctosys just one time instead of 10.

BTW how could I get the offset between sys and hw clock? What does the 
last "xxx seconds" in --show output mean? Obviously not the whole 
offset:

$ export LANG=C; export TZ=UTC
$ date
Thu Jun  5 12:38:21 UTC 2014
$ hwclock --show --utc
Thu Jun  5 13:07:28 2014  -0.062807 seconds

> > 1. I wonder what is the original purpose of this test. Are we
> > really testing hwclock or the kernel or hardware? I mean setting
> > and reading
>
>  The purpose is to test how precisely is able hwclock to set HW time.
>  Long time ago we had problem that each iteration added extra 0.5s to
>  the time.

> > cock 10 times ... could this really discover a bug in our hwclock
> > code?
>
>  Yes.
>
> > 2. Why do we _set_ the time from ntp server at the beginning?
> > Wouldnt it be enough to check whether the offset to ntp is the same
> > before and after the test?
>
>  Probably yes.

Maybe getting sys vs. ntp offset like this:

$ ntp_ip=$(dig 0.fedora.pool.ntp.org ANY +short | head -n 1)
$ sntp -K /tmp/kod "$ntp_ip"
 5 Jun 13:00:38 sntp[6547]: Started sntp
2014-06-05 13:00:38.263771 (-0100) -0.002546 +/- 0.022034 secs

Note we have to use fixed IP to get comparable offsets.

cu,
Rudi

Re: [PATCH] tests: hwclock questions

[Ruediger Meier]

On Thursday 05 June 2014, Ruediger Meier wrote:
> On Thursday 05 June 2014, Karel Zak wrote:
> > On Thu, Jun 05, 2014 at 12:32:24AM +0200, Ruediger Meier wrote:
> > > But before doing this I have two questions.
> >
> >  Maybe we can remove the test at all (or use --force to enable the
> > test). It was always very problematic test
>
> I will try to make it a bit more robust and then see if we still
> should make it optional.

I've rewrote it and sent patch to github
https://github.com/karelzak/util-linux/pull/94

Below the full script (better to read than the patch in this case).
Please comment.

# tests/ts/hwclock/systohc:

TS_TOPDIR="${0%/*}/../.."
TS_DESC="system to hw"
NTP_SERVER="0.fedora.pool.ntp.org"

. $TS_TOPDIR/functions.sh
ts_init "$*"

ts_check_test_command "$TS_CMD_HWCLOCK"

ts_skip_nonroot
ts_check_prog "bc"
ts_check_prog "sntp"

function resolve_host
{
	local host="$1"
	local tmp

	# currently we just resolve default records (might be "A", ipv4 only)
	if type "dig" >/dev/null 2>&1; then
		tmp=$(dig "$host" +short 2>/dev/null) || return 1
	elif type "nslookup" >/dev/null 2>&1; then
		tmp=$(nslookup "$host" 2>/dev/null) || return 1
		tmp=$(echo "$tmp"| grep -A1 "^Name:"| grep "^Address:"| cut -d" " -f2)
	fi

	# we return 1 if tmp is empty
	test -n "$tmp" || return 1
	echo "$tmp" | sort -R | head -n 1
}

function get_offset_sys_ntp
{
	local ips="$@"
	local out

	out=$(sntp -K "$SNTP_KOD" --timeout 1 "$ips") || return 1

	# sed must deliver a signed float or empty string for sure
	out=$(echo "$out" | \
		sed -n 's/.* \(\(+\|-\)[0-9]\{1,\}\.*[0-9]*\) +\/-.*secs$/\1/p')

	[ -n "$out" ] || return 1
	echo "$out"
}

function check_diff_offset
{
	local a=${1#+}
	local b=${2#+}
	local max="$3"
	local tmp

	tmp=$(echo "$a - $b" | bc | tr -d '-')
	echo "$tmp"

	tmp=$(echo "$tmp < $max" | bc)
	[ $tmp -eq 1 ]
}


# we need fixed ntp IP to get comparable offsets
NTP_IP=$(resolve_host "$NTP_SERVER") \
	|| ts_skip "can't resolve hostname $NTP_SERVER"

# needed in get_offset_sys_ntp
SNTP_KOD="$(mktemp "${TS_OUTDIR}/kod-XXXX")"

OFFSET_A=$(get_offset_sys_ntp "$NTP_IP") \
	|| ts_skip "can't get ntp offset 1st, $NTP_IP"
OFFSET_B=$(get_offset_sys_ntp "$NTP_IP") \
	|| ts_skip "can't get ntp offset 2nd, $NTP_IP"

diff=$(check_diff_offset $OFFSET_A $OFFSET_B 0.02) \
	|| ts_skip "unreliable ntp or sys clock offsets: $NTP_IP $OFFSET_A $OFFSET_B +/-$diff"

# hwclock --show should work if we have a hw clock
tmp=$($TS_CMD_HWCLOCK --show 2>&1)
if [ $? != "0" ]; then
	echo "$tmp" | grep -q "Cannot access the Hardware Clock via" \
		&& ts_skip "no hardware clock found"
	ts_failed "hwclock --show"
fi

# call hwclock
# TODO - try to be nice, use --localtime if wanted
for i in `seq 1 10`; do
	# only skip on failure for now
	$TS_CMD_HWCLOCK --systohc || ts_skip "--systohc, $i"
	$TS_CMD_HWCLOCK --hctosys || ts_skip "--hctosys, $i"
done

OFFSET_C=$(get_offset_sys_ntp "$NTP_IP") \
	|| ts_skip "can't get ntp offset 3rd, $NTP_IP"

diff=$(check_diff_offset "$OFFSET_B" "$OFFSET_C" 1.0) \
	|| ts_failed "offsets $NTP_IP: $OFFSET_B $OFFSET_C +/-$diff"

ts_ok "offsets $NTP_IP: $OFFSET_B $OFFSET_C +/-$diff"