[libvirt] [PATCH v2 0/9] admin: Admin API: Introduce client listing API
v2: - gendispatch was modified to be able to generate more client related code - Martin noticed that I added avahi-related field into my tests which caused the tests to fail for him, since he doesn't compile with avahi - fixed - coverity complained that clts might be uninitialized when calling virObjectListFreeCount in virAdmServerListClients. Turned out as a false-positive, but I wanted to silence it so I reworked the logic so that the caller-supplied pointer (initialized to NULL) isn't touched until all the memory operations succeeded. That not only didn't help, but since I removed the ugly check before virObjectListFreeCount (because the method handles NULL well), coverity started to complain that I might assign a signed value to an unsigned function argument, which, although being true, is still a false positive because the pointer will be NULL and will be handled correctly. Erik Skultety (9): rpc: virnetserverclient: Identify clients by an integer ID rpc: virnetserverclient: Introduce new attribute conn_time to client admin: Introduce virAdmClient client-side object rpc: gendispatch: Tune it to support client structure rpc: virnetserver: Support retrieval of a list of clients include: admin: export connection transport constants rpc: virnetserverclient: Implement client connection transport retrieval admin: Introduce listing clients virt-admin: Introduce srv-clients-list command daemon/admin.c | 15 +++ daemon/admin_server.c | 23 ++++ daemon/admin_server.h | 4 + daemon/libvirtd.c | 4 +- include/libvirt/libvirt-admin.h | 37 ++++++ src/admin/admin_protocol.x | 29 ++++- src/admin/admin_remote.c | 6 + src/admin_protocol-structs | 19 +++ src/datatypes.c | 35 +++++ src/datatypes.h | 47 +++++++ src/libvirt-admin.c | 131 +++++++++++++++++++ src/libvirt_admin_private.syms | 3 + src/libvirt_admin_public.syms | 5 + src/libvirt_remote.syms | 2 + src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/gendispatch.pl | 75 +++++++++-- src/rpc/virnetserver.c | 64 +++++++++- src/rpc/virnetserver.h | 6 + src/rpc/virnetserverclient.c | 106 ++++++++++++++-- src/rpc/virnetserverclient.h | 10 +- tests/Makefile.am | 7 + tests/virnetdaemondata/input-data-client-ids.json | 68 ++++++++++ .../input-data-client-timestamp.json | 70 ++++++++++ .../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 | 68 ++++++++++ .../output-data-client-timestamp.json | 70 ++++++++++ .../output-data-initial-nomdns.json | 3 + tests/virnetdaemondata/output-data-initial.json | 3 + .../output-data-no-keepalive-required.json | 6 + tests/virnetdaemonmock.c | 34 +++++ tests/virnetdaemontest.c | 13 +- tests/virnetserverclienttest.c | 2 +- tools/virt-admin.c | 141 +++++++++++++++++++++ tools/virt-admin.pod | 7 + 38 files changed, 1095 insertions(+), 39 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-client-ids.json create mode 100644 tests/virnetdaemondata/input-data-client-timestamp.json create mode 100644 tests/virnetdaemondata/output-data-client-ids.json create mode 100644 tests/virnetdaemondata/output-data-client-timestamp.json create mode 100644 tests/virnetdaemonmock.c -- 2.4.11
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@redhat.com> --- 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 | 68 ++++++++++++++++++++++ .../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 | 68 ++++++++++++++++++++++ .../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, 249 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..7caa2cc --- /dev/null +++ b/tests/virnetdaemondata/input-data-client-ids.json @@ -0,0 +1,68 @@ +{ + "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, + "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..7caa2cc --- /dev/null +++ b/tests/virnetdaemondata/output-data-client-ids.json @@ -0,0 +1,68 @@ +{ + "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, + "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
On 29.04.2016 14:39, Erik Skultety wrote:
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@redhat.com> --- 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 | 68 ++++++++++++++++++++++ .../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 | 68 ++++++++++++++++++++++ .../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, 249 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 */
Any reason this shouldn't be size_t just like the rest below?
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 */
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;
Again, this could be size_t.
bool wantClose; bool delayedClose; virNetSocketPtr sock;
Michal
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 */
Any reason this shouldn't be size_t just like the rest below?
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 */
Well, it can't...according POSIX, size_t isn't guaranteed to be always at least 64-bits, it just has to represent the largest unsigned type supported by an implementation and also according to asm-generic/posix_types.h most 32-bit archs use unsigned int for size_t...which isn't what we actually want, since clients come and go and we just increment the counter till the daemon's end of life and imho we do want the largest type possible here....the same applies below... Erik
On 02.05.2016 16:19, Erik Skultety wrote:
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 */
Any reason this shouldn't be size_t just like the rest below?
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 */
Well, it can't...according POSIX, size_t isn't guaranteed to be always at least 64-bits, it just has to represent the largest unsigned type supported by an implementation and also according to asm-generic/posix_types.h most 32-bit archs use unsigned int for size_t...which isn't what we actually want, since clients come and go and we just increment the counter till the daemon's end of life and imho we do want the largest type possible here....the same applies below...
Interesting. I thought that size_t is always the biggest possible type (not by definition, but by actual code). In that case, we should stick with unsigned long long. ACK to your version. Michal
Besides ID, libvirt should provide several parameters to help the user distinguish two clients from each other. One of them is the connection timestamp. This patch also adds a testcase for proper JSON formatting of the new attribute too (proper formatting of older clients that did not support this attribute yet is included in the existing tests) - in order to testGenerateJSON to work, a mock of time_t time(time_t *timer) needed to be created. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/virnetserverclient.c | 46 +++++++++++++- src/rpc/virnetserverclient.h | 1 + tests/Makefile.am | 7 +++ .../input-data-client-timestamp.json | 70 ++++++++++++++++++++++ .../output-data-client-timestamp.json | 70 ++++++++++++++++++++++ tests/virnetdaemonmock.c | 34 +++++++++++ tests/virnetdaemontest.c | 4 +- 7 files changed, 228 insertions(+), 4 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-client-timestamp.json create mode 100644 tests/virnetdaemondata/output-data-client-timestamp.json create mode 100644 tests/virnetdaemonmock.c diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 7233773..a1543b9 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -85,6 +85,13 @@ struct _virNetServerClient virIdentityPtr identity; + /* Connection timestamp, i.e. when a client connected to the daemon (UTC). + * For old clients restored by post-exec-restart, which did not have this + * attribute, value of 0 (epoch time) is used to indicate we have no + * information about their connection time. + */ + time_t conn_time; + /* Count of messages in the 'tx' queue, * and the server worker pool queue * ie RPC calls in progress. Does not count @@ -355,7 +362,8 @@ virNetServerClientNewInternal(unsigned long long id, virNetTLSContextPtr tls, #endif bool readonly, - size_t nrequests_max) + size_t nrequests_max, + time_t timestamp) { virNetServerClientPtr client; @@ -373,6 +381,7 @@ virNetServerClientNewInternal(unsigned long long id, client->tlsCtxt = virObjectRef(tls); #endif client->nrequests_max = nrequests_max; + client->conn_time = timestamp; client->sockTimer = virEventAddTimeout(-1, virNetServerClientSockTimerFunc, client, NULL); @@ -413,6 +422,7 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, void *privOpaque) { virNetServerClientPtr client; + time_t now; VIR_DEBUG("sock=%p auth=%d tls=%p", sock, auth, #ifdef WITH_GNUTLS @@ -422,11 +432,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, #endif ); + if ((now = time(NULL)) == (time_t) - 1) { + virReportSystemError(errno, "%s", _("failed to get current time")); + return NULL; + } + if (!(client = virNetServerClientNewInternal(id, sock, auth, #ifdef WITH_GNUTLS tls, #endif - readonly, nrequests_max))) + readonly, nrequests_max, + now))) return NULL; if (privNew) { @@ -456,6 +472,7 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec bool readonly; unsigned int nrequests_max; unsigned long long id; + time_t timestamp; if (virJSONValueObjectGetNumberInt(object, "auth", &auth) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -492,6 +509,18 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec } } + if (!virJSONValueObjectHasKey(object, "conn_time")) { + timestamp = 0; + } else { + if (virJSONValueObjectGetNumberLong(object, "conn_time", + (long long *) ×tamp) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed conn_time field in JSON " + "state document")); + return NULL; + } + } + if (!(sock = virNetSocketNewPostExecRestart(child))) { virObjectUnref(sock); return NULL; @@ -504,7 +533,8 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec NULL, #endif readonly, - nrequests_max))) { + nrequests_max, + timestamp))) { virObjectUnref(sock); return NULL; } @@ -552,6 +582,11 @@ virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client) if (virJSONValueObjectAppendNumberUint(object, "nrequests_max", client->nrequests_max) < 0) goto error; + if (client->conn_time && + virJSONValueObjectAppendNumberLong(object, "conn_time", + client->conn_time) < 0) + goto error; + if (!(child = virNetSocketPreExecRestart(client->sock))) goto error; @@ -610,6 +645,11 @@ unsigned long long virNetServerClientGetID(virNetServerClientPtr client) return client->id; } +long long virNetServerClientGetTimestamp(virNetServerClientPtr client) +{ + return client->conn_time; +} + #ifdef WITH_GNUTLS bool virNetServerClientHasTLSSession(virNetServerClientPtr client) { diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 95edb06..e68ef76 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -82,6 +82,7 @@ int virNetServerClientGetAuth(virNetServerClientPtr client); void virNetServerClientSetAuth(virNetServerClientPtr client, int auth); bool virNetServerClientGetReadonly(virNetServerClientPtr client); unsigned long long virNetServerClientGetID(virNetServerClientPtr client); +long long virNetServerClientGetTimestamp(virNetServerClientPtr client); # ifdef WITH_GNUTLS bool virNetServerClientHasTLSSession(virNetServerClientPtr client); diff --git a/tests/Makefile.am b/tests/Makefile.am index 8cf53bf..6d8fa00 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -408,6 +408,7 @@ EXTRA_DIST += $(test_scripts) test_libraries = libshunload.la \ virportallocatormock.la \ + virnetdaemonmock.la \ virnetserverclientmock.la \ vircgroupmock.la \ virpcimock.la \ @@ -927,6 +928,12 @@ virnetdaemontest_SOURCES = \ virnetdaemontest_CFLAGS = $(XDR_CFLAGS) $(AM_CFLAGS) virnetdaemontest_LDADD = $(LDADDS) +virnetdaemonmock_la_SOURCES = \ + virnetdaemonmock.c +virnetdaemonmock_la_CFLAGS = $(AM_CFLAGS) +virnetdaemonmock_la_LDFLAGS = $(MOCKLIBS_LDFLAGS) +virnetdaemonmock_la_LIBADD = $(MOCKLIBS_LIBS) + virnetserverclienttest_SOURCES = \ virnetserverclienttest.c \ testutils.h testutils.c diff --git a/tests/virnetdaemondata/input-data-client-timestamp.json b/tests/virnetdaemondata/input-data-client-timestamp.json new file mode 100644 index 0000000..d069997 --- /dev/null +++ b/tests/virnetdaemondata/input-data-client-timestamp.json @@ -0,0 +1,70 @@ +{ + "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": 3, + "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": 1, + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "conn_time": 1234567890, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "id": 2, + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "conn_time": 1234567890, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemondata/output-data-client-timestamp.json b/tests/virnetdaemondata/output-data-client-timestamp.json new file mode 100644 index 0000000..d069997 --- /dev/null +++ b/tests/virnetdaemondata/output-data-client-timestamp.json @@ -0,0 +1,70 @@ +{ + "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": 3, + "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": 1, + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "conn_time": 1234567890, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "id": 2, + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "conn_time": 1234567890, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + } +} diff --git a/tests/virnetdaemonmock.c b/tests/virnetdaemonmock.c new file mode 100644 index 0000000..6d807a5 --- /dev/null +++ b/tests/virnetdaemonmock.c @@ -0,0 +1,34 @@ +/* + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Erik Skultety <eskultet@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include <time.h> + +#define VIR_FROM_THIS VIR_FROM_NONE + +time_t time(time_t *t) +{ + const time_t ret = 1234567890; + if (t) + *t = ret; + return ret; +} diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index b98c148..714f8d7 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -339,15 +339,17 @@ mymain(void) EXEC_RESTART_TEST("admin-server-names", 2); EXEC_RESTART_TEST("no-keepalive-required", 2); EXEC_RESTART_TEST("client-ids", 1); + EXEC_RESTART_TEST("client-timestamp", 1); EXEC_RESTART_TEST_FAIL("anon-clients", 2); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } +VIRT_TEST_MAIN_PRELOAD(mymain, abs_builddir "/.libs/virnetdaemonmock.so") #else static int mymain(void) { return EXIT_AM_SKIP; } -#endif VIRT_TEST_MAIN(mymain); +#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@redhat.com> --- include/libvirt/libvirt-admin.h | 23 +++++++++++ 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, 214 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index bce6034..feb9473 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,12 @@ int virAdmServerSetThreadPoolParameters(virAdmServerPtr srv, int nparams, unsigned int flags); +/* virAdmClient object accessors */ +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
Now that libvirt-admin supports another client-side object and provided that we want to generate as many both client-side and server-side RPC dispatchers, support for this needs to be added to gendispatch. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- src/rpc/gendispatch.pl | 75 ++++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 61 insertions(+), 14 deletions(-) diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl index 624049b..5564b2e 100755 --- a/src/rpc/gendispatch.pl +++ b/src/rpc/gendispatch.pl @@ -610,6 +610,20 @@ elsif ($mode eq "server") { push(@args_list, "$2"); push(@free_list, " virObjectUnref($2);"); + } elsif ($args_member =~ m/^admin_nonnull_(client) (\S+);/) { + my $type_name = name_to_TypeName($1); + + push(@vars_list, "virNetServerPtr srv = NULL"); + push(@vars_list, "virNetServer${type_name}Ptr $2 = NULL"); + push(@getters_list, + " if (!(srv = get_nonnull_server(priv->dmn, args->$2.srv)))\n" . + " goto cleanup;\n"); + push(@getters_list, + " if (!($2 = get_nonnull_$1(srv, args->$2)))\n" . + " goto cleanup;\n"); + push(@args_list, "$2"); + push(@free_list, " virObjectUnref($2);"); + push(@free_list, " virObjectUnref(srv);"); } elsif ($args_member =~ m/^(\/)?\*/) { # ignore comments } else { @@ -628,7 +642,9 @@ elsif ($mode eq "server") { my $single_ret_list_max_define = "undefined"; my $multi_ret = 0; my $modern_ret_as_list = 0; + my $modern_ret_is_nested = 0; my $modern_ret_struct_name = "undefined"; + my $modern_ret_nested_struct_name = "undefined"; my $single_ret_list_error_msg_type = "undefined"; if ($rettype ne "void" and @@ -649,7 +665,7 @@ elsif ($mode eq "server") { if (!$modern_ret_as_list) { push(@ret_list, "ret->$3 = tmp.$3;"); } - } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { $modern_ret_struct_name = $1; $single_ret_list_error_msg_type = $1; $single_ret_list_name = $2; @@ -829,11 +845,18 @@ elsif ($mode eq "server") { } elsif ($ret_member =~ m/^opaque (\S+)<\S+>;/) { # error out on unannotated arrays die "opaque array without insert@<offset> annotation: $ret_member"; - } elsif ($ret_member =~ m/^admin_nonnull_(server) (\S+);/) { + } elsif ($ret_member =~ m/^admin_nonnull_(server|client) (\S+);/) { my $type_name = name_to_TypeName($1); - push(@vars_list, "virNet${type_name}Ptr $2 = NULL"); - push(@ret_list, "make_nonnull_$1(&ret->$2, $2);"); + if ($1 eq "client") { + push(@vars_list, "virNetServer${type_name}Ptr $2 = NULL"); + push(@ret_list, "make_nonnull_$1(&ret->$2, $2);"); + push(@ret_list, "make_nonnull_server(&ret->$2.srv, srv);"); + } else { + push(@vars_list, "virNet${type_name}Ptr $2 = NULL"); + push(@ret_list, "make_nonnull_$1(&ret->$2, $2);"); + } + push(@free_list, " virObjectUnref($2);"); $single_ret_var = $2; @@ -856,7 +879,13 @@ elsif ($mode eq "server") { my $struct_name = name_to_TypeName($modern_ret_struct_name); if ($structprefix eq "admin") { - $struct_name = "Net${struct_name}"; + if ($modern_ret_struct_name eq "client") { + $modern_ret_is_nested = 1; + $modern_ret_nested_struct_name = "server"; + $struct_name = "NetServer${struct_name}"; + } else { + $struct_name = "Net${struct_name}"; + } } push(@vars_list, "vir${struct_name}Ptr *result = NULL"); @@ -1059,7 +1088,7 @@ elsif ($mode eq "server") { print " if (nresults > $single_ret_list_max_define) {\n"; print " virReportError(VIR_ERR_INTERNAL_ERROR,\n"; print " _(\"Too many ${single_ret_list_error_msg_type}s '%d' for limit '%d'\"),\n"; - print " nresults, $single_ret_list_max_define);\n"; + print " nresults, $single_ret_list_max_define);\n"; print " goto cleanup;\n"; print " }\n"; print "\n"; @@ -1068,8 +1097,15 @@ elsif ($mode eq "server") { print " goto cleanup;\n"; print "\n"; print " ret->$single_ret_list_name.${single_ret_list_name}_len = nresults;\n"; - print " for (i = 0; i < nresults; i++)\n"; - print " make_nonnull_$modern_ret_struct_name(ret->$single_ret_list_name.${single_ret_list_name}_val + i, result[i]);\n"; + if ($modern_ret_is_nested) { + print " for (i = 0; i < nresults; i++) {\n"; + print " make_nonnull_$modern_ret_struct_name(ret->$single_ret_list_name.${single_ret_list_name}_val + i, result[i]);\n"; + print " make_nonnull_$modern_ret_nested_struct_name(&ret->$single_ret_list_name.${single_ret_list_name}_val[i].srv, srv);\n"; + print " }\n"; + } else { + print " for (i = 0; i < nresults; i++)\n"; + print " make_nonnull_$modern_ret_struct_name(ret->$single_ret_list_name.${single_ret_list_name}_val + i, result[i]);\n"; + } print " } else {\n"; print " ret->$single_ret_list_name.${single_ret_list_name}_len = 0;\n"; print " ret->$single_ret_list_name.${single_ret_list_name}_val = NULL;\n"; @@ -1337,13 +1373,17 @@ elsif ($mode eq "client") { push(@args_list, "$type_name $arg_name"); push(@setters_list, "args.$arg_name = $arg_name;"); - } elsif ($args_member =~ m/^admin_nonnull_(server) (\S+);/) { + } elsif ($args_member =~ m/^admin_nonnull_(server|client) (\S+);/) { my $name = $1; my $arg_name = $2; my $type_name = name_to_TypeName($name); if ($is_first_arg) { - $priv_src = "$arg_name->conn"; + if ($name eq "client") { + $priv_src = "$arg_name->srv->conn"; + } else { + $priv_src = "$arg_name->conn"; + } } push(@args_list, "virAdm${type_name}Ptr $arg_name"); @@ -1401,7 +1441,7 @@ elsif ($mode eq "client") { } push(@ret_list, "memcpy(result->$3, ret.$3, sizeof(result->$3));"); - } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server) (\S+)<(\S+)>;/) { + } elsif ($ret_member =~ m/(?:admin|remote)_nonnull_(secret|nwfilter|node_device|interface|network|storage_vol|storage_pool|domain_snapshot|domain|server|client) (\S+)<(\S+)>;/) { my $proc_name = name_to_TypeName($1); if ($structprefix eq "admin") { @@ -1545,12 +1585,19 @@ elsif ($mode eq "client") { $single_ret_var = "unsigned long long rv = 0"; $single_ret_type = "unsigned long long"; } - } elsif ($ret_member =~ m/^admin_nonnull_(server) (\S+);/) { + } elsif ($ret_member =~ m/^admin_nonnull_(server|client) (\S+);/) { my $name = $1; my $arg_name = $2; my $type_name = name_to_TypeName($name); - push(@ret_list, "rv = get_nonnull_$name($priv_src, ret.$arg_name);"); + if ($name eq "client") { + my $clnt = $priv_src; + $clnt =~ s/->conn//; + push(@ret_list, "rv = get_nonnull_$name($clnt, ret.$arg_name);"); + } else { + push(@ret_list, "rv = get_nonnull_$name($priv_src, ret.$arg_name);"); + } + push(@ret_list, "xdr_free((xdrproc_t)xdr_$rettype, (char *)&ret);"); $single_ret_var = "virAdm${type_name}Ptr rv = NULL"; @@ -1779,7 +1826,7 @@ elsif ($mode eq "client") { print " }\n"; print "\n"; } elsif ($modern_ret_as_list) { - if ($modern_ret_struct_name eq "domain_snapshot") { + if ($modern_ret_struct_name =~ m/domain_snapshot|client/) { $priv_src =~ s/->conn//; } print " if (result) {\n"; -- 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@redhat.com> --- 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..fcc79f4 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; + virNetServerClientPtr *list = NULL; + + virObjectLock(srv); + + for (i = 0; i < srv->nclients; i++) { + virNetServerClientPtr client = virObjectRef(srv->clients[i]); + if (VIR_APPEND_ELEMENT(list, nclients, client) < 0) { + virObjectUnref(client); + goto cleanup; + } + } + + *clts = list; + list = NULL; + ret = nclients; + + cleanup: + virObjectListFreeCount(list, 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
We have to expose some constants, in order for the client object transport field to make sense. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- include/libvirt/libvirt-admin.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index feb9473..f83ac1c 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -205,6 +205,16 @@ long long virAdmClientGetTimestamp(virAdmClientPtr client); int virAdmClientGetTransport(virAdmClientPtr client); int virAdmClientFree(virAdmClientPtr client); +typedef enum { + VIR_CLIENT_TRANS_UNIX = 0, /* connection via UNIX socket */ + VIR_CLIENT_TRANS_TCP, /* connection via unencrypted TCP socket */ + VIR_CLIENT_TRANS_TLS, /* connection via encrypted TCP socket */ + +# ifdef VIR_ENUM_SENTINELS + VIR_CLIENT_TRANS_LAST +# endif +} virClientTransport; + # ifdef __cplusplus } # endif -- 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@redhat.com> --- 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
Finally add public method to retrieve the list of currently connected clients to a given server. Signed-off-by: Erik Skultety <eskultet@redhat.com> --- daemon/admin.c | 15 +++++++++++++++ daemon/admin_server.c | 23 +++++++++++++++++++++++ daemon/admin_server.h | 4 ++++ include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 21 ++++++++++++++++++++- src/admin/admin_remote.c | 6 ++++++ src/admin_protocol-structs | 13 +++++++++++++ src/libvirt-admin.c | 41 +++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 ++ src/libvirt_admin_public.syms | 1 + 10 files changed, 129 insertions(+), 1 deletion(-) diff --git a/daemon/admin.c b/daemon/admin.c index 00e7dc3..fcbdee5 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -81,6 +81,12 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, /* Helpers */ +static virNetServerPtr +get_nonnull_server(virNetDaemonPtr dmn, admin_nonnull_server srv) +{ + return virNetDaemonGetServer(dmn, srv.name); +} + static void make_nonnull_server(admin_nonnull_server *srv_dst, virNetServerPtr srv_src) @@ -88,6 +94,15 @@ make_nonnull_server(admin_nonnull_server *srv_dst, ignore_value(VIR_STRDUP_QUIET(srv_dst->name, virNetServerGetName(srv_src))); } +static void +make_nonnull_client(admin_nonnull_client *clt_dst, + virNetServerClientPtr clt_src) +{ + clt_dst->id = virNetServerClientGetID(clt_src); + clt_dst->timestamp = virNetServerClientGetTimestamp(clt_src); + clt_dst->transport = virNetServerClientGetTransport(clt_src); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, diff --git a/daemon/admin_server.c b/daemon/admin_server.c index e39a9bd..0bd142c 100644 --- a/daemon/admin_server.c +++ b/daemon/admin_server.c @@ -178,3 +178,26 @@ adminServerSetThreadPoolParameters(virNetServerPtr srv, return 0; } + +int +adminServerListClients(virNetServerPtr srv, + virNetServerClientPtr **clients, + unsigned int flags) +{ + int ret = -1; + virNetServerClientPtr *clts; + + virCheckFlags(0, -1); + + if ((ret = virNetServerGetClients(srv, &clts)) < 0) + goto cleanup; + + if (clients) { + *clients = clts; + clts = NULL; + } + + cleanup: + virObjectListFreeCount(clts, ret); + return ret; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h index 756e049..3e4786e 100644 --- a/daemon/admin_server.h +++ b/daemon/admin_server.h @@ -46,4 +46,8 @@ adminServerSetThreadPoolParameters(virNetServerPtr srv, int nparams, unsigned int flags); +int adminServerListClients(virNetServerPtr srv, + virNetServerClientPtr **clients, + unsigned int flags); + #endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index f83ac1c..20768e2 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -215,6 +215,10 @@ typedef enum { # endif } virClientTransport; +int virAdmServerListClients(virAdmServerPtr srv, + virAdmClientPtr **clients, + unsigned int flags); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 2f302af..e77ce9e 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -39,6 +39,9 @@ const ADMIN_SERVER_LIST_MAX = 16384; /* Upper limit on number of threadpool parameters */ const ADMIN_SERVER_THREADPOOL_PARAMETERS_MAX = 32; +/* Upper limit on list of clients */ +const ADMIN_CLIENT_LIST_MAX = 16384; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string<ADMIN_STRING_MAX>; @@ -124,6 +127,17 @@ struct admin_server_set_threadpool_parameters_args { unsigned int flags; }; +struct admin_server_list_clients_args { + admin_nonnull_server srv; + unsigned int need_results; + unsigned int flags; +}; + +struct admin_server_list_clients_ret { /* insert@1 */ + admin_nonnull_client clients<ADMIN_CLIENT_LIST_MAX>; + unsigned int ret; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -179,5 +193,10 @@ enum admin_procedure { /** * @generate: none */ - ADMIN_PROC_SERVER_SET_THREADPOOL_PARAMETERS = 7 + ADMIN_PROC_SERVER_SET_THREADPOOL_PARAMETERS = 7, + + /** + * @generate: both + */ + ADMIN_PROC_SERVER_LIST_CLIENTS = 8 }; diff --git a/src/admin/admin_remote.c b/src/admin/admin_remote.c index 2dd692b..b833ea4 100644 --- a/src/admin/admin_remote.c +++ b/src/admin/admin_remote.c @@ -55,6 +55,12 @@ get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server) return virAdmGetServer(conn, server.name); } +static virAdmClientPtr +get_nonnull_client(virAdmServerPtr srv, admin_nonnull_client client) +{ + return virAdmGetClient(srv, client.id, client.timestamp, client.transport); +} + static void make_nonnull_server(admin_nonnull_server *srv_dst, virAdmServerPtr srv_src) { diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index d4ccf9e..055ddd5 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -75,6 +75,18 @@ struct admin_server_set_threadpool_parameters_args { } params; u_int flags; }; +struct admin_server_list_clients_args { + admin_nonnull_server srv; + u_int need_results; + u_int flags; +}; +struct admin_server_list_clients_ret { + struct { + u_int clients_len; + admin_nonnull_client * clients_val; + } clients; + u_int ret; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, @@ -83,4 +95,5 @@ enum admin_procedure { ADMIN_PROC_CONNECT_LOOKUP_SERVER = 5, ADMIN_PROC_SERVER_GET_THREADPOOL_PARAMETERS = 6, ADMIN_PROC_SERVER_SET_THREADPOOL_PARAMETERS = 7, + ADMIN_PROC_SERVER_LIST_CLIENTS = 8, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index b45da72..d4143e4 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -848,3 +848,44 @@ virAdmServerSetThreadPoolParameters(virAdmServerPtr srv, virDispatchError(NULL); return -1; } + +/** + * virAdmServerListClients: + * @srv: a valid server object reference + * @clients: pointer to a list to store an array containing objects or NULL + * if the list is not required (number of clients only) + * @flags: extra flags; not used yet, so callers should always pass 0 + * + * Collect list of all clients connected to daemon on server @srv. + * + * Returns the number of clients connected to daemon on server @srv -1 in case + * of a failure, setting @clients to NULL. There is a guaranteed extra element + * set to NULL in the @clients list returned to make the iteration easier, + * excluding this extra element from the final count. + * Caller is responsible to call virAdmClientFree() on each list element, + * followed by freeing @clients. + */ +int +virAdmServerListClients(virAdmServerPtr srv, + virAdmClientPtr **clients, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("srv=%p, clients=%p, flags=%x", srv, clients, flags); + + virResetLastError(); + virCheckFlagsGoto(0, error); + + if (clients) + *clients = NULL; + + virCheckAdmServerReturn(srv, -1); + if ((ret = remoteAdminServerListClients(srv, clients, flags)) < 0) + goto error; + + return ret; + error: + virDispatchError(NULL); + return -1; +} diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index c407e6e..621b502 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -14,6 +14,8 @@ xdr_admin_connect_lookup_server_ret; xdr_admin_connect_open_args; xdr_admin_server_get_threadpool_parameters_args; xdr_admin_server_get_threadpool_parameters_ret; +xdr_admin_server_list_clients_args; +xdr_admin_server_list_clients_ret; xdr_admin_server_set_threadpool_parameters_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 7c4626e..bf6643a 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -31,4 +31,5 @@ LIBVIRT_ADMIN_1.3.0 { virAdmServerFree; virAdmConnectLookupServer; virAdmServerSetThreadPoolParameters; + virAdmServerListClients; }; -- 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@redhat.com> --- 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), + ×tr) < 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 83a8cea..6f9dd63 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
On 29.04.2016 14:39, Erik Skultety wrote:
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@redhat.com> --- 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), + ×tr) < 0) + goto cleanup; + + vshPrint(ctl, " %-5llu %-15s %-15s\n", + id, vshAdmClientTransportToString(transport), timestr);
@timestr is allocated in each iteration. It should be freed in each iteration too. ==6320== 29 bytes in 1 blocks are definitely lost in loss record 49 of 131 ==6320== at 0x4C2C070: calloc (vg_replace_malloc.c:623) ==6320== by 0x52F4A5F: virAllocN (viralloc.c:191) ==6320== by 0x10C8DC: vshAdmGetTimeStr (virt-admin.c:91) ==6320== by 0x10DB4A: cmdSrvClientsList (virt-admin.c:639) ==6320== by 0x11180A: vshCommandRun (vsh.c:1271) ==6320== by 0x10EAFF: main (virt-admin.c:1070)
+ } + + 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} };
Michal
On 29.04.2016 14:39, Erik Skultety wrote:
v2: - gendispatch was modified to be able to generate more client related code
- Martin noticed that I added avahi-related field into my tests which caused the tests to fail for him, since he doesn't compile with avahi - fixed
- coverity complained that clts might be uninitialized when calling virObjectListFreeCount in virAdmServerListClients. Turned out as a false-positive, but I wanted to silence it so I reworked the logic so that the caller-supplied pointer (initialized to NULL) isn't touched until all the memory operations succeeded. That not only didn't help, but since I removed the ugly check before virObjectListFreeCount (because the method handles NULL well), coverity started to complain that I might assign a signed value to an unsigned function argument, which, although being true, is still a false positive because the pointer will be NULL and will be handled correctly.
Erik Skultety (9): rpc: virnetserverclient: Identify clients by an integer ID rpc: virnetserverclient: Introduce new attribute conn_time to client admin: Introduce virAdmClient client-side object rpc: gendispatch: Tune it to support client structure rpc: virnetserver: Support retrieval of a list of clients include: admin: export connection transport constants rpc: virnetserverclient: Implement client connection transport retrieval admin: Introduce listing clients virt-admin: Introduce srv-clients-list command
daemon/admin.c | 15 +++ daemon/admin_server.c | 23 ++++ daemon/admin_server.h | 4 + daemon/libvirtd.c | 4 +- include/libvirt/libvirt-admin.h | 37 ++++++ src/admin/admin_protocol.x | 29 ++++- src/admin/admin_remote.c | 6 + src/admin_protocol-structs | 19 +++ src/datatypes.c | 35 +++++ src/datatypes.h | 47 +++++++ src/libvirt-admin.c | 131 +++++++++++++++++++ src/libvirt_admin_private.syms | 3 + src/libvirt_admin_public.syms | 5 + src/libvirt_remote.syms | 2 + src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/gendispatch.pl | 75 +++++++++-- src/rpc/virnetserver.c | 64 +++++++++- src/rpc/virnetserver.h | 6 + src/rpc/virnetserverclient.c | 106 ++++++++++++++-- src/rpc/virnetserverclient.h | 10 +- tests/Makefile.am | 7 + tests/virnetdaemondata/input-data-client-ids.json | 68 ++++++++++ .../input-data-client-timestamp.json | 70 ++++++++++ .../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 | 68 ++++++++++ .../output-data-client-timestamp.json | 70 ++++++++++ .../output-data-initial-nomdns.json | 3 + tests/virnetdaemondata/output-data-initial.json | 3 + .../output-data-no-keepalive-required.json | 6 + tests/virnetdaemonmock.c | 34 +++++ tests/virnetdaemontest.c | 13 +- tests/virnetserverclienttest.c | 2 +- tools/virt-admin.c | 141 +++++++++++++++++++++ tools/virt-admin.pod | 7 + 38 files changed, 1095 insertions(+), 39 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-client-ids.json create mode 100644 tests/virnetdaemondata/input-data-client-timestamp.json create mode 100644 tests/virnetdaemondata/output-data-client-ids.json create mode 100644 tests/virnetdaemondata/output-data-client-timestamp.json create mode 100644 tests/virnetdaemonmock.c
ACK series Michal
On 02/05/16 15:59, Michal Privoznik wrote:
On 29.04.2016 14:39, Erik Skultety wrote:
v2: - gendispatch was modified to be able to generate more client related code
- Martin noticed that I added avahi-related field into my tests which caused the tests to fail for him, since he doesn't compile with avahi - fixed
- coverity complained that clts might be uninitialized when calling virObjectListFreeCount in virAdmServerListClients. Turned out as a false-positive, but I wanted to silence it so I reworked the logic so that the caller-supplied pointer (initialized to NULL) isn't touched until all the memory operations succeeded. That not only didn't help, but since I removed the ugly check before virObjectListFreeCount (because the method handles NULL well), coverity started to complain that I might assign a signed value to an unsigned function argument, which, although being true, is still a false positive because the pointer will be NULL and will be handled correctly.
Erik Skultety (9): rpc: virnetserverclient: Identify clients by an integer ID rpc: virnetserverclient: Introduce new attribute conn_time to client admin: Introduce virAdmClient client-side object rpc: gendispatch: Tune it to support client structure rpc: virnetserver: Support retrieval of a list of clients include: admin: export connection transport constants rpc: virnetserverclient: Implement client connection transport retrieval admin: Introduce listing clients virt-admin: Introduce srv-clients-list command
daemon/admin.c | 15 +++ daemon/admin_server.c | 23 ++++ daemon/admin_server.h | 4 + daemon/libvirtd.c | 4 +- include/libvirt/libvirt-admin.h | 37 ++++++ src/admin/admin_protocol.x | 29 ++++- src/admin/admin_remote.c | 6 + src/admin_protocol-structs | 19 +++ src/datatypes.c | 35 +++++ src/datatypes.h | 47 +++++++ src/libvirt-admin.c | 131 +++++++++++++++++++ src/libvirt_admin_private.syms | 3 + src/libvirt_admin_public.syms | 5 + src/libvirt_remote.syms | 2 + src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/gendispatch.pl | 75 +++++++++-- src/rpc/virnetserver.c | 64 +++++++++- src/rpc/virnetserver.h | 6 + src/rpc/virnetserverclient.c | 106 ++++++++++++++-- src/rpc/virnetserverclient.h | 10 +- tests/Makefile.am | 7 + tests/virnetdaemondata/input-data-client-ids.json | 68 ++++++++++ .../input-data-client-timestamp.json | 70 ++++++++++ .../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 | 68 ++++++++++ .../output-data-client-timestamp.json | 70 ++++++++++ .../output-data-initial-nomdns.json | 3 + tests/virnetdaemondata/output-data-initial.json | 3 + .../output-data-no-keepalive-required.json | 6 + tests/virnetdaemonmock.c | 34 +++++ tests/virnetdaemontest.c | 13 +- tests/virnetserverclienttest.c | 2 +- tools/virt-admin.c | 141 +++++++++++++++++++++ tools/virt-admin.pod | 7 + 38 files changed, 1095 insertions(+), 39 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-client-ids.json create mode 100644 tests/virnetdaemondata/input-data-client-timestamp.json create mode 100644 tests/virnetdaemondata/output-data-client-ids.json create mode 100644 tests/virnetdaemondata/output-data-client-timestamp.json create mode 100644 tests/virnetdaemonmock.c
ACK series
Michal
I fixed 9/9 as you suggested and pushed, thank you for review. Erik
participants (2)
-
Erik Skultety -
Michal Privoznik