8:16 a.m.
This is rebased version of:
https://listman.redhat.com/archives/libvir-list/2021-December/226045.html
Michal Prívozník (3):
network: Initialize variables in networkDnsmasqConfContents()
network: Separate DHCP config generator into a function
network: Generate TFTP config regardless of DHCP
src/network/bridge_driver.c | 259 ++++++++++--------
.../networkxml2confdata/netboot-network.conf | 4 +-
tests/networkxml2confdata/netboot-tftp.conf | 13 +
tests/networkxml2confdata/netboot-tftp.xml | 9 +
tests/networkxml2conftest.c | 1 +
tests/networkxml2xmlin/netboot-tftp.xml | 1 +
tests/networkxml2xmlout/netboot-tftp.xml | 1 +
tests/networkxml2xmltest.c | 1 +
8 files changed, 170 insertions(+), 119 deletions(-)
create mode 100644 tests/networkxml2confdata/netboot-tftp.conf
create mode 100644 tests/networkxml2confdata/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlin/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlout/netboot-tftp.xml
--
2.35.1
8:17 a.m.
New subject: [PATCH 1/3] network: Initialize variables in networkDnsmasqConfContents()
In networkDnsmasqConfContents() there's a for() loop which
initializes some variables in its initialization block. This
makes both the loop() statement and variable declaration block
look needlessly ugly. Speaking of variable declaration, also move
some variables which are used only within blocks into their
respective blocks.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index ca1e0ca50f..10099571c2 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1004,15 +1004,14 @@ networkDnsmasqConfContents(virNetworkObj *obj,
{
virNetworkDef *def = virNetworkObjGetDef(obj);
g_auto(virBuffer) configbuf = VIR_BUFFER_INITIALIZER;
- int r;
int nbleases = 0;
size_t i;
virNetworkDNSDef *dns = &def->dns;
bool wantDNS = dns->enable != VIR_TRISTATE_BOOL_NO;
- virNetworkIPDef *ipdef;
- virNetworkIPDef *ipv4def;
- virNetworkIPDef *ipv6def;
- bool ipv6SLAAC;
+ virNetworkIPDef *ipdef = NULL;
+ virNetworkIPDef *ipv4def = NULL;
+ virNetworkIPDef *ipv6def = NULL;
+ bool ipv6SLAAC = false;
*configstr = NULL;
@@ -1211,9 +1210,7 @@ networkDnsmasqConfContents(virNetworkObj *obj,
}
/* Find the first dhcp for both IPv4 and IPv6 */
- for (i = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false;
- (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i));
- i++) {
+ for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i)); i++) {
if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
if (ipdef->nranges || ipdef->nhosts) {
if (ipv4def) {
@@ -1255,6 +1252,7 @@ networkDnsmasqConfContents(virNetworkObj *obj,
while (ipdef) {
int prefix;
+ int r;
prefix = virNetworkIPDefPrefix(ipdef);
if (prefix < 0) {
--
2.35.1
8:17 a.m.
New subject: [PATCH 2/3] network: Separate DHCP config generator into a function
Generating configuration file for dnsmasq is done in
networkDnsmasqConfContents() which is this big, self-contained
function. Separate at least DHCP part into its own function for
better readability.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 227 +++++++++++++++++++-----------------
1 file changed, 120 insertions(+), 107 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 10099571c2..f087e07c52 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -994,6 +994,123 @@ networkDnsmasqConfLocalPTRs(virBuffer *buf,
}
+static int
+networkDnsmasqConfDHCP(virBuffer *buf,
+ virNetworkIPDef *ipdef,
+ const char *bridge,
+ int *nbleases,
+ dnsmasqContext *dctx)
+{
+ int r;
+ int prefix;
+
+ if (!ipdef)
+ return 0;
+
+ prefix = virNetworkIPDefPrefix(ipdef);
+ if (prefix < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("bridge '%s' has an invalid prefix"),
+ bridge);
+ return -1;
+ }
+ for (r = 0; r < ipdef->nranges; r++) {
+ int thisRange;
+ virNetworkDHCPRangeDef range = ipdef->ranges[r];
+ g_autofree char *leasetime = NULL;
+ g_autofree char *saddr = NULL;
+ g_autofree char *eaddr = NULL;
+
+ if (!(saddr = virSocketAddrFormat(&range.addr.start)) ||
+ !(eaddr = virSocketAddrFormat(&range.addr.end)))
+ return -1;
+
+ if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) {
+ virBufferAsprintf(buf, "dhcp-range=%s,%s,%d",
+ saddr, eaddr, prefix);
+ } else {
+ /* IPv4 - dnsmasq requires a netmask rather than prefix */
+ virSocketAddr netmask;
+ g_autofree char *netmaskStr = NULL;
+
+ if (virSocketAddrPrefixToNetmask(prefix, &netmask, AF_INET) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Failed to translate bridge '%s' "
+ "prefix %d to netmask"),
+ bridge, prefix);
+ return -1;
+ }
+
+ if (!(netmaskStr = virSocketAddrFormat(&netmask)))
+ return -1;
+ virBufferAsprintf(buf, "dhcp-range=%s,%s,%s",
+ saddr, eaddr, netmaskStr);
+ }
+
+ if ((leasetime = networkBuildDnsmasqLeaseTime(range.lease)))
+ virBufferAsprintf(buf, ",%s", leasetime);
+
+ virBufferAddLit(buf, "\n");
+
+ thisRange = virSocketAddrGetRange(&range.addr.start,
+ &range.addr.end,
+ &ipdef->address,
+ virNetworkIPDefPrefix(ipdef));
+ if (thisRange < 0)
+ return -1;
+ *nbleases += thisRange;
+ }
+
+ /*
+ * For static-only DHCP, i.e. with no range but at least one
+ * host element, we have to add a special --dhcp-range option
+ * to enable the service in dnsmasq. (this is for dhcp-hosts=
+ * support)
+ */
+ if (!ipdef->nranges && ipdef->nhosts) {
+ g_autofree char *bridgeaddr = virSocketAddrFormat(&ipdef->address);
+ if (!bridgeaddr)
+ return -1;
+ virBufferAsprintf(buf, "dhcp-range=%s,static",
+ bridgeaddr);
+ if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
+ virBufferAsprintf(buf, ",%d", prefix);
+ virBufferAddLit(buf, "\n");
+ }
+
+ if (networkBuildDnsmasqDhcpHostsList(dctx, ipdef) < 0)
+ return -1;
+
+ /* Note: the following is IPv4 only */
+ if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
+ if (ipdef->nranges || ipdef->nhosts) {
+ virBufferAddLit(buf, "dhcp-no-override\n");
+ virBufferAddLit(buf, "dhcp-authoritative\n");
+ }
+
+ if (ipdef->tftproot) {
+ virBufferAddLit(buf, "enable-tftp\n");
+ virBufferAsprintf(buf, "tftp-root=%s\n", ipdef->tftproot);
+ }
+
+ if (ipdef->bootfile) {
+ if (VIR_SOCKET_ADDR_VALID(&ipdef->bootserver)) {
+ g_autofree char *bootserver =
virSocketAddrFormat(&ipdef->bootserver);
+
+ if (!bootserver)
+ return -1;
+ virBufferAsprintf(buf, "dhcp-boot=%s%s%s\n",
+ ipdef->bootfile, ",,", bootserver);
+ } else {
+ virBufferAsprintf(buf, "dhcp-boot=%s\n", ipdef->bootfile);
+ }
+ }
+ }
+
+ return 0;
+}
+
+
int
networkDnsmasqConfContents(virNetworkObj *obj,
const char *pidfile,
@@ -1248,113 +1365,9 @@ networkDnsmasqConfContents(virNetworkObj *obj,
"on the same network interface.");
}
- ipdef = ipv4def ? ipv4def : ipv6def;
-
- while (ipdef) {
- int prefix;
- int r;
-
- prefix = virNetworkIPDefPrefix(ipdef);
- if (prefix < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("bridge '%s' has an invalid prefix"),
- def->bridge);
- return -1;
- }
- for (r = 0; r < ipdef->nranges; r++) {
- int thisRange;
- virNetworkDHCPRangeDef range = ipdef->ranges[r];
- g_autofree char *leasetime = NULL;
- g_autofree char *saddr = NULL;
- g_autofree char *eaddr = NULL;
-
- if (!(saddr = virSocketAddrFormat(&range.addr.start)) ||
- !(eaddr = virSocketAddrFormat(&range.addr.end)))
- return -1;
-
- if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) {
- virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%d",
- saddr, eaddr, prefix);
- } else {
- /* IPv4 - dnsmasq requires a netmask rather than prefix */
- virSocketAddr netmask;
- g_autofree char *netmaskStr = NULL;
-
- if (virSocketAddrPrefixToNetmask(prefix, &netmask, AF_INET) < 0)
{
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Failed to translate bridge '%s'
"
- "prefix %d to netmask"),
- def->bridge, prefix);
- return -1;
- }
-
- if (!(netmaskStr = virSocketAddrFormat(&netmask)))
- return -1;
- virBufferAsprintf(&configbuf, "dhcp-range=%s,%s,%s",
- saddr, eaddr, netmaskStr);
- }
-
- if ((leasetime = networkBuildDnsmasqLeaseTime(range.lease)))
- virBufferAsprintf(&configbuf, ",%s", leasetime);
-
- virBufferAddLit(&configbuf, "\n");
-
- thisRange = virSocketAddrGetRange(&range.addr.start,
- &range.addr.end,
- &ipdef->address,
- virNetworkIPDefPrefix(ipdef));
- if (thisRange < 0)
- return -1;
- nbleases += thisRange;
- }
-
- /*
- * For static-only DHCP, i.e. with no range but at least one
- * host element, we have to add a special --dhcp-range option
- * to enable the service in dnsmasq. (this is for dhcp-hosts=
- * support)
- */
- if (!ipdef->nranges && ipdef->nhosts) {
- g_autofree char *bridgeaddr = virSocketAddrFormat(&ipdef->address);
- if (!bridgeaddr)
- return -1;
- virBufferAsprintf(&configbuf, "dhcp-range=%s,static",
- bridgeaddr);
- if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
- virBufferAsprintf(&configbuf, ",%d", prefix);
- virBufferAddLit(&configbuf, "\n");
- }
-
- if (networkBuildDnsmasqDhcpHostsList(dctx, ipdef) < 0)
- return -1;
-
- /* Note: the following is IPv4 only */
- if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) {
- if (ipdef->nranges || ipdef->nhosts) {
- virBufferAddLit(&configbuf, "dhcp-no-override\n");
- virBufferAddLit(&configbuf, "dhcp-authoritative\n");
- }
-
- if (ipdef->tftproot) {
- virBufferAddLit(&configbuf, "enable-tftp\n");
- virBufferAsprintf(&configbuf, "tftp-root=%s\n",
ipdef->tftproot);
- }
-
- if (ipdef->bootfile) {
- if (VIR_SOCKET_ADDR_VALID(&ipdef->bootserver)) {
- g_autofree char *bootserver =
virSocketAddrFormat(&ipdef->bootserver);
-
- if (!bootserver)
- return -1;
- virBufferAsprintf(&configbuf, "dhcp-boot=%s%s%s\n",
- ipdef->bootfile, ",,", bootserver);
- } else {
- virBufferAsprintf(&configbuf, "dhcp-boot=%s\n",
ipdef->bootfile);
- }
- }
- }
- ipdef = (ipdef == ipv6def) ? NULL : ipv6def;
- }
+ if (networkDnsmasqConfDHCP(&configbuf, ipv4def, def->bridge, &nbleases,
dctx) < 0 ||
+ networkDnsmasqConfDHCP(&configbuf, ipv6def, def->bridge, &nbleases,
dctx) < 0)
+ return -1;
if (nbleases > 0)
virBufferAsprintf(&configbuf, "dhcp-lease-max=%d\n", nbleases);
--
2.35.1
8:17 a.m.
New subject: [PATCH 3/3] network: Generate TFTP config regardless of DHCP
We already allow users to provide TFTP root path in network XML
and not specify any DHCP. This makes sense, because dnsmasq is
not only DHCP server but also TFTP server and users might have
a DHCP server configured on their own, outside of libvirt's
control and want just the TFTP part.
By moving TFTP config generator out of DHCP generator and calling
it for every IPv4 range, users can finally enable just TFTP.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2026765
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 30 ++++++++++++++-----
.../networkxml2confdata/netboot-network.conf | 4 +--
tests/networkxml2confdata/netboot-tftp.conf | 13 ++++++++
tests/networkxml2confdata/netboot-tftp.xml | 9 ++++++
tests/networkxml2conftest.c | 1 +
tests/networkxml2xmlin/netboot-tftp.xml | 1 +
tests/networkxml2xmlout/netboot-tftp.xml | 1 +
tests/networkxml2xmltest.c | 1 +
8 files changed, 50 insertions(+), 10 deletions(-)
create mode 100644 tests/networkxml2confdata/netboot-tftp.conf
create mode 100644 tests/networkxml2confdata/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlin/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlout/netboot-tftp.xml
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index f087e07c52..7449c7e02a 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1088,11 +1088,6 @@ networkDnsmasqConfDHCP(virBuffer *buf,
virBufferAddLit(buf, "dhcp-authoritative\n");
}
- if (ipdef->tftproot) {
- virBufferAddLit(buf, "enable-tftp\n");
- virBufferAsprintf(buf, "tftp-root=%s\n", ipdef->tftproot);
- }
-
if (ipdef->bootfile) {
if (VIR_SOCKET_ADDR_VALID(&ipdef->bootserver)) {
g_autofree char *bootserver =
virSocketAddrFormat(&ipdef->bootserver);
@@ -1111,6 +1106,22 @@ networkDnsmasqConfDHCP(virBuffer *buf,
}
+static void
+networkDnsmasqConfTFTP(virBuffer *buf,
+ virNetworkIPDef *ipdef,
+ bool *enableTFTP)
+{
+ if (!ipdef->tftproot)
+ return;
+
+ if (!*enableTFTP) {
+ virBufferAddLit(buf, "enable-tftp\n");
+ *enableTFTP = true;
+ }
+ virBufferAsprintf(buf, "tftp-root=%s\n", ipdef->tftproot);
+}
+
+
int
networkDnsmasqConfContents(virNetworkObj *obj,
const char *pidfile,
@@ -1129,6 +1140,7 @@ networkDnsmasqConfContents(virNetworkObj *obj,
virNetworkIPDef *ipv4def = NULL;
virNetworkIPDef *ipv6def = NULL;
bool ipv6SLAAC = false;
+ bool enableTFTP = false;
*configstr = NULL;
@@ -1339,6 +1351,8 @@ networkDnsmasqConfContents(virNetworkObj *obj,
ipv4def = ipdef;
}
}
+
+ networkDnsmasqConfTFTP(&configbuf, ipdef, &enableTFTP);
}
if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) {
if (ipdef->nranges || ipdef->nhosts) {
@@ -1500,7 +1514,7 @@ networkStartDhcpDaemon(virNetworkDriverState *driver,
i = 0;
while ((ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i))) {
i++;
- if (ipdef->nranges || ipdef->nhosts)
+ if (ipdef->nranges || ipdef->nhosts || ipdef->tftproot)
needDnsmasq = true;
}
@@ -3255,7 +3269,7 @@ networkUpdate(virNetworkPtr net,
for (i = 0;
(ipdef = virNetworkDefGetIPByIndex(def, AF_INET, i));
i++) {
- if (ipdef->nranges || ipdef->nhosts) {
+ if (ipdef->nranges || ipdef->nhosts || ipdef->tftproot) {
oldDhcpActive = true;
break;
}
@@ -3370,7 +3384,7 @@ networkUpdate(virNetworkPtr net,
for (i = 0; (ipdef = virNetworkDefGetIPByIndex(def, AF_INET, i));
i++) {
- if (ipdef->nranges || ipdef->nhosts) {
+ if (ipdef->nranges || ipdef->nhosts || ipdef->tftproot) {
newDhcpActive = true;
break;
}
diff --git a/tests/networkxml2confdata/netboot-network.conf
b/tests/networkxml2confdata/netboot-network.conf
index a13239a54f..32ef25b05f 100644
--- a/tests/networkxml2confdata/netboot-network.conf
+++ b/tests/networkxml2confdata/netboot-network.conf
@@ -10,11 +10,11 @@ expand-hosts
except-interface=lo
bind-dynamic
interface=virbr1
+enable-tftp
+tftp-root=/var/lib/tftproot
dhcp-range=192.168.122.2,192.168.122.254,255.255.255.0
dhcp-no-override
dhcp-authoritative
-enable-tftp
-tftp-root=/var/lib/tftproot
dhcp-boot=pxeboot.img
dhcp-lease-max=253
dhcp-hostsfile=/var/lib/libvirt/dnsmasq/netboot.hostsfile
diff --git a/tests/networkxml2confdata/netboot-tftp.conf
b/tests/networkxml2confdata/netboot-tftp.conf
new file mode 100644
index 0000000000..45615f3c33
--- /dev/null
+++ b/tests/networkxml2confdata/netboot-tftp.conf
@@ -0,0 +1,13 @@
+##WARNING: THIS IS AN AUTO-GENERATED FILE. CHANGES TO IT ARE LIKELY TO BE
+##OVERWRITTEN AND LOST. Changes to this configuration should be made using:
+## virsh net-edit tftp-only
+## or other application using the libvirt API.
+##
+## dnsmasq conf file created by libvirt
+strict-order
+except-interface=lo
+bind-dynamic
+interface=virbr0
+enable-tftp
+tftp-root=/var/lib/tftproot
+addn-hosts=/var/lib/libvirt/dnsmasq/tftp-only.addnhosts
diff --git a/tests/networkxml2confdata/netboot-tftp.xml
b/tests/networkxml2confdata/netboot-tftp.xml
new file mode 100644
index 0000000000..297f5a7ba1
--- /dev/null
+++ b/tests/networkxml2confdata/netboot-tftp.xml
@@ -0,0 +1,9 @@
+<network>
+ <name>tftp-only</name>
+ <uuid>eb486e5c-4df5-42ee-ae4a-ad8557998d00</uuid>
+ <forward mode='nat'/>
+ <bridge name='virbr0' stp='off' delay='1'/>
+ <ip address='192.168.122.1' netmask='255.255.255.0'>
+ <tftp root='/var/lib/tftproot'/>
+ </ip>
+</network>
diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c
index 0bc9e128e3..a062ff9c3c 100644
--- a/tests/networkxml2conftest.c
+++ b/tests/networkxml2conftest.c
@@ -168,6 +168,7 @@ mymain(void)
DO_TEST("isolated-network", full);
DO_TEST("netboot-network", full);
DO_TEST("netboot-proxy-network", full);
+ DO_TEST("netboot-tftp", full);
DO_TEST("nat-network-dns-srv-record-minimal", full);
DO_TEST("nat-network-name-with-quotes", full);
DO_TEST("routed-network", full);
diff --git a/tests/networkxml2xmlin/netboot-tftp.xml
b/tests/networkxml2xmlin/netboot-tftp.xml
new file mode 120000
index 0000000000..1487de558b
--- /dev/null
+++ b/tests/networkxml2xmlin/netboot-tftp.xml
@@ -0,0 +1 @@
+../networkxml2confdata/netboot-tftp.xml
\ No newline at end of file
diff --git a/tests/networkxml2xmlout/netboot-tftp.xml
b/tests/networkxml2xmlout/netboot-tftp.xml
new file mode 120000
index 0000000000..1487de558b
--- /dev/null
+++ b/tests/networkxml2xmlout/netboot-tftp.xml
@@ -0,0 +1 @@
+../networkxml2confdata/netboot-tftp.xml
\ No newline at end of file
diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c
index ca24305ace..9e8d675a10 100644
--- a/tests/networkxml2xmltest.c
+++ b/tests/networkxml2xmltest.c
@@ -127,6 +127,7 @@ mymain(void)
DO_TEST("nat-network");
DO_TEST("netboot-network");
DO_TEST("netboot-proxy-network");
+ DO_TEST("netboot-tftp");
DO_TEST("nat-network-dns-txt-record");
DO_TEST("nat-network-dns-srv-record");
DO_TEST("nat-network-dns-srv-records");
--
2.35.1
2:28 a.m.
On a Monday in 2022, Michal Privoznik wrote:
This is rebased version of:
https://listman.redhat.com/archives/libvir-list/2021-December/226045.html
Michal Prívozník (3):
network: Initialize variables in networkDnsmasqConfContents()
network: Separate DHCP config generator into a function
network: Generate TFTP config regardless of DHCP
src/network/bridge_driver.c | 259 ++++++++++--------
.../networkxml2confdata/netboot-network.conf | 4 +-
tests/networkxml2confdata/netboot-tftp.conf | 13 +
tests/networkxml2confdata/netboot-tftp.xml | 9 +
tests/networkxml2conftest.c | 1 +
tests/networkxml2xmlin/netboot-tftp.xml | 1 +
tests/networkxml2xmlout/netboot-tftp.xml | 1 +
tests/networkxml2xmltest.c | 1 +
8 files changed, 170 insertions(+), 119 deletions(-)
create mode 100644 tests/networkxml2confdata/netboot-tftp.conf
create mode 100644 tests/networkxml2confdata/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlin/netboot-tftp.xml
create mode 120000 tests/networkxml2xmlout/netboot-tftp.xml
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
904
days inactive
913
days old
4 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik