On 12/22/2010 11:34 AM, Laine Stump wrote:
On 12/20/2010 07:13 PM, Eric Blake wrote:
On 12/20/2010 01:03 AM, Laine Stump wrote:
When a netmask isn't specified for an IPv4 address, one can be implied
based on what network class range the address is in. The
virNetworkDefPrefix function does this for us, so netmask isn't
required.
---
src/conf/network_conf.c | 30 ++++++++++++++++--------------
1 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index b6ac4e3..09566ca 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -461,19 +461,14 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
def->delay = 0;
ipAddress = virXPathString("string(./ip[1]/@address)", ctxt);
- netmask = virXPathString("string(./ip[1]/@netmask)", ctxt);
- if (ipAddress&&
- netmask) {
+ if (ipAddress) {
Before this patch, if you specified one but not the other, then neither
was recognized. Now, if you specify ipAddress but not netmask,
ipAddress is still recognized (good), but if you specify netmask but not
ipAddress, you've caused the def file to be in a state that wasn't
possible before (potentially bad). Should you also add a sanity check
that declares the configuration invalid if netmask is specified without
ipAddress, so you don't have to worry about it in the rest of the
code base?
Yes, good idea. I changed it to give an error if netmask (or prefix,
in later patches) is provided without an IP address.
I also noticed a pre-existing memory leak during error exits, and am
adding a fix for that to the series.
Heh. Getting ahead of myself...
Continuing the git rebase after making those changes, I noticed that 1)
all this code moves into a separate function in patch 09/13, and 2) that
patch already checks for netmask w/o an IP address and gives a proper
error message. For those reasons, I'm going to leave this patch as-is
(since it will only be there to satisfy bisects).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list