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

From: "Daniel P. Berrange" <berrange(a)redhat.com&gt; The virtlockd daemon maintains file locks on behalf of libvirtd and any VMs it is running. These file locks must be held for as long as any VM is running. If virtlockd itself ever quits, then it is expected that a node would be fenced/rebooted. Thus to allow for software upgrads on live systemd, virtlockd needs the

ability to re-exec() itself. Upon receipt of SIGUSR1, virtlockd will save its current live state out to a file /var/run/virtlockd-restart-exec.json It then re-exec()'s itself with exactly the same argv as it originally had, and loads the state file, reconstructing any objects as appropriate. The state file contains information about all locks held and all network services and clients currently active. An example state document is { "server": { "min_workers": 1, "max_workers": 20, "priority_workers": 0, "max_clients": 20, "keepaliveInterval": 4294967295, "keepaliveCount": 0, "keepaliveRequired": false, "services": [ { "auth": 0, "readonly": false, "nrequests_client_max": 1, "socks": [ { "fd": 6, "errfd": -1, "pid": 0, "isClient": false } ] } ], "clients": [ { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 9, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1722, "ownerId": 6, "ownerName": "f18x86_64", "ownerUUID": "97586ba9-df27-9459-c806-f016c8bbd224" } }, { "auth": 0, "readonly": false, "nrequests_max": 1, "sock": { "fd": 10, "errfd": -1, "pid": 0, "isClient": true }, "privateData": { "restricted": true, "ownerPid": 1784, "ownerId": 7, "ownerName": "f16x86_64", "ownerUUID": "7b8e5e42-b875-61e9-b981-91ad8fa46979" } } ] }, "defaultLockspace": { "resources": [ { "name": "/var/lib/libvirt/images/f16x86_64.raw", "path": "/var/lib/libvirt/images/f16x86_64.raw", "fd": 14, "lockHeld": true, "flags": 0, "owners": [ 1784 ] }, { "name": "/var/lib/libvirt/images/shared.img", "path": "/var/lib/libvirt/images/shared.img", "fd": 12, "lockHeld": true, "flags": 1, "owners": [ 1722, 1784 ] }, { "name": "/var/lib/libvirt/images/f18x86_64.img", "path": "/var/lib/libvirt/images/f18x86_64.img", "fd": 11, "lockHeld": true, "flags": 0, "owners": [ 1722 ] } ] }, "lockspaces": [ ], "magic": "30199" } Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- libvirt.spec.in | 5 +- src/locking/lock_daemon.c | 417 ++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 408 insertions(+), 14 deletions(-)

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)))

+ 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);