linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] xfs_io: add the ability to do an O_PATH open
@ 2018-05-16 13:27 Jeff Layton
  2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Jeff Layton @ 2018-05-16 13:27 UTC (permalink / raw)
  To: linux-xfs

From: Jeff Layton <jlayton@redhat.com>

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 io/file.c         | 3 ++-
 io/init.c         | 5 ++++-
 io/io.h           | 1 +
 io/open.c         | 7 ++++++-
 io/stat.c         | 4 +++-
 man/man8/xfs_io.8 | 3 +++
 6 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/io/file.c b/io/file.c
index 349b19cdc420..ff80652b183b 100644
--- a/io/file.c
+++ b/io/file.c
@@ -44,7 +44,8 @@ print_fileio(
 		file->flags & IO_REALTIME ? _(",real-time") : "",
 		file->flags & IO_APPEND ? _(",append-only") : "",
 		file->flags & IO_NONBLOCK ? _(",non-block") : "",
-		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
+		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
+		file->flags & IO_PATH ? _(",path") : "");
 }
 
 int
diff --git a/io/init.c b/io/init.c
index 0336c9623beb..d4e84e82a77e 100644
--- a/io/init.c
+++ b/io/init.c
@@ -154,7 +154,7 @@ init(
 	gettimeofday(&stopwatch, NULL);
 
 	fs_table_initialise(0, NULL, 0, NULL);
-	while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
+	while ((c = getopt(argc, argv, "ac:C:dFfim:Pp:nrRstTVx")) != EOF) {
 		switch (c) {
 		case 'a':
 			flags |= IO_APPEND;
@@ -200,6 +200,9 @@ init(
 		case 't':
 			flags |= IO_TRUNC;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
 		case 'R':
 			flags |= IO_REALTIME;
 			break;
diff --git a/io/io.h b/io/io.h
index a26763610877..0acc332b5dbb 100644
--- a/io/io.h
+++ b/io/io.h
@@ -40,6 +40,7 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
 
 /*
  * Regular file I/O control
diff --git a/io/open.c b/io/open.c
index 2cce0455263a..ba73f1d0361f 100644
--- a/io/open.c
+++ b/io/open.c
@@ -74,6 +74,8 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {
@@ -216,7 +218,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FRTacdfm:nPrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */
@@ -250,6 +252,9 @@ open_f(
 		case 't':
 			flags |= IO_TRUNC;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
 		case 'R':
 		case 'x':	/* backwards compatibility */
 			flags |= IO_REALTIME;
diff --git a/io/stat.c b/io/stat.c
index 41d421525791..2db2736359e0 100644
--- a/io/stat.c
+++ b/io/stat.c
@@ -100,7 +100,9 @@ void print_file_info(void)
 		file->flags & IO_REALTIME ? _(",real-time") : "",
 		file->flags & IO_APPEND ? _(",append-only") : "",
 		file->flags & IO_NONBLOCK ? _(",non-block") : "",
-		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
+		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
+		file->flags & IO_PATH ? _(",path") : "");
+
 }
 
 void print_xfs_info(int verbose)
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index c3ab532da03f..89e8e4be460e 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -153,6 +153,9 @@ truncates on open (O_TRUNC).
 .B \-n
 opens in non-blocking mode if possible (O_NONBLOCK).
 .TP
+.B \-P
+opens the file for location only (O_PATH).
+.TP
 .B \-T
 create a temporary file not linked into the filesystem namespace
 (O_TMPFILE).  The pathname passed must refer to a directory which
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] xfs_io: syncfs can fail
  2018-05-16 13:27 [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Jeff Layton
@ 2018-05-16 13:27 ` Jeff Layton
  2018-05-16 14:26   ` Eric Sandeen
  2018-05-16 17:17   ` [PATCH v2] " Jeff Layton
  2018-05-16 14:32 ` [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Eric Sandeen
  2018-05-16 17:14 ` [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW Jeff Layton
  2 siblings, 2 replies; 9+ messages in thread
From: Jeff Layton @ 2018-05-16 13:27 UTC (permalink / raw)
  To: linux-xfs

From: Jeff Layton <jlayton@redhat.com>

syncfs can return an error. Report one if it does.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 io/sync.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/io/sync.c b/io/sync.c
index c77263804a35..868cea7c051e 100644
--- a/io/sync.c
+++ b/io/sync.c
@@ -41,8 +41,8 @@ syncfs_f(
 	int			argc,
 	char			**argv)
 {
-	/* syncfs can't fail */
-	syncfs(file->fd);
+	if (syncfs(file->fd) < 0)
+		perror("syncfs");
 	return 0;
 }
 #endif
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] xfs_io: syncfs can fail
  2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
@ 2018-05-16 14:26   ` Eric Sandeen
  2018-05-16 17:17   ` [PATCH v2] " Jeff Layton
  1 sibling, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2018-05-16 14:26 UTC (permalink / raw)
  To: Jeff Layton, linux-xfs

On 5/16/18 8:27 AM, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> syncfs can return an error. Report one if it does.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> ---
>   io/sync.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/io/sync.c b/io/sync.c
> index c77263804a35..868cea7c051e 100644
> --- a/io/sync.c
> +++ b/io/sync.c
> @@ -41,8 +41,8 @@ syncfs_f(
>   	int			argc,
>   	char			**argv)
>   {
> -	/* syncfs can't fail */
> -	syncfs(file->fd);
> +	if (syncfs(file->fd) < 0)
> +		perror("syncfs");

In light of dchinner's recent reminder on another of my patches, this
should also set exitcode=1 here, so that the overall xfs_io exit code
will be non-zero if syncfs fails.

(I could just add that on merge if you prefer... assuming I remember)

Thanks,
-Eric

>   	return 0;
>   }
>   #endif
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] xfs_io: add the ability to do an O_PATH open
  2018-05-16 13:27 [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Jeff Layton
  2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
@ 2018-05-16 14:32 ` Eric Sandeen
  2018-05-16 17:14 ` [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW Jeff Layton
  2 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2018-05-16 14:32 UTC (permalink / raw)
  To: Jeff Layton, linux-xfs

On 5/16/18 8:27 AM, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

Thanks.  dhowells also sent something similar long ago,

[PATCH] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW

But you have some things here he missed (and he has some things
you missed, like command help output)

Could you maybe compare this to his patch, and resend a superset
with both SOBs if that seems appropriate?

Thanks,
-Eric

> ---
>   io/file.c         | 3 ++-
>   io/init.c         | 5 ++++-
>   io/io.h           | 1 +
>   io/open.c         | 7 ++++++-
>   io/stat.c         | 4 +++-
>   man/man8/xfs_io.8 | 3 +++
>   6 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/io/file.c b/io/file.c
> index 349b19cdc420..ff80652b183b 100644
> --- a/io/file.c
> +++ b/io/file.c
> @@ -44,7 +44,8 @@ print_fileio(
>   		file->flags & IO_REALTIME ? _(",real-time") : "",
>   		file->flags & IO_APPEND ? _(",append-only") : "",
>   		file->flags & IO_NONBLOCK ? _(",non-block") : "",
> -		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
> +		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
> +		file->flags & IO_PATH ? _(",path") : "");
>   }
>   
>   int
> diff --git a/io/init.c b/io/init.c
> index 0336c9623beb..d4e84e82a77e 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -154,7 +154,7 @@ init(
>   	gettimeofday(&stopwatch, NULL);
>   
>   	fs_table_initialise(0, NULL, 0, NULL);
> -	while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
> +	while ((c = getopt(argc, argv, "ac:C:dFfim:Pp:nrRstTVx")) != EOF) {
>   		switch (c) {
>   		case 'a':
>   			flags |= IO_APPEND;
> @@ -200,6 +200,9 @@ init(
>   		case 't':
>   			flags |= IO_TRUNC;
>   			break;
> +		case 'P':
> +			flags |= IO_PATH;
> +			break;
>   		case 'R':
>   			flags |= IO_REALTIME;
>   			break;
> diff --git a/io/io.h b/io/io.h
> index a26763610877..0acc332b5dbb 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -40,6 +40,7 @@
>   #define IO_FOREIGN	(1<<7)
>   #define IO_NONBLOCK	(1<<8)
>   #define IO_TMPFILE	(1<<9)
> +#define IO_PATH		(1<<10)
>   
>   /*
>    * Regular file I/O control
> diff --git a/io/open.c b/io/open.c
> index 2cce0455263a..ba73f1d0361f 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -74,6 +74,8 @@ openfile(
>   		oflags |= O_NONBLOCK;
>   	if (flags & IO_TMPFILE)
>   		oflags |= O_TMPFILE;
> +	if (flags & IO_PATH)
> +		oflags |= O_PATH;
>   
>   	fd = open(path, oflags, mode);
>   	if (fd < 0) {
> @@ -216,7 +218,7 @@ open_f(
>   		return 0;
>   	}
>   
> -	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
> +	while ((c = getopt(argc, argv, "FRTacdfm:nPrstx")) != EOF) {
>   		switch (c) {
>   		case 'F':
>   			/* Ignored / deprecated now, handled automatically */
> @@ -250,6 +252,9 @@ open_f(
>   		case 't':
>   			flags |= IO_TRUNC;
>   			break;
> +		case 'P':
> +			flags |= IO_PATH;
> +			break;
>   		case 'R':
>   		case 'x':	/* backwards compatibility */
>   			flags |= IO_REALTIME;
> diff --git a/io/stat.c b/io/stat.c
> index 41d421525791..2db2736359e0 100644
> --- a/io/stat.c
> +++ b/io/stat.c
> @@ -100,7 +100,9 @@ void print_file_info(void)
>   		file->flags & IO_REALTIME ? _(",real-time") : "",
>   		file->flags & IO_APPEND ? _(",append-only") : "",
>   		file->flags & IO_NONBLOCK ? _(",non-block") : "",
> -		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
> +		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
> +		file->flags & IO_PATH ? _(",path") : "");
> +
>   }
>   
>   void print_xfs_info(int verbose)
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index c3ab532da03f..89e8e4be460e 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -153,6 +153,9 @@ truncates on open (O_TRUNC).
>   .B \-n
>   opens in non-blocking mode if possible (O_NONBLOCK).
>   .TP
> +.B \-P
> +opens the file for location only (O_PATH).
> +.TP
>   .B \-T
>   create a temporary file not linked into the filesystem namespace
>   (O_TMPFILE).  The pathname passed must refer to a directory which
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW
  2018-05-16 13:27 [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Jeff Layton
  2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
  2018-05-16 14:32 ` [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Eric Sandeen
@ 2018-05-16 17:14 ` Jeff Layton
  2018-05-23 21:51   ` Eric Sandeen
  2 siblings, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2018-05-16 17:14 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs, dhowells

From: David Howells <dhowells@redhat.com>

Allow "open -P" to specify O_PATH so that paths which would otherwise be
unopenable might be opened for stat()'ing.  Such things include files that
would incur an access error or device files for which no corresponding
driver is available.

Allow "-L" to be given in conjunction with O_PATH to specify O_NOFOLLOW
also.

We also have to avoid calling xfsctl() if O_PATH is given as ioctls are
forbidden on such fds.  This means we cannot retrieve the geometry
information on an XFS filesystem, so the record gets cleared instead.  For
the moment, only the xfsctl() calls in the 'open' command are
conditionalised.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 io/file.c         |  6 ++++--
 io/init.c         |  8 +++++++-
 io/io.h           |  2 ++
 io/open.c         | 29 +++++++++++++++++++++++++----
 man/man8/xfs_io.8 | 12 +++++++++++-
 5 files changed, 49 insertions(+), 8 deletions(-)

v3: print new options in print_fileio
    handle -P and -L in init()

diff --git a/io/file.c b/io/file.c
index 349b19cdc420..b7edf6ef841f 100644
--- a/io/file.c
+++ b/io/file.c
@@ -35,7 +35,7 @@ print_fileio(
 	int		index,
 	int		braces)
 {
-	printf(_("%c%03d%c %-14s (%s,%s,%s,%s%s%s%s%s)\n"),
+	printf(_("%c%03d%c %-14s (%s,%s,%s,%s%s%s%s%s%s%s)\n"),
 		braces? '[' : ' ', index, braces? ']' : ' ', file->name,
 		file->flags & IO_FOREIGN ? _("foreign") : _("xfs"),
 		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
@@ -44,7 +44,9 @@ print_fileio(
 		file->flags & IO_REALTIME ? _(",real-time") : "",
 		file->flags & IO_APPEND ? _(",append-only") : "",
 		file->flags & IO_NONBLOCK ? _(",non-block") : "",
-		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
+		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
+		file->flags & IO_PATH ? _(",path") : "",
+		file->flags & IO_NOFOLLOW ? _(",nofollow") : "");
 }
 
 int
diff --git a/io/init.c b/io/init.c
index 0336c9623beb..d54a2fb957cf 100644
--- a/io/init.c
+++ b/io/init.c
@@ -154,7 +154,7 @@ init(
 	gettimeofday(&stopwatch, NULL);
 
 	fs_table_initialise(0, NULL, 0, NULL);
-	while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
+	while ((c = getopt(argc, argv, "ac:C:dFfiLm:p:PnrRstTVx")) != EOF) {
 		switch (c) {
 		case 'a':
 			flags |= IO_APPEND;
@@ -200,6 +200,12 @@ init(
 		case 't':
 			flags |= IO_TRUNC;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
+		case 'L':
+			flags |= IO_NOFOLLOW;
+			break;
 		case 'R':
 			flags |= IO_REALTIME;
 			break;
diff --git a/io/io.h b/io/io.h
index a26763610877..f2569588b28f 100644
--- a/io/io.h
+++ b/io/io.h
@@ -40,6 +40,8 @@
 #define IO_FOREIGN	(1<<7)
 #define IO_NONBLOCK	(1<<8)
 #define IO_TMPFILE	(1<<9)
+#define IO_PATH		(1<<10)
+#define IO_NOFOLLOW	(1<<11)
 
 /*
  * Regular file I/O control
diff --git a/io/open.c b/io/open.c
index 2cce0455263a..7cda47cc85dd 100644
--- a/io/open.c
+++ b/io/open.c
@@ -74,6 +74,10 @@ openfile(
 		oflags |= O_NONBLOCK;
 	if (flags & IO_TMPFILE)
 		oflags |= O_TMPFILE;
+	if (flags & IO_PATH)
+		oflags |= O_PATH;
+	if (flags & IO_NOFOLLOW)
+		oflags |= O_NOFOLLOW;
 
 	fd = open(path, oflags, mode);
 	if (fd < 0) {
@@ -97,13 +101,16 @@ openfile(
 	if (!geom || !platform_test_xfs_fd(fd))
 		return fd;
 
-	if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
+	if (flags & IO_PATH) {
+		/* Can't call ioctl() on O_PATH fds */
+		memset(geom, 0, sizeof(*geom));
+	} else if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
 		perror("XFS_IOC_FSGEOMETRY");
 		close(fd);
 		return -1;
 	}
 
-	if (!(flags & IO_READONLY) && (flags & IO_REALTIME)) {
+	if (!(flags & (IO_READONLY | IO_PATH)) && (flags & IO_REALTIME)) {
 		struct fsxattr	attr;
 
 		if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) {
@@ -191,6 +198,8 @@ open_help(void)
 " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
 " -R -- mark the file as a realtime XFS file immediately after opening it\n"
 " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
+" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
+" -L -- open with O_NOFOLLOW (don't follow symlink)\n"
 " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
 "        some kernels, however, allow sectorsize alignment for direct IO.\n"
 " Note2: the bmap for non-regular files can be obtained provided the file\n"
@@ -216,7 +225,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FLPRTacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */
@@ -257,6 +266,12 @@ open_f(
 		case 'T':
 			flags |= IO_TMPFILE;
 			break;
+		case 'P':
+			flags |= IO_PATH;
+			break;
+		case 'L':
+			flags |= IO_NOFOLLOW;
+			break;
 		default:
 			return command_usage(&open_cmd);
 		}
@@ -270,6 +285,12 @@ open_f(
 		return -1;
 	}
 
+	if ((flags & (IO_PATH|IO_NOFOLLOW)) &&
+	    (flags & ~(IO_PATH|IO_NOFOLLOW))) {
+		fprintf(stderr, _("-P and -L are incompatible with the other options\n"));
+		return -1;
+	}
+
 	fd = openfile(argv[optind], &geometry, flags, mode, &fsp);
 	if (fd < 0)
 		return 0;
@@ -785,7 +806,7 @@ open_init(void)
 	open_cmd.argmax = -1;
 	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
 			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
-	open_cmd.args = _("[-acdrstxT] [-m mode] [path]");
+	open_cmd.args = _("[-acdrstxRTPL] [-m mode] [path]");
 	open_cmd.oneline = _("open the file specified by path");
 	open_cmd.help = open_help;
 
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index c3ab532da03f..a49d067100fe 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -122,7 +122,7 @@ command for more details on any command.
 Display a list of all open files and (optionally) switch to an alternate
 current open file.
 .TP
-.BI "open [[ \-acdfrstRT ] " path " ]"
+.BI "open [[ \-acdfrstRTPL ] " path " ]"
 Closes the current file, and opens the file specified by
 .I path
 instead. Without any arguments, displays statistics about the current
@@ -164,6 +164,16 @@ option.
 .B \-R
 marks the file as a realtime XFS file after
 opening it, if it is not already marked as such.
+.TP
+.B \-P
+opens the path as a referent only (O_PATH).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-L .
+.TP
+.B \-L
+doesn't follow symlinks (O_NOFOLLOW).  This is incompatible with other
+flags specifying other O_xxx flags apart from
+.BR \-P .
 .PD
 .RE
 .TP
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2] xfs_io: syncfs can fail
  2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
  2018-05-16 14:26   ` Eric Sandeen
@ 2018-05-16 17:17   ` Jeff Layton
  2018-05-23 21:37     ` Eric Sandeen
  1 sibling, 1 reply; 9+ messages in thread
From: Jeff Layton @ 2018-05-16 17:17 UTC (permalink / raw)
  To: sandeen; +Cc: linux-xfs

From: Jeff Layton <jlayton@redhat.com>

syncfs can return an error. Report one if it does. Also, ensure that
xfs_io will exit with a non-zero status in that case.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 io/sync.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

v2: also set exitcode when reporting error

diff --git a/io/sync.c b/io/sync.c
index c77263804a35..20ab50a7fcd4 100644
--- a/io/sync.c
+++ b/io/sync.c
@@ -41,8 +41,10 @@ syncfs_f(
 	int			argc,
 	char			**argv)
 {
-	/* syncfs can't fail */
-	syncfs(file->fd);
+	if (syncfs(file->fd) < 0) {
+		perror("syncfs");
+		exitcode = 1;
+	}
 	return 0;
 }
 #endif
-- 
2.17.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] xfs_io: syncfs can fail
  2018-05-16 17:17   ` [PATCH v2] " Jeff Layton
@ 2018-05-23 21:37     ` Eric Sandeen
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2018-05-23 21:37 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-xfs

On 5/16/18 12:17 PM, Jeff Layton wrote:
> From: Jeff Layton <jlayton@redhat.com>
> 
> syncfs can return an error. Report one if it does. Also, ensure that
> xfs_io will exit with a non-zero status in that case.
> 
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>   io/sync.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> v2: also set exitcode when reporting error
> 
> diff --git a/io/sync.c b/io/sync.c
> index c77263804a35..20ab50a7fcd4 100644
> --- a/io/sync.c
> +++ b/io/sync.c
> @@ -41,8 +41,10 @@ syncfs_f(
>   	int			argc,
>   	char			**argv)
>   {
> -	/* syncfs can't fail */
> -	syncfs(file->fd);
> +	if (syncfs(file->fd) < 0) {
> +		perror("syncfs");
> +		exitcode = 1;
> +	}
>   	return 0;
>   }
>   #endif
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW
  2018-05-16 17:14 ` [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW Jeff Layton
@ 2018-05-23 21:51   ` Eric Sandeen
  2018-05-23 22:03     ` Eric Sandeen
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Sandeen @ 2018-05-23 21:51 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-xfs, dhowells



On 5/16/18 12:14 PM, Jeff Layton wrote:
> From: David Howells <dhowells@redhat.com>
> 
> Allow "open -P" to specify O_PATH so that paths which would otherwise be
> unopenable might be opened for stat()'ing.  Such things include files that
> would incur an access error or device files for which no corresponding
> driver is available.
> 
> Allow "-L" to be given in conjunction with O_PATH to specify O_NOFOLLOW
> also.
> 
> We also have to avoid calling xfsctl() if O_PATH is given as ioctls are
> forbidden on such fds.  This means we cannot retrieve the geometry
> information on an XFS filesystem, so the record gets cleared instead.  For
> the moment, only the xfsctl() calls in the 'open' command are
> conditionalised.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Jeff Layton <jlayton@redhat.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>   io/file.c         |  6 ++++--
>   io/init.c         |  8 +++++++-
>   io/io.h           |  2 ++
>   io/open.c         | 29 +++++++++++++++++++++++++----
>   man/man8/xfs_io.8 | 12 +++++++++++-
>   5 files changed, 49 insertions(+), 8 deletions(-)
> 
> v3: print new options in print_fileio
>      handle -P and -L in init()
> 
> diff --git a/io/file.c b/io/file.c
> index 349b19cdc420..b7edf6ef841f 100644
> --- a/io/file.c
> +++ b/io/file.c
> @@ -35,7 +35,7 @@ print_fileio(
>   	int		index,
>   	int		braces)
>   {
> -	printf(_("%c%03d%c %-14s (%s,%s,%s,%s%s%s%s%s)\n"),
> +	printf(_("%c%03d%c %-14s (%s,%s,%s,%s%s%s%s%s%s%s)\n"),
>   		braces? '[' : ' ', index, braces? ']' : ' ', file->name,
>   		file->flags & IO_FOREIGN ? _("foreign") : _("xfs"),
>   		file->flags & IO_OSYNC ? _("sync") : _("non-sync"),
> @@ -44,7 +44,9 @@ print_fileio(
>   		file->flags & IO_REALTIME ? _(",real-time") : "",
>   		file->flags & IO_APPEND ? _(",append-only") : "",
>   		file->flags & IO_NONBLOCK ? _(",non-block") : "",
> -		file->flags & IO_TMPFILE ? _(",tmpfile") : "");
> +		file->flags & IO_TMPFILE ? _(",tmpfile") : "",
> +		file->flags & IO_PATH ? _(",path") : "",
> +		file->flags & IO_NOFOLLOW ? _(",nofollow") : "");
>   }
>   
>   int
> diff --git a/io/init.c b/io/init.c
> index 0336c9623beb..d54a2fb957cf 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -154,7 +154,7 @@ init(
>   	gettimeofday(&stopwatch, NULL);
>   
>   	fs_table_initialise(0, NULL, 0, NULL);
> -	while ((c = getopt(argc, argv, "ac:C:dFfim:p:nrRstTVx")) != EOF) {
> +	while ((c = getopt(argc, argv, "ac:C:dFfiLm:p:PnrRstTVx")) != EOF) {
>   		switch (c) {
>   		case 'a':
>   			flags |= IO_APPEND;
> @@ -200,6 +200,12 @@ init(
>   		case 't':
>   			flags |= IO_TRUNC;
>   			break;
> +		case 'P':
> +			flags |= IO_PATH;
> +			break;
> +		case 'L':
> +			flags |= IO_NOFOLLOW;
> +			break;
>   		case 'R':
>   			flags |= IO_REALTIME;
>   			break;
> diff --git a/io/io.h b/io/io.h
> index a26763610877..f2569588b28f 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -40,6 +40,8 @@
>   #define IO_FOREIGN	(1<<7)
>   #define IO_NONBLOCK	(1<<8)
>   #define IO_TMPFILE	(1<<9)
> +#define IO_PATH		(1<<10)
> +#define IO_NOFOLLOW	(1<<11)
>   
>   /*
>    * Regular file I/O control
> diff --git a/io/open.c b/io/open.c
> index 2cce0455263a..7cda47cc85dd 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -74,6 +74,10 @@ openfile(
>   		oflags |= O_NONBLOCK;
>   	if (flags & IO_TMPFILE)
>   		oflags |= O_TMPFILE;
> +	if (flags & IO_PATH)
> +		oflags |= O_PATH;
> +	if (flags & IO_NOFOLLOW)
> +		oflags |= O_NOFOLLOW;
>   
>   	fd = open(path, oflags, mode);
>   	if (fd < 0) {
> @@ -97,13 +101,16 @@ openfile(
>   	if (!geom || !platform_test_xfs_fd(fd))
>   		return fd;
>   
> -	if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
> +	if (flags & IO_PATH) {
> +		/* Can't call ioctl() on O_PATH fds */
> +		memset(geom, 0, sizeof(*geom));
> +	} else if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
>   		perror("XFS_IOC_FSGEOMETRY");
>   		close(fd);
>   		return -1;
>   	}
>   
> -	if (!(flags & IO_READONLY) && (flags & IO_REALTIME)) {
> +	if (!(flags & (IO_READONLY | IO_PATH)) && (flags & IO_REALTIME)) {
>   		struct fsxattr	attr;
>   
>   		if (xfsctl(path, fd, FS_IOC_FSGETXATTR, &attr) < 0) {
> @@ -191,6 +198,8 @@ open_help(void)
>   " -t -- open with O_TRUNC (truncate the file to zero length if it exists)\n"
>   " -R -- mark the file as a realtime XFS file immediately after opening it\n"
>   " -T -- open with O_TMPFILE (create a file not visible in the namespace)\n"
> +" -P -- open with O_PATH (create an fd that is merely a location reference)\n"
> +" -L -- open with O_NOFOLLOW (don't follow symlink)\n"
>   " Note1: usually read/write direct IO requests must be blocksize aligned;\n"
>   "        some kernels, however, allow sectorsize alignment for direct IO.\n"
>   " Note2: the bmap for non-regular files can be obtained provided the file\n"
> @@ -216,7 +225,7 @@ open_f(
>   		return 0;
>   	}
>   
> -	while ((c = getopt(argc, argv, "FRTacdfm:nrstx")) != EOF) {
> +	while ((c = getopt(argc, argv, "FLPRTacdfm:nrstx")) != EOF) {
>   		switch (c) {
>   		case 'F':
>   			/* Ignored / deprecated now, handled automatically */
> @@ -257,6 +266,12 @@ open_f(
>   		case 'T':
>   			flags |= IO_TMPFILE;
>   			break;
> +		case 'P':
> +			flags |= IO_PATH;
> +			break;
> +		case 'L':
> +			flags |= IO_NOFOLLOW;
> +			break;
>   		default:
>   			return command_usage(&open_cmd);
>   		}
> @@ -270,6 +285,12 @@ open_f(
>   		return -1;
>   	}
>   
> +	if ((flags & (IO_PATH|IO_NOFOLLOW)) &&
> +	    (flags & ~(IO_PATH|IO_NOFOLLOW))) {
> +		fprintf(stderr, _("-P and -L are incompatible with the other options\n"));
> +		return -1;
> +	}
> +
>   	fd = openfile(argv[optind], &geometry, flags, mode, &fsp);
>   	if (fd < 0)
>   		return 0;
> @@ -785,7 +806,7 @@ open_init(void)
>   	open_cmd.argmax = -1;
>   	open_cmd.flags = CMD_NOMAP_OK | CMD_NOFILE_OK |
>   			 CMD_FOREIGN_OK | CMD_FLAG_ONESHOT;
> -	open_cmd.args = _("[-acdrstxT] [-m mode] [path]");
> +	open_cmd.args = _("[-acdrstxRTPL] [-m mode] [path]");
>   	open_cmd.oneline = _("open the file specified by path");
>   	open_cmd.help = open_help;
>   
> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index c3ab532da03f..a49d067100fe 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -122,7 +122,7 @@ command for more details on any command.
>   Display a list of all open files and (optionally) switch to an alternate
>   current open file.
>   .TP
> -.BI "open [[ \-acdfrstRT ] " path " ]"
> +.BI "open [[ \-acdfrstRTPL ] " path " ]"
>   Closes the current file, and opens the file specified by
>   .I path
>   instead. Without any arguments, displays statistics about the current
> @@ -164,6 +164,16 @@ option.
>   .B \-R
>   marks the file as a realtime XFS file after
>   opening it, if it is not already marked as such.
> +.TP
> +.B \-P
> +opens the path as a referent only (O_PATH).  This is incompatible with other
> +flags specifying other O_xxx flags apart from
> +.BR \-L .
> +.TP
> +.B \-L
> +doesn't follow symlinks (O_NOFOLLOW).  This is incompatible with other
> +flags specifying other O_xxx flags apart from
> +.BR \-P .
>   .PD
>   .RE
>   .TP
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW
  2018-05-23 21:51   ` Eric Sandeen
@ 2018-05-23 22:03     ` Eric Sandeen
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Sandeen @ 2018-05-23 22:03 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-xfs, dhowells



On 5/23/18 4:51 PM, Eric Sandeen wrote:
> On 5/16/18 12:14 PM, Jeff Layton wrote:
>> From: David Howells <dhowells@redhat.com>
>>
>> Allow "open -P" to specify O_PATH so that paths which would otherwise be
>> unopenable might be opened for stat()'ing.  Such things include files that
>> would incur an access error or device files for which no corresponding
>> driver is available.
>>
>> Allow "-L" to be given in conjunction with O_PATH to specify O_NOFOLLOW
>> also.
>>
>> We also have to avoid calling xfsctl() if O_PATH is given as ioctls are
>> forbidden on such fds.  This means we cannot retrieve the geometry
>> information on an XFS filesystem, so the record gets cleared instead.  For
>> the moment, only the xfsctl() calls in the 'open' command are
>> conditionalised.
>>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> Signed-off-by: Jeff Layton <jlayton@redhat.com>
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

hm actually I'll need to locally define O_PATH so it builds on older
systems:

+#ifndef O_PATH
+#if defined __alpha__
+#define O_PATH          040000000
+#elif defined(__hppa__)
+#define O_PATH          020000000
+#elif defined(__sparc__)
+#define O_PATH          0x1000000
+#else
+#define O_PATH          010000000
+#endif
+#endif /* O_PATH */

... like we do for O_TMPFILE.  I can do that on the way in.

-Eric

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-05-23 22:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-16 13:27 [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Jeff Layton
2018-05-16 13:27 ` [PATCH 2/2] xfs_io: syncfs can fail Jeff Layton
2018-05-16 14:26   ` Eric Sandeen
2018-05-16 17:17   ` [PATCH v2] " Jeff Layton
2018-05-23 21:37     ` Eric Sandeen
2018-05-16 14:32 ` [PATCH 1/2] xfs_io: add the ability to do an O_PATH open Eric Sandeen
2018-05-16 17:14 ` [PATCH v3] xfs_io: Allow -P and -L to be given to open for O_PATH and O_NOFOLLOW Jeff Layton
2018-05-23 21:51   ` Eric Sandeen
2018-05-23 22:03     ` Eric Sandeen

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).