[Top] [All Lists]

Re: [PATCH] xfsrestore: fix multi stream support

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] xfsrestore: fix multi stream support
From: Rich Johnston <rjohnston@xxxxxxx>
Date: Thu, 3 Oct 2013 08:40:28 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <524C7E4D.60208@xxxxxxxxxxx>
References: <524AF8AE.5030300@xxxxxxx> <524C68C5.2030202@xxxxxxxxxxx> <524C7BF6.5050107@xxxxxxx> <524C7E4D.60208@xxxxxxxxxxx>
User-agent: Mozilla/5.0 (X11; Linux i686; rv:24.0) Gecko/20100101 Thunderbird/24.0
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.

  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?

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).




<Prev in Thread] Current Thread [Next in Thread>