Re: [PATCH 02/13] Move daemon-related parts of virNetServer to virNetDaemon

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

 



On 20.05.2015 07:19, Martin Kletzander wrote:
> This allows to have more servers in one daemon which helps isolating
> some resources.
> 
> Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> ---
>  daemon/libvirtd.c                  | 101 ++---
>  docs/internals.html.in             |   4 +-
>  docs/internals/rpc.html.in         |   7 +
>  po/POTFILES.in                     |   1 +
>  src/Makefile.am                    |   1 +
>  src/libvirt_remote.syms            |  28 +-
>  src/locking/lock_daemon.c          |  63 ++--
>  src/locking/lock_daemon_config.c   |   2 +-
>  src/locking/lock_daemon_dispatch.c |   4 +-
>  src/lxc/lxc_controller.c           |  65 ++--
>  src/rpc/virnetdaemon.c             | 746 +++++++++++++++++++++++++++++++++++++
>  src/rpc/virnetdaemon.h             |  82 ++++
>  src/rpc/virnetserver.c             | 526 ++++----------------------
>  src/rpc/virnetserver.h             |  46 +--
>  src/rpc/virnetserverprogram.h      |   3 +
>  15 files changed, 1074 insertions(+), 605 deletions(-)
>  create mode 100644 src/rpc/virnetdaemon.c
>  create mode 100644 src/rpc/virnetdaemon.h
> 

> diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c
> new file mode 100644
> index 0000000..8d42638
> --- /dev/null
> +++ b/src/rpc/virnetdaemon.c

> +
> +
> +static virClassPtr virNetDaemonClass;
> +
> +void

static

> +virNetDaemonDispose(void *obj)
> +{
> +    virNetDaemonPtr dmn = obj;
> +    size_t i;
> +
> +    VIR_FORCE_CLOSE(dmn->autoShutdownInhibitFd);
> +
> +    for (i = 0; i < dmn->nsignals; i++) {
> +        sigaction(dmn->signals[i]->signum, &dmn->signals[i]->oldaction, NULL);
> +        VIR_FREE(dmn->signals[i]);
> +    }
> +    VIR_FREE(dmn->signals);
> +    VIR_FORCE_CLOSE(dmn->sigread);
> +    VIR_FORCE_CLOSE(dmn->sigwrite);
> +    if (dmn->sigwatch > 0)
> +        virEventRemoveHandle(dmn->sigwatch);
> +
> +    for (i = 0; i < dmn->nservers; i++)
> +        virObjectUnref(dmn->servers[i]);
> +    VIR_FREE(dmn->servers);
> +
> +    virJSONValueFree(dmn->srvObject);
> +}
> +
> +static int
> +virNetDaemonOnceInit(void)
> +{
> +    if (!(virNetDaemonClass = virClassNew(virClassForObjectLockable(),
> +                                          "virNetDaemon",
> +                                          sizeof(virNetDaemon),
> +                                          virNetDaemonDispose)))
> +        return -1;
> +
> +    return 0;
> +}


> +/*
> + * Separate function merely for the purpose of unified error
> + * reporting.
> + */
> +static virNetServerPtr
> +virNetDaemonGetServerInternal(virNetDaemonPtr dmn,
> +                              int subServerID)
> +{
> +    if (subServerID < 0 || subServerID >= dmn->nservers) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Invalid server ID: %d"),
> +                       subServerID);
> +        return NULL;
> +    }
> +
> +    return virObjectRef(dmn->servers[subServerID]);
> +}
> +
> +/*
> + * The server is locked after this function.

No, it's not.

> + */
> +virNetServerPtr
> +virNetDaemonGetServer(virNetDaemonPtr dmn,
> +                      int subServerID)
> +{
> +    virNetServerPtr srv = NULL;
> +
> +    virObjectLock(dmn);
> +    srv = virNetDaemonGetServerInternal(dmn, subServerID);

Did you forget virObjectLock(srv) here?

> +    virObjectUnlock(dmn);
> +
> +    return srv;
> +}
> +

> +void
> +virNetDaemonUpdateServices(virNetDaemonPtr dmn,
> +                           bool enabled)
> +{
> +    size_t i;
> +
> +    virObjectLock(dmn);
> +    for (i = 0; i < dmn->nservers; i++)
> +        virNetServerUpdateServices(dmn->servers[i], enabled);
> +    virObjectUnlock(dmn);

Hm... I guess we need something slightly different. While we may want
one service to stop accepting new clients, we may want the other one to
still accept new ones. But since this is a big split, and so far a
daemon will have only one server, it can be saved for a follow up patch.

> +}
> +

Besides Dan's finding, looking okay. ACK.

Michal

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




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]