[Top] [All Lists]

Re: [PATCH 00/12, DEV-ONLY] xfsprogs: metadata CRC support, first batch

To: xfs@xxxxxxxxxxx
Subject: Re: [PATCH 00/12, DEV-ONLY] xfsprogs: metadata CRC support, first batch
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Tue, 22 Jan 2013 16:51:01 +1100
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <1358776391-22140-1-git-send-email-david@xxxxxxxxxxxxx>
References: <1358776391-22140-1-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Tue, Jan 22, 2013 at 12:52:59AM +1100, Dave Chinner wrote:
> Hi folks,
> Here is the userspace side of the first batch of kernel patches for
> metadata CRC support. It needs to be applied on top of the kernel
> sync patch I sent here:
> http://oss.sgi.com/archives/xfs/2013-01/msg00188.html
> This batch of patches covers the porting of the kernel crc32c code,
> syncing the kernel log recovery code to libxlog, and metadata CRC
> support for inodes, freespace, quota, superblock and symlinks. It
> does not cover directory or attribute metadata - that will be sent
> as a separate patch.
> This code is functional, but not pretty and probably pretty much
> unreviewable. There are outstanding issues like how to report
> verifier errors in a sane way that is source compatible with the
> kernel code, how to factor all the repeated copy-n-paste template
> code so taht kernel and userspace share code sanely, and so on.
> The xfs_db code is not fully converted - xfs_check does not work at
> all on metadata CRC enabled filesystems, so if you want to run
> xfstests, you need to set XFS_CHECK_PROG="/bin/true" in
> common.config. I haven't touched xfs_copy, metadata, quota, io, etc,
> and I know that xfs_io sees metadata CRC enabled filesystems as
> foreign because it doesn't believe that version 5 superblocks are
> valid....

Just a word of note about xfs_db. xfs_db does not use libxfs for
reading and writing stuff from disk, so currently has no
infrastructure for checking or calculating checksums. Hence anything
that you do with xfs_db to write to disk on a CRC enabled filesystem
will corrupt it as it will invalidate the on-disk CRC.

Hence there are various xfstests that use xfs_db to write different
values into structures (e.g. 045, which writes a UUID into the
superblock) will not work. Indeed, this specific operation needs to
be disallowed on version 5 superblocks - the UUID is stamped into
every single piece of metadata, so changing it is, well, a whol elot
more complex that reading and writing the superblocks....

As a result, right now my only focus for xfs_db is for it to be able
to parse the structures correctly. We'll have to decide what the
best approach to fixing these problems in xfs-db are - rewriting it
to use libxfs is a major undertaking, but then again so is
architecting in the verifiers for CRC checking and calculation. And
I don't really like the idea of have two different implementations
of the same thing in the one place at the best of times....



Dave Chinner

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