Eric Sandeen wrote:
> Dave Chinner wrote:
>> On Thu, Jul 24, 2008 at 10:45:42PM -0500, Eric Sandeen wrote:
>>> Niv Sardi-Altivanik wrote:
>>>> remove mounpoint UUID code
>>> Are you sure this didn't change any disk structures? The patch I sent
>>> was RFC and completely untested... (and disclosed as such...) :)
>> Looking at the original patch, it definitely does change the format
>> of log structures on disk. it removes the union of the uuid and rdev
>> in the xfs_inode_log_format[32|64] which takes that entry from 16
>> bytes down to 4 bytes. So I'd suggest that thisss should be removed
>> immediately before it hits public and people start corrupting their
>> filesystems....
>
> Yep. Well crud, I even knew that when I sent it, hence the
> RFC/untested/blah/blah but I suppose I shouldn't send a patch that I
> know to be busted even if it's just as a whaddya-think. I'll pad out
> the union, check all the log structs, run qa & resend.
>
Well, I wouldn't think it is a problem for xfs_dinode_t,
after di_next_unlinked the data structure is basically documentation
and certainly di_a really starts at the attribute fork offset :)
As Dave said, it is a problem for xfs_inode_log_format and friends,
(ones which were changed for 32/64 bit variants).
This is not good but it is only a problem if you need to do log replay
with a new kernel on a dirty log created on the old kernel
(or vice-versa).
I don't think I want to change log replay to handle yet another variant
of inode item :) But it aint as bad as before as there is a cutoff point
and it only becomes a problem on unclean mount at that cutoff point.
Then again, if rc-1 is unstable and we crash out with a dirty log
and then try to replay using a more stable old kernel, it would have
trouble in log replay. (Just thinking aloud of possiblities :-)
> And despite all the talk about community & contributors running qa and
> helping with test coverage - as a general rule do sgi devels run qa too
> before committing?
>
Yes.
--Tim
|