[libvirt] [PATCH v2 0/9] admin API: Introduce server listing API

v2: - tab replacing patch now also includes files Martin added in his keepalive series (trivial, ready to be pushed) - admin_server now marked as renamed instead of deleted and formated with -M - introduction of virAdmServe structure split to a separate patch - resolved server naming issues from review (added fallback data) - refactored lock_daemon a little not to store duplicate reference to a server, since we now have daemon structure - tweaked virnetdaemontest and included new data for it virt-admin still missing in this series, because first we need to handle proper connecting to daemons, i.e. discuss the URI format and then we can add individual commands, so I postponed it in this series and will look at the connect routine first. Erik Skultety (9): test: Replace tabs with spaces in virnetdaemondata json files test: s/{in,out}put-data-admin-nomdns/{in,out}put-data-admin-nomdns-nonames locking: Remove redundant 'srv' element from virLockDaemon rpc: Introduce new elements 'id' and '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 admin: Usage example of the new server listing API (not to be pushed) .gitignore | 1 + Makefile.am | 2 +- configure.ac | 1 + daemon/Makefile.am | 6 +- daemon/admin.c | 174 +++++++++++++++ daemon/admin.h | 36 ++++ daemon/admin_server.c | 103 +++------ daemon/admin_server.h | 23 +- daemon/libvirtd.c | 10 +- examples/admin/Makefile.am | 25 +++ examples/admin/listservers.c | 65 ++++++ include/libvirt/libvirt-admin.h | 12 ++ po/POTFILES.in | 2 +- src/admin/admin_protocol.x | 27 ++- src/admin_protocol-structs | 16 ++ src/datatypes.c | 35 +++ src/datatypes.h | 36 ++++ src/libvirt-admin.c | 171 +++++++++++++++ src/libvirt_admin_private.syms | 5 + src/libvirt_admin_public.syms | 4 + src/libvirt_remote.syms | 3 +- src/locking/lock_daemon.c | 42 ++-- src/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 111 +++++++--- src/rpc/virnetdaemon.h | 30 ++- src/rpc/virnetserver.c | 46 +++- src/rpc/virnetserver.h | 5 + .../input-data-admin-nomdns-names.json | 128 +++++++++++ .../input-data-admin-nomdns-nonames.json | 126 +++++++++++ .../virnetdaemondata/input-data-admin-nomdns.json | 126 ----------- .../input-data-no-keepalive-required.json | 240 ++++++++++----------- .../output-data-admin-nomdns-names.json | 128 +++++++++++ ....json => output-data-admin-nomdns-nonames.json} | 2 + .../virnetdaemondata/output-data-anon-clients.json | 1 + .../output-data-initial-nomdns.json | 1 + tests/virnetdaemondata/output-data-initial.json | 1 + .../output-data-no-keepalive-required.json | 240 ++++++++++----------- tests/virnetdaemontest.c | 52 ++--- 38 files changed, 1492 insertions(+), 546 deletions(-) create mode 100644 daemon/admin.c create mode 100644 daemon/admin.h create mode 100644 examples/admin/Makefile.am create mode 100644 examples/admin/listservers.c create mode 100644 tests/virnetdaemondata/input-data-admin-nomdns-names.json create mode 100644 tests/virnetdaemondata/input-data-admin-nomdns-nonames.json delete mode 100644 tests/virnetdaemondata/input-data-admin-nomdns.json create mode 100644 tests/virnetdaemondata/output-data-admin-nomdns-names.json rename tests/virnetdaemondata/{output-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} (98%) -- 2.4.3

JSON data that are used to initialize tests in virnetdaemontest should be in a consistent format, i.e. not using tabs for indentation, those should be replaced by spaces. --- .../virnetdaemondata/input-data-admin-nomdns.json | 244 ++++++++++----------- .../input-data-no-keepalive-required.json | 240 ++++++++++---------- .../output-data-no-keepalive-required.json | 240 ++++++++++---------- 3 files changed, 362 insertions(+), 362 deletions(-) diff --git a/tests/virnetdaemondata/input-data-admin-nomdns.json b/tests/virnetdaemondata/input-data-admin-nomdns.json index 59bc471..449bcc9 100644 --- a/tests/virnetdaemondata/input-data-admin-nomdns.json +++ b/tests/virnetdaemondata/input-data-admin-nomdns.json @@ -1,126 +1,126 @@ { "servers": [ - { - "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 - } - } - ] - }, - { - "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 - } - } - ] - } + { + "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 + } + } + ] + }, + { + "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/input-data-no-keepalive-required.json b/tests/virnetdaemondata/input-data-no-keepalive-required.json index b5e4dc8..df282ed 100644 --- a/tests/virnetdaemondata/input-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/input-data-no-keepalive-required.json @@ -1,124 +1,124 @@ { "servers": [ - { - "min_workers": 10, - "max_workers": 50, - "priority_workers": 5, - "max_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 - } - } - ] - }, - { - "min_workers": 2, - "max_workers": 50, - "priority_workers": 5, - "max_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 - } - } - ] - } + { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_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 + } + } + ] + }, + { + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_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-no-keepalive-required.json b/tests/virnetdaemondata/output-data-no-keepalive-required.json index b5e4dc8..df282ed 100644 --- a/tests/virnetdaemondata/output-data-no-keepalive-required.json +++ b/tests/virnetdaemondata/output-data-no-keepalive-required.json @@ -1,124 +1,124 @@ { "servers": [ - { - "min_workers": 10, - "max_workers": 50, - "priority_workers": 5, - "max_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 - } - } - ] - }, - { - "min_workers": 2, - "max_workers": 50, - "priority_workers": 5, - "max_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 - } - } - ] - } + { + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_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 + } + } + ] + }, + { + "min_workers": 2, + "max_workers": 50, + "priority_workers": 5, + "max_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 + } + } + ] + } ] } -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:02PM +0200, Erik Skultety wrote:
JSON data that are used to initialize tests in virnetdaemontest should be in a consistent format, i.e. not using tabs for indentation, those should be replaced by spaces. --- .../virnetdaemondata/input-data-admin-nomdns.json | 244 ++++++++++----------- .../input-data-no-keepalive-required.json | 240 ++++++++++---------- .../output-data-no-keepalive-required.json | 240 ++++++++++---------- 3 files changed, 362 insertions(+), 362 deletions(-)
ACK

On 21/08/15 23:44, Martin Kletzander wrote:
On Fri, Aug 21, 2015 at 08:04:02PM +0200, Erik Skultety wrote:
JSON data that are used to initialize tests in virnetdaemontest should be in a consistent format, i.e. not using tabs for indentation, those should be replaced by spaces. --- .../virnetdaemondata/input-data-admin-nomdns.json | 244 ++++++++++----------- .../input-data-no-keepalive-required.json | 240 ++++++++++---------- .../output-data-no-keepalive-required.json | 240 ++++++++++---------- 3 files changed, 362 insertions(+), 362 deletions(-)
ACK Pushed, thanks.
Erik

This minor cosmetical change allows us to later add new test case data including named servers. --- ...nput-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} | 0 ...put-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} | 0 tests/virnetdaemontest.c | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/virnetdaemondata/{input-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} (100%) rename tests/virnetdaemondata/{output-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} (100%) diff --git a/tests/virnetdaemondata/input-data-admin-nomdns.json b/tests/virnetdaemondata/input-data-admin-nomdns-nonames.json similarity index 100% rename from tests/virnetdaemondata/input-data-admin-nomdns.json rename to tests/virnetdaemondata/input-data-admin-nomdns-nonames.json diff --git a/tests/virnetdaemondata/output-data-admin-nomdns.json b/tests/virnetdaemondata/output-data-admin-nomdns-nonames.json similarity index 100% rename from tests/virnetdaemondata/output-data-admin-nomdns.json rename to tests/virnetdaemondata/output-data-admin-nomdns-nonames.json diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index fb8a6c0..bc0a080 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -328,7 +328,7 @@ mymain(void) 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-nonames", 2, true); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:03PM +0200, Erik Skultety wrote:
This minor cosmetical change allows us to later add new test case data including named servers. --- ...nput-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} | 0 ...put-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} | 0 tests/virnetdaemontest.c | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/virnetdaemondata/{input-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} (100%) rename tests/virnetdaemondata/{output-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} (100%)
There is no need to rename this, all the subsequent (new) test files can omit 'nomdns' from now on, for example 'admin-servers' if you add saving of some server data. If you really want this to be named this particular way, please use a normal name for the subject as this is not very easy to find or understand from e.g. oneline log.

On 21/08/15 23:45, Martin Kletzander wrote:
On Fri, Aug 21, 2015 at 08:04:03PM +0200, Erik Skultety wrote:
This minor cosmetical change allows us to later add new test case data including named servers. --- ...nput-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} | 0 ...put-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} | 0 tests/virnetdaemontest.c | 2 +- 3 files changed, 1 insertion(+), 1 deletion(-) rename tests/virnetdaemondata/{input-data-admin-nomdns.json => input-data-admin-nomdns-nonames.json} (100%) rename tests/virnetdaemondata/{output-data-admin-nomdns.json => output-data-admin-nomdns-nonames.json} (100%)
There is no need to rename this, all the subsequent (new) test files can omit 'nomdns' from now on, for example 'admin-servers' if you add saving of some server data. If you really want this to be named this particular way, please use a normal name for the subject as this is not very easy to find or understand from e.g. oneline log. No, I agree with you, it's unnecessary and I dropped the patch from my branch.
Erik

Now that we have virNetDaemon object holding all the data and being capable of referencing multiple servers, having a duplicate reference to a single server stored in virLockDaemon isn't necessary anymore. This patch removes the above described element. --- src/locking/lock_daemon.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c035024..ae3a507 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -62,7 +62,6 @@ VIR_LOG_INIT("locking.lock_daemon"); struct _virLockDaemon { virMutex lock; virNetDaemonPtr dmn; - virNetServerPtr srv; virHashTablePtr lockspaces; virLockSpacePtr defaultLockspace; }; @@ -119,7 +118,6 @@ virLockDaemonFree(virLockDaemonPtr lockd) if (!lockd) return; - virObjectUnref(lockd->srv); virObjectUnref(lockd->dmn); virHashFree(lockd->lockspaces); virLockSpaceFree(lockd->defaultLockspace); @@ -138,6 +136,7 @@ static virLockDaemonPtr virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) { virLockDaemonPtr lockd; + virNetServerPtr srv; if (VIR_ALLOC(lockd) < 0) return NULL; @@ -149,17 +148,17 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) return NULL; } - if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, - config->max_clients, -1, 0, - NULL, - virLockDaemonClientNew, - virLockDaemonClientPreExecRestart, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (!(srv = virNetServerNew(1, 1, 0, config->max_clients, + config->max_clients, -1, 0, + NULL, + virLockDaemonClientNew, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; if (!(lockd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(lockd->dmn, lockd->srv) < 0) + virNetDaemonAddServer(lockd->dmn, srv) < 0) goto error; if (!(lockd->lockspaces = virHashCreate(VIR_LOCK_DAEMON_NUM_LOCKSPACES, @@ -183,6 +182,7 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) virLockDaemonPtr lockd; virJSONValuePtr child; virJSONValuePtr lockspaces; + virNetServerPtr srv; size_t i; int n; @@ -254,12 +254,12 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) goto error; - if (!(lockd->srv = virNetDaemonAddServerPostExec(lockd->dmn, - virLockDaemonClientNew, - virLockDaemonClientNewPostExecRestart, - virLockDaemonClientPreExecRestart, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (!(srv = virNetDaemonAddServerPostExec(lockd->dmn, + virLockDaemonClientNew, + virLockDaemonClientNewPostExecRestart, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; return lockd; @@ -1130,6 +1130,7 @@ virLockDaemonUsage(const char *argv0, bool privileged) } int main(int argc, char **argv) { + virNetServerPtr srv = NULL; virNetServerProgramPtr lockProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; @@ -1353,14 +1354,15 @@ int main(int argc, char **argv) { goto cleanup; } - if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { + srv = virNetDaemonGetServer(lockDaemon->dmn, 0); + if ((rv = virLockDaemonSetupNetworkingSystemD(srv) < 0)) { ret = VIR_LOCK_DAEMON_ERR_NETWORK; goto cleanup; } /* Only do this, if systemd did not pass a FD */ if (rv == 0 && - virLockDaemonSetupNetworkingNative(lockDaemon->srv, sock_file) < 0) { + virLockDaemonSetupNetworkingNative(srv, sock_file) < 0) { ret = VIR_LOCK_DAEMON_ERR_NETWORK; goto cleanup; } @@ -1385,7 +1387,7 @@ int main(int argc, char **argv) { goto cleanup; } - if (virNetServerAddProgram(lockDaemon->srv, lockProgram) < 0) { + if (virNetServerAddProgram(srv, lockProgram) < 0) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; } -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:04PM +0200, Erik Skultety wrote:
Now that we have virNetDaemon object holding all the data and being capable of referencing multiple servers, having a duplicate reference to a single server stored in virLockDaemon isn't necessary anymore. This patch removes the above described element. --- src/locking/lock_daemon.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c035024..ae3a507 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1353,14 +1354,15 @@ int main(int argc, char **argv) { goto cleanup; }
- if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { + srv = virNetDaemonGetServer(lockDaemon->dmn, 0);
This increases the refcount, you need to match it with virObjectUnref(srv); ACK with this squashed in: diff --git i/src/locking/lock_daemon.c w/src/locking/lock_daemon.c index 94f9b11a4829..a50828f56e48 100644 --- i/src/locking/lock_daemon.c +++ w/src/locking/lock_daemon.c @@ -1424,6 +1424,7 @@ int main(int argc, char **argv) { ret = 0; cleanup: + virObjectUnref(srv); virObjectUnref(lockProgram); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { --

On 21/08/15 23:45, Martin Kletzander wrote:
On Fri, Aug 21, 2015 at 08:04:04PM +0200, Erik Skultety wrote:
Now that we have virNetDaemon object holding all the data and being capable of referencing multiple servers, having a duplicate reference to a single server stored in virLockDaemon isn't necessary anymore. This patch removes the above described element. --- src/locking/lock_daemon.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c035024..ae3a507 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -1353,14 +1354,15 @@ int main(int argc, char **argv) { goto cleanup; }
- if ((rv = virLockDaemonSetupNetworkingSystemD(lockDaemon->srv)) < 0) { + srv = virNetDaemonGetServer(lockDaemon->dmn, 0);
This increases the refcount, you need to match it with virObjectUnref(srv);
ACK with this squashed in:
diff --git i/src/locking/lock_daemon.c w/src/locking/lock_daemon.c index 94f9b11a4829..a50828f56e48 100644 --- i/src/locking/lock_daemon.c +++ w/src/locking/lock_daemon.c @@ -1424,6 +1424,7 @@ int main(int argc, char **argv) { ret = 0;
cleanup: + virObjectUnref(srv); virObjectUnref(lockProgram); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { -- Adjusted and pushed, thank you for review.
Erik

By adding these elements, we'll be able to represent the 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/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 1 + src/rpc/virnetserver.c | 18 +++++++++++++++++- src/rpc/virnetserver.h | 1 + tests/virnetdaemontest.c | 2 +- 7 files changed, 24 insertions(+), 4 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 ae3a507..8c07fd6 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -150,7 +150,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/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 48a3597..0984be0 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -926,7 +926,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/virnetdaemon.c b/src/rpc/virnetdaemon.c index 910f266..bdfcfb7 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -69,6 +69,7 @@ struct _virNetDaemon { int sigwrite; int sigwatch; + unsigned int nextSrvId; size_t nservers; virNetServerPtr *servers; virJSONValuePtr srvObject; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 80b5588..ffc1b0d 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -36,6 +36,7 @@ VIR_LOG_INIT("rpc.netserver"); +static unsigned int nextServerId; typedef struct _virNetServerJob virNetServerJob; typedef virNetServerJob *virNetServerJobPtr; @@ -49,6 +50,8 @@ struct _virNetServerJob { struct _virNetServer { virObjectLockable parent; + char *name; + unsigned int id; virThreadPoolPtr workers; char *mdnsGroupName; @@ -312,6 +315,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, int keepaliveInterval, unsigned int keepaliveCount, const char *mdnsGroupName, + const char *serverName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, @@ -332,6 +336,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, srv))) goto error; + srv->id = nextServerId++; srv->nclients_max = max_clients; srv->nclients_unauth_max = max_anonymous_clients; srv->keepaliveInterval = keepaliveInterval; @@ -341,6 +346,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 +386,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 +440,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 +534,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 bc0a080..89d3ca1 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 Fri, Aug 21, 2015 at 08:04:05PM +0200, Erik Skultety wrote:
By adding these elements, we'll be able to represent the 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/lxc/lxc_controller.c | 2 +- src/rpc/virnetdaemon.c | 1 + src/rpc/virnetserver.c | 18 +++++++++++++++++- src/rpc/virnetserver.h | 1 + tests/virnetdaemontest.c | 2 +- 7 files changed, 24 insertions(+), 4 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 ae3a507..8c07fd6 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -150,7 +150,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/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 48a3597..0984be0 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -926,7 +926,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/virnetdaemon.c b/src/rpc/virnetdaemon.c index 910f266..bdfcfb7 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -69,6 +69,7 @@ struct _virNetDaemon { int sigwrite; int sigwatch;
+ unsigned int nextSrvId; size_t nservers; virNetServerPtr *servers; virJSONValuePtr srvObject; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 80b5588..ffc1b0d 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -36,6 +36,7 @@
VIR_LOG_INIT("rpc.netserver");
+static unsigned int nextServerId;
I don't like that the number is being tracked on two places. Since the number is now just sequentially increased (and who knows whether there'll be anything else than 2 servers), we can keep it in one place and we have two options how to do that: 1) Track it in virNetDaemon and pass it to the server being created (that way we can have non-consecutive IDs), 2) Track it in the Server and return the number upon creation (that way we don't need to pass it to the creation function)

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. --- daemon/libvirtd.c | 8 +- src/libvirt_remote.syms | 3 +- src/locking/lock_daemon.c | 16 +-- src/rpc/virnetdaemon.c | 96 +++++++++++----- src/rpc/virnetdaemon.h | 28 ++++- src/rpc/virnetserver.c | 4 + src/rpc/virnetserver.h | 1 + .../input-data-admin-nomdns-names.json | 128 +++++++++++++++++++++ .../output-data-admin-nomdns-names.json | 128 +++++++++++++++++++++ .../output-data-admin-nomdns-nonames.json | 2 + .../virnetdaemondata/output-data-anon-clients.json | 1 + .../output-data-initial-nomdns.json | 1 + tests/virnetdaemondata/output-data-initial.json | 1 + tests/virnetdaemontest.c | 52 +++------ 14 files changed, 390 insertions(+), 79 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-admin-nomdns-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-nomdns-names.json diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index de4953d..5920a96 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1159,6 +1159,8 @@ int main(int argc, char **argv) { bool implicit_conf = false; char *run_dir = NULL; mode_t old_umask; + const char *server_names[] = { "libvirtd", "admin" }; + virNetDaemonFallbackData fbData = { .fb_server_names = server_names }; struct option opts[] = { { "verbose", no_argument, &verbose, 'v'}, @@ -1390,7 +1392,7 @@ int main(int argc, char **argv) { config->keepalive_interval, config->keepalive_count, config->mdns_adv ? config->mdns_name : NULL, - "libvirtd", + server_names[0], remoteClientInitHook, NULL, remoteClientFreeFunc, @@ -1399,12 +1401,14 @@ int main(int argc, char **argv) { goto cleanup; } + virNetDaemonRegisterFallbackData(&fbData); if (!(dmn = virNetDaemonNew()) || virNetDaemonAddServer(dmn, srv) < 0) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; } + /* Beyond this point, nothing should rely on using * getuid/geteuid() == 0, for privilege level checks. */ @@ -1465,7 +1469,7 @@ int main(int argc, char **argv) { config->admin_keepalive_interval, config->admin_keepalive_count, NULL, - "admin", + server_names[1], remoteAdmClientInitHook, NULL, remoteAdmClientFreeFunc, diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 90a453c..4d31b69 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -59,7 +59,7 @@ virNetClientStreamSetError; # rpc/virnetdaemon.h virNetDaemonAddServer; -virNetDaemonAddServerPostExec; +virNetDaemonAddServersPostExec; virNetDaemonAddShutdownInhibition; virNetDaemonAddSignalHandler; virNetDaemonAutoShutdown; @@ -71,6 +71,7 @@ virNetDaemonNew; virNetDaemonNewPostExecRestart; virNetDaemonPreExecRestart; virNetDaemonQuit; +virNetDaemonRegisterFallbackData; virNetDaemonRemoveShutdownInhibition; virNetDaemonRun; virNetDaemonUpdateServices; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 8c07fd6..94f9b11 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -182,7 +182,6 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) virLockDaemonPtr lockd; virJSONValuePtr child; virJSONValuePtr lockspaces; - virNetServerPtr srv; size_t i; int n; @@ -254,12 +253,12 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) goto error; - if (!(srv = virNetDaemonAddServerPostExec(lockd->dmn, - virLockDaemonClientNew, - virLockDaemonClientNewPostExecRestart, - virLockDaemonClientPreExecRestart, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (virNetDaemonAddServersPostExec(lockd->dmn, + virLockDaemonClientNew, + virLockDaemonClientNewPostExecRestart, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)) < 0) goto error; return lockd; @@ -1148,6 +1147,8 @@ int main(int argc, char **argv) { bool privileged = false; virLockDaemonConfigPtr config = NULL; int rv; + const char *server_names[] = { "virtlockd" }; + virNetDaemonFallbackData fbData = { .fb_server_names = server_names }; struct option opts[] = { { "verbose", no_argument, &verbose, 'v'}, @@ -1314,6 +1315,7 @@ int main(int argc, char **argv) { } umask(old_umask); + virNetDaemonRegisterFallbackData(&fbData); if ((rv = virLockDaemonPostExecRestart(state_file, pid_file, &pid_file_fd, diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index bdfcfb7..a3dde4e 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -82,8 +82,8 @@ struct _virNetDaemon { int autoShutdownInhibitFd; }; - static virClassPtr virNetDaemonClass; +static virNetDaemonFallbackDataPtr fbData; static void virNetDaemonDispose(void *obj) @@ -205,38 +205,20 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, return srv; } -virNetServerPtr +static int virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, + virJSONValuePtr object, + const char *fallbackName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, void *clientPrivOpaque) { - virJSONValuePtr object = NULL; virNetServerPtr srv = NULL; - virObjectLock(dmn); - - if (!dmn->srvObject) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot add more servers post-exec than " - "there were pre-exec")); - goto error; - } - - if (virJSONValueIsArray(dmn->srvObject)) { - object = virJSONValueArraySteal(dmn->srvObject, 0); - if (virJSONValueArraySize(dmn->srvObject) == 0) { - virJSONValueFree(dmn->srvObject); - dmn->srvObject = NULL; - } - } else { - object = dmn->srvObject; - dmn->srvObject = NULL; - } - srv = virNetServerNewPostExecRestart(object, + fallbackName, clientPrivNew, clientPrivNewPostExecRestart, clientPrivPreExecRestart, @@ -246,17 +228,67 @@ virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, if (!srv || VIR_APPEND_ELEMENT_COPY(dmn->servers, dmn->nservers, srv) < 0) goto error; - virJSONValueFree(object); - virObjectUnlock(dmn); - return srv; + return 0; error: - virObjectUnlock(dmn); virObjectUnref(srv); - virJSONValueFree(object); - return NULL; + return -1; } +int +virNetDaemonAddServersPostExec(virNetDaemonPtr dmn, + virNetServerClientPrivNew clientPrivNew, + virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, + virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, + virFreeCallback clientPrivFree, + void *clientPrivOpaque) +{ + int nservers; + int ret = -1; + size_t i; + bool new_version; + + virObjectLock(dmn); + + if (!dmn->srvObject) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot add more servers post-exec than " + "there were pre-exec")); + goto cleanup; + } + + if (virJSONValueIsArray(dmn->srvObject)) { + nservers = virJSONValueArraySize(dmn->srvObject); + new_version = true; + } else { + nservers = 1; + new_version = false; + } + + for (i = 0; i < nservers; i++) { + virJSONValuePtr object; + if (new_version) + object = virJSONValueArrayGet(dmn->srvObject, i); + else + object = dmn->srvObject; + if (virNetDaemonAddServerPostExec(dmn, + object, + fbData->fb_server_names[i], + clientPrivNew, + clientPrivNewPostExecRestart, + clientPrivPreExecRestart, + clientPrivFree, + clientPrivOpaque) < 0) + goto cleanup; + } + + ret = 0; + cleanup: + virJSONValueFree(dmn->srvObject); + dmn->srvObject = NULL; + virObjectUnlock(dmn); + return ret; +} virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object) @@ -755,3 +787,9 @@ virNetDaemonHasClients(virNetDaemonPtr dmn) return false; } + +void +virNetDaemonRegisterFallbackData(virNetDaemonFallbackDataPtr data) +{ + fbData = data; +} diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index bb32053..8ad799a 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -35,16 +35,30 @@ # include "virnetserverservice.h" # include "virnetserver.h" +typedef struct _virNetDaemonFallbackData virNetDaemonFallbackData; +typedef virNetDaemonFallbackData *virNetDaemonFallbackDataPtr; + +/* NOTE: Fallback data are necessary for virtlockd which reinitializes its state + * from a JSON after being restarted. New admin API requires a server to have + * a name which previously wasn't required and virtlockd had formatted its state + * without this information, so after an upgrade we need to give the daemon + * a hint about what the server names might be according to their formatted order + * in a JSON file. + */ +struct _virNetDaemonFallbackData { + const char **fb_server_names; +}; + virNetDaemonPtr virNetDaemonNew(void); int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr); -virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, - virNetServerClientPrivNew clientPrivNew, - virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, - virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, - virFreeCallback clientPrivFree, - void *clientPrivOpaque); +int virNetDaemonAddServersPostExec(virNetDaemonPtr dmn, + virNetServerClientPrivNew clientPrivNew, + virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, + virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, + virFreeCallback clientPrivFree, + void *clientPrivOpaque); virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object); @@ -81,4 +95,6 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, int subServerID); +void virNetDaemonRegisterFallbackData(virNetDaemonFallbackDataPtr data); + #endif /* __VIR_NET_DAEMON_H__ */ diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index ffc1b0d..c0a205f 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -367,6 +367,7 @@ virNetServerPtr virNetServerNew(size_t min_workers, virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, + const char *fallbackName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, @@ -388,6 +389,9 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, const char *mdnsGroupName = NULL; const char *serverName = NULL; + if (!(serverName = virJSONValueObjectGetString(object, "name"))) + serverName = fallbackName; + if (virJSONValueObjectGetNumberUint(object, "min_workers", &min_workers) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Missing min_workers data in JSON document")); diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index fb04aa3..96137cc 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 *fallbackName, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, diff --git a/tests/virnetdaemondata/input-data-admin-nomdns-names.json b/tests/virnetdaemondata/input-data-admin-nomdns-names.json new file mode 100644 index 0000000..c9d22e1 --- /dev/null +++ b/tests/virnetdaemondata/input-data-admin-nomdns-names.json @@ -0,0 +1,128 @@ +{ + "servers": [ + { + "name": "adminTestServer0", + "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": "adminTestServer1", + "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-names.json b/tests/virnetdaemondata/output-data-admin-nomdns-names.json new file mode 100644 index 0000000..40c66a7 --- /dev/null +++ b/tests/virnetdaemondata/output-data-admin-nomdns-names.json @@ -0,0 +1,128 @@ +{ + "servers": [ + { + "name": "adminTestServer0", + "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": "adminTestServer1", + "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-admin-nomdns-nonames.json b/tests/virnetdaemondata/output-data-admin-nomdns-nonames.json index a814aeb..4b09311 100644 --- a/tests/virnetdaemondata/output-data-admin-nomdns-nonames.json +++ b/tests/virnetdaemondata/output-data-admin-nomdns-nonames.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-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 89d3ca1..3569253 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, @@ -133,7 +133,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; @@ -155,7 +155,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())) @@ -184,16 +184,13 @@ static char *testGenerateJSON(void) struct testExecRestartData { const char *jsonfile; - int nservers; - bool pass; + virNetDaemonFallbackDataPtr fbData; }; static int testExecRestart(const void *opaque) { - size_t i; int ret = -1; virNetDaemonPtr dmn = NULL; - virNetServerPtr srv = NULL; const struct testExecRestartData *data = opaque; char *infile = NULL, *outfile = NULL; char *injsonstr = NULL, *outjsonstr = NULL; @@ -238,13 +235,9 @@ static int testExecRestart(const void *opaque) if (!(dmn = virNetDaemonNewPostExecRestart(injson))) goto cleanup; - for (i = 0; i < data->nservers; i++) { - if (!(srv = virNetDaemonAddServerPostExec(dmn, - NULL, NULL, NULL, - NULL, NULL))) - goto cleanup; - srv = NULL; - } + virNetDaemonRegisterFallbackData(data->fbData); + if (virNetDaemonAddServersPostExec(dmn, NULL, NULL, NULL, NULL, NULL) < 0) + goto cleanup; if (!(outjson = virNetDaemonPreExecRestart(dmn))) goto cleanup; @@ -255,19 +248,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); @@ -287,6 +272,8 @@ static int mymain(void) { int ret = 0; + const char *server_names[] = { "testServer0", "testServer1" }; + virNetDaemonFallbackData fbData = { .fb_server_names = server_names }; if (virInitialize() < 0 || virEventRegisterDefaultImpl() < 0) { @@ -300,7 +287,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; @@ -309,26 +296,23 @@ mymain(void) return ret; } -# define EXEC_RESTART_TEST_FULL(file, servers, pass) \ +# define EXEC_RESTART_TEST(file) \ do { \ struct testExecRestartData data = { \ - file, servers, pass \ + file, &fbData \ }; \ if (virtTestRun("ExecRestart " file, \ testExecRestart, &data) < 0) \ ret = -1; \ } while (0) -# define EXEC_RESTART_TEST(file) EXEC_RESTART_TEST_FULL(file, 1, true) - # 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-nonames", 2, true); + EXEC_RESTART_TEST("admin-nomdns-nonames"); + EXEC_RESTART_TEST("admin-nomdns-names"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:06PM +0200, 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. --- daemon/libvirtd.c | 8 +- src/libvirt_remote.syms | 3 +- src/locking/lock_daemon.c | 16 +-- src/rpc/virnetdaemon.c | 96 +++++++++++----- src/rpc/virnetdaemon.h | 28 ++++- src/rpc/virnetserver.c | 4 + src/rpc/virnetserver.h | 1 + .../input-data-admin-nomdns-names.json | 128 +++++++++++++++++++++ .../output-data-admin-nomdns-names.json | 128 +++++++++++++++++++++ .../output-data-admin-nomdns-nonames.json | 2 + .../virnetdaemondata/output-data-anon-clients.json | 1 + .../output-data-initial-nomdns.json | 1 + tests/virnetdaemondata/output-data-initial.json | 1 + tests/virnetdaemontest.c | 52 +++------ 14 files changed, 390 insertions(+), 79 deletions(-) create mode 100644 tests/virnetdaemondata/input-data-admin-nomdns-names.json create mode 100644 tests/virnetdaemondata/output-data-admin-nomdns-names.json
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index de4953d..5920a96 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1159,6 +1159,8 @@ int main(int argc, char **argv) { bool implicit_conf = false; char *run_dir = NULL; mode_t old_umask; + const char *server_names[] = { "libvirtd", "admin" }; + virNetDaemonFallbackData fbData = { .fb_server_names = server_names };
This is unnecessarily compliccated approach compared to just passing the number on each ServerPostExec(). And it's also unsafe since you don't have the number of the names kept anywhere neither it ends with NULL pointer. I had one more problem with it but I cannot recall that right now.

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_server.c => admin.c} | 4 ++-- daemon/{admin_server.h => admin.h} | 4 ++-- daemon/libvirtd.c | 4 ++-- po/POTFILES.in | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) rename daemon/{admin_server.c => admin.c} (97%) rename daemon/{admin_server.h => admin.h} (92%) 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_server.c b/daemon/admin.c similarity index 97% rename from daemon/admin_server.c rename to daemon/admin.c index 712a44b..53144ee 100644 --- a/daemon/admin_server.c +++ b/daemon/admin.c @@ -1,5 +1,5 @@ /* - * admin_server.c: + * admin.c: handlers for admin RPC method calls * * Copyright (C) 2014-2015 Red Hat, Inc. * @@ -27,7 +27,7 @@ #include "libvirt_internal.h" #include "admin_protocol.h" -#include "admin_server.h" +#include "admin.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" diff --git a/daemon/admin_server.h b/daemon/admin.h similarity index 92% rename from daemon/admin_server.h rename to daemon/admin.h index 26721a6..1e6802a 100644 --- a/daemon/admin_server.h +++ b/daemon/admin.h @@ -1,7 +1,7 @@ /* - * admin_server.h + * admin.h: handlers for admin RPC method calls * - * Copyright (C) 2014 Red Hat, Inc. + * 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 diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5920a96..4966243 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" @@ -523,7 +523,7 @@ daemonSetupNetworking(virNetServerPtr srv, goto cleanup; /* Temporarily disabled */ - if (sock_path_adm && false) { + if (sock_path_adm) { VIR_DEBUG("Registering unix socket %s", sock_path_adm); if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, unix_sock_adm_mask, diff --git a/po/POTFILES.in b/po/POTFILES.in index 1e52e6a..cef10cb 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,4 +1,4 @@ -daemon/admin_server.c +daemon/admin.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:07PM +0200, 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. --- daemon/Makefile.am | 6 ++++-- daemon/{admin_server.c => admin.c} | 4 ++-- daemon/{admin_server.h => admin.h} | 4 ++-- daemon/libvirtd.c | 4 ++-- po/POTFILES.in | 2 +- 5 files changed, 11 insertions(+), 9 deletions(-) rename daemon/{admin_server.c => admin.c} (97%) rename daemon/{admin_server.h => admin.h} (92%)
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_server.c b/daemon/admin.c similarity index 97% rename from daemon/admin_server.c rename to daemon/admin.c index 712a44b..53144ee 100644 --- a/daemon/admin_server.c +++ b/daemon/admin.c @@ -1,5 +1,5 @@ /* - * admin_server.c: + * admin.c: handlers for admin RPC method calls * * Copyright (C) 2014-2015 Red Hat, Inc. * @@ -27,7 +27,7 @@ #include "libvirt_internal.h"
#include "admin_protocol.h" -#include "admin_server.h" +#include "admin.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" diff --git a/daemon/admin_server.h b/daemon/admin.h similarity index 92% rename from daemon/admin_server.h rename to daemon/admin.h index 26721a6..1e6802a 100644 --- a/daemon/admin_server.h +++ b/daemon/admin.h @@ -1,7 +1,7 @@ /* - * admin_server.h + * admin.h: handlers for admin RPC method calls * - * Copyright (C) 2014 Red Hat, Inc. + * 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 diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 5920a96..4966243 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" @@ -523,7 +523,7 @@ daemonSetupNetworking(virNetServerPtr srv, goto cleanup;
/* Temporarily disabled */ - if (sock_path_adm && false) { + if (sock_path_adm) {
This got here by accident, I guess. We will only enable it after enough support is in for the next release to come out and we will enable it together with distributing header files etc.
VIR_DEBUG("Registering unix socket %s", sock_path_adm); if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm, unix_sock_adm_mask, diff --git a/po/POTFILES.in b/po/POTFILES.in index 1e52e6a..cef10cb 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,4 +1,4 @@ -daemon/admin_server.c +daemon/admin.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h -- 2.4.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

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 | 9 +++++++++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 5 +++++ 5 files changed, 89 insertions(+) diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 9997cc2..f0752f5 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -39,6 +39,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect; +typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -48,6 +50,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 cfc92ff..d062e9a 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,12 +30,21 @@ */ 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; + unsigned hyper id; +}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..9c61ece 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj); static void virStoragePoolDispose(void *obj); virClassPtr virAdmConnectClass; +virClassPtr virAdmServerClass; static void virAdmConnectDispose(void *obj); +static void virAdmServerDispose(void *obj); static int virDataTypesOnceInit(void) @@ -90,6 +92,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool); + DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect); #undef DECLARE_CLASS_COMMON @@ -833,3 +836,35 @@ virAdmConnectDispose(void *obj) if (conn->privateDataFreeFunc) conn->privateDataFreeFunc(conn); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name, const int id) +{ + 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); + ret->id = id; + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %d", srv, srv->id); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index be108fe..1147a7e 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 @@ -402,6 +423,18 @@ struct _virAdmConnect { virFreeCallback privateDataFreeFunc; }; +/** + * _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 */ + unsigned int id; /* the server unique ID */ +}; + /** * _virDomain: @@ -586,4 +619,7 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain, virAdmConnectPtr virAdmConnectNew(void); +virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, + const char *name, + const int id); #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 401cd4e..da559e0 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -6,8 +6,13 @@ # # admin/admin_protocol.x +xdr_admin_connect_list_servers_args; +xdr_admin_connect_list_servers_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 Fri, Aug 21, 2015 at 08:04:08PM +0200, 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 | 9 +++++++++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 5 +++++ 5 files changed, 89 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 9997cc2..f0752f5 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -39,6 +39,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect;
+typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -48,6 +50,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 cfc92ff..d062e9a 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,12 +30,21 @@ */ 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; + unsigned hyper id;
64 bits is a lot, I think, but no biggie, I feel agnostic to this.
+}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..9c61ece 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj); static void virStoragePoolDispose(void *obj);
virClassPtr virAdmConnectClass; +virClassPtr virAdmServerClass;
static void virAdmConnectDispose(void *obj); +static void virAdmServerDispose(void *obj);
static int virDataTypesOnceInit(void) @@ -90,6 +92,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool);
+ DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect);
#undef DECLARE_CLASS_COMMON @@ -833,3 +836,35 @@ virAdmConnectDispose(void *obj) if (conn->privateDataFreeFunc) conn->privateDataFreeFunc(conn); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name, const int id) +{ + 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); + ret->id = id; + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %d", srv, srv->id); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index be108fe..1147a7e 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 @@ -402,6 +423,18 @@ struct _virAdmConnect { virFreeCallback privateDataFreeFunc; };
+/** + * _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 */ + unsigned int id; /* the server unique ID */
If it's 64bits though, this needs to change, so at least one of these declarations must be changed to match the other. My agnosticism swirls slowly into simple int-preference.
+}; +
/** * _virDomain: @@ -586,4 +619,7 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
virAdmConnectPtr virAdmConnectNew(void);
+virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn, + const char *name, + const int id); #endif /* __VIR_DATATYPES_H__ */ diff --git a/src/libvirt_admin_private.syms b/src/libvirt_admin_private.syms index 401cd4e..da559e0 100644 --- a/src/libvirt_admin_private.syms +++ b/src/libvirt_admin_private.syms @@ -6,8 +6,13 @@ #
# admin/admin_protocol.x +xdr_admin_connect_list_servers_args; +xdr_admin_connect_list_servers_ret;
This should be in the following patch.
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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Thu, Sep 03, 2015 at 11:26:06AM +0200, Martin Kletzander wrote:
On Fri, Aug 21, 2015 at 08:04:08PM +0200, 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 | 9 +++++++++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 5 +++++ 5 files changed, 89 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 9997cc2..f0752f5 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -39,6 +39,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect;
+typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -48,6 +50,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 cfc92ff..d062e9a 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,12 +30,21 @@ */ 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; + unsigned hyper id;
64 bits is a lot, I think, but no biggie, I feel agnostic to this.
+}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..9c61ece 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj); static void virStoragePoolDispose(void *obj);
virClassPtr virAdmConnectClass; +virClassPtr virAdmServerClass;
static void virAdmConnectDispose(void *obj); +static void virAdmServerDispose(void *obj);
static int virDataTypesOnceInit(void) @@ -90,6 +92,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool);
+ DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect);
#undef DECLARE_CLASS_COMMON @@ -833,3 +836,35 @@ virAdmConnectDispose(void *obj) if (conn->privateDataFreeFunc) conn->privateDataFreeFunc(conn); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name, const int id) +{ + 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); + ret->id = id; + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %d", srv, srv->id); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index be108fe..1147a7e 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 @@ -402,6 +423,18 @@ struct _virAdmConnect { virFreeCallback privateDataFreeFunc; };
+/** + * _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 */ + unsigned int id; /* the server unique ID */
If it's 64bits though, this needs to change, so at least one of these declarations must be changed to match the other. My agnosticism swirls slowly into simple int-preference.
From a theoretical POV, the number of servers is limited by the size of the array holding the virAdmServer structs. This is in turn size_t limited (well in practice its $RAM limited). So I think we should just use size_t in the struct here, and then use unsigned hyper in the wire which is big enough for any sizet
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On Thu, Sep 03, 2015 at 11:19:02AM +0100, Daniel P. Berrange wrote:
On Thu, Sep 03, 2015 at 11:26:06AM +0200, Martin Kletzander wrote:
On Fri, Aug 21, 2015 at 08:04:08PM +0200, 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 | 9 +++++++++ src/datatypes.c | 35 +++++++++++++++++++++++++++++++++++ src/datatypes.h | 36 ++++++++++++++++++++++++++++++++++++ src/libvirt_admin_private.syms | 5 +++++ 5 files changed, 89 insertions(+)
diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h index 9997cc2..f0752f5 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -39,6 +39,8 @@ extern "C" { */ typedef struct _virAdmConnect virAdmConnect;
+typedef struct _virAdmServer virAdmServer; + /** * virAdmConnectPtr: * @@ -48,6 +50,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 cfc92ff..d062e9a 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -30,12 +30,21 @@ */ 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; + unsigned hyper id;
64 bits is a lot, I think, but no biggie, I feel agnostic to this.
+}; + /*----- Protocol. -----*/ struct admin_connect_open_args { unsigned int flags; diff --git a/src/datatypes.c b/src/datatypes.c index 12bcfc1..9c61ece 100644 --- a/src/datatypes.c +++ b/src/datatypes.c @@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj); static void virStoragePoolDispose(void *obj);
virClassPtr virAdmConnectClass; +virClassPtr virAdmServerClass;
static void virAdmConnectDispose(void *obj); +static void virAdmServerDispose(void *obj);
static int virDataTypesOnceInit(void) @@ -90,6 +92,7 @@ virDataTypesOnceInit(void) DECLARE_CLASS(virStorageVol); DECLARE_CLASS(virStoragePool);
+ DECLARE_CLASS(virAdmServer); DECLARE_CLASS_LOCKABLE(virAdmConnect);
#undef DECLARE_CLASS_COMMON @@ -833,3 +836,35 @@ virAdmConnectDispose(void *obj) if (conn->privateDataFreeFunc) conn->privateDataFreeFunc(conn); } + +virAdmServerPtr +virAdmGetServer(virAdmConnectPtr conn, const char *name, const int id) +{ + 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); + ret->id = id; + + return ret; + error: + virObjectUnref(ret); + return NULL; +} + +static void +virAdmServerDispose(void *obj) +{ + virAdmServerPtr srv = obj; + VIR_DEBUG("release server %p %d", srv, srv->id); + + VIR_FREE(srv->name); + virObjectUnref(srv->conn); +} diff --git a/src/datatypes.h b/src/datatypes.h index be108fe..1147a7e 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 @@ -402,6 +423,18 @@ struct _virAdmConnect { virFreeCallback privateDataFreeFunc; };
+/** + * _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 */ + unsigned int id; /* the server unique ID */
If it's 64bits though, this needs to change, so at least one of these declarations must be changed to match the other. My agnosticism swirls slowly into simple int-preference.
From a theoretical POV, the number of servers is limited by the size of the array holding the virAdmServer structs. This is in turn size_t limited (well in practice its $RAM limited). So I think we should just use size_t in the struct here, and then use unsigned hyper in the wire which is big enough for any sizet
On the other hand, we use integer in the list of domains and I am pretty sure we won't have more than three servers per daemon for a *long* time. Maybe even two. And we didn't have the problem with number of domains and there could be *way* more of those. We also talked about identifying the servers by their unique name, but the id is added there to keep it consistent with other following structures, won't hurt and because we can directly access particular server from the array (even if it is a sparse one). So I think int is *way* more than needed.
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

