[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 0/3] Add dnsmasq module (v2)



On Sat, Apr 24, 2010 at 04:46:33AM +0900, Satoru SATOH wrote:
> On Fri, Apr 23, 2010 at 09:33:20AM +0200, Daniel Veillard wrote:
> > looking at the patches, those looks fine to me, I may have just a couple
> > of cosmetic comments, but I'm wondering if they should be postponed
> > after 0.8.1, or if it's fine to push them in now. On one hand I would
> > prefer to limit the number of actual features in 0.8.1, but on the other
> > hand you have already submitted this many time so I wonder what you
> > think. When you say "prototype implementation" how confident are you
> > about this code ?
> > 
> >   So what do you think ?
> 
> As far as I tested, it works as expected and not aware of any critical
> issues. So if you're ok, I want to get it merge in.

  Okay, after Jim's thorough review and doing a bit of testing I
commited your patch. I changed only a couple of things:
  - the dead store pointed out by Jim in his last review
  - the DNSMASQ_STATE_DIR, the host files are managed by libvirt,
    and even if they are used by dnsmasq, they really need to be stored
    in a directory owned and managed by libvirt. So I fixed
    DNSMASQ_STATE_DIR, to be LOCAL_STATE_DIR "/lib/libvirt/network",
    in practice the same as NETWORK_STATE_DIR. I think its fine because
    the naming of the files can't clash since they use different
    suffixes.

I tested this on my own boxes and this seems to work as expected, there
is just one small bit, assuming one stops a network like in,
virsh net-destroy default, the file

/var/lib/libvirt/network/default.hostsfile

remains while it should really be removed. But it's not a big deal at
this point, but I would like to get a fix for this :-)

  thanks !

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]