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.
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).
Cheers,
Amir.
--
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
