[libvirt] [PATCH 3/3] rpc: add testing of RPC JSON (de)serialization

Martin Kletzander mkletzan at redhat.com
Mon Jun 8 13:28:55 UTC 2015


On Fri, Jun 05, 2015 at 09:47:45AM +0100, Daniel P. Berrange wrote:
>The virNetServer class has the ability to serialize its state
>to a JSON file, and then re-load that data after an in-place
>execve() call to re-connect to active file handles. This data
>format is critical ABI that must have compatibility across
>releases, so it should be tested...
>---
> src/libvirt_remote.syms                            |   1 +
> src/rpc/virnetserver.c                             |   4 +-
> src/rpc/virnetserver.h                             |   3 +
> src/rpc/virnetserverclient.c                       |  13 +-
> src/rpc/virnetserverservice.c                      |   6 +-
> tests/Makefile.am                                  |   7 +
> tests/virnetserverdata/README                      |  14 +
> .../virnetserverdata/input-data-anon-clients.json  |  63 +++++
> tests/virnetserverdata/input-data-initial.json     |  62 +++++
> .../virnetserverdata/output-data-anon-clients.json |  63 +++++
> tests/virnetserverdata/output-data-initial.json    |  63 +++++
> tests/virnetservertest.c                           | 290 +++++++++++++++++++++
> 12 files changed, 579 insertions(+), 10 deletions(-)
> create mode 100644 tests/virnetserverdata/README
> create mode 100644 tests/virnetserverdata/input-data-anon-clients.json
> create mode 100644 tests/virnetserverdata/input-data-initial.json
> create mode 100644 tests/virnetserverdata/output-data-anon-clients.json
> create mode 100644 tests/virnetserverdata/output-data-initial.json
> create mode 100644 tests/virnetservertest.c
>
[...]
>+testCreateServer(const char *host, int family)
>+{
>+    virNetServerPtr srv = NULL;
>+    virNetServerServicePtr svc1 = NULL, svc2 = NULL;
>+    virNetServerClientPtr cln1 = NULL, cln2 = NULL;
>+    virNetSocketPtr sk1 = NULL, sk2 = NULL;
>+    int fdclient[2];
>+
>+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdclient) < 0) {
>+        virReportSystemError(errno, "%s",
>+                             "Cannot create socket pair");
>+        goto cleanup;
>+    }
>+
>+    if (!(srv = virNetServerNew(10, 50, 5, 100, 10,
>+                                120, 5, true,
>+                                "libvirtTest",

It would be nice to have this generate JSON even if compiling without
AVAHI.  This will now fail.

>+                                NULL,
>+                                NULL,
>+                                NULL,
>+                                NULL)))
>+        goto error;
>+
>+    if (!(svc1 = virNetServerServiceNewTCP(host,
>+                                           NULL,
>+                                           family,
>+                                           VIR_NET_SERVER_SERVICE_AUTH_NONE,
>+# ifdef WITH_GNUTLS
>+                                           NULL,
>+# endif
>+                                           true,
>+                                           5,
>+                                           2)))
>+        goto error;
>+
>+    if (!(svc2 = virNetServerServiceNewTCP(host,
>+                                           NULL,
>+                                           VIR_NET_SERVER_SERVICE_AUTH_POLKIT,
>+                                           family,

Compiler won't find it, but you switched the lines here ^^.

>+static int testExecRestart(const void *opaque)
>+{
>+    int ret = -1;
>+    virNetServerPtr srv = NULL;
>+    const struct testExecRestartData *data = opaque;
>+    char *infile = NULL, *outfile = NULL;
>+    char *injsonstr = NULL, *outjsonstr = NULL;
>+    virJSONValuePtr injson = NULL, outjson = NULL;
>+    int fdclient[2] = { -1, -1 }, fdserver[2] = { -1, -1 };
>+
>+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdclient) < 0) {
>+        virReportSystemError(errno, "%s",
>+                             "Cannot create socket pair");
>+        goto cleanup;
>+    }
>+
>+    if (socketpair(PF_UNIX, SOCK_STREAM, 0, fdserver) < 0) {
>+        virReportSystemError(errno, "%s",
>+                             "Cannot create socket pair");
>+        goto cleanup;
>+    }
>+
>+    /* We're blindly assuming the test case isn't using
>+     * fds 100->103 for something else, which is probably
>+     * fairly reasonable in general
>+     */
>+    dup2(fdserver[0], 100);
>+    dup2(fdserver[1], 101);
>+    dup2(fdclient[0], 102);
>+    dup2(fdclient[1], 103);
>+
>+    if (virAsprintf(&infile, "%s/virnetserverdata/input-data-%s.json",
>+                    abs_srcdir, data->jsonfile) < 0)
>+        goto cleanup;
>+
>+    if (virAsprintf(&outfile, "%s/virnetserverdata/output-data-%s.json",
>+                    abs_srcdir, data->jsonfile) < 0)
>+        goto cleanup;
>+
>+    if (virFileReadAll(infile, 8192, &injsonstr) < 0)
>+        goto cleanup;
>+
>+    if (!(injson = virJSONValueFromString(injsonstr)))
>+        goto cleanup;
>+
>+    if (!(srv = virNetServerNewPostExecRestart(injson,
>+                                               NULL, NULL, NULL,
>+                                               NULL, NULL)))
>+        goto cleanup;
>+
>+    if (!(outjson = virNetServerPreExecRestart(srv)))
>+        goto cleanup;
>+
>+    if (!(outjsonstr = virJSONValueToString(outjson, true)))
>+        goto cleanup;
>+
>+    if (virtTestCompareToFile(outjsonstr, outfile) < 0)
>+        goto fail;
>+
>+    ret = 0;
>+
>+ cleanup:
>+    if (ret < 0) {
>+        virErrorPtr err = virGetLastError();
>+        fprintf(stderr, "%s\n", err->message);

The err can be NULL if we don't set an error message.  No need to go
for the virErrorGenericFailure(), but check for the err so we don't
segfault here.

>+    }
>+ fail:
>+    VIR_FREE(infile);
>+    VIR_FREE(outfile);
>+    VIR_FREE(injsonstr);
>+    VIR_FREE(outjsonstr);
>+    virJSONValueFree(injson);
>+    virJSONValueFree(outjson);
>+    virObjectUnref(srv);
>+    VIR_FORCE_CLOSE(fdserver[0]);
>+    VIR_FORCE_CLOSE(fdserver[1]);
>+    VIR_FORCE_CLOSE(fdclient[0]);
>+    VIR_FORCE_CLOSE(fdclient[1]);
>+    return ret;
>+}
>+
>+
>+static int
>+mymain(void)
>+{
>+    int ret = 0;
>+
>+    /* Hack to make it easier to generate new JSON files when
>+     * the RPC classes change. Just set this env var, save
>+     * the generated JSON, and replace the file descriptor
>+     * numbers with 100, 101, 102, 103.
>+     */
>+    if (getenv("VIR_GENERATE_JSON")) {
>+        char *json = testGenerateJSON();
>+        fprintf(stdout, "%s\n", json);
>+        VIR_FREE(json);
>+        return EXIT_SUCCESS;
>+    }
>+
>+# define EXEC_RESTART_TEST(file)                        \
>+    do {                                                \
>+        struct testExecRestartData data = {             \
>+            file                                        \
>+        };                                              \
>+        if (virtTestRun("ExecRestart " file,            \
>+                        testExecRestart, &data) < 0)    \
>+            ret = -1;                                   \
>+    } while (0)
>+
>+    EXEC_RESTART_TEST("initial");
>+    EXEC_RESTART_TEST("anon-clients");
>+

Both tests here will fail without AVAHI.  I suggest having some
avahi-ohly tests and some without the avahi requirement.

ACK with this squashed in:

diff --git c/tests/virnetserverdata/input-data-anon-clients.json w/tests/virnetserverdata/input-data-anon-clients.json
index ed91b57e179b..8a51ff53d6cf 100644
--- c/tests/virnetserverdata/input-data-anon-clients.json
+++ w/tests/virnetserverdata/input-data-anon-clients.json
@@ -7,7 +7,6 @@
     "keepaliveInterval": 120,
     "keepaliveCount": 5,
     "keepaliveRequired": true,
-    "mdnsGroupName": "libvirtTest",
     "services": [
         {
             "auth": 0,
diff --git c/tests/virnetserverdata/input-data-anon-clients.json w/tests/virnetserverdata/input-data-initial-nomdns.json
similarity index 95%
copy from tests/virnetserverdata/input-data-anon-clients.json
copy to tests/virnetserverdata/input-data-initial-nomdns.json
index ed91b57e179b..02bb42748774 100644
--- c/tests/virnetserverdata/input-data-anon-clients.json
+++ w/tests/virnetserverdata/input-data-initial-nomdns.json
@@ -3,11 +3,9 @@
     "max_workers": 50,
     "priority_workers": 5,
     "max_clients": 100,
-    "max_anonymous_clients": 10,
     "keepaliveInterval": 120,
     "keepaliveCount": 5,
     "keepaliveRequired": true,
-    "mdnsGroupName": "libvirtTest",
     "services": [
         {
             "auth": 0,
diff --git c/tests/virnetserverdata/output-data-anon-clients.json w/tests/virnetserverdata/output-data-anon-clients.json
index ed91b57e179b..8a51ff53d6cf 100644
--- c/tests/virnetserverdata/output-data-anon-clients.json
+++ w/tests/virnetserverdata/output-data-anon-clients.json
@@ -7,7 +7,6 @@
     "keepaliveInterval": 120,
     "keepaliveCount": 5,
     "keepaliveRequired": true,
-    "mdnsGroupName": "libvirtTest",
     "services": [
         {
             "auth": 0,
diff --git c/tests/virnetserverdata/input-data-anon-clients.json w/tests/virnetserverdata/output-data-initial-nomdns.json
similarity index 95%
copy from tests/virnetserverdata/input-data-anon-clients.json
copy to tests/virnetserverdata/output-data-initial-nomdns.json
index ed91b57e179b..8d38297af63b 100644
--- c/tests/virnetserverdata/input-data-anon-clients.json
+++ w/tests/virnetserverdata/output-data-initial-nomdns.json
@@ -3,11 +3,10 @@
     "max_workers": 50,
     "priority_workers": 5,
     "max_clients": 100,
-    "max_anonymous_clients": 10,
+    "max_anonymous_clients": 100,
     "keepaliveInterval": 120,
     "keepaliveCount": 5,
     "keepaliveRequired": true,
-    "mdnsGroupName": "libvirtTest",
     "services": [
         {
             "auth": 0,
diff --git c/tests/virnetservertest.c w/tests/virnetservertest.c
index 08431d76b2c0..8c4f39c5f44d 100644
--- c/tests/virnetservertest.c
+++ w/tests/virnetservertest.c
@@ -44,7 +44,11 @@ testCreateServer(const char *host, int family)

     if (!(srv = virNetServerNew(10, 50, 5, 100, 10,
                                 120, 5, true,
+# ifdef WITH_AVAHI
                                 "libvirtTest",
+# else
+                                NULL,
+# endif
                                 NULL,
                                 NULL,
                                 NULL,
@@ -230,7 +234,11 @@ static int testExecRestart(const void *opaque)
  cleanup:
     if (ret < 0) {
         virErrorPtr err = virGetLastError();
-        fprintf(stderr, "%s\n", err->message);
+        /* Rather be safe, we have lot of missing errors */
+        if (err)
+            fprintf(stderr, "%s\n", err->message);
+        else
+            fprintf(stderr, "%s\n", "Unknown error");
     }
  fail:
     VIR_FREE(infile);
@@ -275,7 +283,10 @@ mymain(void)
             ret = -1;                                   \
     } while (0)

+# ifdef WITH_AVAHI
     EXEC_RESTART_TEST("initial");
+# endif
+    EXEC_RESTART_TEST("initial-nomdns");
     EXEC_RESTART_TEST("anon-clients");

     return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
--

Martin
-------------- 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/20150608/48a37959/attachment-0001.sig>


More information about the libvir-list mailing list