* Re: [PATCH v2 0/3] xfs: add O_TMPFILE support
[not found] <1387326161-24530-1-git-send-email-zwu.kernel@gmail.com>
@ 2013-12-26 8:51 ` Christoph Hellwig
[not found] ` <20131226085134.GA32660-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2014-01-07 20:49 ` Ben Myers
0 siblings, 2 replies; 4+ messages in thread
From: Christoph Hellwig @ 2013-12-26 8:51 UTC (permalink / raw)
To: Zhi Yong Wu; +Cc: linux-api, Zhi Yong Wu, xfs
Both patches looks good to me,
Reviewed-by: Christoph Hellwig <hch@lst.de>
But now we need to make sure we can exercise it, which will need
support for tmpfile and flink commands in xfs_io.
And tests that use those commands in xfstests. At a minimum I'd
suggest:
- test creating read-only/read-write/executable files
- check how permission bits actually work for O_TMPFILE and
enforce that (unfortunately I can't find any documentation for that)
- check that creating non-regular files is properly rejected
- check that flink works on O_TMPFILE files, but doesn't work on
on other files
For an additional XFS-specific test I'd recommend:
- creating a tmpfile, use src/godown to force the filesystem down and
check that repair recoveres the unlinked inode list
- creating a tmpfile, flinking it, use src/godown to force the
filesystem down and check that repair does not have to recover
the unlink inode list.
On Wed, Dec 18, 2013 at 08:22:38AM +0800, Zhi Yong Wu wrote:
> From: Zhi Yong Wu <wuzhy@linux.vnet.ibm.com>
>
> HI, folks
>
> It's time to post the latest revision out, welcome to any constructive comment, thanks.
>
> If anyone is interested in playing with it, you can get this patchset from my dev git on github:
> git://github.com/wuzhy/kernel.git xfs_tmpfile
>
> The patchset was tested against the code snippet from Andy Lutomirski and other test cases:
> http://lwn.net/Articles/562296/
> If you have any other better test cases, please let me know, thanks.
>
> #include <stdio.h>
> #include <err.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <string.h>
>
> #define __O_TMPFILE 020000000
> #define O_DIRECTORY 0200000
> #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
> #define AT_EMPTY_PATH 0x1000
>
> int main(int argc, char **argv)
> {
> char buf[128];
>
> if (argc != 3)
> errx(1, "Usage: flinktest PATH linkat|proc");
>
> int fd = open(".", O_TMPFILE | O_RDWR, 0600);
> if (fd == -1)
> err(1, "O_TMPFILE");
> else
> printf("fd #: %d\n", fd);
>
> write(fd, "test", 4);
>
> if (!strcmp(argv[2], "linkat")) {
> if (linkat(fd, "", AT_FDCWD, argv[1], AT_EMPTY_PATH) != 0)
> err(1, "linkat");
> } else if (!strcmp(argv[2], "proc")) {
> sprintf(buf, "/proc/self/fd/%d", fd);
> if (linkat(AT_FDCWD, buf, AT_FDCWD, argv[1], AT_SYMLINK_FOLLOW) != 0)
> err(1, "linkat");
> } else {
> errx(1, "invalid mode");
> }
>
> return 0;
> }
>
> Changelog from v1:
> - Fixed one chunk of the comments from Christoph Hellwig and Jeff Liu.
>
> v1:
> - Addressed the comments from Dave Chinner and Christoph Hellwig.
>
> Zhi Yong Wu (3):
> xfs: factor prid related codes into xfs_get_initial_prid()
> xfs: add O_TMPFILE support
> xfs: allow linkat() on O_TMPFILE files
>
> fs/xfs/xfs_inode.c | 123 ++++++++++++++++++++++++++++++++++++++++++++--
> fs/xfs/xfs_inode.h | 12 +++++
> fs/xfs/xfs_iops.c | 16 ++++++
> fs/xfs/xfs_shared.h | 4 +-
> fs/xfs/xfs_symlink.c | 5 +--
> fs/xfs/xfs_trans_resv.c | 55 +++++++++++++++++++--
> fs/xfs/xfs_trans_resv.h | 2 +
> 7 files changed, 202 insertions(+), 15 deletions(-)
>
> --
> 1.7.6.5
>
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
---end quoted text---
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 0/3] xfs: add O_TMPFILE support
[not found] ` <20131226085134.GA32660-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2013-12-28 10:04 ` Zhi Yong Wu
0 siblings, 0 replies; 4+ messages in thread
From: Zhi Yong Wu @ 2013-12-28 10:04 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: xfstests, Zhi Yong Wu, linux-api-u79uwXL29TY76Z2rM5mHXA
On Thu, Dec 26, 2013 at 4:51 PM, Christoph Hellwig <hch-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote:
> Both patches looks good to me,
>
> Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Indeed thanks for your review. ok, i will place them to my TODO list.
>
> But now we need to make sure we can exercise it, which will need
> support for tmpfile and flink commands in xfs_io.
>
> And tests that use those commands in xfstests. At a minimum I'd
> suggest:
>
> - test creating read-only/read-write/executable files
> - check how permission bits actually work for O_TMPFILE and
> enforce that (unfortunately I can't find any documentation for that)
> - check that creating non-regular files is properly rejected
> - check that flink works on O_TMPFILE files, but doesn't work on
> on other files
>
> For an additional XFS-specific test I'd recommend:
>
> - creating a tmpfile, use src/godown to force the filesystem down and
> check that repair recoveres the unlinked inode list
> - creating a tmpfile, flinking it, use src/godown to force the
> filesystem down and check that repair does not have to recover
> the unlink inode list.
>
> On Wed, Dec 18, 2013 at 08:22:38AM +0800, Zhi Yong Wu wrote:
>> From: Zhi Yong Wu <wuzhy-23VcF4HTsmIX0ybBhKVfKdBPR1lH4CV8@public.gmane.org>
>>
>> HI, folks
>>
>> It's time to post the latest revision out, welcome to any constructive comment, thanks.
>>
>> If anyone is interested in playing with it, you can get this patchset from my dev git on github:
>> git://github.com/wuzhy/kernel.git xfs_tmpfile
>>
>> The patchset was tested against the code snippet from Andy Lutomirski and other test cases:
>> http://lwn.net/Articles/562296/
>> If you have any other better test cases, please let me know, thanks.
>>
>> #include <stdio.h>
>> #include <err.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <string.h>
>>
>> #define __O_TMPFILE 020000000
>> #define O_DIRECTORY 0200000
>> #define O_TMPFILE (__O_TMPFILE | O_DIRECTORY)
>> #define AT_EMPTY_PATH 0x1000
>>
>> int main(int argc, char **argv)
>> {
>> char buf[128];
>>
>> if (argc != 3)
>> errx(1, "Usage: flinktest PATH linkat|proc");
>>
>> int fd = open(".", O_TMPFILE | O_RDWR, 0600);
>> if (fd == -1)
>> err(1, "O_TMPFILE");
>> else
>> printf("fd #: %d\n", fd);
>>
>> write(fd, "test", 4);
>>
>> if (!strcmp(argv[2], "linkat")) {
>> if (linkat(fd, "", AT_FDCWD, argv[1], AT_EMPTY_PATH) != 0)
>> err(1, "linkat");
>> } else if (!strcmp(argv[2], "proc")) {
>> sprintf(buf, "/proc/self/fd/%d", fd);
>> if (linkat(AT_FDCWD, buf, AT_FDCWD, argv[1], AT_SYMLINK_FOLLOW) != 0)
>> err(1, "linkat");
>> } else {
>> errx(1, "invalid mode");
>> }
>>
>> return 0;
>> }
>>
>> Changelog from v1:
>> - Fixed one chunk of the comments from Christoph Hellwig and Jeff Liu.
>>
>> v1:
>> - Addressed the comments from Dave Chinner and Christoph Hellwig.
>>
>> Zhi Yong Wu (3):
>> xfs: factor prid related codes into xfs_get_initial_prid()
>> xfs: add O_TMPFILE support
>> xfs: allow linkat() on O_TMPFILE files
>>
>> fs/xfs/xfs_inode.c | 123 ++++++++++++++++++++++++++++++++++++++++++++--
>> fs/xfs/xfs_inode.h | 12 +++++
>> fs/xfs/xfs_iops.c | 16 ++++++
>> fs/xfs/xfs_shared.h | 4 +-
>> fs/xfs/xfs_symlink.c | 5 +--
>> fs/xfs/xfs_trans_resv.c | 55 +++++++++++++++++++--
>> fs/xfs/xfs_trans_resv.h | 2 +
>> 7 files changed, 202 insertions(+), 15 deletions(-)
>>
>> --
>> 1.7.6.5
>>
>> _______________________________________________
>> xfs mailing list
>> xfs-VZNHf3L845pBDgjK7y7TUQ@public.gmane.org
>> http://oss.sgi.com/mailman/listinfo/xfs
> ---end quoted text---
--
Regards,
Zhi Yong Wu
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 0/3] xfs: add O_TMPFILE support
2013-12-26 8:51 ` [PATCH v2 0/3] xfs: add O_TMPFILE support Christoph Hellwig
[not found] ` <20131226085134.GA32660-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
@ 2014-01-07 20:49 ` Ben Myers
2014-01-07 20:51 ` Christoph Hellwig
1 sibling, 1 reply; 4+ messages in thread
From: Ben Myers @ 2014-01-07 20:49 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Zhi Yong Wu, linux-api, Zhi Yong Wu, xfs
Hey Christoph,
On Thu, Dec 26, 2013 at 12:51:34AM -0800, Christoph Hellwig wrote:
> Both patches looks good to me,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> But now we need to make sure we can exercise it, which will need
> support for tmpfile and flink commands in xfs_io.
Are you suggesting that we hold off on this until such time as the tests are
completed?
Thanks,
Ben
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 0/3] xfs: add O_TMPFILE support
2014-01-07 20:49 ` Ben Myers
@ 2014-01-07 20:51 ` Christoph Hellwig
0 siblings, 0 replies; 4+ messages in thread
From: Christoph Hellwig @ 2014-01-07 20:51 UTC (permalink / raw)
To: Ben Myers; +Cc: Christoph Hellwig, Zhi Yong Wu, xfs, Zhi Yong Wu, linux-api
On Tue, Jan 07, 2014 at 02:49:38PM -0600, Ben Myers wrote:
> Are you suggesting that we hold off on this until such time as the tests are
> completed?
Yes. Code that doesn't come with proper test coverage usually tends to
be buggy.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-01-07 20:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1387326161-24530-1-git-send-email-zwu.kernel@gmail.com>
2013-12-26 8:51 ` [PATCH v2 0/3] xfs: add O_TMPFILE support Christoph Hellwig
[not found] ` <20131226085134.GA32660-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
2013-12-28 10:04 ` Zhi Yong Wu
2014-01-07 20:49 ` Ben Myers
2014-01-07 20:51 ` Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).