[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 01/12] Introduce an internal API for handling file based lockspaces



On 12.09.2012 18:28, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange redhat com>
> 
> The previously introduced virFile{Lock,Unlock} APIs provide a
> way to acquire/release fcntl() locks on individual files. For
> unknown reason though, the POSIX spec says that fcntl() locks
> are released when *any* file handle referring to the same path
> is closed. In the following sequence
> 
>   threadA: fd1 = open("foo")
>   threadB: fd2 = open("foo")
>   threadA: virFileLock(fd1)
>   threadB: virFileLock(fd2)
>   threadB: close(fd2)
> 
> you'd expect threadA to come out holding a lock on 'foo', and
> indeed it does hold a lock for a very short time. Unfortunately
> when threadB does close(fd2) this releases the lock associated
> with fd1. For the current libvirt use case for virFileLock -
> pidfiles - this doesn't matter since the lock is acquired
> at startup while single threaded an never released until
> exit.
> 
> To provide a more generally useful API though, it is necessary
> to introduce a slightly higher level abstraction, which is to
> be referred to as a "lockspace".  This is to be provided by
> a virLockSpacePtr object in src/util/virlockspace.{c,h}. The
> core idea is that the lockspace keeps track of what files are
> already open+locked. This means that when a 2nd thread comes
> along and tries to acquire a lock, it doesn't end up opening
> and closing a new FD. The lockspace just checks the current
> list of held locks and immediately returns VIR_ERR_RESOURCE_BUSY.
> 
> NB, the API as it stands is designed on the basis that the
> files being locked are not being otherwise opened and used
> by the application code. One approach to using this API is to
> acquire locks based on a hash of the filepath.
> 
> eg to lock /var/lib/libvirt/images/foo.img the application
> might do
> 
>    virLockSpacePtr lockspace = virLockSpaceNew("/var/lib/libvirt/imagelocks");
>    lockname = md5sum("/var/lib/libvirt/images/foo.img");
>    virLockSpaceAcquireLock(lockspace, lockname);
> 
> NB, in this example, the caller should ensure that the path
> is canonicalized before calculating the checksum.
> 
> It is also possible to do locks directly on resources by
> using a NULL lockspace directory and then using the file
> path as the lock name eg
> 
>    virLockSpacePtr lockspace = virLockSpaceNew(NULL);
>    virLockSpaceAcquireLock(lockspace, "/var/lib/libvirt/images/foo.img");
> 
> This is only safe to do though if no other part of the process
> will be opening the files. This will be the case when this
> code is used inside the soon-to-be-reposted virlockd daemon
> 
> Signed-off-by: Daniel P. Berrange <berrange redhat com>
> ---
>  .gitignore                  |   1 +
>  include/libvirt/virterror.h |   4 +-
>  po/POTFILES.in              |   1 +
>  src/Makefile.am             |   1 +
>  src/libvirt_private.syms    |  11 +
>  src/util/virlockspace.c     | 558 ++++++++++++++++++++++++++++++++++++++++++++
>  src/util/virlockspace.h     |  58 +++++
>  src/util/virterror.c        |   9 +-
>  tests/Makefile.am           |   7 +-
>  tests/virlockspacetest.c    | 363 ++++++++++++++++++++++++++++
>  10 files changed, 1010 insertions(+), 3 deletions(-)
>  create mode 100644 src/util/virlockspace.c
>  create mode 100644 src/util/virlockspace.h
>  create mode 100644 tests/virlockspacetest.c
> 

ACK but see my comments below.

> diff --git a/.gitignore b/.gitignore
> index d998f0e..7919f74 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -161,6 +161,7 @@
>  /tests/virdrivermoduletest
>  /tests/virhashtest
>  /tests/virkeyfiletest
> +/tests/virlockspacetest
>  /tests/virnet*test
>  /tests/virshtest
>  /tests/virtimetest
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 5140c38..98c2cb2 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -112,7 +112,8 @@ typedef enum {
>      VIR_FROM_PARALLELS = 48,    /* Error from Parallels */
>      VIR_FROM_DEVICE = 49,       /* Error from Device */
>  
> -    VIR_FROM_SSH = 50,       /* Error from libssh2 connection transport */
> +    VIR_FROM_SSH = 50,          /* Error from libssh2 connection transport */
> +    VIR_FROM_LOCKSPACE = 51,    /* Error from lockspace */
>  
>  # ifdef VIR_ENUM_SENTINELS
>      VIR_ERR_DOMAIN_LAST
> @@ -285,6 +286,7 @@ typedef enum {
>      VIR_ERR_SSH = 85,                   /* error in ssh transport driver */
>      VIR_ERR_AGENT_UNRESPONSIVE = 86,    /* guest agent is unresponsive,
>                                             not running or not usable */
> +    VIR_ERR_RESOURCE_BUSY = 87,         /* resource is already in use */
>  } virErrorNumber;
>  
>  /**
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 7a91eb4..c5f4cf7 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -148,6 +148,7 @@ src/util/virdbus.c
>  src/util/virfile.c
>  src/util/virhash.c
>  src/util/virkeyfile.c
> +src/util/virlockspace.c
>  src/util/virnetdev.c
>  src/util/virnetdevbridge.c
>  src/util/virnetdevmacvlan.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 9f27fcf..6860e7f 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -96,6 +96,7 @@ UTIL_SOURCES =							\
>  		util/virkeycode.c util/virkeycode.h		\
>  		util/virkeyfile.c util/virkeyfile.h		\
>  		util/virkeymaps.h				\
> +		util/virlockspace.c util/virlockspace.h		\
>  		util/virmacaddr.h util/virmacaddr.c		\
>  		util/virnetdev.h util/virnetdev.c		\
>  		util/virnetdevbandwidth.h util/virnetdevbandwidth.c \
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 0494e1f..58a9520 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1321,6 +1321,17 @@ virKeyFileHasGroup;
>  virKeyFileGetValueString;
>  
>  
> +# virlockspace.h
> +virLockSpaceAcquireResource;
> +virLockSpaceCreateResource;
> +virLockSpaceDeleteResource;
> +virLockSpaceFree;
> +virLockSpaceGetDirectory;
> +virLockSpaceNew;
> +virLockSpaceReleaseResource;
> +virLockSpaceReleaseResourcesForOwner;
> +
> +
>  # virmacaddr.h
>  virMacAddrCmp;
>  virMacAddrCmpRaw;
> diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
> new file mode 100644
> index 0000000..611592a
> --- /dev/null
> +++ b/src/util/virlockspace.c
> @@ -0,0 +1,558 @@
> +/*
> + * virlockspace.c: simple file based lockspaces
> + *
> + * Copyright (C) 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/>.
> + *
> + */
> +
> +#include <config.h>
> +
> +#include "virlockspace.h"
> +#include "logging.h"
> +#include "memory.h"
> +#include "virterror_internal.h"
> +#include "util.h"
> +#include "virfile.h"
> +#include "virhash.h"
> +#include "threads.h"
> +
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/stat.h>
> +
> +#define VIR_FROM_THIS VIR_FROM_LOCKSPACE
> +
> +typedef struct _virLockSpaceResource virLockSpaceResource;
> +typedef virLockSpaceResource *virLockSpaceResourcePtr;
> +
> +struct _virLockSpaceResource {
> +    char *name;
> +    char *path;
> +    int fd;
> +    bool lockHeld;
> +    unsigned int flags;
> +    size_t nOwners;
> +    pid_t *owners;
> +};
> +
> +struct _virLockSpace {
> +    char *dir;
> +    virMutex lock;
> +
> +    virHashTablePtr resources;
> +};
> +
> +
> +static char *virLockSpaceGetResourcePath(virLockSpacePtr lockspace,
> +                                         const char *resname)
> +{
> +    char *ret;
> +    if (lockspace->dir) {
> +        if (virAsprintf(&ret, "%s/%s", lockspace->dir, resname) < 0) {
> +            virReportOOMError();
> +            return NULL;
> +        }
> +    } else {
> +        if (!(ret = strdup(resname))) {
> +            virReportOOMError();
> +            return NULL;
> +        }
> +    }
> +
> +    return ret;
> +}
> +
> +
> +static void virLockSpaceResourceFree(virLockSpaceResourcePtr res)
> +{
> +    if (!res)
> +        return;
> +
> +    if (res->lockHeld &&
> +        (res->flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE)) {
> +        if (res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) {
> +            /* We must upgrade to an exclusive lock to ensure
> +             * no one else still has it before trying to delete */
> +            if (virFileLock(res->fd, false, 0, 1) < 0) {
> +                VIR_DEBUG("Could not upgrade shared lease to exclusive, not deleting");
> +            } else {
> +                if (unlink(res->path) < 0 &&
> +                    errno != ENOENT) {
> +                    char ebuf[1024];
> +                    VIR_WARN("Failed to unlink resource %s: %s",
> +                             res->path, virStrerror(errno, ebuf, sizeof(ebuf)));
> +                }
> +            }
> +        } else {
> +            if (unlink(res->path) < 0 &&
> +                errno != ENOENT) {
> +                char ebuf[1024];
> +                VIR_WARN("Failed to unlink resource %s: %s",
> +                         res->path, virStrerror(errno, ebuf, sizeof(ebuf)));
> +            }
> +        }
> +    }
> +
> +    VIR_FORCE_CLOSE(res->fd);
> +    VIR_FREE(res->path);
> +    VIR_FREE(res->name);
> +    VIR_FREE(res);
> +}
> +
> +
> +static virLockSpaceResourcePtr
> +virLockSpaceResourceNew(virLockSpacePtr lockspace,
> +                        const char *resname,
> +                        unsigned int flags,
> +                        pid_t owner)
> +{
> +    virLockSpaceResourcePtr res;
> +    bool shared = !!(flags & VIR_LOCK_SPACE_ACQUIRE_SHARED);
> +
> +    if (VIR_ALLOC(res) < 0)
> +        return NULL;
> +
> +    res->fd = -1;
> +    res->flags = flags;
> +
> +    if (!(res->name = strdup(resname)))
> +        goto no_memory;
> +
> +    if (!(res->path = virLockSpaceGetResourcePath(lockspace, resname)))
> +        goto no_memory;
> +
> +    if (flags & VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) {
> +        while (1) {
> +            struct stat a, b;
> +            if ((res->fd = open(res->path, O_RDWR|O_CREAT, 0600)) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Unable to open/create resource %s"),
> +                                     res->path);
> +                goto error;
> +            }
> +
> +            if (virSetCloseExec(res->fd) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Failed to set close-on-exec flag '%s'"),
> +                                     res->path);
> +                goto error;
> +            }
> +
> +            if (fstat(res->fd, &b) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Unable to check status of pid file '%s'"),
> +                                     res->path);
> +                goto error;
> +            }
> +
> +            if (virFileLock(res->fd, shared, 0, 1) < 0) {
> +                if (errno == EACCES || errno == EAGAIN) {
> +                    virReportError(VIR_ERR_RESOURCE_BUSY,
> +                                   _("Lockspace resource '%s' is locked"),
> +                                   resname);
> +                } else {
> +                    virReportSystemError(errno,
> +                                         _("Unable to acquire lock on '%s'"),
> +                                         res->path);
> +                }
> +                goto error;
> +            }
> +
> +            /* Now make sure the pidfile we locked is the same
> +             * one that now exists on the filesystem
> +             */
> +            if (stat(res->path, &a) < 0) {
> +                char ebuf[1024] ATTRIBUTE_UNUSED;
> +                VIR_DEBUG("Resource '%s' disappeared: %s",
> +                          res->path, virStrerror(errno, ebuf, sizeof(ebuf)));
> +                VIR_FORCE_CLOSE(res->fd);
> +                /* Someone else must be racing with us, so try again */
> +                continue;
> +            }
> +
> +            if (a.st_ino == b.st_ino)
> +                break;
> +
> +            VIR_DEBUG("Resource '%s' was recreated", res->path);
> +            VIR_FORCE_CLOSE(res->fd);
> +            /* Someone else must be racing with us, so try again */
> +        }
> +    } else {
> +        if ((res->fd = open(res->path, O_RDWR)) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Unable to open resource %s"),
> +                                 res->path);
> +            goto error;
> +        }
> +
> +        if (virSetCloseExec(res->fd) < 0) {
> +            virReportSystemError(errno,
> +                                 _("Failed to set close-on-exec flag '%s'"),
> +                                 res->path);
> +            goto error;
> +        }
> +
> +        if (virFileLock(res->fd, !!(flags & VIR_LOCK_SPACE_ACQUIRE_SHARED), 0, 0) < 0) {

I think you can s/!!(...)/shared/

> +            if (errno == EACCES || errno == EAGAIN) {
> +                virReportError(VIR_ERR_RESOURCE_BUSY,
> +                               _("Lockspace resource '%s' is locked"),
> +                               resname);
> +            } else {
> +                virReportSystemError(errno,
> +                                     _("Unable to acquire lock on '%s'"),
> +                                     res->path);
> +            }
> +            goto error;
> +        }
> +    }
> +    res->lockHeld = true;
> +
> +    if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0)
> +        goto no_memory;
> +
> +    res->owners[res->nOwners-1] = owner;
> +
> +    return res;
> +
> +no_memory:
> +    virReportOOMError();
> +error:
> +    virLockSpaceResourceFree(res);
> +    return NULL;
> +}
> +
> +
> +static void virLockSpaceResourceDataFree(void *opaque, const void *name ATTRIBUTE_UNUSED)
> +{
> +    virLockSpaceResourcePtr res = opaque;
> +    virLockSpaceResourceFree(res);
> +}
> +
> +
> +virLockSpacePtr virLockSpaceNew(const char *directory)
> +{
> +    virLockSpacePtr lockspace;
> +
> +    VIR_DEBUG("directory=%s", NULLSTR(directory));
> +
> +    if (VIR_ALLOC(lockspace) < 0)
> +        return NULL;
> +
> +    if (virMutexInit(&lockspace->lock) < 0) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                       _("Unable to initialize lockspace mutex"));
> +        VIR_FREE(lockspace);
> +        return NULL;
> +    }
> +
> +    if (directory &&
> +        !(lockspace->dir = strdup(directory)))
> +        goto no_memory;
> +
> +    if (!(lockspace->resources = virHashCreate(10,
> +                                               virLockSpaceResourceDataFree)))

