Re: [PATCH v2 1/3] xfs_quota: add capabilities for use on ext4

["Bill O'Donnell" <billodo@redhat.com>]

On Mon, Aug 22, 2016 at 11:47:42AM +1000, Dave Chinner wrote:
> On Tue, Aug 16, 2016 at 09:16:36AM -0500, Bill O'Donnell wrote:
> > This allows xfs_quota to be used on ext4 for project quota testing
> > in xfstests.
> > 
> > This patch was originally submitted by Dave Chinner
> > (http://oss.sgi.com/archives/xfs/2016-02/msg00131.html)
> > 
> > Resubmitting with the following change:
> > quota/init.c: correct logic error in loop contained in init_args_command()
> > function (lines 85-91).
> 
> What logic error?

In your original patch, in init_args_command():

   	do {
 		fs_path = &fs_table[index++];
-	} while ((fs_path->fs_flags & FS_PROJECT_PATH) && index < fs_count);
+		if (fs_path->fs_flags & FS_PROJECT_PATH)
+			continue;
+		if (!foreign_allowed && (fs_path->fs_flags & FS_FOREIGN))
+			continue;
+	} while (index < fs_count);

The loop should break out, when (fs_path->fs_flags & FS_PROJECT_PATH) is false,
but instead moves onto the next test (and then back to the top). See in the
original while statement, the loop stops when the false condition occurs, that is,
((fs_path->fs_flags & FS_PROJECT_PATH) && index < fs_count) == False.

My commit message was completely terse, sorry. I'll clarify it in v3.

Thanks-
Bill

> 
> Commit messages like this really don't tell the reader anything
> about what is different the original patch.  I've had to go archive
> spelunking to work out what is different, and I'm still not sure
> what the logic error you fixed is....
> 
> And, FWIW, whilst spelunking, I noticed that Eric's last review
> comments on my original patch:
> 
> 	Looks ok, but now with the new option:
> 
> 	1) needs a manpage update
> 	2) usage() should be updated to include -f
> 
> 	3) and I just noticed,
> 
> 	_("foreign mount active, %s command is for XFS filesystems only\n"),
> 
> 	seems kind of unclear; maybe just
> 
> 	_("%s command is for XFS filesystems only\n"),
> 
> have not been addressed by this update.
> 
> Can you please add these these changes, update the commit message
> and resubmit?
> 
> Cheers,
> 
> Dave.
> -- 
> Dave Chinner
> david@fromorbit.com

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