[libvirt] [PATCH v3 0/6] admin: Introduce server listing API

Since v2: - static names of daemons are passed to virNetServerNew directly, instead of creating an array of names like the previous version did - dropped client-side server identification through ID, only name is used - adjusted naming of some methods (prefixes again...) - converted the server listing example to virt-admin command (finally) Erik Skultety (6): rpc: Introduce new element 'name' to virnetserver structure virnetdaemon: Add post exec restart support for multiple servers admin: Move admin_server.{h,c} to admin.{h,c} admin: Introduce virAdmServer structure admin: Introduce adminDaemonConnectListServers API virt-admin: Introduce cmdSrvList daemon/Makefile.am | 6 +- daemon/admin.c | 181 +++++++++++++++++++++ daemon/admin.h | 36 ++++ daemon/admin_server.c | 121 +++++--------- daemon/admin_server.h | 23 ++- daemon/libvirtd.c | 4 +- include/libvirt/libvirt-admin.h | 11 ++ po/POTFILES.in | 2 +- src/admin/admin_protocol.x | 26 ++- src/admin_protocol-structs | 15 ++ src/datatypes.c | 36 ++++ src/datatypes.h | 34 ++++ src/libvirt-admin.c | 148 +++++++++++++++++ src/libvirt_admin_private.syms | 5 + src/libvirt_admin_public.syms | 3 + src/locking/lock_daemon.c | 3 +- src/logging/log_daemon.c | 3 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 15 ++ src/rpc/virnetdaemon.h | 3 + src/rpc/virnetserver.c | 32 +++- src/rpc/virnetserver.h | 5 + .../input-data-admin-server-names.json | 128 +++++++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 2 + .../output-data-admin-server-names.json | 128 +++++++++++++++ .../virnetdaemondata/output-data-anon-clients.json | 1 + .../output-data-initial-nomdns.json | 1 + tests/virnetdaemondata/output-data-initial.json | 1 + tests/virnetdaemontest.c | 40 ++--- tools/virt-admin.c | 62 +++++++ 30 files changed, 946 insertions(+), 131 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h create mode 100644 tests/virnetdaemondata/input-data-admin-server-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-server-names.json -- 2.4.3

