[PATCH] xfs_io: deprecate the "-F" foreign flag

[PATCH] xfs_io: deprecate the "-F" foreign flag

[Eric Sandeen]

There's no real reason to force the user to specify "-F" for non-xfs
files, when we can just test for that after it's opened.

* Remove the -F flag from usage() & man pages, but still accept it.
* Set IO_FOREIGN when we open the file, if the fd tests as non-xfs.

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

diff --git a/io/init.c b/io/init.c
index a166ad1..f416acf 100644
--- a/io/init.c
+++ b/io/init.c
@@ -32,7 +32,7 @@ void
 usage(void)
 {
 	fprintf(stderr,
-		_("Usage: %s [-adFfmrRstx] [-p prog] [-c cmd]... file\n"),
+		_("Usage: %s [-adfmrRstx] [-p prog] [-c cmd]... file\n"),
 		progname);
 	exit(1);
 }
@@ -145,7 +145,7 @@ init(
 			flags |= IO_DIRECT;
 			break;
 		case 'F':
-			flags |= IO_FOREIGN;
+			/* Ignored / deprecated now, handled automatically */
 			break;
 		case 'f':
 			flags |= IO_CREAT;
@@ -188,9 +188,10 @@ init(
 	}
 
 	while (optind < argc) {
-		if ((c = openfile(argv[optind], flags & IO_FOREIGN ?
-					NULL : &geometry, flags, mode)) < 0)
+		if ((c = openfile(argv[optind], &geometry, flags, mode)) < 0)
 			exit(1);
+		if (!platform_test_xfs_fd(c))
+			flags |= IO_FOREIGN;
 		if (addfile(argv[optind], c, &geometry, flags) < 0)
 			exit(1);
 		optind++;
diff --git a/io/open.c b/io/open.c
index f1a6501..97631e2 100644
--- a/io/open.c
+++ b/io/open.c
@@ -163,17 +163,9 @@ openfile(
 		}
 	}
 
-	if (!geom)
+	if (!platform_test_xfs_fd(fd))
 		return fd;
 
-	if (!platform_test_xfs_fd(fd)) {
-		fprintf(stderr, _("%s: specified file "
-			"[\"%s\"] is not on an XFS filesystem\n"),
-			progname, path);
-		close(fd);
-		return -1;
-	}
-
 	if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom) < 0) {
 		perror("XFS_IOC_FSGEOMETRY");
 		close(fd);
@@ -282,10 +274,10 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
-			flags |= IO_FOREIGN;
+			/* Ignored / deprecated now, handled automatically */
 			break;
 		case 'a':
 			flags |= IO_APPEND;
@@ -328,11 +320,13 @@ open_f(
 	if (optind != argc - 1)
 		return command_usage(&open_cmd);
 
-	fd = openfile(argv[optind], flags & IO_FOREIGN ?
-					NULL : &geometry, flags, mode);
+	fd = openfile(argv[optind], &geometry, flags, mode);
 	if (fd < 0)
 		return 0;
 
+	if (!platform_test_xfs_fd(fd))
+		flags |= IO_FOREIGN;
+
 	addfile(argv[optind], fd, &geometry, flags);
 	return 0;
 }
diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index 6fc6bad..332718c 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -4,7 +4,7 @@ xfs_io \- debug the I/O path of an XFS filesystem
 .SH SYNOPSIS
 .B xfs_io
 [
-.B \-adFfmrRstx
+.B \-adfmrRstx
 ] [
 .B \-c
 .I cmd
@@ -37,12 +37,6 @@ Set the program name for prompts and some error messages,
 the default value is
 .BR xfs_io .
 .TP
-.B \-F
-Allow
-.I file
-to reside in non-XFS (foreign) filesystems.
-This mode has a restricted set of commands.
-.TP
 .B \-f
 Create
 .I file

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Christoph Hellwig]

On Thu, Feb 02, 2012 at 11:35:04AM -0600, Eric Sandeen wrote:
> There's no real reason to force the user to specify "-F" for non-xfs
> files, when we can just test for that after it's opened.
> 
> * Remove the -F flag from usage() & man pages, but still accept it.
> * Set IO_FOREIGN when we open the file, if the fd tests as non-xfs.

Looks good. We probably should kill the IO_FOREIGN eventually too, but
let's do the user facing part first.


Reviewed-by: Christoph Hellwig <hch@lst.de>

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Eric Sandeen]

