| Message ID | 158086092087.1989378.18220785148122680849.stgit@magnolia (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fstests: random fixes | expand |
On Wed, Feb 5, 2020 at 2:02 AM Darrick J. Wong <darrick.wong@oracle.com> wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > If xfs_io's utimes command cannot interpret the arguments that are given > to it, it will print out "Bad value for [am]time". Detect when this > happens and drop the file out of the test entirely. > > This is particularly noticeable on 32-bit platforms and the largest > timestamp seconds supported by the filesystem is INT_MAX. In this case, > the maximum value we can cram into tv_sec is INT_MAX, and there is no > way to actually test setting a timestamp of INT_MAX + 1 to test the > clamping. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > tests/generic/402 | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > > diff --git a/tests/generic/402 b/tests/generic/402 > index 2a34d127..32988866 100755 > --- a/tests/generic/402 > +++ b/tests/generic/402 > @@ -63,10 +63,19 @@ run_test_individual() > # check if the time needs update > if [ $update_time -eq 1 ]; then > echo "Updating file: $file to timestamp $timestamp" >> $seqres.full > - $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file > + $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file >> $tmp.utimes 2>&1 Maybe use > instead of >> to be safe. Also I would feel more comfortable if we special case the 0 timestamp against being skipped, to be safe that we don't have a silent regression in xfs_io or something causing all files to be skipped. Otherwise: Reviewed-by: Amir Goldstein <amir73il@gmail.com> > + cat $tmp.utimes >> $seqres.full > + if grep -q "Bad value" "$tmp.utimes"; then > + rm -f $file $tmp.utimes > + return > + fi > + cat $tmp.utimes > + rm $tmp.utimes > if [ $? -ne 0 ]; then > echo "Failed to update times on $file" | tee -a $seqres.full > fi > + else > + test -f $file || return > fi > > tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp)) >
On Tue, Feb 04, 2020 at 04:02:00PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > If xfs_io's utimes command cannot interpret the arguments that are given > to it, it will print out "Bad value for [am]time". Detect when this > happens and drop the file out of the test entirely. > > This is particularly noticeable on 32-bit platforms and the largest > timestamp seconds supported by the filesystem is INT_MAX. In this case, > the maximum value we can cram into tv_sec is INT_MAX, and there is no > way to actually test setting a timestamp of INT_MAX + 1 to test the > clamping. > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > tests/generic/402 | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > > diff --git a/tests/generic/402 b/tests/generic/402 > index 2a34d127..32988866 100755 > --- a/tests/generic/402 > +++ b/tests/generic/402 > @@ -63,10 +63,19 @@ run_test_individual() > # check if the time needs update > if [ $update_time -eq 1 ]; then > echo "Updating file: $file to timestamp $timestamp" >> $seqres.full > - $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file > + $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file >> $tmp.utimes 2>&1 Agree with Amir here, ">" whould be better, instead of appending. > + cat $tmp.utimes >> $seqres.full > + if grep -q "Bad value" "$tmp.utimes"; then Echo a message to $seqres.full about this test being skipped? > + rm -f $file $tmp.utimes > + return > + fi > + cat $tmp.utimes > + rm $tmp.utimes > if [ $? -ne 0 ]; then So here we test the result of "rm $tmp.utimes"? I guess that's always a pass. > echo "Failed to update times on $file" | tee -a $seqres.full > fi > + else > + test -f $file || return Same here, better to be verbose about skipping test. Thanks, Eryu > fi > > tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp)) >
On Sun, Feb 09, 2020 at 11:29:54PM +0800, Eryu Guan wrote: > On Tue, Feb 04, 2020 at 04:02:00PM -0800, Darrick J. Wong wrote: > > From: Darrick J. Wong <darrick.wong@oracle.com> > > > > If xfs_io's utimes command cannot interpret the arguments that are given > > to it, it will print out "Bad value for [am]time". Detect when this > > happens and drop the file out of the test entirely. > > > > This is particularly noticeable on 32-bit platforms and the largest > > timestamp seconds supported by the filesystem is INT_MAX. In this case, > > the maximum value we can cram into tv_sec is INT_MAX, and there is no > > way to actually test setting a timestamp of INT_MAX + 1 to test the > > clamping. > > > > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > > --- > > tests/generic/402 | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/tests/generic/402 b/tests/generic/402 > > index 2a34d127..32988866 100755 > > --- a/tests/generic/402 > > +++ b/tests/generic/402 > > @@ -63,10 +63,19 @@ run_test_individual() > > # check if the time needs update > > if [ $update_time -eq 1 ]; then > > echo "Updating file: $file to timestamp $timestamp" >> $seqres.full > > - $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file > > + $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file >> $tmp.utimes 2>&1 > > Agree with Amir here, ">" whould be better, instead of appending. Fixed. > > + cat $tmp.utimes >> $seqres.full > > + if grep -q "Bad value" "$tmp.utimes"; then > > Echo a message to $seqres.full about this test being skipped? Fixed. > > + rm -f $file $tmp.utimes > > + return > > + fi > > + cat $tmp.utimes > > + rm $tmp.utimes > > if [ $? -ne 0 ]; then > > So here we test the result of "rm $tmp.utimes"? I guess that's always a > pass. Err, oops, I'll save the value of $? from the xfs_io command. > > echo "Failed to update times on $file" | tee -a $seqres.full > > fi > > + else > > + test -f $file || return > > Same here, better to be verbose about skipping test. ok. --D > Thanks, > Eryu > > > fi > > > > tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp)) > >
diff --git a/tests/generic/402 b/tests/generic/402 index 2a34d127..32988866 100755 --- a/tests/generic/402 +++ b/tests/generic/402 @@ -63,10 +63,19 @@ run_test_individual() # check if the time needs update if [ $update_time -eq 1 ]; then echo "Updating file: $file to timestamp $timestamp" >> $seqres.full - $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file + $XFS_IO_PROG -f -c "utimes $timestamp 0 $timestamp 0" $file >> $tmp.utimes 2>&1 + cat $tmp.utimes >> $seqres.full + if grep -q "Bad value" "$tmp.utimes"; then + rm -f $file $tmp.utimes + return + fi + cat $tmp.utimes + rm $tmp.utimes if [ $? -ne 0 ]; then echo "Failed to update times on $file" | tee -a $seqres.full fi + else + test -f $file || return fi tsclamp=$((timestamp<tsmin?tsmin:timestamp>tsmax?tsmax:timestamp))