Re: [RFC PATCH] generic: test negative timespecs are accurate

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 3521 bytes --]

Zorro Lang <zlang@redhat.com> writes:

> On Tue, Sep 10, 2024 at 12:02:05PM +0200, Alyssa Ross wrote:
>> Zorro Lang <zlang@redhat.com> writes:
>> 
>> > On Sat, Sep 07, 2024 at 05:45:28PM +0200, Alyssa Ross wrote:
>> >> Link: https://github.com/koverstreet/bcachefs/issues/743
>> >
>> > Great, a bcachefs regression test case :)
>> >
>> > Can you add a bit more details in commit log, not only a link.
>> >
>> >> Signed-off-by: Alyssa Ross <hi@alyssa.is>
>> >> ---
>> >> This is an adapted version of generic/258, but it tests that the stored 
>> >> timestamp is accurate to the second, rather than just testing the 
>> >> timestamp remains negative.  I created a new test rather than just
>> >> making 258 more precise, because I understand that there may be
>> >> filesystems that don't store timestamps that accurately by design.
>> >> As an example, I've heard that FAT only has 2 second precision, so this
>> >> patch is an RFC because I'm not sure how I should write a _require
>> >> function (if at all) that restricts the test to filesystems that are
>> >> expected to be able to do this.
>> >> 
>> >>  tests/generic/363     | 26 ++++++++++++++++++++++++++
>> >>  tests/generic/363.out |  2 ++
>> >
>> > The g/363 has been taken, please rebase to latest for-next branch.
>> > You can use `xfstests/tools/mvtest generic/363 generic/365` to change
>> > the case number, before rebasing.
>> >
>> >>  2 files changed, 28 insertions(+)
>> >>  create mode 100755 tests/generic/363
>> >>  create mode 100644 tests/generic/363.out
>> >> 
>> >> diff --git a/tests/generic/363 b/tests/generic/363
>> >> new file mode 100755
>> >> index 00000000..50459d01
>> >> --- /dev/null
>> >> +++ b/tests/generic/363
>> >> @@ -0,0 +1,26 @@
>> >> +#! /bin/bash
>> >> +# SPDX-License-Identifier: GPL-2.0
>> >> +# Copyright (c) 2011 Red Hat, Inc.  All Rights Reserved.
>> >> +# Copyright (c) 2024 Alyssa Ross.  All Rights Reserved.
>> >> +#
>> >> +# FS QA Test 363
>> >> +#
>> >> +# Test timestamps prior to epoch with nanosecond components are
>> >> +# accurate to the second.
>> >> +# bcachefs was slightly off.
>> >> +#
>> >> +. ./common/preamble
>> >> +_begin_fstest auto quick bigtime
>> >> +
>> >
>> > As this's a bcachefs regression test:
>> > https://lore.kernel.org/linux-bcachefs/20240907160024.605850-3-hi@alyssa.is/
>> >
>> > So better to mark as:
>> > if [ "$FSTYP" = "bcachefs" ];then
>> > 	_fixed_by_kernel_commit xxxxxxxxxxxx "bcachefs: Fix negative timespecs"
>> > fi
>> > (replace the xxxxxxx if it's merged on mainline linux)
>> >
>> > Others looks good to me, with above changes, I'd like to
>> >
>> > Reviewed-by: Zorro Lang <zlang@redhat.com>
>> 
>> Thanks!  So you don't think the test needs to express any extra
>> requirement that filesystems support to-the-second precision?
>
> Oh, does this test needs 1 second precision? I thought you concerned some
> filesystems cannot be the epoch you set. The _require_negative_timestamps
> will skip the test from exfat and ceph. It doesn't help the FAT, maybe you
> want to add vfat to _require_negative_timestamps.
>
> I thought you might want to output the stored timestamp (accurate to the
> second) to reproduce this bug, if not, why you hope to write this new one,
> not use the g/258 to be the reproducer directly?

I'm sorry, I don't understand the question.  I explained why I created a
new test in my commentary — g/258 doesn't require 1 second precision,
and this test does.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]