xfsrestore: lift directory entry count limitations
cover xfsrestore: lift directory entry count limitations
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 1/2 xfsrestore: turn off NODECHK
The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.
restore/Makefile +1 -1
@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA LTDEPENDENCIES = $(LIBRMT) LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \ - -DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \ + -DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \ -DF_FSSETDM default: depend $(LTCOMMAND)
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
PATCH 2/2 xfsrestore: change nrh_t to be 64 bits instead of 32
In reply to [email protected]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.
restore/namreg.h +2 -2
@@ -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
restore/namreg.c +7 -13
@@ -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 */
restore/tree.c +14 -15
@@ -166,19 +166,18 @@ typedef struct tran tran_t;#define NODESZ 48struct 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
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/2] xfsrestore: turn off NODECHK
In reply to [email protected]Quoted 4 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:
plain text document attachment (
no_node_check) The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.
I was going to suggest you leave it in the Makefile in commented-out form, perhaps with an explanation that it's available for development. But since NODECHK is only ever referenced inside "restore/node.c", maybe such a thing could be done inside that file instead.
I.e., something like:
/*
* NODECHK can be enabled for development and debugging
* in order to enforce additional checks on handles
* when nodes are mapped, unmapped, and freed. It should
* not be used for production code, it cuts into the number
* of directory entries xfsrestore can handle.
*/
/* #define NODECHK */
I'll leave it up to you whether you think that's worth doing (and if so, let me know). Either way:
Quoted 16 lines
Signed-off-by Bill Kendall <[email protected]>Index: xfsdump-kernel.org/restore/Makefile
===================================================================restore/Makefile +1
-1@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA LTDEPENDENCIES = $(LIBRMT) LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \ - -DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \ + -DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \ -DF_FSSETDM default: depend $(LTCOMMAND)
Quoted 4 lines
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/2] xfsrestore: change nrh_t to be 64 bits instead of 32
In reply to [email protected]Quoted 5 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:
plain text document attachment (
namreg_64bit)
Annrh_trefers to a byte offset in a file containing all the pathname components from a dump. At an average filename length of 20 characters, annrh_twould 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.
Provided there are no ill effects due to changing the node struct to hold the 64-bit name registry handle:
Quoted 105 lines
Signed-off-by Bill Kendall <[email protected]>Index:
xfsdump-kernel.org/restore/namreg.h
===================================================================restore/namreg.h +2
-2@@ -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
===================================================================restore/namreg.c +7
-13@@ -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
===================================================================restore/tree.c +14
-15@@ -166,19 +166,18 @@ typedef struct tran tran_t;#define NODESZ 48struct 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 ;
Quoted 4 lines
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 2/2] xfsrestore: change nrh_t to be 64 bits instead of 32
Quoted 11 lines · Alex Elder
Quoted 5 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:
plain text document attachment (
namreg_64bit)
Annrh_trefers to a byte offset in a file containing all the pathname components from a dump. At an average filename length of 20 characters, annrh_twould overflow on a dump containing ~214 million directory entries.This is a good change. The value of an
nrh_tis (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.
Quoted 5 lines
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_tdefinition 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
Quoted 111 lines
Provided there are no ill effects due to changing the node struct to hold the 64-bit name registry handle:
Reviewed-by Alex Elder<[email protected]>Quoted 105 lines
Signed-off-by Bill Kendall<[email protected]>Index:
xfsdump-kernel.org/restore/namreg.h
===================================================================restore/namreg.h +2
-2@@ -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
===================================================================restore/namreg.c +7
-13@@ -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
===================================================================restore/tree.c +14
-15@@ -166,19 +166,18 @@ typedef struct tran tran_t;#define NODESZ 48struct 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 ;
Quoted 6 lines
Quoted 4 lines
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
Re: [PATCH 1/2] xfsrestore: turn off NODECHK
In reply to Alex ElderQuoted 23 lines · Alex Elder
Quoted 4 lines · On Tue, 2010-10-12 at 16:53 -0500, [email protected] wrote:
plain text document attachment (
no_node_check) The NODECHK macro should only be enabled as needed for development/debugging. Having it on limits xfsrestore to 268 million directory entries instead of 4 billion.I was going to suggest you leave it in the
Makefilein commented-out form, perhaps with an explanation that it's available for development. But since NODECHK is only ever referenced inside "restore/node.c", maybe such a thing could be done inside that file instead.I.e., something like:
/*
* NODECHK can be enabled for development and debugging
* in order to enforce additional checks on handles
* when nodes are mapped, unmapped, and freed. It should
* not be used for production code, it cuts into the number
* of directory entries xfsrestore can handle.
*/
/* #define NODECHK */
Quoted 4 lines
I'll leave it up to you whether you think that's worth doing (and if so, let me know). Either way:
Reviewed-by Alex Elder<[email protected]>
Sounds like a good idea. I'll add something like this and repost.
Bill
Quoted 15 lines
Quoted 15 lines
Signed-off-by Bill Kendall<[email protected]>Index: xfsdump-kernel.org/restore/Makefile
===================================================================restore/Makefile +1
-1@@ -103,7 +103,7 @@ LLDLIBS = $(LIBUUID) $(LIBHANDLE) $(LIBA LTDEPENDENCIES = $(LIBRMT) LCFLAGS = -DRESTORE -DRMT -DBASED -DDOSOCKS -DINVCONVFIX -DPIPEINVFIX \ - -DEOMFIX -DSESSCPLT -DWHITEPARSE -DNODECHK -DDIRENTHDR_CHECKSUM \ + -DEOMFIX -DSESSCPLT -DWHITEPARSE -DDIRENTHDR_CHECKSUM \ -DF_FSSETDM
Quoted 8 lines
Quoted 6 lines
restore/Makefile +0
-0default: depend $(LTCOMMAND)_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs
_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs