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

Re: [libvirt] [PATCH 03/10] Basic framework for lock manager plugins



On Thu, May 19, 2011 at 07:24:18AM -0400, Daniel P. Berrange wrote:
> Define the basic framework lock manager plugins. The
> basic plugin API for 3rd parties to implemented is
> defined in
> 
>   src/locking/lock_driver.h
> 
> This allows dlopen()able modules for alternative locking
> schemes, however, we do not install the header. This
> requires lock plugins to be in-tree allowing changing of
> the lock manager plugin API in future.
> 
> The libvirt code for loading & calling into plugins
> is in
> 
>   src/locking/lock_manager.{c,h}
> 
> * include/libvirt/virterror.h, src/util/virterror.c: Add
>   VIR_FROM_LOCKING
> * src/locking/lock_driver.h: API for lock driver plugins
>   to implement
> * src/locking/lock_manager.c, src/locking/lock_manager.h:
>   Internal API for managing locking
> * src/Makefile.am: Add locking code
> ---
>  include/libvirt/virterror.h |    1 +
>  po/POTFILES.in              |    1 +
>  src/Makefile.am             |    3 +-
>  src/libvirt_private.syms    |   14 ++
>  src/locking/README          |  158 +++++++++++++++++++
>  src/locking/lock_driver.h   |  293 +++++++++++++++++++++++++++++++++++
>  src/locking/lock_manager.c  |  357 +++++++++++++++++++++++++++++++++++++++++++
>  src/locking/lock_manager.h  |   65 ++++++++
>  src/util/virterror.c        |    3 +
>  9 files changed, 894 insertions(+), 1 deletions(-)
>  create mode 100644 src/locking/README
>  create mode 100644 src/locking/lock_driver.h
>  create mode 100644 src/locking/lock_manager.c
>  create mode 100644 src/locking/lock_manager.h
> 
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 0708e02..efa4796 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -81,6 +81,7 @@ typedef enum {
>      VIR_FROM_VMWARE = 39,	/* Error from VMware driver */
>      VIR_FROM_EVENT = 40,       /* Error from event loop impl */
>      VIR_FROM_LIBXL = 41,	/* Error from libxenlight driver */
> +    VIR_FROM_LOCKING = 42,      /* Error from lock manager */
>  } virErrorDomain;
>  
>  
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index dd44da2..9c3d287 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -31,6 +31,7 @@ src/fdstream.c
>  src/interface/netcf_driver.c
>  src/internal.h
>  src/libvirt.c
> +src/locking/lock_manager.c
>  src/lxc/lxc_container.c
>  src/lxc/lxc_conf.c
>  src/lxc/lxc_controller.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 58eb2a7..a27838b 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -93,7 +93,8 @@ DRIVER_SOURCES =						\
>  		datatypes.c datatypes.h				\
>  		fdstream.c fdstream.h                 \
>  		$(NODE_INFO_SOURCES)				\
> -		libvirt.c libvirt_internal.h
> +		libvirt.c libvirt_internal.h			\
> +		locking/lock_manager.c locking/lock_manager.h
>  
>  
>  # XML configuration format handling sources
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 1b13c5c..1784c0d 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -588,6 +588,20 @@ virRegisterSecretDriver;
>  virRegisterStorageDriver;
>  
>  
> +# locking.h
> +virLockManagerAcquire;
> +virLockManagerAddResource;
> +virLockManagerFree;
> +virLockManagerInquire;
> +virLockManagerNew;
> +virLockManagerPluginNew;
> +virLockManagerPluginRef;
> +virLockManagerPluginUnref;
> +virLockManagerPluginUsesState;
> +virLockManagerPluginGetName;
> +virLockManagerRelease;
> +
> +
>  # logging.h
>  virLogDefineFilter;
>  virLogDefineOutput;
> diff --git a/src/locking/README b/src/locking/README
> new file mode 100644
> index 0000000..4fa4f89
> --- /dev/null
> +++ b/src/locking/README
> @@ -0,0 +1,158 @@
> +
> +At libvirtd startup:
> +
> +  plugin = virLockManagerPluginLoad("sync-manager");
> +
> +
> +At libvirtd shtudown:
> +
> +  virLockManagerPluginUnload(plugin)
> +
> +
> +At guest startup:
> +
> +  manager = virLockManagerNew(plugin,
> +                              VIR_LOCK_MANAGER_OBJECT_DOMAIN,
> +                              0);
> +
> +  virLockManagerSetParameter(manager, "id", id);
> +  virLockManagerSetParameter(manager, "uuid", uuid);
> +  virLockManagerSetParameter(manager, "name", name);
> +
> +  foreach disk
> +    virLockManagerRegisterResource(manager,
> +                                   VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> +                                   disk.path,
> +                                   ..flags...);
> +
> +  if (!virLockManagerAcquireObject(manager))
> +    abort..
> +
> +  run QEMU
> +
> +
> +At guest shutdown:
> +
> +  ...send QEMU 'quit' monitor command, and/or kill(qemupid)...
> +
> +  if (!virLockManagerShutdown(manager))
> +     kill(supervisorpid); /* XXX or leave it running ??? */
> +
> +  virLockManagerFree(manager);
> +
> +
> +
> +At libvirtd restart with running guests:
> +
> +  foreach still running guest
> +    manager = virLockManagerNew(driver,
> +                                VIR_LOCK_MANAGER_START_DOMAIN,
> +                                VIR_LOCK_MANAGER_NEW_ATTACH);
> +    virLockManagerSetParameter(manager, "id", id);
> +    virLockManagerSetParameter(manager, "uuid", uuid);
> +    virLockManagerSetParameter(manager, "name", name);
> +
> +    if (!virLockManagerGetChild(manager, &qemupid))
> +      kill(supervisorpid); /* XXX or leave it running ??? */
> +
> +
> +
> +With disk hotplug:
> +
> +  if (virLockManagerAcquireResource(manager,
> +                                    VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> +                                    disk.path
> +                                    ..flags..))
> +     ...abort hotplug attempt ...
> +
> +  ...hotplug the device...
> +
> +
> +
> +With disk unhotplug:
> +
> +    ...hotunplug the device...
> +
> +  if (virLockManagerReleaseResource(manager,
> +                                    VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> +                                    disk.path
> +                                    ..flags..))
> +     ...log warning ...
> +
> +
> +
> +During migration:
> +
> +  1. On source host
> +
> +       if (!virLockManagerPrepareMigrate(manager, hosturi))
> +           ..don't start migration..
> +
> +  2. On dest host
> +
> +      manager = virLockManagerNew(driver,
> +                                  VIR_LOCK_MANAGER_START_DOMAIN,
> +                                  VIR_LOCK_MANAGER_NEW_MIGRATE);
> +      virLockManagerSetParameter(manager, "id", id);
> +      virLockManagerSetParameter(manager, "uuid", uuid);
> +      virLockManagerSetParameter(manager, "name", name);
> +
> +      foreach disk
> +        virLockManagerRegisterResource(manager,
> +                                       VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK,
> +                                       disk.path,
> +                                       ..flags...);
> +
> +      char **supervisorargv;
> +      int supervisorargc;
> +
> +      supervisor = virLockManagerGetSupervisorPath(manager);
> +      virLockManagerGetSupervisorArgs(&argv, &argc);
> +
> +      cmd = qemuBuildCommandLine(supervisor, supervisorargv, supervisorargv);
> +
> +      supervisorpid = virCommandExec(cmd);
> +
> +      if (!virLockManagerGetChild(manager, &qemupid))
> +        kill(supervisorpid); /* XXX or leave it running ??? */
> +
> +  3. Initiate migration in QEMU on source and wait for completion
> +
> +  4a. On failure
> +
> +      4a1 On target
> +
> +            virLockManagerCompleteMigrateIn(manager,
> +                                            VIR_LOCK_MANAGER_MIGRATE_CANCEL);
> +            virLockManagerShutdown(manager);
> +            virLockManagerFree(manager);
> +
> +      4a2 On source
> +
> +            virLockManagerCompleteMigrateIn(manager,
> +                                            VIR_LOCK_MANAGER_MIGRATE_CANCEL);
> +
> +  4b. On succcess
> +
> +
> +      4b1 On target
> +
> +            virLockManagerCompleteMigrateIn(manager, 0);
> +
> +      42 On source
> +
> +            virLockManagerCompleteMigrateIn(manager, 0);
> +            virLockManagerShutdown(manager);
> +            virLockManagerFree(manager);
> +
> +
> +Notes:
> +
> +  - If a lock manager impl does just VM level leases, it can
> +    ignore all the resource paths at startup.
> +
> +  - If a lock manager impl does not support migrate
> +    it can return an error from all migrate calls
> +
> +  - If a lock manger impl does not support hotplug
> +    it can return an error from all resource acquire/release calls
> diff --git a/src/locking/lock_driver.h b/src/locking/lock_driver.h
> new file mode 100644
> index 0000000..40a55f6
> --- /dev/null
> +++ b/src/locking/lock_driver.h
> @@ -0,0 +1,293 @@
> +/*
> + * lock_driver.h: Defines the lock driver plugin API
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *

Please add
   * Author: Daniel P. Berrange <berrange redhat com>

well I assume you wrote it :-)

> + */
> +
> +#ifndef __VIR_PLUGINS_LOCK_DRIVER_H__
> +# define __VIR_PLUGINS_LOCK_DRIVER_H__
> +
> +# include "internal.h"
> +
> +typedef struct _virLockManager virLockManager;
> +typedef virLockManager *virLockManagerPtr;
> +
> +typedef struct _virLockDriver virLockDriver;
> +typedef virLockDriver *virLockDriverPtr;
> +
> +typedef struct _virLockManagerParam virLockManagerParam;
> +typedef virLockManagerParam *virLockManagerParamPtr;
> +
> +typedef enum {
> +    /* State passing is used to re-acquire existing leases */
> +    VIR_LOCK_MANAGER_USES_STATE = (1 << 0)
> +} virLockManagerFlags;
> +
> +typedef enum {
> +    /* The managed object is a virtual guest domain */
> +    VIR_LOCK_MANAGER_OBJECT_TYPE_DOMAIN = 0,
> +} virLockManagerObjectType;
> +
> +typedef enum {
> +    /* The resource to be locked is a virtual disk */
> +    VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK = 0,
> +    /* A lease against an arbitrary resource */
> +    VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE = 1,
> +} virLockManagerResourceType;
> +
> +typedef enum {
> +    /* The resource is assigned in readonly mode */
> +    VIR_LOCK_MANAGER_RESOURCE_READONLY = (1 << 0),
> +    /* The resource is assigned in shared, writable mode */
> +    VIR_LOCK_MANAGER_RESOURCE_SHARED   = (1 << 1),
> +} virLockManagerResourceFlags;
> +
> +typedef enum {
> +    /* Don't acquire the resources, just register the object PID */
> +    VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY = (1 << 0)
> +} virLockManagerAcquireFlags;
> +
> +enum {
> +    VIR_LOCK_MANAGER_PARAM_TYPE_STRING,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_INT,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_LONG,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_UINT,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_ULONG,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_DOUBLE,
> +    VIR_LOCK_MANAGER_PARAM_TYPE_UUID,
> +};
> +
> +struct _virLockManagerParam {
> +    int type;
> +    const char *key;
> +    union {
> +        int i;
> +        long long l;
> +        unsigned int ui;
> +        unsigned long long ul;
> +        double d;
> +        char *str;
> +        unsigned char uuid[16];
> +    } value;
> +};
> +
> +
> +/*
> + * Changes in major version denote incompatible ABI changes
> + * Changes in minor version denote new compatible API entry points
> + * Changes in micro version denote new compatible flags
> + */
> +# define VIR_LOCK_MANAGER_VERSION_MAJOR 1
> +# define VIR_LOCK_MANAGER_VERSION_MINOR 0
> +# define VIR_LOCK_MANAGER_VERSION_MICRO 0
> +
> +# define VIR_LOCK_MANAGER_VERSION                           \
> +    ((VIR_LOCK_MANAGER_VERSION_MAJOR * 1000 * 1000) +       \
> +     (VIR_LOCK_MANAGER_VERSION_MINOR * 1000) +              \
> +     (VIR_LOCK_MANAGER_VERSION_MICRO))
> +
> +
> +
> +/**
> + * virLockDriverInit:
> + * @version: the libvirt requested plugin ABI version
> + * @flags: the libvirt requested plugin optional extras
> + *
> + * Allow the plugin to validate the libvirt requested
> + * plugin version / flags. This allows the plugin impl
> + * to block its use in versions of libvirtd which are
> + * too old to support key features.
> + *
> + * NB: A plugin may be loaded multiple times, for different
> + * libvirt drivers (eg QEMU, LXC, UML)
> + *
> + * Returns -1 if the requested version/flags were inadequate
> + */
> +typedef int (*virLockDriverInit)(unsigned int version,
> +                                 unsigned int flags);
> +
> +/**
> + * virLockDriverDeinit:
> + *
> + * Called to release any resources prior to the plugin
> + * being unloaded from memory. Returns -1 to prevent
> + * plugin from being unloaded from memory.
> + */
> +typedef int (*virLockDriverDeinit)(void);
> +
> +/**
> + * virLockManagerNew:
> + * @man: the lock manager context
> + * @type: the type of process to be supervised
> + * @nparams: number of metadata parameters
> + * @params: extra metadata parameters
> + * @flags: optional flags, currently unused
> + *
> + * Initialize a new context to supervise a process, usually
> + * a virtual machine. The lock driver implementation can use
> + * the <code>privateData</code> field of <code>man</code>
> + * to store a pointer to any driver specific state.
> + *
> + * A process of VIR_LOCK_MANAGER_START_DOMAIN will be
> + * given the following parameters
> + *
> + * - id: the domain unique id (unsigned int)
> + * - uuid: the domain uuid (uuid)
> + * - name: the domain name (string)
> + * - pid: process ID to own/owning the lock (unsigned int)
> + *
> + * Returns 0 if successful initialized a new context, -1 on error
> + */
> +typedef int (*virLockDriverNew)(virLockManagerPtr man,
> +                                unsigned int type,
> +                                size_t nparams,
> +                                virLockManagerParamPtr params,
> +                                unsigned int flags);
> +
> +/**
> + * virLockDriverFree:
> + * @manager: the lock manager context
> + *
> + * Release any resources associated with the lock manager
> + * context private data
> + */
> +typedef void (*virLockDriverFree)(virLockManagerPtr man);
> +
> +/**
> + * virLockDriverAddResource:
> + * @manager: the lock manager context
> + * @type: the resource type virLockManagerResourceType
> + * @name: the resource name
> + * @nparams: number of metadata parameters
> + * @params: extra metadata parameters
> + * @flags: the resource access flags
> + *
> + * Assign a resource to a managed object. This will
> + * only be called prior to the object is being locked
> + * when it is inactive. eg, to set the initial  boot
> + * time disk assignments on a VM
> + * The format of @name varies according to
> + * the resource @type. A VIR_LOCK_MANAGER_RESOURCE_TYPE_DISK
> + * will have the fully qualified file path, while a resource
> + * of type VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE will have the
> + * unique name of the lease
> + *
> + * A resource of type VIR_LOCK_MANAGER_RESOURCE_TYPE_LEASE
> + * will receive at least the following extra parameters
> + *
> + *  - 'path': a fully qualified path to the lockspace
> + *  - 'lockspace': globally string identifying the lockspace name
> + *  - 'offset': byte offset within the lease (unsigned long long)
> + *
> + * If no flags are given, the resource is assumed to be
> + * used in exclusive, read-write mode. Access can be
> + * relaxed to readonly, or shared read-write.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverAddResource)(virLockManagerPtr man,
> +                                        unsigned int type,
> +                                        const char *name,
> +                                        size_t nparams,
> +                                        virLockManagerParamPtr params,
> +                                        unsigned int flags);
> +
> +/**
> + * virLockDriverAcquire:
> + * @manager: the lock manager context
> + * @state: the current lock state
> + * @flags: optional flags, currently unused
> + *
> + * Start managing resources for the object. This
> + * must be called from the PID that represents the
> + * object to be managed. If the lock is lost at any
> + * time, the PID will be killed off by the lock manager.
> + * The optional state contains information about the
> + * locks previously held for the object.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverAcquire)(virLockManagerPtr man,
> +                                    const char *state,
> +                                    unsigned int flags);
> +
> +/**
> + * virLockDriverRelease:
> + * @manager: the lock manager context
> + * @state: pointer to be filled with lock state
> + * @flags: optional flags
> + *
> + * Inform the lock manager that the supervised process has
> + * been, or can be stopped.
> + *
> + * Returns 0 on success, or -1 on failure
> + */
> +typedef int (*virLockDriverRelease)(virLockManagerPtr man,
> +                                    char **state,
> +                                    unsigned int flags);
> +
> +/**
> + * virLockDriverInquire:
> + * @manager: the lock manager context
> + * @state: pointer to be filled with lock state
> + * @flags: optional flags, currently unused
> + *
> + * Retrieve the current lock state. The returned
> + * lock state may be NULL if none is required. The
> + * caller is responsible for freeing the lock
> + * state string when it is no longer required
> + *
> + * Returns 0 on success, or -1 on failure.
> + */
> +typedef int (*virLockDriverInquire)(virLockManagerPtr man,
> +                                    char **state,
> +                                    unsigned int flags);
> +
> +
> +struct _virLockManager {
> +    virLockDriverPtr driver;
> +    void *privateData;
> +};
> +
> +/**
> + * The plugin must export a static instance of this
> + * driver table, with the name 'virLockDriverImpl'
> + */
> +struct _virLockDriver {
> +    /**
> +     * @version: the newest implemented plugin ABI version
> +     * @flags: optional flags, currently unused
> +     */
> +    unsigned int version;
> +    unsigned int flags;
> +
> +    virLockDriverInit drvInit;
> +    virLockDriverDeinit drvDeinit;
> +
> +    virLockDriverNew drvNew;
> +    virLockDriverFree drvFree;
> +
> +    virLockDriverAddResource drvAddResource;
> +
> +    virLockDriverAcquire drvAcquire;
> +    virLockDriverRelease drvRelease;
> +    virLockDriverInquire drvInquire;
> +};
> +
> +
> +#endif /* __VIR_PLUGINS_LOCK_DRIVER_H__ */
> diff --git a/src/locking/lock_manager.c b/src/locking/lock_manager.c
> new file mode 100644
> index 0000000..cb96091
> --- /dev/null
> +++ b/src/locking/lock_manager.c
> @@ -0,0 +1,357 @@
> +/*
> + * lock_manager.c: Implements the internal lock manager API
> + *
> + * 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, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307  USA
> + *

Same thing :-)

> + */
> +
> +#include <config.h>
> +
> +#include "lock_manager.h"
> +#include "virterror_internal.h"
> +#include "logging.h"
> +#include "util.h"
> +#include "memory.h"
> +#include "uuid.h"
> +
> +#include <dlfcn.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +
> +#include "configmake.h"
V> +
> +#define VIR_FROM_THIS VIR_FROM_LOCKING
[...]
> +/**
> + * virLockManagerPluginRef:
> + * @plugin: the plugin implementation to ref
> + *
> + * Acquires an additional reference on the plugin.
> + */
> +void virLockManagerPluginRef(virLockManagerPluginPtr plugin)
> +{
> +    plugin->refs++;
> +}
> +
> +
> +/**
> + * virLockManagerPluginUnref:
> + * @plugin: the plugin implementation to unref
> + *
> + * Releases a reference on the plugin. When the last reference
> + * is released, it will attempt to unload the plugin from memory.
> + * The plugin may refuse to allow unloading if this would
> + * result in an unsafe scenario.
> + *
> + */
> +void virLockManagerPluginUnref(virLockManagerPluginPtr plugin)
> +{
> +    if (!plugin)
> +        return;
> +
> +    plugin->refs--;

  Shoudn't we protect those ref/unrefs with a global lock ?
Chances of entering the race there sounds small but this looks racy
Could be done as an improvement.

> +    if (plugin->refs > 0)
> +        return;

  besides minor comments, ACK,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/


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