Re: [libvirt] [PATCH 06/13] Make virtual network netmasks optional

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.