[PATCH] xfsrestore: fix multi stream support

[Rich Johnston]

On 10/02/2013 03:13 PM, Eric Sandeen wrote:
> On 10/2/13 3:03 PM, Rich Johnston wrote:
>>
>>
>> On 10/02/2013 01:41 PM, Eric Sandeen wrote:
>
> ...
>
>>> What ,do you think?
>>
>> Sure go for it. That was one of my test programs but obviously I
>> choose the wrong one. ;) Its really sixes to me.
>>
>> I still think the the check in partial_reg is not needed. I never saw
>> a case where single stream restore hits that check except when there
>> are no extents. Do you have an case/example?
>
> I don't.  It seems like reasonable defensive programming, though,
> so I'm not anxious to remove it, given that nobody really groks
> this code too well.  Could turn it into a warning, maybe, so
> it fires if we do ever get there.
>
> Can you look back in ptools & see when/why it was added?

Looks like it was added Jun 11 1998

xfsrestore can corrupt DMAPI files split across media.

Description
   When a file dumped by xfsdump spans media, the xfs extended
   attributes are saved at the end of each section. xfsrestore
   restores the xfs extended attributes each time that is sees
   them.  This causes a DMAPI file (e.g., a file saved by the
   DMF system) to appear completely restored when in fact there
   are more pieces to restore.

This looks like it can be removed safely becaus this was added for 
multistream support (dumped by xfsdump spans media).
Do you agree?

>
>> We saw this issue with DMF offline files because DMF removes the
>> extents and the file has an attribute which is not restored with the
>> current code using multistream.
>
> Ah, I think there's something about not restoring attributes until
> all of the file has been restored.  So again in this case, the file
> never looks "restored" and it never gets to the attribute restoration?
>
> Oh right, like the comment says:
>
> /* partial_reg - Registers files that are only partially restored by
>   * a dump stream into the persistent state.
>   *
>   * This is done because DMAPI extended attributes must not be set until
>   * the entire file has been restored in order to co-ordinate with the
>   * Data Migration Facility (DMF) daemons.  Since extended attributes are
>   * recorded with each extent group in the dump, this registry is used to
>   * make sure only the final dump stream applies the extended attributes.
>   *
>   * Likewise, certain extended inode flags (e.g. XFS_XFLAG_IMMUTABLE)
>   * should only be set after all data for a file has been restored.
>   */
>
>> So I thinks a simple test case is:
>>
>> Create a file with no extents. Give that file an attribute dump and
>> restore it (both single and multistream) verify the file still has
>> the attribute.
>
> An extended attribute you mean?
Yes
>
>> Your thoughts?
>
> Yeah, go for it w/ a testcase. :)

Looks like multistream is not tested at all.

I would more or less modify 296 to test multistream and include more 
types of files to test (files with extents, with attributes, with both etc).

I will resubmit with your changes (giving credit where credit is do)

>
> I could see where even if we didn't get the dreaded "Out of records"
> message, it might still skip the attribute restore if the file
> never looks "done?"
Yes but only when the data is split (multi stream).


>
> -Eric
>
>   --Rich
>>
>>>
>>> -Eric
>>>
>>
>