1:52 p.m.
Jiri Denemark (4):
virthreadpool: Copy job name
virnetserver: Format functions consistently
virnetserver: Use autoptr for virNetServer and virNetServerClient
virnetserver: Make pool job name less generic
src/rpc/virnetserver.c | 247 ++++++++++++++++++++---------------
src/rpc/virnetserverclient.h | 2 +
src/util/virthreadpool.c | 5 +-
3 files changed, 145 insertions(+), 109 deletions(-)
--
2.34.0
1:52 p.m.
New subject: [libvirt PATCH 1/4] virthreadpool: Copy job name
Currently virThreadPoolNewFull relies on the caller to ensure the job
name outlives the thread pool. Which basically enforces static strings.
Let's drop this implicit requirement by making a copy of the job name.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/util/virthreadpool.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c
index 7bf4333885..426840e435 100644
--- a/src/util/virthreadpool.c
+++ b/src/util/virthreadpool.c
@@ -50,7 +50,7 @@ struct _virThreadPool {
bool quit;
virThreadPoolJobFunc jobFunc;
- const char *jobName;
+ char *jobName;
void *jobOpaque;
virThreadPoolJobList jobList;
size_t jobQueueDepth;
@@ -237,7 +237,7 @@ virThreadPoolNewFull(size_t minWorkers,
pool->jobList.tail = pool->jobList.head = NULL;
pool->jobFunc = func;
- pool->jobName = name;
+ pool->jobName = g_strdup(name);
pool->jobOpaque = opaque;
if (identity)
@@ -312,6 +312,7 @@ void virThreadPoolFree(virThreadPool *pool)
if (pool->identity)
g_object_unref(pool->identity);
+ g_free(pool->jobName);
g_free(pool->workers);
virMutexUnlock(&pool->mutex);
virMutexDestroy(&pool->mutex);
--
2.34.0
1:52 p.m.
New subject: [libvirt PATCH 2/4] virnetserver: Format functions consistently
The file used a pretty inconsistent style for formatting function
headers. Return types were both separate and on the same line as
function names and functions were separated by one, two, and sometimes
even three empty lines. Let's make it consistent by honoring our
preferred coding style.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/rpc/virnetserver.c | 180 +++++++++++++++++++++++++----------------
1 file changed, 112 insertions(+), 68 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index c7b4939398..f016f15f39 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -82,7 +82,8 @@ static void virNetServerUpdateServicesLocked(virNetServer *srv,
static inline size_t virNetServerTrackPendingAuthLocked(virNetServer *srv);
static inline size_t virNetServerTrackCompletedAuthLocked(virNetServer *srv);
-static int virNetServerOnceInit(void)
+static int
+virNetServerOnceInit(void)
{
if (!VIR_CLASS_NEW(virNetServer, virClassForObjectLockable()))
return -1;
@@ -92,7 +93,9 @@ static int virNetServerOnceInit(void)
VIR_ONCE_GLOBAL_INIT(virNetServer);
-unsigned long long virNetServerNextClientID(virNetServer *srv)
+
+unsigned long long
+virNetServerNextClientID(virNetServer *srv)
{
unsigned long long val;
@@ -103,10 +106,12 @@ unsigned long long virNetServerNextClientID(virNetServer *srv)
return val;
}
-static int virNetServerProcessMsg(virNetServer *srv,
- virNetServerClient *client,
- virNetServerProgram *prog,
- virNetMessage *msg)
+
+static int
+virNetServerProcessMsg(virNetServer *srv,
+ virNetServerClient *client,
+ virNetServerProgram *prog,
+ virNetMessage *msg)
{
if (!prog) {
/* Only send back an error for type == CALL. Other
@@ -141,7 +146,10 @@ static int virNetServerProcessMsg(virNetServer *srv,
return 0;
}
-static void virNetServerHandleJob(void *jobOpaque, void *opaque)
+
+static void
+virNetServerHandleJob(void *jobOpaque,
+ void *opaque)
{
virNetServer *srv = opaque;
virNetServerJob *job = jobOpaque;
@@ -165,6 +173,7 @@ static void virNetServerHandleJob(void *jobOpaque, void *opaque)
VIR_FREE(job);
}
+
/**
* virNetServerGetProgramLocked:
* @srv: server (must be locked by the caller)
@@ -186,6 +195,7 @@ virNetServerGetProgramLocked(virNetServer *srv,
return NULL;
}
+
static void
virNetServerDispatchNewMessage(virNetServerClient *client,
virNetMessage *msg,
@@ -289,8 +299,10 @@ virNetServerCheckLimits(virNetServer *srv)
}
}
-int virNetServerAddClient(virNetServer *srv,
- virNetServerClient *client)
+
+int
+virNetServerAddClient(virNetServer *srv,
+ virNetServerClient *client)
{
virObjectLock(srv);
@@ -323,9 +335,11 @@ int virNetServerAddClient(virNetServer *srv,
return -1;
}
-static int virNetServerDispatchNewClient(virNetServerService *svc,
- virNetSocket *clientsock,
- void *opaque)
+
+static int
+virNetServerDispatchNewClient(virNetServerService *svc,
+ virNetSocket *clientsock,
+ void *opaque)
{
virNetServer *srv = opaque;
virNetServerClient *client;
@@ -352,19 +366,20 @@ static int virNetServerDispatchNewClient(virNetServerService *svc,
}
-virNetServer *virNetServerNew(const char *name,
- unsigned long long next_client_id,
- size_t min_workers,
- size_t max_workers,
- size_t priority_workers,
- size_t max_clients,
- size_t max_anonymous_clients,
- int keepaliveInterval,
- unsigned int keepaliveCount,
- virNetServerClientPrivNew clientPrivNew,
- virNetServerClientPrivPreExecRestart
clientPrivPreExecRestart,
- virFreeCallback clientPrivFree,
- void *clientPrivOpaque)
+virNetServer *
+virNetServerNew(const char *name,
+ unsigned long long next_client_id,
+ size_t min_workers,
+ size_t max_workers,
+ size_t priority_workers,
+ size_t max_clients,
+ size_t max_anonymous_clients,
+ int keepaliveInterval,
+ unsigned int keepaliveCount,
+ virNetServerClientPrivNew clientPrivNew,
+ virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
+ virFreeCallback clientPrivFree,
+ void *clientPrivOpaque)
{
virNetServer *srv;
@@ -401,13 +416,14 @@ virNetServer *virNetServerNew(const char *name,
}
-virNetServer *virNetServerNewPostExecRestart(virJSONValue *object,
- const char *name,
- virNetServerClientPrivNew clientPrivNew,
- virNetServerClientPrivNewPostExecRestart
clientPrivNewPostExecRestart,
- virNetServerClientPrivPreExecRestart
clientPrivPreExecRestart,
- virFreeCallback clientPrivFree,
- void *clientPrivOpaque)
+virNetServer *
+virNetServerNewPostExecRestart(virJSONValue *object,
+ const char *name,
+ virNetServerClientPrivNew clientPrivNew,
+ virNetServerClientPrivNewPostExecRestart
clientPrivNewPostExecRestart,
+ virNetServerClientPrivPreExecRestart
clientPrivPreExecRestart,
+ virFreeCallback clientPrivFree,
+ void *clientPrivOpaque)
{
virNetServer *srv = NULL;
virJSONValue *clients;
@@ -553,7 +569,8 @@ virNetServer *virNetServerNewPostExecRestart(virJSONValue *object,
}
-virJSONValue *virNetServerPreExecRestart(virNetServer *srv)
+virJSONValue *
+virNetServerPreExecRestart(virNetServer *srv)
{
g_autoptr(virJSONValue) object = virJSONValueNewObject();
g_autoptr(virJSONValue) clients = virJSONValueNewArray();
@@ -621,9 +638,9 @@ virJSONValue *virNetServerPreExecRestart(virNetServer *srv)
}
-
-int virNetServerAddService(virNetServer *srv,
- virNetServerService *svc)
+int
+virNetServerAddService(virNetServer *srv,
+ virNetServerService *svc)
{
virObjectLock(srv);
@@ -687,17 +704,18 @@ virNetServerAddServiceActivation(virNetServer *srv,
}
-int virNetServerAddServiceTCP(virNetServer *srv,
- virSystemdActivation *act,
- const char *actname,
- const char *nodename,
- const char *service,
- int family,
- int auth,
- virNetTLSContext *tls,
- bool readonly,
- size_t max_queued_clients,
- size_t nrequests_client_max)
+int
+virNetServerAddServiceTCP(virNetServer *srv,
+ virSystemdActivation *act,
+ const char *actname,
+ const char *nodename,
+ const char *service,
+ int family,
+ int auth,
+ virNetTLSContext *tls,
+ bool readonly,
+ size_t max_queued_clients,
+ size_t nrequests_client_max)
{
virNetServerService *svc = NULL;
int ret;
@@ -735,17 +753,18 @@ int virNetServerAddServiceTCP(virNetServer *srv,
}
-int virNetServerAddServiceUNIX(virNetServer *srv,
- virSystemdActivation *act,
- const char *actname,
- const char *path,
- mode_t mask,
- gid_t grp,
- int auth,
- virNetTLSContext *tls,
- bool readonly,
- size_t max_queued_clients,
- size_t nrequests_client_max)
+int
+virNetServerAddServiceUNIX(virNetServer *srv,
+ virSystemdActivation *act,
+ const char *actname,
+ const char *path,
+ mode_t mask,
+ gid_t grp,
+ int auth,
+ virNetTLSContext *tls,
+ bool readonly,
+ size_t max_queued_clients,
+ size_t nrequests_client_max)
{
virNetServerService *svc = NULL;
int ret;
@@ -783,8 +802,9 @@ int virNetServerAddServiceUNIX(virNetServer *srv,
}
-int virNetServerAddProgram(virNetServer *srv,
- virNetServerProgram *prog)
+int
+virNetServerAddProgram(virNetServer *srv,
+ virNetServerProgram *prog)
{
virObjectLock(srv);
@@ -795,8 +815,10 @@ int virNetServerAddProgram(virNetServer *srv,
return 0;
}
-int virNetServerSetTLSContext(virNetServer *srv,
- virNetTLSContext *tls)
+
+int
+virNetServerSetTLSContext(virNetServer *srv,
+ virNetTLSContext *tls)
{
srv->tls = virObjectRef(tls);
return 0;
@@ -856,15 +878,18 @@ virNetServerUpdateServicesLocked(virNetServer *srv,
}
-void virNetServerUpdateServices(virNetServer *srv,
- bool enabled)
+void
+virNetServerUpdateServices(virNetServer *srv,
+ bool enabled)
{
virObjectLock(srv);
virNetServerUpdateServicesLocked(srv, enabled);
virObjectUnlock(srv);
}
-void virNetServerDispose(void *obj)
+
+void
+virNetServerDispose(void *obj)
{
virNetServer *srv = obj;
size_t i;
@@ -886,7 +911,9 @@ void virNetServerDispose(void *obj)
g_free(srv->clients);
}
-void virNetServerClose(virNetServer *srv)
+
+void
+virNetServerClose(virNetServer *srv)
{
size_t i;
@@ -906,18 +933,21 @@ void virNetServerClose(virNetServer *srv)
virObjectUnlock(srv);
}
+
void
virNetServerShutdownWait(virNetServer *srv)
{
virThreadPoolDrain(srv->workers);
}
+
static inline size_t
virNetServerTrackPendingAuthLocked(virNetServer *srv)
{
return ++srv->nclients_unauth;
}
+
static inline size_t
virNetServerTrackCompletedAuthLocked(virNetServer *srv)
{
@@ -937,6 +967,7 @@ virNetServerHasClients(virNetServer *srv)
return ret;
}
+
void
virNetServerProcessClients(virNetServer *srv)
{
@@ -974,12 +1005,14 @@ virNetServerProcessClients(virNetServer *srv)
virObjectUnlock(srv);
}
+
const char *
virNetServerGetName(virNetServer *srv)
{
return srv->name;
}
+
int
virNetServerGetThreadPoolParameters(virNetServer *srv,
size_t *minWorkers,
@@ -1002,6 +1035,7 @@ virNetServerGetThreadPoolParameters(virNetServer *srv,
return 0;
}
+
int
virNetServerSetThreadPoolParameters(virNetServer *srv,
long long int minWorkers,
@@ -1018,6 +1052,7 @@ virNetServerSetThreadPoolParameters(virNetServer *srv,
return ret;
}
+
size_t
virNetServerGetMaxClients(virNetServer *srv)
{
@@ -1030,6 +1065,7 @@ virNetServerGetMaxClients(virNetServer *srv)
return ret;
}
+
size_t
virNetServerGetCurrentClients(virNetServer *srv)
{
@@ -1042,6 +1078,7 @@ virNetServerGetCurrentClients(virNetServer *srv)
return ret;
}
+
size_t
virNetServerGetMaxUnauthClients(virNetServer *srv)
{
@@ -1054,6 +1091,7 @@ virNetServerGetMaxUnauthClients(virNetServer *srv)
return ret;
}
+
size_t
virNetServerGetCurrentUnauthClients(virNetServer *srv)
{
@@ -1067,8 +1105,9 @@ virNetServerGetCurrentUnauthClients(virNetServer *srv)
}
-bool virNetServerNeedsAuth(virNetServer *srv,
- int auth)
+bool
+virNetServerNeedsAuth(virNetServer *srv,
+ int auth)
{
bool ret = false;
size_t i;
@@ -1083,6 +1122,7 @@ bool virNetServerNeedsAuth(virNetServer *srv,
return ret;
}
+
int
virNetServerGetClients(virNetServer *srv,
virNetServerClient ***clts)
@@ -1105,6 +1145,7 @@ virNetServerGetClients(virNetServer *srv,
return nclients;
}
+
virNetServerClient *
virNetServerGetClient(virNetServer *srv,
unsigned long long id)
@@ -1128,6 +1169,7 @@ virNetServerGetClient(virNetServer *srv,
return ret;
}
+
int
virNetServerSetClientLimits(virNetServer *srv,
long long int maxClients,
@@ -1164,6 +1206,7 @@ virNetServerSetClientLimits(virNetServer *srv,
return ret;
}
+
static virNetTLSContext *
virNetServerGetTLSContext(virNetServer *srv)
{
@@ -1182,6 +1225,7 @@ virNetServerGetTLSContext(virNetServer *srv)
return ctxt;
}
+
int
virNetServerUpdateTlsFiles(virNetServer *srv)
{
--
2.34.0
1:52 p.m.
New subject: [libvirt PATCH 3/4] virnetserver: Use autoptr for virNetServer and virNetServerClient
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/rpc/virnetserver.c | 64 +++++++++++++++---------------------
src/rpc/virnetserverclient.h | 2 ++
2 files changed, 28 insertions(+), 38 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index f016f15f39..d0f248e7f5 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -342,7 +342,7 @@ virNetServerDispatchNewClient(virNetServerService *svc,
void *opaque)
{
virNetServer *srv = opaque;
- virNetServerClient *client;
+ g_autoptr(virNetServerClient) client = NULL;
if (!(client = virNetServerClientNew(virNetServerNextClientID(srv),
clientsock,
@@ -358,10 +358,8 @@ virNetServerDispatchNewClient(virNetServerService *svc,
if (virNetServerAddClient(srv, client) < 0) {
virNetServerClientClose(client);
- virObjectUnref(client);
return -1;
}
- virObjectUnref(client);
return 0;
}
@@ -381,7 +379,7 @@ virNetServerNew(const char *name,
virFreeCallback clientPrivFree,
void *clientPrivOpaque)
{
- virNetServer *srv;
+ g_autoptr(virNetServer) srv = NULL;
if (virNetServerInitialize() < 0)
return NULL;
@@ -395,7 +393,7 @@ virNetServerNew(const char *name,
"rpc-worker",
NULL,
srv)))
- goto error;
+ return NULL;
srv->name = g_strdup(name);
@@ -409,10 +407,7 @@ virNetServerNew(const char *name,
srv->clientPrivFree = clientPrivFree;
srv->clientPrivOpaque = clientPrivOpaque;
- return srv;
- error:
- virObjectUnref(srv);
- return NULL;
+ return g_steal_pointer(&srv);
}
@@ -425,7 +420,7 @@ virNetServerNewPostExecRestart(virJSONValue *object,
virFreeCallback clientPrivFree,
void *clientPrivOpaque)
{
- virNetServer *srv = NULL;
+ g_autoptr(virNetServer) srv = NULL;
virJSONValue *clients;
virJSONValue *services;
size_t i;
@@ -441,29 +436,29 @@ virNetServerNewPostExecRestart(virJSONValue *object,
if (virJSONValueObjectGetNumberUint(object, "min_workers",
&min_workers) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing min_workers data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectGetNumberUint(object, "max_workers",
&max_workers) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing max_workers data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectGetNumberUint(object, "priority_workers",
&priority_workers) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing priority_workers data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectGetNumberUint(object, "max_clients",
&max_clients) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing max_clients data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectHasKey(object, "max_anonymous_clients")) {
if (virJSONValueObjectGetNumberUint(object, "max_anonymous_clients",
&max_anonymous_clients) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Malformed max_anonymous_clients data in JSON
document"));
- goto error;
+ return NULL;
}
} else {
max_anonymous_clients = max_clients;
@@ -471,12 +466,12 @@ virNetServerNewPostExecRestart(virJSONValue *object,
if (virJSONValueObjectGetNumberUint(object, "keepaliveInterval",
&keepaliveInterval) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing keepaliveInterval data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectGetNumberUint(object, "keepaliveCount",
&keepaliveCount) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing keepaliveCount data in JSON document"));
- goto error;
+ return NULL;
}
if (virJSONValueObjectGetNumberUlong(object, "next_client_id",
@@ -492,18 +487,18 @@ virNetServerNewPostExecRestart(virJSONValue *object,
keepaliveInterval, keepaliveCount,
clientPrivNew, clientPrivPreExecRestart,
clientPrivFree, clientPrivOpaque)))
- goto error;
+ return NULL;
if (!(services = virJSONValueObjectGet(object, "services"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing services data in JSON document"));
- goto error;
+ return NULL;
}
if (!virJSONValueIsArray(services)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Malformed services array"));
- goto error;
+ return NULL;
}
for (i = 0; i < virJSONValueArraySize(services); i++) {
@@ -512,15 +507,15 @@ virNetServerNewPostExecRestart(virJSONValue *object,
if (!child) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing service data in JSON document"));
- goto error;
+ return NULL;
}
if (!(service = virNetServerServiceNewPostExecRestart(child)))
- goto error;
+ return NULL;
if (virNetServerAddService(srv, service) < 0) {
virObjectUnref(service);
- goto error;
+ return NULL;
}
}
@@ -528,22 +523,22 @@ virNetServerNewPostExecRestart(virJSONValue *object,
if (!(clients = virJSONValueObjectGet(object, "clients"))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing clients data in JSON document"));
- goto error;
+ return NULL;
}
if (!virJSONValueIsArray(clients)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Malformed clients array"));
- goto error;
+ return NULL;
}
for (i = 0; i < virJSONValueArraySize(clients); i++) {
- virNetServerClient *client;
+ g_autoptr(virNetServerClient) client = NULL;
virJSONValue *child = virJSONValueArrayGet(clients, i);
if (!child) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Missing client data in JSON document"));
- goto error;
+ return NULL;
}
if (!(client = virNetServerClientNewPostExecRestart(srv,
@@ -552,20 +547,13 @@ virNetServerNewPostExecRestart(virJSONValue *object,
clientPrivPreExecRestart,
clientPrivFree,
clientPrivOpaque)))
- goto error;
+ return NULL;
- if (virNetServerAddClient(srv, client) < 0) {
- virObjectUnref(client);
- goto error;
- }
- virObjectUnref(client);
+ if (virNetServerAddClient(srv, client) < 0)
+ return NULL;
}
- return srv;
-
- error:
- virObjectUnref(srv);
- return NULL;
+ return g_steal_pointer(&srv);
}
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h
index a9d0afe95a..0d585eb2ce 100644
--- a/src/rpc/virnetserverclient.h
+++ b/src/rpc/virnetserverclient.h
@@ -28,8 +28,10 @@
#include "virjson.h"
typedef struct _virNetServer virNetServer;
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetServer, virObjectUnref);
typedef struct _virNetServerClient virNetServerClient;
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetServerClient, virObjectUnref);
/* This function owns the "msg" pointer it is passed and
* must arrange for virNetMessageFree to be called on it
--
2.34.0
1:52 p.m.
New subject: [libvirt PATCH 4/4] virnetserver: Make pool job name less generic
The generic "rpc-worker" name becomes a name of the associated task,
which may than appear in logs and bring some confusion. Let's add a
server name to it so that one can easily see which daemon the task
belongs to, which is especially useful for split daemons. And since the
name would be too long, we can drop the "-worker" part and just keep it
as "rpc-*" and "prio-rpc-*".
Such confusing entries can, for example, be found in audit log when
SELinux is complaining that "rpc-worker" was denied access to something.
Signed-off-by: Jiri Denemark <jdenemar(a)redhat.com>
---
src/rpc/virnetserver.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index d0f248e7f5..ad581a36dd 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -380,6 +380,7 @@ virNetServerNew(const char *name,
void *clientPrivOpaque)
{
g_autoptr(virNetServer) srv = NULL;
+ g_autofree char *jobName = g_strdup_printf("rpc-%s", name);
if (virNetServerInitialize() < 0)
return NULL;
@@ -390,7 +391,7 @@ virNetServerNew(const char *name,
if (!(srv->workers = virThreadPoolNewFull(min_workers, max_workers,
priority_workers,
virNetServerHandleJob,
- "rpc-worker",
+ jobName,
NULL,
srv)))
return NULL;
--
2.34.0
1226
days inactive
1227
days old
5 comments
2 participants
participants (2)
-
Jiri Denemark -
Michal Prívozník