On 12.09.2012 18:28, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> Add two new APIs virLockSpaceNewPostExecRestart and
> virLockSpacePreExecRestart which allow a virLockSpacePtr
> object to be created from a JSON object and saved to a
> JSON object, for the purposes of re-exec'ing a process.
>
> As well as saving the state in JSON format, the second
> method will disable the O_CLOEXEC flag so that the open
> file descriptors are preserved across the process re-exec()
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> src/libvirt_private.syms | 2 +
> src/util/virlockspace.c | 236 +++++++++++++++++++++++++++++++++++++++++++++++
> src/util/virlockspace.h | 4 +
> 3 files changed, 242 insertions(+)
>
ACK but see my comments below
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 58a9520..6c94584 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1328,6 +1328,8 @@ virLockSpaceDeleteResource;
> virLockSpaceFree;
> virLockSpaceGetDirectory;
> virLockSpaceNew;
> +virLockSpaceNewPostExecRestart;
> +virLockSpacePreExecRestart;
> virLockSpaceReleaseResource;
> virLockSpaceReleaseResourcesForOwner;
>
> diff --git a/src/util/virlockspace.c b/src/util/virlockspace.c
> index 611592a..786513f 100644
> --- a/src/util/virlockspace.c
> +++ b/src/util/virlockspace.c
> @@ -295,6 +295,242 @@ error:
> }
>
>
> +
> +virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object)
> +{
> + virLockSpacePtr lockspace;
> + virJSONValuePtr resources;
> + int n;
> + size_t i;
> +
> + VIR_DEBUG("object=%p", object);
> +
> + 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 (!(lockspace->resources = virHashCreate(10,
> + virLockSpaceResourceDataFree)))
s/10/VIR_LOCK_SPACE_RESOURCE_HASH_TABLE_SIZE/ or the name you invent
> + goto error;
> +
> + if (virJSONValueObjectHasKey(object, "directory")) {
> + const char *dir = virJSONValueObjectGetString(object, "directory");
> + if (!(lockspace->dir = strdup(dir))) {
> + virReportOOMError();
> + goto error;
> + }
> + }
> +
> + if (!(resources = virJSONValueObjectGet(object, "resources"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resources value in JSON document"));
> + goto error;
> + }
> +
> + if ((n = virJSONValueArraySize(resources)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Malformed resources value in JSON document"));
> + goto error;
> + }
> +
> + for (i = 0 ; i < n ; i++) {
> + virJSONValuePtr child = virJSONValueArrayGet(resources, i);
> + virLockSpaceResourcePtr res;
> + const char *tmp;
> + virJSONValuePtr owners;
> + size_t j;
> + int m;
> +
> + if (VIR_ALLOC(res) < 0) {
> + virReportOOMError();
> + goto error;
> + }
> + res->fd = -1;
This is useless ...
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource name in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> + if (!(res->name = strdup(tmp))) {
> + virReportOOMError();
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
> + if (!(tmp = virJSONValueObjectGetString(child, "path"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource path in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> + if (!(res->path = strdup(tmp))) {
> + virReportOOMError();
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> + if (virJSONValueObjectGetNumberInt(child, "fd", &res->fd) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource fd in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
... since we require 'fd' attribute anyway.
> + if (virSetInherit(res->fd, false) < 0) {
> + virReportSystemError(errno, "%s",
> + _("Cannot enable close-on-exec flag"));
> + goto error;
> + }
> + if (virJSONValueObjectGetBoolean(child, "lockHeld", &res->lockHeld) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource lockHeld in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
> + if (virJSONValueObjectGetNumberUint(child, "flags", &res->flags) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource flags in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
> + if (!(owners = virJSONValueObjectGet(child, "owners"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Missing resource owners in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
> + if ((m = virJSONValueArraySize(owners)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Malformed owners value in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
Since we already know the res->owners size, wouldn't it be better to
allocate it here instead of ...
> + for (j = 0 ; j < m ; j++) {
> + unsigned long long int owner;
> + virJSONValuePtr ownerval = virJSONValueArrayGet(owners, j);
> +
> + if (virJSONValueGetNumberUlong(ownerval, &owner) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("Malformed owner value in JSON document"));
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> +
> + if (VIR_EXPAND_N(res->owners, res->nOwners, 1) < 0) {
> + virReportOOMError();
> + virLockSpaceResourceFree(res);
> + goto error;
... doing this?
> + }
> +
> + res->owners[res->nOwners-1] = (pid_t)owner;
> + }
> +
> + if (virHashAddEntry(lockspace->resources, res->name, res) < 0) {
> + virLockSpaceResourceFree(res);
> + goto error;
> + }
> + }
> +
> + return lockspace;
> +
> +error:
> + virLockSpaceFree(lockspace);
> + return NULL;
> +}
> +
> +
> +virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace)
> +{
> + virJSONValuePtr object = virJSONValueNewObject();
> + virJSONValuePtr resources;
> + virHashKeyValuePairPtr pairs = NULL, tmp;
> +
> + if (!object)
> + return NULL;
> +
> + virMutexLock(&lockspace->lock);
> +
> + if (lockspace->dir &&
> + virJSONValueObjectAppendString(object, "directory", lockspace->dir) < 0)
> + goto error;
> +
> + if (!(resources = virJSONValueNewArray()))
> + goto error;
> +
> + if (virJSONValueObjectAppend(object, "resources", resources) < 0) {
> + virJSONValueFree(resources);
> + goto error;
> + }
> +
> + tmp = pairs = virHashGetItems(lockspace->resources, NULL);
> + while (tmp && tmp->value) {
> + virLockSpaceResourcePtr res = (virLockSpaceResourcePtr)tmp->value;
> + virJSONValuePtr child = virJSONValueNewObject();
> + virJSONValuePtr owners = NULL;
> + size_t i;
> +
> + if (virJSONValueArrayAppend(resources, child) < 0) {
> + virJSONValueFree(child);
> + goto error;
> + }
> +
> + if (virJSONValueObjectAppendString(child, "name", res->name) < 0 ||
> + virJSONValueObjectAppendString(child, "path", res->path) < 0 ||
> + virJSONValueObjectAppendNumberInt(child, "fd", res->fd) < 0 ||
> + virJSONValueObjectAppendBoolean(child, "lockHeld", res->lockHeld) < 0 ||
> + virJSONValueObjectAppendNumberUint(child, "flags", res->flags) < 0)
> + goto error;
> +
> + if (virSetInherit(res->fd, true) < 0) {
> + virReportSystemError(errno, "%s",
> + _("Cannot disable close-on-exec flag"));
> + goto error;
> + }
> +
> + if (!(owners = virJSONValueNewArray()))
> + goto error;
> +
> + if (virJSONValueObjectAppend(child, "owners", owners) < 0) {
> + virJSONValueFree(owners);
> + goto error;
> + }
> +
> + for (i = 0 ; i < res->nOwners ; i++) {
> + virJSONValuePtr owner = virJSONValueNewNumberUlong(res->owners[i]);
> + if (!owner)
> + goto error;
> +
> + if (virJSONValueArrayAppend(owners, owner) < 0) {
> + virJSONValueFree(owner);
> + goto error;
> + }
> + }
> +
> + tmp++;
> + }
> + VIR_FREE(pairs);
> +
> + virMutexUnlock(&lockspace->lock);
> + return object;
> +
> + error:
> + VIR_FREE(pairs);
> + virJSONValueFree(object);
> + virMutexUnlock(&lockspace->lock);
> + return NULL;
> +}
> +
> +
> void virLockSpaceFree(virLockSpacePtr lockspace)
> {
> if (!lockspace)
> diff --git a/src/util/virlockspace.h b/src/util/virlockspace.h
> index bd8f91c..9c5128b 100644
> --- a/src/util/virlockspace.h
> +++ b/src/util/virlockspace.h
> @@ -23,11 +23,15 @@
> # define __VIR_LOCK_SPACE_H__
>
> # include "internal.h"
> +# include "json.h"
>
> typedef struct _virLockSpace virLockSpace;
> typedef virLockSpace *virLockSpacePtr;
>
> virLockSpacePtr virLockSpaceNew(const char *directory);
> +virLockSpacePtr virLockSpaceNewPostExecRestart(virJSONValuePtr object);
> +
> +virJSONValuePtr virLockSpacePreExecRestart(virLockSpacePtr lockspace);
>
> void virLockSpaceFree(virLockSpacePtr lockspace);
>
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list