xfs_io: deprecate the "-F" foreign flag

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.

Quoted 5 lines · 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.

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

Reviewed-by Christoph Hellwig <[email protected]>

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

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<[email protected]>

---

io/open.c +1 -9

index f1a6501..97631e2 100644
@@ -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);

On 02/02/12 11:35, Eric Sandeen wrote:

io/open.c open_f( · line 281

Quoted 11 lines

io/open.c +2 -2

@@ -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
[email protected]

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

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<[email protected]>

Signed-off-by Eric Sandeen<[email protected]>

---

io/open.c +0 -1

index ac39ccc..46538ba 100644
@@ -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"

Quoted 25 lines · 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<[email protected]>

Signed-off-by Eric Sandeen<[email protected]>

---

io/open.c +0 -1

index ac39ccc..46538ba 100644
@@ -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;
      }
 

io/open.c open_f( · line 279

Quoted 7 lines

io/open.c +1 -1

-    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

Quoted 26 lines · Mark Tinguely

Quoted 25 lines · 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<[email protected]>

Signed-off-by Eric Sandeen<[email protected]>

---

io/open.c +0 -1

index ac39ccc..46538ba 100644
@@ -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;
       }
 

Quoted 9 lines

io/open.c open_f( · line 279

Quoted 7 lines

io/open.c +1 -1

-    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?

Quoted 3 lines

Reviewed-by Mark Tinguely