On Thu, Jul 25, 2013 at 04:23:32PM +0200, Michal Privoznik wrote:
> Currently, even if max_client limit is hit, we accept() incoming
> connection request, but close it immediately. This has disadvantage of
> not using listen() queue. We should accept() only those clients we
> know we can serve and let all other wait in the (limited) queue.
> ---
> src/rpc/virnetserver.c | 40 ++++++++++++++++++++++++++++++++++++++++
> src/rpc/virnetserverservice.c | 9 +++++++++
> src/rpc/virnetserverservice.h | 4 ++++
> 3 files changed, 53 insertions(+)
>
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index cb770c3..7ea1707 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -315,6 +315,34 @@ static int virNetServerDispatchNewClient(virNetServerServicePtr svc,
> }
>
>
> +static int
> +virNetServerDispatchCheck(virNetServerServicePtr svc,
> + virNetSocketPtr socket,
> + void *opaque)
> +{
> + virNetServerPtr srv = opaque;
> + int ret = -1;
> +
> + VIR_DEBUG("svc=%p socket=%p opaque=%p", svc, socket, opaque);
> +
> + if (srv->nclients >= srv->nclients_max) {
> + VIR_DEBUG("Not accepting client now due to max_clients setting (%zu)",
> + srv->nclients_max);
> +
> + /* We need to temporarily disable the server services in order to
> + * prevent event loop calling us over and over again. */
> +
> + virNetServerUpdateServices(srv, false);
> +
> + goto cleanup;
> + }
> +
> + ret = 0;
> +cleanup:
> + return ret;
> +}
> +
> +
> static void
> virNetServerFatalSignal(int sig, siginfo_t *siginfo ATTRIBUTE_UNUSED,
> void *context ATTRIBUTE_UNUSED)
> @@ -981,6 +1009,7 @@ int virNetServerAddService(virNetServerPtr srv,
>
> virNetServerServiceSetDispatcher(svc,
> virNetServerDispatchNewClient,
> + virNetServerDispatchCheck,
> srv);
>
> virObjectUnlock(srv);
> @@ -1110,6 +1139,8 @@ void virNetServerRun(virNetServerPtr srv)
> virNetServerClientClose(srv->clients[i]);
> if (virNetServerClientIsClosed(srv->clients[i])) {
> virNetServerClientPtr client = srv->clients[i];
> + bool update_services;
> +
> if (srv->nclients > 1) {
> memmove(srv->clients + i,
> srv->clients + i + 1,
> @@ -1120,7 +1151,16 @@ void virNetServerRun(virNetServerPtr srv)
> srv->nclients = 0;
> }
>
> + /* Enable services if we can accept a new client.
> + * The new client can be accepted if we are at the limit. */
> + update_services = srv->nclients == srv->nclients_max - 1;
> +
> virObjectUnlock(srv);
> + if (update_services) {
> + /* Now it makes sense to accept() a new client. */
> + VIR_DEBUG("Re-enabling services");
> + virNetServerUpdateServices(srv, true);
> + }
> virObjectUnref(client);
> virObjectLock(srv);
>
> diff --git a/src/rpc/virnetserverservice.c b/src/rpc/virnetserverservice.c
> index 632f03d..a47c8f8 100644
> --- a/src/rpc/virnetserverservice.c
> +++ b/src/rpc/virnetserverservice.c
> @@ -46,6 +46,7 @@ struct _virNetServerService {
> #endif
>
> virNetServerServiceDispatchFunc dispatchFunc;
> + virNetServerServiceDispatchCheckFunc dispatchCheckFunc;
> void *dispatchOpaque;
> };
>
> @@ -74,6 +75,12 @@ static void virNetServerServiceAccept(virNetSocketPtr sock,
> virNetServerServicePtr svc = opaque;
> virNetSocketPtr clientsock = NULL;
>
> + if (svc->dispatchCheckFunc &&
> + svc->dispatchCheckFunc(svc, sock, svc->dispatchOpaque) < 0) {
> + /* Accept declined */
> + goto cleanup;
> + }
> +
Rather than having this callback, can we not simply change virNetServerAddClient()
to call virNetServerUpdateServices(srv, false); when a new client causes us to hit
the max limit ?
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list