This is the first API to the admin interface. This particular API is 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 | 57 ++++++++++++++ daemon/admin_server.c | 82 +++++++++++++++++++ daemon/admin_server.h | 33 ++++++++ include/libvirt/libvirt-admin.h | 8 ++ src/admin/admin_protocol.x | 18 ++++- src/admin_protocol-structs | 16 ++++ src/libvirt-admin.c | 171 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_public.syms | 4 + src/rpc/virnetdaemon.c | 14 ++++ src/rpc/virnetdaemon.h | 2 + src/rpc/virnetserver.c | 24 ++++++ src/rpc/virnetserver.h | 3 + 13 files changed, 432 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 53144ee..6c5b1a9 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,16 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; } +/* Helpers */ + +static void +make_nonnull_server(admin_nonnull_server *srv_dst, + virAdmServerPtr srv_src) +{ + srv_dst->id = srv_src->id; + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -114,4 +125,50 @@ adminDispatchConnectClose(virNetServerPtr server 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 = + adminDaemonConnectListServers(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..9d55074 --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,82 @@ +/* + * 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 +adminDaemonConnectListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + unsigned int id; + 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); + id = virNetServerGetID(srv); + + virObjectLock(srv); + if (!(srvs[i] = virAdmGetServer(NULL, name, id))) { + 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..aa8f060 --- /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 +adminDaemonConnectListServers(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 f0752f5..83e73ca 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -55,8 +55,16 @@ 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); +unsigned int virAdmGetServerID(virAdmServerPtr srv); +const char *virAdmGetServerName(virAdmServerPtr srv); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index d062e9a..05b31ea 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -50,6 +50,16 @@ struct admin_connect_open_args { unsigned int flags; }; +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; @@ -80,5 +90,11 @@ enum admin_procedure { /** * @generate: client */ - ADMIN_PROC_CONNECT_CLOSE = 2 + ADMIN_PROC_CONNECT_CLOSE = 2, + + /** + * @generate: none + * @priority: high + */ + ADMIN_PROC_CONNECT_LIST_SERVERS = 3 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 3ac31fa..6fcd082 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,8 +1,24 @@ /* -*- c -*- */ +struct admin_nonnull_server { + admin_nonnull_string name; + uint64_t id; +}; struct admin_connect_open_args { u_int flags; }; +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_LIST_SERVERS = 3, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index b3fd0b3..7067c92 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -123,6 +123,16 @@ call(virAdmConnectPtr conn, #include "admin_protocol.h" #include "admin_client.h" +/* Helpers */ +static virAdmServerPtr +get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server) +{ + virAdmServerPtr srv; + + srv = virAdmGetServer(conn, server.name, server.id); + return srv; +} + static bool virAdmGlobalError; static virOnceControl virAdmGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER; @@ -385,3 +395,164 @@ virAdmConnectRef(virAdmConnectPtr conn) return 0; } + +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; +} + + +/** + * virAdmGetServerID: + * @srv: a server object + * + * Get the server ID number. + * + * Returns the server ID number on success, (unsigned int) -1 on failure. + */ +unsigned int +virAdmGetServerID(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + + virResetLastError(); + virCheckAdmServerReturn(srv, (unsigned int)-1); + + return srv->id; +} + +/** + * 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_public.syms b/src/libvirt_admin_public.syms index d9e3c0b..dbb5f76 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -15,4 +15,8 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectOpen; virAdmConnectClose; virAdmConnectRef; + virAdmConnectListServers; + virAdmGetServerID; + virAdmGetServerName; + virAdmServerFree; }; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index a3dde4e..b2f902f 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -205,6 +205,20 @@ 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; +} + static int virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, virJSONValuePtr object, diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 8ad799a..9450575 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -94,6 +94,8 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, int subServerID); +size_t virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers); void virNetDaemonRegisterFallbackData(virNetDaemonFallbackDataPtr data); diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index c0a205f..1ecad79 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -835,6 +835,30 @@ virNetServerHasClients(virNetServerPtr srv) return ret; } +const char * +virNetServerGetName(virNetServerPtr srv) +{ + const char *name; + + virObjectLock(srv); + name = srv->name; + virObjectUnlock(srv); + + return name; +} + +unsigned int +virNetServerGetID(virNetServerPtr srv) +{ + unsigned int id; + + virObjectLock(srv); + id = srv->id; + virObjectUnlock(srv); + + return id; +} + void virNetServerProcessClients(virNetServerPtr srv) { diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 96137cc..d572650 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

On Fri, Aug 21, 2015 at 08:04:09PM +0200, Erik Skultety wrote:
This is the first API to the admin interface. This particular API is 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 | 57 ++++++++++++++ daemon/admin_server.c | 82 +++++++++++++++++++ daemon/admin_server.h | 33 ++++++++ include/libvirt/libvirt-admin.h | 8 ++ src/admin/admin_protocol.x | 18 ++++- src/admin_protocol-structs | 16 ++++ src/libvirt-admin.c | 171 ++++++++++++++++++++++++++++++++++++++++ src/libvirt_admin_public.syms | 4 + src/rpc/virnetdaemon.c | 14 ++++ src/rpc/virnetdaemon.h | 2 + src/rpc/virnetserver.c | 24 ++++++ src/rpc/virnetserver.h | 3 + 13 files changed, 432 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 53144ee..6c5b1a9 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,16 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; }
+/* Helpers */ + +static void +make_nonnull_server(admin_nonnull_server *srv_dst, + virAdmServerPtr srv_src) +{ + srv_dst->id = srv_src->id; + ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name)); +} + /* Functions */ static int adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -114,4 +125,50 @@ adminDispatchConnectClose(virNetServerPtr server 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 = + adminDaemonConnectListServers(priv->dmn,
The 'DaemonConnect' is unnecessary here. I would like the defaults for admin api to be similar to the default API, but with few changes. If something is called on virConnectPtr, it starts with virConnectPtr and its first parameter is virConnectPtr on both client and server. If something is called on virAdmConnectPtr, it would start with virAdm and its first parameter would be virAdmConnectPtr on client, but virNetDaemonPtr on server. I don't see the need for having the virAdmConnectPtr on the server as a part of the API right now. And it's internal, so we can change that later on. Consequantly whe we add an admin API that does something with a server, it will look like this. Let's say the function will be listing clients. On client, it will be virAdmServerListClients(), it's first parameter will be virAdmServerPtr, and on daemon it will be adminServerListClients() and its first parameter will be virNetServerPtr. That said its visible that we will need to do some translation so that the server part can deal with virNetServerPtr and directly even though we only send the id (and name) over RPC. And that's what the helper functions are for, the serialization and de-serialization or RPC stuff. It's not applicable to the listing of servers, but I wanted to mention it for later on, just so we're on the same page and can prepare for the fact that gendispatch.pl will need some work done on it :)
+ 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..9d55074 --- /dev/null +++ b/daemon/admin_server.c @@ -0,0 +1,82 @@ +/* + * 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 +adminDaemonConnectListServers(virNetDaemonPtr dmn, + virAdmServerPtr **servers, + unsigned int flags) +{ + int ret = -1; + unsigned int id; + 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); + id = virNetServerGetID(srv); + + virObjectLock(srv); + if (!(srvs[i] = virAdmGetServer(NULL, name, id))) { + 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..aa8f060 --- /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 +adminDaemonConnectListServers(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 f0752f5..83e73ca 100644 --- a/include/libvirt/libvirt-admin.h +++ b/include/libvirt/libvirt-admin.h @@ -55,8 +55,16 @@ 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);
+unsigned int virAdmGetServerID(virAdmServerPtr srv); +const char *virAdmGetServerName(virAdmServerPtr srv); + # ifdef __cplusplus } # endif diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x index d062e9a..05b31ea 100644 --- a/src/admin/admin_protocol.x +++ b/src/admin/admin_protocol.x @@ -50,6 +50,16 @@ struct admin_connect_open_args { unsigned int flags; };
+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; @@ -80,5 +90,11 @@ enum admin_procedure { /** * @generate: client */ - ADMIN_PROC_CONNECT_CLOSE = 2 + ADMIN_PROC_CONNECT_CLOSE = 2, + + /** + * @generate: none + * @priority: high + */ + ADMIN_PROC_CONNECT_LIST_SERVERS = 3 }; diff --git a/src/admin_protocol-structs b/src/admin_protocol-structs index 3ac31fa..6fcd082 100644 --- a/src/admin_protocol-structs +++ b/src/admin_protocol-structs @@ -1,8 +1,24 @@ /* -*- c -*- */ +struct admin_nonnull_server { + admin_nonnull_string name; + uint64_t id; +}; struct admin_connect_open_args { u_int flags; }; +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_LIST_SERVERS = 3, }; diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index b3fd0b3..7067c92 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -123,6 +123,16 @@ call(virAdmConnectPtr conn, #include "admin_protocol.h" #include "admin_client.h"
+/* Helpers */ +static virAdmServerPtr +get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server) +{ + virAdmServerPtr srv; + + srv = virAdmGetServer(conn, server.name, server.id); + return srv; +} + static bool virAdmGlobalError; static virOnceControl virAdmGlobalOnce = VIR_ONCE_CONTROL_INITIALIZER;
@@ -385,3 +395,164 @@ virAdmConnectRef(virAdmConnectPtr conn)
return 0; } + +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; +} + + +/** + * virAdmGetServerID: + * @srv: a server object + * + * Get the server ID number. + * + * Returns the server ID number on success, (unsigned int) -1 on failure. + */ +unsigned int +virAdmGetServerID(virAdmServerPtr srv) +{ + VIR_DEBUG("server=%p", srv); + + virResetLastError(); + virCheckAdmServerReturn(srv, (unsigned int)-1); + + return srv->id; +} + +/** + * 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_public.syms b/src/libvirt_admin_public.syms index d9e3c0b..dbb5f76 100644 --- a/src/libvirt_admin_public.syms +++ b/src/libvirt_admin_public.syms @@ -15,4 +15,8 @@ LIBVIRT_ADMIN_1.3.0 { virAdmConnectOpen; virAdmConnectClose; virAdmConnectRef; + virAdmConnectListServers; + virAdmGetServerID; + virAdmGetServerName; + virAdmServerFree; }; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index a3dde4e..b2f902f 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -205,6 +205,20 @@ 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; +} + static int virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, virJSONValuePtr object, diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 8ad799a..9450575 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -94,6 +94,8 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn);
virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, int subServerID); +size_t virNetDaemonGetServers(virNetDaemonPtr dmn, + virNetServerPtr **servers);
void virNetDaemonRegisterFallbackData(virNetDaemonFallbackDataPtr data);
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index c0a205f..1ecad79 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -835,6 +835,30 @@ virNetServerHasClients(virNetServerPtr srv) return ret; }
+const char * +virNetServerGetName(virNetServerPtr srv) +{ + const char *name; + + virObjectLock(srv); + name = srv->name; + virObjectUnlock(srv); + + return name; +} + +unsigned int +virNetServerGetID(virNetServerPtr srv) +{ + unsigned int id; + + virObjectLock(srv); + id = srv->id; + virObjectUnlock(srv); + + return id; +} + void virNetServerProcessClients(virNetServerPtr srv) { diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 96137cc..d572650 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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

Not to be actually pushed since majority of this example will be merged into virt-admin once it's ready, i.e. virsh splitting series is merged, but might be good to just see the API's working. --- .gitignore | 1 + Makefile.am | 2 +- configure.ac | 1 + examples/admin/Makefile.am | 25 +++++++++++++++++ examples/admin/listservers.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 examples/admin/Makefile.am create mode 100644 examples/admin/listservers.c diff --git a/.gitignore b/.gitignore index 6bd41be..d77c13e 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,7 @@ /docs/libvirt-refs.xml /docs/search.php /docs/todo.html.in +/examples/admin/listservers /examples/object-events/event-test /examples/dominfo/info1 /examples/domsuspend/suspend diff --git a/Makefile.am b/Makefile.am index 91b943b..c14229e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ examples/dominfo examples/domsuspend examples/apparmor \ examples/xml/nwfilter examples/openauth examples/systemtap \ tools/wireshark examples/dommigrate \ - examples/lxcconvert examples/domtop + examples/lxcconvert examples/domtop examples/admin ACLOCAL_AMFLAGS = -I m4 diff --git a/configure.ac b/configure.ac index 08a0f93..1c2f6e8 100644 --- a/configure.ac +++ b/configure.ac @@ -2809,6 +2809,7 @@ AC_CONFIG_FILES([\ examples/systemtap/Makefile \ examples/xml/nwfilter/Makefile \ examples/lxcconvert/Makefile \ + examples/admin/Makefile \ tools/wireshark/Makefile \ tools/wireshark/src/Makefile]) AC_OUTPUT diff --git a/examples/admin/Makefile.am b/examples/admin/Makefile.am new file mode 100644 index 0000000..8373132 --- /dev/null +++ b/examples/admin/Makefile.am @@ -0,0 +1,25 @@ +## Copyright (C) 2005-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/>. + +INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include +LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) \ + $(top_builddir)/src/libvirt-admin.la $(COVERAGE_LDFLAGS) + +noinst_PROGRAMS=listservers + +listservers_SOURCES=listservers.c +listservers_LDFLAGS= +listservers_LDADD= $(LDADDS) diff --git a/examples/admin/listservers.c b/examples/admin/listservers.c new file mode 100644 index 0000000..34c1f3a --- /dev/null +++ b/examples/admin/listservers.c @@ -0,0 +1,65 @@ +/* + * listservers.c: Demo program to show listing of available servers + * + * 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: Erik Skultety <eskultet@redhat.com> + */ + +#include <stdio.h> +#include <stdlib.h> +#include <libvirt/libvirt-admin.h> + +int main(void) +{ + int ret = -1; + virAdmConnectPtr conn = NULL; + virAdmServerPtr *srvs = NULL; + int nsrvs = 0; + size_t i; + + /* Connect to an admin server on a specific daemon, NULL in this case means + * connect to libvirtd UNIX socket + */ + if (!(conn = virAdmConnectOpen(NULL, 0))) + goto cleanup; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(conn, &srvs, 0)) < 0) { + fprintf(stderr, "Failed to obtain list of available servers\n"); + goto cleanup; + } + + printf(" %-5s %-15s\n", "Id", "Name"); + printf("---------------\n"); + for (i = 0; i < nsrvs; i++) + printf(" %-5d %-15s\n", virAdmGetServerID(srvs[i]), + virAdmGetServerName(srvs[i])); + + ret = nsrvs; + + cleanup: + if (conn) + virAdmConnectClose(conn); + if (srvs) { + for (i = 0; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + free(srvs); + } + + return ret; +} -- 2.4.3

