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

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

 



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

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]