Re: [libvirt] [PATCH 01/10] conf: DHCP - add state for DHCP Relay and On/Off switch
On 28/02/13 03:15, Eric Blake wrote:
On 02/27/2013 07:18 PM, TJ wrote:
> Signed-off-by: TJ <linux(a)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.