On 06/17/2011 06:38 AM, Daniel P. Berrange wrote:
Introduce a configuration file with a single parameter
'require_lease_for_disks', which is used to decide whether
it is allowed to start a guest which has read/write disks,
but without any leases.
* libvirt.spec.in: Add sanlock config file and augeas
lens
* src/Makefile.am: Install sanlock config file and
augeas lens
* src/locking/libvirt_sanlock.aug: Augeas master lens
* src/locking/test_libvirt_sanlock.aug: Augeas test file
* src/locking/sanlock.conf: Example sanlock config
* src/locking/lock_driver_sanlock.c: Wire up loading
of configuration file
Where do we mention this file in the documentation?
# Build our version script. This is composed of three parts:
@@ -1184,9 +1188,26 @@ lockdriver_LTLIBRARIES = sanlock.la
sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES)
sanlock_la_CFLAGS = $(AM_CLFAGS)
sanlock_la_LDFLAGS = -module -avoid-version
-sanlock_la_LIBADD = -lsanlock
+sanlock_la_LIBADD = -lsanlock \
+ ../gnulib/lib/libgnu.la
+
+augeas_DATA += locking/libvirt_sanlock.aug
+augeastest_DATA += locking/test_libvirt_sanlock.aug
+
+EXTRA_DIST += locking/sanlock.conf \
+ locking/libvirt_sanlock.aug \
+ locking/test_libvirt_sanlock.aug
+
+$(builddir)/locking/%-sanlock.conf: $(srcdir)/locking/sanlock.conf
+ $(AM_V_GEN)mkdir locking ; \
+ cp $< $@
What's the purpose of this rule? We already require GNU make, which is
smart enough to look in $(srcdir) if a file is not present in
$(builddir) during a VPATH build. In other words, this is looking a bit
more complex than necessary.
@@ -62,22 +72,76 @@ struct _virLockManagerSanlockPrivate {
/*
* sanlock plugin for the libvirt virLockManager API
*/
+static int virLockManagerSanlockLoadConfig(const char *configFile)
+{
+ virConfPtr conf;
+ virConfValuePtr p;
+
+ if (access(configFile, R_OK) == -1) {
+ if (errno != ENOENT) {
+ virReportSystemError(errno,
+ _("Unable to access config file %s"),
+ configFile);
+ return -1;
+ }
+ return 0;
So a missing conf file is silently treated as success?
+++ b/src/locking/sanlock.conf
@@ -0,0 +1,6 @@
+#
+# Flag to determine whether we allow starting of guests
+# which do not have any <lease> elements defined in their
+# configuration.
+#
+#require_lease_for_disks = 1
I'm not sure we've been doing the best job at being consistent, but when
showing a comment describing an option, is it better to show the default
(where uncommenting makes no changes) or the converse (where
uncommenting instantly provides the most common non-default)? Thus,
it's probably worth documenting whether the default is 0 or 1.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library
http://libvirt.org