[PATCH 1/2] tux3: move TUX3_SUPER_MAGIC to magic.h

[Chao Yu]

Hi Daniel,

> -----Original Message-----
> From: Daniel Phillips [mailto:daniel at phunq.net]
> Sent: Wednesday, August 06, 2014 4:15 PM
> To: Chao Yu
> Cc: phillips at phunq.net; hirofumi at mail.parknet.co.jp; tux3 at phunq.net
> Subject: Re: [PATCH 1/2] tux3: move TUX3_SUPER_MAGIC to magic.h
> 
> On Tuesday, August 5, 2014 9:28:20 PM PDT, Chao Yu wrote:
> > Hi Daniel,
> >
> >> -----Original Message-----
> >> From: Daniel Phillips [mailto:daniel at phunq.net]
> >> Sent: Wednesday, August 06, 2014 5:21 AM
> >> To: Chao Yu; phillips at phunq.net; hirofumi at mail.parknet.co.jp
> >> Cc: tux3 at phunq.net
> >  ...
> >
> > Thanks, I'm very glad to join this team, and I hope this patch
> > could be a good start.
> >
> >> The basic form of this patch looks good, however, you are
> >> missing an explanation of the purpose of
> >> the patch, including how uapi will be using the Tux3 magic.
> >> Some of us know why, but it is better to
> >> explain under the assumption that nobody knows the purpose of
> >> this. A longer explanation is better.
> >
> > Sorry about that.
> 
> No apology needed.
> 
> > For this patch, as near as I can tell, with magic.h,
> > applications could recognize
> > the filesystem by the value which is gotten via statfs.
> > e.g. "stat -f" could get the magic value and translate the
> > value to text, then
> > filesystem type could be shew to user.
> >
> > Is there anything I am missing?
> 
> I think that is about it. Sadly, there is not really much organization
> in the way we define and use filesystem magic numbers, except via stat.

As I checked, stat in coreutil, defines its own magic macro for checking
with return value of statfs instead of using the marco in magic.h.
Dosn't sound bad?

> In a perfect world, mount would have a way of knowing where to look in
> a volume to check for a magic number and would therefore know which
> filesystem code to execute for a mount without being told. Instead, it
> just tries every filesystem and the first one that does not fail is
> assumed to be the correct filesystem.
> 
> Your patch description could be something like "Define the Tux3 magic
> number in linux/magic.h like other filesystems do". The point is, you
> need to say not only what the patch does, but why it does it. In this
> case the "why" is "to be like other filesystems". There isn't actually
> a technical reason for doing it that I know of, but at least, user
> programs will be able to include magic.h and use a symbolic constant
> instead of a literal number, for what that is worth.

Agreed, that is the neat one hit our main point, discarding the unneeded
reason in technical detail that userspace developer knows well.
I will use this one to replace the old one.

> 
> You also need this patch to be its own separate post, not part of a
> series. So, please just repost it that way and it should be good to
> apply.
> 
> For your second patch, you are correct, the vfs does those checks. But
> you need to add a line to fill in s_max_links, unless you can show that
> the default is correct, in which case you would need to add a comment
> to state that. Otherwise, it looks good.

Oh, you're right. My mistaken.
Let me fix this issue and send a v2 patch.

Regards,
Yu

> 
> Regards,
> 
> aniel