Re: [PATCH 2/2] xfsrestore: change nrh_t to be 64 bits instead of 32

[Bill Kendall <wkendall@sgi.com>]

On 10/14/2010 01:45 PM, Alex Elder wrote:
> On Tue, 2010-10-12 at 16:53 -0500, wkendall@sgi.com wrote:
>> plain text document attachment (namreg_64bit)
>> An nrh_t refers to a byte offset in a file containing all the pathname
>> components from a dump. At an average filename length of 20
>> characters, an nrh_t would overflow on a dump containing ~214 million
>> directory entries.
>
> This is a good change.  The value of an nrh_t is (opaquely)
> assigned as a 64-bit file offset, so it makes sense it should
> also be 64 bits.  Much better way of computing HDLMAX also.
>
> Do you know why NODESZ (= 48) is defined as a constant, and
> used separate from sizeof (struct node)?  Is the format of
> a node structure recorded persistently somewhere?  I don't
> know this code well enough to recognize whether the change
> to the node_t definition affects only in-memory structures.

The nodes are stored on disk in the event of a cumulative
restore (level 0 + level 1 + ...) or when resuming a previously
interrupted xfsrestore session. So persistent, but temporary.

There is no versioning of any of the data structures that
xfsrestore stores on disk. It's assumed that you're running
on a system with the same endianness, same page size and same
xfsrestore build.

At a minimum this should be documented in the man page. I'll
also look at what can be done about detecting this at runtime.

Bill


