Re: [libvirt] [PATCH 02/10] conf: Network - add ability to read/write XML DHCP state
On 02/27/2013 07:57 PM, TJ wrote:
From: TJ <linux(a)iam.tj>
Maintain backwards XML compatibility by assuming existing default values
and only adding the additional XML properties if settings are not
default.
Signed-off-by: TJ <linux(a)iam.tj>
---
src/conf/network_conf.c | 28 ++++++++++++++++++++++++----
1 file changed, 24 insertions(+), 4 deletions(-)
+ def->dhcp_enabled = true;
+ if ((tmp = virXMLPropString(node, "enabled"))) {
+ def->dhcp_enabled = strncmp("no", tmp, 2) == 0 ? false :
def->dhcp_enabled;
Yuck. This lets a user pass in trailing garbage. Use STREQ, not strncmp.
For that matter, assigning def->dhcp_enabled to itself looks odd. I'd
probably write:
def->dhcp_enabled = true;
if ((tmp = virXMLPropString(node, "enabled"))) {
if (STREQ(tmp, "no"))
def->dhcp_enabled = false;
VIR_FREE(tmp);
so that it doesn't look so screwy.
+ VIR_FREE(tmp);
+ }
+
+ def->dhcp_relay = false;
+ if ((tmp = virXMLPropString(node, "relay"))) {
+ def->dhcp_relay = strncmp("yes", tmp, 3) == 0 ? true :
def->dhcp_relay;
+ VIR_FREE(tmp);
Same comments.
+ }
cur = node->children;
while (cur != NULL) {
@@ -2180,12 +2193,19 @@ virNetworkIpDefFormat(virBufferPtr buf,
virBufferEscapeString(buf, "<tftp root='%s' />\n",
def->tftproot);
}
- if ((def->nranges || def->nhosts)) {
+ if ((def->nranges || def->nhosts) ||
As long as you are touching this, you can drop the redundant ().
+ !def->dhcp_enabled || def->dhcp_relay) {
int ii;
- virBufferAddLit(buf, "<dhcp>\n");
+ virBufferAddLit(buf, "<dhcp");
+ if (!def->dhcp_enabled)
+ virBufferAddLit(buf, " enabled='no'");
+ if (def->dhcp_relay)
+ virBufferAddLit(buf, " relay='yes'");
+ virBufferAddLit(buf, ">\n");
+
virBufferAdjustIndent(buf, 2);
- for (ii = 0 ; ii < def->nranges ; ii++) {
+ for (ii = 0 ; def->nranges && ii < def->nranges ; ii++) {
This line is a spurious change.
char *saddr =
virSocketAddrFormat(&def->ranges[ii].start);
if (!saddr)
goto error;
@@ -2199,7 +2219,7 @@ virNetworkIpDefFormat(virBufferPtr buf,
VIR_FREE(saddr);
VIR_FREE(eaddr);
}
- for (ii = 0 ; ii < def->nhosts ; ii++) {
+ for (ii = 0 ; def->nhosts && ii < def->nhosts ; ii++) {
So is this one.
virBufferAddLit(buf, "<host ");
if (def->hosts[ii].mac)
virBufferAsprintf(buf, "mac='%s' ",
def->hosts[ii].mac);
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)