[libvirt] [PATCH 2/6] rpc: Introduce new elements 'id' and 'name' to virnetserver structure

[Martin Kletzander]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150811/a7e69ca8/attachment-0001.sig>