10 is a magic constant esp. when used in subsequent patches.
I think you can
  #define VIR_LOCK_SPACE_RESOURCE_HASH_TABLE_SIZE 10
or something.

> +        goto error;
> +
> +    if (directory) {
> +        if (virFileExists(directory)) {
> +            if (!virFileIsDir(directory)) {
> +                virReportError(VIR_ERR_INTERNAL_ERROR,
> +                               _("Lockspace location %s exists, but is not a directory"),
> +                           directory);
> +                goto error;
> +            }
> +        } else {
> +            if (virFileMakePathWithMode(directory, 0700) < 0) {
> +                virReportSystemError(errno,
> +                                     _("Unable to create lockspace %s"),
> +                                     directory);
> +                goto error;
> +            }
> +        }
> +    }
> +
> +    return lockspace;
> +
> +no_memory:
> +    virReportOOMError();
> +error:
> +    virLockSpaceFree(lockspace);
> +    return NULL;
> +}
> +
> +
> +void virLockSpaceFree(virLockSpacePtr lockspace)
> +{
> +    if (!lockspace)
> +        return;
> +
> +    virHashFree(lockspace->resources);
> +    VIR_FREE(lockspace->dir);
> +    virMutexDestroy(&lockspace->lock);
> +    VIR_FREE(lockspace);
> +}
> +
> +
> +const char *virLockSpaceGetDirectory(virLockSpacePtr lockspace)
> +{
> +    return lockspace->dir;
> +}
> +
> +
> +int virLockSpaceCreateResource(virLockSpacePtr lockspace,
> +                               const char *resname)
> +{
> +    int ret = -1;
> +    char *respath = NULL;
> +    virLockSpaceResourcePtr res;
> +
> +    VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname);
> +
> +    virMutexLock(&lockspace->lock);
> +
> +    if ((res = virHashLookup(lockspace->resources, resname))) {
> +        virReportError(VIR_ERR_RESOURCE_BUSY,
> +                       _("Lockspace resource '%s' is locked"),
> +                       resname);
> +        goto cleanup;
> +    }
> +
> +    if (!(respath = virLockSpaceGetResourcePath(lockspace, resname)))
> +        goto cleanup;
> +
> +    if (virFileTouch(respath, 0600) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virMutexUnlock(&lockspace->lock);
> +    VIR_FREE(respath);
> +    return ret;
> +}
> +
> +
> +int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
> +                               const char *resname)
> +{
> +    int ret = -1;
> +    char *respath = NULL;
> +    virLockSpaceResourcePtr res;
> +
> +    VIR_DEBUG("lockspace=%p resname=%s", lockspace, resname);
> +
> +    virMutexLock(&lockspace->lock);
> +
> +    if ((res = virHashLookup(lockspace->resources, resname))) {
> +        virReportError(VIR_ERR_RESOURCE_BUSY,
> +                       _("Lockspace resource '%s' is locked"),
> +                       resname);
> +        goto cleanup;
> +    }
> +
> +    if (!(respath = virLockSpaceGetResourcePath(lockspace, resname)))
> +        goto cleanup;
> +
> +    if (unlink(respath) < 0 &&
> +        errno != ENOENT) {
> +        virReportSystemError(errno,
> +                             _("Unable to delete lockspace resource %s"),
> +                             respath);
> +        goto cleanup;
> +    }
> +
> +    ret = 0;
> +
> +cleanup:
> +    virMutexUnlock(&lockspace->lock);
> +    VIR_FREE(respath);
> +    return ret;
> +}
> +
> +
> +int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
> +                                const char *resname,
> +                                pid_t owner,
> +                                unsigned int flags)
> +{
> +    int ret = -1;
> +    virLockSpaceResourcePtr res;
> +
> +    VIR_DEBUG("lockspace=%p resname=%s flags=%x owner=%lld",
> +              lockspace, resname, flags, (unsigned long long)owner);
> +
> +    virCheckFlags(VIR_LOCK_SPACE_ACQUIRE_SHARED |
> +                  VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE, -1);
> +
> +    virMutexLock(&lockspace->lock);
> +
> +    if ((res = virHashLookup(lockspace->resources, resname))) {
> +        if ((res->flags & VIR_LOCK_SPACE_ACQUIRE_SHARED) &&
> +            (flags & VIR_LOCK_SPACE_ACQUIRE_SHARED)) {
> +
> +            if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0) {
> +                virReportOOMError();
> +                goto cleanup;
> +            }
> +            res->owners[res->nOwners-1] = owner;
> +
> +            goto done;
> +        }
> +        virReportError(VIR_ERR_RESOURCE_BUSY,
> +                       _("Lockspace resource '%s' is locked"),
> +                       resname);
> +        goto cleanup;
> +    }
> +
> +    if (!(res = virLockSpaceResourceNew(lockspace, resname, flags, owner)))
> +        goto cleanup;
> +
> +    if (virHashAddEntry(lockspace->resources, resname, res) < 0) {
> +        virLockSpaceResourceFree(res);
> +        goto cleanup;
> +    }
> +
> +done:
> +    ret = 0;
> +
> +cleanup:
> +    virMutexUnlock(&lockspace->lock);
> +    return ret;
> +}
> +
> +
> +int virLockSpaceReleaseResource(virLockSpacePtr lockspace,
> +                                const char *resname,
> +                                pid_t owner)
> +{
> +    int ret = -1;
> +    virLockSpaceResourcePtr res;
> +    size_t i;
> +
> +    VIR_DEBUG("lockspace=%p resname=%s owner=%lld",
> +              lockspace, resname, (unsigned long long)owner);
> +
> +    virMutexLock(&lockspace->lock);
> +
> +    if (!(res = virHashLookup(lockspace->resources, resname))) {
> +        virReportError(VIR_ERR_RESOURCE_BUSY,
> +                       _("Lockspace resource '%s' is not locked"),
> +                       resname);
> +        goto cleanup;
> +    }
> +
> +    for (i = 0 ; i < res->nOwners ; i++) {
> +        if (res->owners[i] == owner) {
> +            break;
> +        }
> +    }
> +
> +    if (i == res->nOwners) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("owner %lld does not hold the resource lock"),
> +                       (unsigned long long)owner);
> +        goto cleanup;
> +    }
> +
> +    if (i < (res->nOwners - 1))
> +        memmove(res->owners + i,
> +                res->owners + i + 1,
> +                (res->nOwners - i - 1) * sizeof(res->owners[0]));
> +    VIR_SHRINK_N(res->owners, res->nOwners, 1);
> +
> +    if ((res->nOwners == 0) &&
> +        virHashRemoveEntry(lockspace->resources, resname) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virMutexUnlock(&lockspace->lock);
> +    return ret;
> +}
> +
> +
> +struct virLockSpaceRemoveData {
> +    pid_t owner;
> +    size_t count;
> +};
> +
> +
> +static int
> +virLockSpaceRemoveResourcesForOwner(const void *payload,
> +                                    const void *name ATTRIBUTE_UNUSED,
> +                                    const void *opaque)
> +{
> +    virLockSpaceResourcePtr res = (virLockSpaceResourcePtr)payload;
> +    struct virLockSpaceRemoveData *data = (struct virLockSpaceRemoveData *)opaque;
> +    size_t i;
> +
> +    VIR_DEBUG("res %s owner %lld", res->name, (unsigned long long)data->owner);
> +
> +    for (i = 0 ; i < res->nOwners ; i++) {
> +        if (res->owners[i] == data->owner) {
> +            break;
> +        }
> +    }
> +
> +    if (i == res->nOwners)
> +        return 0;
> +
> +    data->count++;
> +
> +    if (i < (res->nOwners - 1))
> +        memmove(res->owners + i,
> +                res->owners + i + 1,
> +                (res->nOwners - i - 1) * sizeof(res->owners[0]));
> +    VIR_SHRINK_N(res->owners, res->nOwners, 1);
> +
> +    if (res->nOwners) {
> +        VIR_DEBUG("Other shared owners remain");
> +        return 0;
> +    }
> +
> +    VIR_DEBUG("No more owners, remove it");
> +    return 1;
> +}
> +
> +
> +int virLockSpaceReleaseResourcesForOwner(virLockSpacePtr lockspace,
> +                                         pid_t owner)
> +{
> +    int ret = 0;
> +    struct virLockSpaceRemoveData data = {
> +        owner, 0
> +    };
> +
> +    VIR_DEBUG("lockspace=%p owner=%lld", lockspace, (unsigned long long)owner);
> +
> +    virMutexLock(&lockspace->lock);
> +
> +    if (virHashRemoveSet(lockspace->resources,
> +                         virLockSpaceRemoveResourcesForOwner,
> +                         &data) < 0)
> +        goto error;
> +
> +    ret = data.count;
> +
> +    virMutexUnlock(&lockspace->lock);
> +    return ret;
> +
> +error:
> +    virMutexUnlock(&lockspace->lock);
> +    return -1;
> +}
> diff --git a/src/util/virlockspace.h b/src/util/virlockspace.h
> new file mode 100644
> index 0000000..bd8f91c
> --- /dev/null
> +++ b/src/util/virlockspace.h
> @@ -0,0 +1,58 @@
> +/*
> + * virlockspace.h: simple file based lockspaces
> + *
> + * Copyright (C) 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/>.
> + *
> + */
> +
> +#ifndef __VIR_LOCK_SPACE_H__
> +# define __VIR_LOCK_SPACE_H__
> +
> +# include "internal.h"
> +
> +typedef struct _virLockSpace virLockSpace;
> +typedef virLockSpace *virLockSpacePtr;
> +
> +virLockSpacePtr virLockSpaceNew(const char *directory);
> +
> +void virLockSpaceFree(virLockSpacePtr lockspace);
> +
> +const char *virLockSpaceGetDirectory(virLockSpacePtr lockspace);
> +
> +int virLockSpaceCreateResource(virLockSpacePtr lockspace,
> +                               const char *resname);
> +int virLockSpaceDeleteResource(virLockSpacePtr lockspace,
> +                               const char *resname);
> +
> +typedef enum {
> +    VIR_LOCK_SPACE_ACQUIRE_SHARED     = (1 << 0),
> +    VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE = (1 << 1),
> +} virLockSpaceAcquireFlags;
> +
> +int virLockSpaceAcquireResource(virLockSpacePtr lockspace,
> +                                const char *resname,
> +                                pid_t owner,
> +                                unsigned int flags);
> +
> +int virLockSpaceReleaseResource(virLockSpacePtr lockspace,
> +                                const char *resname,
> +                                pid_t owner);
> +
> +int virLockSpaceReleaseResourcesForOwner(virLockSpacePtr lockspace,
> +                                         pid_t owner);
> +
> +#endif /* __VIR_LOCK_SPACE_H__ */
> diff --git a/src/util/virterror.c b/src/util/virterror.c
> index 7caa69e..6fade3b 100644
> --- a/src/util/virterror.c
> +++ b/src/util/virterror.c
> @@ -115,7 +115,8 @@ VIR_ENUM_IMPL(virErrorDomain, VIR_ERR_DOMAIN_LAST,
>                "Parallels Cloud Server",
>                "Device Config",
>  
> -              "SSH transport layer" /* 50 */
> +              "SSH transport layer", /* 50 */
> +              "Lock Space",
>      )
>  
>  
> @@ -1206,6 +1207,12 @@ virErrorMsg(virErrorNumber error, const char *info)
>              else
>                  errmsg = _("Guest agent is not responding: %s");
>              break;
> +        case VIR_ERR_RESOURCE_BUSY:
> +            if (info == NULL)
> +                errmsg = _("resource busy");
> +            else
> +                errmsg = _("resource busy %s");
> +            break;
>      }
>      return errmsg;
>  }
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index bec89e2..bcc2163 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -92,7 +92,7 @@ test_programs = virshtest sockettest \
>  	viratomictest \
>  	utiltest virnettlscontexttest shunloadtest \
>  	virtimetest viruritest virkeyfiletest \
> -	virauthconfigtest
> +	virauthconfigtest virlockspacetest
>  
>  if WITH_SECDRIVER_SELINUX
>  test_programs += securityselinuxtest
> @@ -534,6 +534,11 @@ virtimetest_SOURCES = \
>  virtimetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS)
>  virtimetest_LDADD = $(LDADDS)
>  
> +virlockspacetest_SOURCES = \
> +	virlockspacetest.c testutils.h testutils.c
> +virlockspacetest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS)
> +virlockspacetest_LDADD = $(LDADDS)
> +
>  viruritest_SOURCES = \
>  	viruritest.c testutils.h testutils.c
>  viruritest_CFLAGS = -Dabs_builddir="\"$(abs_builddir)\"" $(AM_CFLAGS)
> diff --git a/tests/virlockspacetest.c b/tests/virlockspacetest.c
> new file mode 100644
> index 0000000..ee58f95
> --- /dev/null
> +++ b/tests/virlockspacetest.c
> @@ -0,0 +1,363 @@
> +/*
> + * Copyright (C) 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/>.
> + *
> + * Author: Daniel P. Berrange <berrange redhat com>
> + */
> +
> +#include <config.h>
> +
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <sys/stat.h>
> +
> +#include "testutils.h"
> +#include "util.h"
> +#include "virterror_internal.h"
> +#include "memory.h"
> +#include "logging.h"
> +
> +#include "virlockspace.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_RPC
> +
> +#define LOCKSPACE_DIR abs_builddir "/virlockspacedata"
> +
> +static int testLockSpaceCreate(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLifecycle(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    if (virLockSpaceCreateResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLockExcl(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    if (virLockSpaceCreateResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(), 0) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(), 0) == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLockExclAuto(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    if (virLockSpaceCreateResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLockShr(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    if (virLockSpaceCreateResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_SHARED) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(), 0) == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_SHARED) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLockShrAuto(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(LOCKSPACE_DIR);
> +
> +    if (!virFileIsDir(LOCKSPACE_DIR))
> +        goto cleanup;
> +
> +    if (virLockSpaceCreateResource(lockspace, "foo") < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_SHARED |
> +                                    VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) == 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, "foo", geteuid(),
> +                                    VIR_LOCK_SPACE_ACQUIRE_SHARED |
> +                                    VIR_LOCK_SPACE_ACQUIRE_AUTOCREATE) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, "foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +static int testLockSpaceResourceLockPath(const void *args ATTRIBUTE_UNUSED)
> +{
> +    virLockSpacePtr lockspace;
> +    int ret = -1;
> +
> +    rmdir(LOCKSPACE_DIR);
> +
> +    lockspace = virLockSpaceNew(NULL);
> +
> +    mkdir(LOCKSPACE_DIR, 0700);
> +
> +    if (virLockSpaceCreateResource(lockspace, LOCKSPACE_DIR "/foo") < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, LOCKSPACE_DIR "/foo", geteuid(), 0) < 0)
> +        goto cleanup;
> +
> +    if (!virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    if (virLockSpaceAcquireResource(lockspace, LOCKSPACE_DIR "/foo", geteuid(), 0) == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, LOCKSPACE_DIR "/foo") == 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceReleaseResource(lockspace, LOCKSPACE_DIR "/foo", geteuid()) < 0)
> +        goto cleanup;
> +
> +    if (virLockSpaceDeleteResource(lockspace, LOCKSPACE_DIR "/foo") < 0)
> +        goto cleanup;
> +
> +    if (virFileExists(LOCKSPACE_DIR "/foo"))
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    virLockSpaceFree(lockspace);
> +    rmdir(LOCKSPACE_DIR);
> +    return ret;
> +}
> +
> +
> +
> +static int
> +mymain(void)
> +{
> +    int ret = 0;
> +
> +    signal(SIGPIPE, SIG_IGN);
> +
> +    if (virtTestRun("Lockspace creation", 1, testLockSpaceCreate, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res lifecycle", 1, testLockSpaceResourceLifecycle, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res lock excl", 1, testLockSpaceResourceLockExcl, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res lock shr", 1, testLockSpaceResourceLockShr, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res lock excl auto", 1, testLockSpaceResourceLockExclAuto, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res lock shr auto", 1, testLockSpaceResourceLockShrAuto, NULL) < 0)
> +        ret = -1;
> +
> +    if (virtTestRun("Lockspace res full path", 1, testLockSpaceResourceLockPath, NULL) < 0)
> +        ret = -1;
> +
> +    return ret==0 ? EXIT_SUCCESS : EXIT_FAILURE;
> +}
> +
> +VIRT_TEST_MAIN(mymain)
> 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]