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

[Thomas Lamprecht]

On 11/13/19 1:30 PM, Oguz Bektas wrote:
> hi,
> 
> On Wed, Nov 13, 2019 at 10:33:11AM +0100, Wolfgang Bumiller wrote:
>> Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
>> ---
>> New patch
>>
>>  src/PVE/LXC/Tools.pm | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/src/PVE/LXC/Tools.pm b/src/PVE/LXC/Tools.pm
>> index bebd7d8..0256b6a 100644
>> --- a/src/PVE/LXC/Tools.pm
>> +++ b/src/PVE/LXC/Tools.pm
>> @@ -2,6 +2,8 @@
>>  
>>  package PVE::LXC::Tools;
>>  
>> +use Errno qw(ENOSYS);
>> +
>>  use PVE::SafeSyslog;
>>  
>>  # LXC introduced an `lxc.hook.version` property which allows hooks to be executed in different
>> @@ -130,4 +132,20 @@ sub cgroup_do_write($$) {
>>      return 1;
>>  }
>>  
>> +# Check whether the kernel supports the new mount api. This is used in the pre-start hook and in
>> +# the hotplugging code.
>> +my $cached_can_use_new_mount_api = undef;
>> +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.
Just because we need fsopen for the way Wolfgang implemnts CT mountpoint
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?