On Sat, 14 Mar 2015 13:36:35 +0100, Andreas Rohner wrote:
> On 2015-03-14 04:51, Ryusuke Konishi wrote:
>> On Tue, 24 Feb 2015 20:01:44 +0100, Andreas Rohner wrote:
>>> diff --git a/include/linux/nilfs2_fs.h b/include/linux/nilfs2_fs.h
>>> index 6ffdc09..a3c7593 100644
>>> --- a/include/linux/nilfs2_fs.h
>>> +++ b/include/linux/nilfs2_fs.h
>>> @@ -222,11 +222,13 @@ struct nilfs_super_block {
>>>   */
>>>  #define NILFS_FEATURE_COMPAT_SUFILE_EXTENSION              (1ULL << 0)
>>>  #define NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS               (1ULL << 1)
>>> +#define NILFS_FEATURE_COMPAT_TRACK_SNAPSHOTS               (1ULL << 2)
>>>  
>>>  #define NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT                (1ULL << 0)
>>>  
>>>  #define NILFS_FEATURE_COMPAT_SUPP  (NILFS_FEATURE_COMPAT_SUFILE_EXTENSION \
>>> -                           | NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS)
>>> +                           | NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS \
>>> +                           | NILFS_FEATURE_COMPAT_TRACK_SNAPSHOTS)
>>>  #define NILFS_FEATURE_COMPAT_RO_SUPP       
>>> NILFS_FEATURE_COMPAT_RO_BLOCK_COUNT
>>>  #define NILFS_FEATURE_INCOMPAT_SUPP        0ULL
>>>  
>> 
>> You don't have to add three compat flags just for this one patchset.
>> Please unify it.
>> 
>> #define NILFS_FEATURE_COMPAT_TRACK_LIVE_BLKS         (1ULL << 0)
>> 
>> looks to be enough.
> 
> I could merge the TRACK_LIVE_BLKS and TRACK_SNAPSHOTS flag, but I would
> suggest to at least leave the SUFILE_EXTENSION flag (maybe with a
> different name). The SUFILE_EXTENSION flag has to be set at mkfs time
> and it cannot be set or removed later, because you cannot change the on
> disk format later. I actually set SUFILE_EXTENSION by default in mkfs,
> because it is not harmful and it gives the user the option to switch the
> other flags on later.

I see, it sounds reasonable.

Regards,
Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to