Re: [PATCH v2 03/12] libxfs: don't hardcode cycle 1 into unmount op header

[Dave Chinner <david@fromorbit.com>]

On Fri, Sep 11, 2015 at 02:55:33PM -0400, Brian Foster wrote:
> The libxfs helper to write a log record after zeroing the log fills much
> of the record header and unmount record with dummy data. It also
> hardcodes the cycle number into the transaction oh_tid field as the
> kernel expects to find the cycle stamped at the top of each block and
> the original oh_tid value packed into h_cycle_data of the record header.
> 
> The log clearing code requires the ability to format the log to an
> arbitrary cycle number to fix v5 superblock log recovery ordering
> problems. As a result, the unmount record helper must not hardcode a
> cycle of 1.
> 
> Fix up libxfs_log_header() to pack the unmount record appropriately, as
> is already done for extra blocks that might exist beyond the record. Use
> h_cycle_data for the original 32-bit word of the log record data block
> and stamp the cycle number in its place. This allows unmount_record() to
> work for arbitrary cycle numbers and libxfs_log_header() to pack a cycle
> value that matches the lsn used in the record header. Note that this
> patch does not change behavior as the lsn is still hardcoded to (1:0).
> 
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
>  libxfs/rdwr.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/libxfs/rdwr.c b/libxfs/rdwr.c
> index bc77699..ef18749 100644
> --- a/libxfs/rdwr.c
> +++ b/libxfs/rdwr.c
> @@ -122,7 +122,7 @@ static void unmount_record(void *p)
>  	} magic = { XLOG_UNMOUNT_TYPE, 0, 0 };
>  
>  	memset(p, 0, BBSIZE);
> -	op->oh_tid = cpu_to_be32(1);
> +	op->oh_tid = cpu_to_be32(0xb0c0d0d0);
>  	op->oh_len = cpu_to_be32(sizeof(magic));
>  	op->oh_clientid = XFS_LOG;
>  	op->oh_flags = XLOG_UNMOUNT_TRANS;
> @@ -188,10 +188,6 @@ libxfs_log_header(
>  
>  	len = ((version == 2) && sunit) ? BTOBB(sunit) : 1;
>  
> -	/* note that oh_tid actually contains the cycle number
> -	 * and the tid is stored in h_cycle_data[0] - that's the
> -	 * way things end up on disk.
> -	 */

This note needs to be hoisted up to the  setting of op->oh_tid to
explain the magic number being used...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

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