All of lore.kernel.org
 help / color / mirror / Atom feed
From: Timothy Shimmin <tes@sgi.com>
To: Eric Sandeen <sandeen@sandeen.net>
Cc: Niv Sardi-Altivanik <xaiki@sgi.com>,
	sgi.bugs.xfs@engr.sgi.com, xfs@oss.sgi.com
Subject: Re: TAKE 981498 - remove mounpoint UUID code
Date: Mon, 28 Jul 2008 11:56:51 +1000	[thread overview]
Message-ID: <488D2763.1020607@sgi.com> (raw)
In-Reply-To: <4889EAC9.5070304@sandeen.net>

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

      parent reply	other threads:[~2008-07-28  1:55 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-25  3:38 TAKE 981498 - remove mounpoint UUID code Niv Sardi-Altivanik
2008-07-25  3:45 ` Eric Sandeen
2008-07-25  5:01   ` Dave Chinner
2008-07-25 15:01     ` Eric Sandeen
2008-07-28  1:06       ` Lachlan McIlroy
2008-07-28  1:56       ` Timothy Shimmin [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=488D2763.1020607@sgi.com \
    --to=tes@sgi.com \
    --cc=sandeen@sandeen.net \
    --cc=sgi.bugs.xfs@engr.sgi.com \
    --cc=xaiki@sgi.com \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.