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

Re: [libvirt] [PATCH 04/15] Break virNetServer into virNetSubServers



On Thu, Apr 16, 2015 at 04:46:39PM +0200, Martin Kletzander wrote:
> Each subserver has its own RPC programs, services, workers, keepalive,
> clients etc.  Hence (possible) multiple subservers are properly
> separated.
> 
> The part in remote.c is just mechanical, the same applies to most of the
> code movement from virnetserver.c to virnetsubserver.c.

So, the problem we're facing here is that virNetServer is really filling
two distinct roles in one class - it is the thing that manages the overall
daemon process state, as well as managing the server connection/services.

So the splitting up you're doing does rather make sense. I wonder if we
could name it differently though. eg have virNetDaemon to handle the
process level stuff, and virNetServer to handle the network level stuff.
So what is in virnetserver.c moves to virnetdaemon.c and your new
virnetsubserver.c becomes the new virnetserver.c


> diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h
> index 8c5ae07..6b1630b 100644
> --- a/src/rpc/virnetserver.h
> +++ b/src/rpc/virnetserver.h
> @@ -1,7 +1,7 @@
>  /*
>   * virnetserver.h: generic network RPC server
>   *
> - * Copyright (C) 2006-2011 Red Hat, Inc.
> + * Copyright (C) 2006-2015 Red Hat, Inc.
>   * Copyright (C) 2006 Daniel P. Berrange
>   *
>   * This library is free software; you can redistribute it and/or
> @@ -29,32 +29,37 @@
>  # ifdef WITH_GNUTLS
>  #  include "virnettlscontext.h"
>  # endif
> +# include "virobject.h"
> +# include "virjson.h"
>  # include "virnetserverprogram.h"
>  # include "virnetserverclient.h"
>  # include "virnetserverservice.h"
> -# include "virobject.h"
> -# include "virjson.h"
> -
> -virNetServerPtr virNetServerNew(size_t min_workers,
> -                                size_t max_workers,
> -                                size_t priority_workers,
> -                                size_t max_clients,
> -                                size_t max_anonymous_clients,
> -                                int keepaliveInterval,
> -                                unsigned int keepaliveCount,
> -                                bool keepaliveRequired,
> -                                const char *mdnsGroupName,
> -                                virNetServerClientPrivNew clientPrivNew,
> -                                virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
> -                                virFreeCallback clientPrivFree,
> -                                void *clientPrivOpaque);
> -
> -virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object,
> -                                               virNetServerClientPrivNew clientPrivNew,
> -                                               virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart,
> -                                               virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
> -                                               virFreeCallback clientPrivFree,
> -                                               void *clientPrivOpaque);
> +# include "virnetsubserver.h"
> +
> +virNetServerPtr virNetServerNew(const char *mdnsGroupName);
> +
> +int virNetServerAddSubServer(virNetServerPtr srv,
> +                             size_t min_workers,
> +                             size_t max_workers,
> +                             size_t priority_workers,
> +                             size_t max_clients,
> +                             size_t max_anonymous_clients,
> +                             int keepaliveInterval,
> +                             unsigned int keepaliveCount,
> +                             bool keepaliveRequired,
> +                             virNetServerClientPrivNew clientPrivNew,
> +                             virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
> +                             virFreeCallback clientPrivFree,
> +                             void *clientPrivOpaque);
> +
> +int virNetServerAddSubServerPostExec(virNetServerPtr srv,
> +                                     virNetServerClientPrivNew clientPrivNew,
> +                                     virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart,
> +                                     virNetServerClientPrivPreExecRestart clientPrivPreExecRestart,
> +                                     virFreeCallback clientPrivFree,
> +                                     void *clientPrivOpaque);
> +
> +virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object);
> 
>  virJSONValuePtr virNetServerPreExecRestart(virNetServerPtr srv);
> 
> @@ -76,10 +81,12 @@ int virNetServerAddSignalHandler(virNetServerPtr srv,
>                                   void *opaque);
> 
>  int virNetServerAddService(virNetServerPtr srv,
> +                           int subServerID,
>                             virNetServerServicePtr svc,
>                             const char *mdnsEntryName);
> 
>  int virNetServerAddProgram(virNetServerPtr srv,
> +                           int subServerID,
>                             virNetServerProgramPtr prog);

How about just moving these two methods to take the virNetSubServerPtr as
their first arg, instead of needing to pass virNetServer + subServerID ?
Not a big deal either way though.

Regards,
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 :|


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