On Tue, Aug 11, 2015 at 09:58:57AM +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 | 17 ++++++++++++++++- src/rpc/virnetserver.h | 1 + tests/virnetdaemondata/input-data-admin-nomdns.json | 2 ++ tests/virnetdaemondata/input-data-anon-clients.json | 1 + tests/virnetdaemondata/input-data-initial-nomdns.json | 1 + tests/virnetdaemondata/input-data-initial.json | 1 + tests/virnetdaemontest.c | 2 +- 11 files changed, 28 insertions(+), 4 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 250094b..88ad753 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, + "default",
You're using "default" here, ...
remoteClientInitHook, NULL, remoteClientFreeFunc, diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index c035024..6e67620 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -151,7 +151,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) if (!(lockd->srv = virNetServerNew(1, 1, 0, config->max_clients, config->max_clients, -1, 0, - NULL, + NULL, "virlockd",
... but "virlockd" for the virtlockd (notice your missing 't'). We should be consistent with that, so either use "default" for all default ones or name them based on the binary.
diff --git a/tests/virnetdaemondata/input-data-initial.json b/tests/virnetdaemondata/input-data-initial.json index 7956225..d1c801d 100644 --- a/tests/virnetdaemondata/input-data-initial.json +++ b/tests/virnetdaemondata/input-data-initial.json @@ -7,6 +7,7 @@ "keepaliveCount": 5, "keepaliveRequired": true, "mdnsGroupName": "libvirtTest", + "name": "libvirtTestServer", "services": [ { "auth": 0,
Input data should not have added any field. Read the README in this directory. The input*.json files are what the real binary will read in real life (not tests), so that's exactly what we need to test for.
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 80b5588..ae48cbb 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -427,11 +436,17 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } + if (!(serverName = virJSONValueObjectGetString(object, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Missing server name in JSON document")); + goto error; + } +
And that's why we must *not* fail here. I would rather you add a name of the server to the function being called here. That name will be used in case there is no server name in the input JSON file. It's also easy to test in virnetdaemontest -- you add different default name in the parameter and leave different one in the file. Those files without the name will have the default one in output and the new one will have the same one in output.
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list