On Fri, Aug 21, 2015 at 08:04:10PM +0200, Erik Skultety wrote:
Not to be actually pushed since majority of this example will be merged into virt-admin once it's ready, i.e. virsh splitting series is merged, but might be good to just see the API's working. --- .gitignore | 1 + Makefile.am | 2 +- configure.ac | 1 + examples/admin/Makefile.am | 25 +++++++++++++++++ examples/admin/listservers.c | 65 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 1 deletion(-) create mode 100644 examples/admin/Makefile.am create mode 100644 examples/admin/listservers.c
diff --git a/.gitignore b/.gitignore index 6bd41be..d77c13e 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,7 @@ /docs/libvirt-refs.xml /docs/search.php /docs/todo.html.in +/examples/admin/listservers /examples/object-events/event-test /examples/dominfo/info1 /examples/domsuspend/suspend diff --git a/Makefile.am b/Makefile.am index 91b943b..c14229e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs gnulib/tests \ examples/dominfo examples/domsuspend examples/apparmor \ examples/xml/nwfilter examples/openauth examples/systemtap \ tools/wireshark examples/dommigrate \ - examples/lxcconvert examples/domtop + examples/lxcconvert examples/domtop examples/admin
ACLOCAL_AMFLAGS = -I m4
diff --git a/configure.ac b/configure.ac index 08a0f93..1c2f6e8 100644 --- a/configure.ac +++ b/configure.ac @@ -2809,6 +2809,7 @@ AC_CONFIG_FILES([\ examples/systemtap/Makefile \ examples/xml/nwfilter/Makefile \ examples/lxcconvert/Makefile \ + examples/admin/Makefile \ tools/wireshark/Makefile \ tools/wireshark/src/Makefile]) AC_OUTPUT diff --git a/examples/admin/Makefile.am b/examples/admin/Makefile.am new file mode 100644 index 0000000..8373132 --- /dev/null +++ b/examples/admin/Makefile.am @@ -0,0 +1,25 @@ +## Copyright (C) 2005-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/>. + +INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include +LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) \ + $(top_builddir)/src/libvirt-admin.la $(COVERAGE_LDFLAGS) + +noinst_PROGRAMS=listservers + +listservers_SOURCES=listservers.c +listservers_LDFLAGS= +listservers_LDADD= $(LDADDS) diff --git a/examples/admin/listservers.c b/examples/admin/listservers.c new file mode 100644 index 0000000..34c1f3a --- /dev/null +++ b/examples/admin/listservers.c @@ -0,0 +1,65 @@ +/* + * listservers.c: Demo program to show listing of available servers + * + * 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: Erik Skultety <eskultet@redhat.com> + */ + +#include <stdio.h> +#include <stdlib.h> +#include <libvirt/libvirt-admin.h> + +int main(void) +{ + int ret = -1; + virAdmConnectPtr conn = NULL; + virAdmServerPtr *srvs = NULL; + int nsrvs = 0; + size_t i; + + /* Connect to an admin server on a specific daemon, NULL in this case means + * connect to libvirtd UNIX socket + */ + if (!(conn = virAdmConnectOpen(NULL, 0))) + goto cleanup; + + /* Obtain a list of available servers on the daemon */ + if ((nsrvs = virAdmConnectListServers(conn, &srvs, 0)) < 0) { + fprintf(stderr, "Failed to obtain list of available servers\n"); + goto cleanup; + } + + printf(" %-5s %-15s\n", "Id", "Name"); + printf("---------------\n"); + for (i = 0; i < nsrvs; i++) + printf(" %-5d %-15s\n", virAdmGetServerID(srvs[i]), + virAdmGetServerName(srvs[i])); + + ret = nsrvs; + + cleanup: + if (conn) + virAdmConnectClose(conn); + if (srvs) { + for (i = 0; i < nsrvs; i++) + virAdmServerFree(srvs[i]); + free(srvs); + } + + return ret; +} -- 2.4.3
Looks ok, but with the following squashed in, it's more fun to test ;) diff --git a/examples/admin/listservers.c b/examples/admin/listservers.c index 34c1f3adc8d1..3e96d85ace05 100644 --- a/examples/admin/listservers.c +++ b/examples/admin/listservers.c @@ -24,18 +24,19 @@ #include <stdlib.h> #include <libvirt/libvirt-admin.h> -int main(void) +int main(int argc, char **argv) { int ret = -1; virAdmConnectPtr conn = NULL; virAdmServerPtr *srvs = NULL; int nsrvs = 0; + const char *name = NULL; size_t i; - /* Connect to an admin server on a specific daemon, NULL in this case means - * connect to libvirtd UNIX socket - */ - if (!(conn = virAdmConnectOpen(NULL, 0))) + if (argc > 1) + name = argv[1]; + + if (!(conn = virAdmConnectOpen(name, 0))) goto cleanup; /* Obtain a list of available servers on the daemon */ -- Martin
participants (3)
-
Daniel P. Berrange
-
Erik Skultety
-
Martin Kletzander