REVIEW: fix xfs_bmap -vp confusion

REVIEW: fix xfs_bmap -vp confusion

[Barry Naujok]

[-- Attachment #1: Type: text/plain, Size: 234 bytes --]

If xfs_bmap -vp is run on a file without unwritten extents
and the filesystem does not use sunit/swidth, the FLAGS
column is not shown. This leads to some confusion.

So, when -vp option is used, it will always print the
FLAGS column.

[-- Attachment #2: xfs_bmap_pflag.patch --]
[-- Type: application/octet-stream, Size: 1225 bytes --]

--- a/xfsprogs/io/bmap.c	2007-07-11 13:11:42.000000000 +1000
+++ b/xfsprogs/io/bmap.c	2007-07-11 12:34:33.519572709 +1000
@@ -74,6 +74,7 @@ bmap_f(
 	int			aflag = 0;
 	int			lflag = 0;
 	int			nflag = 0;
+	int			pflag = 0;
 	int			vflag = 0;
 	int			is_rt = 0;
 	int			bmv_iflags = 0;	/* flags for XFS_IOC_GETBMAPX */
@@ -99,6 +100,7 @@ bmap_f(
 			break;
 		case 'p':
 		/* report unwritten preallocated blocks */
+			pflag = 1;
 			bmv_iflags |= BMV_IF_PREALLOC;
 			break;
 		case 'v':	/* Verbose output */
@@ -289,7 +291,7 @@ bmap_f(
 			sunit = (fsgeo.sunit * fsgeo.blocksize) / BBSIZE;
 			swidth = (fsgeo.swidth * fsgeo.blocksize) / BBSIZE;
 		}
-		flg = sunit;
+		flg = sunit | pflag;
 
 		/*
 		 * Go through the extents and figure out the width
@@ -397,14 +399,14 @@ bmap_f(
 					printf("  ");
 				printf(" %*lld", tot_w,
 					(long long)map[i+1].bmv_length);
-				if (flg == FLG_NULL) {
+				if (flg == FLG_NULL && !pflag) {
 					printf("\n");
 				} else {
 					printf(" %-*.*o\n", NFLG, NFLG, flg);
 				}
 			}
 		}
-		if (flg && vflag > 1) {
+		if ((flg || pflag) && vflag > 1) {
 			printf(_(" FLAG Values:\n"));
 			printf(_("    %*.*o Unwritten preallocated extent\n"),
 				NFLG+1, NFLG+1, FLG_PRE);

Re: REVIEW: fix xfs_bmap -vp confusion

[David Chinner]

On Wed, Jul 11, 2007 at 01:18:20PM +1000, Barry Naujok wrote:
> If xfs_bmap -vp is run on a file without unwritten extents
> and the filesystem does not use sunit/swidth, the FLAGS
> column is not shown. This leads to some confusion.
> 
> So, when -vp option is used, it will always print the
> FLAGS column.

I note that it only prints the legend if a flag is set. I guess
this is ok, but maybe it would be more consistent to always
print the legend? Or even to make the legend optional (which
would make dumping more than one file more compact)?

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group

Re: REVIEW: fix xfs_bmap -vp confusion

[Barry Naujok]

On Wed, 11 Jul 2007 15:53:47 +1000, David Chinner <dgc@sgi.com> wrote:

> On Wed, Jul 11, 2007 at 01:18:20PM +1000, Barry Naujok wrote:
>> If xfs_bmap -vp is run on a file without unwritten extents
>> and the filesystem does not use sunit/swidth, the FLAGS
>> column is not shown. This leads to some confusion.
>>
>> So, when -vp option is used, it will always print the
>> FLAGS column.
>
> I note that it only prints the legend if a flag is set. I guess
> this is ok, but maybe it would be more consistent to always
> print the legend? Or even to make the legend optional (which
> would make dumping more than one file more compact)?
>
> Cheers,
>
> Dave.

The legend* is only shown with an extra -v option and the FLAGS
column displayed. The updated man pages posted out yesterday
documents this.

* I assume the legend you refer to is:
  FLAG Values:
     010000 Unwritten preallocated extent
     001000 Doesn't begin on stripe unit
     000100 Doesn't end   on stripe unit
     000010 Doesn't begin on stripe width
     000001 Doesn't end   on stripe width

Re: REVIEW: fix xfs_bmap -vp confusion

[David Chinner]

On Wed, Jul 11, 2007 at 04:04:05PM +1000, Barry Naujok wrote:
> On Wed, 11 Jul 2007 15:53:47 +1000, David Chinner <dgc@sgi.com> wrote:
> 
> >On Wed, Jul 11, 2007 at 01:18:20PM +1000, Barry Naujok wrote:
> >>If xfs_bmap -vp is run on a file without unwritten extents
> >>and the filesystem does not use sunit/swidth, the FLAGS
> >>column is not shown. This leads to some confusion.
> >>
> >>So, when -vp option is used, it will always print the
> >>FLAGS column.
> >
> >I note that it only prints the legend if a flag is set. I guess
> >this is ok, but maybe it would be more consistent to always
> >print the legend? Or even to make the legend optional (which
> >would make dumping more than one file more compact)?
> >
> >Cheers,
> >
> >Dave.
> 
> The legend* is only shown with an extra -v option and the FLAGS
> column displayed.

Ah, that'd be the > 1 check.

Looks good then....

> The updated man pages posted out yesterday documents this.

I haven't had time to read them yet.

Cheers,

Dave.
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group