Admin API needs a way of addressing specific clients. Unlike servers, which we are happy to address by names both because its name reflects its purpose (to some extent) and we only have two of them (so far), naming clients doesn't make any sense, since a) each client is an anonymous, i.e. not recognized after a disconnect followed by a reconnect, b) we can't predict what kind of requests it's going to send to daemon, and c) the are loads of them comming and going, so the only viable option is to use an ID which is of a reasonably wide data type. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- daemon/libvirtd.c | 4 +- src/libvirt_remote.syms | 1 + src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 35 ++++++++++- src/rpc/virnetserver.h | 3 + src/rpc/virnetserverclient.c | 38 ++++++++++-- src/rpc/virnetserverclient.h | 7 ++- tests/virnetdaemondata/input-data-client-ids.json | 69 ++++++++++++++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 6 ++ .../output-data-admin-server-names.json | 6 ++ .../virnetdaemondata/output-data-anon-clients.json | 3 + tests/virnetdaemondata/output-data-client-ids.json | 69 ++++++++++++++++++++++ .../output-data-initial-nomdns.json | 3 + tests/virnetdaemondata/output-data-initial.json | 3 + .../output-data-no-keepalive-required.json | 6 ++ tests/virnetdaemontest.c | 9 ++- tests/virnetserverclienttest.c | 2 +- 19 files changed, 251 insertions(+), 19 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-client-ids.json create mode 100644 tests/virnetdaemondata/output-data-client-ids.json diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 7ec02ad..f5d1e2f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1389,7 +1389,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(srv = virNetServerNew("libvirtd", + if (!(srv = virNetServerNew("libvirtd", 1, config->min_workers, config->max_workers, config->prio_workers, @@ -1464,7 +1464,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (!(srvAdm = virNetServerNew("admin", + if (!(srvAdm = virNetServerNew("admin", 1, config->admin_min_workers, config->admin_max_workers, 0, diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 66f9383..c049874 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -105,6 +105,7 @@ virNetServerGetName; virNetServerHasClients; virNetServerNew; virNetServerNewPostExecRestart; +virNetServerNextClientID; virNetServerPreExecRestart; virNetServerProcessClients; virNetServerStart; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index bfdcfc6..f889a34 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -160,7 +160,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) return NULL; } - if (!(srv = virNetServerNew("virtlockd", + if (!(srv = virNetServerNew("virtlockd", 1, 1, 1, 0, config->max_clients, config->max_clients, -1, 0, NULL, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 70339af..90f8427 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -150,7 +150,7 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) return NULL; } - if (!(logd->srv = virNetServerNew("virtlogd", + if (!(logd->srv = virNetServerNew("virtlogd", 1, 1, 1, 0, config->max_clients, config->max_clients, -1, 0, NULL, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index de59775..0304354 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -934,7 +934,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) LXC_STATE_DIR, ctrl->name) < 0) return -1; - if (!(srv = virNetServerNew("LXC", + if (!(srv = virNetServerNew("LXC", 1, 0, 0, 0, 1, 0, -1, 0, NULL, diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 57bd95c..8216da6 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -65,6 +65,7 @@ struct _virNetServer { size_t nclients; /* Current clients count */ virNetServerClientPtr *clients; /* Clients */ + unsigned long long next_client_id; /* next client ID */ size_t nclients_max; /* Max allowed clients count */ size_t nclients_unauth; /* Unauthenticated clients count */ size_t nclients_unauth_max; /* Max allowed unauth clients count */ @@ -103,6 +104,16 @@ static int virNetServerOnceInit(void) VIR_ONCE_GLOBAL_INIT(virNetServer) +unsigned long long virNetServerNextClientID(virNetServerPtr srv) +{ + unsigned long long val; + + virObjectLock(srv); + val = srv->next_client_id++; + virObjectUnlock(srv); + + return val; +} static int virNetServerProcessMsg(virNetServerPtr srv, virNetServerClientPtr client, @@ -283,7 +294,8 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, virNetServerPtr srv = opaque; virNetServerClientPtr client; - if (!(client = virNetServerClientNew(clientsock, + if (!(client = virNetServerClientNew(virNetServerNextClientID(srv), + clientsock, virNetServerServiceGetAuth(svc), virNetServerServiceIsReadonly(svc), virNetServerServiceGetMaxRequests(svc), @@ -307,6 +319,7 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc, virNetServerPtr virNetServerNew(const char *name, + unsigned long long next_client_id, size_t min_workers, size_t max_workers, size_t priority_workers, @@ -338,6 +351,7 @@ virNetServerPtr virNetServerNew(const char *name, if (VIR_STRDUP(srv->name, name) < 0) goto error; + srv->next_client_id = next_client_id; srv->nclients_max = max_clients; srv->nclients_unauth_max = max_anonymous_clients; srv->keepaliveInterval = keepaliveInterval; @@ -384,6 +398,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int max_anonymous_clients; unsigned int keepaliveInterval; unsigned int keepaliveCount; + unsigned long long next_client_id; const char *mdnsGroupName = NULL; if (virJSONValueObjectGetNumberUint(object, "min_workers", &min_workers) < 0) { @@ -434,7 +449,13 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - if (!(srv = virNetServerNew(name, + if (virJSONValueObjectGetNumberUlong(object, "next_client_id", + &next_client_id) < 0) { + VIR_WARN("Missing next_client_id data in JSON document"); + next_client_id = 1; + } + + if (!(srv = virNetServerNew(name, next_client_id, min_workers, max_workers, priority_workers, max_clients, max_anonymous_clients, @@ -503,7 +524,8 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, clientPrivNewPostExecRestart, clientPrivPreExecRestart, clientPrivFree, - clientPrivOpaque))) + clientPrivOpaque, + srv))) goto error; if (virNetServerAddClient(srv, client) < 0) { @@ -573,6 +595,13 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) goto error; } + if (virJSONValueObjectAppendNumberUlong(object, "next_client_id", + srv->next_client_id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set next_client_id data in JSON document")); + goto error; + } + if (srv->mdnsGroupName && virJSONValueObjectAppendString(object, "mdnsGroupName", srv->mdnsGroupName) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 8b304f6..2ae89ce 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -35,6 +35,7 @@ virNetServerPtr virNetServerNew(const char *name, + unsigned long long next_client_id, size_t min_workers, size_t max_workers, size_t priority_workers, @@ -102,4 +103,6 @@ int virNetServerSetThreadPoolParameters(virNetServerPtr srv, long long int maxWorkers, long long int prioWorkers); +unsigned long long virNetServerNextClientID(virNetServerPtr srv); + #endif /* __VIR_NET_SERVER_H__ */ diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 64dab46..7233773 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -28,6 +28,7 @@ # include <sasl/sasl.h> #endif +#include "virnetserver.h" #include "virnetserverclient.h" #include "virlog.h" @@ -65,6 +66,7 @@ struct _virNetServerClient { virObjectLockable parent; + unsigned long long id; bool wantClose; bool delayedClose; virNetSocketPtr sock; @@ -346,7 +348,8 @@ static void virNetServerClientSockTimerFunc(int timer, static virNetServerClientPtr -virNetServerClientNewInternal(virNetSocketPtr sock, +virNetServerClientNewInternal(unsigned long long id, + virNetSocketPtr sock, int auth, #ifdef WITH_GNUTLS virNetTLSContextPtr tls, @@ -362,6 +365,7 @@ virNetServerClientNewInternal(virNetSocketPtr sock, if (!(client = virObjectLockableNew(virNetServerClientClass))) return NULL; + client->id = id; client->sock = virObjectRef(sock); client->auth = auth; client->readonly = readonly; @@ -395,7 +399,8 @@ virNetServerClientNewInternal(virNetSocketPtr sock, } -virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, +virNetServerClientPtr virNetServerClientNew(unsigned long long id, + virNetSocketPtr sock, int auth, bool readonly, size_t nrequests_max, @@ -417,7 +422,7 @@ virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, #endif ); - if (!(client = virNetServerClientNewInternal(sock, auth, + if (!(client = virNetServerClientNewInternal(id, sock, auth, #ifdef WITH_GNUTLS tls, #endif @@ -441,7 +446,8 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque) + void *privOpaque, + void *opaque) { virJSONValuePtr child; virNetServerClientPtr client = NULL; @@ -449,6 +455,7 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec int auth; bool readonly; unsigned int nrequests_max; + unsigned long long id; if (virJSONValueObjectGetNumberInt(object, "auth", &auth) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -473,12 +480,25 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec return NULL; } + if (!virJSONValueObjectHasKey(object, "id")) { + /* no ID found in, a new one must be generated */ + id = virNetServerNextClientID((virNetServerPtr) opaque); + } else { + if (virJSONValueObjectGetNumberUlong(object, "id", + (unsigned long long *) &id) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed id field in JSON state document")); + return NULL; + } + } + if (!(sock = virNetSocketNewPostExecRestart(child))) { virObjectUnref(sock); return NULL; } - if (!(client = virNetServerClientNewInternal(sock, + if (!(client = virNetServerClientNewInternal(id, + sock, auth, #ifdef WITH_GNUTLS NULL, @@ -521,6 +541,10 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) virObjectLock(client); + if (virJSONValueObjectAppendNumberUlong(object, "id", + client->id) < 0) + goto error; + if (virJSONValueObjectAppendNumberInt(object, "auth", client->auth) < 0) goto error; if (virJSONValueObjectAppendBoolean(object, "readonly", client->readonly) < 0) @@ -581,6 +605,10 @@ bool virNetServerClientGetReadonly(virNetServerClientPtr client) return readonly; } +unsigned long long virNetServerClientGetID(virNetServerClientPtr client) +{ + return client->id; +} #ifdef WITH_GNUTLS bool virNetServerClientHasTLSSession(virNetServerClientPtr client) diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 8d0fd18..95edb06 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -49,7 +49,8 @@ typedef void *(*virNetServerClientPrivNewPostExecRestart)(virNetServerClientPtr typedef void *(*virNetServerClientPrivNew)(virNetServerClientPtr client, void *opaque); -virNetServerClientPtr virNetServerClientNew(virNetSocketPtr sock, +virNetServerClientPtr virNetServerClientNew(unsigned long long id, + virNetSocketPtr sock, int auth, bool readonly, size_t nrequests_max, @@ -65,7 +66,8 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque, + void *opaque); virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client); @@ -79,6 +81,7 @@ void virNetServerClientRemoveFilter(virNetServerClientPtr client, int virNetServerClientGetAuth(virNetServerClientPtr client); void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); +unsigned long long virNetServerClientGetID(virNetServerClientPtr client); # ifdef WITH_GNUTLS bool virNetServerClientHasTLSSession(virNetServerClientPtr client); diff --git a/tests/virnetdaemondata/input-data-client-ids.json b/tests/virnetdaemondata/input-data-client-ids.json new file mode 100644 index 0000000..ac74c83 --- /dev/null +++ b/tests/virnetdaemondata/input-data-client-ids.json @@ -0,0 +1,69 @@ +{ + "servers": { + "testServer0": { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 10, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "next_client_id": 5, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "id": 2, + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "id": 3, + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-admin-nomdns.json b/tests/virnetdaemondata/output-data-admin-nomdns.json index 8827c04..ca3acd1 100644 --- a/tests/virnetdaemondata/output-data-admin-nomdns.json +++ b/tests/virnetdaemondata/output-data-admin-nomdns.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -38,6 +39,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -49,6 +51,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, @@ -69,6 +72,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -99,6 +103,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -110,6 +115,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemondata/output-data-admin-server-names.json b/tests/virnetdaemondata/output-data-admin-server-names.json index 8827c04..ca3acd1 100644 --- a/tests/virnetdaemondata/output-data-admin-server-names.json +++ b/tests/virnetdaemondata/output-data-admin-server-names.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -38,6 +39,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -49,6 +51,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, @@ -69,6 +72,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -99,6 +103,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -110,6 +115,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemondata/output-data-anon-clients.json b/tests/virnetdaemondata/output-data-anon-clients.json index df93e3b..1a1909f 100644 --- a/tests/virnetdaemondata/output-data-anon-clients.json +++ b/tests/virnetdaemondata/output-data-anon-clients.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 10, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -38,6 +39,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -49,6 +51,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemondata/output-data-client-ids.json b/tests/virnetdaemondata/output-data-client-ids.json new file mode 100644 index 0000000..ac74c83 --- /dev/null +++ b/tests/virnetdaemondata/output-data-client-ids.json @@ -0,0 +1,69 @@ +{ + "servers": { + "testServer0": { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 10, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "next_client_id": 5, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "id": 2, + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "id": 3, + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-initial-nomdns.json b/tests/virnetdaemondata/output-data-initial-nomdns.json index 154962b..da007f3 100644 --- a/tests/virnetdaemondata/output-data-initial-nomdns.json +++ b/tests/virnetdaemondata/output-data-initial-nomdns.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -38,6 +39,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -49,6 +51,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemondata/output-data-initial.json b/tests/virnetdaemondata/output-data-initial.json index b7c27df..cdd02c2 100644 --- a/tests/virnetdaemondata/output-data-initial.json +++ b/tests/virnetdaemondata/output-data-initial.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "mdnsGroupName": "libvirtTest", "services": [ { @@ -39,6 +40,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -50,6 +52,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemondata/output-data-no-keepalive-required.json b/tests/virnetdaemondata/output-data-no-keepalive-required.json index 8827c04..ca3acd1 100644 --- a/tests/virnetdaemondata/output-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/output-data-no-keepalive-required.json @@ -8,6 +8,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -38,6 +39,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -49,6 +51,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, @@ -69,6 +72,7 @@ "max_anonymous_clients": 100, "keepaliveInterval": 120, "keepaliveCount": 5, + "next_client_id": 3, "services": [ { "auth": 0, @@ -99,6 +103,7 @@ ], "clients": [ { + "id": 1, "auth": 1, "readonly": true, "nrequests_max": 15, @@ -110,6 +115,7 @@ } }, { + "id": 2, "auth": 2, "readonly": true, "nrequests_max": 66, diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index a20eee6..b98c148 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -49,7 +49,7 @@ testCreateServer(const char *server_name, const char *host, int family) goto cleanup; } - if (!(srv = virNetServerNew(server_name, + if (!(srv = virNetServerNew(server_name, 1, 10, 50, 5, 100, 10, 120, 5, mdns_group, @@ -93,7 +93,8 @@ testCreateServer(const char *server_name, const char *host, int family) if (virNetSocketNewConnectSockFD(fdclient[1], &sk2) < 0) goto error; - if (!(cln1 = virNetServerClientNew(sk1, + if (!(cln1 = virNetServerClientNew(virNetServerNextClientID(srv), + sk1, VIR_NET_SERVER_SERVICE_AUTH_SASL, true, 15, @@ -103,7 +104,8 @@ testCreateServer(const char *server_name, const char *host, int family) NULL, NULL, NULL, NULL))) goto error; - if (!(cln2 = virNetServerClientNew(sk2, + if (!(cln2 = virNetServerClientNew(virNetServerNextClientID(srv), + sk2, VIR_NET_SERVER_SERVICE_AUTH_POLKIT, true, 66, @@ -336,6 +338,7 @@ mymain(void) EXEC_RESTART_TEST("admin-nomdns", 2); EXEC_RESTART_TEST("admin-server-names", 2); EXEC_RESTART_TEST("no-keepalive-required", 2); + EXEC_RESTART_TEST("client-ids", 1); EXEC_RESTART_TEST_FAIL("anon-clients", 2); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; diff --git a/tests/virnetserverclienttest.c b/tests/virnetserverclienttest.c index 9c4b4c6..be15e81 100644 --- a/tests/virnetserverclienttest.c +++ b/tests/virnetserverclienttest.c @@ -52,7 +52,7 @@ static int testIdentity(const void *opaque ATTRIBUTE_UNUSED) } sv[0] = -1; - if (!(client = virNetServerClientNew(sock, 0, false, 1, + if (!(client = virNetServerClientNew(1, sock, 0, false, 1, # ifdef WITH_GNUTLS NULL, # endif -- 2.4.11

