On Mon, Apr 26, 2010 at 05:29:02PM +0200, Daniel Veillard wrote:
> On Sat, Apr 24, 2010 at 04:46:33AM +0900, Satoru SATOH wrote:
> > 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 :-)
Actually I noticed an error in my testing, while dnsmasq was started
with the correct attribute it failed to read the host config file due
to permission problems. The patch below uses instead
/var/lib/libvirt/dnsmasq
directory, created for this purpose and world readable allowing dnsmasq
access.
Daniel
---------------------
Move dnsmasq host file to a separate directory
use /var/lib/libvirt/dnsmasq since /var/lib/libvirt/network is
unreadable by the dnsmasq binary
* src/network/bridge_driver.c: update DNSMASQ_STATE_DIR
* src/Makefile.am: create it on make install
* libvirt.spec.in: take the new directory into account
diff --git a/libvirt.spec.in b/libvirt.spec.in
index a8b078a..090f5ee 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -566,6 +566,7 @@ rm -f $RPM_BUILD_ROOT%{_libdir}/python*/site-packages/*.la
rm -f $RPM_BUILD_ROOT%{_libdir}/python*/site-packages/*.a
%if %{with_network}
+install -d -m 0755 $RPM_BUILD_ROOT%{_datadir}/lib/libvirt/dnsmasq/
# We don't want to install /etc/libvirt/qemu/networks in the main %files list
# because if the admin wants to delete the default network completely, we don't
# want to end up re-incarnating it on every RPM upgrade.
@@ -742,6 +743,7 @@ fi
%if %{with_network}
%dir %{_localstatedir}/run/libvirt/network/
%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/
+%dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/
%endif
%if %{with_qemu}
diff --git a/src/Makefile.am b/src/Makefile.am
index fc64927..d8466f0 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1035,6 +1035,7 @@ if WITH_UML
endif
if WITH_NETWORK
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/network"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/dnsmasq"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/network"
$(MKDIR_P) "$(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart"
$(INSTALL_DATA) $(srcdir)/network/default.xml \
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 22b3927..132392b 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -62,7 +62,7 @@
#define NETWORK_PID_DIR LOCAL_STATE_DIR "/run/libvirt/network"
#define NETWORK_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network"
-#define DNSMASQ_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/network"
+#define DNSMASQ_STATE_DIR LOCAL_STATE_DIR "/lib/libvirt/dnsmasq"
#define VIR_FROM_THIS VIR_FROM_NETWORK
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list