On 2/3/12 9:04 AM, Christoph Hellwig wrote:
> On Thu, Feb 02, 2012 at 11:35:04AM -0600, Eric Sandeen wrote:
>> There's no real reason to force the user to specify "-F" for non-xfs
>> files, when we can just test for that after it's opened.
>>
>> * Remove the -F flag from usage() & man pages, but still accept it.
>> * Set IO_FOREIGN when we open the file, if the fd tests as non-xfs.
> 
> Looks good. We probably should kill the IO_FOREIGN eventually too, but
> let's do the user facing part first.

How would you want to handle non-xfs filesystems in the xfs-specific
commands?  Do the fd test each time?
 
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>


Thanks, I'll merge it.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Christoph Hellwig]

This actually breaks the following nreak tests 249 and 256 for me, can
you please look into fixing these ASAP?

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Eric Sandeen]

On 2/13/12 11:00 AM, Christoph Hellwig wrote:
> This actually breaks the following nreak tests 249 and 256 for me, can
> you please look into fixing these ASAP?

argh will do.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Mark Tinguely]

On 02/02/12 11:35, Eric Sandeen wrote:
> There's no real reason to force the user to specify "-F" for non-xfs
> files, when we can just test for that after it's opened.
>
> * Remove the -F flag from usage()&  man pages, but still accept it.
> * Set IO_FOREIGN when we open the file, if the fd tests as non-xfs.
>
> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
> ---



