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

[Laine Stump]

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).