[libvirt] [PATCH 00/14 v4] Implement virtlockd daemon
This is an update of https://www.redhat.com/archives/libvir-list/2012-September/msg00816.html This series was previously fully acked, but before pushing I noticed some problems and stopped. Since then I've done quite a few more changes to rebase to latest GIT & follow new best practice. I've also added support for a config file for the lockd plugin and the ability to use custom lockspaces for LVM and SCSI IDs. I've also added support to acquire leases based on a SHA256 sum of the file, instead of directly on the file As such I don't feel comfortable pushing, without further reviews. While we're past the freeze date, I'd like to request an exception, for all except the 14th patch. Without the 14th patch, this series has minimal impact on existing code that is used in libvirt so little regression possibility. Further I discovered yesterday that we already have people using this code as custom add-on patches, so I'd like to help them by including it in 1.0.1 .gitignore | 5 bootstrap.conf | 1 cfg.mk | 9 libvirt.spec.in | 16 po/POTFILES.in | 4 run.in | 2 src/Makefile.am | 214 ++++- src/internal.h | 22 src/libvirt_private.syms | 2 src/locking/libvirt_lockd.aug | 33 src/locking/lock_daemon.c | 1441 ++++++++++++++++++++++++++++++++++ src/locking/lock_daemon.h | 56 + src/locking/lock_daemon_config.c | 193 ++++ src/locking/lock_daemon_config.h | 50 + src/locking/lock_daemon_dispatch.c | 434 ++++++++++ src/locking/lock_daemon_dispatch.h | 31 src/locking/lock_driver_lockd.c | 834 +++++++++++++++++++ src/locking/lock_manager.c | 20 src/locking/lock_manager.h | 3 src/locking/lock_protocol.x | 95 ++ src/locking/lockd.conf | 68 + src/locking/test_libvirt_lockd.aug.in | 7 src/locking/virtlockd.init.in | 93 ++ src/locking/virtlockd.service.in | 13 src/locking/virtlockd.socket.in | 8 src/locking/virtlockd.sysconf | 3 src/qemu/qemu.conf | 17 src/qemu/qemu_conf.c | 12 src/qemu/qemu_conf.h | 3 src/qemu/qemu_driver.c | 24 src/qemu/test_libvirtd_qemu.aug.in | 2 src/util/storage_file.c | 30 src/util/storage_file.h | 4 33 files changed, 3697 insertions(+), 52 deletions(-)
From: "Daniel P. Berrange" <berrange@redhat.com> When we invoke lvs or scsi_id to extract ID for block devices, we don't want virCommandWait logging errors messages. Thus we must explicitly check 'status != 0', rather than letting virCommandWait do it. Also move the check for converting from "" to NULL, after the cleanup label, since virCommandRun may set the key to "", even on failure --- src/util/storage_file.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4417404..776d4f2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1199,6 +1199,7 @@ const char *virStorageFileGetLVMKey(const char *path) * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( LVS, "--noheadings", "--unbuffered", "--nosuffix", @@ -1208,7 +1209,13 @@ const char *virStorageFileGetLVMKey(const char *path) /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0) goto cleanup; if (key) { @@ -1228,10 +1235,10 @@ const char *virStorageFileGetLVMKey(const char *path) *nl = '\0'; } +cleanup: if (key && STREQ(key, "")) VIR_FREE(key); -cleanup: virCommandFree(cmd); return key; @@ -1248,6 +1255,7 @@ const char *virStorageFileGetLVMKey(const char *path) const char *virStorageFileGetSCSIKey(const char *path) { char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( "/lib/udev/scsi_id", "--replace-whitespace", @@ -1258,9 +1266,16 @@ const char *virStorageFileGetSCSIKey(const char *path) /* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) goto cleanup; + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0) + goto cleanup; + +cleanup: if (key && STRNEQ(key, "")) { char *nl = strchr(key, '\n'); if (nl) @@ -1269,7 +1284,6 @@ const char *virStorageFileGetSCSIKey(const char *path) VIR_FREE(key); } -cleanup: virCommandFree(cmd); return key; -- 1.7.11.7
On 12/11/12 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When we invoke lvs or scsi_id to extract ID for block devices, we don't want virCommandWait logging errors messages. Thus we must explicitly check 'status != 0', rather than letting virCommandWait do it. Also move the check for converting from "" to NULL, after the cleanup label, since virCommandRun may set the key to "", even on failure --- src/util/storage_file.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4417404..776d4f2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1199,6 +1199,7 @@ const char *virStorageFileGetLVMKey(const char *path) * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( LVS, "--noheadings", "--unbuffered", "--nosuffix", @@ -1208,7 +1209,13 @@ const char *virStorageFileGetLVMKey(const char *path)
/* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0)
You should report an error here or add a comment to explicitly note that this function isn't setting errors in some cases and cleanup paths of callers shouldn't rely on this.
goto cleanup;
if (key) { @@ -1228,10 +1235,10 @@ const char *virStorageFileGetLVMKey(const char *path) *nl = '\0'; }
+cleanup: if (key && STREQ(key, "")) VIR_FREE(key);
-cleanup: virCommandFree(cmd);
return key; @@ -1248,6 +1255,7 @@ const char *virStorageFileGetLVMKey(const char *path) const char *virStorageFileGetSCSIKey(const char *path) { char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( "/lib/udev/scsi_id", "--replace-whitespace", @@ -1258,9 +1266,16 @@ const char *virStorageFileGetSCSIKey(const char *path)
/* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) goto cleanup;
+ /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0)
same here
+ goto cleanup; + +cleanup: if (key && STRNEQ(key, "")) { char *nl = strchr(key, '\n'); if (nl) @@ -1269,7 +1284,6 @@ const char *virStorageFileGetSCSIKey(const char *path) VIR_FREE(key); }
-cleanup: virCommandFree(cmd);
return key;
Otherwise looks good, so ACK if you follow one of the options in my comments. Peter P.S: Hopefully somebody will continue the review on this series as I'm out for today.
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
When we invoke lvs or scsi_id to extract ID for block devices, we don't want virCommandWait logging errors messages. Thus we must explicitly check 'status != 0', rather than letting virCommandWait do it. Also move the check for converting from "" to NULL, after the cleanup label, since virCommandRun may set the key to "", even on failure --- src/util/storage_file.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 4417404..776d4f2 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1199,6 +1199,7 @@ const char *virStorageFileGetLVMKey(const char *path) * 06UgP5-2rhb-w3Bo-3mdR-WeoL-pytO-SAa2ky */ char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( LVS, "--noheadings", "--unbuffered", "--nosuffix", @@ -1208,7 +1209,13 @@ const char *virStorageFileGetLVMKey(const char *path)
/* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) + goto cleanup; + + /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0) goto cleanup;
if (key) { @@ -1228,10 +1235,10 @@ const char *virStorageFileGetLVMKey(const char *path) *nl = '\0'; }
+cleanup: if (key && STREQ(key, "")) VIR_FREE(key);
-cleanup: virCommandFree(cmd);
return key; @@ -1248,6 +1255,7 @@ const char *virStorageFileGetLVMKey(const char *path) const char *virStorageFileGetSCSIKey(const char *path) { char *key = NULL; + int status; virCommandPtr cmd = virCommandNewArgList( "/lib/udev/scsi_id", "--replace-whitespace", @@ -1258,9 +1266,16 @@ const char *virStorageFileGetSCSIKey(const char *path)
/* Run the program and capture its output */ virCommandSetOutputBuffer(cmd, &key); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, &status) < 0) goto cleanup;
+ /* Explicitly check, rather than passing NULL to virCommandRun + * because we don't want to pollute logs with an error message + */ + if (status != 0) + goto cleanup; + +cleanup: if (key && STRNEQ(key, "")) { char *nl = strchr(key, '\n'); if (nl) @@ -1269,7 +1284,6 @@ const char *virStorageFileGetSCSIKey(const char *path) VIR_FREE(key); }
-cleanup: virCommandFree(cmd);
return key;
I agree with Peter, I think you must return an error to be consistent with !HAVE_UDEV version of these functions.
From: "Daniel P. Berrange" <berrange@redhat.com> --- src/libvirt_private.syms | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d083e4..7949d5b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1169,6 +1169,8 @@ virStorageFileChainLookup; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileFreeMetadata; +virStorageFileGetLVMKey; +virStorageFileGetSCSIKey; virStorageFileGetMetadata; virStorageFileGetMetadataFromFD; virStorageFileIsClusterFS; -- 1.7.11.7
On 12/11/12 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/libvirt_private.syms | 2 ++ 1 file changed, 2 insertions(+)
ACK Peter
On 12/11/2012 01:41 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/libvirt_private.syms | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d083e4..7949d5b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1169,6 +1169,8 @@ virStorageFileChainLookup; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileFreeMetadata; +virStorageFileGetLVMKey; +virStorageFileGetSCSIKey; virStorageFileGetMetadata;
Might be worth sorting before pushing. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Dec 11, 2012 at 02:31:00PM -0700, Eric Blake wrote:
On 12/11/2012 01:41 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/libvirt_private.syms | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7d083e4..7949d5b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1169,6 +1169,8 @@ virStorageFileChainLookup; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; virStorageFileFreeMetadata; +virStorageFileGetLVMKey; +virStorageFileGetSCSIKey; virStorageFileGetMetadata;
Might be worth sorting before pushing.
Opps, yes, will sort. It is about time I wrote a test case to validate sort ordering in this file :-) 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> Refactor virLockManagerPluginNew() so that the caller does not need to pass in the config file path itself - just the config directory and driver name. Fix QEMU to actually pass in a config file when creating the default lock manager plugin, rather than NULL. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/locking/lock_manager.c | 20 +++++++++++++++++--- src/locking/lock_manager.h | 3 ++- src/qemu/qemu_conf.c | 12 +++++------- src/qemu/qemu_conf.h | 3 ++- src/qemu/qemu_driver.c | 24 ++++++++++-------------- 5 files changed, 36 insertions(+), 26 deletions(-) diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c index 423997b..3f483b7 100644 --- a/src/locking/lock_manager.c +++ b/src/locking/lock_manager.c @@ -128,7 +128,8 @@ static void virLockManagerLogParams(size_t nparams, */ #if HAVE_DLFCN_H virLockManagerPluginPtr virLockManagerPluginNew(const char *name, - const char *configFile, + const char *driverName, + const char *configDir, unsigned int flags) { void *handle = NULL; @@ -136,6 +137,16 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, virLockManagerPluginPtr plugin = NULL; const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"); char *modfile = NULL; + char *configFile = NULL; + + VIR_DEBUG("name=%s driverName=%s configDir=%s flags=%x", + name, driverName, NULLSTR(configDir), flags); + + if (virAsprintf(&configFile, "%s/%s-%s.conf", + configDir, driverName, name) < 0) { + virReportOOMError(); + return NULL; + } if (STREQ(name, "nop")) { driver = &virLockDriverNop; @@ -147,7 +158,7 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, if (virAsprintf(&modfile, "%s/%s.so", moddir, name) < 0) { virReportOOMError(); - return NULL; + goto cleanup; } if (access(modfile, R_OK) < 0) { @@ -188,10 +199,12 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, goto cleanup; } + VIR_FREE(configFile); VIR_FREE(modfile); return plugin; cleanup: + VIR_FREE(configFile); VIR_FREE(plugin); VIR_FREE(modfile); if (handle) @@ -201,7 +214,8 @@ cleanup: #else /* !HAVE_DLFCN_H */ virLockManagerPluginPtr virLockManagerPluginNew(const char *name ATTRIBUTE_UNUSED, - const char *configFile ATTRIBUTE_UNUSED, + const char *driverName ATTRIBUTE_UNUSED, + const char *configDir ATTRIBUTE_UNUSED, unsigned int flags_unused ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/locking/lock_manager.h b/src/locking/lock_manager.h index 4fee12d..fea9db8 100644 --- a/src/locking/lock_manager.h +++ b/src/locking/lock_manager.h @@ -30,7 +30,8 @@ typedef virLockManagerPlugin *virLockManagerPluginPtr; void virLockManagerSetPluginDir(const char *dir); virLockManagerPluginPtr virLockManagerPluginNew(const char *name, - const char *configFile, + const char *driverName, + const char *configDir, unsigned int flags); void virLockManagerPluginRef(virLockManagerPluginPtr plugin); void virLockManagerPluginUnref(virLockManagerPluginPtr plugin); diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8d380a1..4c506a0 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -117,7 +117,10 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, } #endif - if (!(driver->lockManager = virLockManagerPluginNew("nop", NULL, 0))) + if (!(driver->lockManager = virLockManagerPluginNew("nop", + "qemu", + driver->configBaseDir, + 0))) goto cleanup; driver->keepAliveInterval = 5; @@ -359,15 +362,10 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, p = virConfGetValue(conf, "lock_manager"); CHECK_TYPE("lock_manager", VIR_CONF_STRING); if (p && p->str) { - char *lockConf; virLockManagerPluginUnref(driver->lockManager); - if (virAsprintf(&lockConf, "%s/libvirt/qemu-%s.conf", SYSCONFDIR, p->str) < 0) - goto no_memory; - if (!(driver->lockManager = - virLockManagerPluginNew(p->str, lockConf, 0))) + virLockManagerPluginNew(p->str, "qemu", driver->configBaseDir, 0))) VIR_ERROR(_("Failed to load lock manager %s"), p->str); - VIR_FREE(lockConf); } GET_VALUE_LONG("max_queued", driver->max_queued); diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index d0d25ce..1a39946 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -79,8 +79,9 @@ struct _virQEMUDriver { virDomainObjList domains; - /* These four directories are ones libvirtd uses (so must be root:root + /* These five directories are ones libvirtd uses (so must be root:root * to avoid security risk from QEMU processes */ + char *configBaseDir; char *configDir; char *autostartDir; char *logDir; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2fda44e..00f6b00 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -614,7 +614,6 @@ qemuStartup(bool privileged, virStateInhibitCallback callback, void *opaque) { - char *base = NULL; char *driverConf = NULL; int rc; virConnectPtr conn = NULL; @@ -657,7 +656,7 @@ qemuStartup(bool privileged, "%s/log/libvirt/qemu", LOCALSTATEDIR) == -1) goto out_of_memory; - if ((base = strdup(SYSCONFDIR "/libvirt")) == NULL) + if ((qemu_driver->configBaseDir = strdup(SYSCONFDIR "/libvirt")) == NULL) goto out_of_memory; if (virAsprintf(&qemu_driver->stateDir, @@ -708,16 +707,15 @@ qemuStartup(bool privileged, } VIR_FREE(rundir); - base = virGetUserConfigDirectory(); - if (!base) + if (!(qemu_driver->configBaseDir = virGetUserConfigDirectory())) goto error; - if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", base) == -1) + if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", qemu_driver->configBaseDir) == -1) goto out_of_memory; - if (virAsprintf(&qemu_driver->saveDir, "%s/qemu/save", base) == -1) + if (virAsprintf(&qemu_driver->saveDir, "%s/qemu/save", qemu_driver->configBaseDir) == -1) goto out_of_memory; - if (virAsprintf(&qemu_driver->snapshotDir, "%s/qemu/snapshot", base) == -1) + if (virAsprintf(&qemu_driver->snapshotDir, "%s/qemu/snapshot", qemu_driver->configBaseDir) == -1) goto out_of_memory; - if (virAsprintf(&qemu_driver->autoDumpPath, "%s/qemu/dump", base) == -1) + if (virAsprintf(&qemu_driver->autoDumpPath, "%s/qemu/dump", qemu_driver->configBaseDir) == -1) goto out_of_memory; } @@ -755,13 +753,11 @@ qemuStartup(bool privileged, /* Configuration paths are either ~/.libvirt/qemu/... (session) or * /etc/libvirt/qemu/... (system). */ - if (virAsprintf(&driverConf, "%s/qemu.conf", base) < 0 || - virAsprintf(&qemu_driver->configDir, "%s/qemu", base) < 0 || - virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", base) < 0) + if (virAsprintf(&driverConf, "%s/qemu.conf", qemu_driver->configBaseDir) < 0 || + virAsprintf(&qemu_driver->configDir, "%s/qemu", qemu_driver->configBaseDir) < 0 || + virAsprintf(&qemu_driver->autostartDir, "%s/qemu/autostart", qemu_driver->configBaseDir) < 0) goto out_of_memory; - VIR_FREE(base); - rc = virCgroupForDriver("qemu", &qemu_driver->cgroup, privileged, 1); if (rc < 0) { VIR_INFO("Unable to create cgroup for driver: %s", @@ -934,7 +930,6 @@ error: qemuDriverUnlock(qemu_driver); if (conn) virConnectClose(conn); - VIR_FREE(base); VIR_FREE(driverConf); VIR_FREE(membase); VIR_FREE(mempath); @@ -1074,6 +1069,7 @@ qemuShutdown(void) { qemuDriverCloseCallbackShutdown(qemu_driver); + VIR_FREE(qemu_driver->configBaseDir); VIR_FREE(qemu_driver->configDir); VIR_FREE(qemu_driver->autostartDir); VIR_FREE(qemu_driver->logDir); -- 1.7.11.7
On 12/11/2012 01:41 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Refactor virLockManagerPluginNew() so that the caller does not need to pass in the config file path itself - just the config directory and driver name.
Fix QEMU to actually pass in a config file when creating the default lock manager plugin, rather than NULL.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/locking/lock_manager.c | 20 +++++++++++++++++--- src/locking/lock_manager.h | 3 ++- src/qemu/qemu_conf.c | 12 +++++------- src/qemu/qemu_conf.h | 3 ++- src/qemu/qemu_driver.c | 24 ++++++++++-------------- 5 files changed, 36 insertions(+), 26 deletions(-)
@@ -136,6 +137,16 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, virLockManagerPluginPtr plugin = NULL; const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"); char *modfile = NULL; + char *configFile = NULL; + + VIR_DEBUG("name=%s driverName=%s configDir=%s flags=%x", + name, driverName, NULLSTR(configDir), flags);
This says configDir might be NULL,
+ + if (virAsprintf(&configFile, "%s/%s-%s.conf", + configDir, driverName, name) < 0) {
but this unconditionally dereferences it. Which way did you mean to go?
@@ -708,16 +707,15 @@ qemuStartup(bool privileged, } VIR_FREE(rundir);
- base = virGetUserConfigDirectory(); - if (!base) + if (!(qemu_driver->configBaseDir = virGetUserConfigDirectory())) goto error; - if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", base) == -1) + if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", qemu_driver->configBaseDir) == -1)
This makes for some long lines; is it worth wrapping at the ',' now that you are using a longer name? Or maybe worth declaring: const char *base = qemu_driver->configBaseDir; to avoid having to touch these lines? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Tue, Dec 11, 2012 at 02:57:10PM -0700, Eric Blake wrote:
On 12/11/2012 01:41 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Refactor virLockManagerPluginNew() so that the caller does not need to pass in the config file path itself - just the config directory and driver name.
Fix QEMU to actually pass in a config file when creating the default lock manager plugin, rather than NULL.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/locking/lock_manager.c | 20 +++++++++++++++++--- src/locking/lock_manager.h | 3 ++- src/qemu/qemu_conf.c | 12 +++++------- src/qemu/qemu_conf.h | 3 ++- src/qemu/qemu_driver.c | 24 ++++++++++-------------- 5 files changed, 36 insertions(+), 26 deletions(-)
@@ -136,6 +137,16 @@ virLockManagerPluginPtr virLockManagerPluginNew(const char *name, virLockManagerPluginPtr plugin = NULL; const char *moddir = getenv("LIBVIRT_LOCK_MANAGER_PLUGIN_DIR"); char *modfile = NULL; + char *configFile = NULL; + + VIR_DEBUG("name=%s driverName=%s configDir=%s flags=%x", + name, driverName, NULLSTR(configDir), flags);
This says configDir might be NULL,
The NULLSTR is a mistake - the original param allowed NULLs, but the new one does not.
+ + if (virAsprintf(&configFile, "%s/%s-%s.conf", + configDir, driverName, name) < 0) {
but this unconditionally dereferences it. Which way did you mean to go?
@@ -708,16 +707,15 @@ qemuStartup(bool privileged, } VIR_FREE(rundir);
- base = virGetUserConfigDirectory(); - if (!base) + if (!(qemu_driver->configBaseDir = virGetUserConfigDirectory())) goto error; - if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", base) == -1) + if (virAsprintf(&qemu_driver->libDir, "%s/qemu/lib", qemu_driver->configBaseDir) == -1)
This makes for some long lines; is it worth wrapping at the ',' now that you are using a longer name? Or maybe worth declaring:
const char *base = qemu_driver->configBaseDir;
to avoid having to touch these lines?
Yep, I'll do that. 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> virStorageFileGetLVMKey and virStorageFileGetSCSIKey both return heap allocated strings, so the return value should not be marked const. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/storage_file.c | 8 ++++---- src/util/storage_file.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/util/storage_file.c b/src/util/storage_file.c index 776d4f2..ea2a1bc 100644 --- a/src/util/storage_file.c +++ b/src/util/storage_file.c @@ -1192,7 +1192,7 @@ int virStorageFileIsClusterFS(const char *path) } #ifdef LVS -const char *virStorageFileGetLVMKey(const char *path) +char *virStorageFileGetLVMKey(const char *path) { /* * # lvs --noheadings --unbuffered --nosuffix --options "uuid" LVNAME @@ -1244,7 +1244,7 @@ cleanup: return key; } #else -const char *virStorageFileGetLVMKey(const char *path) +char *virStorageFileGetLVMKey(const char *path) { virReportSystemError(ENOSYS, _("Unable to get LVM key for %s"), path); return NULL; @@ -1252,7 +1252,7 @@ const char *virStorageFileGetLVMKey(const char *path) #endif #ifdef HAVE_UDEV -const char *virStorageFileGetSCSIKey(const char *path) +char *virStorageFileGetSCSIKey(const char *path) { char *key = NULL; int status; @@ -1289,7 +1289,7 @@ cleanup: return key; } #else -const char *virStorageFileGetSCSIKey(const char *path) +char *virStorageFileGetSCSIKey(const char *path) { virReportSystemError(ENOSYS, _("Unable to get SCSI key for %s"), path); return NULL; diff --git a/src/util/storage_file.h b/src/util/storage_file.h index abfaca9..9e4516e 100644 --- a/src/util/storage_file.h +++ b/src/util/storage_file.h @@ -101,7 +101,7 @@ int virStorageFileIsClusterFS(const char *path); int virStorageFileIsSharedFSType(const char *path, int fstypes); -const char *virStorageFileGetLVMKey(const char *path); -const char *virStorageFileGetSCSIKey(const char *path); +char *virStorageFileGetLVMKey(const char *path); +char *virStorageFileGetSCSIKey(const char *path); #endif /* __VIR_STORAGE_FILE_H__ */ -- 1.7.11.7
On 12/11/2012 01:41 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
virStorageFileGetLVMKey and virStorageFileGetSCSIKey both return heap allocated strings, so the return value should not be marked const.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/storage_file.c | 8 ++++---- src/util/storage_file.h | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-)
ACK. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
From: "Daniel P. Berrange" <berrange@redhat.com> The virtlockd daemon will maintain locks on behalf of libvirtd. There are two reasons for it to be separate - Avoid risk of other libvirtd threads accidentally releasing fcntl() locks by opening + closing a file that is locked - Ensure locks can be preserved across libvirtd restarts. virtlockd will need to be able to re-exec itself while maintaining locks. This is simpler to achieve if its sole job is maintaining locks Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 2 + cfg.mk | 6 +- libvirt.spec.in | 7 + po/POTFILES.in | 2 + src/Makefile.am | 89 +++- src/locking/lock_daemon.c | 850 +++++++++++++++++++++++++++++++++++++++ src/locking/lock_daemon.h | 43 ++ src/locking/lock_daemon_config.c | 193 +++++++++ src/locking/lock_daemon_config.h | 50 +++ src/locking/virtlockd.init.in | 93 +++++ src/locking/virtlockd.sysconf | 3 + 11 files changed, 1334 insertions(+), 4 deletions(-) create mode 100644 src/locking/lock_daemon.c create mode 100644 src/locking/lock_daemon.h create mode 100644 src/locking/lock_daemon_config.c create mode 100644 src/locking/lock_daemon_config.h create mode 100644 src/locking/virtlockd.init.in create mode 100644 src/locking/virtlockd.sysconf diff --git a/.gitignore b/.gitignore index 0dadd21..1e3a624 100644 --- a/.gitignore +++ b/.gitignore @@ -123,6 +123,8 @@ /src/test_libvirt*.aug /src/util/virkeymaps.h /src/virt-aa-helper +/src/virtlockd +/src/virtlockd.init /tests/*.log /tests/*.pid /tests/*xml2*test diff --git a/cfg.mk b/cfg.mk index f218eb6..95a1d3a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -655,6 +655,8 @@ sc_prohibit_cross_inclusion: @for dir in $(cross_dirs); do \ case $$dir in \ util/) safe="util";; \ + locking/) \ + safe="($$dir|util|conf|rpc)";; \ cpu/ | locking/ | network/ | rpc/ | security/) \ safe="($$dir|util|conf)";; \ xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ @@ -743,7 +745,7 @@ $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protoco # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$ -_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller +_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon exclude_file_name_regexp--sc_avoid_write = \ ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/(shunload|virnettlscontext)test)\.c$$ @@ -776,7 +778,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|nodeinfo)data/|\.(gif|ico|png|diff)$$) -_src2=src/(util/command|libvirt|lxc/lxc_controller) +_src2=src/(util/command|libvirt|lxc/lxc_controller|locking/lock_daemon) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$) diff --git a/libvirt.spec.in b/libvirt.spec.in index d6e1fbe..e12fca4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1661,9 +1661,11 @@ fi %{_unitdir}/libvirtd.service %else %{_sysconfdir}/rc.d/init.d/libvirtd +%{_sysconfdir}/rc.d/init.d/virtlockd %endif %doc daemon/libvirtd.upstart %config(noreplace) %{_sysconfdir}/sysconfig/libvirtd +%config(noreplace) %{_sysconfdir}/sysconfig/virtlockd %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf %if 0%{?fedora} >= 14 || 0%{?rhel} >= 6 %config(noreplace) %{_sysconfdir}/sysctl.d/libvirtd @@ -1730,6 +1732,10 @@ rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %endif +%if %{with_libvirtd} +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver +%endif + %if %{with_qemu} %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug @@ -1763,6 +1769,7 @@ rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd %attr(0755, root, root) %{_libexecdir}/libvirt_iohelper %attr(0755, root, root) %{_sbindir}/libvirtd +%attr(0755, root, root) %{_sbindir}/virtlockd %{_mandir}/man8/libvirtd.8* diff --git a/po/POTFILES.in b/po/POTFILES.in index 51a1f5c..34c688c 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -48,6 +48,8 @@ src/interface/interface_backend_udev.c src/internal.h src/libvirt.c src/libvirt-qemu.c +src/locking/lock_daemon.c +src/locking/lock_daemon_config.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c diff --git a/src/Makefile.am b/src/Makefile.am index 6d2816d..6a66efd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -148,6 +148,13 @@ LOCK_DRIVER_SANLOCK_SOURCES = \ LOCK_DRIVER_SANLOCK_HELPER_SOURCES = \ locking/sanlock_helper.c +LOCK_DAEMON_SOURCES = \ + locking/lock_daemon.h \ + locking/lock_daemon.c \ + locking/lock_daemon_config.h \ + locking/lock_daemon_config.c \ + $(NULL) + NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c \ @@ -1508,6 +1515,76 @@ libvirt_qemu_la_CFLAGS = $(AM_CFLAGS) libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) +if WITH_LIBVIRTD +sbin_PROGRAMS = virtlockd + +virtlockd_SOURCES = $(LOCK_DAEMON_SOURCES) +virtlockd_CFLAGS = \ + $(AM_CFLAGS) \ + $(NULL) +virtlockd_LDFLAGS = \ + $(AM_LDFLAGS) \ + $(CYGWIN_EXTRA_LDFLAGS) \ + $(MINGW_EXTRA_LDFLAGS) \ + $(NULL) +virtlockd_LDADD = \ + libvirt-net-rpc-server.la \ + libvirt-net-rpc.la \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la \ + $(CYGWIN_EXTRA_LIBADD) \ + $(NULL) +if WITH_DTRACE_PROBES +virtlockd_LDADD += libvirt_probes.lo +endif + +else +EXTRA_DIST += $(LOCK_DAEMON_SOURCES) +endif + +EXTRA_DIST += locking/virtlockd.sysconf + +install-sysconfig: + mkdir -p $(DESTDIR)$(sysconfdir)/sysconfig + $(INSTALL_DATA) $(srcdir)/locking/virtlockd.sysconf \ + $(DESTDIR)$(sysconfdir)/sysconfig/virtlockd + +uninstall-sysconfig: + rm -f $(DESTDIR)$(sysconfdir)/sysconfig/virtlockd + +EXTRA_DIST += locking/virtlockd.init.in + +if WITH_LIBVIRTD +if LIBVIRT_INIT_SCRIPT_RED_HAT +install-init:: virtlockd.init install-sysconfig + mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d + $(INSTALL_SCRIPT) virtlockd.init \ + $(DESTDIR)$(sysconfdir)/rc.d/init.d/virtlockd + +uninstall-init:: uninstall-sysconfig + rm -f $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd + +BUILT_SOURCES += virtlockd.init +else +install-init:: +uninstall-init:: +endif +else +install-init:: +uninstall-init:: +endif + +virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e "s!::localstatedir::!$(localstatedir)!g" \ + -e "s!::sbindir::!$(sbindir)!g" \ + -e "s!::sysconfdir::!$(sysconfdir)!g" \ + < $< > $@-t && \ + chmod a+x $@-t && \ + mv $@-t $@ + + + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1745,7 +1822,11 @@ endif endif EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) -install-data-local: +install-data-local: install-init +if WITH_LIBVIRTD + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" +endif $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/images" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" @@ -1794,7 +1875,11 @@ if WITH_NETWORK $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart/default.xml endif -uninstall-local:: +uninstall-local:: uninstall-init +if WITH_LIBVIRTD + rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" ||: + rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" ||: +endif rmdir "$(DESTDIR)$(localstatedir)/cache/libvirt" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/images" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" ||: diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c new file mode 100644 index 0000000..f821d2e --- /dev/null +++ b/src/locking/lock_daemon.c @@ -0,0 +1,850 @@ +/* + * lock_daemon.c: lock management daemon + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <unistd.h> +#include <fcntl.h> +#include <sys/wait.h> +#include <sys/stat.h> +#include <getopt.h> +#include <stdlib.h> +#include <locale.h> + + +#include "lock_daemon.h" +#include "lock_daemon_config.h" +#include "util.h" +#include "virfile.h" +#include "virpidfile.h" +#include "virterror_internal.h" +#include "logging.h" +#include "memory.h" +#include "conf.h" +#include "rpc/virnetserver.h" +#include "virrandom.h" +#include "virhash.h" + +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_LOCKING + +struct _virLockDaemon { + virMutex lock; + virNetServerPtr srv; +}; + +virLockDaemonPtr lockDaemon = NULL; + +enum { + VIR_LOCK_DAEMON_ERR_NONE = 0, + VIR_LOCK_DAEMON_ERR_PIDFILE, + VIR_LOCK_DAEMON_ERR_RUNDIR, + VIR_LOCK_DAEMON_ERR_INIT, + VIR_LOCK_DAEMON_ERR_SIGNAL, + VIR_LOCK_DAEMON_ERR_PRIVS, + VIR_LOCK_DAEMON_ERR_NETWORK, + VIR_LOCK_DAEMON_ERR_CONFIG, + VIR_LOCK_DAEMON_ERR_HOOKS, + + VIR_LOCK_DAEMON_ERR_LAST +}; + +VIR_ENUM_DECL(virDaemonErr) +VIR_ENUM_IMPL(virDaemonErr, VIR_LOCK_DAEMON_ERR_LAST, + "Initialization successful", + "Unable to obtain pidfile", + "Unable to create rundir", + "Unable to initialize libvirt", + "Unable to setup signal handlers", + "Unable to drop privileges", + "Unable to initialize network sockets", + "Unable to load configuration file", + "Unable to look for hook scripts"); + +static void * +virLockDaemonClientNew(virNetServerClientPtr client, + void *opaque); +static void +virLockDaemonClientFree(void *opaque); + +static void +virLockDaemonFree(virLockDaemonPtr lockd) +{ + if (!lockd) + return; + + virObjectUnref(lockd->srv); + + VIR_FREE(lockd); +} + + +static virLockDaemonPtr +virLockDaemonNew(bool privileged) +{ + virLockDaemonPtr lockd; + + if (VIR_ALLOC(lockd) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&lockd->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + VIR_FREE(lockd); + return NULL; + } + + if (!(lockd->srv = virNetServerNew(1, 1, 0, 20, + -1, 0, + false, NULL, + virLockDaemonClientNew, + NULL, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + goto error; + + return lockd; + +error: + virLockDaemonFree(lockd); + return NULL; +} + + +static int +virLockDaemonForkIntoBackground(const char *argv0) +{ + int statuspipe[2]; + if (pipe(statuspipe) < 0) + return -1; + + pid_t pid = fork(); + switch (pid) { + case 0: + { + int stdinfd = -1; + int stdoutfd = -1; + int nextpid; + + VIR_FORCE_CLOSE(statuspipe[0]); + + if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) + goto cleanup; + if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0) + goto cleanup; + if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) + goto cleanup; + if (VIR_CLOSE(stdinfd) < 0) + goto cleanup; + if (VIR_CLOSE(stdoutfd) < 0) + goto cleanup; + + if (setsid() < 0) + goto cleanup; + + nextpid = fork(); + switch (nextpid) { + case 0: + return statuspipe[1]; + case -1: + return -1; + default: + _exit(0); + } + + cleanup: + VIR_FORCE_CLOSE(stdoutfd); + VIR_FORCE_CLOSE(stdinfd); + return -1; + + } + + case -1: + return -1; + + default: + { + int got, exitstatus = 0; + int ret; + char status; + + VIR_FORCE_CLOSE(statuspipe[1]); + + /* We wait to make sure the first child forked successfully */ + if ((got = waitpid(pid, &exitstatus, 0)) < 0 || + got != pid || + exitstatus != 0) { + return -1; + } + + /* Now block until the second child initializes successfully */ + again: + ret = read(statuspipe[0], &status, 1); + if (ret == -1 && errno == EINTR) + goto again; + + if (ret == 1 && status != 0) { + fprintf(stderr, + _("%s: error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), argv0, + virDaemonErrTypeToString(status)); + } + _exit(ret == 1 && status == 0 ? 0 : 1); + } + } +} + + +static int +virLockDaemonPidFilePath(bool privileged, + char **pidfile) +{ + if (privileged) { + if (!(*pidfile = strdup(LOCALSTATEDIR "/run/virtlockd.pid"))) + goto no_memory; + } else { + char *rundir = NULL; + mode_t old_umask; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + + old_umask = umask(077); + if (virFileMakePath(rundir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(pidfile, "%s/virtlockd.pid", rundir) < 0) { + VIR_FREE(rundir); + goto no_memory; + } + + VIR_FREE(rundir); + } + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + +static int +virLockDaemonUnixSocketPaths(bool privileged, + char **sockfile) +{ + if (privileged) { + if (!(*sockfile = strdup(LOCALSTATEDIR "/run/libvirt/virtlockd-sock"))) + goto no_memory; + } else { + char *rundir = NULL; + mode_t old_umask; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + + old_umask = umask(077); + if (virFileMakePath(rundir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(sockfile, "%s/virtlockd-sock", rundir) < 0) { + VIR_FREE(rundir); + goto no_memory; + } + + VIR_FREE(rundir); + } + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + +static void +virLockDaemonErrorHandler(void *opaque ATTRIBUTE_UNUSED, + virErrorPtr err ATTRIBUTE_UNUSED) +{ + /* Don't do anything, since logging infrastructure already + * took care of reporting the error */ +} + + +/* + * Set up the logging environment + * By default if daemonized all errors go to the logfile libvirtd.log, + * but if verbose or error debugging is asked for then also output + * informational and debug messages. Default size if 64 kB. + */ +static int +virLockDaemonSetupLogging(virLockDaemonConfigPtr config, + bool privileged, + bool verbose, + bool godaemon) +{ + virLogReset(); + + /* + * Libvirtd's order of precedence is: + * cmdline > environment > config + * + * In order to achieve this, we must process configuration in + * different order for the log level versus the filters and + * outputs. Because filters and outputs append, we have to look at + * the environment first and then only check the config file if + * there was no result from the environment. The default output is + * then applied only if there was no setting from either of the + * first two. Because we don't have a way to determine if the log + * level has been set, we must process variables in the opposite + * order, each one overriding the previous. + */ + if (config->log_level != 0) + virLogSetDefaultPriority(config->log_level); + + virLogSetFromEnv(); + + virLogSetBufferSize(config->log_buffer_size); + + if (virLogGetNbFilters() == 0) + virLogParseFilters(config->log_filters); + + if (virLogGetNbOutputs() == 0) + virLogParseOutputs(config->log_outputs); + + /* + * If no defined outputs, and either running + * as daemon or not on a tty, then first try + * to direct it to the systemd journal + * (if it exists).... + */ + if (virLogGetNbOutputs() == 0 && + (godaemon || !isatty(STDIN_FILENO))) { + char *tmp; + if (access("/run/systemd/journal/socket", W_OK) >= 0) { + if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) + goto no_memory; + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + } + + /* + * otherwise direct to libvirtd.log when running + * as daemon. Otherwise the default output is stderr. + */ + if (virLogGetNbOutputs() == 0) { + char *tmp = NULL; + + if (godaemon) { + if (privileged) { + if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlockd.log", + virLogGetDefaultPriority(), + LOCALSTATEDIR) == -1) + goto no_memory; + } else { + char *logdir = virGetUserCacheDirectory(); + mode_t old_umask; + + if (!logdir) + goto error; + + old_umask = umask(077); + if (virFileMakePath(logdir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(&tmp, "%d:file:%s/virtlockd.log", + virLogGetDefaultPriority(), logdir) == -1) { + VIR_FREE(logdir); + goto no_memory; + } + VIR_FREE(logdir); + } + } else { + if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) + goto no_memory; + } + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + + /* + * Command line override for --verbose + */ + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + + +/* Display version information. */ +static void +virLockDaemonVersion(const char *argv0) +{ + printf("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION); +} + +static void +virLockDaemonShutdownHandler(virNetServerPtr srv, + siginfo_t *sig ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virNetServerQuit(srv); +} + +static int +virLockDaemonSetupSignals(virNetServerPtr srv) +{ + if (virNetServerAddSignalHandler(srv, SIGINT, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + if (virNetServerAddSignalHandler(srv, SIGQUIT, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + if (virNetServerAddSignalHandler(srv, SIGTERM, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + return 0; +} + +static int +virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path) +{ + virNetServerServicePtr svc; + + VIR_DEBUG("Setting up networking natively"); + + if (!(svc = virNetServerServiceNewUNIX(sock_path, 0700, 0, 0, false, 1, NULL))) + return -1; + + if (virNetServerAddService(srv, svc, NULL) < 0) { + virObjectUnref(svc); + return -1; + } + return 0; +} + + +static void +virLockDaemonClientFree(void *opaque) +{ + virLockDaemonClientPtr priv = opaque; + + if (!priv) + return; + + VIR_DEBUG("priv=%p client=%lld", + priv, + (unsigned long long)priv->clientPid); + + virMutexDestroy(&priv->lock); + VIR_FREE(priv); +} + + +static void * +virLockDaemonClientNew(virNetServerClientPtr client, + void *opaque) +{ + virLockDaemonClientPtr priv; + uid_t clientuid; + gid_t clientgid; + bool privileged = opaque != NULL; + + if (VIR_ALLOC(priv) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportOOMError(); + return NULL; + } + + if (virNetServerClientGetUNIXIdentity(client, + &clientuid, + &clientgid, + &priv->clientPid) < 0) + goto error; + + VIR_DEBUG("New client pid %llu uid %llu", + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + + if (!privileged) { + if (geteuid() != clientuid) { + virReportError(VIR_ERR_OPERATION_DENIED, + _("Disallowing client %llu with uid %llu"), + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + goto error; + } + } else { + if (clientuid != 0) { + virReportError(VIR_ERR_OPERATION_DENIED, + _("Disallowing client %llu with uid %llu"), + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + goto error; + } + } + + return priv; + +error: + virMutexDestroy(&priv->lock); + VIR_FREE(priv); + return NULL; +} + + +static void +virLockDaemonUsage(const char *argv0, bool privileged) +{ + fprintf(stderr, + _("\n" + "Usage:\n" + " %s [options]\n" + "\n" + "Options:\n" + " -v | --verbose Verbose messages.\n" + " -d | --daemon Run as a daemon & write PID file.\n" + " -t | --timeout <secs> Exit after timeout period.\n" + " -f | --config <file> Configuration file.\n" + " | --version Display version information.\n" + " -p | --pid-file <file> Change name of PID file.\n" + "\n" + "libvirt lock management daemon:\n"), argv0); + + if (privileged) { + fprintf(stderr, + _("\n" + " Default paths:\n" + "\n" + " Configuration file (unless overridden by -f):\n" + " %s/libvirt/virtlockd.conf\n" + "\n" + " Sockets:\n" + " %s/run/libvirt/virtlockd-sock\n" + "\n" + " PID file (unless overridden by -p):\n" + " %s/run/virtlockd.pid\n" + "\n"), + SYSCONFDIR, + LOCALSTATEDIR, + LOCALSTATEDIR); + } else { + fprintf(stderr, + "%s", _("\n" + " Default paths:\n" + "\n" + " Configuration file (unless overridden by -f):\n" + " $XDG_CONFIG_HOME/libvirt/virtlockd.conf\n" + "\n" + " Sockets:\n" + " $XDG_RUNTIME_DIR/libvirt/virtlockd-sock\n" + "\n" + " PID file:\n" + " $XDG_RUNTIME_DIR/libvirt/virtlockd.pid\n" + "\n")); + } +} + +enum { + OPT_VERSION = 129 +}; + +#define MAX_LISTEN 5 +int main(int argc, char **argv) { + char *remote_config_file = NULL; + int statuswrite = -1; + int ret = 1; + int verbose = 0; + int godaemon = 0; + int timeout = 0; + char *run_dir = NULL; + char *pid_file = NULL; + int pid_file_fd = -1; + char *sock_file = NULL; + bool implicit_conf = false; + mode_t old_umask; + bool privileged = false; + virLockDaemonConfigPtr config = NULL; + + struct option opts[] = { + { "verbose", no_argument, &verbose, 1}, + { "daemon", no_argument, &godaemon, 1}, + { "config", required_argument, NULL, 'f'}, + { "timeout", required_argument, NULL, 't'}, + { "pid-file", required_argument, NULL, 'p'}, + { "version", no_argument, NULL, OPT_VERSION }, + { "help", no_argument, NULL, '?' }, + {0, 0, 0, 0} + }; + + privileged = getuid() == 0; + + if (setlocale(LC_ALL, "") == NULL || + bindtextdomain(PACKAGE, LOCALEDIR) == NULL || + textdomain(PACKAGE) == NULL || + virThreadInitialize() < 0 || + virErrorInitialize() < 0) { + fprintf(stderr, _("%s: initialization failed\n"), argv[0]); + exit(EXIT_FAILURE); + } + + while (1) { + int optidx = 0; + int c; + char *tmp; + + c = getopt_long(argc, argv, "ldf:p:t:v", opts, &optidx); + + if (c == -1) { + break; + } + + switch (c) { + case 0: + /* Got one of the flags */ + break; + case 'v': + verbose = 1; + break; + case 'd': + godaemon = 1; + break; + + case 't': + if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0 + || timeout <= 0 + /* Ensure that we can multiply by 1000 without overflowing. */ + || timeout > INT_MAX / 1000) + timeout = -1; + break; + + case 'p': + VIR_FREE(pid_file); + if (!(pid_file = strdup(optarg))) + exit(EXIT_FAILURE); + break; + + case 'f': + VIR_FREE(remote_config_file); + if (!(remote_config_file = strdup(optarg))) + exit(EXIT_FAILURE); + break; + + case OPT_VERSION: + virLockDaemonVersion(argv[0]); + return 0; + + case '?': + virLockDaemonUsage(argv[0], privileged); + return 2; + + default: + fprintf(stderr, _("%s: internal error: unknown flag: %c\n"), + argv[0], c); + exit(EXIT_FAILURE); + } + } + + if (!(config = virLockDaemonConfigNew(privileged))) { + VIR_ERROR(_("Can't create initial configuration")); + exit(EXIT_FAILURE); + } + + /* No explicit config, so try and find a default one */ + if (remote_config_file == NULL) { + implicit_conf = true; + if (virLockDaemonConfigFilePath(privileged, + &remote_config_file) < 0) { + VIR_ERROR(_("Can't determine config path")); + exit(EXIT_FAILURE); + } + } + + /* Read the config file if it exists*/ + if (remote_config_file && + virLockDaemonConfigLoadFile(config, remote_config_file, implicit_conf) < 0) { + virErrorPtr err = virGetLastError(); + if (err && err->message) + VIR_ERROR(_("Can't load config file: %s: %s"), + err->message, remote_config_file); + else + VIR_ERROR(_("Can't load config file: %s"), remote_config_file); + exit(EXIT_FAILURE); + } + + if (virLockDaemonSetupLogging(config, privileged, verbose, godaemon) < 0) { + VIR_ERROR(_("Can't initialize logging")); + exit(EXIT_FAILURE); + } + + if (!pid_file && + virLockDaemonPidFilePath(privileged, + &pid_file) < 0) { + VIR_ERROR(_("Can't determine pid file path.")); + exit(EXIT_FAILURE); + } + VIR_DEBUG("Decided on pid file path '%s'", NULLSTR(pid_file)); + + if (virLockDaemonUnixSocketPaths(privileged, + &sock_file) < 0) { + VIR_ERROR(_("Can't determine socket paths")); + exit(EXIT_FAILURE); + } + VIR_DEBUG("Decided on socket paths '%s'", + sock_file); + + if (godaemon) { + char ebuf[1024]; + + if (chdir("/") < 0) { + VIR_ERROR(_("cannot change to root directory: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto cleanup; + } + + if ((statuswrite = virLockDaemonForkIntoBackground(argv[0])) < 0) { + VIR_ERROR(_("Failed to fork as daemon: %s"), + virStrerror(errno, ebuf, sizeof(ebuf))); + goto cleanup; + } + } + + /* Ensure the rundir exists (on tmpfs on some systems) */ + if (privileged) { + run_dir = strdup(LOCALSTATEDIR "/run/libvirt"); + } else { + run_dir = virGetUserRuntimeDirectory(); + + if (!run_dir) { + VIR_ERROR(_("Can't determine user directory")); + goto cleanup; + } + } + if (!run_dir) { + virReportOOMError(); + goto cleanup; + } + + if (privileged) + old_umask = umask(022); + else + old_umask = umask(077); + VIR_DEBUG("Ensuring run dir '%s' exists", run_dir); + if (virFileMakePath(run_dir) < 0) { + char ebuf[1024]; + VIR_ERROR(_("unable to create rundir %s: %s"), run_dir, + virStrerror(errno, ebuf, sizeof(ebuf))); + ret = VIR_LOCK_DAEMON_ERR_RUNDIR; + goto cleanup; + } + umask(old_umask); + + /* If we have a pidfile set, claim it now, exiting if already taken */ + if ((pid_file_fd = virPidFileAcquirePath(pid_file, getpid())) < 0) { + ret = VIR_LOCK_DAEMON_ERR_PIDFILE; + goto cleanup; + } + + if (!(lockDaemon = virLockDaemonNew(privileged))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + + if (virLockDaemonSetupNetworking(lockDaemon->srv, sock_file) < 0) { + ret = VIR_LOCK_DAEMON_ERR_NETWORK; + goto cleanup; + } + + if ((virLockDaemonSetupSignals(lockDaemon->srv)) < 0) { + ret = VIR_LOCK_DAEMON_ERR_SIGNAL; + goto cleanup; + } + + /* Disable error func, now logging is setup */ + virSetErrorFunc(NULL, virLockDaemonErrorHandler); + + + /* Tell parent of daemon that basic initialization is complete + * In particular we're ready to accept net connections & have + * written the pidfile + */ + if (statuswrite != -1) { + char status = 0; + while (write(statuswrite, &status, 1) == -1 && + errno == EINTR) + ; + VIR_FORCE_CLOSE(statuswrite); + } + + /* Start accepting new clients from network */ + + virNetServerUpdateServices(lockDaemon->srv, true); + virNetServerRun(lockDaemon->srv); + ret = 0; + +cleanup: + virLockDaemonFree(lockDaemon); + if (statuswrite != -1) { + if (ret != 0) { + /* Tell parent of daemon what failed */ + char status = ret; + while (write(statuswrite, &status, 1) == -1 && + errno == EINTR) + ; + } + VIR_FORCE_CLOSE(statuswrite); + } + if (pid_file_fd != -1) + virPidFileReleasePath(pid_file, pid_file_fd); + VIR_FREE(pid_file); + VIR_FREE(sock_file); + VIR_FREE(run_dir); + return ret; +} diff --git a/src/locking/lock_daemon.h b/src/locking/lock_daemon.h new file mode 100644 index 0000000..7bc8c2e --- /dev/null +++ b/src/locking/lock_daemon.h @@ -0,0 +1,43 @@ +/* + * lock_daemon.h: lock management daemon + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_LOCK_DAEMON_H__ +# define __VIR_LOCK_DAEMON_H__ + +# include "virlockspace.h" +# include "threads.h" + +typedef struct _virLockDaemon virLockDaemon; +typedef virLockDaemon *virLockDaemonPtr; + +typedef struct _virLockDaemonClient virLockDaemonClient; +typedef virLockDaemonClient *virLockDaemonClientPtr; + +struct _virLockDaemonClient { + virMutex lock; + + pid_t clientPid; +}; + +extern virLockDaemonPtr lockDaemon; + +#endif /* __VIR_LOCK_DAEMON_H__ */ diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c new file mode 100644 index 0000000..c64de67 --- /dev/null +++ b/src/locking/lock_daemon_config.c @@ -0,0 +1,193 @@ +/* + * lock_daemon_config.h: virtlockd config file handling + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "lock_daemon_config.h" +#include "conf.h" +#include "memory.h" +#include "virterror_internal.h" +#include "logging.h" +#include "rpc/virnetserver.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_CONF + + +/* A helper function used by each of the following macros. */ +static int +checkType(virConfValuePtr p, const char *filename, + const char *key, virConfType required_type) +{ + if (p->type != required_type) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("remoteReadConfigFile: %s: %s: invalid type:" + " got %s; expected %s"), filename, key, + virConfTypeName(p->type), + virConfTypeName(required_type)); + return -1; + } + return 0; +} + +/* If there is no config data for the key, #var_name, then do nothing. + If there is valid data of type VIR_CONF_STRING, and strdup succeeds, + store the result in var_name. Otherwise, (i.e. invalid type, or strdup + failure), give a diagnostic and "goto" the cleanup-and-fail label. */ +#define GET_CONF_STR(conf, filename, var_name) \ + do { \ + virConfValuePtr p = virConfGetValue(conf, #var_name); \ + if (p) { \ + if (checkType(p, filename, #var_name, VIR_CONF_STRING) < 0) \ + goto error; \ + VIR_FREE(data->var_name); \ + if (!(data->var_name = strdup(p->str))) { \ + virReportOOMError(); \ + goto error; \ + } \ + } \ + } while (0) + +/* Like GET_CONF_STR, but for integral values. */ +#define GET_CONF_INT(conf, filename, var_name) \ + do { \ + virConfValuePtr p = virConfGetValue(conf, #var_name); \ + if (p) { \ + if (checkType(p, filename, #var_name, VIR_CONF_LONG) < 0) \ + goto error; \ + data->var_name = p->l; \ + } \ + } while (0) + +int +virLockDaemonConfigFilePath(bool privileged, char **configfile) +{ + if (privileged) { + if (!(*configfile = strdup(SYSCONFDIR "/libvirt/virtlockd.conf"))) + goto no_memory; + } else { + char *configdir = NULL; + + if (!(configdir = virGetUserConfigDirectory())) + goto error; + + if (virAsprintf(configfile, "%s/virtlockd.conf", configdir) < 0) { + VIR_FREE(configdir); + goto no_memory; + } + VIR_FREE(configdir); + } + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + +virLockDaemonConfigPtr +virLockDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) +{ + virLockDaemonConfigPtr data; + + if (VIR_ALLOC(data) < 0) { + virReportOOMError(); + return NULL; + } + + data->log_buffer_size = 64; + + return data; +} + +void +virLockDaemonConfigFree(virLockDaemonConfigPtr data) +{ + if (!data) + return; + + VIR_FREE(data->log_filters); + VIR_FREE(data->log_outputs); + + VIR_FREE(data); +} + +static int +virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data, + const char *filename, + virConfPtr conf) +{ + GET_CONF_INT(conf, filename, log_level); + GET_CONF_STR(conf, filename, log_filters); + GET_CONF_STR(conf, filename, log_outputs); + GET_CONF_INT(conf, filename, log_buffer_size); + + return 0; + +error: + return -1; +} + + +/* Read the config file if it exists. + * Only used in the remote case, hence the name. + */ +int +virLockDaemonConfigLoadFile(virLockDaemonConfigPtr data, + const char *filename, + bool allow_missing) +{ + virConfPtr conf; + int ret; + + if (allow_missing && + access(filename, R_OK) == -1 && + errno == ENOENT) + return 0; + + conf = virConfReadFile(filename, 0); + if (!conf) + return -1; + + ret = virLockDaemonConfigLoadOptions(data, filename, conf); + virConfFree(conf); + return ret; +} + +int virLockDaemonConfigLoadData(virLockDaemonConfigPtr data, + const char *filename, + const char *filedata) +{ + virConfPtr conf; + int ret; + + conf = virConfReadMem(filedata, strlen(filedata), 0); + if (!conf) + return -1; + + ret = virLockDaemonConfigLoadOptions(data, filename, conf); + virConfFree(conf); + return ret; +} diff --git a/src/locking/lock_daemon_config.h b/src/locking/lock_daemon_config.h new file mode 100644 index 0000000..8cb0e5d --- /dev/null +++ b/src/locking/lock_daemon_config.h @@ -0,0 +1,50 @@ +/* + * lock_daemon_config.h: virtlockd config file handling + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * Copyright (C) 2006 Daniel P. Berrange + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_LOCK_DAEMON_CONFIG_H__ +# define __VIR_LOCK_DAEMON_CONFIG_H__ + +# include "internal.h" + +typedef struct _virLockDaemonConfig virLockDaemonConfig; +typedef virLockDaemonConfig *virLockDaemonConfigPtr; + +struct _virLockDaemonConfig { + int log_level; + char *log_filters; + char *log_outputs; + int log_buffer_size; +}; + + +int virLockDaemonConfigFilePath(bool privileged, char **configfile); +virLockDaemonConfigPtr virLockDaemonConfigNew(bool privileged); +void virLockDaemonConfigFree(virLockDaemonConfigPtr data); +int virLockDaemonConfigLoadFile(virLockDaemonConfigPtr data, + const char *filename, + bool allow_missing); +int virLockDaemonConfigLoadData(virLockDaemonConfigPtr data, + const char *filename, + const char *filedata); + +#endif /* __LIBVIRTD_CONFIG_H__ */ diff --git a/src/locking/virtlockd.init.in b/src/locking/virtlockd.init.in new file mode 100644 index 0000000..e55cbf9 --- /dev/null +++ b/src/locking/virtlockd.init.in @@ -0,0 +1,93 @@ +#!/bin/sh + +# the following is the LSB init header see +# http://www.linux-foundation.org/spec//booksets/LSB-Core-generic/LSB-Core-gen... +# +### BEGIN INIT INFO +# Provides: virtlockd +# Default-Start: 3 4 5 +# Short-Description: virtual machine lock manager +# Description: This is a daemon for managing locks +# on virtual machine disk images +### END INIT INFO + +# the following is chkconfig init header +# +# virtlockd: virtual machine lock manager +# +# chkconfig: 345 97 03 +# description: This is a daemon for managing locks \ +# on virtual machine disk images +# +# processname: virtlockd +# pidfile: ::localstatedir::/run/libvirt/virtlockd.pid +# + +# Source function library. +. ::sysconfdir::/rc.d/init.d/functions + +SERVICE=virtlockd +PROCESS=virtlockd +PIDFILE=::localstatedir::/run/libvirt/lockd/$SERVICE.pid + +VIRTLOCKD_ARGS= + +test -f ::sysconfdir::/sysconfig/virtlockd && . ::sysconfdir::/sysconfig/virtlockd + +RETVAL=0 + +start() { + echo -n $"Starting $SERVICE daemon: " + daemon --pidfile $PIDFILE --check $SERVICE $PROCESS --daemon $VIRTLOCKD_ARGS + RETVAL=$? + echo + [ $RETVAL -eq 0 ] && touch ::localstatedir::/lock/subsys/$SERVICE +} + +stop() { + echo -n $"Stopping $SERVICE daemon: " + + killproc -p $PIDFILE $PROCESS + RETVAL=$? + echo + if [ $RETVAL -eq 0 ]; then + rm -f ::localstatedir::/lock/subsys/$SERVICE + rm -f $PIDFILE + fi +} + +restart() { + stop + start +} + +reload() { + echo -n $"Reloading $SERVICE configuration: " + + killproc -p $PIDFILE $PROCESS -HUP + RETVAL=$? + echo + return $RETVAL +} + +# See how we were called. +case "$1" in + start|stop|restart|reload) + $1 + ;; + status) + status -p $PIDFILE $PROCESS + RETVAL=$? + ;; + force-reload) + reload + ;; + condrestart|try-restart) + [ -f ::localstatedir::/lock/subsys/$SERVICE ] && restart || : + ;; + *) + echo $"Usage: $0 {start|stop|status|restart|condrestart|reload|force-reload|try-restart}" + exit 2 + ;; +esac +exit $RETVAL diff --git a/src/locking/virtlockd.sysconf b/src/locking/virtlockd.sysconf new file mode 100644 index 0000000..d44dc46 --- /dev/null +++ b/src/locking/virtlockd.sysconf @@ -0,0 +1,3 @@ +# +# Pass extra arguments to virtlockd +#VIRTLOCKD_ARGS= -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon will maintain locks on behalf of libvirtd. There are two reasons for it to be separate
- Avoid risk of other libvirtd threads accidentally releasing fcntl() locks by opening + closing a file that is locked - Ensure locks can be preserved across libvirtd restarts. virtlockd will need to be able to re-exec itself while maintaining locks. This is simpler to achieve if its sole job is maintaining locks
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 2 + cfg.mk | 6 +- libvirt.spec.in | 7 + po/POTFILES.in | 2 + src/Makefile.am | 89 +++- src/locking/lock_daemon.c | 850 +++++++++++++++++++++++++++++++++++++++ src/locking/lock_daemon.h | 43 ++ src/locking/lock_daemon_config.c | 193 +++++++++ src/locking/lock_daemon_config.h | 50 +++ src/locking/virtlockd.init.in | 93 +++++ src/locking/virtlockd.sysconf | 3 + 11 files changed, 1334 insertions(+), 4 deletions(-) create mode 100644 src/locking/lock_daemon.c create mode 100644 src/locking/lock_daemon.h create mode 100644 src/locking/lock_daemon_config.c create mode 100644 src/locking/lock_daemon_config.h create mode 100644 src/locking/virtlockd.init.in create mode 100644 src/locking/virtlockd.sysconf
diff --git a/.gitignore b/.gitignore index 0dadd21..1e3a624 100644 --- a/.gitignore +++ b/.gitignore @@ -123,6 +123,8 @@ /src/test_libvirt*.aug /src/util/virkeymaps.h /src/virt-aa-helper +/src/virtlockd +/src/virtlockd.init /tests/*.log /tests/*.pid /tests/*xml2*test diff --git a/cfg.mk b/cfg.mk index f218eb6..95a1d3a 100644 --- a/cfg.mk +++ b/cfg.mk @@ -655,6 +655,8 @@ sc_prohibit_cross_inclusion: @for dir in $(cross_dirs); do \ case $$dir in \ util/) safe="util";; \ + locking/) \ + safe="($$dir|util|conf|rpc)";; \ cpu/ | locking/ | network/ | rpc/ | security/) \ safe="($$dir|util|conf)";; \ xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \ @@ -743,7 +745,7 @@ $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protoco # List all syntax-check exemptions: exclude_file_name_regexp--sc_avoid_strcase = ^tools/virsh\.h$$
-_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller +_src1=libvirt|fdstream|qemu/qemu_monitor|util/(command|util)|xen/xend_internal|rpc/virnetsocket|lxc/lxc_controller|locking/lock_daemon exclude_file_name_regexp--sc_avoid_write = \ ^(src/($(_src1))|daemon/libvirtd|tools/console|tests/(shunload|virnettlscontext)test)\.c$$
@@ -776,7 +778,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ (^tests/(qemuhelp|nodeinfo)data/|\.(gif|ico|png|diff)$$)
-_src2=src/(util/command|libvirt|lxc/lxc_controller) +_src2=src/(util/command|libvirt|lxc/lxc_controller|locking/lock_daemon) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ (^($(_src2)|tests/testutils|daemon/libvirtd)\.c$$)
diff --git a/libvirt.spec.in b/libvirt.spec.in index d6e1fbe..e12fca4 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1661,9 +1661,11 @@ fi %{_unitdir}/libvirtd.service %else %{_sysconfdir}/rc.d/init.d/libvirtd +%{_sysconfdir}/rc.d/init.d/virtlockd %endif %doc daemon/libvirtd.upstart %config(noreplace) %{_sysconfdir}/sysconfig/libvirtd +%config(noreplace) %{_sysconfdir}/sysconfig/virtlockd %config(noreplace) %{_sysconfdir}/libvirt/libvirtd.conf %if 0%{?fedora} >= 14 || 0%{?rhel} >= 6 %config(noreplace) %{_sysconfdir}/sysctl.d/libvirtd @@ -1730,6 +1732,10 @@ rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd %dir %attr(0755, root, root) %{_localstatedir}/lib/libvirt/dnsmasq/ %endif
+%if %{with_libvirtd} +%dir %attr(0755, root, root) %{_libdir}/libvirt/lock-driver +%endif + %if %{with_qemu} %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug @@ -1763,6 +1769,7 @@ rm -f $RPM_BUILD_ROOT%{_sysconfdir}/sysctl.d/libvirtd
%attr(0755, root, root) %{_libexecdir}/libvirt_iohelper %attr(0755, root, root) %{_sbindir}/libvirtd +%attr(0755, root, root) %{_sbindir}/virtlockd
%{_mandir}/man8/libvirtd.8*
diff --git a/po/POTFILES.in b/po/POTFILES.in index 51a1f5c..34c688c 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -48,6 +48,8 @@ src/interface/interface_backend_udev.c src/internal.h src/libvirt.c src/libvirt-qemu.c +src/locking/lock_daemon.c +src/locking/lock_daemon_config.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c diff --git a/src/Makefile.am b/src/Makefile.am index 6d2816d..6a66efd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -148,6 +148,13 @@ LOCK_DRIVER_SANLOCK_SOURCES = \ LOCK_DRIVER_SANLOCK_HELPER_SOURCES = \ locking/sanlock_helper.c
+LOCK_DAEMON_SOURCES = \ + locking/lock_daemon.h \ + locking/lock_daemon.c \ + locking/lock_daemon_config.h \ + locking/lock_daemon_config.c \ + $(NULL) + NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c \ @@ -1508,6 +1515,76 @@ libvirt_qemu_la_CFLAGS = $(AM_CFLAGS) libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE)
+if WITH_LIBVIRTD +sbin_PROGRAMS = virtlockd + +virtlockd_SOURCES = $(LOCK_DAEMON_SOURCES) +virtlockd_CFLAGS = \ + $(AM_CFLAGS) \ + $(NULL) +virtlockd_LDFLAGS = \ + $(AM_LDFLAGS) \ + $(CYGWIN_EXTRA_LDFLAGS) \ + $(MINGW_EXTRA_LDFLAGS) \ + $(NULL) +virtlockd_LDADD = \ + libvirt-net-rpc-server.la \ + libvirt-net-rpc.la \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la \ + $(CYGWIN_EXTRA_LIBADD) \ + $(NULL) +if WITH_DTRACE_PROBES +virtlockd_LDADD += libvirt_probes.lo +endif + +else +EXTRA_DIST += $(LOCK_DAEMON_SOURCES) +endif + +EXTRA_DIST += locking/virtlockd.sysconf + +install-sysconfig: + mkdir -p $(DESTDIR)$(sysconfdir)/sysconfig + $(INSTALL_DATA) $(srcdir)/locking/virtlockd.sysconf \ + $(DESTDIR)$(sysconfdir)/sysconfig/virtlockd + +uninstall-sysconfig: + rm -f $(DESTDIR)$(sysconfdir)/sysconfig/virtlockd + +EXTRA_DIST += locking/virtlockd.init.in + +if WITH_LIBVIRTD +if LIBVIRT_INIT_SCRIPT_RED_HAT +install-init:: virtlockd.init install-sysconfig + mkdir -p $(DESTDIR)$(sysconfdir)/rc.d/init.d + $(INSTALL_SCRIPT) virtlockd.init \ + $(DESTDIR)$(sysconfdir)/rc.d/init.d/virtlockd + +uninstall-init:: uninstall-sysconfig + rm -f $(DESTDIR)$(sysconfdir)/rc.d/init.d/libvirtd + +BUILT_SOURCES += virtlockd.init +else +install-init:: +uninstall-init:: +endif +else +install-init:: +uninstall-init:: +endif + +virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e "s!::localstatedir::!$(localstatedir)!g" \ + -e "s!::sbindir::!$(sbindir)!g" \ + -e "s!::sysconfdir::!$(sysconfdir)!g" \ + < $< > $@-t && \ + chmod a+x $@-t && \ + mv $@-t $@ + + + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1745,7 +1822,11 @@ endif endif EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES)
-install-data-local: +install-data-local: install-init +if WITH_LIBVIRTD + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" +endif $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/images" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" @@ -1794,7 +1875,11 @@ if WITH_NETWORK $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart/default.xml endif
-uninstall-local:: +uninstall-local:: uninstall-init +if WITH_LIBVIRTD + rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" ||: + rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" ||: +endif rmdir "$(DESTDIR)$(localstatedir)/cache/libvirt" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/images" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/filesystems" ||: diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c new file mode 100644 index 0000000..f821d2e --- /dev/null +++ b/src/locking/lock_daemon.c @@ -0,0 +1,850 @@ +/* + * lock_daemon.c: lock management daemon + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <unistd.h> +#include <fcntl.h> +#include <sys/wait.h> +#include <sys/stat.h> +#include <getopt.h> +#include <stdlib.h> +#include <locale.h> + + +#include "lock_daemon.h" +#include "lock_daemon_config.h" +#include "util.h" +#include "virfile.h" +#include "virpidfile.h" +#include "virterror_internal.h" +#include "logging.h" +#include "memory.h" +#include "conf.h" +#include "rpc/virnetserver.h" +#include "virrandom.h" +#include "virhash.h"
You may want to sort these so we don't introduce yet another file which needs sorting - assuming we will stick to your idea of sorting header files include after the release.
+ +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_LOCKING + +struct _virLockDaemon { + virMutex lock; + virNetServerPtr srv; +}; + +virLockDaemonPtr lockDaemon = NULL; + +enum { + VIR_LOCK_DAEMON_ERR_NONE = 0, + VIR_LOCK_DAEMON_ERR_PIDFILE, + VIR_LOCK_DAEMON_ERR_RUNDIR, + VIR_LOCK_DAEMON_ERR_INIT, + VIR_LOCK_DAEMON_ERR_SIGNAL, + VIR_LOCK_DAEMON_ERR_PRIVS, + VIR_LOCK_DAEMON_ERR_NETWORK, + VIR_LOCK_DAEMON_ERR_CONFIG, + VIR_LOCK_DAEMON_ERR_HOOKS, + + VIR_LOCK_DAEMON_ERR_LAST +}; + +VIR_ENUM_DECL(virDaemonErr) +VIR_ENUM_IMPL(virDaemonErr, VIR_LOCK_DAEMON_ERR_LAST, + "Initialization successful", + "Unable to obtain pidfile", + "Unable to create rundir", + "Unable to initialize libvirt", + "Unable to setup signal handlers", + "Unable to drop privileges", + "Unable to initialize network sockets", + "Unable to load configuration file", + "Unable to look for hook scripts"); + +static void * +virLockDaemonClientNew(virNetServerClientPtr client, + void *opaque); +static void +virLockDaemonClientFree(void *opaque); + +static void +virLockDaemonFree(virLockDaemonPtr lockd) +{ + if (!lockd) + return; + + virObjectUnref(lockd->srv); + + VIR_FREE(lockd); +} + + +static virLockDaemonPtr +virLockDaemonNew(bool privileged) +{ + virLockDaemonPtr lockd; + + if (VIR_ALLOC(lockd) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&lockd->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + VIR_FREE(lockd); + return NULL; + } + + if (!(lockd->srv = virNetServerNew(1, 1, 0, 20, + -1, 0, + false, NULL, + virLockDaemonClientNew, + NULL, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + goto error; + + return lockd; + +error: + virLockDaemonFree(lockd); + return NULL; +} + + +static int +virLockDaemonForkIntoBackground(const char *argv0) +{ + int statuspipe[2]; + if (pipe(statuspipe) < 0) + return -1; + + pid_t pid = fork(); + switch (pid) { + case 0: + { + int stdinfd = -1; + int stdoutfd = -1; + int nextpid; + + VIR_FORCE_CLOSE(statuspipe[0]); + + if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) + goto cleanup; + if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0) + goto cleanup; + if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) + goto cleanup; + if (VIR_CLOSE(stdinfd) < 0) + goto cleanup; + if (VIR_CLOSE(stdoutfd) < 0) + goto cleanup; + + if (setsid() < 0) + goto cleanup; + + nextpid = fork(); + switch (nextpid) { + case 0: + return statuspipe[1]; + case -1: + return -1; + default: + _exit(0); + } + + cleanup: + VIR_FORCE_CLOSE(stdoutfd); + VIR_FORCE_CLOSE(stdinfd); + return -1; + + } + + case -1: + return -1; + + default: + { + int got, exitstatus = 0; + int ret; + char status; + + VIR_FORCE_CLOSE(statuspipe[1]); + + /* We wait to make sure the first child forked successfully */ + if ((got = waitpid(pid, &exitstatus, 0)) < 0 || + got != pid || + exitstatus != 0) { + return -1; + } + + /* Now block until the second child initializes successfully */ + again: + ret = read(statuspipe[0], &status, 1); + if (ret == -1 && errno == EINTR) + goto again; + + if (ret == 1 && status != 0) { + fprintf(stderr, + _("%s: error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), argv0, + virDaemonErrTypeToString(status)); + } + _exit(ret == 1 && status == 0 ? 0 : 1); + } + } +}
This is basically a copy-paste of daemonForkIntoBackground(). I wonder if we can use (modified) version of it instead of this.
+ + +static int +virLockDaemonPidFilePath(bool privileged, + char **pidfile) +{ + if (privileged) { + if (!(*pidfile = strdup(LOCALSTATEDIR "/run/virtlockd.pid"))) + goto no_memory; + } else { + char *rundir = NULL; + mode_t old_umask; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + + old_umask = umask(077); + if (virFileMakePath(rundir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(pidfile, "%s/virtlockd.pid", rundir) < 0) { + VIR_FREE(rundir); + goto no_memory; + } + + VIR_FREE(rundir); + } + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + +static int +virLockDaemonUnixSocketPaths(bool privileged, + char **sockfile) +{ + if (privileged) { + if (!(*sockfile = strdup(LOCALSTATEDIR "/run/libvirt/virtlockd-sock"))) + goto no_memory; + } else { + char *rundir = NULL; + mode_t old_umask; + + if (!(rundir = virGetUserRuntimeDirectory())) + goto error; + + old_umask = umask(077); + if (virFileMakePath(rundir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(sockfile, "%s/virtlockd-sock", rundir) < 0) { + VIR_FREE(rundir); + goto no_memory; + } + + VIR_FREE(rundir); + } + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +} + + +static void +virLockDaemonErrorHandler(void *opaque ATTRIBUTE_UNUSED, + virErrorPtr err ATTRIBUTE_UNUSED) +{ + /* Don't do anything, since logging infrastructure already + * took care of reporting the error */ +} + + +/* + * Set up the logging environment + * By default if daemonized all errors go to the logfile libvirtd.log, + * but if verbose or error debugging is asked for then also output + * informational and debug messages. Default size if 64 kB. + */ +static int +virLockDaemonSetupLogging(virLockDaemonConfigPtr config, + bool privileged, + bool verbose, + bool godaemon) +{ + virLogReset(); + + /* + * Libvirtd's order of precedence is: + * cmdline > environment > config + * + * In order to achieve this, we must process configuration in + * different order for the log level versus the filters and + * outputs. Because filters and outputs append, we have to look at + * the environment first and then only check the config file if + * there was no result from the environment. The default output is + * then applied only if there was no setting from either of the + * first two. Because we don't have a way to determine if the log + * level has been set, we must process variables in the opposite + * order, each one overriding the previous. + */ + if (config->log_level != 0) + virLogSetDefaultPriority(config->log_level); + + virLogSetFromEnv(); + + virLogSetBufferSize(config->log_buffer_size); + + if (virLogGetNbFilters() == 0) + virLogParseFilters(config->log_filters); + + if (virLogGetNbOutputs() == 0) + virLogParseOutputs(config->log_outputs); + + /* + * If no defined outputs, and either running + * as daemon or not on a tty, then first try + * to direct it to the systemd journal + * (if it exists).... + */ + if (virLogGetNbOutputs() == 0 && + (godaemon || !isatty(STDIN_FILENO))) { + char *tmp; + if (access("/run/systemd/journal/socket", W_OK) >= 0) { + if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) + goto no_memory; + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + } + + /* + * otherwise direct to libvirtd.log when running + * as daemon. Otherwise the default output is stderr. + */ + if (virLogGetNbOutputs() == 0) { + char *tmp = NULL; + + if (godaemon) { + if (privileged) { + if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlockd.log", + virLogGetDefaultPriority(), + LOCALSTATEDIR) == -1) + goto no_memory; + } else { + char *logdir = virGetUserCacheDirectory(); + mode_t old_umask; + + if (!logdir) + goto error; + + old_umask = umask(077); + if (virFileMakePath(logdir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(&tmp, "%d:file:%s/virtlockd.log", + virLogGetDefaultPriority(), logdir) == -1) { + VIR_FREE(logdir); + goto no_memory; + } + VIR_FREE(logdir); + } + } else { + if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) + goto no_memory; + } + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + + /* + * Command line override for --verbose + */ + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +}
Same here. This one is even closer to daemonSetupLogging()
+ + + +/* Display version information. */ +static void +virLockDaemonVersion(const char *argv0) +{ + printf("%s (%s) %s\n", argv0, PACKAGE_NAME, PACKAGE_VERSION); +} + +static void +virLockDaemonShutdownHandler(virNetServerPtr srv, + siginfo_t *sig ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + virNetServerQuit(srv); +} + +static int +virLockDaemonSetupSignals(virNetServerPtr srv) +{ + if (virNetServerAddSignalHandler(srv, SIGINT, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + if (virNetServerAddSignalHandler(srv, SIGQUIT, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + if (virNetServerAddSignalHandler(srv, SIGTERM, virLockDaemonShutdownHandler, NULL) < 0) + return -1; + return 0; +} + +static int +virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path) +{ + virNetServerServicePtr svc; + + VIR_DEBUG("Setting up networking natively"); + + if (!(svc = virNetServerServiceNewUNIX(sock_path, 0700, 0, 0, false, 1, NULL))) + return -1; + + if (virNetServerAddService(srv, svc, NULL) < 0) { + virObjectUnref(svc); + return -1; + } + return 0; +} + + +static void +virLockDaemonClientFree(void *opaque) +{ + virLockDaemonClientPtr priv = opaque; + + if (!priv) + return; + + VIR_DEBUG("priv=%p client=%lld", + priv, + (unsigned long long)priv->clientPid); + + virMutexDestroy(&priv->lock); + VIR_FREE(priv); +} + + +static void * +virLockDaemonClientNew(virNetServerClientPtr client, + void *opaque) +{ + virLockDaemonClientPtr priv; + uid_t clientuid; + gid_t clientgid; + bool privileged = opaque != NULL; + + if (VIR_ALLOC(priv) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportOOMError(); + return NULL; + } + + if (virNetServerClientGetUNIXIdentity(client, + &clientuid, + &clientgid, + &priv->clientPid) < 0) + goto error; + + VIR_DEBUG("New client pid %llu uid %llu", + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + + if (!privileged) { + if (geteuid() != clientuid) { + virReportError(VIR_ERR_OPERATION_DENIED, + _("Disallowing client %llu with uid %llu"), + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + goto error; + } + } else { + if (clientuid != 0) { + virReportError(VIR_ERR_OPERATION_DENIED, + _("Disallowing client %llu with uid %llu"), + (unsigned long long)priv->clientPid, + (unsigned long long)clientuid); + goto error; + } + } + + return priv; + +error: + virMutexDestroy(&priv->lock); + VIR_FREE(priv); + return NULL; +} + + +static void +virLockDaemonUsage(const char *argv0, bool privileged) +{ + fprintf(stderr, + _("\n" + "Usage:\n" + " %s [options]\n" + "\n" + "Options:\n" + " -v | --verbose Verbose messages.\n" + " -d | --daemon Run as a daemon & write PID file.\n" + " -t | --timeout <secs> Exit after timeout period.\n" + " -f | --config <file> Configuration file.\n" + " | --version Display version information.\n" + " -p | --pid-file <file> Change name of PID file.\n" + "\n" + "libvirt lock management daemon:\n"), argv0); + + if (privileged) { + fprintf(stderr, + _("\n" + " Default paths:\n" + "\n" + " Configuration file (unless overridden by -f):\n" + " %s/libvirt/virtlockd.conf\n" + "\n" + " Sockets:\n" + " %s/run/libvirt/virtlockd-sock\n" + "\n" + " PID file (unless overridden by -p):\n" + " %s/run/virtlockd.pid\n" + "\n"), + SYSCONFDIR, + LOCALSTATEDIR, + LOCALSTATEDIR); + } else { + fprintf(stderr, + "%s", _("\n" + " Default paths:\n" + "\n" + " Configuration file (unless overridden by -f):\n" + " $XDG_CONFIG_HOME/libvirt/virtlockd.conf\n" + "\n" + " Sockets:\n" + " $XDG_RUNTIME_DIR/libvirt/virtlockd-sock\n" + "\n" + " PID file:\n" + " $XDG_RUNTIME_DIR/libvirt/virtlockd.pid\n" + "\n"));
just a small nit, if you'd move "%s" to the previous line, this and the previous blocks would be perfectly aligned.
+ } +} + +enum { + OPT_VERSION = 129 +}; + +#define MAX_LISTEN 5 +int main(int argc, char **argv) { + char *remote_config_file = NULL; + int statuswrite = -1; + int ret = 1; + int verbose = 0; + int godaemon = 0; + int timeout = 0;
timeout seems dummy to me. ACK if you either remove it or (more likely) implement it.
On Wed, Dec 12, 2012 at 07:14:20PM +0100, Michal Privoznik wrote:
On 11.12.2012 21:41, Daniel P. Berrange wrote:
+static int +virLockDaemonForkIntoBackground(const char *argv0) +{ + int statuspipe[2]; + if (pipe(statuspipe) < 0) + return -1; + + pid_t pid = fork(); + switch (pid) { + case 0: + { + int stdinfd = -1; + int stdoutfd = -1; + int nextpid; + + VIR_FORCE_CLOSE(statuspipe[0]); + + if ((stdinfd = open("/dev/null", O_RDONLY)) < 0) + goto cleanup; + if ((stdoutfd = open("/dev/null", O_WRONLY)) < 0) + goto cleanup; + if (dup2(stdinfd, STDIN_FILENO) != STDIN_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDOUT_FILENO) != STDOUT_FILENO) + goto cleanup; + if (dup2(stdoutfd, STDERR_FILENO) != STDERR_FILENO) + goto cleanup; + if (VIR_CLOSE(stdinfd) < 0) + goto cleanup; + if (VIR_CLOSE(stdoutfd) < 0) + goto cleanup; + + if (setsid() < 0) + goto cleanup; + + nextpid = fork(); + switch (nextpid) { + case 0: + return statuspipe[1]; + case -1: + return -1; + default: + _exit(0); + } + + cleanup: + VIR_FORCE_CLOSE(stdoutfd); + VIR_FORCE_CLOSE(stdinfd); + return -1; + + } + + case -1: + return -1; + + default: + { + int got, exitstatus = 0; + int ret; + char status; + + VIR_FORCE_CLOSE(statuspipe[1]); + + /* We wait to make sure the first child forked successfully */ + if ((got = waitpid(pid, &exitstatus, 0)) < 0 || + got != pid || + exitstatus != 0) { + return -1; + } + + /* Now block until the second child initializes successfully */ + again: + ret = read(statuspipe[0], &status, 1); + if (ret == -1 && errno == EINTR) + goto again; + + if (ret == 1 && status != 0) { + fprintf(stderr, + _("%s: error: %s. Check /var/log/messages or run without " + "--daemon for more info.\n"), argv0, + virDaemonErrTypeToString(status)); + } + _exit(ret == 1 && status == 0 ? 0 : 1); + } + } +}
This is basically a copy-paste of daemonForkIntoBackground(). I wonder if we can use (modified) version of it instead of this.
Yep, it would be nice to share more code between the daemons in the future, but I didn't want to tackle that right now.
+/* + * Set up the logging environment + * By default if daemonized all errors go to the logfile libvirtd.log, + * but if verbose or error debugging is asked for then also output + * informational and debug messages. Default size if 64 kB. + */ +static int +virLockDaemonSetupLogging(virLockDaemonConfigPtr config, + bool privileged, + bool verbose, + bool godaemon) +{ + virLogReset(); + + /* + * Libvirtd's order of precedence is: + * cmdline > environment > config + * + * In order to achieve this, we must process configuration in + * different order for the log level versus the filters and + * outputs. Because filters and outputs append, we have to look at + * the environment first and then only check the config file if + * there was no result from the environment. The default output is + * then applied only if there was no setting from either of the + * first two. Because we don't have a way to determine if the log + * level has been set, we must process variables in the opposite + * order, each one overriding the previous. + */ + if (config->log_level != 0) + virLogSetDefaultPriority(config->log_level); + + virLogSetFromEnv(); + + virLogSetBufferSize(config->log_buffer_size); + + if (virLogGetNbFilters() == 0) + virLogParseFilters(config->log_filters); + + if (virLogGetNbOutputs() == 0) + virLogParseOutputs(config->log_outputs); + + /* + * If no defined outputs, and either running + * as daemon or not on a tty, then first try + * to direct it to the systemd journal + * (if it exists).... + */ + if (virLogGetNbOutputs() == 0 && + (godaemon || !isatty(STDIN_FILENO))) { + char *tmp; + if (access("/run/systemd/journal/socket", W_OK) >= 0) { + if (virAsprintf(&tmp, "%d:journald", virLogGetDefaultPriority()) < 0) + goto no_memory; + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + } + + /* + * otherwise direct to libvirtd.log when running + * as daemon. Otherwise the default output is stderr. + */ + if (virLogGetNbOutputs() == 0) { + char *tmp = NULL; + + if (godaemon) { + if (privileged) { + if (virAsprintf(&tmp, "%d:file:%s/log/libvirt/virtlockd.log", + virLogGetDefaultPriority(), + LOCALSTATEDIR) == -1) + goto no_memory; + } else { + char *logdir = virGetUserCacheDirectory(); + mode_t old_umask; + + if (!logdir) + goto error; + + old_umask = umask(077); + if (virFileMakePath(logdir) < 0) { + umask(old_umask); + goto error; + } + umask(old_umask); + + if (virAsprintf(&tmp, "%d:file:%s/virtlockd.log", + virLogGetDefaultPriority(), logdir) == -1) { + VIR_FREE(logdir); + goto no_memory; + } + VIR_FREE(logdir); + } + } else { + if (virAsprintf(&tmp, "%d:stderr", virLogGetDefaultPriority()) < 0) + goto no_memory; + } + virLogParseOutputs(tmp); + VIR_FREE(tmp); + } + + /* + * Command line override for --verbose + */ + if ((verbose) && (virLogGetDefaultPriority() > VIR_LOG_INFO)) + virLogSetDefaultPriority(VIR_LOG_INFO); + + return 0; + +no_memory: + virReportOOMError(); +error: + return -1; +}
Same here. This one is even closer to daemonSetupLogging()
Yep, as above.
+#define MAX_LISTEN 5 +int main(int argc, char **argv) { + char *remote_config_file = NULL; + int statuswrite = -1; + int ret = 1; + int verbose = 0; + int godaemon = 0; + int timeout = 0;
timeout seems dummy to me.
Yep, it is not required.
ACK if you either remove it or (more likely) implement it.
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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> The virtlockd daemon will be responsible for managing locks on virtual machines. Communication will be via the standard RPC infrastructure. This provides the XDR protocol definition * src/locking/lock_protocol.x: Wire protocol for virtlockd * src/Makefile.am: Include lock_protocol.[ch] in virtlockd Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 3 ++ src/Makefile.am | 14 ++++++- src/locking/lock_protocol.x | 95 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/locking/lock_protocol.x diff --git a/.gitignore b/.gitignore index 1e3a624..449a1c6 100644 --- a/.gitignore +++ b/.gitignore @@ -108,6 +108,7 @@ /src/libvirt_*helper /src/libvirt_*probes.h /src/libvirt_lxc +/src/locking/lock_protocol.[ch] /src/locking/qemu-sanlock.conf /src/locking/test_libvirt_sanlock.aug /src/lxc/lxc_controller_dispatch.h diff --git a/cfg.mk b/cfg.mk index 95a1d3a..1fe007e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -827,3 +827,6 @@ exclude_file_name_regexp--sc_unmarked_diagnostics = \ ^(docs/apibuild.py|tests/virt-aa-helper-test)$$ exclude_file_name_regexp--sc_size_of_brackets = cfg.mk + +exclude_file_name_regexp--sc_correct_id_types = \ + (^src/locking/lock_protocol.x$$) diff --git a/src/Makefile.am b/src/Makefile.am index 6a66efd..2023f88 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -148,6 +148,15 @@ LOCK_DRIVER_SANLOCK_SOURCES = \ LOCK_DRIVER_SANLOCK_HELPER_SOURCES = \ locking/sanlock_helper.c +LOCK_PROTOCOL_GENERATED = \ + locking/lock_protocol.h \ + locking/lock_protocol.c \ + $(NULL) + +EXTRA_DIST += locking/lock_protocol.x +BUILT_SOURCES += $(LOCK_PROTOCOL_GENERATED) +MAINTAINERCLEANFILES += $(LOCK_PROTOCOL_GENERATED) + LOCK_DAEMON_SOURCES = \ locking/lock_daemon.h \ locking/lock_daemon.c \ @@ -1518,7 +1527,10 @@ EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) if WITH_LIBVIRTD sbin_PROGRAMS = virtlockd -virtlockd_SOURCES = $(LOCK_DAEMON_SOURCES) +virtlockd_SOURCES = \ + $(LOCK_DAEMON_SOURCES) \ + $(LOCK_PROTOCOL_GENERATED) \ + $(NULL) virtlockd_CFLAGS = \ $(AM_CFLAGS) \ $(NULL) diff --git a/src/locking/lock_protocol.x b/src/locking/lock_protocol.x new file mode 100644 index 0000000..5f40f9a --- /dev/null +++ b/src/locking/lock_protocol.x @@ -0,0 +1,95 @@ +/* -*- c -*- + */ + +%#include "internal.h" + +typedef opaque virLockSpaceProtocolUUID[VIR_UUID_BUFLEN]; + +/* Length of long, but not unbounded, strings. + * This is an arbitrary limit designed to stop the decoder from trying + * to allocate unbounded amounts of memory when fed with a bad message. + */ +const VIR_LOCK_SPACE_PROTOCOL_STRING_MAX = 65536; + +/* A long string, which may NOT be NULL. */ +typedef string virLockSpaceProtocolNonNullString<VIR_LOCK_SPACE_PROTOCOL_STRING_MAX>; + +/* A long string, which may be NULL. */ +typedef virLockSpaceProtocolNonNullString *virLockSpaceProtocolString; + +struct virLockSpaceProtocolOwner { + virLockSpaceProtocolUUID uuid; + virLockSpaceProtocolNonNullString name; + unsigned int id; + unsigned int pid; +}; + +struct virLockSpaceProtocolRegisterArgs { + virLockSpaceProtocolOwner owner; + unsigned int flags; +}; + +struct virLockSpaceProtocolRestrictArgs { + unsigned int flags; +}; + +struct virLockSpaceProtocolNewArgs { + virLockSpaceProtocolNonNullString path; + unsigned int flags; +}; + +struct virLockSpaceProtocolCreateResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + unsigned int flags; +}; + +struct virLockSpaceProtocolDeleteResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + unsigned int flags; +}; + +enum virLockSpaceProtocolAcquireResourceFlags { + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED = 1, + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE = 2 +}; + +struct virLockSpaceProtocolAcquireResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + unsigned int flags; +}; + +struct virLockSpaceProtocolReleaseResourceArgs { + virLockSpaceProtocolNonNullString path; + virLockSpaceProtocolNonNullString name; + unsigned int flags; +}; + +struct virLockSpaceProtocolCreateLockSpaceArgs { + virLockSpaceProtocolNonNullString path; +}; + + +/* Define the program number, protocol version and procedure numbers here. */ +const VIR_LOCK_SPACE_PROTOCOL_PROGRAM = 0xEA7BEEF; +const VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION = 1; + +enum virLockSpaceProtocolProcedure { + /* Each function must have a two-word comment. The first word is + * whether remote_generator.pl handles daemon, the second whether + * it handles src/remote. Additional flags can be specified after a + * pipe. + */ + VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER = 1, /* skipgen skipgen */ + VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT = 2, /* skipgen skipgen */ + VIR_LOCK_SPACE_PROTOCOL_PROC_NEW = 3, /* skipgen skipgen */ + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_RESOURCE = 4, /* skipgen skipgen */ + VIR_LOCK_SPACE_PROTOCOL_PROC_DELETE_RESOURCE = 5, /* skipgen skipgen */ + + VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE = 6, /* skipgen skipgen */ + VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE = 7, /* skipgen skipgen */ + + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE = 8 /* skipgen skipgen */ +}; -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon will be responsible for managing locks on virtual machines. Communication will be via the standard RPC infrastructure. This provides the XDR protocol definition
* src/locking/lock_protocol.x: Wire protocol for virtlockd * src/Makefile.am: Include lock_protocol.[ch] in virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 3 ++ src/Makefile.am | 14 ++++++- src/locking/lock_protocol.x | 95 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/locking/lock_protocol.x
ACK Michal
On 12/12/2012 11:14 AM, Michal Privoznik wrote:
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon will be responsible for managing locks on virtual machines. Communication will be via the standard RPC infrastructure. This provides the XDR protocol definition
* src/locking/lock_protocol.x: Wire protocol for virtlockd * src/Makefile.am: Include lock_protocol.[ch] in virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 3 ++ src/Makefile.am | 14 ++++++- src/locking/lock_protocol.x | 95 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/locking/lock_protocol.x
ACK
Should we also tweak src/Makefile.am and add src/lock_protocol-structs to verify that we don't make any ABI changes to the wire protocol? -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 12.12.2012 19:32, Eric Blake wrote:
On 12/12/2012 11:14 AM, Michal Privoznik wrote:
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon will be responsible for managing locks on virtual machines. Communication will be via the standard RPC infrastructure. This provides the XDR protocol definition
* src/locking/lock_protocol.x: Wire protocol for virtlockd * src/Makefile.am: Include lock_protocol.[ch] in virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 3 ++ src/Makefile.am | 14 ++++++- src/locking/lock_protocol.x | 95 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/locking/lock_protocol.x
ACK
Should we also tweak src/Makefile.am and add src/lock_protocol-structs to verify that we don't make any ABI changes to the wire protocol?
We definitely should. However, since this is just an initial implementation so there's nothing to break so far. Moreover, given how close to release we are (and how much users wants this in - see Dan's comment in the cover letter) I think it is not a show stopper for now. However, if he adds it just before pushing that would be great. On the other hand - virlockd is to be shipped within libvirtd package. And it's not to be used by users directly. So the requirements to backward compatibility seems slightly relaxed to me when compared to virsh+libvirtd. But then again - that's not a green light to breaking the compatibility, it's rather the opposite. Michal
On Wed, Dec 12, 2012 at 11:32:18AM -0700, Eric Blake wrote:
On 12/12/2012 11:14 AM, Michal Privoznik wrote:
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon will be responsible for managing locks on virtual machines. Communication will be via the standard RPC infrastructure. This provides the XDR protocol definition
* src/locking/lock_protocol.x: Wire protocol for virtlockd * src/Makefile.am: Include lock_protocol.[ch] in virtlockd
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 3 ++ src/Makefile.am | 14 ++++++- src/locking/lock_protocol.x | 95 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 src/locking/lock_protocol.x
ACK
Should we also tweak src/Makefile.am and add src/lock_protocol-structs to verify that we don't make any ABI changes to the wire protocol?
Yep, but it'll need a little refactoring of the makefile due to the conditional builds, so I'll do that as a later patch Same point about the LXC controller protocol 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> Introduce a lock_daemon_dispatch.c file which implements the server side dispatcher the RPC APIs previously defined in the lock protocol. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + po/POTFILES.in | 1 + src/Makefile.am | 14 ++ src/internal.h | 22 ++ src/locking/lock_daemon.c | 133 +++++++++++- src/locking/lock_daemon.h | 13 ++ src/locking/lock_daemon_dispatch.c | 434 +++++++++++++++++++++++++++++++++++++ src/locking/lock_daemon_dispatch.h | 31 +++ 8 files changed, 647 insertions(+), 2 deletions(-) create mode 100644 src/locking/lock_daemon_dispatch.c create mode 100644 src/locking/lock_daemon_dispatch.h diff --git a/.gitignore b/.gitignore index 449a1c6..6de2308 100644 --- a/.gitignore +++ b/.gitignore @@ -108,6 +108,7 @@ /src/libvirt_*helper /src/libvirt_*probes.h /src/libvirt_lxc +/src/locking/lock_daemon_dispatch_stubs.h /src/locking/lock_protocol.[ch] /src/locking/qemu-sanlock.conf /src/locking/test_libvirt_sanlock.aug diff --git a/po/POTFILES.in b/po/POTFILES.in index 34c688c..c6c72a8 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -50,6 +50,7 @@ src/libvirt.c src/libvirt-qemu.c src/locking/lock_daemon.c src/locking/lock_daemon_config.c +src/locking/lock_daemon_dispatch.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c diff --git a/src/Makefile.am b/src/Makefile.am index 2023f88..f0a9a03 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -157,13 +157,26 @@ EXTRA_DIST += locking/lock_protocol.x BUILT_SOURCES += $(LOCK_PROTOCOL_GENERATED) MAINTAINERCLEANFILES += $(LOCK_PROTOCOL_GENERATED) +LOCK_DAEMON_GENERATED = \ + locking/lock_daemon_dispatch_stubs.h + $(NULL) + +BUILT_SOURCES += $(LOCK_DAEMON_GENERATED) +MAINTAINERCLEANFILES += $(LOCK_DAEMON_GENERATED) + LOCK_DAEMON_SOURCES = \ locking/lock_daemon.h \ locking/lock_daemon.c \ locking/lock_daemon_config.h \ locking/lock_daemon_config.c \ + locking/lock_daemon_dispatch.c \ + locking/lock_daemon_dispatch.h \ $(NULL) +$(srcdir)/locking/lock_daemon_dispatch_stubs.h: locking/lock_protocol.x $(srcdir)/rpc/gendispatch.pl Makefile.am + $(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl -b virLockSpaceProtocol VIR_LOCK_SPACE_PROTOCOL $< > $@ + + NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c \ @@ -1530,6 +1543,7 @@ sbin_PROGRAMS = virtlockd virtlockd_SOURCES = \ $(LOCK_DAEMON_SOURCES) \ $(LOCK_PROTOCOL_GENERATED) \ + $(LOCK_DAEMON_GENERATED) \ $(NULL) virtlockd_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/src/internal.h b/src/internal.h index d69bd14..8d96660 100644 --- a/src/internal.h +++ b/src/internal.h @@ -236,6 +236,28 @@ } \ } while (0) +/** + * virCheckFlagsGoto: + * @supported: an OR'ed set of supported flags + * @label: label to jump to on error + * + * To avoid memory leaks this macro has to be used before any non-trivial + * code which could possibly allocate some memory. + * + * Returns nothing. Jumps to a label if unsupported flags were + * passed to it. + */ +# define virCheckFlagsGoto(supported, label) \ + do { \ + unsigned long __unsuppflags = flags & ~(supported); \ + if (__unsuppflags) { \ + virReportInvalidArg(flags, \ + _("unsupported flags (0x%lx) in function %s"), \ + __unsuppflags, __FUNCTION__); \ + goto label; \ + } \ + } while (0) + # define virCheckNonNullArgReturn(argname, retval) \ do { \ if (argname == NULL) { \ diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f821d2e..3c007f1 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -36,6 +36,7 @@ #include "util.h" #include "virfile.h" #include "virpidfile.h" +#include "virprocess.h" #include "virterror_internal.h" #include "logging.h" #include "memory.h" @@ -44,13 +45,20 @@ #include "virrandom.h" #include "virhash.h" +#include "locking/lock_daemon_dispatch.h" +#include "locking/lock_protocol.h" + #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_LOCKING +#define VIR_LOCK_DAEMON_NUM_LOCKSPACES 3 + struct _virLockDaemon { virMutex lock; virNetServerPtr srv; + virHashTablePtr lockspaces; + virLockSpacePtr defaultLockspace; }; virLockDaemonPtr lockDaemon = NULL; @@ -94,11 +102,19 @@ virLockDaemonFree(virLockDaemonPtr lockd) return; virObjectUnref(lockd->srv); + virHashFree(lockd->lockspaces); + virLockSpaceFree(lockd->defaultLockspace); VIR_FREE(lockd); } +static void virLockDaemonLockSpaceDataFree(void *data, + const void *key ATTRIBUTE_UNUSED) +{ + virLockSpaceFree(data); +} + static virLockDaemonPtr virLockDaemonNew(bool privileged) { @@ -125,6 +141,13 @@ virLockDaemonNew(bool privileged) (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; + if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, + virLockDaemonLockSpaceDataFree))) + goto error; + + if (!(lockd->defaultLockspace = virLockSpaceNew(NULL))) + goto error; + return lockd; error: @@ -133,6 +156,31 @@ error: } +int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, + const char *path, + virLockSpacePtr lockspace) +{ + int ret; + virMutexLock(&lockd->lock); + ret = virHashAddEntry(lockd->lockspaces, path, lockspace); + virMutexUnlock(&lockd->lock); + return ret; +} + +virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, + const char *path) +{ + virLockSpacePtr lockspace; + virMutexLock(&lockd->lock); + if (path && STRNEQ(path, "")) + lockspace = virHashLookup(lockd->lockspaces, path); + else + lockspace = lockd->defaultLockspace; + virMutexUnlock(&lockd->lock); + return lockspace; +} + + static int virLockDaemonForkIntoBackground(const char *argv0) { @@ -466,6 +514,30 @@ virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path) } +struct virLockDaemonClientReleaseData { + virLockDaemonClientPtr client; + bool hadSomeLeases; + bool gotError; +}; + +static void +virLockDaemonClientReleaseLockspace(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virLockSpacePtr lockspace = payload; + struct virLockDaemonClientReleaseData *data = opaque; + int rc; + + rc = virLockSpaceReleaseResourcesForOwner(lockspace, + data->client->clientPid); + if (rc > 0) + data->hadSomeLeases = true; + else if (rc < 0) + data->gotError = true; +} + + static void virLockDaemonClientFree(void *opaque) { @@ -474,9 +546,52 @@ virLockDaemonClientFree(void *opaque) if (!priv) return; - VIR_DEBUG("priv=%p client=%lld", + VIR_DEBUG("priv=%p client=%lld owner=%lld", priv, - (unsigned long long)priv->clientPid); + (unsigned long long)priv->clientPid, + (unsigned long long)priv->ownerPid); + + /* If client & owner match, this is the lock holder */ + if (priv->clientPid == priv->ownerPid) { + size_t i; + struct virLockDaemonClientReleaseData data = { + .client = priv, .hadSomeLeases = false, .gotError = false + }; + + /* Release all locks associated with this + * owner in all lockspaces */ + virMutexLock(&lockDaemon->lock); + virHashForEach(lockDaemon->lockspaces, + virLockDaemonClientReleaseLockspace, + &data); + virLockDaemonClientReleaseLockspace(lockDaemon->defaultLockspace, + "", + &data); + virMutexUnlock(&lockDaemon->lock); + + /* If the client had some active leases when it + * closed the connection, we must kill it off + * to make sure it doesn't do nasty stuff */ + if (data.gotError || data.hadSomeLeases) { + for (i = 0 ; i < 15 ; i++) { + int signum; + if (i == 0) + signum = SIGTERM; + else if (i == 8) + signum = SIGKILL; + else + signum = 0; + if (virProcessKill(priv->clientPid, signum) < 0) { + if (errno == ESRCH) + break; + + VIR_WARN("Failed to kill off pid %lld", + (unsigned long long)priv->clientPid); + } + usleep(200 * 1000); + } + } + } virMutexDestroy(&priv->lock); VIR_FREE(priv); @@ -598,6 +713,7 @@ enum { #define MAX_LISTEN 5 int main(int argc, char **argv) { + virNetServerProgramPtr lockProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -807,6 +923,18 @@ int main(int argc, char **argv) { goto cleanup; } + if (!(lockProgram = virNetServerProgramNew(VIR_LOCK_SPACE_PROTOCOL_PROGRAM, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION, + virLockSpaceProtocolProcs, + virLockSpaceProtocolNProcs))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(lockDaemon->srv, lockProgram) < 0) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virLockDaemonErrorHandler); @@ -830,6 +958,7 @@ int main(int argc, char **argv) { ret = 0; cleanup: + virObjectUnref(lockProgram); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { if (ret != 0) { diff --git a/src/locking/lock_daemon.h b/src/locking/lock_daemon.h index 7bc8c2e..619f8f2 100644 --- a/src/locking/lock_daemon.h +++ b/src/locking/lock_daemon.h @@ -34,10 +34,23 @@ typedef virLockDaemonClient *virLockDaemonClientPtr; struct _virLockDaemonClient { virMutex lock; + bool restricted; + + pid_t ownerPid; + char *ownerName; + unsigned char ownerUUID[VIR_UUID_BUFLEN]; + unsigned int ownerId; pid_t clientPid; }; extern virLockDaemonPtr lockDaemon; +int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, + const char *path, + virLockSpacePtr lockspace); + +virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, + const char *path); + #endif /* __VIR_LOCK_DAEMON_H__ */ diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c new file mode 100644 index 0000000..f4797a8 --- /dev/null +++ b/src/locking/lock_daemon_dispatch.c @@ -0,0 +1,434 @@ +/* + * lock_daemon_dispatch.c: lock management daemon dispatch + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "rpc/virnetserver.h" +#include "rpc/virnetserverclient.h" +#include "util.h" +#include "logging.h" + +#include "lock_daemon.h" +#include "lock_protocol.h" +#include "lock_daemon_dispatch_stubs.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_RPC + +static int +virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolAcquireResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + unsigned int newFlags; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(VIR_LOCK_SPACE_ACQUIRE_SHARED | + VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + newFlags = 0; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE; + + if (virLockSpaceAcquireResource(lockspace, + args->name, + priv->ownerPid, + newFlags) < 0) { + VIR_ERROR("FAILED"); + goto cleanup; + } + + VIR_ERROR("OK"); + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchCreateResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolCreateResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + if (virLockSpaceCreateResource(lockspace, args->name) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchDeleteResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolDeleteResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + if (virLockSpaceDeleteResource(lockspace, args->name) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchNew(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolNewArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!args->path || STREQ(args->path, "")) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("the default lockspace already exists")); + goto cleanup; + } + + if ((lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s already exists"), + args->path); + goto cleanup; + } + virResetLastError(); + + lockspace = virLockSpaceNew(args->path); + virLockDaemonAddLockSpace(lockDaemon, args->path, lockspace); + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchRegister(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolRegisterArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have already been registered")); + goto cleanup; + } + + if (!(priv->ownerName = strdup(args->owner.name))) { + virReportOOMError(); + goto cleanup; + } + memcpy(priv->ownerUUID, args->owner.uuid, VIR_UUID_BUFLEN); + priv->ownerId = args->owner.id; + priv->ownerPid = args->owner.pid; + VIR_DEBUG("ownerName=%s ownerId=%d ownerPid=%lld", + priv->ownerName, priv->ownerId, (unsigned long long)priv->ownerPid); + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchReleaseResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolReleaseResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + if (virLockSpaceReleaseResource(lockspace, + args->name, + priv->ownerPid) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchRestrict(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolRestrictArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(0, cleanup); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + priv->restricted = true; + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + + +static int +virLockSpaceProtocolDispatchCreateLockSpace(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolCreateLockSpaceArgs *args) +{ + int rv = -1; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + + virMutexLock(&priv->lock); + + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if ((lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Lockspace for path %s already exists"), + args->path); + goto cleanup; + } + + if (!(lockspace = virLockSpaceNew(args->path))) + goto cleanup; + + if (virLockDaemonAddLockSpace(lockDaemon, args->path, lockspace) < 0) { + virLockSpaceFree(lockspace); + goto cleanup; + } + + rv = 0; + +cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} diff --git a/src/locking/lock_daemon_dispatch.h b/src/locking/lock_daemon_dispatch.h new file mode 100644 index 0000000..a193a58 --- /dev/null +++ b/src/locking/lock_daemon_dispatch.h @@ -0,0 +1,31 @@ +/* + * lock_daemon_dispatch.h: lock management daemon dispatch + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#ifndef __VIR_LOCK_DAEMON_DISPATCH_H__ +# define __VIR_LOCK_DAEMON_DISPATCH_H__ + +# include "rpc/virnetserverprogram.h" + +extern virNetServerProgramProc virLockSpaceProtocolProcs[]; +extern size_t virLockSpaceProtocolNProcs; + +#endif /* __VIR_LOCK_DAEMON_DISPATCH_H__ */ -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
Introduce a lock_daemon_dispatch.c file which implements the server side dispatcher the RPC APIs previously defined in the lock protocol.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + po/POTFILES.in | 1 + src/Makefile.am | 14 ++ src/internal.h | 22 ++ src/locking/lock_daemon.c | 133 +++++++++++- src/locking/lock_daemon.h | 13 ++ src/locking/lock_daemon_dispatch.c | 434 +++++++++++++++++++++++++++++++++++++ src/locking/lock_daemon_dispatch.h | 31 +++ 8 files changed, 647 insertions(+), 2 deletions(-) create mode 100644 src/locking/lock_daemon_dispatch.c create mode 100644 src/locking/lock_daemon_dispatch.h
diff --git a/.gitignore b/.gitignore index 449a1c6..6de2308 100644 --- a/.gitignore +++ b/.gitignore @@ -108,6 +108,7 @@ /src/libvirt_*helper /src/libvirt_*probes.h /src/libvirt_lxc +/src/locking/lock_daemon_dispatch_stubs.h /src/locking/lock_protocol.[ch] /src/locking/qemu-sanlock.conf /src/locking/test_libvirt_sanlock.aug diff --git a/po/POTFILES.in b/po/POTFILES.in index 34c688c..c6c72a8 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -50,6 +50,7 @@ src/libvirt.c src/libvirt-qemu.c src/locking/lock_daemon.c src/locking/lock_daemon_config.c +src/locking/lock_daemon_dispatch.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c diff --git a/src/Makefile.am b/src/Makefile.am index 2023f88..f0a9a03 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -157,13 +157,26 @@ EXTRA_DIST += locking/lock_protocol.x BUILT_SOURCES += $(LOCK_PROTOCOL_GENERATED) MAINTAINERCLEANFILES += $(LOCK_PROTOCOL_GENERATED)
+LOCK_DAEMON_GENERATED = \ + locking/lock_daemon_dispatch_stubs.h + $(NULL) + +BUILT_SOURCES += $(LOCK_DAEMON_GENERATED) +MAINTAINERCLEANFILES += $(LOCK_DAEMON_GENERATED) + LOCK_DAEMON_SOURCES = \ locking/lock_daemon.h \ locking/lock_daemon.c \ locking/lock_daemon_config.h \ locking/lock_daemon_config.c \ + locking/lock_daemon_dispatch.c \ + locking/lock_daemon_dispatch.h \ $(NULL)
+$(srcdir)/locking/lock_daemon_dispatch_stubs.h: locking/lock_protocol.x $(srcdir)/rpc/gendispatch.pl Makefile.am + $(AM_V_GEN)perl -w $(srcdir)/rpc/gendispatch.pl -b virLockSpaceProtocol VIR_LOCK_SPACE_PROTOCOL $< > $@ + + NETDEV_CONF_SOURCES = \ conf/netdev_bandwidth_conf.h conf/netdev_bandwidth_conf.c \ conf/netdev_vport_profile_conf.h conf/netdev_vport_profile_conf.c \ @@ -1530,6 +1543,7 @@ sbin_PROGRAMS = virtlockd virtlockd_SOURCES = \ $(LOCK_DAEMON_SOURCES) \ $(LOCK_PROTOCOL_GENERATED) \ + $(LOCK_DAEMON_GENERATED) \ $(NULL) virtlockd_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/src/internal.h b/src/internal.h index d69bd14..8d96660 100644 --- a/src/internal.h +++ b/src/internal.h @@ -236,6 +236,28 @@ } \ } while (0)
+/** + * virCheckFlagsGoto: + * @supported: an OR'ed set of supported flags + * @label: label to jump to on error + * + * To avoid memory leaks this macro has to be used before any non-trivial + * code which could possibly allocate some memory. + * + * Returns nothing. Jumps to a label if unsupported flags were + * passed to it. + */ +# define virCheckFlagsGoto(supported, label) \ + do { \ + unsigned long __unsuppflags = flags & ~(supported); \ + if (__unsuppflags) { \ + virReportInvalidArg(flags, \ + _("unsupported flags (0x%lx) in function %s"), \ + __unsuppflags, __FUNCTION__); \ + goto label; \ + } \ + } while (0) + # define virCheckNonNullArgReturn(argname, retval) \ do { \ if (argname == NULL) { \ diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index f821d2e..3c007f1 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -36,6 +36,7 @@ #include "util.h" #include "virfile.h" #include "virpidfile.h" +#include "virprocess.h" #include "virterror_internal.h" #include "logging.h" #include "memory.h" @@ -44,13 +45,20 @@ #include "virrandom.h" #include "virhash.h"
+#include "locking/lock_daemon_dispatch.h" +#include "locking/lock_protocol.h" + #include "configmake.h"
#define VIR_FROM_THIS VIR_FROM_LOCKING
+#define VIR_LOCK_DAEMON_NUM_LOCKSPACES 3 + struct _virLockDaemon { virMutex lock; virNetServerPtr srv; + virHashTablePtr lockspaces; + virLockSpacePtr defaultLockspace; };
virLockDaemonPtr lockDaemon = NULL; @@ -94,11 +102,19 @@ virLockDaemonFree(virLockDaemonPtr lockd) return;
virObjectUnref(lockd->srv); + virHashFree(lockd->lockspaces); + virLockSpaceFree(lockd->defaultLockspace);
VIR_FREE(lockd); }
+static void virLockDaemonLockSpaceDataFree(void *data, + const void *key ATTRIBUTE_UNUSED) +{ + virLockSpaceFree(data); +} + static virLockDaemonPtr virLockDaemonNew(bool privileged) { @@ -125,6 +141,13 @@ virLockDaemonNew(bool privileged) (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error;
+ if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, + virLockDaemonLockSpaceDataFree))) + goto error; + + if (!(lockd->defaultLockspace = virLockSpaceNew(NULL))) + goto error; + return lockd;
error: @@ -133,6 +156,31 @@ error: }
+int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, + const char *path, + virLockSpacePtr lockspace) +{ + int ret; + virMutexLock(&lockd->lock); + ret = virHashAddEntry(lockd->lockspaces, path, lockspace); + virMutexUnlock(&lockd->lock); + return ret; +} + +virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, + const char *path) +{ + virLockSpacePtr lockspace; + virMutexLock(&lockd->lock); + if (path && STRNEQ(path, "")) + lockspace = virHashLookup(lockd->lockspaces, path); + else + lockspace = lockd->defaultLockspace; + virMutexUnlock(&lockd->lock); + return lockspace; +} + + static int virLockDaemonForkIntoBackground(const char *argv0) { @@ -466,6 +514,30 @@ virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path) }
+struct virLockDaemonClientReleaseData { + virLockDaemonClientPtr client; + bool hadSomeLeases; + bool gotError; +}; + +static void +virLockDaemonClientReleaseLockspace(void *payload, + const void *name ATTRIBUTE_UNUSED, + void *opaque) +{ + virLockSpacePtr lockspace = payload; + struct virLockDaemonClientReleaseData *data = opaque; + int rc; + + rc = virLockSpaceReleaseResourcesForOwner(lockspace, + data->client->clientPid); + if (rc > 0) + data->hadSomeLeases = true; + else if (rc < 0) + data->gotError = true; +} + + static void virLockDaemonClientFree(void *opaque) { @@ -474,9 +546,52 @@ virLockDaemonClientFree(void *opaque) if (!priv) return;
- VIR_DEBUG("priv=%p client=%lld", + VIR_DEBUG("priv=%p client=%lld owner=%lld", priv, - (unsigned long long)priv->clientPid); + (unsigned long long)priv->clientPid, + (unsigned long long)priv->ownerPid); + + /* If client & owner match, this is the lock holder */ + if (priv->clientPid == priv->ownerPid) { + size_t i; + struct virLockDaemonClientReleaseData data = { + .client = priv, .hadSomeLeases = false, .gotError = false + }; + + /* Release all locks associated with this + * owner in all lockspaces */ + virMutexLock(&lockDaemon->lock); + virHashForEach(lockDaemon->lockspaces, + virLockDaemonClientReleaseLockspace, + &data); + virLockDaemonClientReleaseLockspace(lockDaemon->defaultLockspace, + "", + &data); + virMutexUnlock(&lockDaemon->lock); + + /* If the client had some active leases when it + * closed the connection, we must kill it off + * to make sure it doesn't do nasty stuff */ + if (data.gotError || data.hadSomeLeases) { + for (i = 0 ; i < 15 ; i++) { + int signum; + if (i == 0) + signum = SIGTERM; + else if (i == 8) + signum = SIGKILL; + else + signum = 0; + if (virProcessKill(priv->clientPid, signum) < 0) { + if (errno == ESRCH) + break; + + VIR_WARN("Failed to kill off pid %lld", + (unsigned long long)priv->clientPid); + } + usleep(200 * 1000); + } + } + }
virMutexDestroy(&priv->lock); VIR_FREE(priv); @@ -598,6 +713,7 @@ enum {
#define MAX_LISTEN 5 int main(int argc, char **argv) { + virNetServerProgramPtr lockProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -807,6 +923,18 @@ int main(int argc, char **argv) { goto cleanup; }
+ if (!(lockProgram = virNetServerProgramNew(VIR_LOCK_SPACE_PROTOCOL_PROGRAM, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION, + virLockSpaceProtocolProcs, + virLockSpaceProtocolNProcs))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(lockDaemon->srv, lockProgram) < 0) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virLockDaemonErrorHandler);
@@ -830,6 +958,7 @@ int main(int argc, char **argv) { ret = 0;
cleanup: + virObjectUnref(lockProgram); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { if (ret != 0) { diff --git a/src/locking/lock_daemon.h b/src/locking/lock_daemon.h index 7bc8c2e..619f8f2 100644 --- a/src/locking/lock_daemon.h +++ b/src/locking/lock_daemon.h @@ -34,10 +34,23 @@ typedef virLockDaemonClient *virLockDaemonClientPtr;
struct _virLockDaemonClient { virMutex lock; + bool restricted; + + pid_t ownerPid; + char *ownerName; + unsigned char ownerUUID[VIR_UUID_BUFLEN]; + unsigned int ownerId;
pid_t clientPid; };
extern virLockDaemonPtr lockDaemon;
+int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, + const char *path, + virLockSpacePtr lockspace); + +virLockSpacePtr virLockDaemonFindLockSpace(virLockDaemonPtr lockd, + const char *path); + #endif /* __VIR_LOCK_DAEMON_H__ */ diff --git a/src/locking/lock_daemon_dispatch.c b/src/locking/lock_daemon_dispatch.c new file mode 100644 index 0000000..f4797a8 --- /dev/null +++ b/src/locking/lock_daemon_dispatch.c @@ -0,0 +1,434 @@ +/* + * lock_daemon_dispatch.c: lock management daemon dispatch + * + * Copyright (C) 2006-2012 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "rpc/virnetserver.h" +#include "rpc/virnetserverclient.h" +#include "util.h" +#include "logging.h" + +#include "lock_daemon.h" +#include "lock_protocol.h" +#include "lock_daemon_dispatch_stubs.h" +#include "virterror_internal.h" + +#define VIR_FROM_THIS VIR_FROM_RPC + +static int +virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolAcquireResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + unsigned int newFlags; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(VIR_LOCK_SPACE_ACQUIRE_SHARED | + VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, cleanup);
se here you are effectively checking 'flags' to VIR_LOCK_SPACE_ACQUIRE_SHARED and VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE ...
+ + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + newFlags = 0; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE;
while here you test them to a different set of flags. I understand you want to do translation from protocol representation of flags to API one since they may (but shouldn't) diverge. You need to update the virCheckFlagsGoto() then.
+ + if (virLockSpaceAcquireResource(lockspace, + args->name, + priv->ownerPid, + newFlags) < 0) { + VIR_ERROR("FAILED"); + goto cleanup; + } + + VIR_ERROR("OK");
indentation Otherwise I haven't spotted anything wrong. ACK Michal
On Wed, Dec 12, 2012 at 07:14:15PM +0100, Michal Privoznik wrote:
On 11.12.2012 21:41, Daniel P. Berrange wrote:
+ +static int +virLockSpaceProtocolDispatchAcquireResource(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + virLockSpaceProtocolAcquireResourceArgs *args) +{ + int rv = -1; + unsigned int flags = args->flags; + virLockDaemonClientPtr priv = + virNetServerClientGetPrivateData(client); + virLockSpacePtr lockspace; + unsigned int newFlags; + + virMutexLock(&priv->lock); + + virCheckFlagsGoto(VIR_LOCK_SPACE_ACQUIRE_SHARED | + VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, cleanup);
se here you are effectively checking 'flags' to VIR_LOCK_SPACE_ACQUIRE_SHARED and VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE ...
Yep, obviously the current code is wrong. I'll fix.
+ + if (priv->restricted) { + virReportError(VIR_ERR_OPERATION_DENIED, "%s", + _("lock manager connection has been restricted")); + goto cleanup; + } + + if (!priv->ownerPid) { + virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("lock owner details have not been registered")); + goto cleanup; + } + + if (!(lockspace = virLockDaemonFindLockSpace(lockDaemon, args->path))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Lockspace for path %s does not exist"), + args->path); + goto cleanup; + } + + newFlags = 0; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_SHARED; + if (flags & VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE) + newFlags |= VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE;
while here you test them to a different set of flags. I understand you want to do translation from protocol representation of flags to API one since they may (but shouldn't) diverge. You need to update the virCheckFlagsGoto() then.
+ + if (virLockSpaceAcquireResource(lockspace, + args->name, + priv->ownerPid, + newFlags) < 0) { + VIR_ERROR("FAILED"); + goto cleanup; + } + + VIR_ERROR("OK");
indentation
Those two VIR_ERROR lines will just be killed - they were random debug. 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 :|
From: "Daniel P. Berrange" <berrange@redhat.com> This enhancement virtlockd so that it can receive a pre-opened UNIX domain socket from systemd at launch time, and adds the systemd service/socket unit files * daemon/libvirtd.service.in: Require virtlockd to be running * libvirt.spec.in: Add virtlockd systemd files * src/Makefile.am: Install systemd files * src/locking/lock_daemon.c: Support socket activation * src/locking/virtlockd.service.in, src/locking/virtlockd.socket.in: systemd unit files * src/rpc/virnetserverservice.c, src/rpc/virnetserverservice.h: Add virNetServerServiceNewFD() method * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketNewListenFD method Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 6 ++++ src/Makefile.am | 51 ++++++++++++++++++++++++++-- src/locking/lock_daemon.c | 73 ++++++++++++++++++++++++++++++++++++++-- src/locking/virtlockd.service.in | 13 +++++++ src/locking/virtlockd.socket.in | 8 +++++ 5 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 src/locking/virtlockd.service.in create mode 100644 src/locking/virtlockd.socket.in diff --git a/libvirt.spec.in b/libvirt.spec.in index e12fca4..aebece3 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1499,6 +1499,7 @@ done %else if [ $1 -eq 1 ] ; then # Initial installation + /bin/systemctl enable virtlockd.socket >/dev/null 2>&1 || : /bin/systemctl enable libvirtd.service >/dev/null 2>&1 || : fi %endif @@ -1526,8 +1527,10 @@ fi %else if [ $1 -eq 0 ] ; then # Package removal, not upgrade + /bin/systemctl --no-reload disable virtlockd.socket > /dev/null 2>&1 || : /bin/systemctl --no-reload disable libvirtd.service > /dev/null 2>&1 || : /bin/systemctl stop libvirtd.service > /dev/null 2>&1 || : + /bin/systemctl stop virtlockd.service > /dev/null 2>&1 || : fi %endif %else @@ -1545,6 +1548,7 @@ fi /bin/systemctl daemon-reload >/dev/null 2>&1 || : if [ $1 -ge 1 ] ; then # Package upgrade, not uninstall + /bin/systemctl try-restart virtlockd.service >/dev/null 2>&1 || : /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : fi %endif @@ -1659,6 +1663,8 @@ fi %if %{with_systemd} %{_unitdir}/libvirtd.service +%{_unitdir}/virtlockd.service +%{_unitdir}/virtlockd.socket %else %{_sysconfdir}/rc.d/init.d/libvirtd %{_sysconfdir}/rc.d/init.d/virtlockd diff --git a/src/Makefile.am b/src/Makefile.am index f0a9a03..8dbb6a5 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1611,6 +1611,53 @@ virtlockd.init: locking/virtlockd.init.in $(top_builddir)/config.status +EXTRA_DIST += locking/virtlockd.service.in locking/virtlockd.socket.in + +if WITH_LIBVIRTD +if LIBVIRT_INIT_SCRIPT_SYSTEMD + +SYSTEMD_UNIT_DIR = /lib/systemd/system + +BUILT_SOURCES += virtlockd.service virtlockd.socket + +install-systemd: virtlockd.init install-sysconfig + mkdir -p $(DESTDIR)$(SYSTEMD_UNIT_DIR) + $(INSTALL_SCRIPT) virtlockd.service \ + $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ + $(INSTALL_SCRIPT) virtlockd.socket \ + $(DESTDIR)$(SYSTEMD_UNIT_DIR)/ + +uninstall-systemd: uninstall-sysconfig + rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/virtlockd.service + rm -f $(DESTDIR)$(SYSTEMD_UNIT_DIR)/virtlockd.socket +else +install-systemd: +uninstall-systemd: +endif +else +install-systemd: +uninstall-systemd: +endif + +virtlockd.service: locking/virtlockd.service.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e "s!::localstatedir::!$(localstatedir)!g" \ + -e "s!::sbindir::!$(sbindir)!g" \ + -e "s!::sysconfdir::!$(sysconfdir)!g" \ + < $< > $@-t && \ + chmod a+x $@-t && \ + mv $@-t $@ + +virtlockd.socket: locking/virtlockd.socket.in $(top_builddir)/config.status + $(AM_V_GEN)sed \ + -e "s!::localstatedir::!$(localstatedir)!g" \ + -e "s!::sbindir::!$(sbindir)!g" \ + -e "s!::sysconfdir::!$(sysconfdir)!g" \ + < $< > $@-t && \ + chmod a+x $@-t && \ + mv $@-t $@ + + if HAVE_SANLOCK lockdriverdir = $(libdir)/libvirt/lock-driver lockdriver_LTLIBRARIES = sanlock.la @@ -1848,7 +1895,7 @@ endif endif EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) -install-data-local: install-init +install-data-local: install-init install-systemd if WITH_LIBVIRTD $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" @@ -1901,7 +1948,7 @@ if WITH_NETWORK $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart/default.xml endif -uninstall-local:: uninstall-init +uninstall-local:: uninstall-init uninstall-systemd if WITH_LIBVIRTD rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" ||: rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" ||: diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 3c007f1..7727deb 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -496,8 +496,69 @@ virLockDaemonSetupSignals(virNetServerPtr srv) return 0; } + static int -virLockDaemonSetupNetworking(virNetServerPtr srv, const char *sock_path) +virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv) +{ + virNetServerServicePtr svc; + const char *pidstr; + const char *fdstr; + unsigned long long procid; + unsigned int nfds; + + if (!(pidstr = getenv("LISTEN_PID"))) { + VIR_DEBUG("No LISTEN_FDS from systemd"); + return 0; + } + + if (virStrToLong_ull(pidstr, NULL, 10, &procid) < 0) { + VIR_DEBUG("Malformed LISTEN_PID from systemd %s", pidstr); + return 0; + } + + if ((pid_t)procid != getpid()) { + VIR_DEBUG("LISTEN_PID %s is not for us %llu", + pidstr, (unsigned long long)getpid()); + return 0; + } + + if (!(fdstr = getenv("LISTEN_FDS"))) { + VIR_DEBUG("No LISTEN_FDS from systemd"); + return 0; + } + + if (virStrToLong_ui(fdstr, NULL, 10, &nfds) < 0) { + VIR_DEBUG("Malformed LISTEN_FDS from systemd %s", fdstr); + return 0; + } + + if (nfds > 1) { + VIR_DEBUG("Too many (%d) file descriptors from systemd", + nfds); + nfds = 1; + } + + unsetenv("LISTEN_PID"); + unsetenv("LISTEN_FDS"); + + if (nfds == 0) + return 0; + + /* Systemd passes FDs, starting immediately after stderr, + * so the first FD we'll get is '3'. */ + if (!(svc = virNetServerServiceNewFD(3, 0, false, 1, NULL))) + return -1; + + if (virNetServerAddService(srv, svc, NULL) < 0) { + virObjectUnref(svc); + return -1; + } + return 1; +} + + +static int +virLockDaemonSetupNetworkingNative(virNetServerPtr srv, const char *sock_path) { virNetServerServicePtr svc; @@ -728,6 +789,7 @@ int main(int argc, char **argv) { mode_t old_umask; bool privileged = false; virLockDaemonConfigPtr config = NULL; + int rv; struct option opts[] = { { "verbose", no_argument, &verbose, 1}, @@ -913,7 +975,14 @@ int main(int argc, char **argv) { goto cleanup; } - if (virLockDaemonSetupNetworking(lockDaemon->srv, sock_file) < 0) { + if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { + ret = VIR_LOCK_DAEMON_ERR_NETWORK; + goto cleanup; + } + + /* Only do this, if systemd did not pass a FD */ + if (rv == 0 && + virLockDaemonSetupNetworkingNative(lockDaemon->srv, sock_file) < 0) { ret = VIR_LOCK_DAEMON_ERR_NETWORK; goto cleanup; } diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in new file mode 100644 index 0000000..a9f9f93 --- /dev/null +++ b/src/locking/virtlockd.service.in @@ -0,0 +1,13 @@ +[Unit] +Description=Virtual machine lock manager +Requires=virtlockd.socket +After=syslog.target + +[Service] +EnvironmentFile=-/etc/sysconfig/virtlockd +ExecStart=@sbindir@/virtlockd +ExecReload=/bin/kill -HUP $MAINPID +# Loosing the locks is a really bad thing that will +# cause the machine to be fenced (rebooted), so make +# sure we discourage OOM killer +OOMScoreAdjust=-900 diff --git a/src/locking/virtlockd.socket.in b/src/locking/virtlockd.socket.in new file mode 100644 index 0000000..0589a29 --- /dev/null +++ b/src/locking/virtlockd.socket.in @@ -0,0 +1,8 @@ +[Unit] +Description=Virtual machine lock manager socket + +[Socket] +ListenStream=/var/run/libvirt/virtlockd/virtlockd.sock + +[Install] +WantedBy=multi-user.target -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This enhancement virtlockd so that it can receive a pre-opened UNIX domain socket from systemd at launch time, and adds the systemd service/socket unit files
* daemon/libvirtd.service.in: Require virtlockd to be running * libvirt.spec.in: Add virtlockd systemd files * src/Makefile.am: Install systemd files * src/locking/lock_daemon.c: Support socket activation * src/locking/virtlockd.service.in, src/locking/virtlockd.socket.in: systemd unit files * src/rpc/virnetserverservice.c, src/rpc/virnetserverservice.h: Add virNetServerServiceNewFD() method * src/rpc/virnetsocket.c, src/rpc/virnetsocket.h: Add virNetSocketNewListenFD method
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 6 ++++ src/Makefile.am | 51 ++++++++++++++++++++++++++-- src/locking/lock_daemon.c | 73 ++++++++++++++++++++++++++++++++++++++-- src/locking/virtlockd.service.in | 13 +++++++ src/locking/virtlockd.socket.in | 8 +++++ 5 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 src/locking/virtlockd.service.in create mode 100644 src/locking/virtlockd.socket.in
I don't consider myself as systemd expert (nor fan), but this all seems sane. ACK unless somebody else objects. Michal
From: "Daniel P. Berrange" <berrange@redhat.com> The virtlockd daemon maintains file locks on behalf of libvirtd and any VMs it is running. These file locks must be held for as long as any VM is running. If virtlockd itself ever quits, then it is expected that a node would be fenced/rebooted. Thus to allow for software upgrads on live systemd, virtlockd needs the ability to re-exec() itself. Upon receipt of SIGUSR1, virtlockd will save its current live state out to a file /var/run/virtlockd-restart-exec.json It then re-exec()'s itself with exactly the same argv as it originally had, and loads the state file, reconstructing any objects as appropriate. The state file contains information about all locks held and all network services and clients currently active. An example state document is { "server": { "min_workers": 1, "max_workers": 20, "priority_workers": 0, "max_clients": 20, "keepaliveInterval": 4294967295, "keepaliveCount": 0, "keepaliveRequired": false, "services": [ { "auth": 0, "readonly": false, "nrequests_client_max": 1, "socks": [ { "fd": 6, "errfd": -1, "pid": 0, "isClient": false } ] } ], "clients": [ { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 9, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1722, "ownerId": 6, "ownerName": "f18x86_64", "ownerUUID": "97586ba9-df27-9459-c806-f016c8bbd224" } }, { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 10, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1784, "ownerId": 7, "ownerName": "f16x86_64", "ownerUUID": "7b8e5e42-b875-61e9-b981-91ad8fa46979" } } ] }, "defaultLockspace": { "resources": [ { "name": "/var/lib/libvirt/images/f16x86_64.raw", "path": "/var/lib/libvirt/images/f16x86_64.raw", "fd": 14, "lockHeld": true, "flags": 0, "owners": [ 1784 ] }, { "name": "/var/lib/libvirt/images/shared.img", "path": "/var/lib/libvirt/images/shared.img", "fd": 12, "lockHeld": true, "flags": 1, "owners": [ 1722, 1784 ] }, { "name": "/var/lib/libvirt/images/f18x86_64.img", "path": "/var/lib/libvirt/images/f18x86_64.img", "fd": 11, "lockHeld": true, "flags": 0, "owners": [ 1722 ] } ] }, "lockspaces": [ ], "magic": "30199" } Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 5 +- src/locking/lock_daemon.c | 417 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 409 insertions(+), 13 deletions(-) diff --git a/libvirt.spec.in b/libvirt.spec.in index aebece3..a2c2ee9 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1548,7 +1548,10 @@ fi /bin/systemctl daemon-reload >/dev/null 2>&1 || : if [ $1 -ge 1 ] ; then # Package upgrade, not uninstall - /bin/systemctl try-restart virtlockd.service >/dev/null 2>&1 || : + /bin/systemctl status virtlockd.service >/dev/null 2>&1 + if [ $? = 1 ] ; then + /bin/systemctl kill --signal=USR1 virtlockd.service >/dev/null 2>&1 || : + fi /bin/systemctl try-restart libvirtd.service >/dev/null 2>&1 || : fi %endif diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 7727deb..f24e4ab 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -44,6 +44,7 @@ #include "rpc/virnetserver.h" #include "virrandom.h" #include "virhash.h" +#include "uuid.h" #include "locking/lock_daemon_dispatch.h" #include "locking/lock_protocol.h" @@ -63,6 +64,8 @@ struct _virLockDaemon { virLockDaemonPtr lockDaemon = NULL; +static bool execRestart = false; + enum { VIR_LOCK_DAEMON_ERR_NONE = 0, VIR_LOCK_DAEMON_ERR_PIDFILE, @@ -95,6 +98,14 @@ virLockDaemonClientNew(virNetServerClientPtr client, static void virLockDaemonClientFree(void *opaque); +static void * +virLockDaemonClientNewPostExecRestart(virNetServerClientPtr client, + virJSONValuePtr object, + void *opaque); +static virJSONValuePtr +virLockDaemonClientPreExecRestart(virNetServerClientPtr client, + void *opaque); + static void virLockDaemonFree(virLockDaemonPtr lockd) { @@ -136,7 +147,7 @@ virLockDaemonNew(bool privileged) -1, 0, false, NULL, virLockDaemonClientNew, - NULL, + virLockDaemonClientPreExecRestart, virLockDaemonClientFree, (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; @@ -156,6 +167,90 @@ error: } +static virLockDaemonPtr +virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) +{ + virLockDaemonPtr lockd; + virJSONValuePtr child; + virJSONValuePtr lockspaces; + size_t i; + int n; + + if (VIR_ALLOC(lockd) < 0) { + virReportOOMError(); + return NULL; + } + + if (virMutexInit(&lockd->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize mutex")); + VIR_FREE(lockd); + return NULL; + } + + if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, + virLockDaemonLockSpaceDataFree))) + goto error; + + if (!(child = virJSONValueObjectGet(object, "defaultLockspace"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing defaultLockspace data from JSON file")); + goto error; + } + + if (!(lockd->defaultLockspace = + virLockSpaceNewPostExecRestart(child))) + goto error; + + if (!(lockspaces = virJSONValueObjectGet(object, "lockspaces"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing lockspaces data from JSON file")); + goto error; + } + + if ((n = virJSONValueArraySize(lockspaces)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed lockspaces data from JSON file")); + goto error; + } + + for (i = 0 ; i < n ; i++) { + virLockSpacePtr lockspace; + + child = virJSONValueArrayGet(lockspaces, i); + + if (!(lockspace = virLockSpaceNewPostExecRestart(child))) + goto error; + + if (virHashAddEntry(lockd->lockspaces, + virLockSpaceGetDirectory(lockspace), + lockspace) < 0) { + virLockSpaceFree(lockspace); + } + } + + if (!(child = virJSONValueObjectGet(object, "server"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing server data from JSON file")); + goto error; + } + + if (!(lockd->srv = virNetServerNewPostExecRestart(child, + virLockDaemonClientNew, + virLockDaemonClientNewPostExecRestart, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + goto error; + + return lockd; + +error: + virLockDaemonFree(lockd); + return NULL; +} + + int virLockDaemonAddLockSpace(virLockDaemonPtr lockd, const char *path, virLockSpacePtr lockspace) @@ -484,6 +579,15 @@ virLockDaemonShutdownHandler(virNetServerPtr srv, virNetServerQuit(srv); } +static void +virLockDaemonExecRestartHandler(virNetServerPtr srv, + siginfo_t *sig ATTRIBUTE_UNUSED, + void *opaque ATTRIBUTE_UNUSED) +{ + execRestart = true; + virNetServerQuit(srv); +} + static int virLockDaemonSetupSignals(virNetServerPtr srv) { @@ -493,6 +597,8 @@ virLockDaemonSetupSignals(virNetServerPtr srv) return -1; if (virNetServerAddSignalHandler(srv, SIGTERM, virLockDaemonShutdownHandler, NULL) < 0) return -1; + if (virNetServerAddSignalHandler(srv, SIGUSR1, virLockDaemonExecRestartHandler, NULL) < 0) + return -1; return 0; } @@ -506,6 +612,8 @@ virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv) unsigned long long procid; unsigned int nfds; + VIR_DEBUG("Setting up networking from systemd"); + if (!(pidstr = getenv("LISTEN_PID"))) { VIR_DEBUG("No LISTEN_FDS from systemd"); return 0; @@ -716,6 +824,277 @@ error: } +static void * +virLockDaemonClientNewPostExecRestart(virNetServerClientPtr client, + virJSONValuePtr object, + void *opaque) +{ + virLockDaemonClientPtr priv = virLockDaemonClientNew(client, opaque); + unsigned int ownerPid; + const char *ownerUUID; + const char *ownerName; + + if (!priv) + return NULL; + + if (virJSONValueObjectGetBoolean(object, "restricted", &priv->restricted) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing restricted data in JSON document")); + goto error; + } + if (virJSONValueObjectGetNumberUint(object, "ownerPid", &ownerPid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ownerPid data in JSON document")); + goto error; + } + priv->ownerPid = (pid_t)ownerPid; + if (virJSONValueObjectGetNumberUint(object, "ownerId", &priv->ownerId) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ownerId data in JSON document")); + goto error; + } + if (!(ownerName = virJSONValueObjectGetString(object, "ownerName"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ownerName data in JSON document")); + goto error; + } + if (!(priv->ownerName = strdup(ownerName))) { + virReportOOMError(); + goto error; + } + if (!(ownerUUID = virJSONValueObjectGetString(object, "ownerUUID"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ownerUUID data in JSON document")); + goto error; + } + if (virUUIDParse(ownerUUID, priv->ownerUUID) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ownerUUID data in JSON document")); + goto error; + } + return priv; + +error: + virLockDaemonClientFree(priv); + return NULL; +} + + +static virJSONValuePtr +virLockDaemonClientPreExecRestart(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + virLockDaemonClientPtr priv = opaque; + virJSONValuePtr object = virJSONValueNewObject(); + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (!object) + return NULL; + + if (virJSONValueObjectAppendBoolean(object, "restricted", priv->restricted) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set restricted data in JSON document")); + goto error; + } + if (virJSONValueObjectAppendNumberUint(object, "ownerPid", priv->ownerPid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set ownerPid data in JSON document")); + goto error; + } + if (virJSONValueObjectAppendNumberUint(object, "ownerId", priv->ownerId) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set ownerId data in JSON document")); + goto error; + } + if (virJSONValueObjectAppendString(object, "ownerName", priv->ownerName) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set ownerName data in JSON document")); + goto error; + } + virUUIDFormat(priv->ownerUUID, uuidstr); + if (virJSONValueObjectAppendString(object, "ownerUUID", uuidstr) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set ownerUUID data in JSON document")); + goto error; + } + + return object; + +error: + virJSONValueFree(object); + return NULL; +} + + +#define VIR_LOCK_DAEMON_RESTART_EXEC_FILE LOCALSTATEDIR "/run/virtlockd-restart-exec.json" + +static char * +virLockDaemonGetExecRestartMagic(void) +{ + char *ret; + + if (virAsprintf(&ret, "%lld", + (long long int)getpid()) < 0) { + virReportOOMError(); + return NULL; + } + + return ret; +} + + +static int +virLockDaemonPostExecRestart(bool privileged) +{ + const char *gotmagic; + char *wantmagic = NULL; + int ret = -1; + char *state = NULL; + virJSONValuePtr object = NULL; + + VIR_DEBUG("Running post-restart exec"); + + if (!virFileExists(VIR_LOCK_DAEMON_RESTART_EXEC_FILE)) { + VIR_DEBUG("No restart file %s present", + VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + ret = 0; + goto cleanup; + } + + if (virFileReadAll(VIR_LOCK_DAEMON_RESTART_EXEC_FILE, + 1024 * 1024 * 10, /* 10 MB */ + &state) < 0) + goto cleanup; + + VIR_DEBUG("Loading state %s", state); + + if (!(object = virJSONValueFromString(state))) + goto cleanup; + + gotmagic = virJSONValueObjectGetString(object, "magic"); + if (!gotmagic) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing magic data in JSON document")); + goto cleanup; + } + + if (!(wantmagic = virLockDaemonGetExecRestartMagic())) + goto cleanup; + + if (STRNEQ(gotmagic, wantmagic)) { + VIR_WARN("Found restart exec file with old magic %s vs wanted %s", + gotmagic, wantmagic); + ret = 0; + goto cleanup; + } + + if (!(lockDaemon = virLockDaemonNewPostExecRestart(object, privileged))) + goto cleanup; + + ret = 1; + +cleanup: + unlink(VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + VIR_FREE(wantmagic); + VIR_FREE(state); + virJSONValueFree(object); + return ret; +} + + +static int +virLockDaemonPreExecRestart(virNetServerPtr srv, + char **argv) +{ + virJSONValuePtr child; + char *state = NULL; + int ret = -1; + virJSONValuePtr object; + char *magic; + virHashKeyValuePairPtr pairs = NULL, tmp; + virJSONValuePtr lockspaces; + + VIR_DEBUG("Running pre-restart exec"); + + if (!(object = virJSONValueNewObject())) + goto cleanup; + + if (!(child = virNetServerPreExecRestart(srv))) + goto cleanup; + + if (virJSONValueObjectAppend(object, "server", child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + if (!(child = virLockSpacePreExecRestart(lockDaemon->defaultLockspace))) + goto cleanup; + + if (virJSONValueObjectAppend(object, "defaultLockspace", child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + if (!(lockspaces = virJSONValueNewArray())) + goto cleanup; + if (virJSONValueObjectAppend(object, "lockspaces", lockspaces) < 0) { + virJSONValueFree(lockspaces); + goto cleanup; + } + + + tmp = pairs = virHashGetItems(lockDaemon->lockspaces, NULL); + while (tmp && tmp->key) { + virLockSpacePtr lockspace = (virLockSpacePtr)tmp->value; + + if (!(child = virLockSpacePreExecRestart(lockspace))) + goto cleanup; + + if (virJSONValueArrayAppend(lockspaces, child) < 0) { + virJSONValueFree(child); + goto cleanup; + } + + tmp++; + } + + if (!(magic = virLockDaemonGetExecRestartMagic())) + goto cleanup; + + if (virJSONValueObjectAppendString(object, "magic", magic) < 0) { + VIR_FREE(magic); + goto cleanup; + } + + if (!(state = virJSONValueToString(object, true))) + goto cleanup; + + VIR_DEBUG("Saving state %s", state); + + if (virFileWriteStr(VIR_LOCK_DAEMON_RESTART_EXEC_FILE, + state, 0700) < 0) { + virReportSystemError(errno, + _("Unable to save state file %s"), + VIR_LOCK_DAEMON_RESTART_EXEC_FILE); + goto cleanup; + } + + if (execv(argv[0], argv) < 0) { + virReportSystemError(errno, "%s", + _("Unable to restart self")); + goto cleanup; + } + + abort(); /* This should be impossible to reach */ + +cleanup: + VIR_FREE(pairs); + VIR_FREE(state); + virJSONValueFree(object); + return ret; +} + + static void virLockDaemonUsage(const char *argv0, bool privileged) { @@ -970,21 +1349,30 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(lockDaemon = virLockDaemonNew(privileged))) { + if ((rv = virLockDaemonPostExecRestart(privileged)) < 0) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; } - if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { - ret = VIR_LOCK_DAEMON_ERR_NETWORK; - goto cleanup; - } + /* rv == 1, means we setup everything from saved state, + * so we only setup stuff from scratch if rv == 0 */ + if (rv == 0) { + if (!(lockDaemon = virLockDaemonNew(privileged))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } - /* Only do this, if systemd did not pass a FD */ - if (rv == 0 && - virLockDaemonSetupNetworkingNative(lockDaemon->srv, sock_file) < 0) { - ret = VIR_LOCK_DAEMON_ERR_NETWORK; - goto cleanup; + if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { + ret = VIR_LOCK_DAEMON_ERR_NETWORK; + goto cleanup; + } + + /* Only do this, if systemd did not pass a FD */ + if (rv == 0 && + virLockDaemonSetupNetworkingNative(lockDaemon->srv, sock_file) < 0) { + ret = VIR_LOCK_DAEMON_ERR_NETWORK; + goto cleanup; + } } if ((virLockDaemonSetupSignals(lockDaemon->srv)) < 0) { @@ -1024,7 +1412,12 @@ int main(int argc, char **argv) { virNetServerUpdateServices(lockDaemon->srv, true); virNetServerRun(lockDaemon->srv); - ret = 0; + + if (execRestart && + virLockDaemonPreExecRestart(lockDaemon->srv, argv) < 0) + ret = -1; + else + ret = 0; cleanup: virObjectUnref(lockProgram); -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The virtlockd daemon maintains file locks on behalf of libvirtd and any VMs it is running. These file locks must be held for as long as any VM is running. If virtlockd itself ever quits, then it is expected that a node would be fenced/rebooted. Thus to allow for software upgrads on live systemd, virtlockd needs the ability to re-exec() itself.
Upon receipt of SIGUSR1, virtlockd will save its current live state out to a file /var/run/virtlockd-restart-exec.json It then re-exec()'s itself with exactly the same argv as it originally had, and loads the state file, reconstructing any objects as appropriate.
The state file contains information about all locks held and all network services and clients currently active. An example state document is
{ "server": { "min_workers": 1, "max_workers": 20, "priority_workers": 0, "max_clients": 20, "keepaliveInterval": 4294967295, "keepaliveCount": 0, "keepaliveRequired": false, "services": [ { "auth": 0, "readonly": false, "nrequests_client_max": 1, "socks": [ { "fd": 6, "errfd": -1, "pid": 0, "isClient": false } ] } ], "clients": [ { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 9, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1722, "ownerId": 6, "ownerName": "f18x86_64", "ownerUUID": "97586ba9-df27-9459-c806-f016c8bbd224" } }, { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 10, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1784, "ownerId": 7, "ownerName": "f16x86_64", "ownerUUID": "7b8e5e42-b875-61e9-b981-91ad8fa46979" } } ] }, "defaultLockspace": { "resources": [ { "name": "/var/lib/libvirt/images/f16x86_64.raw", "path": "/var/lib/libvirt/images/f16x86_64.raw", "fd": 14, "lockHeld": true, "flags": 0, "owners": [ 1784 ] }, { "name": "/var/lib/libvirt/images/shared.img", "path": "/var/lib/libvirt/images/shared.img", "fd": 12, "lockHeld": true, "flags": 1, "owners": [ 1722, 1784 ] }, { "name": "/var/lib/libvirt/images/f18x86_64.img", "path": "/var/lib/libvirt/images/f18x86_64.img", "fd": 11, "lockHeld": true, "flags": 0, "owners": [ 1722 ] } ] }, "lockspaces": [
], "magic": "30199" }
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- libvirt.spec.in | 5 +- src/locking/lock_daemon.c | 417 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 409 insertions(+), 13 deletions(-)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> This adds a 'lockd' lock driver which is just a client which talks to the lockd daemon to perform all locking. This will be the default lock driver for any hypervisor which needs one. * src/Makefile.am: Add lockd.so plugin * src/locking/lock_driver_lockd.c: Lockd driver impl Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + po/POTFILES.in | 1 + run.in | 2 + src/Makefile.am | 54 ++- src/locking/libvirt_lockd.aug | 32 ++ src/locking/lock_driver_lockd.c | 658 ++++++++++++++++++++++++++++++++++ src/locking/lockd.conf | 18 + src/locking/test_libvirt_lockd.aug.in | 6 + 8 files changed, 766 insertions(+), 6 deletions(-) create mode 100644 src/locking/libvirt_lockd.aug create mode 100644 src/locking/lock_driver_lockd.c create mode 100644 src/locking/lockd.conf create mode 100644 src/locking/test_libvirt_lockd.aug.in diff --git a/.gitignore b/.gitignore index 6de2308..8023b76 100644 --- a/.gitignore +++ b/.gitignore @@ -110,6 +110,7 @@ /src/libvirt_lxc /src/locking/lock_daemon_dispatch_stubs.h /src/locking/lock_protocol.[ch] +/src/locking/qemu-lockd.conf /src/locking/qemu-sanlock.conf /src/locking/test_libvirt_sanlock.aug /src/lxc/lxc_controller_dispatch.h diff --git a/po/POTFILES.in b/po/POTFILES.in index c6c72a8..53bf7f0 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -51,6 +51,7 @@ src/libvirt-qemu.c src/locking/lock_daemon.c src/locking/lock_daemon_config.c src/locking/lock_daemon_dispatch.c +src/locking/lock_driver_lockd.c src/locking/lock_driver_sanlock.c src/locking/lock_manager.c src/locking/sanlock_helper.c diff --git a/run.in b/run.in index f7d30ca..845eef5 100644 --- a/run.in +++ b/run.in @@ -53,6 +53,8 @@ fi export LD_LIBRARY_PATH export LIBVIRT_DRIVER_DIR="$b/src/.libs" +export LIBVIRT_LOCK_MANAGER_PLUGIN_DIR="$b/src/.libs" +export VIRTLOCKD_PATH="$b/src/virtlockd" export LIBVIRTD_PATH="$b/daemon/libvirtd" # For Python. diff --git a/src/Makefile.am b/src/Makefile.am index 8dbb6a5..5808653 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -164,6 +164,10 @@ LOCK_DAEMON_GENERATED = \ BUILT_SOURCES += $(LOCK_DAEMON_GENERATED) MAINTAINERCLEANFILES += $(LOCK_DAEMON_GENERATED) +LOCK_DRIVER_LOCKD_SOURCES = \ + locking/lock_driver_lockd.c \ + $(NULL) + LOCK_DAEMON_SOURCES = \ locking/lock_daemon.h \ locking/lock_daemon.c \ @@ -1298,9 +1302,14 @@ EXTRA_DIST += \ check-local: check-augeas -.PHONY: check-augeas check-augeas-qemu check-augeas-lxc check-augeas-sanlock +.PHONY: check-augeas \ + check-augeas-qemu \ + check-augeas-lxc \ + check-augeas-sanlock \ + check-augeas-lockd \ + $(NULL) -check-augeas: check-augeas-qemu check-augeas-lxc check-augeas-sanlock +check-augeas: check-augeas-qemu check-augeas-lxc check-augeas-sanlock check-augeas-lockd AUG_GENTEST = $(PERL) $(top_srcdir)/build-aux/augeas-gentest.pl EXTRA_DIST += $(top_srcdir)/build-aux/augeas-gentest.pl @@ -1344,6 +1353,15 @@ else check-augeas-sanlock: endif +test_libvirt_lockd.aug: locking/test_libvirt_lockd.aug.in \ + locking/qemu-lockd.conf $(AUG_GENTEST) + $(AM_V_GEN)$(AUG_GENTEST) locking/qemu-lockd.conf $< $@ + +check-augeas-lockd: test_libvirt_lockd.aug + $(AM_V_GEN)if test -x '$(AUGPARSE)'; then \ + '$(AUGPARSE)' -I $(srcdir)/locking test_libvirt_lockd.aug; \ + fi + # # Build our version script. This is composed of three parts: # @@ -1537,7 +1555,32 @@ libvirt_qemu_la_CFLAGS = $(AM_CFLAGS) libvirt_qemu_la_LIBADD = libvirt.la $(CYGWIN_EXTRA_LIBADD) EXTRA_DIST += $(LIBVIRT_QEMU_SYMBOL_FILE) +lockdriverdir = $(libdir)/libvirt/lock-driver +lockdriver_LTLIBRARIES = + if WITH_LIBVIRTD +lockdriver_LTLIBRARIES += lockd.la +lockd_la_SOURCES = \ + $(LOCK_DRIVER_LOCKD_SOURCES) \ + $(LOCK_PROTOCOL_GENERATED) \ + $(NULL) +lockd_la_CFLAGS = $(AM_CFLAGS) +lockd_la_LDFLAGS = -module -avoid-version +lockd_la_LIBADD = ../gnulib/lib/libgnu.la libvirt-net-rpc.la libvirt-net-rpc-client.la +if WITH_DTRACE_PROBES +lockd_la_LIBADD += libvirt_probes.lo +endif +if WITH_QEMU +nodist_conf_DATA = locking/qemu-lockd.conf +BUILT_SOURCES += locking/qemu-lockd.conf +DISTCLEANFILES += locking/qemu-lockd.conf +endif + +locking/%-lockd.conf: $(srcdir)/locking/lockd.conf + $(AM_V_GEN)$(MKDIR_P) locking ; \ + cp $< $@ + + sbin_PROGRAMS = virtlockd virtlockd_SOURCES = \ @@ -1565,7 +1608,8 @@ virtlockd_LDADD += libvirt_probes.lo endif else -EXTRA_DIST += $(LOCK_DAEMON_SOURCES) +EXTRA_DIST += $(LOCK_DAEMON_SOURCES) \ + $(LOCK_DRIVER_LOCKD_SOURCES) endif EXTRA_DIST += locking/virtlockd.sysconf @@ -1659,9 +1703,7 @@ virtlockd.socket: locking/virtlockd.socket.in $(top_builddir)/config.status if HAVE_SANLOCK -lockdriverdir = $(libdir)/libvirt/lock-driver -lockdriver_LTLIBRARIES = sanlock.la - +lockdriver_LTLIBRARIES += sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) sanlock_la_LDFLAGS = -module -avoid-version diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug new file mode 100644 index 0000000..4649644 --- /dev/null +++ b/src/locking/libvirt_lockd.aug @@ -0,0 +1,32 @@ +(* /etc/libvirt/qemu-lockd.conf *) + +module Libvirt_lockd = + autoload xfm + + let eol = del /[ \t]*\n/ "\n" + let value_sep = del /[ \t]*=[ \t]*/ " = " + let indent = del /[ \t]*/ "" + + let str_val = del /\"/ "\"" . store /[^\"]*/ . del /\"/ "\"" + let bool_val = store /0|1/ + let int_val = store /[0-9]+/ + + let str_entry (kw:string) = [ key kw . value_sep . str_val ] + let bool_entry (kw:string) = [ key kw . value_sep . bool_val ] + let int_entry (kw:string) = [ key kw . value_sep . int_val ] + + + (* Each enty in the config is one of the following three ... *) + let entry = bool_entry "auto_disk_leases" + | bool_entry "require_lease_for_disks" + let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] + let empty = [ label "#empty" . eol ] + + let record = indent . entry . eol + + let lns = ( record | comment | empty ) * + + let filter = incl "/etc/libvirt/qemu-lockd.conf" + . Util.stdexcl + + let xfm = transform lns filter diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c new file mode 100644 index 0000000..089b284 --- /dev/null +++ b/src/locking/lock_driver_lockd.c @@ -0,0 +1,658 @@ +/* + * lock_driver_lockd.c: A lock driver which locks nothing + * + * Copyright (C) 2010-2011 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; If not, see + * <http://www.gnu.org/licenses/>. + * + */ + +#include <config.h> + +#include "lock_driver.h" +#include "conf.h" +#include "memory.h" +#include "logging.h" +#include "uuid.h" +#include "util.h" +#include "virfile.h" +#include "virterror_internal.h" +#include "rpc/virnetclient.h" +#include "lock_protocol.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_LOCKING + +#define virLockError(code, ...) \ + virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \ + __FUNCTION__, __LINE__, __VA_ARGS__) + +typedef struct _virLockManagerLockDaemonPrivate virLockManagerLockDaemonPrivate; +typedef virLockManagerLockDaemonPrivate *virLockManagerLockDaemonPrivatePtr; + +typedef struct _virLockManagerLockDaemonResource virLockManagerLockDaemonResource; +typedef virLockManagerLockDaemonResource *virLockManagerLockDaemonResourcePtr; + +typedef struct _virLockManagerLockDaemonDriver virLockManagerLockDaemonDriver; +typedef virLockManagerLockDaemonDriver *virLockManagerLockDaemonDriverPtr; + +struct _virLockManagerLockDaemonResource { + char *lockspace; + char *name; + unsigned int flags; +}; + +struct _virLockManagerLockDaemonPrivate { + unsigned char uuid[VIR_UUID_BUFLEN]; + char *name; + int id; + pid_t pid; + + size_t nresources; + virLockManagerLockDaemonResourcePtr resources; + + bool hasRWDisks; +}; + + +struct _virLockManagerLockDaemonDriver { + bool autoDiskLease; + bool requireLeaseForDisks; +}; + +static virLockManagerLockDaemonDriverPtr driver = NULL; + +#define VIRTLOCKD_PATH SBINDIR "/virtlockd" + +static const char * +virLockManagerLockDaemonFindDaemon(void) +{ + const char *customDaemon = getenv("VIRTLOCKD_PATH"); + + if (customDaemon) + return customDaemon; + + if (virFileIsExecutable(VIRTLOCKD_PATH)) + return VIRTLOCKD_PATH; + + return NULL; +} + +static int virLockManagerLockDaemonLoadConfig(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; + } + + if (!(conf = virConfReadFile(configFile, 0))) + return -1; + +#define CHECK_TYPE(name,typ) if (p && p->type != (typ)) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + "%s: %s: expected type " #typ, \ + configFile, (name)); \ + virConfFree(conf); \ + return -1; \ + } + + p = virConfGetValue(conf, "auto_disk_leases"); + CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG); + if (p) driver->autoDiskLease = p->l; + + p = virConfGetValue(conf, "require_lease_for_disks"); + CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG); + if (p) + driver->requireLeaseForDisks = p->l; + else + driver->requireLeaseForDisks = !driver->autoDiskLease; + + virConfFree(conf); + return 0; +} + + +static char *virLockManagerLockDaemonPath(bool privileged) +{ + char *path; + if (privileged) { + if (!(path = strdup(LOCALSTATEDIR "/run/libvirt/virtlockd-sock"))) { + virReportOOMError(); + return NULL; + } + } else { + char *rundir = NULL; + + if (!(rundir = virGetUserRuntimeDirectory())) + return NULL; + + if (virAsprintf(&path, "%s/virtlockd-sock", rundir) < 0) { + VIR_FREE(rundir); + virReportOOMError(); + return NULL; + } + + } + return path; +} + + +static int +virLockManagerLockDaemonConnectionRegister(virLockManagerPtr lock, + virNetClientPtr client, + virNetClientProgramPtr program, + int *counter) +{ + virLockManagerLockDaemonPrivatePtr priv = lock->privateData; + virLockSpaceProtocolRegisterArgs args; + int rv = -1; + + memset(&args, 0, sizeof(args)); + + args.flags = 0; + memcpy(args.owner.uuid, priv->uuid, VIR_UUID_BUFLEN); + args.owner.name = priv->name; + args.owner.id = priv->id; + args.owner.pid = priv->pid; + + if (virNetClientProgramCall(program, + client, + (*counter)++, + VIR_LOCK_SPACE_PROTOCOL_PROC_REGISTER, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolRegisterArgs, (char*)&args, + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; + + rv = 0; + +cleanup: + return rv; +} + + +static int +virLockManagerLockDaemonConnectionRestrict(virLockManagerPtr lock ATTRIBUTE_UNUSED, + virNetClientPtr client, + virNetClientProgramPtr program, + int *counter) +{ + virLockSpaceProtocolRestrictArgs args; + int rv = -1; + + memset(&args, 0, sizeof(args)); + + args.flags = 0; + + if (virNetClientProgramCall(program, + client, + (*counter)++, + VIR_LOCK_SPACE_PROTOCOL_PROC_RESTRICT, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolRestrictArgs, (char*)&args, + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; + + rv = 0; + +cleanup: + return rv; +} + + +static virNetClientPtr virLockManagerLockDaemonConnectionNew(bool privileged, + virNetClientProgramPtr *prog) +{ + virNetClientPtr client = NULL; + char *lockdpath; + const char *daemonPath = NULL; + + *prog = NULL; + + if (!(lockdpath = virLockManagerLockDaemonPath(privileged))) + goto error; + + if (!privileged) + daemonPath = virLockManagerLockDaemonFindDaemon(); + + if (!(client = virNetClientNewUNIX(lockdpath, + daemonPath != NULL, + daemonPath))) + goto error; + + if (!(*prog = virNetClientProgramNew(VIR_LOCK_SPACE_PROTOCOL_PROGRAM, + VIR_LOCK_SPACE_PROTOCOL_PROGRAM_VERSION, + NULL, + 0, + NULL))) + goto error; + + if (virNetClientAddProgram(client, *prog) < 0) + goto error; + + VIR_FREE(lockdpath); + + return client; + +error: + VIR_FREE(lockdpath); + virNetClientClose(client); + virObjectUnref(client); + virObjectUnref(*prog); + return NULL; +} + + +static virNetClientPtr +virLockManagerLockDaemonConnect(virLockManagerPtr lock, + virNetClientProgramPtr *program, + int *counter) +{ + virNetClientPtr client; + + if (!(client = virLockManagerLockDaemonConnectionNew(getuid() == 0, program))) + return NULL; + + if (virLockManagerLockDaemonConnectionRegister(lock, + client, + *program, + counter) < 0) + goto error; + + return client; + +error: + virNetClientClose(client); + virObjectUnref(client); + return NULL; +} + + +static int virLockManagerLockDaemonDeinit(void); + +static int virLockManagerLockDaemonInit(unsigned int version, + const char *configFile, + unsigned int flags) +{ + VIR_DEBUG("version=%u configFile=%s flags=%x", version, NULLSTR(configFile), flags); + + virCheckFlags(0, -1); + + if (driver) + return 0; + + if (VIR_ALLOC(driver) < 0) { + virReportOOMError(); + return -1; + } + + driver->requireLeaseForDisks = true; + driver->autoDiskLease = true; + + if (virLockManagerLockDaemonLoadConfig(configFile) < 0) + goto error; + + return 0; + +error: + virLockManagerLockDaemonDeinit(); + return -1; +} + +static int virLockManagerLockDaemonDeinit(void) +{ + if (!driver) + return 0; + + VIR_FREE(driver); + + return 0; +} + +static void virLockManagerLockDaemonFree(virLockManagerPtr lock) +{ + virLockManagerLockDaemonPrivatePtr priv = lock->privateData; + size_t i; + + if (!priv) + return; + + lock->privateData = NULL; + + for (i = 0 ; i < priv->nresources ; i++) { + VIR_FREE(priv->resources[i].lockspace); + VIR_FREE(priv->resources[i].name); + } + VIR_FREE(priv->resources); + + VIR_FREE(priv->name); + + VIR_FREE(priv); +} + + +static int virLockManagerLockDaemonNew(virLockManagerPtr lock, + unsigned int type, + size_t nparams, + virLockManagerParamPtr params, + unsigned int flags) +{ + virLockManagerLockDaemonPrivatePtr priv; + size_t i; + + virCheckFlags(VIR_LOCK_MANAGER_USES_STATE, -1); + + if (VIR_ALLOC(priv) < 0) { + virReportOOMError(); + return -1; + } + lock->privateData = priv; + + switch (type) { + case VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN: + for (i = 0 ; i < nparams ; i++) { + if (STREQ(params[i].key, "uuid")) { + memcpy(priv->uuid, params[i].value.uuid, VIR_UUID_BUFLEN); + } else if (STREQ(params[i].key, "name")) { + if (!(priv->name = strdup(params[i].value.str))) { + virReportOOMError(); + return -1; + } + } else if (STREQ(params[i].key, "id")) { + priv->id = params[i].value.i; + } else if (STREQ(params[i].key, "pid")) { + priv->pid = params[i].value.i; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected parameter %s for object"), + params[i].key); + } + } + if (priv->id == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing ID parameter for domain object")); + return -1; + } + if (priv->pid == 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing PID parameter for domain object")); + return -1; + } + if (!priv->name) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing name parameter for domain object")); + return -1; + } + if (!virUUIDIsValid(priv->uuid)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing UUID parameter for domain object")); + return -1; + } + break; + + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown lock manager object type %d"), + type); + return -1; + } + + return 0; +} + + +static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, + unsigned int type, + const char *name, + size_t nparams, + virLockManagerParamPtr params, + unsigned int flags) +{ + virLockManagerLockDaemonPrivatePtr priv = lock->privateData; + char *newName; + char *newLockspace = NULL; + + virCheckFlags(VIR_LOCK_MANAGER_RESOURCE_READONLY | + VIR_LOCK_MANAGER_RESOURCE_SHARED, -1); + + if (flags & VIR_LOCK_MANAGER_RESOURCE_READONLY) + return 0; + + switch (type) { + case VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK: + if (params || nparams) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unexpected parameters for disk resource")); + return -1; + } + if (!driver->autoDiskLease) { + if (!(flags & (VIR_LOCK_MANAGER_RESOURCE_SHARED | + VIR_LOCK_MANAGER_RESOURCE_READONLY))) + priv->hasRWDisks = true; + return 0; + } + if (!(newLockspace = strdup(""))) { + virReportOOMError(); + return -1; + } + break; + case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE: { + size_t i; + char *path = NULL; + char *lockspace = NULL; + for (i = 0 ; i < nparams ; i++) { + if (STREQ(params[i].key, "offset")) { + if (params[i].value.ul != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Offset must be zero for this lock manager")); + return -1; + } + } else if (STREQ(params[i].key, "lockspace")) { + lockspace = params[i].value.str; + } else if (STREQ(params[i].key, "path")) { + path = params[i].value.str; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected parameter %s for lease resource"), + params[i].key); + return -1; + } + } + if (!path || !lockspace) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing path or lockspace for lease resource")); + return -1; + } + if (virAsprintf(&newLockspace, "%s/%s", + path, lockspace) < 0) { + virReportOOMError(); + return -1; + } + } break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown lock manager object type %d"), + type); + return -1; + } + + if (!(newName = strdup(name))) + goto no_memory; + + if (VIR_EXPAND_N(priv->resources, priv->nresources, 1) < 0) + goto no_memory; + + priv->resources[priv->nresources-1].lockspace = newLockspace; + priv->resources[priv->nresources-1].name = newName; + + if (flags & VIR_LOCK_MANAGER_RESOURCE_SHARED) + priv->resources[priv->nresources-1].flags |= + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED; + + return 0; + +no_memory: + virReportOOMError(); + VIR_FREE(newName); + return -1; +} + + +static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, + const char *state ATTRIBUTE_UNUSED, + unsigned int flags, + int *fd) +{ + virNetClientPtr client = NULL; + virNetClientProgramPtr program = NULL; + int counter = 0; + int rv = -1; + virLockManagerLockDaemonPrivatePtr priv = lock->privateData; + + virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY | + VIR_LOCK_MANAGER_ACQUIRE_RESTRICT, -1); + + if (priv->nresources == 0 && + priv->hasRWDisks && + driver->requireLeaseForDisks) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Read/write, exclusive access, disks were present, but no leases specified")); + return -1; + } + + if (!(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) + goto cleanup; + + if (fd && + (*fd = virNetClientDupFD(client, false)) < 0) + goto cleanup; + + if (!(flags & VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY)) { + size_t i; + for (i = 0 ; i < priv->nresources ; i++) { + virLockSpaceProtocolAcquireResourceArgs args; + + memset(&args, 0, sizeof(args)); + + if (priv->resources[i].lockspace) + args.path = priv->resources[i].lockspace; + args.name = priv->resources[i].name; + args.flags = priv->resources[i].flags; + + if (virNetClientProgramCall(program, + client, + counter++, + VIR_LOCK_SPACE_PROTOCOL_PROC_ACQUIRE_RESOURCE, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolAcquireResourceArgs, &args, + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; + } + } + + if ((flags & VIR_LOCK_MANAGER_ACQUIRE_RESTRICT) && + virLockManagerLockDaemonConnectionRestrict(lock, client, program, &counter) < 0) + goto cleanup; + + rv = 0; + +cleanup: + if (rv != 0 && fd) + VIR_FORCE_CLOSE(*fd); + virNetClientClose(client); + virObjectUnref(client); + virObjectUnref(program); + + return rv; +} + +static int virLockManagerLockDaemonRelease(virLockManagerPtr lock, + char **state, + unsigned int flags) +{ + virNetClientPtr client = NULL; + virNetClientProgramPtr program = NULL; + int counter = 0; + virLockSpaceProtocolReleaseResourceArgs args; + int rv = -1; + + memset(&args, 0, sizeof(args)); + + if (state) + *state = NULL; + + if (!(client = virLockManagerLockDaemonConnect(lock, &program, &counter))) + goto cleanup; + + args.flags = flags; + + if (virNetClientProgramCall(program, + client, + counter++, + VIR_LOCK_SPACE_PROTOCOL_PROC_RELEASE_RESOURCE, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolReleaseResourceArgs, &args, + (xdrproc_t)xdr_void, NULL) < 0) + goto cleanup; + + rv = 0; + +cleanup: + virNetClientClose(client); + virObjectUnref(client); + virObjectUnref(program); + + return rv; +} + + +static int virLockManagerLockDaemonInquire(virLockManagerPtr lock ATTRIBUTE_UNUSED, + char **state, + unsigned int flags) +{ + virCheckFlags(0, -1); + + if (state) + *state = NULL; + + return 0; +} + +virLockDriver virLockDriverImpl = +{ + .version = VIR_LOCK_MANAGER_VERSION, + .flags = 0, + + .drvInit = virLockManagerLockDaemonInit, + .drvDeinit = virLockManagerLockDaemonDeinit, + + .drvNew = virLockManagerLockDaemonNew, + .drvFree = virLockManagerLockDaemonFree, + + .drvAddResource = virLockManagerLockDaemonAddResource, + + .drvAcquire = virLockManagerLockDaemonAcquire, + .drvRelease = virLockManagerLockDaemonRelease, + + .drvInquire = virLockManagerLockDaemonInquire, +}; diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf new file mode 100644 index 0000000..0b885c5 --- /dev/null +++ b/src/locking/lockd.conf @@ -0,0 +1,18 @@ + +# +# The default lockd behaviour is to acquire locks directly +# against each configured disk file / block device. If the +# application wishes to instead manually manage leases in +# the guest XML, then this parameter can be disabled +# +#auto_disk_leases = 0 + +# +# Flag to determine whether we allow starting of guests +# which do not have any <lease> elements defined in their +# configuration. +# +# If 'auto_disk_leases' is disabled, this setting defaults +# to enabled, otherwise it defaults to disabled. +# +#require_lease_for_disks = 1 diff --git a/src/locking/test_libvirt_lockd.aug.in b/src/locking/test_libvirt_lockd.aug.in new file mode 100644 index 0000000..5be0d99 --- /dev/null +++ b/src/locking/test_libvirt_lockd.aug.in @@ -0,0 +1,6 @@ +module Test_libvirt_lockd = + ::CONFIG:: + + test Libvirt_lockd.lns get conf = +{ "auto_disk_leases" = "0" } +{ "require_lease_for_disks" = "1" } -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
This adds a 'lockd' lock driver which is just a client which talks to the lockd daemon to perform all locking. This will be the default lock driver for any hypervisor which needs one.
* src/Makefile.am: Add lockd.so plugin * src/locking/lock_driver_lockd.c: Lockd driver impl
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + po/POTFILES.in | 1 + run.in | 2 + src/Makefile.am | 54 ++- src/locking/libvirt_lockd.aug | 32 ++ src/locking/lock_driver_lockd.c | 658 ++++++++++++++++++++++++++++++++++ src/locking/lockd.conf | 18 + src/locking/test_libvirt_lockd.aug.in | 6 + 8 files changed, 766 insertions(+), 6 deletions(-) create mode 100644 src/locking/libvirt_lockd.aug create mode 100644 src/locking/lock_driver_lockd.c create mode 100644 src/locking/lockd.conf create mode 100644 src/locking/test_libvirt_lockd.aug.in
ACK if you fix compilation issue: CC lockd_la-lock_driver_lockd.lo In file included from locking/lock_driver_lockd.c:24:0: locking/lock_driver.h:26:26: fatal error: domain_conf.h: No such file or directory
From: "Daniel P. Berrange" <berrange@redhat.com> The default lockd driver behavour is to acquire leases directly on the disk files. This introduces an alternative mode, where leases are acquire indirectly on a file that is based on a SHA256 hash of the disk filename. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- bootstrap.conf | 1 + src/Makefile.am | 4 +- src/locking/libvirt_lockd.aug | 1 + src/locking/lock_driver_lockd.c | 128 +++++++++++++++++++++++++++++++--- src/locking/lockd.conf | 22 ++++++ src/locking/test_libvirt_lockd.aug.in | 1 + 6 files changed, 148 insertions(+), 9 deletions(-) diff --git a/bootstrap.conf b/bootstrap.conf index 59dd258..37a0ae1 100644 --- a/bootstrap.conf +++ b/bootstrap.conf @@ -36,6 +36,7 @@ connect configmake count-one-bits crypto/md5 +crypto/sha256 dirname-lgpl environ execinfo diff --git a/src/Makefile.am b/src/Makefile.am index 5808653..0a1b98f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1564,7 +1564,7 @@ lockd_la_SOURCES = \ $(LOCK_DRIVER_LOCKD_SOURCES) \ $(LOCK_PROTOCOL_GENERATED) \ $(NULL) -lockd_la_CFLAGS = $(AM_CFLAGS) +lockd_la_CFLAGS = -I$(top_srcdir)/src/conf $(AM_CFLAGS) lockd_la_LDFLAGS = -module -avoid-version lockd_la_LIBADD = ../gnulib/lib/libgnu.la libvirt-net-rpc.la libvirt-net-rpc-client.la if WITH_DTRACE_PROBES @@ -1940,6 +1940,7 @@ EXTRA_DIST += $(SECURITY_DRIVER_APPARMOR_HELPER_SOURCES) install-data-local: install-init install-systemd if WITH_LIBVIRTD $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd/files" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" endif $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt" @@ -1992,6 +1993,7 @@ endif uninstall-local:: uninstall-init uninstall-systemd if WITH_LIBVIRTD + rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd/files" ||: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/lockd" ||: rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/lockd" ||: endif diff --git a/src/locking/libvirt_lockd.aug b/src/locking/libvirt_lockd.aug index 4649644..dafd8f9 100644 --- a/src/locking/libvirt_lockd.aug +++ b/src/locking/libvirt_lockd.aug @@ -19,6 +19,7 @@ module Libvirt_lockd = (* Each enty in the config is one of the following three ... *) let entry = bool_entry "auto_disk_leases" | bool_entry "require_lease_for_disks" + | str_entry "file_lockspace_dir" let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index 089b284..aa0f94a 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -32,6 +32,7 @@ #include "rpc/virnetclient.h" #include "lock_protocol.h" #include "configmake.h" +#include "sha256.h" #define VIR_FROM_THIS VIR_FROM_LOCKING @@ -70,6 +71,8 @@ struct _virLockManagerLockDaemonPrivate { struct _virLockManagerLockDaemonDriver { bool autoDiskLease; bool requireLeaseForDisks; + + char *fileLockSpaceDir; }; static virLockManagerLockDaemonDriverPtr driver = NULL; @@ -120,6 +123,17 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) CHECK_TYPE("auto_disk_leases", VIR_CONF_LONG); if (p) driver->autoDiskLease = p->l; + p = virConfGetValue(conf, "file_lockspace_dir"); + CHECK_TYPE("file_lockspace_dir", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->fileLockSpaceDir); + if (!(driver->fileLockSpaceDir = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + } + p = virConfGetValue(conf, "require_lease_for_disks"); CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG); if (p) @@ -288,6 +302,47 @@ error: } +static int virLockManagerLockDaemonSetupLockspace(const char *path) +{ + virNetClientPtr client; + virNetClientProgramPtr program = NULL; + virLockSpaceProtocolCreateLockSpaceArgs args; + int rv = -1; + int counter = 0; + + memset(&args, 0, sizeof(args)); + args.path = (char*)path; + + if (!(client = virLockManagerLockDaemonConnectionNew(getuid() == 0, &program))) + return -1; + + if (virNetClientProgramCall(program, + client, + counter++, + VIR_LOCK_SPACE_PROTOCOL_PROC_CREATE_LOCKSPACE, + 0, NULL, NULL, NULL, + (xdrproc_t)xdr_virLockSpaceProtocolCreateLockSpaceArgs, (char*)&args, + (xdrproc_t)xdr_void, NULL) < 0) { + virErrorPtr err = virGetLastError(); + if (err && err->code == VIR_ERR_OPERATION_INVALID) { + /* The lockspace already exists */ + virResetLastError(); + rv = 0; + } else { + goto cleanup; + } + } + + rv = 0; + +cleanup: + virObjectUnref(program); + virNetClientClose(client); + virObjectUnref(client); + return rv; +} + + static int virLockManagerLockDaemonDeinit(void); static int virLockManagerLockDaemonInit(unsigned int version, @@ -312,6 +367,13 @@ static int virLockManagerLockDaemonInit(unsigned int version, if (virLockManagerLockDaemonLoadConfig(configFile) < 0) goto error; + if (driver->autoDiskLease) { + if (driver->fileLockSpaceDir && + virLockManagerLockDaemonSetupLockspace(driver->fileLockSpaceDir) < 0) + goto error; + } + + return 0; error: @@ -324,6 +386,7 @@ static int virLockManagerLockDaemonDeinit(void) if (!driver) return 0; + VIR_FREE(driver->fileLockSpaceDir); VIR_FREE(driver); return 0; @@ -421,6 +484,36 @@ static int virLockManagerLockDaemonNew(virLockManagerPtr lock, } +static const char hex[] = { '0', '1', '2', '3', '4', '5', '6', '7', + '8', '9', 'a', 'b', 'c', 'd', 'e', 'f' }; + +static char *virLockManagerLockDaemonDiskLeaseName(const char *path) +{ + unsigned char buf[SHA256_DIGEST_SIZE]; + char *ret; + int i; + + if (!(sha256_buffer(path, strlen(path), buf))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to compute sha256 checksum")); + return NULL; + } + + if (VIR_ALLOC_N(ret, (SHA256_DIGEST_SIZE * 2) + 1) < 0) { + virReportOOMError(); + return NULL; + } + + for (i = 0 ; i < SHA256_DIGEST_SIZE ; i++) { + ret[i*2] = hex[(buf[i] >> 4) & 0xf]; + ret[(i*2)+1] = hex[buf[i] & 0xf]; + } + ret[(SHA256_DIGEST_SIZE * 2) + 1] = '\0'; + + return ret; +} + + static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, unsigned int type, const char *name, @@ -429,8 +522,9 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, unsigned int flags) { virLockManagerLockDaemonPrivatePtr priv = lock->privateData; - char *newName; + char *newName = NULL; char *newLockspace = NULL; + bool autoCreate = false; virCheckFlags(VIR_LOCK_MANAGER_RESOURCE_READONLY | VIR_LOCK_MANAGER_RESOURCE_SHARED, -1); @@ -451,10 +545,22 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, priv->hasRWDisks = true; return 0; } - if (!(newLockspace = strdup(""))) { - virReportOOMError(); - return -1; + + if (driver->fileLockSpaceDir) { + if (!(newLockspace = strdup(driver->fileLockSpaceDir))) + goto no_memory; + if (!(newName = virLockManagerLockDaemonDiskLeaseName(name))) + goto no_memory; + autoCreate = true; + VIR_DEBUG("Using indirect lease %s for %s", newName, name); + } else { + if (!(newLockspace = strdup(""))) + goto no_memory; + if (!(newName = strdup(name))) + goto no_memory; + VIR_DEBUG("Using direct lease for %s", name); } + break; case VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE: { size_t i; @@ -488,6 +594,9 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, virReportOOMError(); return -1; } + if (!(newName = strdup(name))) + goto no_memory; + } break; default: virReportError(VIR_ERR_INTERNAL_ERROR, @@ -496,9 +605,6 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, return -1; } - if (!(newName = strdup(name))) - goto no_memory; - if (VIR_EXPAND_N(priv->resources, priv->nresources, 1) < 0) goto no_memory; @@ -509,10 +615,15 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, priv->resources[priv->nresources-1].flags |= VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_SHARED; + if (autoCreate) + priv->resources[priv->nresources-1].flags |= + VIR_LOCK_SPACE_PROTOCOL_ACQUIRE_RESOURCE_AUTOCREATE; + return 0; no_memory: virReportOOMError(); + VIR_FREE(newLockspace); VIR_FREE(newName); return -1; } @@ -521,6 +632,7 @@ no_memory: static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, const char *state ATTRIBUTE_UNUSED, unsigned int flags, + virDomainLockFailureAction action ATTRIBUTE_UNUSED, int *fd) { virNetClientPtr client = NULL; @@ -555,7 +667,7 @@ static int virLockManagerLockDaemonAcquire(virLockManagerPtr lock, memset(&args, 0, sizeof(args)); if (priv->resources[i].lockspace) - args.path = priv->resources[i].lockspace; + args.path = priv->resources[i].lockspace; args.name = priv->resources[i].name; args.flags = priv->resources[i].flags; diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf index 0b885c5..7545fd9 100644 --- a/src/locking/lockd.conf +++ b/src/locking/lockd.conf @@ -16,3 +16,25 @@ # to enabled, otherwise it defaults to disabled. # #require_lease_for_disks = 1 + + +# +# The default lockd behaviour is to use the "direct" +# lockspace, where the locks are acquired against the +# actual file paths associated with the <disk> devices. +# +# Setting a directory here causes lockd to use "indirect" +# lockspace, where a hash of the <disk> file path is +# used to create a file in the lockspace directory. The +# locks are then held on these hash files instead. +# +# This can be useful if the file paths refer to block +# devices which are shared, since /dev fcntl() locks +# don't propagate across hosts. It is also useful if +# the filesystem does not support fcntl() locks. +# +# Typically this directory would be located on a shared +# filesystem visible to all hosts accessing the same +# storage. +# +#file_lockspace_dir = "/var/lib/libvirt/lockd/files" diff --git a/src/locking/test_libvirt_lockd.aug.in b/src/locking/test_libvirt_lockd.aug.in index 5be0d99..2e65af6 100644 --- a/src/locking/test_libvirt_lockd.aug.in +++ b/src/locking/test_libvirt_lockd.aug.in @@ -4,3 +4,4 @@ module Test_libvirt_lockd = test Libvirt_lockd.lns get conf = { "auto_disk_leases" = "0" } { "require_lease_for_disks" = "1" } +{ "file_lockspace_dir" = "/var/lib/libvirt/lockd/files" } -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The default lockd driver behavour is to acquire leases directly on the disk files. This introduces an alternative mode, where leases are acquire indirectly on a file that is based on a SHA256 hash of the disk filename.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- bootstrap.conf | 1 + src/Makefile.am | 4 +- src/locking/libvirt_lockd.aug | 1 + src/locking/lock_driver_lockd.c | 128 +++++++++++++++++++++++++++++++--- src/locking/lockd.conf | 22 ++++++ src/locking/test_libvirt_lockd.aug.in | 1 + 6 files changed, 148 insertions(+), 9 deletions(-)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> --- src/locking/lock_driver_lockd.c | 35 ++++++++++++++++++++++++++++++++++- src/locking/lockd.conf | 14 ++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index aa0f94a..abae52d 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -73,6 +73,7 @@ struct _virLockManagerLockDaemonDriver { bool requireLeaseForDisks; char *fileLockSpaceDir; + char *lvmLockSpaceDir; }; static virLockManagerLockDaemonDriverPtr driver = NULL; @@ -134,6 +135,17 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) } } + p = virConfGetValue(conf, "lvm_lockspace_dir"); + CHECK_TYPE("lvm_lockspace_dir", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->lvmLockSpaceDir); + if (!(driver->lvmLockSpaceDir = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + } + p = virConfGetValue(conf, "require_lease_for_disks"); CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG); if (p) @@ -371,8 +383,11 @@ static int virLockManagerLockDaemonInit(unsigned int version, if (driver->fileLockSpaceDir && virLockManagerLockDaemonSetupLockspace(driver->fileLockSpaceDir) < 0) goto error; - } + if (driver->lvmLockSpaceDir && + virLockManagerLockDaemonSetupLockspace(driver->lvmLockSpaceDir) < 0) + goto error; + } return 0; @@ -546,6 +561,24 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, return 0; } + /* XXX we should somehow pass in TYPE=BLOCK info + * from the domain_lock code, instead of assuming /dev + */ + if (STRPREFIX(name, "/dev") && + driver->lvmLockSpaceDir) { + VIR_DEBUG("Trying to find an LVM UUID for %s", name); + newName = virStorageFileGetLVMKey(name); + if (newName) { + VIR_DEBUG("Got an LVM UUID %s for %s", newName, name); + if (!(newLockspace = strdup(driver->lvmLockSpaceDir))) + goto no_memory; + autoCreate = true; + break; + } + virResetLastError(); + /* Fallback to generic non-block code */ + } + if (driver->fileLockSpaceDir) { if (!(newLockspace = strdup(driver->fileLockSpaceDir))) goto no_memory; diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf index 7545fd9..00f9b49 100644 --- a/src/locking/lockd.conf +++ b/src/locking/lockd.conf @@ -38,3 +38,17 @@ # storage. # #file_lockspace_dir = "/var/lib/libvirt/lockd/files" + + +# +# When using LVM volumes that can be visible across +# multiple, it is desirable to do locking based on +# the unique UUID associated with each volume, instead +# of their paths. Setting this path causes libvirt to +# do UUID based locking for LVM. +# +# Typically this directory would be located on a shared +# filesystem visible to all hosts accessing the same +# storage. +# +#lvm_lockspace_dir = "/var/lib/libvirt/lockd/lvmvolumes" -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/locking/lock_driver_lockd.c | 35 ++++++++++++++++++++++++++++++++++- src/locking/lockd.conf | 14 ++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> --- src/locking/lock_driver_lockd.c | 31 +++++++++++++++++++++++++++++++ src/locking/lockd.conf | 14 ++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c index abae52d..8b07875 100644 --- a/src/locking/lock_driver_lockd.c +++ b/src/locking/lock_driver_lockd.c @@ -74,6 +74,7 @@ struct _virLockManagerLockDaemonDriver { char *fileLockSpaceDir; char *lvmLockSpaceDir; + char *scsiLockSpaceDir; }; static virLockManagerLockDaemonDriverPtr driver = NULL; @@ -146,6 +147,17 @@ static int virLockManagerLockDaemonLoadConfig(const char *configFile) } } + p = virConfGetValue(conf, "scsi_lockspace_dir"); + CHECK_TYPE("scsi_lockspace_dir", VIR_CONF_STRING); + if (p && p->str) { + VIR_FREE(driver->scsiLockSpaceDir); + if (!(driver->scsiLockSpaceDir = strdup(p->str))) { + virReportOOMError(); + virConfFree(conf); + return -1; + } + } + p = virConfGetValue(conf, "require_lease_for_disks"); CHECK_TYPE("require_lease_for_disks", VIR_CONF_LONG); if (p) @@ -387,6 +399,10 @@ static int virLockManagerLockDaemonInit(unsigned int version, if (driver->lvmLockSpaceDir && virLockManagerLockDaemonSetupLockspace(driver->lvmLockSpaceDir) < 0) goto error; + + if (driver->scsiLockSpaceDir && + virLockManagerLockDaemonSetupLockspace(driver->scsiLockSpaceDir) < 0) + goto error; } return 0; @@ -579,6 +595,21 @@ static int virLockManagerLockDaemonAddResource(virLockManagerPtr lock, /* Fallback to generic non-block code */ } + if (STRPREFIX(name, "/dev") && + driver->scsiLockSpaceDir) { + VIR_DEBUG("Trying to find an SCSI ID for %s", name); + newName = virStorageFileGetSCSIKey(name); + if (newName) { + VIR_DEBUG("Got an SCSI ID %s for %s", newName, name); + if (!(newLockspace = strdup(driver->scsiLockSpaceDir))) + goto no_memory; + autoCreate = true; + break; + } + virResetLastError(); + /* Fallback to generic non-block code */ + } + if (driver->fileLockSpaceDir) { if (!(newLockspace = strdup(driver->fileLockSpaceDir))) goto no_memory; diff --git a/src/locking/lockd.conf b/src/locking/lockd.conf index 00f9b49..85edb91 100644 --- a/src/locking/lockd.conf +++ b/src/locking/lockd.conf @@ -52,3 +52,17 @@ # storage. # #lvm_lockspace_dir = "/var/lib/libvirt/lockd/lvmvolumes" + + +# +# When using SCSI volumes that can be visible across +# multiple, it is desirable to do locking based on +# the unique UUID associated with each volume, instead +# of their paths. Setting this path causes libvirt to +# do UUID based locking for SCSI. +# +# Typically this directory would be located on a shared +# filesystem visible to all hosts accessing the same +# storage. +# +#scsi_lockspace_dir = "/var/lib/libvirt/lockd/scsivolumes" -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
--- src/locking/lock_driver_lockd.c | 31 +++++++++++++++++++++++++++++++ src/locking/lockd.conf | 14 ++++++++++++++ 2 files changed, 45 insertions(+)
ACK Michal
From: "Daniel P. Berrange" <berrange@redhat.com> The current default QEMU lock manager is the 'nop' lock manager, which obviously does not perform any locking. The new virtlockd daemon is able to work out of the box with zero configuration in single-host only mode. Enable this as the default lock manager for all QEMU guests * src/qemu/qemu.conf: Update docs for lock_driver parameter * src/qemu/qemu_conf.c: Change default lock manager to 'lockd' Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu.conf | 17 ++++++++++------- src/qemu/qemu_conf.c | 2 +- src/qemu/test_libvirtd_qemu.aug.in | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index dd853c8..ada18df 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -356,14 +356,17 @@ # #allow_disk_format_probing = 1 - -# To enable 'Sanlock' project based locking of the file -# content (to prevent two VMs writing to the same -# disk), uncomment this +# By default the QEMU driver talks to the 'virtlockd' +# daemon to acquire exclusive locks for all guest disk +# images associated with a running VM. # -#lock_manager = "sanlock" - - +# To revert to behaviour of previous releases which did +# not acquire any locks, set this to 'nop'. +# +# To enable use of the external 'sanlock' locking +# daemon, change this to 'sanlock'. +# +#lock_manager = "lockd" # Set limit of maximum APIs queued on one domain. All other APIs # over this threshold will fail on acquiring job lock. Specially, diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 4c506a0..1365e70 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -117,7 +117,7 @@ int qemuLoadDriverConfig(virQEMUDriverPtr driver, } #endif - if (!(driver->lockManager = virLockManagerPluginNew("nop", + if (!(driver->lockManager = virLockManagerPluginNew("lockd", "qemu", driver->configBaseDir, 0))) diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 2892204..65fb22d 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -56,7 +56,7 @@ module Test_libvirtd_qemu = { "mac_filter" = "1" } { "relaxed_acs_check" = "1" } { "allow_disk_format_probing" = "1" } -{ "lock_manager" = "sanlock" } +{ "lock_manager" = "lockd" } { "max_queued" = "0" } { "keepalive_interval" = "5" } { "keepalive_count" = "5" } -- 1.7.11.7
On 11.12.2012 21:41, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange@redhat.com>
The current default QEMU lock manager is the 'nop' lock manager, which obviously does not perform any locking. The new virtlockd daemon is able to work out of the box with zero configuration in single-host only mode. Enable this as the default lock manager for all QEMU guests
* src/qemu/qemu.conf: Update docs for lock_driver parameter * src/qemu/qemu_conf.c: Change default lock manager to 'lockd'
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/qemu/qemu.conf | 17 ++++++++++------- src/qemu/qemu_conf.c | 2 +- src/qemu/test_libvirtd_qemu.aug.in | 2 +- 3 files changed, 12 insertions(+), 9 deletions(-)
ACK Michal
On 11.12.2012 21:41, Daniel P. Berrange wrote:
This is an update of
https://www.redhat.com/archives/libvir-list/2012-September/msg00816.html
This series was previously fully acked, but before pushing I noticed some problems and stopped. Since then I've done quite a few more changes to rebase to latest GIT & follow new best practice.
I've also added support for a config file for the lockd plugin and the ability to use custom lockspaces for LVM and SCSI IDs. I've also added support to acquire leases based on a SHA256 sum of the file, instead of directly on the file
As such I don't feel comfortable pushing, without further reviews.
While we're past the freeze date, I'd like to request an exception, for all except the 14th patch. Without the 14th patch, this series has minimal impact on existing code that is used in libvirt so little regression possibility. Further I discovered yesterday that we already have people using this code as custom add-on patches, so I'd like to help them by including it in 1.0.1
.gitignore | 5 bootstrap.conf | 1 cfg.mk | 9 libvirt.spec.in | 16 po/POTFILES.in | 4 run.in | 2 src/Makefile.am | 214 ++++- src/internal.h | 22 src/libvirt_private.syms | 2 src/locking/libvirt_lockd.aug | 33 src/locking/lock_daemon.c | 1441 ++++++++++++++++++++++++++++++++++ src/locking/lock_daemon.h | 56 + src/locking/lock_daemon_config.c | 193 ++++ src/locking/lock_daemon_config.h | 50 + src/locking/lock_daemon_dispatch.c | 434 ++++++++++ src/locking/lock_daemon_dispatch.h | 31 src/locking/lock_driver_lockd.c | 834 +++++++++++++++++++ src/locking/lock_manager.c | 20 src/locking/lock_manager.h | 3 src/locking/lock_protocol.x | 95 ++ src/locking/lockd.conf | 68 + src/locking/test_libvirt_lockd.aug.in | 7 src/locking/virtlockd.init.in | 93 ++ src/locking/virtlockd.service.in | 13 src/locking/virtlockd.socket.in | 8 src/locking/virtlockd.sysconf | 3 src/qemu/qemu.conf | 17 src/qemu/qemu_conf.c | 12 src/qemu/qemu_conf.h | 3 src/qemu/qemu_driver.c | 24 src/qemu/test_libvirtd_qemu.aug.in | 2 src/util/storage_file.c | 30 src/util/storage_file.h | 4 33 files changed, 3697 insertions(+), 52 deletions(-)
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
You will have yet another merge conflicts. The commit a4e44e674eb833af10c74af0c297dd5b3e60fa67 is the last one that this set can be applied on cleanly. I've conditionally ACKed the rest of patches but I think pushing at this phase should get either green or red based on upstream decision. I vote for pushing in ASAP. We will have another -rc probably so still time to fix bugs. Michal
On Tue, Dec 11, 2012 at 08:41:34PM +0000, Daniel P. Berrange wrote:
While we're past the freeze date, I'd like to request an exception, for all except the 14th patch. Without the 14th patch, this series has minimal impact on existing code that is used in libvirt so little regression possibility. Further I discovered yesterday that we already have people using this code as custom add-on patches, so I'd like to help them by including it in 1.0.1
No one has any opinions on this ? 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 :|
On 12/13/2012 04:26 AM, Daniel P. Berrange wrote:
On Tue, Dec 11, 2012 at 08:41:34PM +0000, Daniel P. Berrange wrote:
While we're past the freeze date, I'd like to request an exception, for all except the 14th patch. Without the 14th patch, this series has minimal impact on existing code that is used in libvirt so little regression possibility. Further I discovered yesterday that we already have people using this code as custom add-on patches, so I'd like to help them by including it in 1.0.1
No one has any opinions on this ?
I think the series has been posted long enough, and even mostly had acks prior to the freeze, so it's not like a late-breaking code addition, rather that it has just taken us a while to get it stable. I can agree with getting it in the tree now, before rc2, as it does improve reliability of libvirt out-of-the-box on a single machine. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On Thu, Dec 13, 2012 at 05:46:56AM -0700, Eric Blake wrote:
On 12/13/2012 04:26 AM, Daniel P. Berrange wrote:
On Tue, Dec 11, 2012 at 08:41:34PM +0000, Daniel P. Berrange wrote:
While we're past the freeze date, I'd like to request an exception, for all except the 14th patch. Without the 14th patch, this series has minimal impact on existing code that is used in libvirt so little regression possibility. Further I discovered yesterday that we already have people using this code as custom add-on patches, so I'd like to help them by including it in 1.0.1
No one has any opinions on this ?
I think the series has been posted long enough, and even mostly had acks prior to the freeze, so it's not like a late-breaking code addition, rather that it has just taken us a while to get it stable. I can agree with getting it in the tree now, before rc2, as it does improve reliability of libvirt out-of-the-box on a single machine.
Ok, based on your & Michael's ACKs, I'm pushing this (minus the change to make it default). I also validated it didn't break Win32 builds. Hopefully the same is true of BSD/OS-X, but appreciate some testing of that... 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 :|
participants (4)
-
Daniel P. Berrange -
Eric Blake -
Michal Privoznik -
Peter Krempa