>
> Provided there are no ill effects due to changing the
> node struct to hold the 64-bit name registry handle:
>
> Reviewed-by: Alex Elder<aelder@sgi.com>
>
>>
>> Signed-off-by: Bill Kendall<wkendall@sgi.com>
>>
>> Index: xfsdump-kernel.org/restore/namreg.h
>> ===================================================================
>> --- xfsdump-kernel.org.orig/restore/namreg.h
>> +++ xfsdump-kernel.org/restore/namreg.h
>> @@ -26,8 +26,8 @@
>>
>>   /* nrh_t - handle to a registered name
>>    */
>> -typedef size32_t nrh_t;
>> -#define NRH_NULL	SIZE32MAX
>> +typedef size64_t nrh_t;
>> +#define NRH_NULL	SIZE64MAX
>>
>>
>>   /* namreg_init - creates the name registry. resync is TRUE if the
>> Index: xfsdump-kernel.org/restore/namreg.c
>> ===================================================================
>> --- xfsdump-kernel.org.orig/restore/namreg.c
>> +++ xfsdump-kernel.org/restore/namreg.c
>> @@ -72,26 +72,20 @@ typedef struct namreg_tran namreg_tran_t
>>    */
>>   #define CHKBITCNT		2
>>   #define	CHKBITSHIFT		( NBBY * sizeof( nrh_t ) - CHKBITCNT )
>> -#define	CHKBITLOMASK		( ( 1<<  CHKBITCNT ) - 1 )
>> +#define	CHKBITLOMASK		( ( 1ULL<<  CHKBITCNT ) - 1 )
>>   #define	CHKBITMASK		( CHKBITLOMASK<<  CHKBITSHIFT )
>>   #define CHKHDLCNT		CHKBITSHIFT
>> -#define CHKHDLMASK		( ( 1<<  CHKHDLCNT ) - 1 )
>> -#define CHKGETBIT( h )		( ( h>>  CHKBITSHIFT )&  CHKBITLOMASK )
>> -#define CHKGETHDL( h )		( h&  CHKHDLMASK )
>> -#define CHKMKHDL( c, h )	( ( ( c<<  CHKBITSHIFT )&  CHKBITMASK )	\
>> +#define CHKHDLMASK		( ( 1ULL<<  CHKHDLCNT ) - 1 )
>> +#define CHKGETBIT( h )		( ( (h)>>  CHKBITSHIFT )&  CHKBITLOMASK )
>> +#define CHKGETHDL( h )		( (h)&  CHKHDLMASK )
>> +#define CHKMKHDL( c, h )	( ( ( (c)<<  CHKBITSHIFT )&  CHKBITMASK )	\
>>   				  |					\
>> -				  ( h&  CHKHDLMASK ))
>> +				  ( (h)&  CHKHDLMASK ))
>>   #define HDLMAX			( ( off64_t )CHKHDLMASK )
>>
>>   #else /* NAMREGCHK */
>>
>> -#define HDLMAX			( ( ( off64_t )1			\
>> -				<<					\
>> -				    ( ( off64_t )NBBY			\
>> -				      *					\
>> -				      ( off64_t )sizeof( nrh_t )))	\
>> -				  -					\
>> -				  ( off64_t )2 ) /* 2 to avoid NRH_NULL */
>> +#define HDLMAX			( NRH_NULL - 1 )
>>
>>   #endif /* NAMREGCHK */
>>
>> Index: xfsdump-kernel.org/restore/tree.c
>> ===================================================================
>> --- xfsdump-kernel.org.orig/restore/tree.c
>> +++ xfsdump-kernel.org/restore/tree.c
>> @@ -166,19 +166,18 @@ typedef struct tran tran_t;
>>   #define NODESZ	48
>>
>>   struct node {
>> -	xfs_ino_t n_ino;		/* 8  8 ino */
>> -	nrh_t n_nrh;		/* 4 12 handle to name in name registry */
>> -	dah_t n_dah;		/* 4 16 handle to directory attributes */
>> -	nh_t n_hashh;		/* 4 20 hash array */
>> -	nh_t n_parh;		/* 4 24 parent */
>> -	nh_t n_sibh;		/* 4 28 sibling list */
>> -	nh_t n_sibprevh;	/* 4 32 prev sibling list - dbl link list */
>> -	nh_t n_cldh;		/* 4 36 children list */
>> -	nh_t n_lnkh;		/* 4 40 hard link list */
>> -	gen_t n_gen;		/* 2 42 generation count mod 0x10000 */
>> -	u_char_t n_flags;	/* 1 43 action and state flags */
>> -	u_char_t n_nodehkbyte;	/* 1 44 given to node abstraction */
>> -	int32_t pad;		/* 4 48 padding to 8 byte boundary */
>> +	xfs_ino_t n_ino;	/* 8  8 ino */
>> +	nrh_t n_nrh;		/* 8 16 handle to name in name registry */
>> +	dah_t n_dah;		/* 4 20 handle to directory attributes */
>> +	nh_t n_hashh;		/* 4 24 hash array */
>> +	nh_t n_parh;		/* 4 28 parent */
>> +	nh_t n_sibh;		/* 4 32 sibling list */
>> +	nh_t n_sibprevh;	/* 4 36 prev sibling list - dbl link list */
>> +	nh_t n_cldh;		/* 4 40 children list */
>> +	nh_t n_lnkh;		/* 4 44 hard link list */
>> +	gen_t n_gen;		/* 2 46 generation count mod 0x10000 */
>> +	u_char_t n_flags;	/* 1 47 action and state flags */
>> +	u_char_t n_nodehkbyte;	/* 1 48 given to node abstraction */
>>   };
>>
>>   typedef struct node node_t;
>> @@ -3393,9 +3392,9 @@ Node_free( nh_t *nhp )
>>   		namreg_del( np->n_nrh );
>>   		np->n_nrh = NRH_NULL;
>>   	}
>> -	if ( np->n_dah != NRH_NULL ) {
>> +	if ( np->n_dah != DAH_NULL ) {
>>   		dirattr_del( np->n_dah );
>> -		np->n_dah = NRH_NULL;
>> +		np->n_dah = DAH_NULL;
>>   	}
>>   	np->n_flags = 0;
>>   	np->n_parh = NH_NULL;
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@oss.sgi.com
>> http://oss.sgi.com/mailman/listinfo/xfs
>
>

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