[libvirt] [PATCH 01/10] conf: DHCP - add state for DHCP Relay and On/Off switch

[TJ]

On 28/02/13 03:15, Eric Blake wrote:
> On 02/27/2013 07:18 PM, TJ wrote:
>> Signed-off-by: TJ <linux at iam.tj>
> 
> Can you use a full legal name, instead of a two-letter pseudonym?

TJ is my full name.

> 
> Your commit message is missing some substance, such as a summary of what
> is being added.

The first line of the commit message is in the email subject, and describes the commit.

"conf: DHCP - add state for DHCP Relay and On/Off switch"

> 
>> ---
>>  src/conf/network_conf.h | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
>> index c509915..8400eab 100644
>> --- a/src/conf/network_conf.h
>> +++ b/src/conf/network_conf.h
>> @@ -144,6 +144,10 @@ struct _virNetworkIpDef {
>>      char *bootfile;
>>      virSocketAddr bootserver;
>>  +    /* when false no DHCP server is started */
>> +    bool dhcp_enabled;
> 
> Your patch is whitespace-damaged.  Did you sent it with 'git send-email'?

Yes. It barfed first time and added 9 emails into one post! Second time it seemed to be OK.

> 
>> +    /* when true ranges are ignored and a DHCP relay-agent started */
>> +    bool dhcp_relay;
>>     };
>>   typedef struct _virNetworkForwardIfDef virNetworkForwardIfDef;
>> @@ -234,6 +238,7 @@ typedef virNetworkObj *virNetworkObjPtr;
>>  struct _virNetworkObj {
>>      virMutex lock;
>>  +    pid_t dhcprelayPid;
>>      pid_t dnsmasqPid;
> 
> Does your series ever allow dnsmasq and dhcprelay to run at the same
> time, or can we use a single pid_t field that covers the mutually
> exclusive choice of which helper is running based on the rest of the config?
> 

When local DHCP server services are disabled dnsmasq is still launched since there are several non-DHCP settings in the generated config.

In my test-bed for these patches dnsmasq and dhcp-helper will be started alongside each other.

If this series is accepted I was intending to propose adding <dns enable='(yes|no)'/> in the same style used for <dhcp> so that dnsmasq can be
totally disabled if un-needed.