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

Re: [libvirt] [PATCH 11/12] Add support for re-exec() of virtlockd upon SIGUSR1



On 12.09.2012 18:29, 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

s/upgrads/upgrades/

> 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, 408 insertions(+), 14 deletions(-)

ACK with one nit

> 
> diff --git a/libvirt.spec.in b/libvirt.spec.in
> index 62938e5..71f838b 100644
> --- a/libvirt.spec.in
> +++ b/libvirt.spec.in
> @@ -1470,7 +1470,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 7bc6917..ab9ca35 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -42,6 +42,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"
> @@ -64,6 +65,7 @@ struct _virLockDaemon {
>  virLockDaemonPtr lockDaemon = NULL;
>  
>  static bool privileged;
> +static bool execRestart = false;
>  
>  enum {
>      VIR_LOCK_DAEMON_ERR_NONE = 0,
> @@ -97,6 +99,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)
>  {
> @@ -138,7 +148,7 @@ virLockDaemonNew(void)
>                                         -1, 0,
>                                         NULL, NULL,
>                                         virLockDaemonClientNew,
> -                                       NULL,
> +                                       virLockDaemonClientPreExecRestart,
>                                         virLockDaemonClientFree,
>                                         NULL)))
>          goto error;
> @@ -158,6 +168,90 @@ error:
>  }
>  
>  
> +static virLockDaemonPtr
> +virLockDaemonNewPostExecRestart(virJSONValuePtr object)
> +{
> +    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(3,
> +                                            virLockDaemonLockSpaceDataFree)))

Again, s/3/{macro}/

> +        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,
> +                                                      NULL)))
> +        goto error;
> +
> +    return lockd;
> +
> +error:
> +    virLockDaemonFree(lockd);
> +    return NULL;
> +}
> +
> +
>  int virLockDaemonAddLockSpace(virLockDaemonPtr lockd,
>                                const char *path,
>                                virLockSpacePtr lockspace)
> @@ -442,6 +536,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)
>  {
> @@ -451,6 +554,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;
>  }
>  
> @@ -464,6 +569,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;
> @@ -674,6 +781,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(void)
> +{
> +    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)))
> +        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)
>  {
> @@ -866,22 +1244,30 @@ int main(int argc, char **argv) {
>          goto cleanup;
>      }
>  
> -
> -    if (!(lockDaemon = virLockDaemonNew())) {
> +    if ((rv = virLockDaemonPostExecRestart()) < 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())) {
> +            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) {
> @@ -921,7 +1307,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);
> 


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