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

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

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

Attachment: signature.asc
Description: PGP signature

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