[libvirt] [PATCH v2 0/3] Testing of RPC JSON (de)serialization

This is a re-post of the previous patch I sent: https://www.redhat.com/archives/libvir-list/2015-May/msg00812.html The only change is to actually send the 2 pre-requisite patches for it ! Daniel P. Berrange (3): rpc: allow selection of TCP address family rpc: add API for checking IPv4/6 availability rpc: add testing of RPC JSON (de)serialization daemon/libvirtd.c | 2 + src/libvirt_remote.syms | 2 + src/libxl/libxl_migration.c | 8 +- src/qemu/qemu_migration.c | 4 +- src/remote/remote_driver.c | 3 +- src/rpc/virnetclient.c | 12 +- src/rpc/virnetclient.h | 4 +- src/rpc/virnetserver.c | 4 +- src/rpc/virnetserver.h | 3 + src/rpc/virnetserverclient.c | 13 +- src/rpc/virnetserverservice.c | 8 +- src/rpc/virnetserverservice.h | 1 + src/rpc/virnetsocket.c | 74 +++++- src/rpc/virnetsocket.h | 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 +++++++++++++++++++++ tests/virnetsockettest.c | 44 +--- 22 files changed, 695 insertions(+), 55 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 -- 2.4.1

By default, getaddrinfo() will return addresses for both IPv4 and IPv6 if both protocols are enabled, and so the RPC code will listen/connect to both protocols too. There may be cases where it is desirable to restrict this to just one of the two protocols, so add an 'int family' parameter to all the TCP related APIs. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 ++ src/libxl/libxl_migration.c | 8 ++++++-- src/qemu/qemu_migration.c | 4 +++- src/remote/remote_driver.c | 3 ++- src/rpc/virnetclient.c | 12 +++++++++--- src/rpc/virnetclient.h | 4 +++- src/rpc/virnetserverservice.c | 2 ++ src/rpc/virnetserverservice.h | 1 + src/rpc/virnetsocket.c | 8 +++++++- src/rpc/virnetsocket.h | 3 +++ tests/virnetsockettest.c | 8 ++++++-- 11 files changed, 44 insertions(+), 11 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 3e7f87c..1e2c002 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -509,6 +509,7 @@ daemonSetupNetworking(virNetServerPtr srv, config->listen_addr, config->tcp_port); if (!(svcTCP = virNetServerServiceNewTCP(config->listen_addr, config->tcp_port, + AF_UNSPEC, config->auth_tcp, #if WITH_GNUTLS NULL, @@ -552,6 +553,7 @@ daemonSetupNetworking(virNetServerPtr srv, if (!(svcTLS = virNetServerServiceNewTCP(config->listen_addr, config->tls_port, + AF_UNSPEC, config->auth_tls, ctxt, false, diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 10b5bd6..39e4a65 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -389,7 +389,9 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, snprintf(portstr, sizeof(portstr), "%d", port); - if (virNetSocketNewListenTCP(hostname, portstr, &socks, &nsocks) < 0) { + if (virNetSocketNewListenTCP(hostname, portstr, + AF_UNSPEC, + &socks, &nsocks) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("Fail to create socket for incoming migration")); goto error; @@ -491,7 +493,9 @@ libxlDomainMigrationPerform(libxlDriverPrivatePtr driver, snprintf(portstr, sizeof(portstr), "%d", port); /* socket connect to dst host:port */ - if (virNetSocketNewConnectTCP(hostname, portstr, &sock) < 0) { + if (virNetSocketNewConnectTCP(hostname, portstr, + AF_UNSPEC, + &sock) < 0) { virReportSystemError(saved_errno, _("unable to connect to '%s:%s'"), hostname, portstr); diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f7432e8..0929e7a 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -3866,7 +3866,9 @@ qemuMigrationConnect(virQEMUDriverPtr driver, if (virSecurityManagerSetSocketLabel(driver->securityManager, vm->def) < 0) goto cleanup; - if (virNetSocketNewConnectTCP(host, port, &sock) == 0) { + if (virNetSocketNewConnectTCP(host, port, + AF_UNSPEC, + &sock) == 0) { spec->dest.fd.qemu = virNetSocketDupFD(sock, true); virObjectUnref(sock); } diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index dd8dab6..273799b 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -819,7 +819,7 @@ doRemoteOpen(virConnectPtr conn, /*FALLTHROUGH*/ case trans_tcp: - priv->client = virNetClientNewTCP(priv->hostname, port); + priv->client = virNetClientNewTCP(priv->hostname, port, AF_UNSPEC); if (!priv->client) goto failed; @@ -854,6 +854,7 @@ doRemoteOpen(virConnectPtr conn, priv->client = virNetClientNewLibSSH2(priv->hostname, port, + AF_UNSPEC, username, keyfile, knownHosts, diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c index 7fca055..d0b96b6 100644 --- a/src/rpc/virnetclient.c +++ b/src/rpc/virnetclient.c @@ -349,11 +349,14 @@ virNetClientPtr virNetClientNewUNIX(const char *path, virNetClientPtr virNetClientNewTCP(const char *nodename, - const char *service) + const char *service, + int family) { virNetSocketPtr sock; - if (virNetSocketNewConnectTCP(nodename, service, &sock) < 0) + if (virNetSocketNewConnectTCP(nodename, service, + family, + &sock) < 0) return NULL; return virNetClientNew(sock, nodename); @@ -383,6 +386,7 @@ virNetClientPtr virNetClientNewSSH(const char *nodename, VAR = VAL; virNetClientPtr virNetClientNewLibSSH2(const char *host, const char *port, + int family, const char *username, const char *privkeyPath, const char *knownHostsPath, @@ -473,7 +477,9 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host, if (!(command = virBufferContentAndReset(&buf))) goto no_memory; - if (virNetSocketNewConnectLibSSH2(host, port, username, privkey, + if (virNetSocketNewConnectLibSSH2(host, port, + family, + username, privkey, knownhosts, knownHostsVerify, authMethods, command, authPtr, uri, &sock) != 0) goto cleanup; diff --git a/src/rpc/virnetclient.h b/src/rpc/virnetclient.h index 3bcde63..38f929c 100644 --- a/src/rpc/virnetclient.h +++ b/src/rpc/virnetclient.h @@ -41,7 +41,8 @@ virNetClientPtr virNetClientNewUNIX(const char *path, const char *binary); virNetClientPtr virNetClientNewTCP(const char *nodename, - const char *service); + const char *service, + int family); virNetClientPtr virNetClientNewSSH(const char *nodename, const char *service, @@ -55,6 +56,7 @@ virNetClientPtr virNetClientNewSSH(const char *nodename, virNetClientPtr virNetClientNewLibSSH2(const char *host, const char *port, + int family, const char *username, const char *privkeyPath, const char *knownHostsPath, diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c index d84b6de..d3cf31a 100644 --- a/src/rpc/virnetserverservice.c +++ b/src/rpc/virnetserverservice.c @@ -143,6 +143,7 @@ virNetServerServiceNewFDOrUNIX(const char *path, virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, const char *service, + int family, int auth, #if WITH_GNUTLS virNetTLSContextPtr tls, @@ -169,6 +170,7 @@ virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, if (virNetSocketNewListenTCP(nodename, service, + family, &svc->socks, &svc->nsocks) < 0) goto error; diff --git a/src/rpc/virnetserverservice.h b/src/rpc/virnetserverservice.h index b1d6c2d..9afa0b1 100644 --- a/src/rpc/virnetserverservice.h +++ b/src/rpc/virnetserverservice.h @@ -51,6 +51,7 @@ virNetServerServicePtr virNetServerServiceNewFDOrUNIX(const char *path, unsigned int *cur_fd); virNetServerServicePtr virNetServerServiceNewTCP(const char *nodename, const char *service, + int family, int auth, # if WITH_GNUTLS virNetTLSContextPtr tls, diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index 51f94d4..d9f3e11 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -219,6 +219,7 @@ static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, int virNetSocketNewListenTCP(const char *nodename, const char *service, + int family, virNetSocketPtr **retsocks, size_t *nretsocks) { @@ -236,6 +237,7 @@ int virNetSocketNewListenTCP(const char *nodename, *nretsocks = 0; memset(&hints, 0, sizeof(hints)); + hints.ai_family = family; hints.ai_flags = AI_PASSIVE; hints.ai_socktype = SOCK_STREAM; @@ -454,6 +456,7 @@ int virNetSocketNewListenFD(int fd, int virNetSocketNewConnectTCP(const char *nodename, const char *service, + int family, virNetSocketPtr *retsock) { struct addrinfo *ai = NULL; @@ -470,6 +473,7 @@ int virNetSocketNewConnectTCP(const char *nodename, memset(&remoteAddr, 0, sizeof(remoteAddr)); memset(&hints, 0, sizeof(hints)); + hints.ai_family = family; hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG; hints.ai_socktype = SOCK_STREAM; @@ -801,6 +805,7 @@ int virNetSocketNewConnectSSH(const char *nodename, int virNetSocketNewConnectLibSSH2(const char *host, const char *port, + int family, const char *username, const char *privkey, const char *knownHosts, @@ -892,7 +897,7 @@ virNetSocketNewConnectLibSSH2(const char *host, } /* connect to remote server */ - if ((ret = virNetSocketNewConnectTCP(host, port, &sock)) < 0) + if ((ret = virNetSocketNewConnectTCP(host, port, family, &sock)) < 0) goto error; /* connect to the host using ssh */ @@ -915,6 +920,7 @@ virNetSocketNewConnectLibSSH2(const char *host, int virNetSocketNewConnectLibSSH2(const char *host ATTRIBUTE_UNUSED, const char *port ATTRIBUTE_UNUSED, + int family ATTRIBUTE_UNUSED, const char *username ATTRIBUTE_UNUSED, const char *privkey ATTRIBUTE_UNUSED, const char *knownHosts ATTRIBUTE_UNUSED, diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index 86bc2f6..a8ff8a9 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -47,6 +47,7 @@ typedef void (*virNetSocketIOFunc)(virNetSocketPtr sock, int virNetSocketNewListenTCP(const char *nodename, const char *service, + int family, virNetSocketPtr **addrs, size_t *naddrs); @@ -61,6 +62,7 @@ int virNetSocketNewListenFD(int fd, int virNetSocketNewConnectTCP(const char *nodename, const char *service, + int family, virNetSocketPtr *addr); int virNetSocketNewConnectUNIX(const char *path, @@ -84,6 +86,7 @@ int virNetSocketNewConnectSSH(const char *nodename, int virNetSocketNewConnectLibSSH2(const char *host, const char *port, + int family, const char *username, const char *privkey, const char *knownHosts, diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index 5d91f26..f609484 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -166,7 +166,9 @@ static int testSocketTCPAccept(const void *opaque) snprintf(portstr, sizeof(portstr), "%d", data->port); - if (virNetSocketNewListenTCP(data->lnode, portstr, &lsock, &nlsock) < 0) + if (virNetSocketNewListenTCP(data->lnode, portstr, + AF_UNSPEC, + &lsock, &nlsock) < 0) goto cleanup; for (i = 0; i < nlsock; i++) { @@ -174,7 +176,9 @@ static int testSocketTCPAccept(const void *opaque) goto cleanup; } - if (virNetSocketNewConnectTCP(data->cnode, portstr, &csock) < 0) + if (virNetSocketNewConnectTCP(data->cnode, portstr, + AF_UNSPEC, + &csock) < 0) goto cleanup; virObjectUnref(csock); -- 2.4.1

On Fri, Jun 05, 2015 at 09:47:43AM +0100, Daniel P. Berrange wrote:
By default, getaddrinfo() will return addresses for both IPv4 and IPv6 if both protocols are enabled, and so the RPC code will listen/connect to both protocols too. There may be cases where it is desirable to restrict this to just one of the two protocols, so add an 'int family' parameter to all the TCP related APIs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 2 ++ src/libxl/libxl_migration.c | 8 ++++++-- src/qemu/qemu_migration.c | 4 +++- src/remote/remote_driver.c | 3 ++- src/rpc/virnetclient.c | 12 +++++++++--- src/rpc/virnetclient.h | 4 +++- src/rpc/virnetserverservice.c | 2 ++ src/rpc/virnetserverservice.h | 1 + src/rpc/virnetsocket.c | 8 +++++++- src/rpc/virnetsocket.h | 3 +++ tests/virnetsockettest.c | 8 ++++++-- 11 files changed, 44 insertions(+), 11 deletions(-)
ACK

The socket test suite has a function for checking if IPv4 or IPv6 are available, and returning a free socket. The first bit of that will be needed in another test, so pull that logic out into a separate helper method. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_remote.syms | 1 + src/rpc/virnetsocket.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 3 +++ tests/virnetsockettest.c | 36 ++------------------------ 4 files changed, 72 insertions(+), 34 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index 6b520b5..950e56e 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -171,6 +171,7 @@ virNetServerServiceToggle; # rpc/virnetsocket.h virNetSocketAccept; virNetSocketAddIOCallback; +virNetSocketCheckProtocols; virNetSocketClose; virNetSocketDupFD; virNetSocketGetFD; diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c index d9f3e11..2497f67 100644 --- a/src/rpc/virnetsocket.c +++ b/src/rpc/virnetsocket.c @@ -29,6 +29,9 @@ #include <sys/wait.h> #include <signal.h> #include <fcntl.h> +#ifdef HAVE_IFADDRS_H +# include <ifaddrs.h> +#endif #include <netdb.h> #ifdef HAVE_NETINET_TCP_H @@ -142,6 +145,69 @@ static int virNetSocketForkDaemon(const char *binary) } #endif +int virNetSocketCheckProtocols(bool *hasIPv4, + bool *hasIPv6) +{ +#ifdef HAVE_IFADDRS_H + struct ifaddrs *ifaddr = NULL, *ifa; + struct addrinfo hints; + struct addrinfo *ai = NULL; + int ret = -1; + int gaierr; + + memset(&hints, 0, sizeof(hints)); + + *hasIPv4 = *hasIPv6 = false; + + if (getifaddrs(&ifaddr) < 0) { + virReportSystemError(errno, "%s", + _("Cannot get host interface addresses")); + goto cleanup; + } + + for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) { + if (!ifa->ifa_addr) + continue; + + if (ifa->ifa_addr->sa_family == AF_INET) + *hasIPv4 = true; + if (ifa->ifa_addr->sa_family == AF_INET6) + *hasIPv6 = true; + } + + freeifaddrs(ifaddr); + + hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG; + hints.ai_family = AF_INET6; + hints.ai_socktype = SOCK_STREAM; + + if ((gaierr = getaddrinfo("::1", NULL, &hints, &ai)) != 0) { + if (gaierr == EAI_ADDRFAMILY || + gaierr == EAI_FAMILY) { + *hasIPv6 = false; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot resolve ::1 address: %s"), + gai_strerror(gaierr)); + goto cleanup; + } + } + + freeaddrinfo(ai); + + VIR_DEBUG("Protocols: v4 %d v6 %d\n", *hasIPv4, *hasIPv6); + + ret = 0; + cleanup: + return ret; +#else + *hasIPv4 = *hasIPv6 = false; + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Cannot check address family on this platform")); + return -1; +#endif +} + static virNetSocketPtr virNetSocketNew(virSocketAddrPtr localAddr, virSocketAddrPtr remoteAddr, diff --git a/src/rpc/virnetsocket.h b/src/rpc/virnetsocket.h index a8ff8a9..5de3d92 100644 --- a/src/rpc/virnetsocket.h +++ b/src/rpc/virnetsocket.h @@ -45,6 +45,9 @@ typedef void (*virNetSocketIOFunc)(virNetSocketPtr sock, void *opaque); +int virNetSocketCheckProtocols(bool *hasIPv4, + bool *hasIPv6); + int virNetSocketNewListenTCP(const char *nodename, const char *service, int family, diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c index f609484..1ababad 100644 --- a/tests/virnetsockettest.c +++ b/tests/virnetsockettest.c @@ -48,47 +48,15 @@ static int checkProtocols(bool *hasIPv4, bool *hasIPv6, int *freePort) { - struct ifaddrs *ifaddr = NULL, *ifa; - struct addrinfo hints; - struct addrinfo *ai = NULL; struct sockaddr_in in4; struct sockaddr_in6 in6; int s4 = -1, s6 = -1; size_t i; int ret = -1; - memset(&hints, 0, sizeof(hints)); - - *hasIPv4 = *hasIPv6 = false; *freePort = 0; - - if (getifaddrs(&ifaddr) < 0) { - perror("getifaddrs"); - goto cleanup; - } - - for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) { - if (!ifa->ifa_addr) - continue; - - if (ifa->ifa_addr->sa_family == AF_INET) - *hasIPv4 = true; - if (ifa->ifa_addr->sa_family == AF_INET6) - *hasIPv6 = true; - } - - hints.ai_flags = AI_PASSIVE | AI_ADDRCONFIG; - hints.ai_family = AF_INET6; - hints.ai_socktype = SOCK_STREAM; - - if (getaddrinfo("::1", "5672", &hints, &ai) != 0) - *hasIPv6 = false; - - freeaddrinfo(ai); - - VIR_DEBUG("Protocols: v4 %d v6 %d\n", *hasIPv4, *hasIPv6); - - freeifaddrs(ifaddr); + if (virNetSocketCheckProtocols(hasIPv4, hasIPv6) < 0) + return -1; for (i = 0; i < 50; i++) { int only = 1; -- 2.4.1

On Fri, Jun 05, 2015 at 09:47:44AM +0100, Daniel P. Berrange wrote:
The socket test suite has a function for checking if IPv4 or IPv6 are available, and returning a free socket. The first bit of that will be needed in another test, so pull that logic out into a separate helper method.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_remote.syms | 1 + src/rpc/virnetsocket.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++ src/rpc/virnetsocket.h | 3 +++ tests/virnetsockettest.c | 36 ++------------------------ 4 files changed, 72 insertions(+), 34 deletions(-)
ACK

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; } } diff --git a/tests/Makefile.am b/tests/Makefile.am index 8e2dbec..c9e2c8a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -188,6 +188,7 @@ if WITH_REMOTE test_programs += \ virnetmessagetest \ virnetsockettest \ + virnetservertest \ virnetserverclienttest \ $(NULL) if WITH_GNUTLS @@ -921,6 +922,12 @@ virnetsockettest_SOURCES = \ virnetsockettest.c testutils.h testutils.c virnetsockettest_LDADD = $(LDADDS) +virnetservertest_SOURCES = \ + virnetservertest.c \ + testutils.h testutils.c +virnetservertest_CFLAGS = $(XDR_CFLAGS) $(AM_CFLAGS) +virnetservertest_LDADD = $(LDADDS) + virnetserverclienttest_SOURCES = \ virnetserverclienttest.c \ testutils.h testutils.c diff --git a/tests/virnetserverdata/README b/tests/virnetserverdata/README new file mode 100644 index 0000000..d6d79d2 --- /dev/null +++ b/tests/virnetserverdata/README @@ -0,0 +1,14 @@ + virnetservertest data files + =========================== + +The various input-data-*.json files are a record of all the historical +formats that libvirt has been able to produce data for. Everytime a +new field is added to the JSON output, a *new* input data file should +be created. We must not add new fields to existing input-data files, +nor must we ever re-structure them if code changes, as we must check +new code handles the legacy formats. + +The various output-data-*.json files are the record of what the *new* +JSON output should look like for the correspondingly named input-data +file. It is permissible to change the existing output-data-*.json +files if the format we save in is updated. diff --git a/tests/virnetserverdata/input-data-anon-clients.json b/tests/virnetserverdata/input-data-anon-clients.json new file mode 100644 index 0000000..ed91b57 --- /dev/null +++ b/tests/virnetserverdata/input-data-anon-clients.json @@ -0,0 +1,63 @@ +{ + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 10, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] +} diff --git a/tests/virnetserverdata/input-data-initial.json b/tests/virnetserverdata/input-data-initial.json new file mode 100644 index 0000000..7956225 --- /dev/null +++ b/tests/virnetserverdata/input-data-initial.json @@ -0,0 +1,62 @@ +{ + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] +} diff --git a/tests/virnetserverdata/output-data-anon-clients.json b/tests/virnetserverdata/output-data-anon-clients.json new file mode 100644 index 0000000..ed91b57 --- /dev/null +++ b/tests/virnetserverdata/output-data-anon-clients.json @@ -0,0 +1,63 @@ +{ + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 10, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] +} diff --git a/tests/virnetserverdata/output-data-initial.json b/tests/virnetserverdata/output-data-initial.json new file mode 100644 index 0000000..da02aac --- /dev/null +++ b/tests/virnetserverdata/output-data-initial.json @@ -0,0 +1,63 @@ +{ + "min_workers": 10, + "max_workers": 50, + "priority_workers": 5, + "max_clients": 100, + "max_anonymous_clients": 100, + "keepaliveInterval": 120, + "keepaliveCount": 5, + "keepaliveRequired": true, + "mdnsGroupName": "libvirtTest", + "services": [ + { + "auth": 0, + "readonly": true, + "nrequests_client_max": 2, + "socks": [ + { + "fd": 100, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + }, + { + "auth": 2, + "readonly": false, + "nrequests_client_max": 5, + "socks": [ + { + "fd": 101, + "errfd": -1, + "pid": 0, + "isClient": false + } + ] + } + ], + "clients": [ + { + "auth": 1, + "readonly": true, + "nrequests_max": 15, + "sock": { + "fd": 102, + "errfd": -1, + "pid": -1, + "isClient": true + } + }, + { + "auth": 2, + "readonly": true, + "nrequests_max": 66, + "sock": { + "fd": 103, + "errfd": -1, + "pid": -1, + "isClient": true + } + } + ] +} diff --git a/tests/virnetservertest.c b/tests/virnetservertest.c new file mode 100644 index 0000000..08431d7 --- /dev/null +++ b/tests/virnetservertest.c @@ -0,0 +1,290 @@ +/* + * Copyright (C) 2015 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include "testutils.h" +#include "virerror.h" +#include "rpc/virnetserver.h" + +#define VIR_FROM_THIS VIR_FROM_RPC + +#ifdef HAVE_SOCKETPAIR +static virNetServerPtr +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", + 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, +# ifdef WITH_GNUTLS + NULL, +# endif + false, + 25, + 5))) + goto error; + + if (virNetServerAddService(srv, svc1, "libvirt-ro") < 0) + goto error; + if (virNetServerAddService(srv, svc2, "libvirt-ro") < 0) + goto error; + + if (virNetSocketNewConnectSockFD(fdclient[0], &sk1) < 0) + goto error; + if (virNetSocketNewConnectSockFD(fdclient[1], &sk2) < 0) + goto error; + + if (!(cln1 = virNetServerClientNew(sk1, + VIR_NET_SERVER_SERVICE_AUTH_SASL, + true, + 15, +# ifdef WITH_GNUTLS + NULL, +# endif + NULL, NULL, NULL, NULL))) + goto error; + + if (!(cln2 = virNetServerClientNew(sk2, + VIR_NET_SERVER_SERVICE_AUTH_POLKIT, + true, + 66, +# ifdef WITH_GNUTLS + NULL, +# endif + NULL, NULL, NULL, NULL))) + goto error; + + if (virNetServerAddClient(srv, cln1) < 0) + goto error; + + if (virNetServerAddClient(srv, cln2) < 0) + goto error; + + cleanup: + virObjectUnref(cln1); + virObjectUnref(cln2); + virObjectUnref(svc1); + virObjectUnref(svc2); + return srv; + + error: + virObjectUnref(srv); + srv = NULL; + goto cleanup; +} + +static char *testGenerateJSON(void) +{ + virNetServerPtr srv = NULL; + virJSONValuePtr json = NULL; + char *jsonstr = NULL; + bool has_ipv4, has_ipv6; + + /* Our pre-saved JSON file is created so that each service + * only has one socket. If we let libvirt bind to IPv4 and + * IPv6 we might end up with two sockets, so force one or + * the other based on what's available on thehost + */ + if (virNetSocketCheckProtocols(&has_ipv4, + &has_ipv6) < 0) + return NULL; + + if (!has_ipv4 && !has_ipv6) + return NULL; + + if (!(srv = testCreateServer( + has_ipv4 ? "127.0.0.1" : "::1", + has_ipv4 ? AF_INET : AF_INET6))) + goto cleanup; + + if (!(json = virNetServerPreExecRestart(srv))) + goto cleanup; + + if (!(jsonstr = virJSONValueToString(json, true))) + goto cleanup; + fprintf(stderr, "%s\n", jsonstr); + cleanup: + virNetServerClose(srv); + virObjectUnref(srv); + virJSONValueFree(json); + return jsonstr; +} + + +struct testExecRestartData { + const char *jsonfile; +}; + +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); + } + 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"); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} +#else +static int +mymain(void) +{ + return EXIT_AM_SKIP; +} +#endif +VIRT_TEST_MAIN(mymain); -- 2.4.1

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

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
participants (2)
-
Daniel P. Berrange
-
Martin Kletzander