all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: "Fabian Grünbichler" <f.gruenbichler@proxmox.com>
To: Denis Knauf <deac+git@denkn.at>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 container 2/2] close #3471: Support for NetworkManager on CentOS.
Date: Thu, 17 Jun 2021 14:33:29 +0200	[thread overview]
Message-ID: <1623933097.d1qp4xibda.astroid@nora.none> (raw)
In-Reply-To: <92168992-a4f5-76ee-c4f1-66a7e837a303@proxmox.com>

On June 17, 2021 2:28 pm, Dominik Csapak wrote:
> hi,
> 
> did not took a look at the whole patch, but
> some things stood out, comments inline
> 
> On 6/16/21 20:23, Denis Knauf wrote:
>> NetworkManager is used on RHEL8 and similar distributions like CentOS8,
>> AlmaLinux8 and RockyLinux8.
>> On RHEL7/CentOS7 it was not mandatory, but possible to use
>> NetworkManager.
>> The past config was used on CentOS6/7 withour NetworkManager, so tests
>> were also testing this basic network-config.
>> 
>> The patches is compatible to any RHEL7/RHEL8-compatible distribution with/without
>> NetworkManager.
>> 
>> Because the NetworkManager requires a UUID, the tests were extended by
>> special behaviour in case of UUIDs in `ifcfg-eth*`.
>> 
>> Successfully tested against:
>> * CentOS7 without NetworkManager
>> * CentOS7 with NetworkManager
>> * CentOS8
>> * RockyLinux 8
>> 
>> Signed-off-by: Denis Knauf <deac+git@denkn.at>

...

> 
>>   	my $data = '';
>>   	my $bootproto = '';
>>   
>> @@ -213,7 +214,7 @@ sub setup_network {
>>   	    if ($d->{ip6} eq 'auto') {
>>   		$data .= "IPV6_AUTOCONF=yes\n";
>>   	    }
>> -	    if ($d->{ip6} eq 'dhcp') {
>> +	    elsif ($d->{ip6} eq 'dhcp') {
> 
> this change does nothing?

it changes the condition for the else branch below (previously, ip6 == 
auto would take it, now it does not - see the bug report).

> 
>>   		$data .= "DHCPV6C=yes\n";
>>   	    } else {
>>   		$data .= "IPV6ADDR=$d->{ip6}\n";
>> @@ -229,6 +230,15 @@ sub setup_network {
>>   	    }
>>   	}




  reply	other threads:[~2021-06-17 12:33 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-16 18:23 [pve-devel] [PATCH v2 container 1/2] CentOS6 is obsolete. Tests renamed to centos7 Denis Knauf
2021-06-16 18:23 ` [pve-devel] [PATCH v2 container 2/2] close #3471: Support for NetworkManager on CentOS Denis Knauf
2021-06-17 12:28   ` Dominik Csapak
2021-06-17 12:33     ` Fabian Grünbichler [this message]
2021-06-17 12:58       ` Dominik Csapak

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1623933097.d1qp4xibda.astroid@nora.none \
    --to=f.gruenbichler@proxmox.com \
    --cc=deac+git@denkn.at \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal