public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons
@ 2025-12-17 15:06 Maximiliano Sandoval
  2025-12-17 21:31 ` Thomas Lamprecht
  2025-12-17 21:36 ` Thomas Lamprecht
  0 siblings, 2 replies; 6+ messages in thread
From: Maximiliano Sandoval @ 2025-12-17 15:06 UTC (permalink / raw)
  To: pve-devel

Network manager uses GLib's key file format [1]. As per `man 5
nm-settings-keyfile`:

> lists are separated by character ;

additionally, table 6 contains an example:

> Example: dns=1.2.3.4;8.8.8.8;8.8.4.4;

[1] https://docs.gtk.org/glib/struct.KeyFile.html

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---

Running `nmcli` with this patch on a new rockylinux 10 container results in 

```
DNS configuration:
        servers: 10.10.10.2 10.10.10.22
        domains: testinstall
        interface: eth0
```

and 
```
$ grep 'dns=' /etc/NetworkManager/system-connections/eth0.nmconnection
dns=10.10.10.2;10.10.10.22
```

and domain resolution seems to works as expected.


 src/PVE/LXC/Setup/CentOS.pm | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/PVE/LXC/Setup/CentOS.pm b/src/PVE/LXC/Setup/CentOS.pm
index 0826977..7bccca2 100644
--- a/src/PVE/LXC/Setup/CentOS.pm
+++ b/src/PVE/LXC/Setup/CentOS.pm
@@ -244,7 +244,7 @@ sub setup_network_with_networkmanager {
                 }
             }
             if (@name_servers_v4) {
-                $data .= "dns=" . join(',', @name_servers_v4) . "\n";
+                $data .= "dns=" . join(';', @name_servers_v4) . "\n";
                 $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
                     if $searchdomains;
             }
@@ -270,7 +270,7 @@ sub setup_network_with_networkmanager {
                 }
             }
             if (@name_servers_v6) {
-                $data .= "dns=" . join(',', @name_servers_v6) . "\n";
+                $data .= "dns=" . join(';', @name_servers_v6) . "\n";
                 $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
                     if $searchdomains;
             }
-- 
2.47.3



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons
  2025-12-17 15:06 [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons Maximiliano Sandoval
@ 2025-12-17 21:31 ` Thomas Lamprecht
  2025-12-17 21:36 ` Thomas Lamprecht
  1 sibling, 0 replies; 6+ messages in thread
From: Thomas Lamprecht @ 2025-12-17 21:31 UTC (permalink / raw)
  To: Proxmox VE development discussion, Maximiliano Sandoval

On 17/12/2025 16:06, Maximiliano Sandoval wrote:
> Network manager uses GLib's key file format [1]. As per `man 5
> nm-settings-keyfile`:
> 
>> lists are separated by character ;
> 
> additionally, table 6 contains an example:
> 
>> Example: dns=1.2.3.4;8.8.8.8;8.8.4.4;
> 
> [1] https://docs.gtk.org/glib/struct.KeyFile.html
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> 
> Running `nmcli` with this patch on a new rockylinux 10 container results in 
> 
> ```
> DNS configuration:
>         servers: 10.10.10.2 10.10.10.22
>         domains: testinstall
>         interface: eth0
> ```
> 
> and 
> ```
> $ grep 'dns=' /etc/NetworkManager/system-connections/eth0.nmconnection
> dns=10.10.10.2;10.10.10.22
> ```
> 
> and domain resolution seems to works as expected.

We got an existing (simple) test system for these things in the pve-container
repo While modern distros do not have been added all to diligently to that,
it still would be nice to have as follow-up here.




_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons
  2025-12-17 15:06 [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons Maximiliano Sandoval
  2025-12-17 21:31 ` Thomas Lamprecht
@ 2025-12-17 21:36 ` Thomas Lamprecht
  2025-12-18  8:21   ` Maximiliano Sandoval
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2025-12-17 21:36 UTC (permalink / raw)
  To: Proxmox VE development discussion, Maximiliano Sandoval

