[libvirt] [PATCH 0/4] Add admin protocol support for virtlogd/virtlockd

The initial admin protocol support was only integrated into libvirtd. This series extracts that code so that it is reusable with all the daemons we have (and more than we'll get). Daniel P. Berrange (4): admin: move admins server impl/dispatch into src/admin directory util: add virGetUNIXSocketPath helper logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd .gitignore | 1 + cfg.mk | 3 +- daemon/Makefile.am | 33 +---- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 -- po/POTFILES.in | 4 +- src/Makefile.am | 33 ++++- {daemon => src/admin}/admin_server.c | 4 +- {daemon => src/admin}/admin_server.h | 6 +- .../admin.c => src/admin/admin_server_dispatch.c | 21 ++-- .../admin.h => src/admin/admin_server_dispatch.h | 9 +- src/libvirt-admin.c | 23 +++- src/locking/lock_daemon.c | 132 +++++++++++++++----- src/locking/lock_daemon_config.c | 3 + src/locking/lock_daemon_config.h | 1 + src/locking/test_virtlockd.aug.in | 4 + src/locking/virtlockd-admin.socket.in | 10 ++ src/locking/virtlockd.aug | 1 + src/locking/virtlockd.conf | 6 + src/locking/virtlockd.service.in | 1 + src/logging/log_daemon.c | 135 +++++++++++++++------ src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 + src/logging/virtlogd-admin.socket.in | 10 ++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + src/util/virutil.c | 45 +++++++ src/util/virutil.h | 1 + 29 files changed, 371 insertions(+), 137 deletions(-) rename {daemon => src/admin}/admin_server.c (99%) rename {daemon => src/admin}/admin_server.h (96%) rename daemon/admin.c => src/admin/admin_server_dispatch.c (96%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (83%) create mode 100644 src/locking/virtlockd-admin.socket.in create mode 100644 src/logging/virtlogd-admin.socket.in -- 2.14.3

The admin server functionality is a generic concept that should be wired up into all libvirt daemons, but is currently integrated with the libvirtd code. Move it all into the src/admin directory to prepare for broader reuse. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 2 +- daemon/Makefile.am | 33 +--------------------- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 ------- po/POTFILES.in | 4 +-- src/Makefile.am | 31 +++++++++++++++++++- {daemon => src/admin}/admin_server.c | 4 +-- {daemon => src/admin}/admin_server.h | 6 ++-- .../admin.c => src/admin/admin_server_dispatch.c | 21 +++++++++----- .../admin.h => src/admin/admin_server_dispatch.h | 9 +++--- 11 files changed, 60 insertions(+), 63 deletions(-) rename {daemon => src/admin}/admin_server.c (99%) rename {daemon => src/admin}/admin_server.h (96%) rename daemon/admin.c => src/admin/admin_server_dispatch.c (96%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (83%) diff --git a/.gitignore b/.gitignore index e2eaff1724..189116a3d0 100644 --- a/.gitignore +++ b/.gitignore @@ -123,6 +123,7 @@ /src/access/viraccessapicheckqemu.h /src/admin/admin_client.h /src/admin/admin_protocol.[ch] +/src/admin/admin_server_dispatch_stubs.h /src/esx/*.generated.* /src/hyperv/*.generated.* /src/libvirt*.def diff --git a/cfg.mk b/cfg.mk index 5cdeb7c650..4635f9f771 100644 --- a/cfg.mk +++ b/cfg.mk @@ -768,7 +768,7 @@ sc_prohibit_gettext_markup: # lower-level code must not include higher-level headers. cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) cross_dirs_re=($(subst / ,/|,$(cross_dirs))) -mid_dirs=access|conf|cpu|locking|logging|network|node_device|rpc|security|storage +mid_dirs=access|admin|conf|cpu|locking|logging|network|node_device|rpc|security|storage sc_prohibit_cross_inclusion: @for dir in $(cross_dirs); do \ case $$dir in \ diff --git a/daemon/Makefile.am b/daemon/Makefile.am index b0c28d2313..efd5ff19f5 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -37,7 +37,6 @@ DAEMON_GENERATED = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ - admin_dispatch.h \ $(NULL) DAEMON_SOURCES = \ @@ -61,7 +60,6 @@ EXTRA_DIST = \ remote_dispatch.h \ lxc_dispatch.h \ qemu_dispatch.h \ - admin_dispatch.h \ libvirtd.conf \ libvirtd.init.in \ libvirtd.upstart \ @@ -111,12 +109,6 @@ qemu_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ --mode=server qemu QEMU $(QEMU_PROTOCOL) \ > $(srcdir)/qemu_dispatch.h -admin_dispatch.h: $(top_srcdir)/src/rpc/gendispatch.pl \ - $(ADMIN_PROTOCOL) - $(AM_V_GEN)$(PERL) -w $(top_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 @@ -136,27 +128,6 @@ libvirtd_conf_la_LDFLAGS = \ $(NULL) libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) -noinst_LTLIBRARIES += libvirtd_admin.la -libvirtd_admin_la_SOURCES = \ - admin.c admin.h admin_server.c admin_server.h - -libvirtd_admin_la_CFLAGS = \ - $(AM_CFLAGS) \ - $(XDR_CFLAGS) \ - $(PIE_CFLAGS) \ - $(WARN_CFLAGS) \ - $(LIBXML_CFLAGS) \ - $(COVERAGE_CFLAGS) \ - $(NULL) -libvirtd_admin_la_LDFLAGS = \ - $(PIE_LDFLAGS) \ - $(RELRO_LDFLAGS) \ - $(COVERAGE_LDFLAGS) \ - $(NO_INDIRECT_LDFLAGS) \ - $(NULL) -libvirtd_admin_la_LIBADD = \ - ../src/libvirt-admin.la - man8_MANS = libvirtd.8 sbin_PROGRAMS = libvirtd @@ -202,7 +173,7 @@ endif WITH_DTRACE_PROBES libvirtd_LDADD += \ libvirtd_conf.la \ - libvirtd_admin.la \ + ../src/libvirt_driver_admin.la \ ../src/libvirt-lxc.la \ ../src/libvirt-qemu.la \ ../src/libvirt_driver_remote.la \ @@ -269,8 +240,6 @@ endif ! WITH_POLKIT remote.c: $(DAEMON_GENERATED) remote.h: $(DAEMON_GENERATED) -admin.c: $(DAEMON_GENERATED) -admin.h: $(DAEMON_GENERATED) LOGROTATE_CONFS = libvirtd.qemu.logrotate libvirtd.lxc.logrotate \ libvirtd.libxl.logrotate libvirtd.uml.logrotate \ diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 6d3b83355b..48bdc27a5f 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -43,7 +43,7 @@ #include "libvirtd.h" #include "libvirtd-config.h" -#include "admin.h" +#include "admin/admin_server_dispatch.h" #include "viruuid.h" #include "remote_driver.h" #include "viralloc.h" diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index b5707461fe..082c4bc4df 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -30,7 +30,6 @@ # include <rpc/types.h> # include <rpc/xdr.h> # include "remote_protocol.h" -# include "admin_protocol.h" # include "lxc_protocol.h" # include "qemu_protocol.h" # include "virthread.h" @@ -44,8 +43,6 @@ typedef struct daemonClientStream daemonClientStream; typedef daemonClientStream *daemonClientStreamPtr; typedef struct daemonClientPrivate daemonClientPrivate; typedef daemonClientPrivate *daemonClientPrivatePtr; -typedef struct daemonAdmClientPrivate daemonAdmClientPrivate; -typedef daemonAdmClientPrivate *daemonAdmClientPrivatePtr; typedef struct daemonClientEventCallback daemonClientEventCallback; typedef daemonClientEventCallback *daemonClientEventCallbackPtr; @@ -81,13 +78,6 @@ struct daemonClientPrivate { daemonClientStreamPtr streams; }; -/* Separate private data for admin connection */ -struct daemonAdmClientPrivate { - /* Just a placeholder, not that there is anything to be locked */ - virMutex lock; - - virNetDaemonPtr dmn; -}; # if WITH_SASL extern virNetSASLContextPtr saslCtxt; diff --git a/po/POTFILES.in b/po/POTFILES.in index c1fa23427e..4c0abb18ab 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,6 +1,4 @@ -daemon/admin.c daemon/admin_dispatch.h -daemon/admin_server.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h @@ -12,6 +10,8 @@ gnulib/lib/getopt.c gnulib/lib/regcomp.c src/access/viraccessdriverpolkit.c src/access/viraccessmanager.c +src/admin/admin_server.c +src/admin/admin_server_dispatch.c src/bhyve/bhyve_capabilities.c src/bhyve/bhyve_command.c src/bhyve/bhyve_device.c diff --git a/src/Makefile.am b/src/Makefile.am index 166c9a8e91..fd8756f10c 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -546,7 +546,9 @@ ADMIN_PROTOCOL = $(srcdir)/admin/admin_protocol.x ADMIN_PROTOCOL_GENERATED = \ admin/admin_protocol.c \ admin/admin_protocol.h \ - admin/admin_client.h + admin/admin_client.h \ + admin/admin_server_dispatch_stubs.h \ + $(NULL) admin/admin_client.h: $(srcdir)/rpc/gendispatch.pl \ $(ADMIN_PROTOCOL) Makefile.am @@ -554,6 +556,12 @@ admin/admin_client.h: $(srcdir)/rpc/gendispatch.pl \ admin ADMIN $(ADMIN_PROTOCOL) \ > $(srcdir)/admin/admin_client.h +admin/admin_server_dispatch_stubs.h: $(srcdir)/rpc/gendispatch.pl \ + $(ADMIN_PROTOCOL) Makefile.am + $(AM_V_GEN)$(PERL) -w $(srcdir)/rpc/gendispatch.pl --mode=server \ + admin ADMIN $(ADMIN_PROTOCOL) \ + > $(srcdir)/admin/admin_server_dispatch_stubs.h + EXTRA_DIST += $(ADMIN_PROTOCOL) $(ADMIN_PROTOCOL_GENERATED) BUILT_SOURCES += $(ADMIN_PROTOCOL_GENERATED) MAINTAINERCLEANFILES += $(ADMIN_PROTOCOL_GENERATED) @@ -2314,6 +2322,27 @@ libvirt_admin.syms: libvirt_admin_public.syms $(ADMIN_SYM_FILES) \ chmod a-w $@-tmp && \ mv $@-tmp libvirt_admin.syms + +admin/admin_server_dispatch.c: admin/admin_server_dispatch_stubs.h + +noinst_LTLIBRARIES += libvirt_driver_admin.la +libvirt_driver_admin_la_SOURCES = \ + admin/admin_protocol.c \ + admin/admin_protocol.h \ + admin/admin_server.c \ + admin/admin_server.h \ + admin/admin_server_dispatch.c \ + admin/admin_server_dispatch.h \ + admin/admin_server_dispatch_stubs.h \ + $(NULL) +libvirt_driver_admin_la_CFLAGS = \ + $(AM_CFLAGS) \ + -I$(top_srcdir)/src/util \ + -I$(top_srcdir)/src/admin \ + $(NULL) +libvirt_driver_admin_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_admin_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) + # admin/admin_remote.c is being included in libvirt-admin.c, so we # need to include it in the dist EXTRA_DIST += admin/admin_remote.c diff --git a/daemon/admin_server.c b/src/admin/admin_server.c similarity index 99% rename from daemon/admin_server.c rename to src/admin/admin_server.c index d8a3323e39..f2c1a8f8ec 100644 --- a/daemon/admin_server.c +++ b/src/admin/admin_server.c @@ -29,8 +29,8 @@ #include "virerror.h" #include "viridentity.h" #include "virlog.h" -#include "virnetdaemon.h" -#include "virnetserver.h" +#include "rpc/virnetdaemon.h" +#include "rpc/virnetserver.h" #include "virstring.h" #include "virthreadpool.h" #include "virtypedparam.h" diff --git a/daemon/admin_server.h b/src/admin/admin_server.h similarity index 96% rename from daemon/admin_server.h rename to src/admin/admin_server.h index 0baffa7ece..3e0c35fa29 100644 --- a/daemon/admin_server.h +++ b/src/admin/admin_server.h @@ -21,8 +21,8 @@ * Martin Kletzander <mkletzan@redhat.com> */ -#ifndef __LIBVIRTD_ADMIN_SERVER_H__ -# define __LIBVIRTD_ADMIN_SERVER_H__ +#ifndef __ADMIN_SERVER_H__ +# define __ADMIN_SERVER_H__ # include "rpc/virnetdaemon.h" # include "rpc/virnetserver.h" @@ -72,4 +72,4 @@ int adminServerSetClientLimits(virNetServerPtr srv, int nparams, unsigned int flags); -#endif /* __LIBVIRTD_ADMIN_SERVER_H__ */ +#endif /* __ADMIN_SERVER_H__ */ diff --git a/daemon/admin.c b/src/admin/admin_server_dispatch.c similarity index 96% rename from daemon/admin.c rename to src/admin/admin_server_dispatch.c index baf310c7bb..068358b049 100644 --- a/daemon/admin.c +++ b/src/admin/admin_server_dispatch.c @@ -1,5 +1,5 @@ /* - * admin.c: handlers for admin RPC method calls + * admin_server_dispatch.c: handlers for admin RPC method calls * * Copyright (C) 2014-2016 Red Hat, Inc. * @@ -23,18 +23,16 @@ #include <config.h> #include "internal.h" -#include "libvirtd.h" #include "libvirt_internal.h" -#include "admin_protocol.h" -#include "admin.h" +#include "admin_server_dispatch.h" #include "admin_server.h" #include "datatypes.h" #include "viralloc.h" #include "virerror.h" #include "virlog.h" -#include "virnetdaemon.h" -#include "virnetserver.h" +#include "rpc/virnetdaemon.h" +#include "rpc/virnetserver.h" #include "virstring.h" #include "virthreadjob.h" #include "virtypedparam.h" @@ -43,6 +41,15 @@ VIR_LOG_INIT("daemon.admin"); +typedef struct daemonAdmClientPrivate daemonAdmClientPrivate; +typedef daemonAdmClientPrivate *daemonAdmClientPrivatePtr; +/* Separate private data for admin connection */ +struct daemonAdmClientPrivate { + /* Just a placeholder, not that there is anything to be locked */ + virMutex lock; + + virNetDaemonPtr dmn; +}; void remoteAdmClientFreeFunc(void *data) @@ -487,4 +494,4 @@ adminDispatchConnectGetLoggingFilters(virNetServerPtr server ATTRIBUTE_UNUSED, return 0; } -#include "admin_dispatch.h" +#include "admin_server_dispatch_stubs.h" diff --git a/daemon/admin.h b/src/admin/admin_server_dispatch.h similarity index 83% rename from daemon/admin.h rename to src/admin/admin_server_dispatch.h index c869cc7aab..01723e5c43 100644 --- a/daemon/admin.h +++ b/src/admin/admin_server_dispatch.h @@ -1,5 +1,5 @@ /* - * admin.h: handlers for admin RPC method calls + * admin_server_dispatch.h: handlers for admin RPC method calls * * Copyright (C) 2014-2016 Red Hat, Inc. * @@ -20,11 +20,12 @@ * Author: Martin Kletzander <mkletzan@redhat.com> */ -#ifndef __LIBVIRTD_ADMIN_H__ -# define __LIBVIRTD_ADMIN_H__ +#ifndef __ADMIN_SERVER_DISPATCH_H__ +# define __ADMIN_SERVER_DISPATCH_H__ # include "rpc/virnetserverprogram.h" # include "rpc/virnetserverclient.h" +# include "admin/admin_protocol.h" extern virNetServerProgramProc adminProcs[]; @@ -33,4 +34,4 @@ extern size_t adminNProcs; void remoteAdmClientFreeFunc(void *data); void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); -#endif /* __ADMIN_REMOTE_H__ */ +#endif /* __ADMIN_SERVER_DISPATCH_H__ */ -- 2.14.3

On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
The admin server functionality is a generic concept that should be wired up into all libvirt daemons, but is currently integrated with the libvirtd code. Move it all into the src/admin directory to prepare for broader reuse.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- .gitignore | 1 + cfg.mk | 2 +- daemon/Makefile.am | 33 +--------------------- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 ------- po/POTFILES.in | 4 +-- src/Makefile.am | 31 +++++++++++++++++++- {daemon => src/admin}/admin_server.c | 4 +-- {daemon => src/admin}/admin_server.h | 6 ++-- .../admin.c => src/admin/admin_server_dispatch.c | 21 +++++++++----- .../admin.h => src/admin/admin_server_dispatch.h | 9 +++--- 11 files changed, 60 insertions(+), 63 deletions(-) rename {daemon => src/admin}/admin_server.c (99%) rename {daemon => src/admin}/admin_server.h (96%) rename daemon/admin.c => src/admin/admin_server_dispatch.c (96%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (83%)
Missed couple of occurrences: diff --git i/cfg.mk w/cfg.mk index 4635f9f77..4743f48f3 100644 --- i/cfg.mk +++ w/cfg.mk @@ -1119,7 +1119,7 @@ sc_po_check: \ $(srcdir)/daemon/remote_dispatch.h \ $(srcdir)/daemon/qemu_dispatch.h \ $(srcdir)/src/remote/remote_client_bodies.h \ - $(srcdir)/daemon/admin_dispatch.h \ + $(srcdir)/src/admin/admin_server_dispatch_stubs.h \ $(srcdir)/src/admin/admin_client.h $(srcdir)/daemon/remote_dispatch.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C daemon remote_dispatch.h @@ -1127,8 +1127,8 @@ $(srcdir)/daemon/qemu_dispatch.h: $(srcdir)/src/remote/qemu_protocol.x $(MAKE) -C daemon qemu_dispatch.h $(srcdir)/src/remote/remote_client_bodies.h: $(srcdir)/src/remote/remote_protocol.x $(MAKE) -C src remote/remote_client_bodies.h -$(srcdir)/daemon/admin_dispatch.h: $(srcdir)/src/admin/admin_protocol.x - $(MAKE) -C daemon admin_dispatch.h + $(srcdir)/src/admin/admin_server_dispatch_stubs.h: $(srcdir)/src/admin/admin_protocol.x + $(MAKE) -C src admin/admin_server_dispatch_stubs.h $(srcdir)/src/admin/admin_client.h: $(srcdir)/src/admin/admin_protocol.x $(MAKE) -C src admin/admin_client.h diff --git i/po/POTFILES.in w/po/POTFILES.in index 4c0abb18a..106b26277 100644 --- i/po/POTFILES.in +++ w/po/POTFILES.in @@ -1,4 +1,3 @@ -daemon/admin_dispatch.h daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h Michal

When receiving multiple socket FDs from systemd, it is critical to know what socket address each corresponds to so we can setup the right protocols on each. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virutil.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 1 + 2 files changed, 46 insertions(+) diff --git a/src/util/virutil.c b/src/util/virutil.c index e9dbaf3d7a..8352e0f1f1 100644 --- a/src/util/virutil.c +++ b/src/util/virutil.c @@ -68,6 +68,10 @@ # include <shlobj.h> #endif +#ifdef HAVE_SYS_UN_H +# include <sys/un.h> +#endif + #include "c-ctype.h" #include "mgetgroups.h" #include "virerror.h" @@ -1978,6 +1982,47 @@ virGetListenFDs(void) #endif /* WIN32 */ +#ifdef HAVE_SYS_UN_H +char *virGetUNIXSocketPath(int fd) +{ + struct sockaddr_storage ss = { 0 }; + struct sockaddr_un *un = (struct sockaddr_un *)&ss; + socklen_t len = sizeof(ss); + char *path; + + if (getsockname(fd, (struct sockaddr *)&ss, &len) < 0) { + virReportSystemError(errno, _("Unable to get address of FD %d"), fd); + return NULL; + } + + if (ss.ss_family != AF_UNIX) { + virReportSystemError(EINVAL, _("FD %d is not a UNIX socket, has af=%d"), + fd, ss.ss_family); + return NULL; + } + + if (un->sun_path[0] == '\0') + un->sun_path[0] = '@'; + + if (VIR_ALLOC_N(path, sizeof(un->sun_path) + 1) < 0) + return NULL; + + memcpy(path, un->sun_path, sizeof(un->sun_path)); + path[sizeof(un->sun_path)] = '\0'; + return path; +} + +#else /* HAVE_SYS_UN_H */ + +char *virGetUNIXSocketPath(int fd) +{ + virReportSystemError(ENOSYS, "%s", + _("UNIX sockets not supported on this platform"); + return NULL; +} + +#endif /* HAVE_SYS_UN_H */ + #ifndef WIN32 long virGetSystemPageSize(void) { diff --git a/src/util/virutil.h b/src/util/virutil.h index 9381ad5682..be0f6b0ea8 100644 --- a/src/util/virutil.h +++ b/src/util/virutil.h @@ -207,6 +207,7 @@ verify((int)VIR_TRISTATE_BOOL_NO == (int)VIR_TRISTATE_SWITCH_OFF); verify((int)VIR_TRISTATE_BOOL_ABSENT == (int)VIR_TRISTATE_SWITCH_ABSENT); unsigned int virGetListenFDs(void); +char *virGetUNIXSocketPath(int fd); long virGetSystemPageSize(void) ATTRIBUTE_NOINLINE; long virGetSystemPageSizeKB(void) ATTRIBUTE_NOINLINE; -- 2.14.3

On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
When receiving multiple socket FDs from systemd, it is critical to know what socket address each corresponds to so we can setup the right protocols on each.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/util/virutil.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 1 + 2 files changed, 46 insertions(+)
Don't forget to actually expose the symbol: diff --git i/src/libvirt_private.syms w/src/libvirt_private.syms index bc8cc1fba..54e3b9130 100644 --- i/src/libvirt_private.syms +++ w/src/libvirt_private.syms @@ -2962,6 +2962,7 @@ virGetListenFDs; virGetSelfLastChanged; virGetSystemPageSize; virGetSystemPageSizeKB; +virGetUNIXSocketPath; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; virGetUserConfigDirectory; Michal

Add a virtlogd-admin-sock can serves the admin protocol for the virtlogd daemon and define a virtlogd:///{system,session} URI scheme for connecting to it. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/libvirt-admin.c | 20 ++++-- src/logging/log_daemon.c | 135 ++++++++++++++++++++++++++--------- src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 ++ src/logging/virtlogd-admin.socket.in | 10 +++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + 9 files changed, 136 insertions(+), 40 deletions(-) create mode 100644 src/logging/virtlogd-admin.socket.in diff --git a/src/Makefile.am b/src/Makefile.am index fd8756f10c..828306fd35 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2672,6 +2672,7 @@ virtlogd_LDFLAGS = \ $(PIE_LDFLAGS) \ $(NULL) virtlogd_LDADD = \ + libvirt_driver_admin.la \ libvirt-net-rpc-server.la \ libvirt-net-rpc.la \ libvirt_util.la \ diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 48e6d7fc8a..9d1bff536b 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -38,8 +38,9 @@ #define VIR_FROM_THIS VIR_FROM_ADMIN -#define LIBVIRTD_ADMIN_SOCK_NAME "/libvirt-admin-sock" -#define LIBVIRTD_ADMIN_UNIX_SOCKET LOCALSTATEDIR "/run/libvirt" LIBVIRTD_ADMIN_SOCK_NAME +#define LIBVIRTD_ADMIN_SOCK_NAME "libvirt-admin-sock" +#define VIRTLOGD_ADMIN_SOCK_NAME "virtlogd-admin-sock" + VIR_LOG_INIT("libvirt-admin"); @@ -128,18 +129,25 @@ getSocketPath(virURIPtr uri) } if (!sock_path) { - if (STRNEQ_NULLABLE(uri->scheme, "libvirtd")) { + const char *sockbase = NULL; + if (STREQ_NULLABLE(uri->scheme, "libvirtd")) { + sockbase = LIBVIRTD_ADMIN_SOCK_NAME; + } else if (STREQ_NULLABLE(uri->scheme, "virtlogd")) { + sockbase = VIRTLOGD_ADMIN_SOCK_NAME; + } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported URI scheme '%s'"), NULLSTR(uri->scheme)); goto error; } + if (STREQ_NULLABLE(uri->path, "/system")) { - if (VIR_STRDUP(sock_path, LIBVIRTD_ADMIN_UNIX_SOCKET) < 0) + if (virAsprintf(&sock_path, LOCALSTATEDIR "/run/libvirt/%s", + sockbase) < 0) goto error; } else if (STREQ_NULLABLE(uri->path, "/session")) { - if (!rundir || virAsprintf(&sock_path, "%s%s", rundir, - LIBVIRTD_ADMIN_SOCK_NAME) < 0) + if (!rundir || virAsprintf(&sock_path, "%s/%s", rundir, + sockbase) < 0) goto error; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 7e8c9cfc29..ec2fd0b933 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -32,6 +32,7 @@ #include "log_daemon.h" #include "log_daemon_config.h" +#include "admin/admin_server_dispatch.h" #include "virutil.h" #include "virfile.h" #include "virpidfile.h" @@ -137,7 +138,7 @@ static virLogDaemonPtr virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) { virLogDaemonPtr logd; - virNetServerPtr srv; + virNetServerPtr srv = NULL; if (VIR_ALLOC(logd) < 0) return NULL; @@ -149,6 +150,9 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) return NULL; } + if (!(logd->dmn = virNetDaemonNew())) + goto error; + if (!(srv = virNetServerNew("virtlogd", 1, 1, 1, 0, config->max_clients, config->max_clients, -1, 0, @@ -159,8 +163,22 @@ virLogDaemonNew(virLogDaemonConfigPtr config, bool privileged) (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; - if (!(logd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(logd->dmn, srv) < 0) + if (virNetDaemonAddServer(logd->dmn, srv) < 0) + goto error; + virObjectUnref(srv); + srv = NULL; + + if (!(srv = virNetServerNew("admin", 1, + 1, 1, 0, config->admin_max_clients, + config->admin_max_clients, -1, 0, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + logd->dmn))) + goto error; + + if (virNetDaemonAddServer(logd->dmn, srv) < 0) goto error; virObjectUnref(srv); srv = NULL; @@ -337,10 +355,12 @@ virLogDaemonForkIntoBackground(const char *argv0) static int virLogDaemonUnixSocketPaths(bool privileged, - char **sockfile) + char **sockfile, + char **adminSockfile) { if (privileged) { - if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/virtlogd-sock") < 0) + if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/virtlogd-sock") < 0 || + VIR_STRDUP(*adminSockfile, LOCALSTATEDIR "/run/libvirt/virtlogd-admin-sock") < 0) goto error; } else { char *rundir = NULL; @@ -357,7 +377,8 @@ virLogDaemonUnixSocketPaths(bool privileged, } umask(old_umask); - if (virAsprintf(sockfile, "%s/virtlogd-sock", rundir) < 0) { + if (virAsprintf(sockfile, "%s/virtlogd-sock", rundir) < 0 || + virAsprintf(adminSockfile, "%s/virtlogd-admin-sock", rundir) < 0) { VIR_FREE(rundir); goto error; } @@ -468,29 +489,50 @@ virLogDaemonSetupSignals(virNetDaemonPtr dmn) static int -virLogDaemonSetupNetworkingSystemD(virNetServerPtr srv) +virLogDaemonSetupNetworkingSystemD(virNetServerPtr logSrv, virNetServerPtr adminSrv) { - virNetServerServicePtr svc; unsigned int nfds; + size_t i; if ((nfds = virGetListenFDs()) == 0) return 0; - if (nfds > 1) + if (nfds > 2) VIR_DEBUG("Too many (%d) file descriptors from systemd", nfds); - nfds = 1; - /* Systemd passes FDs, starting immediately after stderr, - * so the first FD we'll get is '3'. */ - if (!(svc = virNetServerServiceNewFD(3, 0, + for (i = 0; i < nfds && i < 2; i++) { + virNetServerServicePtr svc; + char *path = virGetUNIXSocketPath(3 + i); + virNetServerPtr srv; + + if (!path) + return -1; + + if (strstr(path, "virtlogd-admin-sock")) { + srv = adminSrv; + } else if (strstr(path, "virtlogd-sock")) { + srv = logSrv; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown UNIX socket %s passed in"), + path); + VIR_FREE(path); + return -1; + } + VIR_FREE(path); + + /* Systemd passes FDs, starting immediately after stderr, + * so the first FD we'll get is '3'. */ + if (!(svc = virNetServerServiceNewFD(3 + i, 0, #if WITH_GNUTLS - NULL, + NULL, #endif - false, 0, 1))) - return -1; + false, 0, 1))) + return -1; - if (virNetServerAddService(srv, svc, NULL) < 0) { - virObjectUnref(svc); - return -1; + if (virNetServerAddService(srv, svc, NULL) < 0) { + virObjectUnref(svc); + return -1; + } } return 1; } @@ -861,8 +903,10 @@ virLogDaemonUsage(const char *argv0, bool privileged) } int main(int argc, char **argv) { - virNetServerPtr srv = NULL; + virNetServerPtr logSrv = NULL; + virNetServerPtr adminSrv = NULL; virNetServerProgramPtr logProgram = NULL; + virNetServerProgramPtr adminProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -872,6 +916,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; int pid_file_fd = -1; char *sock_file = NULL; + char *admin_sock_file = NULL; int timeout = -1; /* -t: Shutdown timeout */ char *state_file = NULL; bool implicit_conf = false; @@ -999,12 +1044,13 @@ int main(int argc, char **argv) { VIR_DEBUG("Decided on pid file path '%s'", NULLSTR(pid_file)); if (virLogDaemonUnixSocketPaths(privileged, - &sock_file) < 0) { + &sock_file, + &admin_sock_file) < 0) { VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); } - VIR_DEBUG("Decided on socket paths '%s'", - sock_file); + VIR_DEBUG("Decided on socket paths '%s' and '%s'", + sock_file, admin_sock_file); if (virLogDaemonExecRestartStatePath(privileged, &state_file) < 0) { @@ -1081,22 +1127,27 @@ int main(int argc, char **argv) { goto cleanup; } - srv = virNetDaemonGetServer(logDaemon->dmn, "virtlogd"); - if ((rv = virLogDaemonSetupNetworkingSystemD(srv)) < 0) { + logSrv = virNetDaemonGetServer(logDaemon->dmn, "virtlogd"); + adminSrv = virNetDaemonGetServer(logDaemon->dmn, "admin"); + if ((rv = virLogDaemonSetupNetworkingSystemD(logSrv, adminSrv)) < 0) { ret = VIR_LOG_DAEMON_ERR_NETWORK; goto cleanup; } /* Only do this, if systemd did not pass a FD */ - if (rv == 0 && - virLogDaemonSetupNetworkingNative(srv, sock_file) < 0) { - ret = VIR_LOG_DAEMON_ERR_NETWORK; - goto cleanup; + if (rv == 0) { + if (virLogDaemonSetupNetworkingNative(logSrv, sock_file) < 0 || + virLogDaemonSetupNetworkingNative(adminSrv, admin_sock_file) < 0) { + ret = VIR_LOG_DAEMON_ERR_NETWORK; + goto cleanup; + } } - virObjectUnref(srv); + virObjectUnref(logSrv); + virObjectUnref(adminSrv); } - srv = virNetDaemonGetServer(logDaemon->dmn, "virtlogd"); + logSrv = virNetDaemonGetServer(logDaemon->dmn, "virtlogd"); + adminSrv = virNetDaemonGetServer(logDaemon->dmn, "admin"); if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); @@ -1116,7 +1167,19 @@ int main(int argc, char **argv) { ret = VIR_LOG_DAEMON_ERR_INIT; goto cleanup; } - if (virNetServerAddProgram(srv, logProgram) < 0) { + if (virNetServerAddProgram(logSrv, logProgram) < 0) { + ret = VIR_LOG_DAEMON_ERR_INIT; + goto cleanup; + } + + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM, + ADMIN_PROTOCOL_VERSION, + adminProcs, + adminNProcs))) { + ret = VIR_LOG_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(adminSrv, adminProgram) < 0) { ret = VIR_LOG_DAEMON_ERR_INIT; goto cleanup; } @@ -1138,7 +1201,8 @@ int main(int argc, char **argv) { /* Start accepting new clients from network */ - virNetServerUpdateServices(srv, true); + virNetServerUpdateServices(logSrv, true); + virNetServerUpdateServices(adminSrv, true); virNetDaemonRun(logDaemon->dmn); if (execRestart && @@ -1151,7 +1215,9 @@ int main(int argc, char **argv) { cleanup: virObjectUnref(logProgram); - virObjectUnref(srv); + virObjectUnref(adminProgram); + virObjectUnref(logSrv); + virObjectUnref(adminSrv); virLogDaemonFree(logDaemon); if (statuswrite != -1) { if (ret != 0) { @@ -1167,6 +1233,7 @@ int main(int argc, char **argv) { virPidFileReleasePath(pid_file, pid_file_fd); VIR_FREE(pid_file); VIR_FREE(sock_file); + VIR_FREE(admin_sock_file); VIR_FREE(state_file); VIR_FREE(run_dir); VIR_FREE(remote_config_file); diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index cf58e6230e..3226b2c484 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -73,6 +73,7 @@ virLogDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL; data->max_clients = 1024; + data->admin_max_clients = 5000; data->max_size = 1024 * 1024 * 2; data->max_backups = 3; @@ -103,6 +104,8 @@ virLogDaemonConfigLoadOptions(virLogDaemonConfigPtr data, return -1; if (virConfGetValueUInt(conf, "max_clients", &data->max_clients) < 0) return -1; + if (virConfGetValueUInt(conf, "admin_max_clients", &data->admin_max_clients) < 0) + return -1; if (virConfGetValueSizeT(conf, "max_size", &data->max_size) < 0) return -1; if (virConfGetValueSizeT(conf, "max_backups", &data->max_backups) < 0) diff --git a/src/logging/log_daemon_config.h b/src/logging/log_daemon_config.h index 72d77d5e6f..53101b0610 100644 --- a/src/logging/log_daemon_config.h +++ b/src/logging/log_daemon_config.h @@ -34,6 +34,7 @@ struct _virLogDaemonConfig { char *log_filters; char *log_outputs; unsigned int max_clients; + unsigned int admin_max_clients; size_t max_backups; size_t max_size; diff --git a/src/logging/test_virtlogd.aug.in b/src/logging/test_virtlogd.aug.in index 3e6888fd48..ee3fae5cde 100644 --- a/src/logging/test_virtlogd.aug.in +++ b/src/logging/test_virtlogd.aug.in @@ -2,6 +2,8 @@ module Test_virtlogd = let conf = "log_level = 3 log_filters=\"3:remote 4:event\" log_outputs=\"3:syslog:virtlogd\" +max_clients = 10 +admin_max_clients = 10 max_size = 131072 max_backups = 3 " @@ -10,5 +12,7 @@ max_backups = 3 { "log_level" = "3" } { "log_filters" = "3:remote 4:event" } { "log_outputs" = "3:syslog:virtlogd" } + { "max_clients" = "10" } + { "admin_max_clients" = "10" } { "max_size" = "131072" } { "max_backups" = "3" } diff --git a/src/logging/virtlogd-admin.socket.in b/src/logging/virtlogd-admin.socket.in new file mode 100644 index 0000000000..595e6c4c4b --- /dev/null +++ b/src/logging/virtlogd-admin.socket.in @@ -0,0 +1,10 @@ +[Unit] +Description=Virtual machine log manager socket +Before=libvirtd.service + +[Socket] +ListenStream=@localstatedir@/run/libvirt/virtlogd-admin-sock +Service=virtlogd.service + +[Install] +WantedBy=sockets.target diff --git a/src/logging/virtlogd.aug b/src/logging/virtlogd.aug index 5ed174230d..4fdcba72af 100644 --- a/src/logging/virtlogd.aug +++ b/src/logging/virtlogd.aug @@ -29,6 +29,7 @@ module Virtlogd = | str_entry "log_outputs" | int_entry "log_buffer_size" | int_entry "max_clients" + | int_entry "admin_max_clients" | int_entry "max_size" | int_entry "max_backups" diff --git a/src/logging/virtlogd.service.in b/src/logging/virtlogd.service.in index aa9aa698b5..3d9ae36150 100644 --- a/src/logging/virtlogd.service.in +++ b/src/logging/virtlogd.service.in @@ -1,6 +1,7 @@ [Unit] Description=Virtual machine log manager Requires=virtlogd.socket +Requires=virtlogd-admin.socket Before=libvirtd.service Documentation=man:virtlogd(8) Documentation=https://libvirt.org -- 2.14.3

On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
Add a virtlogd-admin-sock can serves the admin protocol for the virtlogd daemon and define a virtlogd:///{system,session} URI scheme for connecting to it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/libvirt-admin.c | 20 ++++-- src/logging/log_daemon.c | 135 ++++++++++++++++++++++++++--------- src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 ++ src/logging/virtlogd-admin.socket.in | 10 +++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + 9 files changed, 136 insertions(+), 40 deletions(-) create mode 100644 src/logging/virtlogd-admin.socket.in
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index cf58e6230e..3226b2c484 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -73,6 +73,7 @@ virLogDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL;
data->max_clients = 1024; + data->admin_max_clients = 5000; data->max_size = 1024 * 1024 * 2; data->max_backups = 3;
5000 seems like huge default. Perhaps 5 is enough? Or 10 so that we match the value from the aug test. Which brigns up interesting question - how come we don't need src/logging/virtlogd.conf change too? And also, how can it be that the aug test doesn't test virtlogd.conf but some crafted input? We should have something similar to qemu.conf test.
@@ -103,6 +104,8 @@ virLogDaemonConfigLoadOptions(virLogDaemonConfigPtr data, return -1; if (virConfGetValueUInt(conf, "max_clients", &data->max_clients) < 0) return -1; + if (virConfGetValueUInt(conf, "admin_max_clients", &data->admin_max_clients) < 0) + return -1; if (virConfGetValueSizeT(conf, "max_size", &data->max_size) < 0) return -1; if (virConfGetValueSizeT(conf, "max_backups", &data->max_backups) < 0) diff --git a/src/logging/log_daemon_config.h b/src/logging/log_daemon_config.h index 72d77d5e6f..53101b0610 100644 --- a/src/logging/log_daemon_config.h +++ b/src/logging/log_daemon_config.h @@ -34,6 +34,7 @@ struct _virLogDaemonConfig { char *log_filters; char *log_outputs; unsigned int max_clients; + unsigned int admin_max_clients;
size_t max_backups; size_t max_size; diff --git a/src/logging/test_virtlogd.aug.in b/src/logging/test_virtlogd.aug.in index 3e6888fd48..ee3fae5cde 100644 --- a/src/logging/test_virtlogd.aug.in +++ b/src/logging/test_virtlogd.aug.in @@ -2,6 +2,8 @@ module Test_virtlogd = let conf = "log_level = 3 log_filters=\"3:remote 4:event\" log_outputs=\"3:syslog:virtlogd\" +max_clients = 10 +admin_max_clients = 10 max_size = 131072 max_backups = 3 " @@ -10,5 +12,7 @@ max_backups = 3 { "log_level" = "3" } { "log_filters" = "3:remote 4:event" } { "log_outputs" = "3:syslog:virtlogd" } + { "max_clients" = "10" } + { "admin_max_clients" = "10" }
Expand the TABs please. Michal

On Mon, Jan 22, 2018 at 04:44:13PM +0100, Michal Privoznik wrote:
On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
Add a virtlogd-admin-sock can serves the admin protocol for the virtlogd daemon and define a virtlogd:///{system,session} URI scheme for connecting to it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 1 + src/libvirt-admin.c | 20 ++++-- src/logging/log_daemon.c | 135 ++++++++++++++++++++++++++--------- src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 ++ src/logging/virtlogd-admin.socket.in | 10 +++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + 9 files changed, 136 insertions(+), 40 deletions(-) create mode 100644 src/logging/virtlogd-admin.socket.in
diff --git a/src/logging/log_daemon_config.c b/src/logging/log_daemon_config.c index cf58e6230e..3226b2c484 100644 --- a/src/logging/log_daemon_config.c +++ b/src/logging/log_daemon_config.c @@ -73,6 +73,7 @@ virLogDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL;
data->max_clients = 1024; + data->admin_max_clients = 5000; data->max_size = 1024 * 1024 * 2; data->max_backups = 3;
5000 seems like huge default. Perhaps 5 is enough? Or 10 so that we match the value from the aug test. Which brigns up interesting question - how come we don't need src/logging/virtlogd.conf change too? And also, how can it be that the aug test doesn't test virtlogd.conf but some crafted input? We should have something similar to qemu.conf test.
5000 was blindly copied from libvirtd - it is odd that it doesn't match the default listed in the ocnfig file there too. I'll see about fixing the aug test to work like libvirtd's test does. BTW, other flaw in this patch is that I've broken exec-restarts.... Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|

Add a virtlockd-admin-sock can serves the admin protocol for the virtlockd daemon and define a virtlockd:///{system,session} URI scheme for connecting to it. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- cfg.mk | 1 - src/Makefile.am | 1 + src/libvirt-admin.c | 3 + src/locking/lock_daemon.c | 132 +++++++++++++++++++++++++--------- src/locking/lock_daemon_config.c | 3 + src/locking/lock_daemon_config.h | 1 + src/locking/test_virtlockd.aug.in | 4 ++ src/locking/virtlockd-admin.socket.in | 10 +++ src/locking/virtlockd.aug | 1 + src/locking/virtlockd.conf | 6 ++ src/locking/virtlockd.service.in | 1 + 11 files changed, 129 insertions(+), 34 deletions(-) create mode 100644 src/locking/virtlockd-admin.socket.in diff --git a/cfg.mk b/cfg.mk index 4635f9f771..1d0a885c1d 100644 --- a/cfg.mk +++ b/cfg.mk @@ -774,7 +774,6 @@ sc_prohibit_cross_inclusion: case $$dir in \ util/) safe="util";; \ access/ | conf/) safe="($$dir|conf|util)";; \ - locking/) safe="($$dir|util|conf|rpc)";; \ cpu/| network/| node_device/| rpc/| security/| storage/) \ safe="($$dir|util|conf|storage)";; \ xenapi/ | xenconfig/ ) safe="($$dir|util|conf|xen|cpu)";; \ diff --git a/src/Makefile.am b/src/Makefile.am index 828306fd35..42eee2ad90 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2646,6 +2646,7 @@ virtlockd_LDFLAGS = \ $(PIE_LDFLAGS) \ $(NULL) virtlockd_LDADD = \ + libvirt_driver_admin.la \ libvirt-net-rpc-server.la \ libvirt-net-rpc.la \ libvirt_util.la \ diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c index 9d1bff536b..de595a9f7f 100644 --- a/src/libvirt-admin.c +++ b/src/libvirt-admin.c @@ -40,6 +40,7 @@ #define LIBVIRTD_ADMIN_SOCK_NAME "libvirt-admin-sock" #define VIRTLOGD_ADMIN_SOCK_NAME "virtlogd-admin-sock" +#define VIRTLOCKD_ADMIN_SOCK_NAME "virtlockd-admin-sock" VIR_LOG_INIT("libvirt-admin"); @@ -134,6 +135,8 @@ getSocketPath(virURIPtr uri) sockbase = LIBVIRTD_ADMIN_SOCK_NAME; } else if (STREQ_NULLABLE(uri->scheme, "virtlogd")) { sockbase = VIRTLOGD_ADMIN_SOCK_NAME; + } else if (STREQ_NULLABLE(uri->scheme, "virtlockd")) { + sockbase = VIRTLOCKD_ADMIN_SOCK_NAME; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported URI scheme '%s'"), diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 6751b57bc5..d87118b546 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -32,6 +32,7 @@ #include "lock_daemon.h" #include "lock_daemon_config.h" +#include "admin/admin_server_dispatch.h" #include "virutil.h" #include "virfile.h" #include "virpidfile.h" @@ -148,7 +149,7 @@ static virLockDaemonPtr virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) { virLockDaemonPtr lockd; - virNetServerPtr srv; + virNetServerPtr srv = NULL; if (VIR_ALLOC(lockd) < 0) return NULL; @@ -160,6 +161,9 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) return NULL; } + if (!(lockd->dmn = virNetDaemonNew())) + goto error; + if (!(srv = virNetServerNew("virtlockd", 1, 1, 1, 0, config->max_clients, config->max_clients, -1, 0, @@ -170,9 +174,23 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; - if (!(lockd->dmn = virNetDaemonNew()) || - virNetDaemonAddServer(lockd->dmn, srv) < 0) + if (virNetDaemonAddServer(lockd->dmn, srv) < 0) + goto error; + virObjectUnref(srv); + srv = NULL; + + if (!(srv = virNetServerNew("admin", 1, + 1, 1, 0, config->admin_max_clients, + config->admin_max_clients, -1, 0, + NULL, + remoteAdmClientInitHook, + NULL, + remoteAdmClientFreeFunc, + lockd->dmn))) goto error; + + if (virNetDaemonAddServer(lockd->dmn, srv) < 0) + goto error; virObjectUnref(srv); srv = NULL; @@ -403,10 +421,12 @@ virLockDaemonForkIntoBackground(const char *argv0) static int virLockDaemonUnixSocketPaths(bool privileged, - char **sockfile) + char **sockfile, + char **adminSockfile) { if (privileged) { - if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/virtlockd-sock") < 0) + if (VIR_STRDUP(*sockfile, LOCALSTATEDIR "/run/libvirt/virtlockd-sock") < 0 || + VIR_STRDUP(*adminSockfile, LOCALSTATEDIR "/run/libvirt/virtlockd-admin-sock") < 0) goto error; } else { char *rundir = NULL; @@ -423,7 +443,8 @@ virLockDaemonUnixSocketPaths(bool privileged, } umask(old_umask); - if (virAsprintf(sockfile, "%s/virtlockd-sock", rundir) < 0) { + if (virAsprintf(sockfile, "%s/virtlockd-sock", rundir) < 0 || + virAsprintf(adminSockfile, "%s/virtlockd-admin-sock", rundir) < 0) { VIR_FREE(rundir); goto error; } @@ -540,29 +561,50 @@ virLockDaemonSetupSignals(virNetDaemonPtr dmn) static int -virLockDaemonSetupNetworkingSystemD(virNetServerPtr srv) +virLockDaemonSetupNetworkingSystemD(virNetServerPtr lockSrv, virNetServerPtr adminSrv) { - virNetServerServicePtr svc; unsigned int nfds; + size_t i; if ((nfds = virGetListenFDs()) == 0) return 0; - if (nfds > 1) + if (nfds > 2) VIR_DEBUG("Too many (%d) file descriptors from systemd", nfds); - nfds = 1; - /* Systemd passes FDs, starting immediately after stderr, - * so the first FD we'll get is '3'. */ - if (!(svc = virNetServerServiceNewFD(3, 0, + for (i = 0; i < nfds && i < 2; i++) { + virNetServerServicePtr svc; + char *path = virGetUNIXSocketPath(3 + i); + virNetServerPtr srv; + + if (!path) + return -1; + + if (strstr(path, "virtlockd-admin-sock")) { + srv = adminSrv; + } else if (strstr(path, "virtlockd-sock")) { + srv = lockSrv; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unknown UNIX socket %s passed in"), + path); + VIR_FREE(path); + return -1; + } + VIR_FREE(path); + + /* Systemd passes FDs, starting immediately after stderr, + * so the first FD we'll get is '3'. */ + if (!(svc = virNetServerServiceNewFD(3 + i, 0, #if WITH_GNUTLS - NULL, + NULL, #endif - false, 0, 1))) - return -1; + false, 0, 1))) + return -1; - if (virNetServerAddService(srv, svc, NULL) < 0) { - virObjectUnref(svc); - return -1; + if (virNetServerAddService(srv, svc, NULL) < 0) { + virObjectUnref(svc); + return -1; + } } return 1; } @@ -1095,8 +1137,10 @@ virLockDaemonUsage(const char *argv0, bool privileged) } int main(int argc, char **argv) { - virNetServerPtr srv = NULL; + virNetServerPtr lockSrv = NULL; + virNetServerPtr adminSrv = NULL; virNetServerProgramPtr lockProgram = NULL; + virNetServerProgramPtr adminProgram = NULL; char *remote_config_file = NULL; int statuswrite = -1; int ret = 1; @@ -1106,6 +1150,7 @@ int main(int argc, char **argv) { char *pid_file = NULL; int pid_file_fd = -1; char *sock_file = NULL; + char *admin_sock_file = NULL; int timeout = -1; /* -t: Shutdown timeout */ char *state_file = NULL; bool implicit_conf = false; @@ -1233,12 +1278,13 @@ int main(int argc, char **argv) { VIR_DEBUG("Decided on pid file path '%s'", NULLSTR(pid_file)); if (virLockDaemonUnixSocketPaths(privileged, - &sock_file) < 0) { + &sock_file, + &admin_sock_file) < 0) { VIR_ERROR(_("Can't determine socket paths")); exit(EXIT_FAILURE); } - VIR_DEBUG("Decided on socket paths '%s'", - sock_file); + VIR_DEBUG("Decided on socket paths '%s' and '%s'", + sock_file, admin_sock_file); if (virLockDaemonExecRestartStatePath(privileged, &state_file) < 0) { @@ -1313,22 +1359,27 @@ int main(int argc, char **argv) { goto cleanup; } - srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); - if ((rv = virLockDaemonSetupNetworkingSystemD(srv)) < 0) { + lockSrv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); + adminSrv = virNetDaemonGetServer(lockDaemon->dmn, "admin"); + if ((rv = virLockDaemonSetupNetworkingSystemD(lockSrv, adminSrv)) < 0) { ret = VIR_LOCK_DAEMON_ERR_NETWORK; goto cleanup; } /* Only do this, if systemd did not pass a FD */ - if (rv == 0 && - virLockDaemonSetupNetworkingNative(srv, sock_file) < 0) { - ret = VIR_LOCK_DAEMON_ERR_NETWORK; - goto cleanup; + if (rv == 0) { + if (virLockDaemonSetupNetworkingNative(lockSrv, sock_file) < 0 || + virLockDaemonSetupNetworkingNative(adminSrv, admin_sock_file) < 0) { + ret = VIR_LOCK_DAEMON_ERR_NETWORK; + goto cleanup; + } } - virObjectUnref(srv); + virObjectUnref(lockSrv); + virObjectUnref(adminSrv); } - srv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); + lockSrv = virNetDaemonGetServer(lockDaemon->dmn, "virtlockd"); + adminSrv = virNetDaemonGetServer(lockDaemon->dmn, "admin"); if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); @@ -1349,7 +1400,19 @@ int main(int argc, char **argv) { goto cleanup; } - if (virNetServerAddProgram(srv, lockProgram) < 0) { + if (virNetServerAddProgram(lockSrv, lockProgram) < 0) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + + if (!(adminProgram = virNetServerProgramNew(ADMIN_PROGRAM, + ADMIN_PROTOCOL_VERSION, + adminProcs, + adminNProcs))) { + ret = VIR_LOCK_DAEMON_ERR_INIT; + goto cleanup; + } + if (virNetServerAddProgram(adminSrv, adminProgram) < 0) { ret = VIR_LOCK_DAEMON_ERR_INIT; goto cleanup; } @@ -1384,8 +1447,10 @@ int main(int argc, char **argv) { ret = 0; cleanup: - virObjectUnref(srv); virObjectUnref(lockProgram); + virObjectUnref(adminProgram); + virObjectUnref(lockSrv); + virObjectUnref(adminSrv); virLockDaemonFree(lockDaemon); if (statuswrite != -1) { if (ret != 0) { @@ -1401,6 +1466,7 @@ int main(int argc, char **argv) { virPidFileReleasePath(pid_file, pid_file_fd); VIR_FREE(pid_file); VIR_FREE(sock_file); + VIR_FREE(admin_sock_file); VIR_FREE(state_file); VIR_FREE(run_dir); return ret; diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c index 20824b870c..efa5655a30 100644 --- a/src/locking/lock_daemon_config.c +++ b/src/locking/lock_daemon_config.c @@ -72,6 +72,7 @@ virLockDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL; data->max_clients = 1024; + data->admin_max_clients = 5000; return data; } @@ -100,6 +101,8 @@ virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data, return -1; if (virConfGetValueUInt(conf, "max_clients", &data->max_clients) < 0) return -1; + if (virConfGetValueUInt(conf, "admin_max_clients", &data->admin_max_clients) < 0) + return -1; return 0; } diff --git a/src/locking/lock_daemon_config.h b/src/locking/lock_daemon_config.h index 6ab84c6a0a..3e642208f5 100644 --- a/src/locking/lock_daemon_config.h +++ b/src/locking/lock_daemon_config.h @@ -34,6 +34,7 @@ struct _virLockDaemonConfig { char *log_filters; char *log_outputs; unsigned int max_clients; + unsigned int admin_max_clients; }; diff --git a/src/locking/test_virtlockd.aug.in b/src/locking/test_virtlockd.aug.in index 799818e5d1..2d69816b5c 100644 --- a/src/locking/test_virtlockd.aug.in +++ b/src/locking/test_virtlockd.aug.in @@ -3,6 +3,8 @@ module Test_virtlockd = log_filters=\"3:remote 4:event\" log_outputs=\"3:syslog:libvirtd\" log_buffer_size = 64 +max_clients = 10 +admin_max_clients = 10 " test Virtlockd.lns get conf = @@ -10,3 +12,5 @@ log_buffer_size = 64 { "log_filters" = "3:remote 4:event" } { "log_outputs" = "3:syslog:libvirtd" } { "log_buffer_size" = "64" } + { "max_clients" = "10" } + { "admin_max_clients" = "10" } diff --git a/src/locking/virtlockd-admin.socket.in b/src/locking/virtlockd-admin.socket.in new file mode 100644 index 0000000000..1fa0a3dc33 --- /dev/null +++ b/src/locking/virtlockd-admin.socket.in @@ -0,0 +1,10 @@ +[Unit] +Description=Virtual machine lock manager admin socket +Before=libvirtd.service + +[Socket] +ListenStream=@localstatedir@/run/libvirt/virtlockd-admin-sock +Server=virtlockd.service + +[Install] +WantedBy=sockets.target diff --git a/src/locking/virtlockd.aug b/src/locking/virtlockd.aug index ec8d2b5c0a..7b152ed407 100644 --- a/src/locking/virtlockd.aug +++ b/src/locking/virtlockd.aug @@ -29,6 +29,7 @@ module Virtlockd = | str_entry "log_outputs" | int_entry "log_buffer_size" | int_entry "max_clients" + | int_entry "admin_max_clients" (* Each enty in the config is one of the following three ... *) let entry = logging_entry diff --git a/src/locking/virtlockd.conf b/src/locking/virtlockd.conf index 4c935d0a2c..24b69aa425 100644 --- a/src/locking/virtlockd.conf +++ b/src/locking/virtlockd.conf @@ -65,3 +65,9 @@ # to virtlockd. So 'max_clients' will affect how many VMs can # be run on a host #max_clients = 1024 + +# Same processing controls, but this time for the admin interface. +# For description of each option, be so kind to scroll few lines +# upwards. + +#admin_max_clients = 5 diff --git a/src/locking/virtlockd.service.in b/src/locking/virtlockd.service.in index 07e48478b7..3c9d587032 100644 --- a/src/locking/virtlockd.service.in +++ b/src/locking/virtlockd.service.in @@ -1,6 +1,7 @@ [Unit] Description=Virtual machine lock manager Requires=virtlockd.socket +Requires=virtlockd-admin.socket Before=libvirtd.service Documentation=man:virtlockd(8) Documentation=https://libvirt.org -- 2.14.3

On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
Add a virtlockd-admin-sock can serves the admin protocol for the virtlockd daemon and define a virtlockd:///{system,session} URI scheme for connecting to it.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- cfg.mk | 1 - src/Makefile.am | 1 + src/libvirt-admin.c | 3 + src/locking/lock_daemon.c | 132 +++++++++++++++++++++++++--------- src/locking/lock_daemon_config.c | 3 + src/locking/lock_daemon_config.h | 1 + src/locking/test_virtlockd.aug.in | 4 ++ src/locking/virtlockd-admin.socket.in | 10 +++ src/locking/virtlockd.aug | 1 + src/locking/virtlockd.conf | 6 ++ src/locking/virtlockd.service.in | 1 + 11 files changed, 129 insertions(+), 34 deletions(-) create mode 100644 src/locking/virtlockd-admin.socket.in
diff --git a/src/locking/lock_daemon_config.c b/src/locking/lock_daemon_config.c index 20824b870c..efa5655a30 100644 --- a/src/locking/lock_daemon_config.c +++ b/src/locking/lock_daemon_config.c @@ -72,6 +72,7 @@ virLockDaemonConfigNew(bool privileged ATTRIBUTE_UNUSED) return NULL;
data->max_clients = 1024; + data->admin_max_clients = 5000;
Again, looks like too much. 5 would match value in the config file.
return data; } @@ -100,6 +101,8 @@ virLockDaemonConfigLoadOptions(virLockDaemonConfigPtr data, return -1; if (virConfGetValueUInt(conf, "max_clients", &data->max_clients) < 0) return -1; + if (virConfGetValueUInt(conf, "admin_max_clients", &data->admin_max_clients) < 0) + return -1;
return 0; } diff --git a/src/locking/lock_daemon_config.h b/src/locking/lock_daemon_config.h index 6ab84c6a0a..3e642208f5 100644 --- a/src/locking/lock_daemon_config.h +++ b/src/locking/lock_daemon_config.h @@ -34,6 +34,7 @@ struct _virLockDaemonConfig { char *log_filters; char *log_outputs; unsigned int max_clients; + unsigned int admin_max_clients; };
diff --git a/src/locking/test_virtlockd.aug.in b/src/locking/test_virtlockd.aug.in index 799818e5d1..2d69816b5c 100644 --- a/src/locking/test_virtlockd.aug.in +++ b/src/locking/test_virtlockd.aug.in @@ -3,6 +3,8 @@ module Test_virtlockd = log_filters=\"3:remote 4:event\" log_outputs=\"3:syslog:libvirtd\" log_buffer_size = 64 +max_clients = 10 +admin_max_clients = 10 "
test Virtlockd.lns get conf = @@ -10,3 +12,5 @@ log_buffer_size = 64 { "log_filters" = "3:remote 4:event" } { "log_outputs" = "3:syslog:libvirtd" } { "log_buffer_size" = "64" } + { "max_clients" = "10" } + { "admin_max_clients" = "10" }
Please expand the TABs. Michal

On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
The initial admin protocol support was only integrated into libvirtd. This series extracts that code so that it is reusable with all the daemons we have (and more than we'll get).
Daniel P. Berrange (4): admin: move admins server impl/dispatch into src/admin directory util: add virGetUNIXSocketPath helper logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
.gitignore | 1 + cfg.mk | 3 +- daemon/Makefile.am | 33 +---- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 -- po/POTFILES.in | 4 +- src/Makefile.am | 33 ++++- {daemon => src/admin}/admin_server.c | 4 +- {daemon => src/admin}/admin_server.h | 6 +- .../admin.c => src/admin/admin_server_dispatch.c | 21 ++-- .../admin.h => src/admin/admin_server_dispatch.h | 9 +- src/libvirt-admin.c | 23 +++- src/locking/lock_daemon.c | 132 +++++++++++++++----- src/locking/lock_daemon_config.c | 3 + src/locking/lock_daemon_config.h | 1 + src/locking/test_virtlockd.aug.in | 4 + src/locking/virtlockd-admin.socket.in | 10 ++ src/locking/virtlockd.aug | 1 + src/locking/virtlockd.conf | 6 + src/locking/virtlockd.service.in | 1 + src/logging/log_daemon.c | 135 +++++++++++++++------ src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 + src/logging/virtlogd-admin.socket.in | 10 ++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + src/util/virutil.c | 45 +++++++ src/util/virutil.h | 1 + 29 files changed, 371 insertions(+), 137 deletions(-) rename {daemon => src/admin}/admin_server.c (99%) rename {daemon => src/admin}/admin_server.h (96%) rename daemon/admin.c => src/admin/admin_server_dispatch.c (96%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (83%) create mode 100644 src/locking/virtlockd-admin.socket.in create mode 100644 src/logging/virtlogd-admin.socket.in
ACK if you fix all the nits I've found. Michal

On Mon, Jan 22, 2018 at 04:44:10PM +0100, Michal Privoznik wrote:
On 01/19/2018 06:09 PM, Daniel P. Berrange wrote:
The initial admin protocol support was only integrated into libvirtd. This series extracts that code so that it is reusable with all the daemons we have (and more than we'll get).
Daniel P. Berrange (4): admin: move admins server impl/dispatch into src/admin directory util: add virGetUNIXSocketPath helper logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
.gitignore | 1 + cfg.mk | 3 +- daemon/Makefile.am | 33 +---- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 -- po/POTFILES.in | 4 +- src/Makefile.am | 33 ++++- {daemon => src/admin}/admin_server.c | 4 +- {daemon => src/admin}/admin_server.h | 6 +- .../admin.c => src/admin/admin_server_dispatch.c | 21 ++-- .../admin.h => src/admin/admin_server_dispatch.h | 9 +- src/libvirt-admin.c | 23 +++- src/locking/lock_daemon.c | 132 +++++++++++++++----- src/locking/lock_daemon_config.c | 3 + src/locking/lock_daemon_config.h | 1 + src/locking/test_virtlockd.aug.in | 4 + src/locking/virtlockd-admin.socket.in | 10 ++ src/locking/virtlockd.aug | 1 + src/locking/virtlockd.conf | 6 + src/locking/virtlockd.service.in | 1 + src/logging/log_daemon.c | 135 +++++++++++++++------ src/logging/log_daemon_config.c | 3 + src/logging/log_daemon_config.h | 1 + src/logging/test_virtlogd.aug.in | 4 + src/logging/virtlogd-admin.socket.in | 10 ++ src/logging/virtlogd.aug | 1 + src/logging/virtlogd.service.in | 1 + src/util/virutil.c | 45 +++++++ src/util/virutil.h | 1 + 29 files changed, 371 insertions(+), 137 deletions(-) rename {daemon => src/admin}/admin_server.c (99%) rename {daemon => src/admin}/admin_server.h (96%) rename daemon/admin.c => src/admin/admin_server_dispatch.c (96%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (83%) create mode 100644 src/locking/virtlockd-admin.socket.in create mode 100644 src/logging/virtlogd-admin.socket.in
ACK if you fix all the nits I've found.
I'm going to repost due to the fact that exec-restart is fubar in this version. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
participants (2)
-
Daniel P. Berrange
-
Michal Privoznik