Besides ID, the object also stores static data like connection transport and connection timestamp, since once obtained a list of all clients connected to a server, from user's perspective, it would be nice to know whether a given client is remote or local only and when did it connect to the daemon. Along with the object introduction, all necessary client-side methods necessary to work with the object are added as well. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- include/libvirt/libvirt-admin.h | 22 ++++++++++ src/admin/admin_protocol.x | 8 ++++ src/admin_protocol-structs | 6 +++ src/datatypes.c | 35 ++++++++++++++++ src/datatypes.h | 47 +++++++++++++++++++++ src/libvirt-admin.c | 90 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 1 + src/libvirt_admin_public.syms | 4 ++ 8 files changed, 213 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index bce6034..f85a7e2 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -51,6 +51,14 @@ typedef struct _virAdmConnect virAdmConnect; typedef struct _virAdmServer virAdmServer; /** + * virAdmClient: + * + * a virAdmClient is a private structure and client-side representation of + * a remote server's client object (as server sees clients connected to it) + */ +typedef struct _virAdmClient virAdmClient; + +/** * virAdmConnectPtr: * * a virAdmConnectPtr is pointer to a virAdmConnect private structure, @@ -68,6 +76,15 @@ typedef virAdmConnect *virAdmConnectPtr; */ typedef virAdmServer *virAdmServerPtr; +/** + * virAdmClientPtr: + * + * a virAdmClientPtr is a pointer to a virAdmClient structure, + * this is the type used to reference client-side representation of a + * client object throughout all the APIs. + */ +typedef virAdmClient *virAdmClientPtr; + virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); int virAdmConnectRef(virAdmConnectPtr conn); @@ -182,6 +199,11 @@ int virAdmServerSetThreadPoolParameters(virAdmServerPtr srv, int nparams, unsigned int flags); +unsigned long long virAdmClientGetID(virAdmClientPtr client); +long long virAdmClientGetTimestamp(virAdmClientPtr client); +int virAdmClientGetTransport(virAdmClientPtr client); +int virAdmClientFree(virAdmClientPtr client); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index c701698..2f302af 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -72,6 +72,14 @@ struct admin_nonnull_server { admin_nonnull_string name; }; +/* A client which may NOT be NULL */ +struct admin_nonnull_client { + admin_nonnull_server srv; + unsigned hyper id; + hyper timestamp; + unsigned int transport; +}; + /*----- Protocol. -----*/ struct admin_connect_open_args { diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 650d31d..d4ccf9e 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -27,6 +27,12 @@ struct admin_typed_param { struct admin_nonnull_server { admin_nonnull_string name; }; +struct admin_nonnull_client { + admin_nonnull_server srv; + uint64_t id; + int64_t timestamp; + u_int transport; +}; struct admin_connect_open_args { u_int flags; }; diff --git a/src/datatypes.c b/src/datatypes.c index 696e8c0..ff0c46f 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -66,7 +66,9 @@ static void virAdmConnectDispose(void *obj); static void virAdmConnectCloseCallbackDataDispose(void *obj); virClassPtr virAdmServerClass; +virClassPtr virAdmClientClass; static void virAdmServerDispose(void *obj); +static void virAdmClientDispose(void *obj); static int virDataTypesOnceInit(void) @@ -98,6 +100,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS_LOCKABLE(virAdmConnect); DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData); DECLARE_CLASS(virAdmServer); + DECLARE_CLASS(virAdmClient); #undef DECLARE_CLASS_COMMON #undef DECLARE_CLASS_LOCKABLE @@ -962,3 +965,35 @@ virAdmServerDispose(void *obj) VIR_FREE(srv->name); virObjectUnref(srv->conn); } + +virAdmClientPtr +virAdmGetClient(virAdmServerPtr srv, const unsigned long long id, + unsigned long long timestamp, unsigned int transport) +{ + virAdmClientPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + goto error; + + if (!(ret = virObjectNew(virAdmClientClass))) + goto error; + + ret->id = id; + ret->timestamp = timestamp; + ret->transport = transport; + ret->srv = virObjectRef(srv); + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmClientDispose(void *obj) +{ + virAdmClientPtr clt = obj; + VIR_DEBUG("release client clt=%p, id=%llu", clt, clt->id); + + virObjectUnref(clt->srv); +} diff --git a/src/datatypes.h b/src/datatypes.h index 92e6863..8ccc7b0 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -43,6 +43,7 @@ extern virClassPtr virStoragePoolClass; extern virClassPtr virAdmConnectClass; extern virClassPtr virAdmServerClass; +extern virClassPtr virAdmClientClass; # define virCheckConnectReturn(obj, retval) \ do { \ @@ -342,6 +343,32 @@ extern virClassPtr virAdmServerClass; } \ } while (0); +# define virCheckAdmClientReturn(obj, retval) \ + do { \ + virAdmClientPtr _clt = (obj); \ + if (!virObjectIsClass(_clt, virAdmClientClass) || \ + !virObjectIsClass(_clt->srv, virAdmServerClass) || \ + !virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) +# define virCheckAdmClientGoto(obj, label) \ + do { \ + virAdmClientPtr _clt = (obj); \ + if (!virObjectIsClass(_clt, virAdmClientClass) || \ + !virObjectIsClass(_clt->srv, virAdmServerClass) || \ + !virObjectIsClass(_clt->srv->conn, virAdmConnectClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0); + /** * VIR_DOMAIN_DEBUG: * @dom: domain @@ -450,6 +477,21 @@ struct _virAdmServer { char *name; /* the server external name */ }; +/** + * _virAdmClient: + * + * Internal structure associated to a client connected to daemon + */ +struct _virAdmClient { + virObject object; + virAdmServerPtr srv; /* pointer to the server client is + * connected to, which also holds a + * reference back to the admin connection + */ + unsigned long long id; /* client's ID */ + long long timestamp; /* connection timestamp */ + unsigned int transport; /* connection type as virClientTransport */ +}; /** * _virDomain: @@ -637,6 +679,11 @@ virAdmConnectPtr virAdmConnectNew(void); virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, const char *name); +virAdmClientPtr virAdmGetClient(virAdmServerPtr srv, + unsigned long long id, + unsigned long long timestamp, + unsigned int transport); + virConnectCloseCallbackDataPtr virNewConnectCloseCallbackData(void); void virConnectCloseCallbackDataRegister(virConnectCloseCallbackDataPtr close, virConnectPtr conn, diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index df71649..b45da72 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -600,6 +600,96 @@ int virAdmServerFree(virAdmServerPtr srv) } /** + * virAdmClientGetID: + * @client: a client object + * + * Get client's unique numeric ID. + * + * Returns numeric value used for client's ID or -1 in case of an error. + */ +unsigned long long +virAdmClientGetID(virAdmClientPtr client) +{ + VIR_DEBUG("client=%p", client); + + virResetLastError(); + virCheckAdmClientReturn(client, -1); + return client->id; +} + +/** + * virAdmClientGetTimestamp: + * @client: a client object + * + * Get client's connection time. + * A situation may happen, that some clients had connected prior to the update + * to admin API, thus, libvirt assigns these clients epoch time to express that + * it doesn't know when the client connected. + * + * Returns client's connection timestamp (seconds from epoch in UTC) or 0 + * (epoch time) if libvirt doesn't have any information about client's + * connection time, or -1 in case of an error. + */ +long long +virAdmClientGetTimestamp(virAdmClientPtr client) +{ + VIR_DEBUG("client=%p", client); + + virResetLastError(); + virCheckAdmClientReturn(client, -1); + return client->timestamp; +} + +/** + * virAdmClientGetTransport: + * @client: a client object + * + * Get client's connection transport type. This information can be helpful to + * differentiate between clients connected locally or remotely. An exception to + * this would be SSH which is one of libvirt's supported transports. + * Although SSH creates a channel between two (preferably) remote endpoints, + * the client process libvirt spawns automatically on the remote side will + * still connect to a UNIX socket, thus becoming indistinguishable from any + * other locally connected clients. + * + * Returns integer representation of the connection transport used by client + * @client, this will be one of virClientTransport. + */ +int +virAdmClientGetTransport(virAdmClientPtr client) +{ + VIR_DEBUG("client=%p", client); + + virResetLastError(); + virCheckAdmClientReturn(client, -1); + return client->transport; +} + +/** + * virAdmClientFree: + * @client: a client object + * + * Release the client object. The running instance is kept alive. The data + * structure is freed and should not be used thereafter. + * + * Returns 0 in success, -1 on failure. + */ +int virAdmClientFree(virAdmClientPtr client) +{ + VIR_DEBUG("client=%p", client); + + virResetLastError(); + + if (!client) + return 0; + + virCheckAdmClientReturn(client, -1); + + virObjectUnref(client); + return 0; +} + +/** * virAdmConnectListServers: * @conn: daemon connection reference * @servers: Pointer to a list to store an array containing objects or NULL diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index b150d8a..c407e6e 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -17,6 +17,7 @@ xdr_admin_server_get_threadpool_parameters_ret; xdr_admin_server_set_threadpool_parameters_args; # datatypes.h +virAdmClientClass; virAdmConnectClass; virAdmGetServer; virAdmServerClass; diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 0a16444..7c4626e 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -12,6 +12,10 @@ # LIBVIRT_ADMIN_1.3.0 { global: + virAdmClientFree; + virAdmClientGetID; + virAdmClientGetTimestamp; + virAdmClientGetTransport; virAdmConnectOpen; virAdmConnectClose; virAdmConnectRef; -- 2.4.11

For now, the list copy is done simply by locking the whole server, walking the original and increasing the refcount on each object. We may want to change the list to a lockable object (like list of domains) later in the future if we discover some performance issues related to locking the whole server in order to walk the whole list of clients, possibly issuing some 'ForEach' callback. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/libvirt_remote.syms | 1 + src/rpc/virnetserver.c | 29 +++++++++++++++++++++++++++++ src/rpc/virnetserver.h | 3 +++ 3 files changed, 33 insertions(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index c049874..ea92043 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -101,6 +101,7 @@ virNetServerAddClient; virNetServerAddProgram; virNetServerAddService; virNetServerClose; +virNetServerGetClients; virNetServerGetName; virNetServerHasClients; virNetServerNew; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 8216da6..e52d765 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -943,3 +943,32 @@ virNetServerSetThreadPoolParameters(virNetServerPtr srv, virObjectUnlock(srv); return ret; } + +int +virNetServerGetClients(virNetServerPtr srv, + virNetServerClientPtr **clts) +{ + int ret = -1; + size_t i; + size_t nclients = 0; + + *clts = NULL; + + virObjectLock(srv); + + for (i = 0; i < srv->nclients; i++) { + virNetServerClientPtr client = virObjectRef(srv->clients[i]); + if (VIR_APPEND_ELEMENT(*clts, nclients, client) < 0) { + virObjectUnref(client); + goto cleanup; + } + } + + ret = nclients; + + cleanup: + if (ret < 0) + virObjectListFreeCount(*clts, nclients); + virObjectUnlock(srv); + return ret; +} diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 2ae89ce..f5bb200 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -105,4 +105,7 @@ int virNetServerSetThreadPoolParameters(virNetServerPtr srv, unsigned long long virNetServerNextClientID(virNetServerPtr srv); +int virNetServerGetClients(virNetServerPtr srv, + virNetServerClientPtr **clients); + #endif /* __VIR_NET_SERVER_H__ */ -- 2.4.11

Although we document 6 types of transport that we support, internally we can only differentiate between TCP, TLS, and UNIX transports only, since both SSH and libssh2 transports, due to using netcat, behave in the exactly the same way as a UNIX socket. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- src/rpc/virnetserverclient.c | 22 ++++++++++++++++++++++ src/rpc/virnetserverclient.h | 2 +- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index a1543b9..a9d70e1 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1568,3 +1568,25 @@ virNetServerClientStartKeepAlive(virNetServerClientPtr client) virObjectUnlock(client); return ret; } + +int +virNetServerClientGetTransport(virNetServerClientPtr client) +{ + int ret = -1; + + virObjectLock(client); + + if (client->sock && virNetSocketIsLocal(client->sock)) + ret = VIR_CLIENT_TRANS_UNIX; + else + ret = VIR_CLIENT_TRANS_TCP; + +#ifdef WITH_GNUTLS + if (client->tls) + ret = VIR_CLIENT_TRANS_TLS; +#endif + + virObjectUnlock(client); + + return ret; +} diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index e68ef76..55a8af1 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -144,6 +144,6 @@ int virNetServerClientSendMessage(virNetServerClientPtr client, virNetMessagePtr msg); bool virNetServerClientNeedAuth(virNetServerClientPtr client); - +int virNetServerClientGetTransport(virNetServerClientPtr client); #endif /* __VIR_NET_SERVER_CLIENT_H__ */ -- 2.4.11

Wire-up the public client listing API. Along with this change, a private time simple conversion method to interpret client's timestamp obtained from server has been added as well. Format used to for time output is as follows: YYYY-mm-DD HH:MM:SS+ZZZZ. Although libvirt exposes methods time-related methods through virtime.h internally, it utilizes millisecond precision which we don't need in this case, especially when connection timestamps use precision to seconds only. This is just a convenience int to string conversion method. To reflect the new API, man page has been adjusted accordingly. Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; --- tools/virt-admin.c | 141 +++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virt-admin.pod | 7 +++ 2 files changed, 148 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 22160ad..f0fecdd 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -38,6 +38,7 @@ #include "virstring.h" #include "virthread.h" #include "virgettext.h" +#include "virtime.h" /* Gnulib doesn't guarantee SA_SIGINFO support. */ #ifndef SA_SIGINFO @@ -46,11 +47,64 @@ #define VIRT_ADMIN_PROMPT "virt-admin # " +/* we don't need precision to milliseconds in this module */ +#define VIRT_ADMIN_TIME_BUFLEN VIR_TIME_STRING_BUFLEN - 3 + static char *progname; static const vshCmdGrp cmdGroups[]; static const vshClientHooks hooks; +VIR_ENUM_DECL(virClientTransport) +VIR_ENUM_IMPL(virClientTransport, + VIR_CLIENT_TRANS_LAST, + N_("unix"), + N_("tcp"), + N_("tls")) + +static const char * +vshAdmClientTransportToString(int transport) +{ + const char *str = virClientTransportTypeToString(transport); + return str ? _(str) : _("unknown"); +} + +/* + * vshAdmGetTimeStr: + * + * Produces string representation (local time) of @then + * (seconds since epoch UTC) using format 'YYYY-MM-DD HH:MM:SS+ZZZZ'. + * + * Returns 0 if conversion finished successfully, -1 in case of an error. + * Caller is responsible for freeing the string returned. + */ +static int +vshAdmGetTimeStr(vshControl *ctl, time_t then, char **result) +{ + + char *tmp = NULL; + struct tm timeinfo; + + if (!localtime_r(&then, &timeinfo)) + goto error; + + if (VIR_ALLOC_N(tmp, VIR_TIME_STRING_BUFLEN) < 0) + goto error; + + if (strftime(tmp, VIR_TIME_STRING_BUFLEN, "%Y-%m-%d %H:%M:%S%z", + &timeinfo) == 0) { + VIR_FREE(tmp); + goto error; + } + + *result = tmp; + return 0; + + error: + vshError(ctl, "%s", _("Timestamp string conversion failed")); + return -1; +} + /* * vshAdmCatchDisconnect: * @@ -520,6 +574,87 @@ cmdSrvThreadpoolSet(vshControl *ctl, const vshCmd *cmd) goto cleanup; } +/* ------------------------ + * Command srv-clients-list + * ------------------------ + */ + +static const vshCmdInfo info_srv_clients_list[] = { + {.name = "help", + .data = N_("list clients connected to <server>") + }, + {.name = "desc", + .data = N_("List all manageable clients connected to <server>.") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_srv_clients_list[] = { + {.name = "server", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("server which to list connected clients from"), + }, + {.name = NULL} +}; + +static bool +cmdSrvClientsList(vshControl *ctl, const vshCmd *cmd) +{ + int nclts = 0; + size_t i; + bool ret = false; + const char *srvname = NULL; + unsigned long long id; + virClientTransport transport; + char *timestr = NULL; + virAdmServerPtr srv = NULL; + virAdmClientPtr *clts = NULL; + vshAdmControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, "server", &srvname) < 0) + return false; + + if (!(srv = virAdmConnectLookupServer(priv->conn, srvname, 0))) + goto cleanup; + + /* Obtain a list of clients connected to server @srv */ + if ((nclts = virAdmServerListClients(srv, &clts, 0)) < 0) { + vshError(ctl, _("failed to obtain list of connected clients " + "from server '%s'"), virAdmServerGetName(srv)); + goto cleanup; + } + + vshPrintExtra(ctl, " %-5s %-15s %-15s\n%s\n", _("Id"), _("Transport"), + _("Connected since"), + "-------------------------" + "-------------------------"); + + for (i = 0; i < nclts; i++) { + virAdmClientPtr client = clts[i]; + id = virAdmClientGetID(client); + transport = virAdmClientGetTransport(client); + if (vshAdmGetTimeStr(ctl, virAdmClientGetTimestamp(client), + &timestr) < 0) + goto cleanup; + + vshPrint(ctl, " %-5llu %-15s %-15s\n", + id, vshAdmClientTransportToString(transport), timestr); + } + + ret = true; + + cleanup: + if (clts) { + for (i = 0; i < nclts; i++) + virAdmClientFree(clts[i]); + VIR_FREE(clts); + } + virAdmServerFree(srv); + VIR_FREE(timestr); + return ret; +} + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -825,6 +960,12 @@ static const vshCmdDef monitoringCmds[] = { .info = info_srv_threadpool_info, .flags = 0 }, + {.name = "srv-clients-list", + .handler = cmdSrvClientsList, + .opts = opts_srv_clients_list, + .info = info_srv_clients_list, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virt-admin.pod b/tools/virt-admin.pod index 411951a..40b2980 100644 --- a/tools/virt-admin.pod +++ b/tools/virt-admin.pod @@ -220,6 +220,13 @@ The current number of active priority workers in a threadpool. =back +=item B<srv-clients-list> I<server> + +Print a table showing the list of clients connected to <server>, also providing +information about transport type used on client's connection (supported +transports include B<unix>, B<tcp>, and B<tls>), as well as providing +information about client's connection time (system local time is used). + =back =head1 ENVIRONMENT -- 2.4.11