On 12.09.2012 18:29, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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);
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list