On 28/02/13 03:15, Eric Blake wrote:
> On 02/27/2013 07:18 PM, TJ wrote:
>> Signed-off-by: TJ <linux@xxxxxx>
>
> 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.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list