[Ocfs2-devel] [PATCH] ocfs2: Implementation of local and global quota file handling

[Joel Becker]

On Mon, Oct 20, 2008 at 07:23:57PM +0200, Jan Kara wrote:
> Signed-off-by: Jan Kara <jack at suse.cz>

	The commit message could maybe use a little discussion of what
quota bits go in the local file and what goes in the global.  It seems
like less is written to the global file, but I might be wrong about
that (eg, generic quota code might be writing there).

> diff --git a/fs/ocfs2/quota.h b/fs/ocfs2/quota.h
> new file mode 100644
> index 0000000..33b47eb
> --- /dev/null
> +++ b/fs/ocfs2/quota.h
> @@ -0,0 +1,182 @@
> +/*
> + * quota.h for OCFS2
> + *
> + * On disk quota structures for local and global quota file, in-memory
> + * structures.

	I'm thinking the on-disk structures belong in ocfs2_fs.h, which
is where we define the disk format, and the in-memory structures belong
here.

> +/* Information header of local quota file (immediately follows the generic
> + * header) */
> +struct ocfs2_local_disk_dqinfo {
> +	__le32 dqi_flags;	/* Flags for quota file */
> +	__le32 dqi_chunks;	/* Number of chunks of quota structures
> +				 * with a bitmap */
> +	__le32 dqi_blocks;	/* Number of blocks allocated for quota file */
> +};

	Also, we'd love the on-disk structures to have the size/offset
annotations we put in ocfs2_fs.h.  Yes, some of the smaller structures
are pretty obvious, but having the hexadecimal values in the header
often helps reading binary dumps.

> +	while (toread > 0) {
> +		tocopy = min(sb->s_blocksize - offset, toread);
> +		bh = ocfs2_bread(gqinode, blk, &err, 0);
> +		if (!bh) {
> +			mlog_errno(err);
> +			return err;
> +		}

	Fair warning - you use ocfs2_bread() a lot in here, but
ocfs2_bread() is going away.  Before I realized you were going to use it
(and conceptually that makes sense), it moved into fs/ocfs2/dir.c.  Then
it morphed into ocfs2_read_dir_block()as part of my read-metadata branch
at
http://oss.oracle.com/git/jlbec/linux-2.6.git/?p=jlbec/linux-2.6.git;a=shortlog;h=read-metadata
	In the end, this code will be doing ocfs2_read_quota_block(), or
perhaps different variations thereof (if there are different types of
blocks in the quota file).  I'm going to refactor ocfs2_read_dir_block()
and introduce the function ocfs2_read_virt_blocks().  It will look
exactly like ocfs2_read_blocks() does in my read-metadata branch, except
that it will take virtual block numbers rather than physical ones.
Thus, you'll be able to write ocfs2_read_quota_block() as a thin
wrapper around it, and replace your ocfs2_bread() calls easily.
	Obviously, you don't have to worry too much until this code
moves atop the latest kernel code.  And I'm happy to help.


> +/* Write bufferhead into the fs */
> +static int ocfs2_modify_bh(struct inode *inode, struct buffer_head *bh,
> +		void (*modify)(struct buffer_head *, void *), void *private)
> +{
> +	struct super_block *sb = inode->i_sb;
> +	handle_t *handle;
> +	int status;
> +
> +	handle = ocfs2_start_trans(OCFS2_SB(sb), 1);
> +	if (IS_ERR(handle)) {
> +		status = PTR_ERR(handle);
> +		mlog_errno(status);
> +		return status;
> +	}
> +	status = ocfs2_journal_access(handle, inode, bh,
> +				      OCFS2_JOURNAL_ACCESS_WRITE);
> +	if (status < 0) {
> +		mlog_errno(status);
> +		ocfs2_commit_trans(OCFS2_SB(sb), handle);
> +		return status;
> +	}
> +	lock_buffer(bh);
> +	modify(bh, private);
> +	unlock_buffer(bh);
> +	status = ocfs2_journal_dirty(handle, bh);
> +	if (status < 0) {
> +		mlog_errno(status);
> +		ocfs2_commit_trans(OCFS2_SB(sb), handle);
> +		return status;
> +	}

	ocfs2_journal_dirty() can't sanely return an error, because
journal_dirty() can't either.  Someday it will become void.  You can
take out all the error checking of your ocfs2_journal_dirty() calls.
	Is this modify_bh() function the reason you need nested
transactions?  The callers can't be audited for their transaction state?

Joel

-- 

"It is not the function of our government to keep the citizen from
 falling into error; it is the function of the citizen to keep the
 government from falling into error."
	- Robert H. Jackson

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127