By adding this element, we'll be able to reference servers on client side. This is merely because when listing clients or managing clients, it would be convenient to know which server they're connected to. Also reflect this change in virnetdaemontest as well. --- daemon/libvirtd.c | 2 ++ src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 16 ++++++++++++++-- src/rpc/virnetserver.h | 1 + tests/virnetdaemontest.c | 2 +- 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 250094b..de4953d 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1390,6 +1390,7 @@ int main(int argc, char **argv) { config->keepalive_interval, config->keepalive_count, config->mdns_adv ? config->mdns_name : NULL, + "libvirtd", remoteClientInitHook, NULL, remoteClientFreeFunc, @@ -1464,6 +1465,7 @@ int main(int argc, char **argv) { config->admin_keepalive_interval, config->admin_keepalive_count, NULL, + "admin", remoteAdmClientInitHook, NULL, remoteAdmClientFreeFunc, diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index a5a40fe..568b657 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -162,7 +162,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) if (!(srv = virNetServerNew(1, 1, 0, config->max_clients, config->max_clients, -1, 0, - NULL, + NULL, "virtlockd", virLockDaemonClientNew, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 6da5876..ea7bfcf 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -152,7 +152,7 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) if (!(logd->srv = virNetServerNew(1, 1, 0, config->max_clients, config->max_clients, -1, 0, - NULL, + NULL, "virtlogd", virLogDaemonClientNew, virLogDaemonClientPreExecRestart, virLogDaemonClientFree, diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 3e5d2b4..149b6d5 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -930,7 +930,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl) if (!(srv = virNetServerNew(0, 0, 0, 1, 0, -1, 0, - NULL, + NULL, "LXC", virLXCControllerClientPrivateNew, NULL, virLXCControllerClientPrivateFree, diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 547e52e..d4dc41f 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -36,7 +36,6 @@ VIR_LOG_INIT("rpc.netserver"); - typedef struct _virNetServerJob virNetServerJob; typedef virNetServerJob *virNetServerJobPtr; @@ -49,6 +48,7 @@ struct _virNetServerJob { struct _virNetServer { virObjectLockable parent; + char *name; virThreadPoolPtr workers; char *mdnsGroupName; @@ -312,6 +312,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, int keepaliveInterval, unsigned int keepaliveCount, const char *mdnsGroupName, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, @@ -341,6 +342,9 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv->clientPrivFree = clientPrivFree; srv->clientPrivOpaque = clientPrivOpaque; + if (VIR_STRDUP(srv->name, serverName) < 0) + goto error; + if (VIR_STRDUP(srv->mdnsGroupName, mdnsGroupName) < 0) goto error; if (srv->mdnsGroupName) { @@ -378,6 +382,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int keepaliveInterval; unsigned int keepaliveCount; const char *mdnsGroupName = NULL; + const char *serverName = NULL; if (virJSONValueObjectGetNumberUint(object, "min_workers", &min_workers) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -431,7 +436,7 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, priority_workers, max_clients, max_anonymous_clients, keepaliveInterval, keepaliveCount, - mdnsGroupName, + mdnsGroupName, serverName, clientPrivNew, clientPrivPreExecRestart, clientPrivFree, clientPrivOpaque))) goto error; @@ -525,6 +530,13 @@ virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv) if (!(object = virJSONValueNewObject())) goto error; + if (srv->name && + virJSONValueObjectAppendString(object, "name", srv->name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot set name data in JSON document")); + goto error; + } + if (virJSONValueObjectAppendNumberUint(object, "min_workers", virThreadPoolGetMinWorkers(srv->workers)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 89d8db9..fb04aa3 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -42,6 +42,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, int keepaliveInterval, unsigned int keepaliveCount, const char *mdnsGroupName, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 24cbd54..754b6fb 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -51,7 +51,7 @@ testCreateServer(const char *host, int family) if (!(srv = virNetServerNew(10, 50, 5, 100, 10, 120, 5, - mdns_group, + mdns_group, "test-server", NULL, NULL, NULL, -- 2.4.3

On Mon, Nov 30, 2015 at 04:03:00PM +0100, Erik Skultety wrote:
By adding this element, we'll be able to reference servers on client side. This is merely because when listing clients or managing clients, it would be convenient to know which server they're connected to. Also reflect this change in virnetdaemontest as well. --- daemon/libvirtd.c | 2 ++ src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 16 ++++++++++++++-- src/rpc/virnetserver.h | 1 + tests/virnetdaemontest.c | 2 +- 7 files changed, 21 insertions(+), 6 deletions(-)
ACK

On Mon, Nov 30, 2015 at 04:03:00PM +0100, Erik Skultety wrote:
By adding this element, we'll be able to reference servers on client side. This is merely because when listing clients or managing clients, it would be convenient to know which server they're connected to. Also reflect this change in virnetdaemontest as well. --- daemon/libvirtd.c | 2 ++ src/locking/lock_daemon.c | 2 +- src/logging/log_daemon.c | 2 +- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetserver.c | 16 ++++++++++++++-- src/rpc/virnetserver.h | 1 + tests/virnetdaemontest.c | 2 +- 7 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 547e52e..d4dc41f 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -36,7 +36,6 @@
VIR_LOG_INIT("rpc.netserver");
-
I forgot to tell you to leave this double empty line in.
typedef struct _virNetServerJob virNetServerJob; typedef virNetServerJob *virNetServerJobPtr;

Since the daemon can manage and add (at fresh start) multiple servers, we also should be able to add them from a JSON state file in case of a daemon restart. This patch introduces virNetDaemonAddServersPostExec method which harvests the data about servers from a JSON file supporting both old format with a single server and a new one storing an array of servers. The method makes use of the original virNetDaemonAddServerPostExec, declaring the latter as static. Patch also updates virnetdaemontest accordingly. --- src/locking/lock_daemon.c | 1 + src/logging/log_daemon.c | 1 + src/rpc/virnetdaemon.c | 2 + src/rpc/virnetdaemon.h | 1 + src/rpc/virnetserver.c | 6 +- src/rpc/virnetserver.h | 1 + .../input-data-admin-server-names.json | 128 +++++++++++++++++++++ .../virnetdaemondata/output-data-admin-nomdns.json | 2 + .../output-data-admin-server-names.json | 128 +++++++++++++++++++++ .../virnetdaemondata/output-data-anon-clients.json | 1 + .../output-data-initial-nomdns.json | 1 + tests/virnetdaemondata/output-data-initial.json | 1 + tests/virnetdaemontest.c | 40 +++---- 13 files changed, 288 insertions(+), 25 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-admin-server-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-server-names.json diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 568b657..5ebb972 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -267,6 +267,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) goto error; if (!(srv = virNetDaemonAddServerPostExec(lockd->dmn, + "virtlockd", virLockDaemonClientNew, virLockDaemonClientNewPostExecRestart, virLockDaemonClientPreExecRestart, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index ea7bfcf..9b7be3c 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -209,6 +209,7 @@ virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) goto error; if (!(logd->srv = virNetDaemonAddServerPostExec(logd->dmn, + "virtlogd", virLogDaemonClientNew, virLogDaemonClientNewPostExecRestart, virLogDaemonClientPreExecRestart, diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 910f266..5324873 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -206,6 +206,7 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -236,6 +237,7 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, } srv = virNetServerNewPostExecRestart(object, + serverName, clientPrivNew, clientPrivNewPostExecRestart, clientPrivPreExecRestart, diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index bb32053..bb7de29 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -40,6 +40,7 @@ virNetDaemonPtr virNetDaemonNew(void); int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr); virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index d4dc41f..2e06dcc 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -363,6 +363,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -382,7 +383,10 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, unsigned int keepaliveInterval; unsigned int keepaliveCount; const char *mdnsGroupName = NULL; - const char *serverName = NULL; + const char *srvName = NULL; + + if (!(srvName = virJSONValueObjectGetString(object, "name"))) + srvName = serverName; if (virJSONValueObjectGetNumberUint(object, "min_workers", &min_workers) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index fb04aa3..60707d1 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -49,6 +49,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, void *clientPrivOpaque); virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, diff --git a/tests/virnetdaemondata/input-data-admin-server-names.json b/tests/virnetdaemondata/input-data-admin-server-names.json new file mode 100644 index 0000000..608023f --- /dev/null +++ b/tests/virnetdaemondata/input-data-admin-server-names.json @@ -0,0 +1,128 @@ +{ + "servers": [ + { + "name": "testServer0", + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "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": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + }, + { + "name": "testServer1", + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "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": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "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 a814aeb..4b09311 100644 --- a/tests/virnetdaemondata/output-data-admin-nomdns.json +++ b/tests/virnetdaemondata/output-data-admin-nomdns.json @@ -1,6 +1,7 @@ { "servers": [ { + "name": "testServer0", "min_workers": 10, "max_workers": 50, "priority_workers": 5, @@ -62,6 +63,7 @@ ] }, { + "name": "testServer1", "min_workers": 2, "max_workers": 50, "priority_workers": 5, diff --git a/tests/virnetdaemondata/output-data-admin-server-names.json b/tests/virnetdaemondata/output-data-admin-server-names.json new file mode 100644 index 0000000..4b09311 --- /dev/null +++ b/tests/virnetdaemondata/output-data-admin-server-names.json @@ -0,0 +1,128 @@ +{ + "servers": [ + { + "name": "testServer0", + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 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": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + }, + { + "name": "testServer1", + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 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": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] + } + ] +} diff --git a/tests/virnetdaemondata/output-data-anon-clients.json b/tests/virnetdaemondata/output-data-anon-clients.json index 05fc0ae..043f072 100644 --- a/tests/virnetdaemondata/output-data-anon-clients.json +++ b/tests/virnetdaemondata/output-data-anon-clients.json @@ -1,6 +1,7 @@ { "servers": [ { + "name": "testServer0", "min_workers": 10, "max_workers": 50, "priority_workers": 5, diff --git a/tests/virnetdaemondata/output-data-initial-nomdns.json b/tests/virnetdaemondata/output-data-initial-nomdns.json index 400e47b..00bebc3 100644 --- a/tests/virnetdaemondata/output-data-initial-nomdns.json +++ b/tests/virnetdaemondata/output-data-initial-nomdns.json @@ -1,6 +1,7 @@ { "servers": [ { + "name": "testServer0", "min_workers": 10, "max_workers": 50, "priority_workers": 5, diff --git a/tests/virnetdaemondata/output-data-initial.json b/tests/virnetdaemondata/output-data-initial.json index e875cff..63a4872 100644 --- a/tests/virnetdaemondata/output-data-initial.json +++ b/tests/virnetdaemondata/output-data-initial.json @@ -1,6 +1,7 @@ { "servers": [ { + "name": "testServer0", "min_workers": 10, "max_workers": 50, "priority_workers": 5, diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 754b6fb..b487235 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -28,7 +28,7 @@ #ifdef HAVE_SOCKETPAIR static virNetServerPtr -testCreateServer(const char *host, int family) +testCreateServer(const char *host, int family, const char *name) { virNetServerPtr srv = NULL; virNetServerServicePtr svc1 = NULL, svc2 = NULL; @@ -51,7 +51,7 @@ testCreateServer(const char *host, int family) if (!(srv = virNetServerNew(10, 50, 5, 100, 10, 120, 5, - mdns_group, "test-server", + mdns_group, name, NULL, NULL, NULL, @@ -135,7 +135,7 @@ testCreateServer(const char *host, int family) goto cleanup; } -static char *testGenerateJSON(void) +static char *testGenerateJSON(const char *server_name) { virNetDaemonPtr dmn = NULL; virNetServerPtr srv = NULL; @@ -157,7 +157,7 @@ static char *testGenerateJSON(void) if (!(srv = testCreateServer( has_ipv4 ? "127.0.0.1" : "::1", - has_ipv4 ? AF_INET : AF_INET6))) + has_ipv4 ? AF_INET : AF_INET6, server_name))) goto cleanup; if (!(dmn = virNetDaemonNew())) @@ -186,8 +186,8 @@ static char *testGenerateJSON(void) struct testExecRestartData { const char *jsonfile; + const char **serverNames; int nservers; - bool pass; }; static int testExecRestart(const void *opaque) @@ -241,7 +241,7 @@ static int testExecRestart(const void *opaque) goto cleanup; for (i = 0; i < data->nservers; i++) { - if (!(srv = virNetDaemonAddServerPostExec(dmn, + if (!(srv = virNetDaemonAddServerPostExec(dmn, data->serverNames[i], NULL, NULL, NULL, NULL, NULL))) goto cleanup; @@ -257,19 +257,11 @@ static int testExecRestart(const void *opaque) if (virtTestCompareToFile(outjsonstr, outfile) < 0) goto cleanup; - if (!data->pass) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "Test should've failed"); - goto cleanup; - } - ret = 0; cleanup: - if (ret < 0) { - if (!data->pass) - ret = 0; - else - virDispatchError(NULL); - } + if (ret < 0) + virDispatchError(NULL); + VIR_FREE(infile); VIR_FREE(outfile); VIR_FREE(injsonstr); @@ -289,6 +281,7 @@ static int mymain(void) { int ret = 0; + const char *server_names[] = { "testServer0", "testServer1" }; if (virInitialize() < 0 || virEventRegisterDefaultImpl() < 0) { @@ -302,7 +295,7 @@ mymain(void) * numbers with 100, 101, 102, 103. */ if (getenv("VIR_GENERATE_JSON")) { - char *json = testGenerateJSON(); + char *json = testGenerateJSON(server_names[0]); if (!json) return EXIT_FAILURE; @@ -311,26 +304,25 @@ mymain(void) return ret; } -# define EXEC_RESTART_TEST_FULL(file, servers, pass) \ +# define EXEC_RESTART_TEST_FULL(file, nservers) \ do { \ struct testExecRestartData data = { \ - file, servers, pass \ + file, server_names, nservers \ }; \ if (virtTestRun("ExecRestart " file, \ testExecRestart, &data) < 0) \ ret = -1; \ } while (0) -# define EXEC_RESTART_TEST(file) EXEC_RESTART_TEST_FULL(file, 1, true) +# define EXEC_RESTART_TEST(file) EXEC_RESTART_TEST_FULL(file, 1) # ifdef WITH_AVAHI EXEC_RESTART_TEST("initial"); # endif EXEC_RESTART_TEST("initial-nomdns"); EXEC_RESTART_TEST("anon-clients"); - - EXEC_RESTART_TEST_FULL("anon-clients", 2, false); - EXEC_RESTART_TEST_FULL("admin-nomdns", 2, true); + EXEC_RESTART_TEST_FULL("admin-nomdns", 2); + EXEC_RESTART_TEST_FULL("admin-server-names", 2); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3

On Mon, Nov 30, 2015 at 04:03:01PM +0100, Erik Skultety wrote:
Since the daemon can manage and add (at fresh start) multiple servers, we also should be able to add them from a JSON state file in case of a daemon restart. This patch introduces virNetDaemonAddServersPostExec method which harvests the data about servers from a JSON file supporting both old format with a single server and a new one storing an array of servers. The method makes use of the original virNetDaemonAddServerPostExec, declaring the latter as static. Patch also updates virnetdaemontest accordingly.
There's only one thing wrong with this patch: In virnetdaemontest you remove the ability for testing something that should've failed. You also effectively remove one such test. Was it because you forgot about it or did you remove it intentionally? I would prefer it to stay there. The removal is the only thing keeping me from acking this patch. One more tiny thing below.
diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 754b6fb..b487235 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -157,7 +157,7 @@ static char *testGenerateJSON(void)
if (!(srv = testCreateServer( has_ipv4 ? "127.0.0.1" : "::1",
Join these two lines together ^^
- has_ipv4 ? AF_INET : AF_INET6))) + has_ipv4 ? AF_INET : AF_INET6, server_name)))
and then indent this one accordingly.

On 10/01/16 16:27, Martin Kletzander wrote:
On Mon, Nov 30, 2015 at 04:03:01PM +0100, Erik Skultety wrote:
Since the daemon can manage and add (at fresh start) multiple servers, we also should be able to add them from a JSON state file in case of a daemon restart. This patch introduces virNetDaemonAddServersPostExec method which harvests the data about servers from a JSON file supporting both old format with a single server and a new one storing an array of servers. The method makes use of the original virNetDaemonAddServerPostExec, declaring the latter as static. Patch also updates virnetdaemontest accordingly.
There's only one thing wrong with this patch: In virnetdaemontest you remove the ability for testing something that should've failed. You also effectively remove one such test. Was it because you forgot about it or did you remove it intentionally? I would prefer it to stay there. The removal is the only thing keeping me from acking this patch.
Hmm, I can't think of any reasons why I did that, but you're definitely right about including such a test in there. However, I did a small investigation and found out, that the current solution doesn't work as expected, see the following scenarios: SCENARIO 1} Include a test, that should pass, but fails unexpectedly OUT: TEST: virnetdaemontest 1) ExecRestart initial ... OK 2) ExecRestart initial-nomdns ... OK 3) ExecRestart anon-clients ... OK 4) ExecRestart anon-clients ... libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec FAILED 5) ExecRestart admin-nomdns ... OK I separated test case 4 so it is clear that there are 2 identical error messages caused by multiple calls to virDispatchError, first being in virnetdaemontest and the other one in virtTestRun routine. SCENARIO 2} Include a test which should fail, but would actually pass TEST: virnetdaemontest 1) ExecRestart initial ... OK 2) ExecRestart initial-nomdns ... OK 3) ExecRestart anon-clients ... OK 4) ExecRestart anon-clients ... libvirt: XML-RPC error : internal error: Cannot add more servers post-exec than there were pre-exec OK 5) ExecRestart admin-nomdns ... libvirt: XML-RPC error : internal error: Test should've failed OK Testcase 5 failed with error stating that the test should have failed the execution but it didn't. Yet, the testcase returns success. I'll include all necessary changes in this patch once I adjust all the remaining ones in this v3 series and post them all. Erik

This change is merely because admin_server would contain all the code from dispatchers and helpers to the actual APIs. Admin should have similar structure to the daemon-side remote driver - dispatchers and helpers in a separate module, APIs in a separate module. Best viewed with -M. --- daemon/Makefile.am | 6 ++- daemon/admin.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++ daemon/admin.h | 36 +++++++++++++++ daemon/admin_server.c | 126 -------------------------------------------------- daemon/admin_server.h | 36 --------------- daemon/libvirtd.c | 2 +- po/POTFILES.in | 2 +- 7 files changed, 168 insertions(+), 166 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h delete mode 100644 daemon/admin_server.c delete mode 100644 daemon/admin_server.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index be1b5a9..5b13960 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -42,6 +42,7 @@ DAEMON_SOURCES = \ libvirtd.c libvirtd.h \ remote.c remote.h \ stream.c stream.h \ + admin.c admin.h \ $(DAEMON_GENERATED) LIBVIRTD_CONF_SOURCES = libvirtd-config.c libvirtd-config.h @@ -128,7 +129,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirtd_admin.la libvirtd_admin_la_SOURCES = \ - admin_server.c admin_server.h + admin.c admin.h libvirtd_admin_la_CFLAGS = \ $(AM_CFLAGS) \ @@ -319,7 +320,8 @@ endif ! WITH_POLKIT remote.c: $(DAEMON_GENERATED) remote.h: $(DAEMON_GENERATED) -admin_server.c: $(DAEMON_GENERATED) +admin.c: $(DAEMON_GENERATED) +admin.h: $(DAEMON_GENERATED) LOGROTATE_CONFS = libvirtd.qemu.logrotate libvirtd.lxc.logrotate \ libvirtd.libxl.logrotate libvirtd.uml.logrotate \ diff --git a/daemon/admin.c b/daemon/admin.c new file mode 100644 index 0000000..6899318 --- /dev/null +++ b/daemon/admin.c @@ -0,0 +1,126 @@ +/* + * admin.c: handlers for admin RPC method calls + * + * Copyright (C) 2014-2015 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: Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include "internal.h" +#include "libvirtd.h" +#include "libvirt_internal.h" + +#include "admin_protocol.h" +#include "admin.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" +#include "virthreadjob.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin"); + + +void +remoteAdmClientFreeFunc(void *data) +{ + struct daemonAdmClientPrivate *priv = data; + + virMutexDestroy(&priv->lock); + virObjectUnref(priv->dmn); + VIR_FREE(priv); +} + +void * +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) +{ + struct daemonAdmClientPrivate *priv; + + if (VIR_ALLOC(priv) < 0) + return NULL; + + if (virMutexInit(&priv->lock) < 0) { + VIR_FREE(priv); + virReportSystemError(errno, "%s", _("unable to init mutex")); + return NULL; + } + + /* + * We don't necessarily need to ref this object right now as there + * must be one ref being held throughout the life of the daemon, + * but let's just be safe for future. + */ + priv->dmn = virObjectRef(opaque); + + return priv; +} + +/* Functions */ +static int +adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr, + struct admin_connect_open_args *args) +{ + unsigned int flags; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + int ret = -1; + + VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); + virMutexLock(&priv->lock); + + flags = args->flags; + virCheckFlagsGoto(0, cleanup); + + ret = 0; + cleanup: + if (ret < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return ret; +} + +static int +adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) +{ + virNetServerClientDelayedClose(client); + return 0; +} + +static int +adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + unsigned long long *libVer) +{ + if (libVer) + *libVer = LIBVIR_VERSION_NUMBER; + return 0; +} + +#include "admin_dispatch.h" diff --git a/daemon/admin.h b/daemon/admin.h new file mode 100644 index 0000000..1e6802a --- /dev/null +++ b/daemon/admin.h @@ -0,0 +1,36 @@ +/* + * admin.h: handlers for admin RPC method calls + * + * Copyright (C) 2015 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: Martin Kletzander <mkletzan@redhat.com> + */ + +#ifndef __LIBVIRTD_ADMIN_H__ +# define __LIBVIRTD_ADMIN_H__ + +# include "rpc/virnetserverprogram.h" +# include "rpc/virnetserverclient.h" + + +extern virNetServerProgramProc adminProcs[]; +extern size_t adminNProcs; + +void remoteAdmClientFreeFunc(void *data); +void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); + +#endif /* __ADMIN_REMOTE_H__ */ diff --git a/daemon/admin_server.c b/daemon/admin_server.c deleted file mode 100644 index 189091e..0000000 --- a/daemon/admin_server.c +++ /dev/null @@ -1,126 +0,0 @@ -/* - * admin_server.c: - * - * Copyright (C) 2014-2015 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: Martin Kletzander <mkletzan@redhat.com> - */ - -#include <config.h> - -#include "internal.h" -#include "libvirtd.h" -#include "libvirt_internal.h" - -#include "admin_protocol.h" -#include "admin_server.h" -#include "datatypes.h" -#include "viralloc.h" -#include "virerror.h" -#include "virlog.h" -#include "virnetdaemon.h" -#include "virnetserver.h" -#include "virstring.h" -#include "virthreadjob.h" - -#define VIR_FROM_THIS VIR_FROM_ADMIN - -VIR_LOG_INIT("daemon.admin"); - - -void -remoteAdmClientFreeFunc(void *data) -{ - struct daemonAdmClientPrivate *priv = data; - - virMutexDestroy(&priv->lock); - virObjectUnref(priv->dmn); - VIR_FREE(priv); -} - -void * -remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, - void *opaque) -{ - struct daemonAdmClientPrivate *priv; - - if (VIR_ALLOC(priv) < 0) - return NULL; - - if (virMutexInit(&priv->lock) < 0) { - VIR_FREE(priv); - virReportSystemError(errno, "%s", _("unable to init mutex")); - return NULL; - } - - /* - * We don't necessarily need to ref this object right now as there - * must be one ref being held throughout the life of the daemon, - * but let's just be safe for future. - */ - priv->dmn = virObjectRef(opaque); - - return priv; -} - -/* Functions */ -static int -adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr, - struct admin_connect_open_args *args) -{ - unsigned int flags; - struct daemonAdmClientPrivate *priv = - virNetServerClientGetPrivateData(client); - int ret = -1; - - VIR_DEBUG("priv=%p dmn=%p", priv, priv->dmn); - virMutexLock(&priv->lock); - - flags = args->flags; - virCheckFlagsGoto(0, cleanup); - - ret = 0; - cleanup: - if (ret < 0) - virNetMessageSaveError(rerr); - virMutexUnlock(&priv->lock); - return ret; -} - -static int -adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED, - virNetServerClientPtr client, - virNetMessagePtr msg ATTRIBUTE_UNUSED, - virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED) -{ - virNetServerClientDelayedClose(client); - return 0; -} - -static int -adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, - unsigned long long *libVer) -{ - if (libVer) - *libVer = LIBVIR_VERSION_NUMBER; - return 0; -} - -#include "admin_dispatch.h" diff --git a/daemon/admin_server.h b/daemon/admin_server.h deleted file mode 100644 index 26721a6..0000000 --- a/daemon/admin_server.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * admin_server.h - * - * Copyright (C) 2014 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: Martin Kletzander <mkletzan@redhat.com> - */ - -#ifndef __LIBVIRTD_ADMIN_H__ -# define __LIBVIRTD_ADMIN_H__ - -# include "rpc/virnetserverprogram.h" -# include "rpc/virnetserverclient.h" - - -extern virNetServerProgramProc adminProcs[]; -extern size_t adminNProcs; - -void remoteAdmClientFreeFunc(void *data); -void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); - -#endif /* __ADMIN_REMOTE_H__ */ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index de4953d..7c17ce7 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -44,7 +44,7 @@ #include "libvirtd.h" #include "libvirtd-config.h" -#include "admin_server.h" +#include "admin.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" diff --git a/po/POTFILES.in b/po/POTFILES.in index 82e8d3e..4e17676 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,5 +1,5 @@ daemon/admin_dispatch.h -daemon/admin_server.c +daemon/admin.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.3

On Mon, Nov 30, 2015 at 04:03:02PM +0100, Erik Skultety wrote:
This change is merely because admin_server would contain all the code from dispatchers and helpers to the actual APIs. Admin should have similar structure to the daemon-side remote driver - dispatchers and helpers in a separate module, APIs in a separate module.
Best viewed with -M.
And if you saw it with -M you would see this is just a rename. Apart from two things:
diff --git a/daemon/Makefile.am b/daemon/Makefile.am index be1b5a9..5b13960 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -42,6 +42,7 @@ DAEMON_SOURCES = \ libvirtd.c libvirtd.h \ remote.c remote.h \ stream.c stream.h \ + admin.c admin.h \
a) You are compiling the admin.[ch] inside the daemon for some reason, even though it's in libvirtd_admin_la that is added there as well.
@@ -319,7 +320,8 @@ endif ! WITH_POLKIT
remote.c: $(DAEMON_GENERATED) remote.h: $(DAEMON_GENERATED) -admin_server.c: $(DAEMON_GENERATED) +admin.c: $(DAEMON_GENERATED) +admin.h: $(DAEMON_GENERATED)
This was missing, not really needed, but nicer to have it mentioned.
diff --git a/daemon/admin_server.h b/daemon/admin_server.h deleted file mode 100644 index 26721a6..0000000 --- a/daemon/admin_server.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * admin_server.h - * - * Copyright (C) 2014 Red Hat, Inc. - *
b) You changed this 2014 copyright ...
diff --git a/daemon/admin.h b/daemon/admin.h new file mode 100644 index 0000000..1e6802a --- /dev/null +++ b/daemon/admin.h @@ -0,0 +1,36 @@ +/* + * admin.h: handlers for admin RPC method calls + * + * Copyright (C) 2015 Red Hat, Inc. + *
... to 2015 here. ACK with those two things fixed.

This is the key structure of all management operations performed on the daemon/clients. An admin client needs to be able to identify another client (either admin or non-privileged client) to perform an action on it. This identification includes a server the client is connected to, thus a client-side representation of a server is needed. --- include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 8 ++++++++ src/datatypes.c | 36 ++++++++++++++++++++++++++++++++++++ src/datatypes.h | 34 ++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 3 +++ 5 files changed, 85 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index ab9df96..23420c7 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -42,6 +42,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect; +typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -51,6 +53,8 @@ typedef struct _virAdmConnect virAdmConnect; */ typedef virAdmConnect *virAdmConnectPtr; +typedef virAdmServer *virAdmServerPtr; + virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 878983d..2f1f534 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,12 +30,20 @@ */ const ADMIN_STRING_MAX = 4194304; +/* Upper limit on list of servers */ +const ADMIN_SERVER_LIST_MAX = 16384; + /* A long string, which may NOT be NULL. */ typedef string admin_nonnull_string<ADMIN_STRING_MAX>; /* A long string, which may be NULL. */ typedef admin_nonnull_string *admin_string; +/* A server which may NOT be NULL */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/datatypes.c b/src/datatypes.c index c832d80..cfc0d06 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -61,10 +61,14 @@ static void virStoragePoolDispose(void *obj); virClassPtr virAdmConnectClass; virClassPtr virAdmConnectCloseCallbackDataClass; +virClassPtr virAdmServerClass; static void virAdmConnectDispose(void *obj); static void virAdmConnectCloseCallbackDataDispose(void *obj); +static void virAdmConnectDispose(void *obj); +static void virAdmServerDispose(void *obj); + static int virDataTypesOnceInit(void) { @@ -92,6 +96,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool); + DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect); DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData); @@ -859,3 +864,34 @@ virAdmConnectCloseCallbackDataDispose(void *obj) virObjectUnlock(cb_data); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name) +{ + virAdmServerPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + goto error; + + if (!(ret = virObjectNew(virAdmServerClass))) + goto error; + if (VIR_STRDUP(ret->name, name) < 0) + goto error; + + ret->conn = virObjectRef(conn); + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %s", srv, srv->name); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index 1b1777d..fc84ac6 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -42,6 +42,7 @@ extern virClassPtr virStorageVolClass; extern virClassPtr virStoragePoolClass; extern virClassPtr virAdmConnectClass; +extern virClassPtr virAdmServerClass; # define virCheckConnectReturn(obj, retval) \ do { \ @@ -317,6 +318,26 @@ extern virClassPtr virAdmConnectClass; } \ } while (0) +# define virCheckAdmServerReturn(obj, retval) \ + do { \ + if (!virObjectIsClass(obj, virAdmServerClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) +# define virCheckAdmServerGoto(obj, label) \ + do { \ + if (!virObjectIsClass(obj, virAdmServerClass)) { \ + virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0); + /** * VIR_DOMAIN_DEBUG: * @dom: domain @@ -417,6 +438,17 @@ struct _virAdmConnect { virAdmConnectCloseCallbackDataPtr closeCallback; }; +/** + * _virAdmServer: + * + * Internal structure associated to a daemon server + */ +struct _virAdmServer { + virObject object; + virAdmConnectPtr conn; /* pointer back to the admin connection */ + char *name; /* the server external name */ +}; + /** * _virDomain: @@ -601,4 +633,6 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, virAdmConnectPtr virAdmConnectNew(void); +virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, + const char *name); #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 85380dc..31f1f8d 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -9,6 +9,9 @@ xdr_admin_connect_get_lib_version_ret; xdr_admin_connect_open_args; +# datatypes.h +virAdmGetServer; + # Let emacs know we want case-insensitive sorting # Local Variables: # sort-fold-case: t -- 2.4.3

On Mon, Nov 30, 2015 at 04:03:03PM +0100, Erik Skultety wrote:
This is the key structure of all management operations performed on the daemon/clients. An admin client needs to be able to identify another client (either admin or non-privileged client) to perform an action on it. This identification includes a server the client is connected to, thus a client-side representation of a server is needed. --- include/libvirt/libvirt-admin.h | 4 ++++ src/admin/admin_protocol.x | 8 ++++++++ src/datatypes.c | 36 ++++++++++++++++++++++++++++++++++++ src/datatypes.h | 34 ++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 3 +++ 5 files changed, 85 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index ab9df96..23420c7 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -42,6 +42,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect;
+typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -51,6 +53,8 @@ typedef struct _virAdmConnect virAdmConnect; */ typedef virAdmConnect *virAdmConnectPtr;
+typedef virAdmServer *virAdmServerPtr; +
Some (even one-line) docstring would be nice since this is a public header file.
diff --git a/src/datatypes.c b/src/datatypes.c index c832d80..cfc0d06 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -61,10 +61,14 @@ static void virStoragePoolDispose(void *obj);
virClassPtr virAdmConnectClass; virClassPtr virAdmConnectCloseCallbackDataClass; +virClassPtr virAdmServerClass;
static void virAdmConnectDispose(void *obj); static void virAdmConnectCloseCallbackDataDispose(void *obj);
+static void virAdmConnectDispose(void *obj);
Duplicated for no reason.
+static void virAdmServerDispose(void *obj); + static int virDataTypesOnceInit(void) { @@ -92,6 +96,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool);
+ DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect); DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData);
@@ -859,3 +864,34 @@ virAdmConnectCloseCallbackDataDispose(void *obj)
virObjectUnlock(cb_data); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name) +{ + virAdmServerPtr ret = NULL; + + if (virDataTypesInitialize() < 0) + goto error; + + if (!(ret = virObjectNew(virAdmServerClass))) + goto error; + if (VIR_STRDUP(ret->name, name) < 0) + goto error; + + ret->conn = virObjectRef(conn); + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %s", srv, srv->name); +
Some labels for the values would be nice, at least single quotes.
+ VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index 1b1777d..fc84ac6 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -42,6 +42,7 @@ extern virClassPtr virStorageVolClass; extern virClassPtr virStoragePoolClass;
extern virClassPtr virAdmConnectClass; +extern virClassPtr virAdmServerClass;
# define virCheckConnectReturn(obj, retval) \ do { \ @@ -317,6 +318,26 @@ extern virClassPtr virAdmConnectClass; } \ } while (0)
+# define virCheckAdmServerReturn(obj, retval) \ + do { \ + if (!virObjectIsClass(obj, virAdmServerClass)) { \
You should check that obj->conn is virAdmConnectClass
+ virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + virDispatchError(NULL); \ + return retval; \ + } \ + } while (0) +# define virCheckAdmServerGoto(obj, label) \ + do { \ + if (!virObjectIsClass(obj, virAdmServerClass)) { \
same here But the thing I was wondering the most is, how would you call a public function that will return virAdmServerPtr based on its name? I hope it's not virAdmGetServerFlags() =D Anyway, that's just a joke. ACK with those aforementioned things fixed.

This API is merely a convenience API, i.e. when managing clients connected to daemon's servers, we should know (convenience) which server the specific client is connected to. This implies a client-side representation of a server along with a basic API to let the administrating client know what servers are actually available on the daemon. --- daemon/Makefile.am | 2 +- daemon/admin.c | 55 +++++++++++++++ daemon/admin_server.c | 79 +++++++++++++++++++++ daemon/admin_server.h | 33 +++++++++ include/libvirt/libvirt-admin.h | 7 ++ src/admin/admin_protocol.x | 18 ++++- src/admin_protocol-structs | 15 ++++ src/libvirt-admin.c | 148 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 2 + src/libvirt_admin_public.syms | 3 + src/rpc/virnetdaemon.c | 13 ++++ src/rpc/virnetdaemon.h | 2 + src/rpc/virnetserver.c | 12 ++++ src/rpc/virnetserver.h | 3 + 14 files changed, 390 insertions(+), 2 deletions(-) create mode 100644 daemon/admin_server.c create mode 100644 daemon/admin_server.h diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 5b13960..d255c89 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -129,7 +129,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) noinst_LTLIBRARIES += libvirtd_admin.la libvirtd_admin_la_SOURCES = \ - admin.c admin.h + admin.c admin.h admin_server.c admin_server.h libvirtd_admin_la_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/daemon/admin.c b/daemon/admin.c index 6899318..0db7806 100644 --- a/daemon/admin.c +++ b/daemon/admin.c @@ -28,6 +28,7 @@ #include "admin_protocol.h" #include "admin.h" +#include "admin_server.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" @@ -77,6 +78,15 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; } +/* Helpers */ + +static void +make_nonnull_server(admin_nonnull_server *srv_dst, + virAdmServerPtr srv_src) +{ + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -123,4 +133,49 @@ adminConnectGetLibVersion(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, return 0; } +static int +adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + admin_connect_list_servers_args *args, + admin_connect_list_servers_ret *ret) +{ + virAdmServerPtr *servers = NULL; + int nservers = 0; + int rv = -1; + size_t i; + struct daemonAdmClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if ((nservers = + adminDaemonListServers(priv->dmn, + args->need_results ? &servers : NULL, + args->flags)) < 0) + goto cleanup; + + if (servers && nservers) { + if (VIR_ALLOC_N(ret->servers.servers_val, nservers) < 0) + goto cleanup; + + ret->servers.servers_len = nservers; + for (i = 0; i < nservers; i++) + make_nonnull_server(ret->servers.servers_val + i, servers[i]); + } else { + ret->servers.servers_len = 0; + ret->servers.servers_val = NULL; + } + + ret->ret = nservers; + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + if (servers && nservers > 0) + for (i = 0; i < nservers; i++) + virObjectUnref(servers[i]); + VIR_FREE(servers); + return rv; +} #include "admin_dispatch.h" diff --git a/daemon/admin_server.c b/daemon/admin_server.c new file mode 100644 index 0000000..0cdd6b3 --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,79 @@ +/* + * admin_server.c: admin methods to manage daemons and clients + * + * Copyright (C) 2014-2015 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: Martin Kletzander <mkletzan@redhat.com> + */ + +#include <config.h> + +#include "admin_server.h" +#include "datatypes.h" +#include "viralloc.h" +#include "virerror.h" +#include "virlog.h" +#include "virnetdaemon.h" +#include "virnetserver.h" +#include "virstring.h" + +#define VIR_FROM_THIS VIR_FROM_ADMIN + +VIR_LOG_INIT("daemon.admin_server"); + +int +adminDaemonListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + const char *name = NULL; + virNetServerPtr *srv_objs = NULL; + virAdmServerPtr *srvs = NULL; + size_t i; + size_t nsrvs = 0; + + virCheckFlags(0, -1); + + nsrvs = virNetDaemonGetServers(dmn, &srv_objs); + if (servers) { + if (VIR_ALLOC_N(srvs, nsrvs + 1) < 0) + goto cleanup; + + for (i = 0; i < nsrvs; i++) { + virNetServerPtr srv = srv_objs[i]; + + name = virNetServerGetName(srv); + virObjectLock(srv); + if (!(srvs[i] = virAdmGetServer(NULL, name))) { + virObjectUnlock(srv); + goto cleanup; + } + + virObjectUnlock(srv); + } + + *servers = srvs; + srvs = NULL; + } + + ret = nsrvs; + + cleanup: + virObjectListFree(srvs); + return ret; +} diff --git a/daemon/admin_server.h b/daemon/admin_server.h new file mode 100644 index 0000000..9558802 --- /dev/null +++ b/daemon/admin_server.h @@ -0,0 +1,33 @@ +/* + * admin_server.h: admin methods to manage daemons and clients + * + * Copyright (C) 2015 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: Martin Kletzander <mkletzan@redhat.com> + */ + +#ifndef __LIBVIRTD_ADMIN_SERVER_H__ +# define __LIBVIRTD_ADMIN_SERVER_H__ + +# include "rpc/virnetdaemon.h" + +int +adminDaemonListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags); + +#endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 23420c7..d75aad4 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -58,6 +58,11 @@ typedef virAdmServer *virAdmServerPtr; virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags); int virAdmConnectClose(virAdmConnectPtr conn); +int virAdmConnectListServers(virAdmConnectPtr conn, + virAdmServerPtr **servers, + unsigned int flags); + +int virAdmServerFree(virAdmServerPtr srv); int virAdmConnectRef(virAdmConnectPtr conn); int virAdmConnectIsAlive(virAdmConnectPtr conn); @@ -87,6 +92,8 @@ int virAdmConnectRegisterCloseCallback(virAdmConnectPtr conn, int virAdmConnectUnregisterCloseCallback(virAdmConnectPtr conn, virAdmConnectCloseFunc cb); +const char *virAdmGetServerName(virAdmServerPtr srv); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index 2f1f534..14ab2a3 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -53,6 +53,16 @@ struct admin_connect_get_lib_version_ret { unsigned hyper libVer; }; +struct admin_connect_list_servers_args { + unsigned int need_results; + unsigned int flags; +}; + +struct admin_connect_list_servers_ret { + admin_nonnull_server servers<ADMIN_SERVER_LIST_MAX>; + unsigned int ret; +}; + /* Define the program number, protocol version and procedure numbers here. */ const ADMIN_PROGRAM = 0x06900690; const ADMIN_PROTOCOL_VERSION = 1; @@ -88,5 +98,11 @@ enum admin_procedure { /** * @generate: both */ - ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3 + ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, + + /** + * @generate: none + * @priority: high + */ + ADMIN_PROC_CONNECT_LIST_SERVERS = 4 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 809379b..8f2633a 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,12 +1,27 @@ /* -*- c -*- */ +struct admin_nonnull_server { + admin_nonnull_string name; +}; struct admin_connect_open_args { u_int flags; }; struct admin_connect_get_lib_version_ret { uint64_t libVer; }; +struct admin_connect_list_servers_args { + u_int need_results; + u_int flags; +}; +struct admin_connect_list_servers_ret { + struct { + u_int servers_len; + admin_nonnull_server * servers_val; + } servers; + u_int ret; +}; enum admin_procedure { ADMIN_PROC_CONNECT_OPEN = 1, ADMIN_PROC_CONNECT_CLOSE = 2, ADMIN_PROC_CONNECT_GET_LIB_VERSION = 3, + ADMIN_PROC_CONNECT_LIST_SERVERS = 4, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 6e6da81..35fbcd0 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -45,6 +45,13 @@ VIR_LOG_INIT("libvirt-admin"); #include "admin_remote.c" +/* Helpers */ +static virAdmServerPtr +get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server) +{ + return virAdmGetServer(conn, server.name); +} + static bool virAdmGlobalError; static virOnceControl virAdmGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER; @@ -549,7 +556,148 @@ int virAdmConnectGetLibVersion(virAdmConnectPtr conn, goto error; return 0; + error: + virDispatchError(NULL); + return -1; +} + +static int +remoteAdminConnectListServers(virAdmConnectPtr conn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int rv = -1; + size_t i; + virAdmServerPtr *tmp_srvs = NULL; + remoteAdminPrivPtr priv = conn->privateData; + admin_connect_list_servers_args args; + admin_connect_list_servers_ret ret; + + args.need_results = !!servers; + args.flags = flags; + + memset(&ret, 0, sizeof(ret)); + if (call(conn, + 0, + ADMIN_PROC_CONNECT_LIST_SERVERS, + (xdrproc_t) xdr_admin_connect_list_servers_args, + (char *) &args, + (xdrproc_t) xdr_admin_connect_list_servers_ret, + (char *) &ret) == -1) + goto done; + + if (ret.servers.servers_len > ADMIN_SERVER_LIST_MAX) { + virReportError(VIR_ERR_RPC, + _("Too many servers '%d' for limit '%d'"), + ret.servers.servers_len, ADMIN_SERVER_LIST_MAX); + goto cleanup; + } + + if (servers) { + if (VIR_ALLOC_N(tmp_srvs, ret.servers.servers_len + 1) < 0) + goto cleanup; + + for (i = 0; i < ret.servers.servers_len; i++) { + tmp_srvs[i] = get_nonnull_server(conn, ret.servers.servers_val[i]); + if (!tmp_srvs[i]) + goto cleanup; + } + *servers = tmp_srvs; + tmp_srvs = NULL; + } + + rv = ret.ret; + + cleanup: + if (tmp_srvs) { + for (i = 0; i < ret.servers.servers_len; i++) + virObjectUnref(tmp_srvs[i]); + VIR_FREE(tmp_srvs); + } + + xdr_free((xdrproc_t) xdr_admin_connect_list_servers_ret, (char *) &ret); + + done: + virObjectUnlock(priv); + return rv; +} + + +/** + * virAdmGetServerName: + * @srv: a server object + * + * Get the public name for specified server + * + * Returns a pointer to the name or NULL. The string doesn't need to be + * deallocated since its lifetime will be the same as the server object. + */ +const char * +virAdmGetServerName(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + virResetLastError(); + virCheckAdmServerReturn(srv, NULL); + + return srv->name; +} + +/** + * virAdmServerFree: + * @srv: server object + * + * Release the server object. The running instance is kept alive. + * The data structure is freed and should not be used thereafter. + * + * Returns 0 on success, -1 on failure. + */ +int virAdmServerFree(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + + virResetLastError(); + virCheckAdmServerReturn(srv, -1); + + virObjectUnref(srv); + return 0; +} + +/** + * virAdmConnectListServers: + * @conn: daemon connection reference + * @servers: Pointer to a list to store an array containing objects or NULL + * if the list is not required (number of servers only) + * @flags: bitwise-OR of virAdmConnectListServersFlags + * + * Collect list of all servers provided by daemon the client is connected to. + * + * Returns the number of servers available on daemon side or -1 in case of a + * failure, setting @servers to NULL. There is a guaranteed extra element set + * to NULL in the @servers list returned to make the iteration easier, excluding + * this extra element from the final count. + * Caller is responsible to call virAdmServerFree() on each list element, + * followed by freeing @servers. + */ +int +virAdmConnectListServers(virAdmConnectPtr conn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + + VIR_DEBUG("conn=%p, servers=%p, flags=%x", conn, servers, flags); + + virResetLastError(); + + if (servers) + *servers = NULL; + + virCheckAdmConnectReturn(conn, -1); + if ((ret = remoteAdminConnectListServers(conn, servers, 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 31f1f8d..ae6b9dd 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -7,6 +7,8 @@ # admin/admin_protocol.x xdr_admin_connect_get_lib_version_ret; +xdr_admin_connect_list_servers_args; +xdr_admin_connect_list_servers_ret; xdr_admin_connect_open_args; # datatypes.h diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms index 33b1db4..7963550 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -21,4 +21,7 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectGetLibVersion; virAdmConnectRegisterCloseCallback; virAdmConnectUnregisterCloseCallback; + virAdmConnectListServers; + virAdmGetServerName; + virAdmServerFree; }; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 5324873..9adde69 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -204,6 +204,19 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, return srv; } +size_t +virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers) +{ + size_t nservers; + + virObjectLock(dmn); + nservers = dmn->nservers; + *servers = dmn->servers; + virObjectUnlock(dmn); + + return nservers; +} virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, const char *serverName, diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index bb7de29..12a90c0 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -81,5 +81,7 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, int subServerID); +size_t virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers); #endif /* __VIR_NET_DAEMON_H__ */ diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 2e06dcc..bcc9fbb 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -831,6 +831,18 @@ virNetServerHasClients(virNetServerPtr srv) return ret; } +const char * +virNetServerGetName(virNetServerPtr srv) +{ + const char *name; + + virObjectLock(srv); + name = srv->name; + virObjectUnlock(srv); + + return name; +} + void virNetServerProcessClients(virNetServerPtr srv) { diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 60707d1..c30d622 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -87,4 +87,7 @@ void virNetServerUpdateServices(virNetServerPtr srv, bool enabled); int virNetServerStart(virNetServerPtr srv); +const char *virNetServerGetName(virNetServerPtr srv); +unsigned int virNetServerGetID(virNetServerPtr srv); + #endif /* __VIR_NET_SERVER_H__ */ -- 2.4.3

Since we introduced listing API earlier in these series, it's time to wire up the API to the virt-admin client. --- tools/virt-admin.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/tools/virt-admin.c b/tools/virt-admin.c index 1372963..d54087a 100644 --- a/tools/virt-admin.c +++ b/tools/virt-admin.c @@ -300,6 +300,57 @@ cmdConnect(vshControl *ctl, const vshCmd *cmd) return !!priv->conn; } + +/* --------------- + * Command Srv-list + * --------------- + */ + +static const vshCmdInfo info_srv_list[] = { + {.name = "help", + .data = N_("list available servers on a daemon") + }, + {.name = "desc", + .data = N_("List all manageable servers on a daemon.") + }, + {.name = NULL} +}; + +static bool +cmdSrvList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) +{ + int nsrvs = 0; + size_t i; + bool ret = false; + const char *uri = NULL; + virAdmServerPtr *srvs = NULL; + vshAdmControlPtr priv = ctl->privData; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(priv->conn, &srvs, 0)) < 0) { + uri = virAdmConnectGetURI(priv->conn); + vshError(ctl, _("failed to obtain list of available servers from %s"), + NULLSTR(uri)); + goto cleanup; + } + + printf(" %-5s %-15s\n", "Id", "Name"); + printf("---------------\n"); + for (i = 0; i < nsrvs; i++) + vshPrint(ctl, " %-5zu %-15s\n", i, virAdmGetServerName(srvs[i])); + + ret = true; + cleanup: + if (srvs) { + for (i = 0; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + VIR_FREE(srvs); + } + + return ret; +} + + static void * vshAdmConnectionHandler(vshControl *ctl) { @@ -592,8 +643,19 @@ static const vshCmdDef vshAdmCmds[] = { {.name = NULL} }; +static const vshCmdDef monitoringCmds[] = { + {.name = "srv-list", + .handler = cmdSrvList, + .opts = NULL, + .info = info_srv_list, + .flags = 0 + }, + {.name = NULL} +}; + static const vshCmdGrp cmdGroups[] = { {"Virt-admin itself", "virt-admin", vshAdmCmds}, + {"Monitoring commands", "monitor", monitoringCmds}, {NULL, NULL, NULL} }; -- 2.4.3
participants (2)
-
Erik Skultety
-
Martin Kletzander