TAKE 981498 - remove mounpoint UUID code

TAKE 981498 - remove mounpoint UUID code

[Niv Sardi-Altivanik]

remove mounpoint UUID code

It looks like all of the below is unused... and according
to Nathan,

"dont think it even got used/implemented anywhere, but i think it 
was meant to be an auto-mount kinda thing... such that when you look 
up at that point, it knows to mount the device with that uuid there, 
if its not already it was never really written anywhere ... just an 
idea in doug doucettes brain i think."

Think it'll ever go anywhere, or should it get pruned?

The below builds; not at all tested, until I get an idea if it's worth
doing.  Need to double check that some structures might not need padding
out to keep things compatible/consistent...



Date:  Fri Jul 25 13:37:43 AEST 2008
Workarea:  itchy.melbourne.sgi.com:/i386/home/xaiki/Wrk/ptools/xfs-2.6
Inspected by:  esandeen,hch,xaiki

The following file(s) were checked into:
  longdrop.melbourne.sgi.com:/isms/linux/2.6.x-xfs-melb


Modid:  xfs-linux-melb:xfs-kern:31766a
fs/xfs/xfsidbg.c - 1.356 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfsidbg.c.diff?r1=text&tr1=1.356&r2=text&tr2=1.355&f=h
fs/xfs/xfs_itable.c - 1.167 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_itable.c.diff?r1=text&tr1=1.167&r2=text&tr2=1.166&f=h
fs/xfs/xfs_inode_item.c - 1.137 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode_item.c.diff?r1=text&tr1=1.137&r2=text&tr2=1.136&f=h
fs/xfs/xfs_inode_item.h - 1.51 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode_item.h.diff?r1=text&tr1=1.51&r2=text&tr2=1.50&f=h
fs/xfs/xfs_log_recover.c - 1.345 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_log_recover.c.diff?r1=text&tr1=1.345&r2=text&tr2=1.344&f=h
fs/xfs/xfs_inode.c - 1.510 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.c.diff?r1=text&tr1=1.510&r2=text&tr2=1.509&f=h
fs/xfs/xfs_inode.h - 1.251 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.h.diff?r1=text&tr1=1.251&r2=text&tr2=1.250&f=h
fs/xfs/xfs_attr_leaf.c - 1.115 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_attr_leaf.c.diff?r1=text&tr1=1.115&r2=text&tr2=1.114&f=h
fs/xfs/xfs_bmap.c - 1.399 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_bmap.c.diff?r1=text&tr1=1.399&r2=text&tr2=1.398&f=h
fs/xfs/xfs_dinode.h - 1.85 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_dinode.h.diff?r1=text&tr1=1.85&r2=text&tr2=1.84&f=h
fs/xfs/dmapi/xfs_dm.c - 1.77 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/dmapi/xfs_dm.c.diff?r1=text&tr1=1.77&r2=text&tr2=1.76&f=h

Re: TAKE 981498 - remove mounpoint UUID code

[Eric Sandeen]

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

-Eric

> It looks like all of the below is unused... and according
> to Nathan,
> 
> "dont think it even got used/implemented anywhere, but i think it 
> was meant to be an auto-mount kinda thing... such that when you look 
> up at that point, it knows to mount the device with that uuid there, 
> if its not already it was never really written anywhere ... just an 
> idea in doug doucettes brain i think."
> 
> Think it'll ever go anywhere, or should it get pruned?
> 
> The below builds; not at all tested, until I get an idea if it's worth
> doing.  Need to double check that some structures might not need padding
> out to keep things compatible/consistent...
> 
> 
> 
> Date:  Fri Jul 25 13:37:43 AEST 2008
> Workarea:  itchy.melbourne.sgi.com:/i386/home/xaiki/Wrk/ptools/xfs-2.6
> Inspected by:  esandeen,hch,xaiki
> 
> The following file(s) were checked into:
>   longdrop.melbourne.sgi.com:/isms/linux/2.6.x-xfs-melb
> 
> 
> Modid:  xfs-linux-melb:xfs-kern:31766a
> fs/xfs/xfsidbg.c - 1.356 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfsidbg.c.diff?r1=text&tr1=1.356&r2=text&tr2=1.355&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfsidbg.c.diff?r1=text&tr1=1.356&r2=text&tr2=1.355&f=h
> fs/xfs/xfs_itable.c - 1.167 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_itable.c.diff?r1=text&tr1=1.167&r2=text&tr2=1.166&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_itable.c.diff?r1=text&tr1=1.167&r2=text&tr2=1.166&f=h
> fs/xfs/xfs_inode_item.c - 1.137 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_inode_item.c.diff?r1=text&tr1=1.137&r2=text&tr2=1.136&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode_item.c.diff?r1=text&tr1=1.137&r2=text&tr2=1.136&f=h
> fs/xfs/xfs_inode_item.h - 1.51 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_inode_item.h.diff?r1=text&tr1=1.51&r2=text&tr2=1.50&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode_item.h.diff?r1=text&tr1=1.51&r2=text&tr2=1.50&f=h
> fs/xfs/xfs_log_recover.c - 1.345 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_log_recover.c.diff?r1=text&tr1=1.345&r2=text&tr2=1.344&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_log_recover.c.diff?r1=text&tr1=1.345&r2=text&tr2=1.344&f=h
> fs/xfs/xfs_inode.c - 1.510 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_inode.c.diff?r1=text&tr1=1.510&r2=text&tr2=1.509&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.c.diff?r1=text&tr1=1.510&r2=text&tr2=1.509&f=h
> fs/xfs/xfs_inode.h - 1.251 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_inode.h.diff?r1=text&tr1=1.251&r2=text&tr2=1.250&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_inode.h.diff?r1=text&tr1=1.251&r2=text&tr2=1.250&f=h
> fs/xfs/xfs_attr_leaf.c - 1.115 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_attr_leaf.c.diff?r1=text&tr1=1.115&r2=text&tr2=1.114&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_attr_leaf.c.diff?r1=text&tr1=1.115&r2=text&tr2=1.114&f=h
> fs/xfs/xfs_bmap.c - 1.399 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_bmap.c.diff?r1=text&tr1=1.399&r2=text&tr2=1.398&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_bmap.c.diff?r1=text&tr1=1.399&r2=text&tr2=1.398&f=h
> fs/xfs/xfs_dinode.h - 1.85 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> xfs_dinode.h.diff?r1=text&tr1=1.85&r2=text&tr2=1.84&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/xfs_dinode.h.diff?r1=text&tr1=1.85&r2=text&tr2=1.84&f=h
> fs/xfs/dmapi/xfs_dm.c - 1.77 - changed
http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/> dmapi/xfs_dm.c.diff?r1=text&tr1=1.77&r2=text&tr2=1.76&f=h
> http://oss.sgi.com/cgi-bin/cvsweb.cgi/xfs-linux/dmapi/xfs_dm.c.diff?r1=text&tr1=1.77&r2=text&tr2=1.76&f=h
> 
> 

Re: TAKE 981498 - remove mounpoint UUID code

[Dave Chinner]

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

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: TAKE 981498 - remove mounpoint UUID code

[Eric Sandeen]

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.

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?

-Eric

> Cheers,
> 
> Dave.

Re: TAKE 981498 - remove mounpoint UUID code

[Lachlan McIlroy]

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.
> 
> 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.  Well we are supposed to anyway.

Re: TAKE 981498 - remove mounpoint UUID code

[Timothy Shimmin]

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