在 2018年4月1日，下午7:33，Amir Goldstein <amir73il@xxxxxxxxx> 写道：
>
> On Sun, Apr 1, 2018 at 2:06 PM, cgxu519@xxxxxxx <cgxu519@xxxxxxx> wrote:
>> 在 2018年4月1日，下午6:02，Amir Goldstein <amir73il@xxxxxxxxx> 写道：
>>>
> [...]
>>>
>>>>
>>>>>
>>>>>> +static int ovl_remount(struct super_block *sb, int *flags, char *data)
>>>>>> +{
>>>>>> + struct ovl_fs *ofs = sb->s_fs_info;
>>>>>> + struct ovl_config *old_config = &ofs->config;
>>>>>> + struct ovl_config *new_config;
>>>>>> + bool remount = true;
>>>>>
>>>>> No need for helper var.
>>>>>
>>>>>> + int err;
>>>>>> +
>>>>>> + if (!(*flags & SB_RDONLY) && ovl_force_readonly(ofs))
>>>>>> + return -EROFS;
>>>>>> +
>>>>>> + new_config = kzalloc(sizeof(struct ovl_config), GFP_KERNEL);
>>>>>> + if (!new_config)
>>>>>> + return -ENOMEM;
>>>>>> +
>>>>>> + new_config->index = old_config->index;
>>>>>> + new_config->nfs_export = old_config->nfs_export;
>>>>>> +
>>>>>> + err = ovl_parse_opt((char *)data, new_config, remount);
>>>>>> + if (err)
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + err = -EINVAL;
>>>>>> + if (new_config->lowerdir &&
>>>>>> + !ovl_string_option_equal(new_config->lowerdir,
>>>>>> + old_config->lowerdir))
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + if (new_config->upperdir &&
>>>>>> + !ovl_string_option_equal(new_config->upperdir,
>>>>>> + old_config->upperdir))
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + if (new_config->workdir &&
>>>>>> + !ovl_string_option_equal(new_config->workdir,
>>>>>> + old_config->workdir))
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + if (new_config->redirect_mode &&
>>>>>> + !ovl_string_option_equal(new_config->redirect_mode,
>>>>>> + old_config->redirect_mode))
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + if (new_config->index != old_config->index)
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + if (new_config->nfs_export != old_config->nfs_export)
>>>>>> + goto out_err;
>>>>>> +
>>>>>> + return 0;
>>>>>> +out_err:
>>>>>
>>>>> This error really calls for error in dmesg preferably pointing user
>>>>> to the offending mount option.
>>>>>
>>>>> For this reason, I think it might be better to implement those checks
>>>>> inside ovl_parse_opt() while parsing each option.
>>>>>
>>>>> Instead of passing bool remount, you can pass ovl_config *oldconfig
>>>>> to signify remount.
>>>>>
>>>>> For string options, you can replace the following code with a small helper
>>>>> or macro to also include the remount logic:
>>>>>
>>>>> kfree(config->upperdir);
>>>>> config->upperdir = match_strdup(&args[0]);
>>>>> if (!config->upperdir)
>>>>> return -ENOMEM;
>>>>> if (oldconfig && old->config->upperdir &&
>>>>> !strcmp(config->upperdir, old->config->upperdir)
>>>>> goto remount_err; // where you can
>>>>> print an error with args[0]
>>>>>
>>>>
>>>> Actually, in my initial version I did just like what you wrote here,
>>>> but considering some future potential abilities(for example ro -> rw)
>>>> that we can add later, I decided to get new value through parsing.
>>>>
>>>
>>> I don't see how my suggestion limits to ability to allow for setting
>>> new options on remount. I just gave an example of a parameter that
>>> cannot be changed from NULL to non-NULL.
>>
>> I think your suggestion will work and probably effective way to do it
>> if we prohibit changing anything in parsing. but if we allow changeable
>> options, I just worry put too many logics of remount into option parsing.
>>
>
> Not much logic. Only 3 classifications that can be encoded in 3 different
> parsing helpers:
> 1. disallow change
> 2. allow change NULL->non-NULL
> 3. allow change non-NULL->non-NULL
>
> The parsing code doesn't do anything except setting the new
> values in config struct and applying one of the limitations above
> while parsing.
For example,
In ro->rw case, We need check a condition that from mount flags(rw)
to consider if workdir/upperdir change to non-NULL from NULL is
reasonable. So we have to pass mount flag to option parsing, right?
If we specify index=on with ro->rw, then in the index case, at least
we should check if workdir/upperdir mount option and rw mount flags have
already set and based on that to consider index=on is reasonable.
[…]
>
> Setting up overlayfs based on new config values, whether
> old values where NULL or non-NULL is the job for remount code
> not of parsing code.
>
> […]
>
>>
>>>
>>> What was the motivation of this work for you?
>>> Do you have a specific set of options you want to change in remount?
>>
>> The initial motivation is from dynamically updating container image without downtime,
>> dynamic updating will significantly improve productivity(specially on communication for downtime)
>> when we change and update base image. overlayfs is only a part work in total solution
>> and actually we may need to reorganize/replace layers(maybe include lowers), but I think
>> analyzing and implementing remount will be good staring point.
>>
>> By the way, don’t you think it will be special competitive feature of overlayfs compare to
>> other docker storage backend?
>>
>
> Maybe. I did not understand the use case exactly.
> I understand the use case of "rotate" - adding new upper and setting current
> upper at the top of lower layers stack.
>
> BTW, I have implemented remount for the overlayfs snapshot feature:
> https://github.com/amir73il/linux/commits/ovl_snapshot
>
> The interesting part about this implementation is not the remount itself,
> but the functions ovl_snapshot_revalidate() and ovl_snapshot_barrier().
> If you plan to embark on a mission to implement live "rotate", you should
> take a look at this implementation.
>
> The idea is that when remount changes layers configuration, you need
> to increment a "filesystem configuration version". The current fs version
> should be stored in every dentry and dentries should be lazily invalidated
> when the fs version has changed (on remount).
Very good reference!
Thanks,
Chengguang.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
