xfs
[Top] [All Lists]

Re: [PATCH 1/3] quota: Add a new quotactl command Q_XGETQSTATV

To: Chandra Seetharaman <sekharan@xxxxxxxxxx>
Subject: Re: [PATCH 1/3] quota: Add a new quotactl command Q_XGETQSTATV
From: Rich Johnston <rjohnston@xxxxxxx>
Date: Tue, 13 Aug 2013 15:42:23 -0500
Cc: <xfs@xxxxxxxxxxx>, linux-fsdevel <linux-fsdevel@xxxxxxxxxxxxxxx>, Steven Whitehouse <swhiteho@xxxxxxxxxx>, Jan Kara <jack@xxxxxxx>, Abhijith Das <adas@xxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1375828029-26360-2-git-send-email-sekharan@xxxxxxxxxx>
References: <1375828029-26360-1-git-send-email-sekharan@xxxxxxxxxx> <1375828029-26360-2-git-send-email-sekharan@xxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:14.0) Gecko/20120714 Thunderbird/14.0
Hey Chandra,

Nice addition to quotas.  Just a couple of comments.

On 08/06/2013 05:27 PM, Chandra Seetharaman wrote:

. . .

+/*
+ * fs_quota_statv is the used by Q_XGETQSTATV for a given file system. It
Remove extra word       ^^^^

+ * provides a centralized way to get meta information about the quota
+ * subsystem. eg. space taken up for user, group, and project quotas, number
+ * of dquots currently incore.
+ *
+ * This version has proper versioning support with appropriate padding for
+ * future expansions, and ability to expand for future without creating any
+ * backwward compatibility issues.
backward is misspelled (extra w)

+ *
+ * For Q_XGETQSTATV, user space caller need to specify 
fs_quota_statv.qs_version
+ * to the version of data they are interested in. Kernel will fill the data
+ * fields relevant to that version.
+ *
+ * If kernel does not support user space caller specified version, EINVAL will
+ * be returned. User space caller can then reduce the version number and retry
+ * the same command.
+ */
This was a little difficult to follow, suggest rewording a little, something like:

Q_XGETQSTATV uses the passed in value of the requested version via
fs_quota_statv.qs_version to determine the return data layout of fs_quota_statv.


It would be nice to get this information documented in the quotactl(2)
manpage similar to Q_XGETQSTAT.


<Prev in Thread] Current Thread [Next in Thread>