On 17/12/2025 16:06, Maximiliano Sandoval wrote:
> Network manager uses GLib's key file format [1]. As per `man 5
> nm-settings-keyfile`:
> 
>> lists are separated by character ;

this is just an example though, marked by the "etc" you omitted...

[1] you linked yourself actually spells out:

"Lists are separated by a separator character, typically ; or ,.
To use the list separator character in a value in a list, it has
to be escaped by prefixing it with a backslash."

And the nm-settings-keyfile man page has tons of usages of comma
as separator. Please verify this.

> 
> additionally, table 6 contains an example:
> 
>> Example: dns=1.2.3.4;8.8.8.8;8.8.4.4;
> 
> [1] https://docs.gtk.org/glib/struct.KeyFile.html



> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> 
> Running `nmcli` with this patch on a new rockylinux 10 container results in 
> 
> ```
> DNS configuration:
>         servers: 10.10.10.2 10.10.10.22
>         domains: testinstall
>         interface: eth0
> ```
> 
> and 
> ```
> $ grep 'dns=' /etc/NetworkManager/system-connections/eth0.nmconnection
> dns=10.10.10.2;10.10.10.22
> ```
> 
> and domain resolution seems to works as expected.
> 
> 
>  src/PVE/LXC/Setup/CentOS.pm | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/PVE/LXC/Setup/CentOS.pm b/src/PVE/LXC/Setup/CentOS.pm
> index 0826977..7bccca2 100644
> --- a/src/PVE/LXC/Setup/CentOS.pm
> +++ b/src/PVE/LXC/Setup/CentOS.pm
> @@ -244,7 +244,7 @@ sub setup_network_with_networkmanager {
>                  }
>              }
>              if (@name_servers_v4) {
> -                $data .= "dns=" . join(',', @name_servers_v4) . "\n";
> +                $data .= "dns=" . join(';', @name_servers_v4) . "\n";
>                  $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
>                      if $searchdomains;
>              }
> @@ -270,7 +270,7 @@ sub setup_network_with_networkmanager {
>                  }
>              }
>              if (@name_servers_v6) {
> -                $data .= "dns=" . join(',', @name_servers_v6) . "\n";
> +                $data .= "dns=" . join(';', @name_servers_v6) . "\n";
>                  $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
>                      if $searchdomains;
>              }



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons
  2025-12-17 21:36 ` Thomas Lamprecht
@ 2025-12-18  8:21   ` Maximiliano Sandoval
  2025-12-18 14:09     ` Thomas Lamprecht
  0 siblings, 1 reply; 6+ messages in thread
From: Maximiliano Sandoval @ 2025-12-18  8:21 UTC (permalink / raw)
  To: Thomas Lamprecht; +Cc: Proxmox VE development discussion

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> On 17/12/2025 16:06, Maximiliano Sandoval wrote:
>> Network manager uses GLib's key file format [1]. As per `man 5
>> nm-settings-keyfile`:
>> 
>>> lists are separated by character ;
>
> this is just an example though, marked by the "etc" you omitted...
>
> [1] you linked yourself actually spells out:
>
> "Lists are separated by a separator character, typically ; or ,.
> To use the list separator character in a value in a list, it has
> to be escaped by prefixing it with a backslash."

Yes, but the quote is from the nm-settings man page. GLib's .ini format
allows to decide which separator to use **globally**, see [2], it does
not allow for some lists will be handled with one separator while others
with another, not without out-of-tree parsing at least. There are some
exceptions in nm-settings though, but they are mentioned in the man page
and are not handled as lists, namely:

```
Also, some lists of complex values (addresses, routes, routing-rules),
instead of using a semicolon separated list, use one key-value pair per
list element,
```


[2] https://docs.gtk.org/glib/method.KeyFile.set_list_separator.html

>
> And the nm-settings-keyfile man page has tons of usages of comma
> as separator. Please verify this.
>
>> 
>> additionally, table 6 contains an example:
>> 
>>> Example: dns=1.2.3.4;8.8.8.8;8.8.4.4;
>> 
>> [1] https://docs.gtk.org/glib/struct.KeyFile.html
>
>
>
>> 
>> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
>> ---
>> 
>> Running `nmcli` with this patch on a new rockylinux 10 container results in 
>> 
>> ```
>> DNS configuration:
>>         servers: 10.10.10.2 10.10.10.22
>>         domains: testinstall
>>         interface: eth0
>> ```
>> 
>> and 
>> ```
>> $ grep 'dns=' /etc/NetworkManager/system-connections/eth0.nmconnection
>> dns=10.10.10.2;10.10.10.22
>> ```
>> 
>> and domain resolution seems to works as expected.
>> 
>> 
>>  src/PVE/LXC/Setup/CentOS.pm | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/PVE/LXC/Setup/CentOS.pm b/src/PVE/LXC/Setup/CentOS.pm
>> index 0826977..7bccca2 100644
>> --- a/src/PVE/LXC/Setup/CentOS.pm
>> +++ b/src/PVE/LXC/Setup/CentOS.pm
>> @@ -244,7 +244,7 @@ sub setup_network_with_networkmanager {
>>                  }
>>              }
>>              if (@name_servers_v4) {
>> -                $data .= "dns=" . join(',', @name_servers_v4) . "\n";
>> +                $data .= "dns=" . join(';', @name_servers_v4) . "\n";
>>                  $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
>>                      if $searchdomains;
>>              }
>> @@ -270,7 +270,7 @@ sub setup_network_with_networkmanager {
>>                  }
>>              }
>>              if (@name_servers_v6) {
>> -                $data .= "dns=" . join(',', @name_servers_v6) . "\n";
>> +                $data .= "dns=" . join(';', @name_servers_v6) . "\n";
>>                  $data .= "dns-search=" . join(' ', PVE::Tools::split_list($searchdomains)) . "\n"
>>                      if $searchdomains;
>>              }

-- 
Maximiliano


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons
  2025-12-18  8:21   ` Maximiliano Sandoval
