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

[Martin Kletzander]

On Wed, May 20, 2015 at 06:11:14PM +0200, Michal Privoznik wrote:
>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 at redhat.com>
>> ---
>>  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.
>

I see I forgot to reply on few things here.

You're right, that's a leftover.

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

Nope, the beauty of this is that just refs up the server and returns
it.  Similarly to virQEMUDriverGetCapabilities().

>> +    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.
>

What you describe is still possible, you can call
virNetServerUpdateServices() on one server, nobody is saying you
cannot.  This function just abstracts starting all servers together
into one function.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150521/008ec67f/attachment-0001.sig>