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

[Martin Kletzander]

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

diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
index 950e56e..bdd68f6 100644
--- a/src/libvirt_remote.syms
+++ b/src/libvirt_remote.syms
@@ -77,6 +77,7 @@ xdr_virNetMessageError;


# rpc/virnetserver.h
+virNetServerAddClient;
virNetServerAddProgram;
virNetServerAddService;
virNetServerAddShutdownInhibition;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 42427dc..091a1dc 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -259,8 +259,8 @@ static int 
virNetServerDispatchNewMessage(virNetServerClientPtr client,
}


-static int virNetServerAddClient(virNetServerPtr srv,
-                                 virNetServerClientPtr client)
+int virNetServerAddClient(virNetServerPtr srv,
+                          virNetServerClientPtr client)
{
    virObjectLock(srv);

diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
index 8c5ae07..4b452be 100644
--- a/src/rpc/virnetserver.h
+++ b/src/rpc/virnetserver.h
@@ -79,6 +79,9 @@ int virNetServerAddService(virNetServerPtr srv,
                           virNetServerServicePtr svc,
                           const char *mdnsEntryName);

+int virNetServerAddClient(virNetServerPtr srv,
+                          virNetServerClientPtr client);
+
int virNetServerAddProgram(virNetServerPtr srv,
                           virNetServerProgramPtr prog);

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 34c1994..0e3a71f 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -536,13 +536,14 @@ virJSONValuePtr 
virNetServerClientPreExecRestart(virNetServerClientPtr client)
        goto error;
    }

-    if (client->privateData && client->privateDataPreExecRestart &&
-        !(child = client->privateDataPreExecRestart(client, 
client->privateData)))
-        goto error;
+    if (client->privateData && client->privateDataPreExecRestart) {
+        if (!(child = client->privateDataPreExecRestart(client, 
client->privateData)))
+            goto error;

-    if (virJSONValueObjectAppend(object, "privateData", child) < 0) {
-        virJSONValueFree(child);
-        goto error;
+        if (virJSONValueObjectAppend(object, "privateData", child) < 0) {
+            virJSONValueFree(child);
+            goto error;
+        }
    }

    virObjectUnlock(client);
diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
index d3cf31a..4df26cb 100644
--- a/src/rpc/virnetserverservice.c
+++ b/src/rpc/virnetserverservice.c
@@ -303,12 +303,15 @@ virNetServerServicePtr virNetServerServiceNewFD(int fd,

        /* IO callback is initially disabled, until we're ready
         * to deal with incoming clients */
+        virObjectRef(svc);
        if (virNetSocketAddIOCallback(svc->socks[i],
                                      0,
                                      virNetServerServiceAccept,
                                      svc,
-                                      virObjectFreeCallback) < 0)
+                                      virObjectFreeCallback) < 0) {
+            virObjectUnref(svc);
            goto error;
+        }
    }


@@ -388,7 +391,6 @@ virNetServerServicePtr 
virNetServerServiceNewPostExecRestart(virJSONValuePtr obj
                                      svc,
                                      virObjectFreeCallback) < 0) {
            virObjectUnref(svc);
-            virObjectUnref(sock);
            goto error;
        }
    }

I forgot to say that these hunks are already in, so it won't apply
with these, of course, but you'll note that when rebasing.

Martin

signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list