From nobody Wed May 14 21:35:02 2025 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1516714306038309.6006211483003; Tue, 23 Jan 2018 05:31:46 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D35FD53144; Tue, 23 Jan 2018 13:31:44 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id A24977BB4E; Tue, 23 Jan 2018 13:31:44 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 5E30C1800B69; Tue, 23 Jan 2018 13:31:44 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id w0NDOfvE027077 for ; Tue, 23 Jan 2018 08:24:41 -0500 Received: by smtp.corp.redhat.com (Postfix) id 022B87B14E; Tue, 23 Jan 2018 13:24:41 +0000 (UTC) Received: from t460.redhat.com (unknown [10.33.36.82]) by smtp.corp.redhat.com (Postfix) with ESMTP id EA2C47BB41; Tue, 23 Jan 2018 13:24:39 +0000 (UTC) From: "Daniel P. Berrange" To: libvir-list@redhat.com Date: Tue, 23 Jan 2018 13:23:45 +0000 Message-Id: <20180123132347.21944-10-berrange@redhat.com> In-Reply-To: <20180123132347.21944-1-berrange@redhat.com> References: <20180123132347.21944-1-berrange@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH v2 09/11] rpc: refactor virNetServer setup for post-exec restarts X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.25]); Tue, 23 Jan 2018 13:31:45 +0000 (UTC) X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" With the current code it is neccessary to call virNetDaemonNewPostExecRestart() and then for each server that needs restarting you are supposed to call virNetDaemonAddSeverPostExecRestart() This is fine if there's only ever one server, but as soon as you have two servers it is impossible to use this design. The code has no idea which servers were recorded in the JSON state doc, nor in which order the hash table serialized its keys. So this patch changes things so that we only call virNetDaemonNewPostExecRestart() passing in a callback, which is invoked once for each server found int he JSON state doc. Signed-off-by: Daniel P. Berrange --- src/libvirt_remote.syms | 1 - src/locking/lock_daemon.c | 41 +++++++++---- src/logging/log_daemon.c | 41 +++++++++---- src/rpc/virnetdaemon.c | 151 ++++++++++++++++++++++++------------------= ---- src/rpc/virnetdaemon.h | 18 +++--- tests/virnetdaemontest.c | 37 ++++++++++-- 6 files changed, 177 insertions(+), 112 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index fab6ab9dff..b75cbb99b2 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -61,7 +61,6 @@ virNetClientStreamSetError; =20 # rpc/virnetdaemon.h virNetDaemonAddServer; -virNetDaemonAddServerPostExec; virNetDaemonAddShutdownInhibition; virNetDaemonAddSignalHandler; virNetDaemonAutoShutdown; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 6751b57bc5..b1f0665aaa 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -192,15 +192,38 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool = privileged) } =20 =20 +static virNetServerPtr +virLockDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + if (STREQ(name, "virtlockd")) { + return virNetServerNewPostExecRestart(object, + name, + virLockDaemonClientNew, + virLockDaemonClientNewPostEx= ecRestart, + virLockDaemonClientPreExecRe= start, + virLockDaemonClientFree, + opaque); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected server name '%s' during restart"), + name); + return NULL; + } +} + + static virLockDaemonPtr virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) { virLockDaemonPtr lockd; virJSONValuePtr child; virJSONValuePtr lockspaces; - virNetServerPtr srv; size_t i; ssize_t n; + const char *serverNames[] =3D { "virtlockd" }; =20 if (VIR_ALLOC(lockd) < 0) return NULL; @@ -267,18 +290,12 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr objec= t, bool privileged) } } =20 - if (!(lockd->dmn =3D virNetDaemonNewPostExecRestart(child))) - goto error; - - if (!(srv =3D virNetDaemonAddServerPostExec(lockd->dmn, - "virtlockd", - virLockDaemonClientNew, - virLockDaemonClientNewPostEx= ecRestart, - virLockDaemonClientPreExecRe= start, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged= ? 0x1 : 0x0)))) + if (!(lockd->dmn =3D virNetDaemonNewPostExecRestart(child, + ARRAY_CARDINALITY(se= rverNames), + serverNames, + virLockDaemonNewServ= erPostExecRestart, + (void*)(intptr_t)(pr= ivileged ? 0x1 : 0x0)))) goto error; - virObjectUnref(srv); =20 return lockd; =20 diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 7e8c9cfc29..33133af2af 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -188,13 +188,36 @@ virLogDaemonGetHandler(virLogDaemonPtr dmn) } =20 =20 +static virNetServerPtr +virLogDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + if (STREQ(name, "virtlogd")) { + return virNetServerNewPostExecRestart(object, + name, + virLogDaemonClientNew, + virLogDaemonClientNewPostExe= cRestart, + virLogDaemonClientPreExecRes= tart, + virLogDaemonClientFree, + opaque); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected server name '%s' during restart"), + name); + return NULL; + } +} + + static virLogDaemonPtr virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged, virLogDaemonConfigPtr config) { virLogDaemonPtr logd; - virNetServerPtr srv; virJSONValuePtr child; + const char *serverNames[] =3D { "virtlogd" }; =20 if (VIR_ALLOC(logd) < 0) return NULL; @@ -212,18 +235,12 @@ virLogDaemonNewPostExecRestart(virJSONValuePtr object= , bool privileged, goto error; } =20 - if (!(logd->dmn =3D virNetDaemonNewPostExecRestart(child))) - goto error; - - if (!(srv =3D virNetDaemonAddServerPostExec(logd->dmn, - "virtlogd", - virLogDaemonClientNew, - virLogDaemonClientNewPostExe= cRestart, - virLogDaemonClientPreExecRes= tart, - virLogDaemonClientFree, - (void*)(intptr_t)(privileged= ? 0x1 : 0x0)))) + if (!(logd->dmn =3D virNetDaemonNewPostExecRestart(child, + ARRAY_CARDINALITY(ser= verNames), + serverNames, + virLogDaemonNewServer= PostExecRestart, + (void*)(intptr_t)(pri= vileged ? 0x1 : 0x0)))) goto error; - virObjectUnref(srv); =20 if (!(child =3D virJSONValueObjectGet(object, "handler"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 5d61a255c6..6f00bfd9d1 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -262,85 +262,38 @@ virNetDaemonGetServers(virNetDaemonPtr dmn, } =20 =20 -virNetServerPtr -virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, - const char *serverName, - virNetServerClientPrivNew clientPrivNew, - virNetServerClientPrivNewPostExecRestart cli= entPrivNewPostExecRestart, - virNetServerClientPrivPreExecRestart clientP= rivPreExecRestart, - virFreeCallback clientPrivFree, - void *clientPrivOpaque) -{ - virJSONValuePtr object =3D NULL; - virNetServerPtr srv =3D NULL; - - virObjectLock(dmn); - - if (!dmn->srvObject) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot add more servers post-exec than " - "there were pre-exec")); - goto error; - } - - if (virJSONValueIsArray(dmn->srvObject)) { - object =3D virJSONValueArraySteal(dmn->srvObject, 0); - if (virJSONValueArraySize(dmn->srvObject) =3D=3D 0) { - virJSONValueFree(dmn->srvObject); - dmn->srvObject =3D NULL; - } - } else if (virJSONValueObjectGetByType(dmn->srvObject, - "min_workers", - VIR_JSON_TYPE_NUMBER)) { - object =3D dmn->srvObject; - dmn->srvObject =3D NULL; - } else { - int ret =3D virJSONValueObjectRemoveKey(dmn->srvObject, - serverName, - &object); - if (ret !=3D 1) { - if (ret =3D=3D 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Server '%s' not found in JSON"), serverN= ame); - } - goto error; - } - - if (virJSONValueObjectKeysNumber(dmn->srvObject) =3D=3D 0) { - virJSONValueFree(dmn->srvObject); - dmn->srvObject =3D NULL; - } - } +struct virNetDaemonServerData { + virNetDaemonPtr dmn; + virNetDaemonNewServerPostExecRestart cb; + void *opaque; +}; =20 - srv =3D virNetServerNewPostExecRestart(object, - serverName, - clientPrivNew, - clientPrivNewPostExecRestart, - clientPrivPreExecRestart, - clientPrivFree, - clientPrivOpaque); +static int +virNetDaemonServerIterator(const char *key, + virJSONValuePtr value, + void *opaque) +{ + struct virNetDaemonServerData *data =3D opaque; + virNetServerPtr srv; =20 + VIR_DEBUG("Creating server '%s'", key); + srv =3D data->cb(data->dmn, key, value, data->opaque); if (!srv) - goto error; - - if (virHashAddEntry(dmn->servers, serverName, srv) < 0) - goto error; - virObjectRef(srv); + return -1; =20 - virJSONValueFree(object); - virObjectUnlock(dmn); - return srv; + if (virHashAddEntry(data->dmn->servers, key, srv) < 0) + return -1; =20 - error: - virObjectUnlock(dmn); - virObjectUnref(srv); - virJSONValueFree(object); - return NULL; + return 0; } =20 =20 virNetDaemonPtr -virNetDaemonNewPostExecRestart(virJSONValuePtr object) +virNetDaemonNewPostExecRestart(virJSONValuePtr object, + size_t nDefServerNames, + const char **defServerNames, + virNetDaemonNewServerPostExecRestart cb, + void *opaque) { virNetDaemonPtr dmn =3D NULL; virJSONValuePtr servers =3D virJSONValueObjectGet(object, "servers"); @@ -355,10 +308,64 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object) goto error; } =20 - if (!(dmn->srvObject =3D virJSONValueCopy(new_version ? servers : obje= ct))) - goto error; + if (!new_version) { + virNetServerPtr srv; + + if (nDefServerNames < 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No default server names provided")); + goto error; + } + + VIR_DEBUG("No 'servers' data, creating default '%s' name", defServ= erNames[0]); + + srv =3D cb(dmn, defServerNames[0], object, opaque); + + if (virHashAddEntry(dmn->servers, defServerNames[0], srv) < 0) + goto error; + } else if (virJSONValueIsArray(servers)) { + size_t i; + ssize_t n =3D virJSONValueArraySize(servers); + if (n < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Server count %zd should be positive"), n); + goto error; + } + if (n > nDefServerNames) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Server count %zd greater than default name c= ount %zu"), + n, nDefServerNames); + goto error; + } + + for (i =3D 0; i < n; i++) { + virNetServerPtr srv; + virJSONValuePtr value =3D virJSONValueArrayGet(servers, i); + + VIR_DEBUG("Creating server '%s'", defServerNames[i]); + srv =3D cb(dmn, defServerNames[i], value, opaque); + if (!srv) + goto error; + + if (virHashAddEntry(dmn->servers, defServerNames[i], srv) < 0)= { + virObjectUnref(srv); + goto error; + } + } + } else { + struct virNetDaemonServerData data =3D { + dmn, + cb, + opaque, + }; + if (virJSONValueObjectForeachKeyValue(servers, + virNetDaemonServerIterator, + &data) < 0) + goto error; + } =20 return dmn; + error: virObjectUnref(dmn); return NULL; diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 72c1df69b4..6576c463b5 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -40,15 +40,15 @@ virNetDaemonPtr virNetDaemonNew(void); int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr srv); =20 -virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, - const char *serverName, - virNetServerClientPrivNew cl= ientPrivNew, - virNetServerClientPrivNewPos= tExecRestart clientPrivNewPostExecRestart, - virNetServerClientPrivPreExe= cRestart clientPrivPreExecRestart, - virFreeCallback clientPrivFr= ee, - void *clientPrivOpaque); - -virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object); +typedef virNetServerPtr (*virNetDaemonNewServerPostExecRestart)(virNetDaem= onPtr dmn, + const char= *name, + virJSONVal= uePtr object, + void *opaq= ue); +virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object, + size_t nDefServerNames, + const char **defServerNames, + virNetDaemonNewServerPostEx= ecRestart cb, + void *opaque); =20 virJSONValuePtr virNetDaemonPreExecRestart(virNetDaemonPtr dmn); =20 diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 3e60f09007..435513d314 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -194,12 +194,32 @@ struct testExecRestartData { bool pass; }; =20 +static virNetServerPtr +testNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + struct testExecRestartData *data =3D opaque; + size_t i; + for (i =3D 0; i < data->nservers; i++) { + if (STREQ(data->serverNames[i], name)) { + return virNetServerNewPostExecRestart(object, + name, + NULL, NULL, NULL, + NULL, NULL); + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, "Unexpected server name '%s'", = name); + return NULL; +} + static int testExecRestart(const void *opaque) { size_t i; int ret =3D -1; virNetDaemonPtr dmn =3D NULL; - virNetServerPtr srv =3D NULL; const struct testExecRestartData *data =3D opaque; char *infile =3D NULL, *outfile =3D NULL; char *injsonstr =3D NULL, *outjsonstr =3D NULL; @@ -241,15 +261,20 @@ static int testExecRestart(const void *opaque) if (!(injson =3D virJSONValueFromString(injsonstr))) goto cleanup; =20 - if (!(dmn =3D virNetDaemonNewPostExecRestart(injson))) + if (!(dmn =3D virNetDaemonNewPostExecRestart(injson, + data->nservers, + data->serverNames, + testNewServerPostExecRestar= t, + (void *)data))) goto cleanup; =20 for (i =3D 0; i < data->nservers; i++) { - if (!(srv =3D virNetDaemonAddServerPostExec(dmn, data->serverNames= [i], - NULL, NULL, NULL, - NULL, NULL))) + if (!virNetDaemonHasServer(dmn, data->serverNames[i])) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Server %s was not created", + data->serverNames[i]); goto cleanup; - srv =3D NULL; + } } =20 if (!(outjson =3D virNetDaemonPreExecRestart(dmn))) --=20 2.14.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list