On 7/31/17 4:07 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Extend typ_t to (optionally) store a pointer to a function to calculate
> the CRC of the block, provide functions to do this for the dir3 and
> attr3 types, and then wire up the write command so that we can modify
> directory and extended attribute block fields.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> db/attr.c | 32 ++++++++++++++++++++++++++++++++
> db/attr.h | 1 +
> db/dir2.c | 37 +++++++++++++++++++++++++++++++++++++
> db/dir2.h | 1 +
> db/type.c | 8 ++++----
> db/type.h | 2 ++
> db/write.c | 3 +++
> 7 files changed, 80 insertions(+), 4 deletions(-)
>
>
> diff --git a/db/attr.c b/db/attr.c
> index 98fb069..2fa6690 100644
> --- a/db/attr.c
> +++ b/db/attr.c
> @@ -602,6 +602,38 @@ const struct field attr3_remote_crc_flds[] = {
> { NULL }
> };
>
> +/* Set the CRC. */
> +void
> +xfs_attr3_set_crc(
> + struct xfs_buf *bp)
> +{
> + __be32 magic32;
> + __be16 magic16;
> +
> + magic32 = *(__be32 *)bp->b_addr;
> + magic16 = ((struct xfs_da_blkinfo *)bp->b_addr)->magic;
> +
> + switch (magic16) {
> + case cpu_to_be16(XFS_ATTR3_LEAF_MAGIC):
Isn't it more typical to endian-swap magic16 and not have the
swap in each case statement? *shrug*
> + xfs_buf_update_cksum(bp, XFS_ATTR3_LEAF_CRC_OFF);
> + return;
> + case cpu_to_be16(XFS_DA3_NODE_MAGIC):
> + xfs_buf_update_cksum(bp, XFS_DA3_NODE_CRC_OFF);
> + return;
> + default:
> + break;
> + }
...
>
> diff --git a/db/write.c b/db/write.c
> index d24ea05..266bde4 100644
> --- a/db/write.c
> +++ b/db/write.c
> @@ -173,6 +173,9 @@ write_f(
> } else if (iocur_top->dquot_buf) {
> local_ops.verify_write = xfs_verify_recalc_dquot_crc;
> dbprintf(_("Allowing write of corrupted dquot with good CRC\n"));
> + } else if (iocur_top->typ->crc_off == TYP_F_CRC_FUNC) {
> + local_ops.verify_write = iocur_top->typ->set_crc;
> + dbprintf(_("Allowing write of corrupted data with good CRC\n"));
> } else { /* invalid data */
> local_ops.verify_write = xfs_verify_recalc_crc;
> dbprintf(_("Allowing write of corrupted data with good CRC\n"));
looking at this else if else if else if gunk makes me think that this has
grown a bit out of control.
We have other special types that require unique crc setting functions - dquots
and inodes. In those cases, we have TYP_F_NO_CRC_OFF but a special indicator
that they are of this type (ino_buf, dquot_buf), with a hard-coded recalculation
routine (xfs_verify_recalc_dquot_crc, xfs_verify_recalc_inode_crc). Now for /other/
types you've extended typ_t and put the function in there as an op, which makes
sense.
It seems to me that it would be logical to move the existing crc recalculation
routines for dquots & inodes into this new op as well, and locate & name everything
consistently, no?
Thanks,
-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
