12:02 p.m.
On 06/24/2011 07:02 AM, Daniel P. Berrange wrote:
The current sanlock plugin requires a central management
application to manually add <lease> elements to each guest,
to protect resources that are assigned to it (eg writable
disks). This makes the sanlock plugin useless for usage
in more adhoc deployment environments where there is no
s/adhoc/ad hoc/
To make use of this capability the admin will need todo
several tasks:
s/todo/to do/
+static int virLockManagerSanlockSetupLockspace(void)
+{
+ int fd = -1;
+ struct stat st;
+ int rv;
+ struct sanlk_lockspace ls;
+ char *path = NULL;
+
+ if (virAsprintf(&path, "%s/%s",
+ driver->autoDiskLeasePath,
+ VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE) < 0) {
+ virReportOOMError();
+ goto error;
+ }
+
+ memcpy(ls.name, VIR_LOCK_MANAGER_SANLOCK_AUTO_DISK_LOCKSPACE, SANLK_NAME_LEN);
+ ls.host_id = 0; /* Doesn't matter for initialization */
+ ls.flags = 0;
+ memcpy(&ls.host_id_disk, path, sizeof(struct sanlk_disk));
Dangerous. This could copy more bytes than strlen(path) and cross a
page boundary, or it could silently truncate if strlen(path) is longer
than sizeof(struct sanlk_disk). I think you need to use virStrncpy here.
-static int virLockManagerSanlockAddResource(virLockManagerPtr lock,
- unsigned int type,
- const char *name,
- size_t nparams,
- virLockManagerParamPtr params,
- unsigned int flags)
+
+static const char hex[] = { '0', '1', '2', '3',
'4', '5', '6', '7',
+ '8', '9', 'a', 'b',
'c', 'd', 'e', 'f' };
I would have written this:
static const char hex[] = "0123456789abcdef";
but either way works.
+
+static int virLockManagerSanlockCreateLease(struct sanlk_resource *res)
+{
+ int fd = -1;
+ struct stat st;
+ int rv;
+
+ if (stat(res->disks[0].path, &st) < 0) {
+ VIR_DEBUG("Lockspace %s does not yet exist", res->disks[0].path);
+ if ((fd = open(res->disks[0].path, O_WRONLY|O_CREAT|O_EXCL, 0600)) < 0) {
This feels like common copy-and-paste code; should attaching to the
lockspace be factored into a common helper routine?
+#
+# When automatically creating leases for disks, each host which
+# can access the filesystem mounted at 'disk_lease_dir' will need
+# to have a unique host ID assigned. The 'max_hosts' parameter
+# specifies an upper limit on the number of participating hosts.
+#
+# Each additional host requires 1 sector of disk space, usually
+# 512 bytes. The default is 64, and can be reduced if you don't
+# have many hosts, or increased if you have more.
+#
+#max_hosts = 64
Weren't there some list comments about either dropping this parameter,
or changing it to 2000, and that sanlock has changed to use more than
512 bytes per host now?
http://www.redhat.com/archives/libvir-list/2011-June/msg01237.html
+++ b/tools/Makefile.am
@@ -12,6 +12,7 @@ EXTRA_DIST = \
$(ICON_FILES) \
virt-xml-validate.in \
virt-pki-validate.in \
+ virt-sanlock-cleanup.in \
virsh.pod \
libvirt-guests.init.sh \
libvirt-guests.sysconf
@@ -19,8 +20,14 @@ EXTRA_DIST = \
bin_SCRIPTS = virt-xml-validate virt-pki-validate
bin_PROGRAMS = virsh
-dist_man1_MANS = virt-xml-validate.1 virt-pki-validate.1 virsh.1
+if HAVE_SANLOCK
+sbin_SCRIPTS = virt-sanlock-cleanup
+endif
+dist_man1_MANS = virt-xml-validate.1 virt-pki-validate.1 virsh.1
+if HAVE_SANLOCK
+dist_man8_MANS = virt-sanlock-cleanup.8
Ouch. Conditional distribution...
+endif
virt-xml-validate: virt-xml-validate.in Makefile
$(AM_V_GEN)sed -e 's,[@]SCHEMADIR@,$(pkgdatadir)/schemas,' < $< > $@
\
@@ -36,6 +43,14 @@ virt-pki-validate: virt-pki-validate.in Makefile
virt-pki-validate.1: virt-pki-validate.in
$(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
+virt-sanlock-cleanup: virt-sanlock-cleanup.in Makefile
+ $(AM_V_GEN)sed -e 's,[@]SYSCONFDIR@,$(sysconfdir),' \
+ -e 's,[@]LOCALSTATEDIR@,$(localstatedir),' < $< > $@ \
+ || (rm $@ && exit 1) && chmod +x $@
+
+virt-sanlock-cleanup.8: virt-sanlock-cleanup.in
+ $(AM_V_GEN)$(POD2MAN) $< $(srcdir)/$@
...that depends on perl. Commit 6db98a2d4b previously fixed a bug just
like this; the idea being that we have to unconditionally build
virt-sanlock-cleanup.8.in using $(POD2MAN) and put that in the tarball,
then conditionally build virt-sanlock-cleanup.8 using only sed, so that
end users can run 'make dist' regardless of configure options and can
still avoid perl.
But since I wrote commit 6db98a2d4b, I'm happy to clean this up as a
separate followup patch, if you'd like.
+++ b/tools/virt-sanlock-cleanup.in
@@ -0,0 +1,96 @@
+#!/bin/sh
+
+# A script to cleanup resource leases auto-created by
+# the libvirt lock plugin for sanlock
+
+verbose=1
+if test "$1" = "-q" ; then
if test "x$1" = "x-q" ; then
The leading x is necessary, to prevent test from going haywire if $1
happens to be something like [.
+=head1 EXIT STATUS
+
+Upon successful processing of leases cleanup, the exit status
+will be 0 will be set. Upon fatal eror a non-zero status will
s/eror/error/
ACK with the spelling and memcpy fixes, and up to you whether you make
the remaining suggested changes now or leave them for followups.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org