[libvirt] [PATCH 10/13] Add support for admin API in libvirt daemon
Martin Kletzander
mkletzan at redhat.com
Fri May 22 05:22:39 UTC 2015
On Thu, May 21, 2015 at 05:46:54PM +0200, Michal Privoznik wrote:
>On 20.05.2015 07:19, Martin Kletzander wrote:
>> For this to pe properly separated from other protocols used by the
>> server, there is second server added which allows access to the whole
>> virNetDaemon to its clients.
>>
>> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
>> ---
>> cfg.mk | 3 ++
>> daemon/Makefile.am | 34 ++++++++++++++-
>> daemon/admin_server.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> daemon/admin_server.h | 36 ++++++++++++++++
>> daemon/libvirtd.c | 104 +++++++++++++++++++++++++++++++++++++++-----
>> daemon/libvirtd.h | 14 +++++-
>> po/POTFILES.in | 1 +
>> 7 files changed, 295 insertions(+), 13 deletions(-)
>> create mode 100644 daemon/admin_server.c
>> create mode 100644 daemon/admin_server.h
>>
>
>
>> diff --git a/daemon/Makefile.am b/daemon/Makefile.am
>> index 42dec5d..45c6e9b 100644
>> --- a/daemon/Makefile.am
>> +++ b/daemon/Makefile.am
>> @@ -1,6 +1,6 @@
>> ## Process this file with automake to produce Makefile.in
>>
>> -## Copyright (C) 2005-2014 Red Hat, Inc.
>> +## Copyright (C) 2005-2015 Red Hat, Inc.
>> ##
>> ## This library is free software; you can redistribute it and/or
>> ## modify it under the terms of the GNU Lesser General Public
>> @@ -25,6 +25,7 @@ INCLUDES = \
>> -I$(top_srcdir)/src/conf \
>> -I$(top_srcdir)/src/rpc \
>> -I$(top_srcdir)/src/remote \
>> + -I$(top_srcdir)/src/admin \
>> -I$(top_srcdir)/src/access \
>> $(GETTEXT_CPPFLAGS)
>>
>> @@ -34,6 +35,7 @@ DAEMON_GENERATED = \
>> remote_dispatch.h \
>> lxc_dispatch.h \
>> qemu_dispatch.h \
>> + admin_dispatch.h \
>> $(NULL)
>>
>> DAEMON_SOURCES = \
>> @@ -49,6 +51,7 @@ EXTRA_DIST = \
>> remote_dispatch.h \
>> lxc_dispatch.h \
>> qemu_dispatch.h \
>> + admin_dispatch.h \
>> libvirtd.conf \
>> libvirtd.init.in \
>> libvirtd.upstart \
>> @@ -78,6 +81,9 @@ BUILT_SOURCES =
>> REMOTE_PROTOCOL = $(top_srcdir)/src/remote/remote_protocol.x
>> LXC_PROTOCOL = $(top_srcdir)/src/remote/lxc_protocol.x
>> QEMU_PROTOCOL = $(top_srcdir)/src/remote/qemu_protocol.x
>> +ADMIN_PROTOCOL = $(top_srcdir)/src/admin/admin_protocol.x
>> +
>> +BUILT_SOURCES += admin_dispatch.h
>
>No. This one is not really a built source. Into BUILT_SOURCES should go
>only those files which are built from the .tar.gz. This one is built
>from git. I mean, you need whole set of developer tools (e.g. rpcgen in
>this case) to generate some files which are part of released package.
>For instance, previously unpacking the package, and running 'make clean'
>would not clean anything. Now it cleans admin_dispatch.h. Worse, in
>order to generate it you need rpcgen, which you previously didn't.
>
>>
>> remote_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \
>> $(REMOTE_PROTOCOL)
>> @@ -97,6 +103,12 @@ qemu_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \
>> --mode=server qemu QEMU $(QEMU_PROTOCOL) \
>> > $(srcdir)/qemu_dispatch.h
>>
>> +admin_dispatch.h: $(srcdir)/../src/rpc/gendispatch.pl \
>> + $(ADMIN_PROTOCOL)
>> + $(AM_V_GEN)$(PERL) -w $(srcdir)/../src/rpc/gendispatch.pl \
>> + --mode=server admin ADMIN $(ADMIN_PROTOCOL) \
>> + > $(srcdir)/admin_dispatch.h
>> +
>> if WITH_LIBVIRTD
>>
>> # Build a convenience library, for reuse in tests/libvirtdconftest
>
>> diff --git a/daemon/admin_server.c b/daemon/admin_server.c
>> new file mode 100644
>> index 0000000..810908b
>> --- /dev/null
>> +++ b/daemon/admin_server.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * admin_server.c:
>> + *
>> + * Copyright (C) 2014-2015 Red Hat, Inc.
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Lesser General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2.1 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> + * Lesser General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Lesser General Public
>> + * License along with this library. If not, see
>> + * <http://www.gnu.org/licenses/>.
>> + *
>> + * Author: Martin Kletzander <mkletzan at redhat.com>
>> + */
>> +
>> +#include <config.h>
>> +
>> +#include "internal.h"
>> +#include "libvirtd.h"
>> +#include "libvirt_internal.h"
>> +
>> +#include "admin_protocol.h"
>> +#include "admin_server.h"
>> +#include "datatypes.h"
>> +#include "viralloc.h"
>> +#include "virerror.h"
>> +#include "virlog.h"
>> +#include "virnetdaemon.h"
>> +#include "virnetserver.h"
>> +#include "virstring.h"
>> +#include "virthreadjob.h"
>> +
>> +#define VIR_FROM_THIS VIR_FROM_ADMIN
>> +
>> +VIR_LOG_INIT("daemon.admin");
>> +
>> +
>> +void
>> +remoteAdmClientFreeFunc(void *data)
>> +{
>> + struct daemonAdmClientPrivate *priv = data;
>> +
>> + virObjectUnref(priv->dmn);
>
>virMutexDestroy(&priv->lock);
>
>> + VIR_FREE(data);
>> +}
>> +
>> +void *
>> +remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED,
>> + void *opaque)
>> +{
>> + struct daemonAdmClientPrivate *priv;
>> +
>> + if (VIR_ALLOC(priv) < 0)
>> + return NULL;
>> +
>> + if (virMutexInit(&priv->lock) < 0) {
>> + VIR_FREE(priv);
>> + virReportSystemError(errno, "%s", _("unable to init mutex"));
>> + return NULL;
>> + }
>> +
>> + /*
>> + * We don't necessarily need to ref this object right now as there
>> + * must be one ref being held throughout the life of the daemon,
>> + * but let's just be safe for future.
>> + */
>> + priv->dmn = virObjectRef(opaque);
>> +
>> + return priv;
>> +}
>
>> diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
>> index 22ba6cb..1205671 100644
>> --- a/daemon/libvirtd.c
>> +++ b/daemon/libvirtd.c
>> @@ -44,6 +44,7 @@
>> #include "libvirtd.h"
>> #include "libvirtd-config.h"
>>
>> +#include "admin_server.h"
>> #include "viruuid.h"
>> #include "remote_driver.h"
>> #include "viralloc.h"
>> @@ -112,6 +113,7 @@ VIR_LOG_INIT("daemon.libvirtd");
>> virNetSASLContextPtr saslCtxt = NULL;
>> #endif
>> virNetServerProgramPtr remoteProgram = NULL;
>> +virNetServerProgramPtr adminProgram = NULL;
>> virNetServerProgramPtr qemuProgram = NULL;
>> virNetServerProgramPtr lxcProgram = NULL;
>>
>> @@ -253,18 +255,24 @@ static int
>> daemonUnixSocketPaths(struct daemonConfig *config,
>> bool privileged,
>> char **sockfile,
>> - char **rosockfile)
>> + char **rosockfile,
>> + char **admsockfile)
>> {
>> if (config->unix_sock_dir) {
>> if (virAsprintf(sockfile, "%s/libvirt-sock", config->unix_sock_dir) < 0)
>> goto error;
>> - if (privileged &&
>> - virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0)
>> - goto error;
>> +
>> + if (privileged) {
>> + if (virAsprintf(rosockfile, "%s/libvirt-sock-ro", config->unix_sock_dir) < 0)
>> + goto error;
>> + if (virAsprintf(admsockfile, "%s/libvirt-admin-sock", config->unix_sock_dir) < 0)
>> + goto error;
>> + }
>> } else {
>> if (privileged) {
>> if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock") < 0 ||
>> - VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0)
>> + VIR_STRDUP(*rosockfile, LOCALSTATEDIR "/run/libvirt/libvirt-sock-ro") < 0 ||
>> + VIR_STRDUP(*admsockfile, LOCALSTATEDIR "/run/libvirt/libvirt-admin-sock") < 0)
>> goto error;
>> } else {
>> char *rundir = NULL;
>> @@ -280,7 +288,8 @@ daemonUnixSocketPaths(struct daemonConfig *config,
>> }
>> umask(old_umask);
>>
>> - if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0) {
>> + if (virAsprintf(sockfile, "%s/libvirt-sock", rundir) < 0 ||
>> + virAsprintf(admsockfile, "%s/libvirt-admin-sock", rundir) < 0) {
>> VIR_FREE(rundir);
>> goto error;
>> }
>> @@ -427,13 +436,16 @@ static void daemonInitialize(void)
>>
>> static int ATTRIBUTE_NONNULL(3)
>> daemonSetupNetworking(virNetServerPtr srv,
>> + virNetServerPtr srvAdm,
>> struct daemonConfig *config,
>> const char *sock_path,
>> const char *sock_path_ro,
>> + const char *sock_path_adm,
>> bool ipsock,
>> bool privileged)
>> {
>> virNetServerServicePtr svc = NULL;
>> + virNetServerServicePtr svcAdm = NULL;
>> virNetServerServicePtr svcRO = NULL;
>> virNetServerServicePtr svcTCP = NULL;
>> #if WITH_GNUTLS
>> @@ -442,6 +454,7 @@ daemonSetupNetworking(virNetServerPtr srv,
>> gid_t unix_sock_gid = 0;
>> int unix_sock_ro_mask = 0;
>> int unix_sock_rw_mask = 0;
>> + int unix_sock_adm_mask = 0;
>>
>> unsigned int cur_fd = STDERR_FILENO + 1;
>> unsigned int nfds = virGetListenFDs();
>> @@ -461,6 +474,11 @@ daemonSetupNetworking(virNetServerPtr srv,
>> goto error;
>> }
>>
>> + if (virStrToLong_i(config->unix_sock_admin_perms, NULL, 8, &unix_sock_adm_mask) != 0) {
>> + VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_admin_perms);
>> + goto error;
>> + }
>> +
>> if (virStrToLong_i(config->unix_sock_rw_perms, NULL, 8, &unix_sock_rw_mask) != 0) {
>> VIR_ERROR(_("Failed to parse mode '%s'"), config->unix_sock_rw_perms);
>> goto error;
>> @@ -503,6 +521,24 @@ daemonSetupNetworking(virNetServerPtr srv,
>> virNetServerAddService(srv, svcRO, NULL) < 0)
>> goto error;
>>
>> + if (sock_path_adm) {
>> + VIR_DEBUG("Registering unix socket %s", sock_path_adm);
>> + if (!(svcAdm = virNetServerServiceNewUNIX(sock_path_adm,
>> + unix_sock_adm_mask,
>> + unix_sock_gid,
>> + REMOTE_AUTH_NONE,
>> +#if WITH_GNUTLS
>> + NULL,
>> +#endif
>> + true,
>> + config->max_queued_clients,
>> + config->max_client_requests)))
>> + goto error;
>> + }
>> +
>> + if (virNetServerAddService(srvAdm, svcAdm, NULL) < 0)
>> + goto error;
>> +
>> if (ipsock) {
>> if (config->listen_tcp) {
>> VIR_DEBUG("Registering TCP socket %s:%s",
>> @@ -600,6 +636,7 @@ daemonSetupNetworking(virNetServerPtr srv,
>> virObjectUnref(svcTCP);
>> virObjectUnref(svc);
>> virObjectUnref(svcRO);
>> + virObjectUnref(svcAdm);
>> return -1;
>> }
>>
>> @@ -1100,6 +1137,7 @@ daemonUsage(const char *argv0, bool privileged)
>> int main(int argc, char **argv) {
>> virNetDaemonPtr dmn = NULL;
>> virNetServerPtr srv = NULL;
>> + virNetServerPtr srvAdm = NULL;
>> char *remote_config_file = NULL;
>> int statuswrite = -1;
>> int ret = 1;
>> @@ -1107,6 +1145,7 @@ int main(int argc, char **argv) {
>> char *pid_file = NULL;
>> char *sock_file = NULL;
>> char *sock_file_ro = NULL;
>> + char *sock_file_adm = NULL;
>> int timeout = -1; /* -t: Shutdown timeout */
>> int verbose = 0;
>> int godaemon = 0;
>> @@ -1274,12 +1313,15 @@ int main(int argc, char **argv) {
>> if (daemonUnixSocketPaths(config,
>> privileged,
>> &sock_file,
>> - &sock_file_ro) < 0) {
>> + &sock_file_ro,
>> + &sock_file_adm) < 0) {
>> VIR_ERROR(_("Can't determine socket paths"));
>> exit(EXIT_FAILURE);
>> }
>> - VIR_DEBUG("Decided on socket paths '%s' and '%s'",
>> - sock_file, NULLSTR(sock_file_ro));
>> + VIR_DEBUG("Decided on socket paths '%s', '%s' and '%s'",
>> + sock_file,
>> + NULLSTR(sock_file_ro),
>> + NULLSTR(sock_file_adm));
>>
>> if (godaemon) {
>> char ebuf[1024];
>> @@ -1411,6 +1453,40 @@ int main(int argc, char **argv) {
>> goto cleanup;
>> }
>>
>> + if (!(srvAdm = virNetServerNew(config->min_workers,
>> + config->max_workers,
>> + config->prio_workers,
>> + config->max_clients,
>> + config->max_anonymous_clients,
>
>Hm.. This may work for now, but I guess we want different limits here.
>Users running libvirt in production may have really big worker pool,
>while admin API will suffice one or two, like 640 kilo of RAM (TM).
>
Yes, makes sense, I forgot this was a trial part as well. I'll add
the configuration and different defaults.
>> + config->keepalive_interval,
>> + config->keepalive_count,
>
>I guess keepalive on a socket unix is overkill. The eventloop will see
>EOF immediately as client closes the socket.
>
It's not about seeing EOF, but about *not* seeing EOF. We want to
disconnect from clients that are caught in an endless loop.
>> + !!config->keepalive_required,
>> + NULL,
>> + remoteAdmClientInitHook,
>> + NULL,
>> + remoteAdmClientFreeFunc,
>> + dmn))) {
>> + ret = VIR_DAEMON_ERR_INIT;
>> + goto cleanup;
>> + }
>> +
>> + if (virNetDaemonAddServer(dmn, srvAdm) < 0) {
>> + ret = VIR_DAEMON_ERR_INIT;
>> + goto cleanup;
>> + }
>
>This is what I was talking about in reply to 02/13. Imagine the limits
>for domain worker pool and sockets were set really low on daemon
>startup. No new client can be accepted there. I would expect mgmt
>application to connect to admin API and size up the limits. However, due
>to bug in 2/13 all servers are suspended. I'll leave it up to you where
>you'd like to solve it.
>
There's nothing to solve. The client is being added in a particular
server and that server will call virNetServerUpdateServicesLocked()
only on itself, not virNetDaemonUpdateServices() on the daemon.
>> +
>> + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM,
>> + ADMIN_PROTOCOL_VERSION,
>> + adminProcs,
>> + adminNProcs))) {
>> + ret = VIR_DAEMON_ERR_INIT;
>> + goto cleanup;
>> + }
>> + if (virNetServerAddProgram(srvAdm, adminProgram) < 0) {
>> + ret = VIR_DAEMON_ERR_INIT;
>> + goto cleanup;
>> + }
>> +
>
>Otherwise looking good.
>
>Michal
-------------- 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/8b65ca2c/attachment-0001.sig>
More information about the libvir-list
mailing list