> diff --git a/io/open.c b/io/open.c
> index f1a6501..97631e2 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -163,17 +163,9 @@ openfile(
>   		}
>   	}
>
> -	if (!geom)
> +	if (!platform_test_xfs_fd(fd))
>   		return fd;
>
> -	if (!platform_test_xfs_fd(fd)) {
> -		fprintf(stderr, _("%s: specified file "
> -			"[\"%s\"] is not on an XFS filesystem\n"),
> -			progname, path);
> -		close(fd);
> -		return -1;
> -	}
> -
>   	if (xfsctl(path, fd, XFS_IOC_FSGEOMETRY, geom)<  0) {
>   		perror("XFS_IOC_FSGEOMETRY");
>   		close(fd);
> @@ -282,10 +274,10 @@ open_f(
>   		return 0;
>   	}
>
> -	while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
> +	while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
>   		switch (c) {
>   		case 'F':
> -			flags |= IO_FOREIGN;
> +			/* Ignored / deprecated now, handled automatically */
>   			break;


The "-F" is still in the command line open's help in open.

cxfs_io> help open
open [-acdrstx] [path] -- open the file specified by path

  opens a new file in the requested mode

  Example:
  'open -cd /tmp/data' - creates/opens data file read-write for direct IO

  Opens a file for subsequent use by all of the other xfs_io commands.
  With no arguments, open uses the stat command to show the current file.
  -F -- foreign filesystem file, disallow XFS-specific commands
  -a -- open with the O_APPEND flag (append-only mode)
  -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)
  -f -- open with O_CREAT (create the file if it doesn't exist)
  -m -- permissions to use in case a new file is created (default 0600)
  -n -- open with O_NONBLOCK
  -r -- open with O_RDONLY, the default is O_RDWR
  -s -- open with O_SYNC
  -t -- open with O_TRUNC (truncate the file to zero length if it exists)
  -R -- mark the file as a realtime XFS file immediately after opening it
  Note1: usually read/write direct IO requests must be blocksize aligned;
         some kernels, however, allow sectorsize alignment for direct IO.
  Note2: the bmap for non-regular files can be obtained provided the file
         was opened correctly (in particular, must be opened read-only).


The "F" was taken out of the command line's getopt(), so it can't be 
specified. I guess either put the "F" back in the the open.c getopt() or 
take it complete from the command line.

xfs_io> open -F somefile
open: invalid option -- 'F'
open [-acdrstx] [path] -- open the file specified by path

Thanks,

--Mark Tinguely
tinguely@sgi.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: deprecate the "-F" foreign flag

[Eric Sandeen]

On 3/8/12 3:39 PM, Mark Tinguely wrote:
> On 02/02/12 11:35, Eric Sandeen wrote:

...

>> @@ -282,10 +274,10 @@ open_f(
>>           return 0;
>>       }
>>
>> -    while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
>> +    while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
>>           switch (c) {
>>           case 'F':
>> -            flags |= IO_FOREIGN;
>> +            /* Ignored / deprecated now, handled automatically */
>>               break;
> 
> 
> The "-F" is still in the command line open's help in open.
> 
> cxfs_io> help open
> open [-acdrstx] [path] -- open the file specified by path
> 
>  opens a new file in the requested mode
> 
>  Example:
>  'open -cd /tmp/data' - creates/opens data file read-write for direct IO
> 
>  Opens a file for subsequent use by all of the other xfs_io commands.
>  With no arguments, open uses the stat command to show the current file.
>  -F -- foreign filesystem file, disallow XFS-specific commands
>  -a -- open with the O_APPEND flag (append-only mode)
>  -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)
>  -f -- open with O_CREAT (create the file if it doesn't exist)
>  -m -- permissions to use in case a new file is created (default 0600)
>  -n -- open with O_NONBLOCK
>  -r -- open with O_RDONLY, the default is O_RDWR
>  -s -- open with O_SYNC
>  -t -- open with O_TRUNC (truncate the file to zero length if it exists)
>  -R -- mark the file as a realtime XFS file immediately after opening it
>  Note1: usually read/write direct IO requests must be blocksize aligned;
>         some kernels, however, allow sectorsize alignment for direct IO.
>  Note2: the bmap for non-regular files can be obtained provided the file
>         was opened correctly (in particular, must be opened read-only).
> 
> 
> The "F" was taken out of the command line's getopt(), so it can't be
> specified. I guess either put the "F" back in the the open.c getopt()
> or take it complete from the command line.

Ah crud, sorry.  Patch shortly.

-Eric

> xfs_io> open -F somefile
> open: invalid option -- 'F'
> open [-acdrstx] [path] -- open the file specified by path
> 
> Thanks,
> 
> --Mark Tinguely
> tinguely@sgi.com
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

[PATCH] xfs_io: allow -F in open args

[Eric Sandeen]

Now that -F ("foreign") is automagic, we should no longer list
it in the help output for open, but we should still accept
it for compatibility; esp. as it is still in the case statement.
Oops.

Reported-by: Mark Tinguely <tinguely@sgi.com>
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/io/open.c b/io/open.c
index ac39ccc..46538ba 100644
--- a/io/open.c
+++ b/io/open.c
@@ -240,7 +240,6 @@ open_help(void)
 "\n"
 " Opens a file for subsequent use by all of the other xfs_io commands.\n"
 " With no arguments, open uses the stat command to show the current file.\n"
-" -F -- foreign filesystem file, disallow XFS-specific commands\n"
 " -a -- open with the O_APPEND flag (append-only mode)\n"
 " -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)\n"
 " -f -- open with O_CREAT (create the file if it doesn't exist)\n"
@@ -274,7 +273,7 @@ open_f(
 		return 0;
 	}
 
-	while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
+	while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
 		switch (c) {
 		case 'F':
 			/* Ignored / deprecated now, handled automatically */


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: allow -F in open args

[Mark Tinguely]

On 03/08/12 16:20, Eric Sandeen wrote:
> Now that -F ("foreign") is automagic, we should no longer list
> it in the help output for open, but we should still accept
> it for compatibility; esp. as it is still in the case statement.
> Oops.
>
> Reported-by: Mark Tinguely<tinguely@sgi.com>
> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
> ---
>
> diff --git a/io/open.c b/io/open.c
> index ac39ccc..46538ba 100644
> --- a/io/open.c
> +++ b/io/open.c
> @@ -240,7 +240,6 @@ open_help(void)
>   "\n"
>   " Opens a file for subsequent use by all of the other xfs_io commands.\n"
>   " With no arguments, open uses the stat command to show the current file.\n"
> -" -F -- foreign filesystem file, disallow XFS-specific commands\n"
>   " -a -- open with the O_APPEND flag (append-only mode)\n"
>   " -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)\n"
>   " -f -- open with O_CREAT (create the file if it doesn't exist)\n"
> @@ -274,7 +273,7 @@ open_f(
>   		return 0;
>   	}
>
> -	while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
> +	while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
>   		switch (c) {
>   		case 'F':
>   			/* Ignored / deprecated now, handled automatically */
>
>

Thank-you. Probably could take it out of the man page under the open I/O 
command now too.

Reviewed-by: Mark Tinguely


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: allow -F in open args

[Eric Sandeen]

On 3/8/12 4:37 PM, Mark Tinguely wrote:
> On 03/08/12 16:20, Eric Sandeen wrote:
>> Now that -F ("foreign") is automagic, we should no longer list
>> it in the help output for open, but we should still accept
>> it for compatibility; esp. as it is still in the case statement.
>> Oops.
>>
>> Reported-by: Mark Tinguely<tinguely@sgi.com>
>> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
>> ---
>>
>> diff --git a/io/open.c b/io/open.c
>> index ac39ccc..46538ba 100644
>> --- a/io/open.c
>> +++ b/io/open.c
>> @@ -240,7 +240,6 @@ open_help(void)
>>   "\n"
>>   " Opens a file for subsequent use by all of the other xfs_io commands.\n"
>>   " With no arguments, open uses the stat command to show the current file.\n"
>> -" -F -- foreign filesystem file, disallow XFS-specific commands\n"
>>   " -a -- open with the O_APPEND flag (append-only mode)\n"
>>   " -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)\n"
>>   " -f -- open with O_CREAT (create the file if it doesn't exist)\n"
>> @@ -274,7 +273,7 @@ open_f(
>>           return 0;
>>       }
>>
>> -    while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
>> +    while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
>>           switch (c) {
>>           case 'F':
>>               /* Ignored / deprecated now, handled automatically */
>>
>>
> 
> Thank-you. Probably could take it out of the man page under the open I/O command now too.

Gah.  With your blessing I'll commit that as well?

> Reviewed-by: Mark Tinguely
> 
> 

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [PATCH] xfs_io: allow -F in open args

[Mark Tinguely]

On 03/08/12 16:37, Eric Sandeen wrote:
> On 3/8/12 4:37 PM, Mark Tinguely wrote:
>> On 03/08/12 16:20, Eric Sandeen wrote:
>>> Now that -F ("foreign") is automagic, we should no longer list
>>> it in the help output for open, but we should still accept
>>> it for compatibility; esp. as it is still in the case statement.
>>> Oops.
>>>
>>> Reported-by: Mark Tinguely<tinguely@sgi.com>
>>> Signed-off-by: Eric Sandeen<sandeen@redhat.com>
>>> ---
>>>
>>> diff --git a/io/open.c b/io/open.c
>>> index ac39ccc..46538ba 100644
>>> --- a/io/open.c
>>> +++ b/io/open.c
>>> @@ -240,7 +240,6 @@ open_help(void)
>>>    "\n"
>>>    " Opens a file for subsequent use by all of the other xfs_io commands.\n"
>>>    " With no arguments, open uses the stat command to show the current file.\n"
>>> -" -F -- foreign filesystem file, disallow XFS-specific commands\n"
>>>    " -a -- open with the O_APPEND flag (append-only mode)\n"
>>>    " -d -- open with O_DIRECT (non-buffered IO, note alignment constraints)\n"
>>>    " -f -- open with O_CREAT (create the file if it doesn't exist)\n"
>>> @@ -274,7 +273,7 @@ open_f(
>>>            return 0;
>>>        }
>>>
>>> -    while ((c = getopt(argc, argv, "Racdfm:nrstx")) != EOF) {
>>> +    while ((c = getopt(argc, argv, "FRacdfm:nrstx")) != EOF) {
>>>            switch (c) {
>>>            case 'F':
>>>                /* Ignored / deprecated now, handled automatically */
>>>
>>>
>>
>> Thank-you. Probably could take it out of the man page under the open I/O command now too.
>
> Gah.  With your blessing I'll commit that as well?
>
>> Reviewed-by: Mark Tinguely
>>
>>
>

Yes, and I should have saw it before too. Good idea to remove the flag.

--Mark Tinguely

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs