[libvirt] [RFC,PATCH] network: add 'netboot' option to dhcp config
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options. This change introduces a 'netboot' tag to the dhcp configuration: <network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <netboot root="/srv/tftp" file="pxeboot.img"/> </dhcp> </ip> </network> When root= and file= attributes are present, these are passed to the arguments to dnsmasq: dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> --- docs/formatnetwork.html.in | 6 ++++++ docs/schemas/network.rng | 6 ++++++ src/network_conf.c | 28 ++++++++++++++++++++++++++++ src/network_conf.h | 9 +++++++++ src/network_driver.c | 10 ++++++++++ 5 files changed, 59 insertions(+) diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index fd68430..3186498 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -138,6 +138,12 @@ name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> </dd> + <dt><code>netboot</code></dt> + <dd>The optional <code>netboot</code> element specified network boot + options to be provided by the DHCP server. Two attributes are + required: <code>root</code> (the root path of the TFTP server), and + <code>file</code> (the file to be used for the boot image). + </dd> </dl> <h2><a name="examples">Example configuration</a></h2> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index a4281a5..571e916 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -76,6 +76,12 @@ <attribute name="ip"><text/></attribute> </element> </zeroOrMore> + <optional> + <element name="netboot"> + <attribute name="root"><text/></attribute> + <attribute name="file"><text/></attribute> + </element> + </optional> </element> </element> </optional> diff --git a/src/network_conf.c b/src/network_conf.c index 3764bb4..87bb5e4 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -115,6 +115,9 @@ void virNetworkDefFree(virNetworkDefPtr def) } VIR_FREE(def->hosts); + VIR_FREE(def->tftproot); + VIR_FREE(def->bootfile); + VIR_FREE(def); } @@ -299,6 +302,24 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, def->hosts[def->nhosts].name = (char *)name; def->hosts[def->nhosts].ip = (char *)ip; def->nhosts++; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "netboot")) { + xmlChar *root, *file; + + if (!(root = xmlGetProp(cur, BAD_CAST "root"))) { + cur = cur->next; + continue; + } + + if (!(file = xmlGetProp(cur, BAD_CAST "file"))) { + cur = cur->next; + xmlFree(root); + continue; + } + + def->tftproot = (char *)root; + def->bootfile = (char *)file; } cur = cur->next; @@ -621,6 +642,13 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, "ip='%s' ", def->hosts[i].ip); virBufferAddLit(&buf, "/>\n"); } + if (virNetworkDefProvidesNetboot(def)) { + virBufferAddLit(&buf, " <netboot"); + virBufferEscapeString(&buf, " root='%s'", def->tftproot); + virBufferEscapeString(&buf, " file='%s'", def->bootfile); + virBufferAddLit(&buf, " />\n"); + } + virBufferAddLit(&buf, " </dhcp>\n"); } diff --git a/src/network_conf.h b/src/network_conf.h index 4076f9a..f6f7c1e 100644 --- a/src/network_conf.h +++ b/src/network_conf.h @@ -78,8 +78,17 @@ struct _virNetworkDef { unsigned int nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; + + char *tftproot; + char *bootfile; }; +static inline int +virNetworkDefProvidesNetboot(const virNetworkDefPtr def) +{ + return def->tftproot && def->bootfile; +} + typedef struct _virNetworkObj virNetworkObj; typedef virNetworkObj *virNetworkObjPtr; struct _virNetworkObj { diff --git a/src/network_driver.c b/src/network_driver.c index 49855bf..cf462f2 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -400,6 +400,8 @@ networkBuildDnsmasqArgv(virConnectPtr conn, (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ (2 * network->def->nhosts) + + /* --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img */ + (virNetworkDefProvidesNetboot(network->def) ? 5 : 0) + 1; /* NULL */ if (VIR_ALLOC_N(*argv, len) < 0) @@ -478,6 +480,14 @@ networkBuildDnsmasqArgv(virConnectPtr conn, APPEND_ARG(*argv, i++, buf); } + if (virNetworkDefProvidesNetboot(network->def)) { + APPEND_ARG(*argv, i++, "--enable-tftp"); + APPEND_ARG(*argv, i++, "--tftp-root"); + APPEND_ARG(*argv, i++, network->def->tftproot); + APPEND_ARG(*argv, i++, "--dhcp-boot"); + APPEND_ARG(*argv, i++, network->def->bootfile); + } + #undef APPEND_ARG return 0;
On Fri, 2009-09-11 at 14:47 +1000, Jeremy Kerr wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces a 'netboot' tag to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <netboot root="/srv/tftp" file="pxeboot.img"/> </dhcp> </ip> </network>
When root= and file= attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
I very much like this idea - e.g. I'd really like to have this to give people simple instructions for testing gPXE in next week's Fedora Test Day. The argument was made before that it's pointless to use PXE like this when you can just explicitly configure a kernel/initrd, but that misses the point that sometimes you do explicitly want to use PXE, even just for testing purposes. Patch looks good to me too, ACK Cheers, Mark.
On Fri, 2009-09-11 at 09:22 +0100, Mark McLoughlin wrote:
On Fri, 2009-09-11 at 14:47 +1000, Jeremy Kerr wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces a 'netboot' tag to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <netboot root="/srv/tftp" file="pxeboot.img"/> </dhcp> </ip> </network>
When root= and file= attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
I very much like this idea - e.g. I'd really like to have this to give people simple instructions for testing gPXE in next week's Fedora Test Day.
The argument was made before that it's pointless to use PXE like this when you can just explicitly configure a kernel/initrd, but that misses the point that sometimes you do explicitly want to use PXE, even just for testing purposes.
Patch looks good to me too, ACK
Previous discussion was here: http://www.redhat.com/archives/fedora-virt/2009-June/msg00154.html Cheers, Mark.
I very much like this idea - e.g. I'd really like to have this to give people simple instructions for testing gPXE in next week's Fedora Test Day.
On the subject of gPXE has anybody else found that it doesn't seem to be working at all? I'm using the virt-preview packages on F11 and ever since qemu switched to use gPXE network booting has been completely broken. I get the gPXE banner message appear and then nothing happens. Monitoring the network shows no signs of any DHCP requests being sent at all. Tom -- Tom Hughes (tom@compton.nu) http://www.compton.nu/
On Fri, 2009-09-11 at 09:44 +0100, Tom Hughes wrote:
I very much like this idea - e.g. I'd really like to have this to give people simple instructions for testing gPXE in next week's Fedora Test Day.
On the subject of gPXE has anybody else found that it doesn't seem to be working at all?
I'm using the virt-preview packages on F11 and ever since qemu switched to use gPXE network booting has been completely broken. I get the gPXE banner message appear and then nothing happens. Monitoring the network shows no signs of any DHCP requests being sent at all.
It's probably this: https://bugzilla.redhat.com/512358 It's fixed in the F-12 kernel, but we need that fix backported to F-11. Cheers, Mark.
On Fri, Sep 11, 2009 at 09:22:09AM +0100, Mark McLoughlin wrote:
On Fri, 2009-09-11 at 14:47 +1000, Jeremy Kerr wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces a 'netboot' tag to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <netboot root="/srv/tftp" file="pxeboot.img"/> </dhcp> </ip> </network>
When root= and file= attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
I very much like this idea - e.g. I'd really like to have this to give people simple instructions for testing gPXE in next week's Fedora Test Day.
The argument was made before that it's pointless to use PXE like this when you can just explicitly configure a kernel/initrd, but that misses the point that sometimes you do explicitly want to use PXE, even just for testing purposes.
Patch looks good to me too, ACK
I like the idea too. But this opens the door to outside access or just limits it to the guest ? In any case make sure you have an up to date dnsmasq https://cert.belnet.be/belnetadvisories/rhsa-20091238-01-important-dnsmasq-s... Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On 09/11/2009 06:47 AM, Jeremy Kerr wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces a 'netboot' tag to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <netboot root="/srv/tftp" file="pxeboot.img"/> </dhcp> </ip> </network>
When root= and file= attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
Since there is only one TFTP server running in the network, it is not possible to specify different roots for different dhcp ranges. I think the schema should be <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/srv/tftp"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> where in the future the bootp argument could grow a server attribute as mentioned by Jeremy. Paolo
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options. This change introduces the appropriate tags to the dhcp configuration: <network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network> When the attributes are present, these are passed to the arguments to dnsmasq: dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp /> At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> 2009-09-14 Paolo Bonzini <pbonzini@redhat.com> Jeremy Kerr <jk@ozlabs.org> * docs/formatnetwork.html.in: Document new tags. * docs/formatnetwork.html: Regenerate. * docs/schemas/network.rng: Update. * src/network_conf.c (virNetworkDefFree): Free new fields. (virNetworkDHCPRangeDefParseXML): Parse <bootp>. (virNetworkIPParseXML): New, parsing <dhcp> and <tftp>. (virNetworkDefParseXML): Use virNetworkIPParseXML instead of virNetworkDHCPRangeDefParseXML. (virNetworkDefFormat): Pretty print new fields. * src/network_conf.h (struct _virNetworkDef): Add netboot fields. * src/network_driver.c (networkBuildDnsmasqArgv): Add TFTP and BOOTP arguments. * tests/Makefile.am (EXTRA_DIST): Add networkschemadata. * tests/networkschematest: Look in networkschemadata. * tests/networkschemadata/netboot-network.xml: New. --- docs/formatnetwork.html | 12 +++++- docs/formatnetwork.html.in | 12 +++++- docs/schemas/network.rng | 10 ++++ src/network_conf.c | 60 +++++++++++++++++++++++++- src/network_conf.h | 3 + src/network_driver.c | 14 ++++++ tests/Makefile.am | 1 + tests/networkschemadata/netboot-network.xml | 12 +++++ tests/networkschematest | 2 +- 9 files changed, 120 insertions(+), 6 deletions(-) create mode 100644 tests/networkschemadata/netboot-network.xml diff --git a/docs/formatnetwork.html b/docs/formatnetwork.html index 845e558..1f01f5d 100644 --- a/docs/formatnetwork.html +++ b/docs/formatnetwork.html @@ -235,7 +235,13 @@ address will be their default route. The <code>netmask</code> attribute defines the significant bits of the network address, again specified in dotted-decimal format. <span class="since">Since 0.3.0</span> - </dd><dt><code>dhcp</code></dt><dd>Immediately within the <code>ip</code> element there is an + </dd><dt><code>tftp</code></dt><dd>Immediately within + the <code>ip</code> element there is an optional <code>tftp</code> + element. The presence of this element and of its attribute + <code>root</code> enables TFTP services. The attribute specifies + the path to the root directory served via TFTP. + <span class="since">Since 0.7.1</span> + </dd><dt><code>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further contain one or more <code>range</code> elements. @@ -253,6 +259,10 @@ assigned to that host (via the <code>ip</code> attribute), and the name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> + </dd><dt><code>bootp</code></dt><dd>The optional <code>bootp</code> + element specifies BOOTP options to be provided by the DHCP server. + Only one attribute is supported, <code>file</code>, giving the file + to be used for the boot image). <span class="since">Since 0.7.1.</span> </dd></dl> <h2> <a name="examples" id="examples">Example configuration</a> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index fd68430..fb93a12 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -114,7 +114,13 @@ attribute defines the significant bits of the network address, again specified in dotted-decimal format. <span class="since">Since 0.3.0</span> </dd> - <dt><code>dhcp</code></dt> + </dd><dt><code>tftp</code></dt><dd>Immediately within + the <code>ip</code> element there is an optional <code>tftp</code> + element. The presence of this element and of its attribute + <code>root</code> enables TFTP services. The attribute specifies + the path to the root directory served via TFTP. + <span class="since">Since 0.7.1</span> + </dd><dt><code>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an <dd>Immediately within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further @@ -138,6 +144,10 @@ name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> </dd> + </dd><dt><code>bootp</code></dt><dd>The optional <code>bootp</code> + element specifies BOOTP options to be provided by the DHCP server. + Only one attribute is supported, <code>file</code>, giving the file + to be used for the boot image). <span class="since">Since 0.7.1.</span> </dl> <h2><a name="examples">Example configuration</a></h2> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index a4281a5..042e013 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -60,6 +60,11 @@ <optional> <attribute name="netmask"><text/></attribute> </optional> + <optional> + <element name="tftp"> + <attribute name="root"><text/></attribute> + </element> + </optional> <!-- Define the range(s) of IP addresses that the DHCP server should hand out --> <element name="dhcp"> @@ -76,6 +81,11 @@ <attribute name="ip"><text/></attribute> </element> </zeroOrMore> + <optional> + <element name="bootp"> + <attribute name="file"><text/></attribute> + </element> + </optional> </element> </element> </optional> diff --git a/src/network_conf.c b/src/network_conf.c index 3764bb4..14eb543 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -115,6 +115,9 @@ void virNetworkDefFree(virNetworkDefPtr def) } VIR_FREE(def->hosts); + VIR_FREE(def->tftproot); + VIR_FREE(def->bootfile); + VIR_FREE(def); } @@ -299,6 +302,17 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, def->hosts[def->nhosts].name = (char *)name; def->hosts[def->nhosts].ip = (char *)ip; def->nhosts++; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "bootp")) { + xmlChar *file; + + if (!(file = xmlGetProp(cur, BAD_CAST "file"))) { + cur = cur->next; + continue; + } + + def->bootfile = (char *)file; } cur = cur->next; @@ -307,6 +321,37 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, return 0; } +static int +virNetworkIPParseXML(virConnectPtr conn, + virNetworkDefPtr def, + xmlNodePtr node) { + xmlNodePtr cur; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "dhcp")) { + int result = virNetworkDHCPRangeDefParseXML(conn, def, cur); + if (result) + return result; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "tftp")) { + xmlChar *root; + + if (!(root = xmlGetProp(cur, BAD_CAST "root"))) { + cur = cur->next; + continue; + } + + def->tftproot = (char *)root; + } + + cur = cur->next; + } + return 0; +} + static virNetworkDefPtr virNetworkDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) @@ -363,7 +408,7 @@ virNetworkDefParseXML(virConnectPtr conn, /* XXX someday we want IPv6 too, so inet_aton won't work there */ struct in_addr inaddress, innetmask; char *netaddr; - xmlNodePtr dhcp; + xmlNodePtr ip; if (inet_pton(AF_INET, def->ipAddress, &inaddress) <= 0) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -386,8 +431,8 @@ virNetworkDefParseXML(virConnectPtr conn, goto error; } - if ((dhcp = virXPathNode(conn, "./ip[1]/dhcp[1]", ctxt)) && - virNetworkDHCPRangeDefParseXML(conn, def, dhcp) < 0) + if ((ip = virXPathNode(conn, "./ip[1]", ctxt)) && + virNetworkIPParseXML(conn, def, ip) < 0) goto error; } @@ -605,6 +650,10 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferAddLit(&buf, ">\n"); + if (def->tftproot) { + virBufferEscapeString(&buf, " <tftp root='%s' />\n", + def->tftproot); + } if ((def->nranges || def->nhosts)) { int i; virBufferAddLit(&buf, " <dhcp>\n"); @@ -621,6 +670,11 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, "ip='%s' ", def->hosts[i].ip); virBufferAddLit(&buf, "/>\n"); } + if (def->bootfile) { + virBufferEscapeString(&buf, " <bootp file='%s' />\n", + def->bootfile); + } + virBufferAddLit(&buf, " </dhcp>\n"); } diff --git a/src/network_conf.h b/src/network_conf.h index 4076f9a..e983a01 100644 --- a/src/network_conf.h +++ b/src/network_conf.h @@ -78,6 +78,9 @@ struct _virNetworkDef { unsigned int nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; + + char *tftproot; + char *bootfile; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/network_driver.c b/src/network_driver.c index 49855bf..2fe5c00 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -400,6 +400,10 @@ networkBuildDnsmasqArgv(virConnectPtr conn, (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ (2 * network->def->nhosts) + + /* --enable-tftp --tftp-root /srv/tftp */ + (network->def->tftproot ? 3 : 0) + + /* --dhcp-boot pxeboot.img */ + (network->def->bootfile ? 2 : 0) + 1; /* NULL */ if (VIR_ALLOC_N(*argv, len) < 0) @@ -478,6 +482,16 @@ networkBuildDnsmasqArgv(virConnectPtr conn, APPEND_ARG(*argv, i++, buf); } + if (network->def->tftproot) { + APPEND_ARG(*argv, i++, "--enable-tftp"); + APPEND_ARG(*argv, i++, "--tftp-root"); + APPEND_ARG(*argv, i++, network->def->tftproot); + } + if (network->def->bootfile) { + APPEND_ARG(*argv, i++, "--dhcp-boot"); + APPEND_ARG(*argv, i++, network->def->bootfile); + } + #undef APPEND_ARG return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index 74e98d1..2699343 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -51,6 +51,7 @@ EXTRA_DIST = \ capabilityschematest \ capabilityschemadata \ networkschematest \ + networkschemadata \ domainschematest \ domainschemadata \ interfaceschemadata \ diff --git a/tests/networkschemadata/netboot-network.xml b/tests/networkschemadata/netboot-network.xml new file mode 100644 index 0000000..7274ee6 --- /dev/null +++ b/tests/networkschemadata/netboot-network.xml @@ -0,0 +1,12 @@ +<network> + <name>netboot</name> + <bridge name="virbr1" /> + <forward/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <tftp root="/var/lib/tftproot" /> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254" /> + <bootp file="pxeboot.img" /> + </dhcp> + </ip> +</network> diff --git a/tests/networkschematest b/tests/networkschematest index 1d7cffc..a1491e1 100755 --- a/tests/networkschematest +++ b/tests/networkschematest @@ -3,7 +3,7 @@ test -z "$srcdir" && srcdir=`pwd` test -z "$abs_srcdir" && abs_srcdir=`pwd` -DIRS="../qemud" +DIRS="../qemud networkschemadata" n=0 f=0 -- 1.6.2.5
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options. This change introduces the appropriate tags to the dhcp configuration: <network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network> When the attributes are present, these are passed to the arguments to dnsmasq: dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp /> Possible future features in this field include the following: - At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute to <bootp>. - Currently, the BOOTP file cannot be assigned to a specific <range> or <host>. Even if this was implemented, it would still make sense to allow <bootp> appearing directly within the <dhcp> element, providing a global default. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> 2009-09-15 Paolo Bonzini <pbonzini@redhat.com> Jeremy Kerr <jk@ozlabs.org> * docs/formatnetwork.html.in: Document new tags. * docs/formatnetwork.html: Regenerate. * docs/schemas/network.rng: Update. * src/network_conf.c (virNetworkDefFree): Free new fields. (virNetworkDHCPRangeDefParseXML): Parse <bootp>. (virNetworkIPParseXML): New, parsing <dhcp> and <tftp>. (virNetworkDefParseXML): Use virNetworkIPParseXML instead of virNetworkDHCPRangeDefParseXML. (virNetworkDefFormat): Pretty print new fields. * src/network_conf.h (struct _virNetworkDef): Add netboot fields. * src/network_driver.c (networkBuildDnsmasqArgv): Add TFTP and BOOTP arguments. * tests/Makefile.am (EXTRA_DIST): Add networkschemadata. * tests/networkschematest: Look in networkschemadata. * tests/networkschemadata/netboot-network.xml: New. --- Compared to v1, I fixed wrong HTML that was pointed out to me offline by Jiri Denemark. Note that "make" will give errors for it but will _not_ abort. docs/formatnetwork.html | 15 ++++++- docs/formatnetwork.html.in | 17 ++++++-- docs/schemas/network.rng | 10 ++++ src/network_conf.c | 60 +++++++++++++++++++++++++- src/network_conf.h | 3 + src/network_driver.c | 14 ++++++ tests/Makefile.am | 1 + tests/networkschemadata/netboot-network.xml | 12 +++++ tests/networkschematest | 2 +- 9 files changed, 124 insertions(+), 10 deletions(-) create mode 100644 tests/networkschemadata/netboot-network.xml diff --git a/docs/formatnetwork.html b/docs/formatnetwork.html index 845e558..e97b9be 100644 --- a/docs/formatnetwork.html +++ b/docs/formatnetwork.html @@ -235,7 +235,13 @@ address will be their default route. The <code>netmask</code> attribute defines the significant bits of the network address, again specified in dotted-decimal format. <span class="since">Since 0.3.0</span> - </dd><dt><code>dhcp</code></dt><dd>Immediately within the <code>ip</code> element there is an + </dd><dt><code>tftp</code></dt><dd>Immediately within + the <code>ip</code> element there is an optional <code>tftp</code> + element. The presence of this element and of its attribute + <code>root</code> enables TFTP services. The attribute specifies + the path to the root directory served via TFTP. + <span class="since">Since 0.7.1</span> + </dd><dt><code>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further contain one or more <code>range</code> elements. @@ -253,7 +259,12 @@ assigned to that host (via the <code>ip</code> attribute), and the name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> - </dd></dl> + </dd><dt><code>bootp</code></dt><dd>The optional <code>bootp</code> + element specifies BOOTP options to be provided by the DHCP server. + Only one attribute is supported, <code>file</code>, giving the file + to be used for the boot image). The BOOTP options currently have to + be the same for all address ranges and statically assigned addresses.<span class="since">Since 0.7.1.</span> + </dd></dl> <h2> <a name="examples" id="examples">Example configuration</a> </h2> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index fd68430..e471385 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -113,9 +113,13 @@ address will be their default route. The <code>netmask</code> attribute defines the significant bits of the network address, again specified in dotted-decimal format. <span class="since">Since 0.3.0</span> - </dd> - <dt><code>dhcp</code></dt> - <dd>Immediately within the <code>ip</code> element there is an + </dd><dt><code>tftp</code></dt><dd>Immediately within + the <code>ip</code> element there is an optional <code>tftp</code> + element. The presence of this element and of its attribute + <code>root</code> enables TFTP services. The attribute specifies + the path to the root directory served via TFTP. + <span class="since">Since 0.7.1</span> + </dd><dt><code>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further contain one or more <code>range</code> elements. @@ -137,7 +141,12 @@ assigned to that host (via the <code>ip</code> attribute), and the name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> - </dd> + </dd><dt><code>bootp</code></dt><dd>The optional <code>bootp</code> + element specifies BOOTP options to be provided by the DHCP server. + Only one attribute is supported, <code>file</code>, giving the file + to be used for the boot image). The BOOTP options currently have to + be the same for all address ranges and statically assigned addresses.<span + class="since">Since 0.7.1.</span> </dl> <h2><a name="examples">Example configuration</a></h2> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index a4281a5..042e013 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -60,6 +60,11 @@ <optional> <attribute name="netmask"><text/></attribute> </optional> + <optional> + <element name="tftp"> + <attribute name="root"><text/></attribute> + </element> + </optional> <!-- Define the range(s) of IP addresses that the DHCP server should hand out --> <element name="dhcp"> @@ -76,6 +81,11 @@ <attribute name="ip"><text/></attribute> </element> </zeroOrMore> + <optional> + <element name="bootp"> + <attribute name="file"><text/></attribute> + </element> + </optional> </element> </element> </optional> diff --git a/src/network_conf.c b/src/network_conf.c index 3764bb4..14eb543 100644 --- a/src/network_conf.c +++ b/src/network_conf.c @@ -115,6 +115,9 @@ void virNetworkDefFree(virNetworkDefPtr def) } VIR_FREE(def->hosts); + VIR_FREE(def->tftproot); + VIR_FREE(def->bootfile); + VIR_FREE(def); } @@ -299,6 +302,17 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, def->hosts[def->nhosts].name = (char *)name; def->hosts[def->nhosts].ip = (char *)ip; def->nhosts++; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "bootp")) { + xmlChar *file; + + if (!(file = xmlGetProp(cur, BAD_CAST "file"))) { + cur = cur->next; + continue; + } + + def->bootfile = (char *)file; } cur = cur->next; @@ -307,6 +321,37 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, return 0; } +static int +virNetworkIPParseXML(virConnectPtr conn, + virNetworkDefPtr def, + xmlNodePtr node) { + xmlNodePtr cur; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "dhcp")) { + int result = virNetworkDHCPRangeDefParseXML(conn, def, cur); + if (result) + return result; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "tftp")) { + xmlChar *root; + + if (!(root = xmlGetProp(cur, BAD_CAST "root"))) { + cur = cur->next; + continue; + } + + def->tftproot = (char *)root; + } + + cur = cur->next; + } + return 0; +} + static virNetworkDefPtr virNetworkDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) @@ -363,7 +408,7 @@ virNetworkDefParseXML(virConnectPtr conn, /* XXX someday we want IPv6 too, so inet_aton won't work there */ struct in_addr inaddress, innetmask; char *netaddr; - xmlNodePtr dhcp; + xmlNodePtr ip; if (inet_pton(AF_INET, def->ipAddress, &inaddress) <= 0) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -386,8 +431,8 @@ virNetworkDefParseXML(virConnectPtr conn, goto error; } - if ((dhcp = virXPathNode(conn, "./ip[1]/dhcp[1]", ctxt)) && - virNetworkDHCPRangeDefParseXML(conn, def, dhcp) < 0) + if ((ip = virXPathNode(conn, "./ip[1]", ctxt)) && + virNetworkIPParseXML(conn, def, ip) < 0) goto error; } @@ -605,6 +650,10 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferAddLit(&buf, ">\n"); + if (def->tftproot) { + virBufferEscapeString(&buf, " <tftp root='%s' />\n", + def->tftproot); + } if ((def->nranges || def->nhosts)) { int i; virBufferAddLit(&buf, " <dhcp>\n"); @@ -621,6 +670,11 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, "ip='%s' ", def->hosts[i].ip); virBufferAddLit(&buf, "/>\n"); } + if (def->bootfile) { + virBufferEscapeString(&buf, " <bootp file='%s' />\n", + def->bootfile); + } + virBufferAddLit(&buf, " </dhcp>\n"); } diff --git a/src/network_conf.h b/src/network_conf.h index 4076f9a..e983a01 100644 --- a/src/network_conf.h +++ b/src/network_conf.h @@ -78,6 +78,9 @@ struct _virNetworkDef { unsigned int nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; + + char *tftproot; + char *bootfile; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/network_driver.c b/src/network_driver.c index 49855bf..2fe5c00 100644 --- a/src/network_driver.c +++ b/src/network_driver.c @@ -400,6 +400,10 @@ networkBuildDnsmasqArgv(virConnectPtr conn, (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ (2 * network->def->nhosts) + + /* --enable-tftp --tftp-root /srv/tftp */ + (network->def->tftproot ? 3 : 0) + + /* --dhcp-boot pxeboot.img */ + (network->def->bootfile ? 2 : 0) + 1; /* NULL */ if (VIR_ALLOC_N(*argv, len) < 0) @@ -478,6 +482,16 @@ networkBuildDnsmasqArgv(virConnectPtr conn, APPEND_ARG(*argv, i++, buf); } + if (network->def->tftproot) { + APPEND_ARG(*argv, i++, "--enable-tftp"); + APPEND_ARG(*argv, i++, "--tftp-root"); + APPEND_ARG(*argv, i++, network->def->tftproot); + } + if (network->def->bootfile) { + APPEND_ARG(*argv, i++, "--dhcp-boot"); + APPEND_ARG(*argv, i++, network->def->bootfile); + } + #undef APPEND_ARG return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index 74e98d1..2699343 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -51,6 +51,7 @@ EXTRA_DIST = \ capabilityschematest \ capabilityschemadata \ networkschematest \ + networkschemadata \ domainschematest \ domainschemadata \ interfaceschemadata \ diff --git a/tests/networkschemadata/netboot-network.xml b/tests/networkschemadata/netboot-network.xml new file mode 100644 index 0000000..7274ee6 --- /dev/null +++ b/tests/networkschemadata/netboot-network.xml @@ -0,0 +1,12 @@ +<network> + <name>netboot</name> + <bridge name="virbr1" /> + <forward/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <tftp root="/var/lib/tftproot" /> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254" /> + <bootp file="pxeboot.img" /> + </dhcp> + </ip> +</network> diff --git a/tests/networkschematest b/tests/networkschematest index 1d7cffc..a1491e1 100755 --- a/tests/networkschematest +++ b/tests/networkschematest @@ -3,7 +3,7 @@ test -z "$srcdir" && srcdir=`pwd` test -z "$abs_srcdir" && abs_srcdir=`pwd` -DIRS="../qemud" +DIRS="../qemud networkschemadata" n=0 f=0 -- 1.6.2.5
On Tue, Sep 15, 2009 at 11:13:50AM +0200, Paolo Bonzini wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces the appropriate tags to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network>
When the attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp />
Possible future features in this field include the following:
- At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute to <bootp>.
I think we possibly ought to move <tftp> one level higher up, outside of the <ip> tag, since you can have a TFTP server regardless of whether we've configured IP/dhcp details, and in the future we may well have multiple <ip> tags.
- Currently, the BOOTP file cannot be assigned to a specific <range> or <host>. Even if this was implemented, it would still make sense to allow <bootp> appearing directly within the <dhcp> element, providing a global default.
Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
On 09/15/2009 11:43 AM, Daniel P. Berrange wrote:
I think we possibly ought to move<tftp> one level higher up, outside of the<ip> tag, since you can have a TFTP server regardless of whether we've configured IP/dhcp details, and in the future we may well have multiple<ip> tags.
In principle, the ip@address attribute would be the address to which the TFTP server would bind. So if you have multiple <ip> tags you would also start multiple TFTP servers, each bound to a different address and possibly with a different root. At that point, we may certainly introduce the possibility to specify the <tftp> tag one level up to easily specify a default (just like <bootp>). However, given the current capabilities of libvirt it seems to me that this is the most logical schema. Paolo
On Tue, Sep 15, 2009 at 12:09:53PM +0200, Paolo Bonzini wrote:
On 09/15/2009 11:43 AM, Daniel P. Berrange wrote:
I think we possibly ought to move<tftp> one level higher up, outside of the<ip> tag, since you can have a TFTP server regardless of whether we've configured IP/dhcp details, and in the future we may well have multiple<ip> tags.
In principle, the ip@address attribute would be the address to which the TFTP server would bind. So if you have multiple <ip> tags you would also start multiple TFTP servers, each bound to a different address and possibly with a different root.
Ah yes, that is a good point. In that case ACK to your 3rd patch Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options. This change introduces the appropriate tags to the dhcp configuration: <network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network> When the attributes are present, these are passed to the arguments to dnsmasq: dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp /> At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute. Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> 2009-09-21 Paolo Bonzini <pbonzini@redhat.com> Jeremy Kerr <jk@ozlabs.org> * docs/formatnetwork.html.in: Document new tags. * docs/formatnetwork.html: Regenerate. * docs/schemas/network.rng: Update. * src/network_conf.c (virNetworkDefFree): Free new fields. (virNetworkDHCPRangeDefParseXML): Parse <bootp>. (virNetworkIPParseXML): New, parsing <dhcp> and <tftp>. (virNetworkDefParseXML): Use virNetworkIPParseXML instead of virNetworkDHCPRangeDefParseXML. (virNetworkDefFormat): Pretty print new fields. * src/network_conf.h (struct _virNetworkDef): Add netboot fields. * src/network_driver.c (networkBuildDnsmasqArgv): Add TFTP and BOOTP arguments. * tests/Makefile.am (EXTRA_DIST): Add networkschemadata. * tests/networkschematest: Look in networkschemadata. * tests/networkschemadata/netboot-network.xml: New. --- Changes from v2: Rebased after the big tree reorg. docs/formatnetwork.html.in | 17 ++++++-- docs/schemas/network.rng | 10 ++++ src/conf/network_conf.c | 60 +++++++++++++++++++++++++- src/conf/network_conf.h | 3 + src/network/bridge_driver.c | 14 ++++++ tests/Makefile.am | 1 + tests/networkschemadata/netboot-network.xml | 12 +++++ tests/networkschematest | 2 +- 8 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 tests/networkschemadata/netboot-network.xml diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in index fd68430..e471385 100644 --- a/docs/formatnetwork.html.in +++ b/docs/formatnetwork.html.in @@ -113,9 +113,13 @@ address will be their default route. The <code>netmask</code> attribute defines the significant bits of the network address, again specified in dotted-decimal format. <span class="since">Since 0.3.0</span> - </dd> - <dt><code>dhcp</code></dt> - <dd>Immediately within the <code>ip</code> element there is an + </dd><dt><code>tftp</code></dt><dd>Immediately within + the <code>ip</code> element there is an optional <code>tftp</code> + element. The presence of this element and of its attribute + <code>root</code> enables TFTP services. The attribute specifies + the path to the root directory served via TFTP. + <span class="since">Since 0.7.1</span> + </dd><dt><code>dhcp</code></dt><dd>Also within the <code>ip</code> element there is an optional <code>dhcp</code> element. The presence of this element enables DHCP services on the virtual network. It will further contain one or more <code>range</code> elements. @@ -137,7 +141,12 @@ assigned to that host (via the <code>ip</code> attribute), and the name to be given that host by the DHCP server (via the <code>name</code> attribute). <span class="since">Since 0.4.5</span> - </dd> + </dd><dt><code>bootp</code></dt><dd>The optional <code>bootp</code> + element specifies BOOTP options to be provided by the DHCP server. + Only one attribute is supported, <code>file</code>, giving the file + to be used for the boot image). The BOOTP options currently have to + be the same for all address ranges and statically assigned addresses.<span + class="since">Since 0.7.1.</span> </dl> <h2><a name="examples">Example configuration</a></h2> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng index a4281a5..042e013 100644 --- a/docs/schemas/network.rng +++ b/docs/schemas/network.rng @@ -60,6 +60,11 @@ <optional> <attribute name="netmask"><text/></attribute> </optional> + <optional> + <element name="tftp"> + <attribute name="root"><text/></attribute> + </element> + </optional> <!-- Define the range(s) of IP addresses that the DHCP server should hand out --> <element name="dhcp"> @@ -76,6 +81,11 @@ <attribute name="ip"><text/></attribute> </element> </zeroOrMore> + <optional> + <element name="bootp"> + <attribute name="file"><text/></attribute> + </element> + </optional> </element> </element> </optional> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3764bb4..14eb543 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -115,6 +115,9 @@ void virNetworkDefFree(virNetworkDefPtr def) } VIR_FREE(def->hosts); + VIR_FREE(def->tftproot); + VIR_FREE(def->bootfile); + VIR_FREE(def); } @@ -299,6 +302,17 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, def->hosts[def->nhosts].name = (char *)name; def->hosts[def->nhosts].ip = (char *)ip; def->nhosts++; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "bootp")) { + xmlChar *file; + + if (!(file = xmlGetProp(cur, BAD_CAST "file"))) { + cur = cur->next; + continue; + } + + def->bootfile = (char *)file; } cur = cur->next; @@ -307,6 +321,37 @@ virNetworkDHCPRangeDefParseXML(virConnectPtr conn, return 0; } +static int +virNetworkIPParseXML(virConnectPtr conn, + virNetworkDefPtr def, + xmlNodePtr node) { + xmlNodePtr cur; + + cur = node->children; + while (cur != NULL) { + if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "dhcp")) { + int result = virNetworkDHCPRangeDefParseXML(conn, def, cur); + if (result) + return result; + + } else if (cur->type == XML_ELEMENT_NODE && + xmlStrEqual(cur->name, BAD_CAST "tftp")) { + xmlChar *root; + + if (!(root = xmlGetProp(cur, BAD_CAST "root"))) { + cur = cur->next; + continue; + } + + def->tftproot = (char *)root; + } + + cur = cur->next; + } + return 0; +} + static virNetworkDefPtr virNetworkDefParseXML(virConnectPtr conn, xmlXPathContextPtr ctxt) @@ -363,7 +408,7 @@ virNetworkDefParseXML(virConnectPtr conn, /* XXX someday we want IPv6 too, so inet_aton won't work there */ struct in_addr inaddress, innetmask; char *netaddr; - xmlNodePtr dhcp; + xmlNodePtr ip; if (inet_pton(AF_INET, def->ipAddress, &inaddress) <= 0) { virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR, @@ -386,8 +431,8 @@ virNetworkDefParseXML(virConnectPtr conn, goto error; } - if ((dhcp = virXPathNode(conn, "./ip[1]/dhcp[1]", ctxt)) && - virNetworkDHCPRangeDefParseXML(conn, def, dhcp) < 0) + if ((ip = virXPathNode(conn, "./ip[1]", ctxt)) && + virNetworkIPParseXML(conn, def, ip) < 0) goto error; } @@ -605,6 +650,10 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferAddLit(&buf, ">\n"); + if (def->tftproot) { + virBufferEscapeString(&buf, " <tftp root='%s' />\n", + def->tftproot); + } if ((def->nranges || def->nhosts)) { int i; virBufferAddLit(&buf, " <dhcp>\n"); @@ -621,6 +670,11 @@ char *virNetworkDefFormat(virConnectPtr conn, virBufferVSprintf(&buf, "ip='%s' ", def->hosts[i].ip); virBufferAddLit(&buf, "/>\n"); } + if (def->bootfile) { + virBufferEscapeString(&buf, " <bootp file='%s' />\n", + def->bootfile); + } + virBufferAddLit(&buf, " </dhcp>\n"); } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 4076f9a..e983a01 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -78,6 +78,9 @@ struct _virNetworkDef { unsigned int nhosts; /* Zero or more dhcp hosts */ virNetworkDHCPHostDefPtr hosts; + + char *tftproot; + char *bootfile; }; typedef struct _virNetworkObj virNetworkObj; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index fd3d08f..95bc810 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -400,6 +400,10 @@ networkBuildDnsmasqArgv(virConnectPtr conn, (2 * network->def->nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-host 01:23:45:67:89:0a,hostname,10.0.0.3 */ (2 * network->def->nhosts) + + /* --enable-tftp --tftp-root /srv/tftp */ + (network->def->tftproot ? 3 : 0) + + /* --dhcp-boot pxeboot.img */ + (network->def->bootfile ? 2 : 0) + 1; /* NULL */ if (VIR_ALLOC_N(*argv, len) < 0) @@ -478,6 +482,16 @@ networkBuildDnsmasqArgv(virConnectPtr conn, APPEND_ARG(*argv, i++, buf); } + if (network->def->tftproot) { + APPEND_ARG(*argv, i++, "--enable-tftp"); + APPEND_ARG(*argv, i++, "--tftp-root"); + APPEND_ARG(*argv, i++, network->def->tftproot); + } + if (network->def->bootfile) { + APPEND_ARG(*argv, i++, "--dhcp-boot"); + APPEND_ARG(*argv, i++, network->def->bootfile); + } + #undef APPEND_ARG return 0; diff --git a/tests/Makefile.am b/tests/Makefile.am index 4cc21c9..abda4ad 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -53,6 +53,7 @@ EXTRA_DIST = \ capabilityschematest \ capabilityschemadata \ networkschematest \ + networkschemadata \ domainschematest \ domainschemadata \ interfaceschemadata \ diff --git a/tests/networkschemadata/netboot-network.xml b/tests/networkschemadata/netboot-network.xml new file mode 100644 index 0000000..7274ee6 --- /dev/null +++ b/tests/networkschemadata/netboot-network.xml @@ -0,0 +1,12 @@ +<network> + <name>netboot</name> + <bridge name="virbr1" /> + <forward/> + <ip address="192.168.122.1" netmask="255.255.255.0"> + <tftp root="/var/lib/tftproot" /> + <dhcp> + <range start="192.168.122.2" end="192.168.122.254" /> + <bootp file="pxeboot.img" /> + </dhcp> + </ip> +</network> diff --git a/tests/networkschematest b/tests/networkschematest index ac22bc1..3fc0f27 100755 --- a/tests/networkschematest +++ b/tests/networkschematest @@ -3,7 +3,7 @@ test -z "$srcdir" && srcdir=`pwd` test -z "$abs_srcdir" && abs_srcdir=`pwd` -DIRS="../src/network" +DIRS="../src/network networkschemadata" n=0 f=0 -- 1.6.2.5
On Mon, Sep 21, 2009 at 10:50:25PM +0200, Paolo Bonzini wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces the appropriate tags to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network>
When the attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp />
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
2009-09-21 Paolo Bonzini <pbonzini@redhat.com> Jeremy Kerr <jk@ozlabs.org>
* docs/formatnetwork.html.in: Document new tags. * docs/formatnetwork.html: Regenerate. * docs/schemas/network.rng: Update. * src/network_conf.c (virNetworkDefFree): Free new fields. (virNetworkDHCPRangeDefParseXML): Parse <bootp>. (virNetworkIPParseXML): New, parsing <dhcp> and <tftp>. (virNetworkDefParseXML): Use virNetworkIPParseXML instead of virNetworkDHCPRangeDefParseXML. (virNetworkDefFormat): Pretty print new fields. * src/network_conf.h (struct _virNetworkDef): Add netboot fields. * src/network_driver.c (networkBuildDnsmasqArgv): Add TFTP and BOOTP arguments.
* tests/Makefile.am (EXTRA_DIST): Add networkschemadata. * tests/networkschematest: Look in networkschemadata. * tests/networkschemadata/netboot-network.xml: New. --- Changes from v2: Rebased after the big tree reorg.
docs/formatnetwork.html.in | 17 ++++++-- docs/schemas/network.rng | 10 ++++ src/conf/network_conf.c | 60 +++++++++++++++++++++++++- src/conf/network_conf.h | 3 + src/network/bridge_driver.c | 14 ++++++ tests/Makefile.am | 1 + tests/networkschemadata/netboot-network.xml | 12 +++++ tests/networkschematest | 2 +- 8 files changed, 111 insertions(+), 8 deletions(-) create mode 100644 tests/networkschemadata/netboot-network.xml
ACK, lets commit this now. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
Paolo Bonzini wrote:
Currently, libvirtd will start a dnsmasq process for the virtual network, but (aside from killing the dnsmasq process and replacing it), there's no way to define tftp boot options.
This change introduces the appropriate tags to the dhcp configuration:
<network> <name>default</name> <bridge name="virbr%d" /> <forward/> <ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/var/lib/tftproot" /> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip> </network>
When the attributes are present, these are passed to the arguments to dnsmasq:
dnsmasq [...] --enable-tftp --tftp-root /srv/tftp --dhcp-boot pxeboot.img ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^ from <tftp /> from <bootp />
At present, only local tftp servers are supported (ie, dnsmasq runs as the tftp server), but we could improve this in future by adding a server= attribute.
Signed-off-by: Jeremy Kerr <jk@ozlabs.org> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Since everyone seems to be happy with this now, I've pushed it. Thanks, -- Chris Lalancette
Hi Paolo,
Since there is only one TFTP server running in the network
IIRC, it's possible to have more than one: the Server-Name (id 66) of the DHCP response specifies which. However, we probably only ever want to start one server, so the <tftp> tag should work fine.
, it is not possible to specify different roots for different dhcp ranges. I think the schema should be
<ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/srv/tftp"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip>
If you want it to be specific to the range, shouldn't it be within the <range> tag? Currently the code will only parse one <dhcp> tag. Otherwise, looks fine - I'm happy with either method. One thing that I've tried to keep in mind is that the tftp service may be provided by a separate machine in the network, so we may need some way in the future to represent that - maybe no <tftp> tag, and use server="w.x.y.z" in the bootp tag. Also, would be cool to have separate files for different hosts, but that may be thinking too far ahead at this stage :) Cheers, Jeremy
On 09/15/2009 01:01 AM, Jeremy Kerr wrote:
Hi Paolo,
Since there is only one TFTP server running in the network
IIRC, it's possible to have more than one: the Server-Name (id 66) of the DHCP response specifies which. However, we probably only ever want to start one server, so the<tftp> tag should work fine.
Yes, what I meant is that in general the TFTP server started by libvirt will run on the address provided by /network/ip@address. So libvirt can only start one TFTP server even though in the future it might support multiple DHCP ranges. In this case, some ranges may not support BOOTP and some may, and they can give different boot files, but all must share a single dnsmasq-provided TFTP server (using a server attribute would be fine; but that TFTP server will not be started by libvirt). This can be seen from the fact that BOOTP is nothing more than a few options within a DHCP packet (i.e. <dhcp><bootp/></dhcp>), but TFTP binds on a completely different port and could be a separate process (hence <tftp> is a sibling of <dhcp>). dnsmasq is special.
, it is not possible to specify different roots for different dhcp ranges. I think the schema should be
<ip address="192.168.122.1" netmask="255.255.255.0"> <tftp root="/srv/tftp"> <dhcp> <range start="192.168.122.2" end="192.168.122.254" /> <bootp file="pxeboot.img"/> </dhcp> </ip>
If you want it to be specific to the range, shouldn't it be within the<range> tag? Currently the code will only parse one<dhcp> tag.
No, I want it to be specific to the _network_, since it will bind to /network/ip@address.
One thing that I've tried to keep in mind is that the tftp service may be provided by a separate machine in the network, so we may need some way in the future to represent that - maybe no<tftp> tag
That's already implemented by my patch, and can be used with an external TFTP server, for example started via (x)inetd.
and use server="w.x.y.z" in the bootp tag. Also, would be cool to have separate files for different hosts, but that may be thinking too far ahead at this stage :)
Yes. That's something you'd get for free if libvirt supported multiple DHCP ranges. Paolo
participants (8)
-
Chris Lalancette -
Daniel P. Berrange -
Daniel Veillard -
Jeremy Kerr -
Mark McLoughlin -
Paolo Bonzini -
Paolo Bonzini -
Tom Hughes