[libvirt] [PATCH v2 1/9] rpc: virnetserverclient: Identify clients by an integer ID

[Michal Privoznik]

On 29.04.2016 14:39, Erik Skultety wrote:
> Admin API needs a way of addressing specific clients. Unlike servers, which we
> are happy to address by names both because its name reflects its purpose (to
> some extent) and we only have two of them (so far), naming clients doesn't make
> any sense, since a) each client is an anonymous, i.e. not recognized after a
> disconnect followed by a reconnect, b) we can't predict what kind of requests
> it's going to send to daemon, and c) the are loads of them comming and going,
> so the only viable option is to use an ID which is of a reasonably wide data
> type.
> 
> Signed-off-by: Erik Skultety <eskultet at redhat.com>
> ---
>  daemon/libvirtd.c                                  |  4 +-
>  src/libvirt_remote.syms                            |  1 +
>  src/locking/lock_daemon.c                          |  2 +-
>  src/logging/log_daemon.c                           |  2 +-
>  src/lxc/lxc_controller.c                           |  2 +-
>  src/rpc/virnetserver.c                             | 35 ++++++++++-
>  src/rpc/virnetserver.h                             |  3 +
>  src/rpc/virnetserverclient.c                       | 38 ++++++++++--
>  src/rpc/virnetserverclient.h                       |  7 ++-
>  tests/virnetdaemondata/input-data-client-ids.json  | 68 ++++++++++++++++++++++
>  .../virnetdaemondata/output-data-admin-nomdns.json |  6 ++
>  .../output-data-admin-server-names.json            |  6 ++
>  .../virnetdaemondata/output-data-anon-clients.json |  3 +
>  tests/virnetdaemondata/output-data-client-ids.json | 68 ++++++++++++++++++++++
>  .../output-data-initial-nomdns.json                |  3 +
>  tests/virnetdaemondata/output-data-initial.json    |  3 +
>  .../output-data-no-keepalive-required.json         |  6 ++
>  tests/virnetdaemontest.c                           |  9 ++-
>  tests/virnetserverclienttest.c                     |  2 +-
>  19 files changed, 249 insertions(+), 19 deletions(-)
>  create mode 100644 tests/virnetdaemondata/input-data-client-ids.json
>  create mode 100644 tests/virnetdaemondata/output-data-client-ids.json
> 
> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> index 7ec02ad..f5d1e2f 100644
> --- a/daemon/libvirtd.c
> +++ b/daemon/libvirtd.c
> @@ -1389,7 +1389,7 @@ int main(int argc, char **argv) {
>          goto cleanup;
>      }
>  
> -    if (!(srv = virNetServerNew("libvirtd",
> +    if (!(srv = virNetServerNew("libvirtd", 1,
>                                  config->min_workers,
>                                  config->max_workers,
>                                  config->prio_workers,
> @@ -1464,7 +1464,7 @@ int main(int argc, char **argv) {
>          goto cleanup;
>      }
>  
> -    if (!(srvAdm = virNetServerNew("admin",
> +    if (!(srvAdm = virNetServerNew("admin", 1,
>                                     config->admin_min_workers,
>                                     config->admin_max_workers,
>                                     0,
> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
> index 66f9383..c049874 100644
> --- a/src/libvirt_remote.syms
> +++ b/src/libvirt_remote.syms
> @@ -105,6 +105,7 @@ virNetServerGetName;
>  virNetServerHasClients;
>  virNetServerNew;
>  virNetServerNewPostExecRestart;
> +virNetServerNextClientID;
>  virNetServerPreExecRestart;
>  virNetServerProcessClients;
>  virNetServerStart;
> diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c
> index bfdcfc6..f889a34 100644
> --- a/src/locking/lock_daemon.c
> +++ b/src/locking/lock_daemon.c
> @@ -160,7 +160,7 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged)
>          return NULL;
>      }
>  
> -    if (!(srv = virNetServerNew("virtlockd",
> +    if (!(srv = virNetServerNew("virtlockd", 1,
>                                  1, 1, 0, config->max_clients,
>                                  config->max_clients, -1, 0,
>                                  NULL,
> diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c
> index 70339af..90f8427 100644
> --- a/src/logging/log_daemon.c
> +++ b/src/logging/log_daemon.c
> @@ -150,7 +150,7 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged)
>          return NULL;
>      }
>  
> -    if (!(logd->srv = virNetServerNew("virtlogd",
> +    if (!(logd->srv = virNetServerNew("virtlogd", 1,
>                                        1, 1, 0, config->max_clients,
>                                        config->max_clients, -1, 0,
>                                        NULL,
> diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
> index de59775..0304354 100644
> --- a/src/lxc/lxc_controller.c
> +++ b/src/lxc/lxc_controller.c
> @@ -934,7 +934,7 @@ static int virLXCControllerSetupServer(virLXCControllerPtr ctrl)
>                      LXC_STATE_DIR, ctrl->name) < 0)
>          return -1;
>  
> -    if (!(srv = virNetServerNew("LXC",
> +    if (!(srv = virNetServerNew("LXC", 1,
>                                  0, 0, 0, 1,
>                                  0, -1, 0,
>                                  NULL,
> diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
> index 57bd95c..8216da6 100644
> --- a/src/rpc/virnetserver.c
> +++ b/src/rpc/virnetserver.c
> @@ -65,6 +65,7 @@ struct _virNetServer {
>  
>      size_t nclients;                    /* Current clients count */
>      virNetServerClientPtr *clients;     /* Clients */
> +    unsigned long long next_client_id;  /* next client ID */

Any reason this shouldn't be size_t just like the rest below?

>      size_t nclients_max;                /* Max allowed clients count */
>      size_t nclients_unauth;             /* Unauthenticated clients count */
>      size_t nclients_unauth_max;         /* Max allowed unauth clients count */


> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
> index 64dab46..7233773 100644
> --- a/src/rpc/virnetserverclient.c
> +++ b/src/rpc/virnetserverclient.c
> @@ -28,6 +28,7 @@
>  # include <sasl/sasl.h>
>  #endif
>  
> +#include "virnetserver.h"
>  #include "virnetserverclient.h"
>  
>  #include "virlog.h"
> @@ -65,6 +66,7 @@ struct _virNetServerClient
>  {
>      virObjectLockable parent;
>  
> +    unsigned long long id;

Again, this could be size_t.

>      bool wantClose;
>      bool delayedClose;
>      virNetSocketPtr sock;

Michal