On Thu, Jun 23, 2011 at 08:05:46AM -0600, Eric Blake wrote:
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?
I've got a new patch which adds docs to the website.
-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.
I'm not sure what you mean ? This rules generates qemu-sanlock.conf from sanlock.conf, so how can we do without it ?
@@ -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?
Yep, that's the way we deal with libvirtd.conf and qemu.conf too.
+++ 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.
Yep, that's in the next patch Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|