[pve-devel] applied: [PATCH container] PVE::LXC::Setup::Fedora: use systemd-networkd

[Dominik Csapak]

hi, it seems that the patch is not pushed yet ... (at least not on 
git.promxox.com)

also one comment inline

On 07/18/2018 10:46 AM, Wolfgang Bumiller wrote:
> applied, but I wonder if we should enable systemd-networkd.service in
> template_fixup() like we do for Arch? At least if there's no
> /etc/sysconfig/network?
> 
> On Tue, Jul 17, 2018 at 06:13:45PM +0200, Stoiko Ivanov wrote:
>> Upstream's templates seem to have switched to systemd-networkd for
>> fedora > 25. Since then various workarounds have been suggested (starting
>> the legacy network.service in /etc/rc.local). This patch tries to accomodate
>> both network-configuration options for the affected and available templates
>> (25, 26, 27), by configuring both services.
>>
>> Signed-off-by: Stoiko Ivanov <s.ivanov at proxmox.com>
>> ---
>>   src/PVE/LXC/Setup/Fedora.pm | 20 ++++++++++++++++++++
>>   1 file changed, 20 insertions(+)
>>
>> diff --git a/src/PVE/LXC/Setup/Fedora.pm b/src/PVE/LXC/Setup/Fedora.pm
>> index d19b162..c54aeee 100644
>> --- a/src/PVE/LXC/Setup/Fedora.pm
>> +++ b/src/PVE/LXC/Setup/Fedora.pm
>> @@ -31,4 +31,24 @@ sub setup_init {
>>       $self->setup_container_getty_service($conf);
>>   }
>>   
>> +sub setup_network {
>> +    my ($self, $conf) = @_;
>> +
>> +    # systemd-networkd is default in fedora-templates from upstream since
>> +    # 25, however quite a few workarounds were posted in the forum, recommending
>> +    # to start the (legacy) network.service via /etc/rc.local for fedora > 25.
> 
> Why would someone do that via /etc/rc.local of all places...
> 
>> +    # /etc/sysconfig/network is not present in the templates for fedora > 25.
>> +    # use its presence to decide, whether to configure the legacy config
>> +    # additionally for 25, 26, 27.
>> +
>> +    my $sysconfig_used = $self->ct_file_exists("/etc/sysconfig/network");
>> +
>> +    my $version = $self->{version};
>> +
>> +    my $setup_sysconfig = 1 if ($version <= 24 || ($self->{version} <= 27 && $sysconfig_used));
>> +    my $setup_systemd = 1 if ($self->{version} >= 25 );

we should avoid to define variables in such conditional statements.
from perldoc perlsyn:

NOTE: The behaviour of a my, state, or our modified with a statement 
modifier conditional or loop construct (for example, my $x if ... ) is 
undefined. The value of the my variable may be undef, any previously 
assigned value, or possibly anything else. Don't rely on it. Future 
versions of perl might do something different from the version of perl 
you try it out on. Here be dragons.

>> +
>> +    $self->SUPER::setup_network($conf) if $setup_sysconfig;
>> +    $self->SUPER::setup_systemd_networkd($conf) if $setup_systemd;
>> +}
>>   1;
>> -- 
>> 2.11.0
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel at pve.proxmox.com
> https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
>