[pve-devel] [PATCH v2 container 1/9] tools: add can_use_new_mount_api helper

[Thomas Lamprecht]

On 11/14/19 9:45 AM, Wolfgang Bumiller wrote:
> On Wed, Nov 13, 2019 at 02:46:57PM +0100, Thomas Lamprecht wrote:
>> On 11/13/19 1:30 PM, Oguz Bektas wrote:
>>> On Wed, Nov 13, 2019 at 10:33:11AM +0100, Wolfgang Bumiller wrote:
>>>> +sub can_use_new_mount_api() {
>>>
>>> i don't like these names.. isn't can_use_new_mount_api() a bit too
>>> vague? "new" is also relative, it won't be "new" in a few releases
>>>
>>> maybe something like can_hotplug_mountpoint() would be better,
>>> describing what it checks in a more verbose way?
>>
>> can_hotplug is not really better, as it got the direction wrong.
> 
> That was my main point, what the function actually does has nothing to
> do with hotplugging. In fact, strictly speaking it's not even a
> requirement, as even with the old api we *could* potentially do mount
> point hotplugging. It just involves a little more black magic and
> ritualistic sacrifices.

heh

> 
>> Just because we need fsopen for the way Wolfgang implemnts CT mountpoint
> 
> I actually ended up only using move_mount(), but it's part of the same
> change set in the kernel. (But there's a good chance we may want to use
> a few more in the future. Although then we probably won't be needing the
> early check anymore.) Anyway, I suppose I can use move_mount(-1, NULL,
> -1, NULL) instead, too ;-)

Any of those should work, yeah, and FWIW I think the name is OK, this mount
API rework is in the talking since a few years and I hardly think that we
get another anytime soon™ and if we can start to use that here and the name
still holds ;P

> 
>> hotplug, it does not means that fsopen can only used for that in the
>> future. You tend to call checks for the thing you want to do if they
>> succeed, but most of the time they should be called for what they really
>> check, if that is a combined check to see if hotplug is possible that name
>> could be OK, but that isn't such a method.
>>
>> something in the form of "can_fsopen_syscall" would work better, IMO.
>>
>> Also, we may want to have a "can do syscall X" check in common?
> 
> I've mostly decided against that because normally I'd rather just "do
> it" and then deal with the error after the fact. In the mount case
> however my main concern was that it's just a lot cheaper than going
> through the entire mount process (potentially involving network access,
> creating directories etc., switching namespaces...) just to fail at the
> very end.
> 

makes sense