[Ocfs2-tools-devel] [PATCH] Make tunefs.ocfs2 understand all journal options.

[Sunil Mushran]

On 12/20/2011 06:51 PM, Goldwyn Rodrigues wrote:
> On Tue, Nov 22, 2011 at 1:49 PM, Joel Becker<jlbec at evilplan.org>  wrote:
>> On Thu, Nov 03, 2011 at 06:16:28PM +0100, Michal Srb wrote:
>>> The manual page of tunefs.ocfs2 says that tunefs can understand the same -J
>>> options as mkfs.ocfs2 does. That is: size,[no]block32,[no]block64. However
>>> tunefs.ocfs2 understands only the size. This adds the ability to change the
>>> block option too.
>>> ---
>>>   tunefs.ocfs2/Makefile               |    1 +
>>>   tunefs.ocfs2/libocfs2ne.c           |   18 +++++-
>>>   tunefs.ocfs2/libocfs2ne.h           |    2 +-
>>>   tunefs.ocfs2/ocfs2ne.c              |   30 ++++++++++
>>>   tunefs.ocfs2/op_set_journal_block.c |  109 +++++++++++++++++++++++++++++++++++
>>>   tunefs.ocfs2/op_set_journal_size.c  |    6 +-
>>>   tunefs.ocfs2/op_set_slot_count.c    |    5 +-
>>>   7 files changed, 164 insertions(+), 7 deletions(-)
>>>   create mode 100644 tunefs.ocfs2/op_set_journal_block.c
>>>
>>> diff --git a/tunefs.ocfs2/Makefile b/tunefs.ocfs2/Makefile
>>> index 3847d0f..995d717 100644
>>> --- a/tunefs.ocfs2/Makefile
>>> +++ b/tunefs.ocfs2/Makefile
>>> @@ -40,6 +40,7 @@ OCFS2NE_OPERATIONS =                        \
>>>        op_resize_volume                \
>>>        op_set_label                    \
>>>        op_set_journal_size             \
>>> +     op_set_journal_block            \
>>>        op_set_slot_count               \
>>>        op_update_cluster_stack         \
>>>        op_set_quota_sync_interval      \
>>> diff --git a/tunefs.ocfs2/libocfs2ne.c b/tunefs.ocfs2/libocfs2ne.c
>>> index 8e814e7..7965727 100644
>>> --- a/tunefs.ocfs2/libocfs2ne.c
>>> +++ b/tunefs.ocfs2/libocfs2ne.c
>>> @@ -331,7 +331,7 @@ errcode_t tunefs_clear_in_progress(ocfs2_filesys *fs, int flag)
>>>        return ocfs2_write_primary_super(fs);
>>>   }
>>>
>>> -errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size)
>>> +errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size, ocfs2_fs_options mask, ocfs2_fs_options options)
>>>   {
>>>        errcode_t ret = 0;
>>>        char jrnl_file[OCFS2_MAX_FILENAME_LEN];
>>> @@ -343,6 +343,8 @@ errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size)
>>>        struct ocfs2_dinode *di;
>>>        struct tunefs_filesystem_state *state = tunefs_get_state(fs);
>>>        struct tools_progress *prog;
>>> +     ocfs2_fs_options new_features;
>>> +     int features_change;
>>>
>>>        num_clusters =
>>>                ocfs2_clusters_in_blocks(fs,
>>> @@ -379,6 +381,16 @@ errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size)
>>>                return ret;
>>>        }
>>>
>>> +     new_features.opt_compat = (state->ts_journal_features.opt_compat&  ~mask.opt_compat)
>>> +                               | (options.opt_compat&  mask.opt_compat);
>>> +     new_features.opt_incompat = (state->ts_journal_features.opt_incompat&  ~mask.opt_incompat)
>>> +                                 | (options.opt_incompat&  mask.opt_incompat);
>>> +     new_features.opt_ro_compat = (state->ts_journal_features.opt_ro_compat&  ~mask.opt_ro_compat)
>>> +                                 | (options.opt_ro_compat&  mask.opt_ro_compat);
>>> +     features_change = (new_features.opt_compat ^ state->ts_journal_features.opt_compat) ||
>>> +                       (new_features.opt_incompat ^ state->ts_journal_features.opt_incompat) ||
>>> +                       (new_features.opt_ro_compat ^ state->ts_journal_features.opt_ro_compat);
>>> +
>>>        for (i = 0; i<  max_slots; ++i) {
>>>                ocfs2_sprintf_system_inode_name(jrnl_file,
>>>                                                OCFS2_MAX_FILENAME_LEN,
>>> @@ -404,7 +416,7 @@ errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size)
>>>                }
>>>
>>>                di = (struct ocfs2_dinode *)buf;
>>> -             if (num_clusters == di->i_clusters) {
>>> +             if (num_clusters == di->i_clusters&&  !features_change) {
>>>                        tools_progress_step(prog, 1);
>>>                        continue;
>>>                }
>>> @@ -413,7 +425,7 @@ errcode_t tunefs_set_journal_size(ocfs2_filesys *fs, uint64_t new_size)
>>>                         "Resizing journal \"%s\" to %"PRIu32" clusters\n",
>>>                         jrnl_file, num_clusters);
>>>                ret = ocfs2_make_journal(fs, blkno, num_clusters,
>>> -&state->ts_journal_features);
>>> +&new_features);
>>         Nowhere in any of this do you check the filesystem you are
>> changing.  While ocfs2_make_journal() will reformat your journal, it
>> doesn't handle any of the special case checking that we have in mkfs.
>> Things like:
>>
>> - If your filesystem is larger than can be described by 32bits, you
>>   can't go to block32, at least without shrinking your filesystem below
>>   the 32bit boundary.
> Is it possible to shrink an ocfs2 filesystem? Is it in development?
>
> [I was on vacation so sorry if I have missed on something]

No. But you are missing his point. The feature as you've implemented allows
users to modify the journal feature without any checks. While it is alright
to allow users to change to block64, it is not alright to allow the change to
block32 without first checking the size of the volume.

I am wondering whether the correct fix would be to just enable this feature
during resize.

Or, maybe just enable it by default during mkfs because users won't be able
to online resize across the boundary safely.