public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: viro@ZenIV.linux.org.uk, torvalds@linux-foundation.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	bfields@fieldses.org, hch@infradead.org,
	akpm@linux-foundation.org, dhowells@redhat.com, zab@redhat.com,
	jack@suse.cz, luto@amacapital.net, mszeredi@suse.cz
Subject: Re: [PATCH 00/13] cross rename v4
Date: Mon, 17 Feb 2014 19:19:11 +1100	[thread overview]
Message-ID: <20140217081911.GZ13647@dastard> (raw)
In-Reply-To: <20140212171852.GA4026@tucsk.piliscsaba.szeredi.hu>

On Wed, Feb 12, 2014 at 06:18:52PM +0100, Miklos Szeredi wrote:
> On Tue, Feb 11, 2014 at 05:01:41PM +0100, Miklos Szeredi wrote:
> > On Mon, Feb 10, 2014 at 09:51:45PM +1100, Dave Chinner wrote:
> 
> > > Miklos, can you please write an xfstest for this new API? That way
> > > we can verify that the behaviour is as documented, and we can ensure
> > > that when we implement it on other filesystems it works exactly the
> > > same on all filesystems?
> 
> This is a standalone testprog, but I guess it's trivial to integrate into
> xfstests.

Same problem with integrating any standalone test program into
xfstests - we end up with a standalone pass/fail test instead of a
bunch of components we can reuse and refactor for other tests.  But
we can work around that for the moment.

[ FWIW, the normal way to write an xfstest like this is to write a
small helper program that just does the renameat2() syscall (we
often use xfs_io to provide this) and everything is just shell
scripts to drive the helper program in the necessary way. We don't
directly check that mode, size, destination of a file is correct -
just stat(1) on the expected destinations is sufficient to capture
this information. stdout is captured by the test harness and used to match
against a golden output. If the match fails, the test fails.

This would allow us to use the same test infrastructure for testing
a coreutils binary that implemented renameat2 when that comes
along... ]

> Please let me know what you think.

We need to be able to test whether the syscall exists so we
add a function like:

_requires_renameat2(
{
	[ -x src/renameat2_test ] || _notrun "renameat2_test not found."
	src/renameat2_test -t || _notrun "kernel does not support renameat2"
}

to gracefully avoid the test on kernels that don't support the
syscall. Indeed, the test needs to be able to be built on kernels
that don't have the right header files that define the syscall
number, either, so there's going to need to be some #ifndef
__NR_renameat2 type clauses in there as well...

And finally, it needs comments to explain what the test is actually
testing - if you don't document what the test is supposed to be
checking, how do we know that it is testing is actually correct?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2014-02-17  8:19 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-07 16:48 [PATCH 00/13] cross rename v4 Miklos Szeredi
2014-02-07 16:48 ` [PATCH 01/13] vfs: add d_is_dir() Miklos Szeredi
2014-02-07 17:36   ` J. Bruce Fields
2014-02-07 19:30     ` David Howells
2014-02-07 16:49 ` [PATCH 02/13] vfs: rename: move d_move() up Miklos Szeredi
2014-02-07 16:49 ` [PATCH 03/13] vfs: rename: use common code for dir and non-dir Miklos Szeredi
2014-02-07 16:49 ` [PATCH 04/13] vfs: add renameat2 syscall Miklos Szeredi
2014-02-07 16:49 ` [PATCH 05/13] vfs: add RENAME_NOREPLACE flag Miklos Szeredi
2014-02-07 16:49 ` [PATCH 06/13] security: add flags to rename hooks Miklos Szeredi
2014-02-07 16:49 ` [PATCH 07/13] vfs: lock_two_nondirectories: allow directory args Miklos Szeredi
2014-02-07 21:16   ` J. Bruce Fields
2014-02-11 15:32     ` Miklos Szeredi
2014-02-07 16:49 ` [PATCH 08/13] vfs: add cross-rename Miklos Szeredi
2014-02-07 22:40   ` J. Bruce Fields
2014-02-11 15:55     ` Miklos Szeredi
2014-02-07 16:49 ` [PATCH 09/13] ext4: rename: create ext4_renament structure for local vars Miklos Szeredi
2014-02-07 16:49 ` [PATCH 10/13] ext4: rename: move EMLINK check up Miklos Szeredi
2014-02-07 16:49 ` [PATCH 11/13] ext4: rename: split out helper functions Miklos Szeredi
2014-02-07 16:49 ` [PATCH 12/13] ext4: add cross rename support Miklos Szeredi
2014-02-11 21:23   ` Jan Kara
2014-02-07 16:49 ` [PATCH 13/13] vfs: merge rename2 into rename Miklos Szeredi
2014-02-07 22:46 ` [PATCH 00/13] cross rename v4 J. Bruce Fields
2014-02-11 15:57   ` Miklos Szeredi
2014-02-13 19:32     ` J. Bruce Fields
2014-02-10 10:51 ` Dave Chinner
2014-02-11 16:01   ` Miklos Szeredi
2014-02-12 17:18     ` Miklos Szeredi
2014-02-17  8:19       ` Dave Chinner [this message]
2014-02-17 18:04         ` Theodore Ts'o
2014-03-19 13:57         ` xfstest for renameat2 system call (was: [PATCH 00/13] cross rename v4) Miklos Szeredi
2014-04-08  1:23           ` Dave Chinner
2014-02-13 15:54 ` [PATCH 00/13] cross rename v4 David Howells
2014-02-13 16:25   ` Miklos Szeredi
2014-02-13 16:42     ` David Howells
2014-02-13 17:28       ` Miklos Szeredi
2014-02-13 18:21         ` Andy Lutomirski
2014-02-13 18:29         ` Linus Torvalds
2014-02-13 18:56           ` Miklos Szeredi
2014-02-13 19:20             ` Linus Torvalds
2014-02-13 19:02           ` David Howells
2014-02-13 19:32             ` Linus Torvalds
2014-02-13 20:17               ` Eric W. Biederman
2014-02-13 20:28               ` Miklos Szeredi
2014-02-24 17:12                 ` Miklos Szeredi
2014-02-24 17:49                   ` Linus Torvalds
2014-02-25  4:07                   ` J. R. Okajima
2014-02-26 15:15                   ` Jan Kara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140217081911.GZ13647@dastard \
    --to=david@fromorbit.com \
    --cc=akpm@linux-foundation.org \
    --cc=bfields@fieldses.org \
    --cc=dhowells@redhat.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=miklos@szeredi.hu \
    --cc=mszeredi@suse.cz \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@ZenIV.linux.org.uk \
    --cc=zab@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox