On 12/18/25 20:35, Laine Stump wrote:
> On 12/18/25 5:00 AM, Michal Privoznik via Devel wrote:
>> From: Michal Privoznik <[email protected]>
>>
>> In the <dns/> section of network configuration users can set up
>> forwarding of DNS requests to custom DNS servers. These are
>> specified using 'addr' attribute. But configuring port wasn't
>> possible, until now. New 'port' attribute is introduced, which
>> allows overriding the default DNS port for given address.
>>
>> Signed-off-by: Michal Privoznik <[email protected]>
>> ---
>> docs/formatnetwork.rst | 8 +++--
>> src/conf/network_conf.c | 36 +++++++++++++++----
>> src/conf/schemas/network.rng | 5 +++
>> .../nat-network-dns-forwarders.xml | 2 +-
>> .../nat-network-dns-forwarders.xml | 2 +-
>> 5 files changed, 42 insertions(+), 11 deletions(-)
>>
>> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
>> index 8cd26de72f..fe44fd28c3 100644
>> --- a/src/conf/network_conf.c
>> +++ b/src/conf/network_conf.c
>> @@ -901,12 +901,32 @@ virNetworkDNSDefParseXML(const char *networkName,
>> for (i = 0; i < nfwds; i++) {
>> g_autofree char *addr = virXMLPropString(fwdNodes[i],
>> "addr");
>> - if (addr && virSocketAddrParse(&def->forwarders[i].addr,
>> - addr, AF_UNSPEC) < 0) {
>> - virReportError(VIR_ERR_XML_ERROR,
>> - _("Invalid forwarder IP address '%1$s'
>> in network '%2$s'"),
>> - addr, networkName);
>> - return -1;
>> + if (addr) {
>> + int port = -1;
>> + int rc;
>> +
>> + if (virSocketAddrParse(&def->forwarders[i].addr,
>> + addr, AF_UNSPEC) < 0) {
>> + virReportError(VIR_ERR_XML_ERROR,
>> + _("Invalid forwarder IP address
>> '%1$s' in network '%2$s'"),
>> + addr, networkName);
>> + return -1;
>> + }
>> +
>> + if ((rc = virXMLPropInt(fwdNodes[i], "port", 10,
>> + VIR_XML_PROP_NONZERO |
>> + VIR_XML_PROP_NONNEGATIVE,
>> + &port, -1)) < 0) {
>> + return -1;
>> + } else if (rc > 0) {
>> + if (port > 65535) {
>> + virReportError(VIR_ERR_INVALID_ARG,
>> + _("port '%1$d' out of range"),
>> port);
>> + return -1;
>> + }
>> +
>> + virSocketAddrSetPort(&def->forwarders[i].addr,
>> port);
>> + }
>
> One validation hole is that if someone specifies a port without an
> address, the port will just be ignored (and also not stored, so it will
> just disappear from the config).
That's intentional. Hence slightly weird wording in documentation.
>
> Otherwise
>
> Reviewed-by: Laine Stump <[email protected]>
>
>> }
>> def->forwarders[i].domain =
>> virXMLPropString(fwdNodes[i], "domain");
>> if (!(addr || def->forwarders[i].domain)) {
>> @@ -1986,11 +2006,15 @@ virNetworkDNSDefFormat(virBuffer *buf,
>> }
>> if (VIR_SOCKET_ADDR_VALID(&def->forwarders[i].addr)) {
>> g_autofree char *addr = virSocketAddrFormat(&def-
>> >forwarders[i].addr);
>> + int port = virSocketAddrGetPort(&def->forwarders[i].addr);
>> if (!addr)
>> return -1;
>> virBufferAsprintf(buf, " addr='%s'", addr);
>> +
>> + if (port > 0)
>> + virBufferAsprintf(buf, " port='%d'", port);
>> }
>> virBufferAddLit(buf, "/>\n");
>> }
>> diff --git a/src/conf/schemas/network.rng b/src/conf/schemas/network.rng
>> index b7c8551fad..0d293af93b 100644
>> --- a/src/conf/schemas/network.rng
>> +++ b/src/conf/schemas/network.rng
>> @@ -287,6 +287,11 @@
>> <optional>
>> <attribute name="domain"><ref name="dnsName"/></
>> attribute>
>> </optional>
>> + <optional>
>> + <attribute name="port">
>> + <ref name="unsignedShort"/>
>
> You could instead use <ref name="port"/> (which is defined as an integer
> between 1 and 65535), but the only difference is that XML validation
> would flag a "0" value, and the parser already does that (and provides a
> much better error message) so it's kind of pointless (also I looked it
> up and we don't consistently use "port"). (I guess I'm more wondering
> why we bother having the special type in the RNG, rather than wondering
> why you didn't use it - its value seems dubious :-P)
Honestly, I just looked a few lines up and down to see how is the
attribute declared elsewhere and found unsignedShort. But since we
already have "port" type might as well use it.
Thanks,
Michal