@ 2025-12-18 14:09     ` Thomas Lamprecht
  2025-12-18 15:04       ` [pve-devel] superseded: " Maximiliano Sandoval
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Lamprecht @ 2025-12-18 14:09 UTC (permalink / raw)
  To: Maximiliano Sandoval; +Cc: Proxmox VE development discussion

On 18/12/2025 09:21, Maximiliano Sandoval wrote:
> Yes, but the quote is from the nm-settings man page. GLib's .ini format
> allows to decide which separator to use **globally**, see [2], it does
> not allow for some lists will be handled with one separator while others
> with another, not without out-of-tree parsing at least. There are some
> exceptions in nm-settings though, but they are mentioned in the man page
> and are not handled as lists, namely:
> 
> ```
> Also, some lists of complex values (addresses, routes, routing-rules),
> instead of using a semicolon separated list, use one key-value pair per
> list element,
> ```
> 
> [2] https://docs.gtk.org/glib/method.KeyFile.set_list_separator.html

Oof... But in this should IMO then get added to the commit message, as
when I check the docs and see examples with either variant, but without
having mentioned that it depends which variant is OK when, then any
rationale in the commit messages helps me a ton to figure that out for
myself quickly.

Do you mind re-sending this with the commit message amended? Maybe we can
also introduce a local glib_join_list helper that is basically just a
wrapper like sub { return join(';', @_); } with a comment above?
We have quite some distros we support here and things do change but in
varying rates, so such semantic helpers can be quite nice to have.

Adding already a test case would be also appreciated, hopefully the
"src/test/test-centos10-001" one can be copied and adapted to cover
this case here.


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [pve-devel] superseded: [PATCH container] fix #7156: setup: separate dns list with semicolons
  2025-12-18 14:09     ` Thomas Lamprecht
@ 2025-12-18 15:04       ` Maximiliano Sandoval
  0 siblings, 0 replies; 6+ messages in thread
From: Maximiliano Sandoval @ 2025-12-18 15:04 UTC (permalink / raw)
  To: Thomas Lamprecht; +Cc: Proxmox VE development discussion

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> On 18/12/2025 09:21, Maximiliano Sandoval wrote:
>> Yes, but the quote is from the nm-settings man page. GLib's .ini format
>> allows to decide which separator to use **globally**, see [2], it does
>> not allow for some lists will be handled with one separator while others
>> with another, not without out-of-tree parsing at least. There are some
>> exceptions in nm-settings though, but they are mentioned in the man page
>> and are not handled as lists, namely:
>> 
>> ```
>> Also, some lists of complex values (addresses, routes, routing-rules),
>> instead of using a semicolon separated list, use one key-value pair per
>> list element,
>> ```
>> 
>> [2] https://docs.gtk.org/glib/method.KeyFile.set_list_separator.html
>
> Oof... But in this should IMO then get added to the commit message, as
> when I check the docs and see examples with either variant, but without
> having mentioned that it depends which variant is OK when, then any
> rationale in the commit messages helps me a ton to figure that out for
> myself quickly.
>
> Do you mind re-sending this with the commit message amended? Maybe we can
> also introduce a local glib_join_list helper that is basically just a
> wrapper like sub { return join(';', @_); } with a comment above?
> We have quite some distros we support here and things do change but in
> varying rates, so such semantic helpers can be quite nice to have.
>
> Adding already a test case would be also appreciated, hopefully the
> "src/test/test-centos10-001" one can be copied and adapted to cover
> this case here.

Superseded-by: https://lore.proxmox.com/all/20251218150126.832313-1-m.sandoval@proxmox.com/

-- 
Maximiliano


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-12-18 15:03 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-12-17 15:06 [pve-devel] [PATCH container] fix #7156: setup: separate dns list with semicolons Maximiliano Sandoval
2025-12-17 21:31 ` Thomas Lamprecht
2025-12-17 21:36 ` Thomas Lamprecht
2025-12-18  8:21   ` Maximiliano Sandoval
2025-12-18 14:09     ` Thomas Lamprecht
2025-12-18 15:04       ` [pve-devel] superseded: " Maximiliano Sandoval

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal