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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]