On 11.12.2012 21:41, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> .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@xxxxxxxxxx>
> + */
> +
> +#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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list