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

[Daniel Phillips]

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

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.

Regards,

aniel