Re: [PATCH V2] xfsdump: handle large, wholly-sparse files

Eric Sandeen
2013-10-03T23:16:57Z

Thanks Rich. I think we need a 3rd party reviewer now. ;)

> On Oct 3, 2013, at 6:11 PM, Rich Johnston <rjohnston@sgi.com> wrote:
>
> In restore_extent_group(), we loop over all extent headers for an inode
> in the stream, and add up the cumulatively restored size, accounting
> for both HOLE and DATA records and advancing restoredsz as we go.
>
> But for a wholly-sparse file, we have no HOLE header, only
> a LAST header, and restoredsz remains at 0.
>
> This makes it look like it's a partially-restored file, split
> across streams because the final restoredsz for this stream is
> less than the file size, and we go to partial_reg(), which
> allocates one slot in persp->a.parrest[] for this inode. But
> we've also called partial_reg() with offset/sz of 0/0, which is
> less than the file size so this inode never looks "done."
> Normally partial_check2() would clear the persp->a.parrest[]
> slot in the array when the file is fully restored, but in
> this case, that is never satisfied. So all stream slots
> get filled as we encounter more inodes like this, and we
> eventually get:
>
> "partial_reg: Out of records. Extend attrs applied early."
>
> Fix this by recognizing that if we hit a LAST header with
> no restoredsz set (i.e. the LAST header is the only header),
> set restoredsz to EOF (bstatp->bs_size) to indicate that
> restoration of this file is complete, skip the call to
> partial_reg(), and all is well.
>
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Acked-by: Rich Johnston <rjohnston@sgi.com>
>
> ---
> Test will be submitted later.
>
> Change History
>
> Original patch "[PATCH] xfsrestore: fix multi stream support"
> - Patch rename, fixes to code and commit messages by sandeen.
> - Additional changes by rjohnston
> V2
> - Remove changes by rjohnston.
>
> ---
> restore/content.c | 5 +++++
> 1 file changed, 5 insertions(+), 0 deletions(-)
>
> Index: b/restore/content.c
> ===================================================================
> --- a/restore/content.c
> +++ b/restore/content.c
> @@ -7516,6 +7516,11 @@ restore_extent_group( drive_t *drivep,
> * we are done.
> */
> if ( ehdr.eh_type == EXTENTHDR_TYPE_LAST ) {
> + /* For a wholly sparse file, there is no HOLE
> + * record; advance restoredsz to EOF.
> + */
> + if (!restoredsz)
> + restoredsz = bstatp->bs_size;
> break;
> }