[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Coly Li]

Currently f_fsid of struct kstatfs returned from ocfs2_statfs() is undefined (at least it should be
filled with 0). Since in some conditions, f_fsid value might be used as (f_fsid, ino) pare to
uniquely identify a file, ocfs2 should return a defined unique f_fsid value from ocfs2_statfs().

This patch uses uuid_hash as a unique ID to initiate f_fsid value, the 32bits width is enough for
ocfs2 membership so far.

Signed-off-by: Coly Li <coly.li@suse.de>
---
 fs/ocfs2/super.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 43ed113..fdab98a 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1425,6 +1425,8 @@ static int ocfs2_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_bavail = buf->f_bfree;
 	buf->f_files = numbits;
 	buf->f_ffree = freebits;
+	buf->f_fsid.val[0] = osb->uuid_hash & 0xFFFFFFFFUL;
+	buf->f_fsid.val[1] = 0;

 	brelse(bh);

-- 
Coly Li
SuSE Labs

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Sunil Mushran]

Wondering whether this id needs to be endian consistent.
For failover nfs setups maybe.

Coly Li wrote:
> Currently f_fsid of struct kstatfs returned from ocfs2_statfs() is undefined (at least it should be
> filled with 0). Since in some conditions, f_fsid value might be used as (f_fsid, ino) pare to
> uniquely identify a file, ocfs2 should return a defined unique f_fsid value from ocfs2_statfs().
>
> This patch uses uuid_hash as a unique ID to initiate f_fsid value, the 32bits width is enough for
> ocfs2 membership so far.
>
> Signed-off-by: Coly Li <coly.li@suse.de>
> ---
>  fs/ocfs2/super.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
> index 43ed113..fdab98a 100644
> --- a/fs/ocfs2/super.c
> +++ b/fs/ocfs2/super.c
> @@ -1425,6 +1425,8 @@ static int ocfs2_statfs(struct dentry *dentry, struct kstatfs *buf)
>  	buf->f_bavail = buf->f_bfree;
>  	buf->f_files = numbits;
>  	buf->f_ffree = freebits;
> +	buf->f_fsid.val[0] = osb->uuid_hash & 0xFFFFFFFFUL;
> +	buf->f_fsid.val[1] = 0;
>
>  	brelse(bh);
>
>   

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Coly Li]

Sunil Mushran Wrote:
> Wondering whether this id needs to be endian consistent.
> For failover nfs setups maybe.

IMHO, It is. Because osb->uuid_hash is fetched by:
        osb->uuid_hash = le32_to_cpu(di->id2.i_super.s_uuid_hash);
CMIIW, It should work in nfs failover.

> 
> Coly Li wrote:
>> Currently f_fsid of struct kstatfs returned from ocfs2_statfs() is
>> undefined (at least it should be
>> filled with 0). Since in some conditions, f_fsid value might be used
>> as (f_fsid, ino) pare to
>> uniquely identify a file, ocfs2 should return a defined unique f_fsid
>> value from ocfs2_statfs().
>>
>> This patch uses uuid_hash as a unique ID to initiate f_fsid value, the
>> 32bits width is enough for
>> ocfs2 membership so far.
>>
>> Signed-off-by: Coly Li <coly.li@suse.de>
>> ---
>>  fs/ocfs2/super.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>> index 43ed113..fdab98a 100644
>> --- a/fs/ocfs2/super.c
>> +++ b/fs/ocfs2/super.c
>> @@ -1425,6 +1425,8 @@ static int ocfs2_statfs(struct dentry *dentry,
>> struct kstatfs *buf)
>>      buf->f_bavail = buf->f_bfree;
>>      buf->f_files = numbits;
>>      buf->f_ffree = freebits;
>> +    buf->f_fsid.val[0] = osb->uuid_hash & 0xFFFFFFFFUL;
>> +    buf->f_fsid.val[1] = 0;
>>
>>      brelse(bh);
>>
>>   
> 

-- 
Coly Li
SuSE Labs

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Sunil Mushran]

It's worse. As far as I can see, the s_uuid_hash is never populated.
So we have two problems. The second issue is urgent as we are
using it in xattr.

Coly Li wrote:
> Sunil Mushran Wrote:
>   
>> Wondering whether this id needs to be endian consistent.
>> For failover nfs setups maybe.
>>     
>
> IMHO, It is. Because osb->uuid_hash is fetched by:
>         osb->uuid_hash = le32_to_cpu(di->id2.i_super.s_uuid_hash);
> CMIIW, It should work in nfs failover.
>
>   
>> Coly Li wrote:
>>     
>>> Currently f_fsid of struct kstatfs returned from ocfs2_statfs() is
>>> undefined (at least it should be
>>> filled with 0). Since in some conditions, f_fsid value might be used
>>> as (f_fsid, ino) pare to
>>> uniquely identify a file, ocfs2 should return a defined unique f_fsid
>>> value from ocfs2_statfs().
>>>
>>> This patch uses uuid_hash as a unique ID to initiate f_fsid value, the
>>> 32bits width is enough for
>>> ocfs2 membership so far.
>>>
>>> Signed-off-by: Coly Li <coly.li@suse.de>
>>> ---
>>>  fs/ocfs2/super.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
>>> index 43ed113..fdab98a 100644
>>> --- a/fs/ocfs2/super.c
>>> +++ b/fs/ocfs2/super.c
>>> @@ -1425,6 +1425,8 @@ static int ocfs2_statfs(struct dentry *dentry,
>>> struct kstatfs *buf)
>>>      buf->f_bavail = buf->f_bfree;
>>>      buf->f_files = numbits;
>>>      buf->f_ffree = freebits;
>>> +    buf->f_fsid.val[0] = osb->uuid_hash & 0xFFFFFFFFUL;
>>> +    buf->f_fsid.val[1] = 0;
>>>
>>>      brelse(bh);
>>>
>>>   
>>>       
>
>   

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Mark Fasheh]

On Thu, Jan 15, 2009 at 11:59:36AM -0800, Sunil Mushran wrote:
> It's worse. As far as I can see, the s_uuid_hash is never populated.
> So we have two problems. The second issue is urgent as we are
> using it in xattr.

Well, mkfs.ocfs2/tunefs.ocfs2 is supposed to populate it s_uuid_hash based
on the *original* uuid of the file system. We don't dynamically calculate it
because uuid might change due to a clone operation, which would then break
the hashing of that particular file system.

Since s_uuid_hash is feature specific, I think perhaps we should just use
some part of the uuid, which is always populated. For this, I think we could
do the same as the o2cb dlm key (which has to also be pretty unique) -
crc32_le(uuid).
	--Mark

--
Mark Fasheh

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Sunil Mushran]

Mark Fasheh wrote:
> Well, mkfs.ocfs2/tunefs.ocfs2 is supposed to populate it s_uuid_hash based
> on the *original* uuid of the file system. We don't dynamically calculate it
> because uuid might change due to a clone operation, which would then break
> the hashing of that particular file system.

OK. I missed that bit.

> Since s_uuid_hash is feature specific, I think perhaps we should just use
> some part of the uuid, which is always populated. For this, I think we could
> do the same as the o2cb dlm key (which has to also be pretty unique) -
> crc32_le(uuid)

I was thinking the same.

[Ocfs2-devel] Integrating OCFS2 and CLVM

[Hagmann, Michael]

Hi

Does anyone work on the integration from clvmd for ocfs2?
On the Roadmap it's named as 1.4+

http://oss.oracle.com/osswiki/OCFS2/Roadmap?highlight=(roadmap)

That's really a important Feature in a large enviroment.

Thanks for your Info

Michael Hagmann

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Coly Li]

Sunil Mushran Wrote:
> It's worse. As far as I can see, the s_uuid_hash is never populated.
> So we have two problems. The second issue is urgent as we are
> using it in xattr.
> 
Thanks for pointing out this :)
I have updated the patch and sent out as v4.

> Coly Li wrote:
>> Sunil Mushran Wrote:
>>  
>>> Wondering whether this id needs to be endian consistent.
>>> For failover nfs setups maybe.
>>>     
>>
>> IMHO, It is. Because osb->uuid_hash is fetched by:
>>         osb->uuid_hash = le32_to_cpu(di->id2.i_super.s_uuid_hash);
>> CMIIW, It should work in nfs failover.
>>
>>  
[snip]
-- 
Coly Li
SuSE Labs

[Ocfs2-devel] [PATCH] ocfs2: return f_fsid info in ocfs2_statfs()

[Coly Li]

Sunil Mushran Wrote:
> Mark Fasheh wrote:
>> Well, mkfs.ocfs2/tunefs.ocfs2 is supposed to populate it s_uuid_hash
>> based
>> on the *original* uuid of the file system. We don't dynamically
>> calculate it
>> because uuid might change due to a clone operation, which would then
>> break
>> the hashing of that particular file system.
> 
> OK. I missed that bit.
> 
>> Since s_uuid_hash is feature specific, I think perhaps we should just use
>> some part of the uuid, which is always populated. For this, I think we
>> could
>> do the same as the o2cb dlm key (which has to also be pretty unique) -
>> crc32_le(uuid)
> 
> I was thinking the same.

Now I use crc32_le(osb->uuid_str) to generate fsid value. osb->uuid_str is string representation of
s_uuid. Since uuid_str is byte-by-byte, the f_fsid value should be endian consistent.

Would you please to review the v4 patch once more ?

Thanks for your comments.
-- 
Coly Li
SuSE Labs