[libvirt] [PATCH v2 00/11] 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). In v2: - Fixed completely broken post-exec restart support for admin servers - Misc fixes from v1 review Daniel P. Berrange (11): admin: move admins server impl/dispatch into src/admin directory libvirtd: rename virNetServerClient callback impls to match type names admin: add support for post-exec restart callbacks util: add virGetUNIXSocketPath helper rpc: clarify "void *" values passed to client callbacks rpc: pass virNetServer to post-exec restart callback in typesafe manner rpc: annotate various parameters as being required to be non-NULL rpc: add method for checking if a named server exists rpc: refactor virNetServer setup for post-exec restarts logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd .gitignore | 1 + cfg.mk | 10 +- daemon/Makefile.am | 33 +--- daemon/libvirtd.c | 10 +- daemon/libvirtd.h | 10 -- daemon/remote.c | 6 +- daemon/remote.h | 6 +- po/POTFILES.in | 6 +- 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 | 48 ++++-- .../admin.h => src/admin/admin_server_dispatch.h | 18 +- src/libvirt-admin.c | 23 ++- src/libvirt_private.syms | 1 + src/libvirt_remote.syms | 2 +- src/locking/lock_daemon.c | 186 +++++++++++++++----- 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 | 188 ++++++++++++++++----- 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/rpc/virnetdaemon.c | 164 ++++++++++-------- src/rpc/virnetdaemon.h | 20 ++- src/rpc/virnetserver.c | 6 +- src/rpc/virnetserver.h | 7 +- src/rpc/virnetserverclient.c | 14 +- src/rpc/virnetserverclient.h | 23 ++- src/rpc/virnetserverprogram.h | 3 - src/util/virutil.c | 45 +++++ src/util/virutil.h | 1 + tests/virnetdaemontest.c | 37 +++- 41 files changed, 670 insertions(+), 287 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 (92%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (59%) 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 | 9 +++--- daemon/Makefile.am | 33 +--------------------- daemon/libvirtd.c | 2 +- daemon/libvirtd.h | 10 ------- po/POTFILES.in | 6 ++-- 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, 65 insertions(+), 67 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..1a5de2b154 100644 --- a/cfg.mk +++ b/cfg.mk @@ -33,6 +33,7 @@ gnulib_dir = $(srcdir)/.gnulib # This is all gnulib files, as well as generated files for RPC code. generated_files = \ $(srcdir)/daemon/*_dispatch.h \ + $(srcdir)/src/*/*_dispatch_stubs.h \ $(srcdir)/src/*/*_dispatch.h \ $(srcdir)/src/remote/*_client_bodies.h \ $(srcdir)/src/*/*_protocol.[ch] \ @@ -768,7 +769,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 \ @@ -1119,7 +1120,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 +1128,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 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..daf9a78a1d 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -1,6 +1,3 @@ -daemon/admin.c -daemon/admin_dispatch.h -daemon/admin_server.c daemon/libvirtd-config.c daemon/libvirtd.c daemon/qemu_dispatch.h @@ -12,6 +9,9 @@ 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/admin/admin_server_dispatch_stubs.h 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
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- daemon/libvirtd.c | 8 ++++---- daemon/remote.c | 6 +++--- daemon/remote.h | 6 +++--- src/admin/admin_server_dispatch.c | 6 +++--- src/admin/admin_server_dispatch.h | 4 ++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 48bdc27a5f..c5bb12b8ce 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -1311,9 +1311,9 @@ int main(int argc, char **argv) { config->keepalive_interval, config->keepalive_count, config->mdns_adv ? config->mdns_name : NULL, - remoteClientInitHook, + remoteClientNew, NULL, - remoteClientFreeFunc, + remoteClientFree, NULL))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; @@ -1385,9 +1385,9 @@ int main(int argc, char **argv) { config->admin_keepalive_interval, config->admin_keepalive_count, NULL, - remoteAdmClientInitHook, + remoteAdmClientNew, NULL, - remoteAdmClientFreeFunc, + remoteAdmClientFree, dmn))) { ret = VIR_DAEMON_ERR_INIT; goto cleanup; diff --git a/daemon/remote.c b/daemon/remote.c index b6fe6d8539..6de4bd00d4 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1734,7 +1734,7 @@ remoteClientFreePrivateCallbacks(struct daemonClientPrivate *priv) * We keep the libvirt connection open until any async * jobs have finished, then clean it up elsewhere */ -void remoteClientFreeFunc(void *data) +void remoteClientFree(void *data) { struct daemonClientPrivate *priv = data; @@ -1757,8 +1757,8 @@ static void remoteClientCloseFunc(virNetServerClientPtr client) } -void *remoteClientInitHook(virNetServerClientPtr client, - void *opaque ATTRIBUTE_UNUSED) +void *remoteClientNew(virNetServerClientPtr client, + void *opaque ATTRIBUTE_UNUSED) { struct daemonClientPrivate *priv; diff --git a/daemon/remote.h b/daemon/remote.h index d42a19e92e..c1bce9ba2e 100644 --- a/daemon/remote.h +++ b/daemon/remote.h @@ -38,8 +38,8 @@ extern size_t lxcNProcs; extern virNetServerProgramProc qemuProcs[]; extern size_t qemuNProcs; -void remoteClientFreeFunc(void *data); -void *remoteClientInitHook(virNetServerClientPtr client, - void *opaque); +void remoteClientFree(void *data); +void *remoteClientNew(virNetServerClientPtr client, + void *opaque); #endif /* __LIBVIRTD_REMOTE_H__ */ diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index 068358b049..9f110025a2 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -52,7 +52,7 @@ struct daemonAdmClientPrivate { }; void -remoteAdmClientFreeFunc(void *data) +remoteAdmClientFree(void *data) { struct daemonAdmClientPrivate *priv = data; @@ -62,8 +62,8 @@ remoteAdmClientFreeFunc(void *data) } void * -remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED, - void *opaque) +remoteAdmClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *opaque) { struct daemonAdmClientPrivate *priv; diff --git a/src/admin/admin_server_dispatch.h b/src/admin/admin_server_dispatch.h index 01723e5c43..ff6cfcd607 100644 --- a/src/admin/admin_server_dispatch.h +++ b/src/admin/admin_server_dispatch.h @@ -31,7 +31,7 @@ extern virNetServerProgramProc adminProcs[]; extern size_t adminNProcs; -void remoteAdmClientFreeFunc(void *data); -void *remoteAdmClientInitHook(virNetServerClientPtr client, void *opaque); +void remoteAdmClientFree(void *data); +void *remoteAdmClientNew(virNetServerClientPtr client, void *opaque); #endif /* __ADMIN_SERVER_DISPATCH_H__ */ -- 2.14.3
We don't have any per-client private data we need to persist, but the RPC infrastructure requires that we provide the callbacks and serialize an empty JSON object. This makes us future proof going forwards. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/admin/admin_server_dispatch.c | 21 +++++++++++++++++++++ src/admin/admin_server_dispatch.h | 5 +++++ 2 files changed, 26 insertions(+) diff --git a/src/admin/admin_server_dispatch.c b/src/admin/admin_server_dispatch.c index 9f110025a2..b78ff902c0 100644 --- a/src/admin/admin_server_dispatch.c +++ b/src/admin/admin_server_dispatch.c @@ -86,6 +86,27 @@ remoteAdmClientNew(virNetServerClientPtr client ATTRIBUTE_UNUSED, return priv; } +void *remoteAdmClientNewPostExecRestart(virNetServerClientPtr client, + virJSONValuePtr object ATTRIBUTE_UNUSED, + void *opaque) +{ + return remoteAdmClientNew(client, opaque); +} + +virJSONValuePtr remoteAdmClientPreExecRestart(virNetServerClientPtr client ATTRIBUTE_UNUSED, + void *data ATTRIBUTE_UNUSED) +{ + virJSONValuePtr object = virJSONValueNewObject(); + + if (!object) + return NULL; + + /* No content to add at this time - just need empty object */ + + return object; +} + + /* Helpers */ static virNetServerPtr diff --git a/src/admin/admin_server_dispatch.h b/src/admin/admin_server_dispatch.h index ff6cfcd607..c998cf3cba 100644 --- a/src/admin/admin_server_dispatch.h +++ b/src/admin/admin_server_dispatch.h @@ -33,5 +33,10 @@ extern size_t adminNProcs; void remoteAdmClientFree(void *data); void *remoteAdmClientNew(virNetServerClientPtr client, void *opaque); +void *remoteAdmClientNewPostExecRestart(virNetServerClientPtr client, + virJSONValuePtr object, + void *opaque); +virJSONValuePtr remoteAdmClientPreExecRestart(virNetServerClientPtr client, + void *data); #endif /* __ADMIN_SERVER_DISPATCH_H__ */ -- 2.14.3
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/libvirt_private.syms | 1 + src/util/virutil.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virutil.h | 1 + 3 files changed, 47 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bc8cc1fba9..54e3b9130b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2962,6 +2962,7 @@ virGetListenFDs; virGetSelfLastChanged; virGetSystemPageSize; virGetSystemPageSizeKB; +virGetUNIXSocketPath; virGetUnprivSGIOSysfsPath; virGetUserCacheDirectory; virGetUserConfigDirectory; 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
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserverclient.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 0fa8745191..14ace9e522 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -41,11 +41,20 @@ typedef int (*virNetServerClientFilterFunc)(virNetServerClientPtr client, virNetMessagePtr msg, void *opaque); +/* + * @data: value allocated by virNetServerClintPrivNew(PostExecRestart) callback + */ typedef virJSONValuePtr (*virNetServerClientPrivPreExecRestart)(virNetServerClientPtr client, void *data); +/* + * @opaque: value of @privOpaque from virNetServerClientNewPostExecRestart + */ typedef void *(*virNetServerClientPrivNewPostExecRestart)(virNetServerClientPtr client, virJSONValuePtr object, void *opaque); +/* + * @opaque: value of @privOpaque from virNetServerClientNew + */ typedef void *(*virNetServerClientPrivNew)(virNetServerClientPtr client, void *opaque); -- 2.14.3
The virNetServer class is passing a pointer to itself to the virNetServerClient as a 'void *' pointer. This is presumably due to fact that the virnetserverclient.h file doesn't see the virNetServerPtr typedef. The typedef is easily movable though, which lets us get typesafe parameter passing, removing the confusion of passing two distinct 'void *' pointers to one method. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.c | 6 +++--- src/rpc/virnetserverclient.c | 14 +++++++------- src/rpc/virnetserverclient.h | 9 ++++++--- src/rpc/virnetserverprogram.h | 3 --- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index 77a4c0b8dc..5e50b58be8 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -540,12 +540,12 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, goto error; } - if (!(client = virNetServerClientNewPostExecRestart(child, + if (!(client = virNetServerClientNewPostExecRestart(srv, + child, clientPrivNewPostExecRestart, clientPrivPreExecRestart, clientPrivFree, - clientPrivOpaque, - srv))) + clientPrivOpaque))) goto error; if (virNetServerAddClient(srv, client) < 0) { diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 1f335d3b1e..d81a3892ff 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -479,12 +479,12 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, } -virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr object, +virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, + virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque, - void *opaque) + void *privOpaque) { virJSONValuePtr child; virNetServerClientPtr client = NULL; @@ -540,12 +540,12 @@ virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr objec if (!virJSONValueObjectHasKey(object, "id")) { /* no ID found in, a new one must be generated */ - id = virNetServerNextClientID((virNetServerPtr) opaque); + id = virNetServerNextClientID(srv); } else { if (virJSONValueObjectGetNumberUlong(object, "id", &id) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Malformed id field in JSON state document")); - return NULL; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Malformed id field in JSON state document")); + return NULL; } } diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 14ace9e522..3c48759abc 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -30,6 +30,9 @@ # include "virobject.h" # include "virjson.h" +typedef struct _virNetServer virNetServer; +typedef virNetServer *virNetServerPtr; + typedef struct _virNetServerClient virNetServerClient; typedef virNetServerClient *virNetServerClientPtr; @@ -71,12 +74,12 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virFreeCallback privFree, void *privOpaque); -virNetServerClientPtr virNetServerClientNewPostExecRestart(virJSONValuePtr object, +virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, + virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque, - void *opaque); + void *privOpaque); virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client); diff --git a/src/rpc/virnetserverprogram.h b/src/rpc/virnetserverprogram.h index 1731c9e1d1..c2a5635c95 100644 --- a/src/rpc/virnetserverprogram.h +++ b/src/rpc/virnetserverprogram.h @@ -31,9 +31,6 @@ typedef struct _virNetDaemon virNetDaemon; typedef virNetDaemon *virNetDaemonPtr; -typedef struct _virNetServer virNetServer; -typedef virNetServer *virNetServerPtr; - typedef struct _virNetServerService virNetServerService; typedef virNetServerService *virNetServerServicePtr; -- 2.14.3
The server name and client data callbacks need to be non-NULL or the system will crash at various times. This is particularly bad when some of the crashes only occur post-exec restart. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.h | 7 +++++-- src/rpc/virnetserverclient.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/rpc/virnetserver.h b/src/rpc/virnetserver.h index 7728a67f5f..a79c39fdb2 100644 --- a/src/rpc/virnetserver.h +++ b/src/rpc/virnetserver.h @@ -47,7 +47,8 @@ virNetServerPtr virNetServerNew(const char *name, virNetServerClientPrivNew clientPrivNew, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, - void *clientPrivOpaque); + void *clientPrivOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(11) ATTRIBUTE_NONNULL(13); virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, const char *name, @@ -55,7 +56,9 @@ virNetServerPtr virNetServerNewPostExecRestart(virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, virFreeCallback clientPrivFree, - void *clientPrivOpaque); + void *clientPrivOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5) ATTRIBUTE_NONNULL(6); void virNetServerClose(virNetServerPtr srv); diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3c48759abc..4a0d3cc25e 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivNew privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9); virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5); virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client); -- 2.14.3
On 01/23/2018 08:23 AM, Daniel P. Berrange wrote:
The server name and client data callbacks need to be non-NULL or the system will crash at various times. This is particularly bad when some of the crashes only occur post-exec restart.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.h | 7 +++++-- src/rpc/virnetserverclient.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-)
My Coverity build was not happy this morning...
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3c48759abc..4a0d3cc25e 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivNew privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9);
This one caused the Coverity build to fail because virNetServerClientNew checks "if (privNew)" before assigning client->privateData
virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Likewise here too for virNetServerClientNewPostExecRestart w/ privNew Finally, the "tests/virnetserverclienttest.c fails to build because of the NULL argument check. Reproducible if you enable static analysis... John
virJSONValuePtr virNetServerClientPreExecRestart(virNetServerClientPtr client);
On 02/01/2018 06:57 AM, John Ferlan wrote:
On 01/23/2018 08:23 AM, Daniel P. Berrange wrote:
The server name and client data callbacks need to be non-NULL or the system will crash at various times. This is particularly bad when some of the crashes only occur post-exec restart.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.h | 7 +++++-- src/rpc/virnetserverclient.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-)
My Coverity build was not happy this morning...
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3c48759abc..4a0d3cc25e 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivNew privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9);
This one caused the Coverity build to fail because virNetServerClientNew checks "if (privNew)" before assigning client->privateData
Also wouldn't the NONNULL's need to change based on "#ifdef WITH_GNUTLS" for arguments after #5?
virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Likewise here too for virNetServerClientNewPostExecRestart w/ privNew
Finally, the "tests/virnetserverclienttest.c fails to build because of the NULL argument check.
And virnetdaemontest.c... John
On Thu, Feb 01, 2018 at 07:14:26AM -0500, John Ferlan wrote:
On 02/01/2018 06:57 AM, John Ferlan wrote:
On 01/23/2018 08:23 AM, Daniel P. Berrange wrote:
The server name and client data callbacks need to be non-NULL or the system will crash at various times. This is particularly bad when some of the crashes only occur post-exec restart.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.h | 7 +++++-- src/rpc/virnetserverclient.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-)
My Coverity build was not happy this morning...
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3c48759abc..4a0d3cc25e 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivNew privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9);
This one caused the Coverity build to fail because virNetServerClientNew checks "if (privNew)" before assigning client->privateData
Also wouldn't the NONNULL's need to change based on "#ifdef WITH_GNUTLS" for arguments after #5?
Oh fun yes. 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 :|
On Thu, Feb 01, 2018 at 06:57:49AM -0500, John Ferlan wrote:
On 01/23/2018 08:23 AM, Daniel P. Berrange wrote:
The server name and client data callbacks need to be non-NULL or the system will crash at various times. This is particularly bad when some of the crashes only occur post-exec restart.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/rpc/virnetserver.h | 7 +++++-- src/rpc/virnetserverclient.h | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-)
My Coverity build was not happy this morning...
diff --git a/src/rpc/virnetserverclient.h b/src/rpc/virnetserverclient.h index 3c48759abc..4a0d3cc25e 100644 --- a/src/rpc/virnetserverclient.h +++ b/src/rpc/virnetserverclient.h @@ -72,14 +72,17 @@ virNetServerClientPtr virNetServerClientNew(unsigned long long id, virNetServerClientPrivNew privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(7) ATTRIBUTE_NONNULL(9);
This one caused the Coverity build to fail because virNetServerClientNew checks "if (privNew)" before assigning client->privateData
virNetServerClientPtr virNetServerClientNewPostExecRestart(virNetServerPtr srv, virJSONValuePtr object, virNetServerClientPrivNewPostExecRestart privNew, virNetServerClientPrivPreExecRestart privPreExecRestart, virFreeCallback privFree, - void *privOpaque); + void *privOpaque) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_NONNULL(4) ATTRIBUTE_NONNULL(5);
Likewise here too for virNetServerClientNewPostExecRestart w/ privNew
Fun, I'm trying to understand again how I caused the crash when these were NULL. I think it was because in the virNetServerClientNew() call they were non-NULL, but the virNetServerClientNewPostExecRestart() I had passed NULL, or vica-verca. Anyway since all production code callers pass non-NULL, I think these should be marked NONNULL regardless of crash possibility....
Finally, the "tests/virnetserverclienttest.c fails to build because of the NULL argument check.
...so I'll fix this test and remove the if (...) checks :-) 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 :|
It is not possible to blindly call virNetDaemonGetServer() because in a post-exec restart scenario, some servers may not exist and this method will pollute the error logs. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_remote.syms | 1 + src/rpc/virnetdaemon.c | 13 +++++++++++++ src/rpc/virnetdaemon.h | 2 ++ 3 files changed, 16 insertions(+) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index a181c4cf7f..fab6ab9dff 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -69,6 +69,7 @@ virNetDaemonClose; virNetDaemonGetServer; virNetDaemonGetServers; virNetDaemonHasClients; +virNetDaemonHasServer; virNetDaemonIsPrivileged; virNetDaemonNew; virNetDaemonNewPostExecRestart; diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 8c21414897..5d61a255c6 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -195,6 +195,19 @@ virNetDaemonGetServer(virNetDaemonPtr dmn, return srv; } +bool +virNetDaemonHasServer(virNetDaemonPtr dmn, + const char *serverName) +{ + void *ent; + + virObjectLock(dmn); + ent = virHashLookup(dmn->servers, serverName); + virObjectUnlock(dmn); + + return ent != NULL; +} + struct collectData { virNetServerPtr **servers; diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 211009c503..72c1df69b4 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -83,5 +83,7 @@ bool virNetDaemonHasClients(virNetDaemonPtr dmn); virNetServerPtr virNetDaemonGetServer(virNetDaemonPtr dmn, const char *serverName); ssize_t virNetDaemonGetServers(virNetDaemonPtr dmn, virNetServerPtr **servers); +bool virNetDaemonHasServer(virNetDaemonPtr dmn, + const char *serverName); #endif /* __VIR_NET_DAEMON_H__ */ -- 2.14.3
With the current code it is neccessary to call virNetDaemonNewPostExecRestart() and then for each server that needs restarting you are supposed to call virNetDaemonAddSeverPostExecRestart() This is fine if there's only ever one server, but as soon as you have two servers it is impossible to use this design. The code has no idea which servers were recorded in the JSON state doc, nor in which order the hash table serialized its keys. So this patch changes things so that we only call virNetDaemonNewPostExecRestart() passing in a callback, which is invoked once for each server found int he JSON state doc. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/libvirt_remote.syms | 1 - src/locking/lock_daemon.c | 41 +++++++++---- src/logging/log_daemon.c | 41 +++++++++---- src/rpc/virnetdaemon.c | 151 ++++++++++++++++++++++++---------------------- src/rpc/virnetdaemon.h | 18 +++--- tests/virnetdaemontest.c | 37 ++++++++++-- 6 files changed, 177 insertions(+), 112 deletions(-) diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index fab6ab9dff..b75cbb99b2 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -61,7 +61,6 @@ virNetClientStreamSetError; # rpc/virnetdaemon.h virNetDaemonAddServer; -virNetDaemonAddServerPostExec; virNetDaemonAddShutdownInhibition; virNetDaemonAddSignalHandler; virNetDaemonAutoShutdown; diff --git a/src/locking/lock_daemon.c b/src/locking/lock_daemon.c index 6751b57bc5..b1f0665aaa 100644 --- a/src/locking/lock_daemon.c +++ b/src/locking/lock_daemon.c @@ -192,15 +192,38 @@ virLockDaemonNew(virLockDaemonConfigPtr config, bool privileged) } +static virNetServerPtr +virLockDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + if (STREQ(name, "virtlockd")) { + return virNetServerNewPostExecRestart(object, + name, + virLockDaemonClientNew, + virLockDaemonClientNewPostExecRestart, + virLockDaemonClientPreExecRestart, + virLockDaemonClientFree, + opaque); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected server name '%s' during restart"), + name); + return NULL; + } +} + + static virLockDaemonPtr virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) { virLockDaemonPtr lockd; virJSONValuePtr child; virJSONValuePtr lockspaces; - virNetServerPtr srv; size_t i; ssize_t n; + const char *serverNames[] = { "virtlockd" }; if (VIR_ALLOC(lockd) < 0) return NULL; @@ -267,18 +290,12 @@ virLockDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged) } } - if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child))) - goto error; - - if (!(srv = virNetDaemonAddServerPostExec(lockd->dmn, - "virtlockd", - virLockDaemonClientNew, - virLockDaemonClientNewPostExecRestart, - virLockDaemonClientPreExecRestart, - virLockDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (!(lockd->dmn = virNetDaemonNewPostExecRestart(child, + ARRAY_CARDINALITY(serverNames), + serverNames, + virLockDaemonNewServerPostExecRestart, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; - virObjectUnref(srv); return lockd; diff --git a/src/logging/log_daemon.c b/src/logging/log_daemon.c index 7e8c9cfc29..33133af2af 100644 --- a/src/logging/log_daemon.c +++ b/src/logging/log_daemon.c @@ -188,13 +188,36 @@ virLogDaemonGetHandler(virLogDaemonPtr dmn) } +static virNetServerPtr +virLogDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + if (STREQ(name, "virtlogd")) { + return virNetServerNewPostExecRestart(object, + name, + virLogDaemonClientNew, + virLogDaemonClientNewPostExecRestart, + virLogDaemonClientPreExecRestart, + virLogDaemonClientFree, + opaque); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected server name '%s' during restart"), + name); + return NULL; + } +} + + static virLogDaemonPtr virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged, virLogDaemonConfigPtr config) { virLogDaemonPtr logd; - virNetServerPtr srv; virJSONValuePtr child; + const char *serverNames[] = { "virtlogd" }; if (VIR_ALLOC(logd) < 0) return NULL; @@ -212,18 +235,12 @@ virLogDaemonNewPostExecRestart(virJSONValuePtr object, bool privileged, goto error; } - if (!(logd->dmn = virNetDaemonNewPostExecRestart(child))) - goto error; - - if (!(srv = virNetDaemonAddServerPostExec(logd->dmn, - "virtlogd", - virLogDaemonClientNew, - virLogDaemonClientNewPostExecRestart, - virLogDaemonClientPreExecRestart, - virLogDaemonClientFree, - (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) + if (!(logd->dmn = virNetDaemonNewPostExecRestart(child, + ARRAY_CARDINALITY(serverNames), + serverNames, + virLogDaemonNewServerPostExecRestart, + (void*)(intptr_t)(privileged ? 0x1 : 0x0)))) goto error; - virObjectUnref(srv); if (!(child = virJSONValueObjectGet(object, "handler"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/rpc/virnetdaemon.c b/src/rpc/virnetdaemon.c index 5d61a255c6..6f00bfd9d1 100644 --- a/src/rpc/virnetdaemon.c +++ b/src/rpc/virnetdaemon.c @@ -262,85 +262,38 @@ virNetDaemonGetServers(virNetDaemonPtr dmn, } -virNetServerPtr -virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, - const char *serverName, - virNetServerClientPrivNew clientPrivNew, - virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, - virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, - virFreeCallback clientPrivFree, - void *clientPrivOpaque) -{ - virJSONValuePtr object = NULL; - virNetServerPtr srv = NULL; - - virObjectLock(dmn); - - if (!dmn->srvObject) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Cannot add more servers post-exec than " - "there were pre-exec")); - goto error; - } - - if (virJSONValueIsArray(dmn->srvObject)) { - object = virJSONValueArraySteal(dmn->srvObject, 0); - if (virJSONValueArraySize(dmn->srvObject) == 0) { - virJSONValueFree(dmn->srvObject); - dmn->srvObject = NULL; - } - } else if (virJSONValueObjectGetByType(dmn->srvObject, - "min_workers", - VIR_JSON_TYPE_NUMBER)) { - object = dmn->srvObject; - dmn->srvObject = NULL; - } else { - int ret = virJSONValueObjectRemoveKey(dmn->srvObject, - serverName, - &object); - if (ret != 1) { - if (ret == 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Server '%s' not found in JSON"), serverName); - } - goto error; - } - - if (virJSONValueObjectKeysNumber(dmn->srvObject) == 0) { - virJSONValueFree(dmn->srvObject); - dmn->srvObject = NULL; - } - } +struct virNetDaemonServerData { + virNetDaemonPtr dmn; + virNetDaemonNewServerPostExecRestart cb; + void *opaque; +}; - srv = virNetServerNewPostExecRestart(object, - serverName, - clientPrivNew, - clientPrivNewPostExecRestart, - clientPrivPreExecRestart, - clientPrivFree, - clientPrivOpaque); +static int +virNetDaemonServerIterator(const char *key, + virJSONValuePtr value, + void *opaque) +{ + struct virNetDaemonServerData *data = opaque; + virNetServerPtr srv; + VIR_DEBUG("Creating server '%s'", key); + srv = data->cb(data->dmn, key, value, data->opaque); if (!srv) - goto error; - - if (virHashAddEntry(dmn->servers, serverName, srv) < 0) - goto error; - virObjectRef(srv); + return -1; - virJSONValueFree(object); - virObjectUnlock(dmn); - return srv; + if (virHashAddEntry(data->dmn->servers, key, srv) < 0) + return -1; - error: - virObjectUnlock(dmn); - virObjectUnref(srv); - virJSONValueFree(object); - return NULL; + return 0; } virNetDaemonPtr -virNetDaemonNewPostExecRestart(virJSONValuePtr object) +virNetDaemonNewPostExecRestart(virJSONValuePtr object, + size_t nDefServerNames, + const char **defServerNames, + virNetDaemonNewServerPostExecRestart cb, + void *opaque) { virNetDaemonPtr dmn = NULL; virJSONValuePtr servers = virJSONValueObjectGet(object, "servers"); @@ -355,10 +308,64 @@ virNetDaemonNewPostExecRestart(virJSONValuePtr object) goto error; } - if (!(dmn->srvObject = virJSONValueCopy(new_version ? servers : object))) - goto error; + if (!new_version) { + virNetServerPtr srv; + + if (nDefServerNames < 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("No default server names provided")); + goto error; + } + + VIR_DEBUG("No 'servers' data, creating default '%s' name", defServerNames[0]); + + srv = cb(dmn, defServerNames[0], object, opaque); + + if (virHashAddEntry(dmn->servers, defServerNames[0], srv) < 0) + goto error; + } else if (virJSONValueIsArray(servers)) { + size_t i; + ssize_t n = virJSONValueArraySize(servers); + if (n < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Server count %zd should be positive"), n); + goto error; + } + if (n > nDefServerNames) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Server count %zd greater than default name count %zu"), + n, nDefServerNames); + goto error; + } + + for (i = 0; i < n; i++) { + virNetServerPtr srv; + virJSONValuePtr value = virJSONValueArrayGet(servers, i); + + VIR_DEBUG("Creating server '%s'", defServerNames[i]); + srv = cb(dmn, defServerNames[i], value, opaque); + if (!srv) + goto error; + + if (virHashAddEntry(dmn->servers, defServerNames[i], srv) < 0) { + virObjectUnref(srv); + goto error; + } + } + } else { + struct virNetDaemonServerData data = { + dmn, + cb, + opaque, + }; + if (virJSONValueObjectForeachKeyValue(servers, + virNetDaemonServerIterator, + &data) < 0) + goto error; + } return dmn; + error: virObjectUnref(dmn); return NULL; diff --git a/src/rpc/virnetdaemon.h b/src/rpc/virnetdaemon.h index 72c1df69b4..6576c463b5 100644 --- a/src/rpc/virnetdaemon.h +++ b/src/rpc/virnetdaemon.h @@ -40,15 +40,15 @@ virNetDaemonPtr virNetDaemonNew(void); int virNetDaemonAddServer(virNetDaemonPtr dmn, virNetServerPtr srv); -virNetServerPtr virNetDaemonAddServerPostExec(virNetDaemonPtr dmn, - const char *serverName, - virNetServerClientPrivNew clientPrivNew, - virNetServerClientPrivNewPostExecRestart clientPrivNewPostExecRestart, - virNetServerClientPrivPreExecRestart clientPrivPreExecRestart, - virFreeCallback clientPrivFree, - void *clientPrivOpaque); - -virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object); +typedef virNetServerPtr (*virNetDaemonNewServerPostExecRestart)(virNetDaemonPtr dmn, + const char *name, + virJSONValuePtr object, + void *opaque); +virNetDaemonPtr virNetDaemonNewPostExecRestart(virJSONValuePtr object, + size_t nDefServerNames, + const char **defServerNames, + virNetDaemonNewServerPostExecRestart cb, + void *opaque); virJSONValuePtr virNetDaemonPreExecRestart(virNetDaemonPtr dmn); diff --git a/tests/virnetdaemontest.c b/tests/virnetdaemontest.c index 3e60f09007..435513d314 100644 --- a/tests/virnetdaemontest.c +++ b/tests/virnetdaemontest.c @@ -194,12 +194,32 @@ struct testExecRestartData { bool pass; }; +static virNetServerPtr +testNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, + const char *name, + virJSONValuePtr object, + void *opaque) +{ + struct testExecRestartData *data = opaque; + size_t i; + for (i = 0; i < data->nservers; i++) { + if (STREQ(data->serverNames[i], name)) { + return virNetServerNewPostExecRestart(object, + name, + NULL, NULL, NULL, + NULL, NULL); + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, "Unexpected server name '%s'", name); + return NULL; +} + static int testExecRestart(const void *opaque) { size_t i; int ret = -1; virNetDaemonPtr dmn = NULL; - virNetServerPtr srv = NULL; const struct testExecRestartData *data = opaque; char *infile = NULL, *outfile = NULL; char *injsonstr = NULL, *outjsonstr = NULL; @@ -241,15 +261,20 @@ static int testExecRestart(const void *opaque) if (!(injson = virJSONValueFromString(injsonstr))) goto cleanup; - if (!(dmn = virNetDaemonNewPostExecRestart(injson))) + if (!(dmn = virNetDaemonNewPostExecRestart(injson, + data->nservers, + data->serverNames, + testNewServerPostExecRestart, + (void *)data))) goto cleanup; for (i = 0; i < data->nservers; i++) { - if (!(srv = virNetDaemonAddServerPostExec(dmn, data->serverNames[i], - NULL, NULL, NULL, - NULL, NULL))) + if (!virNetDaemonHasServer(dmn, data->serverNames[i])) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Server %s was not created", + data->serverNames[i]); goto cleanup; - srv = NULL; + } } if (!(outjson = virNetDaemonPreExecRestart(dmn))) -- 2.14.3
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 | 149 +++++++++++++++++++++++++++-------- 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, 149 insertions(+), 41 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 33133af2af..d54d26ab9d 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, + remoteAdmClientNew, + remoteAdmClientPreExecRestart, + remoteAdmClientFree, + logd->dmn))) + goto error; + + if (virNetDaemonAddServer(logd->dmn, srv) < 0) goto error; virObjectUnref(srv); srv = NULL; @@ -189,7 +207,7 @@ virLogDaemonGetHandler(virLogDaemonPtr dmn) static virNetServerPtr -virLogDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, +virLogDaemonNewServerPostExecRestart(virNetDaemonPtr dmn, const char *name, virJSONValuePtr object, void *opaque) @@ -202,6 +220,14 @@ virLogDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, virLogDaemonClientPreExecRestart, virLogDaemonClientFree, opaque); + } else if (STREQ(name, "admin")) { + return virNetServerNewPostExecRestart(object, + name, + remoteAdmClientNew, + remoteAdmClientNewPostExecRestart, + remoteAdmClientPreExecRestart, + remoteAdmClientFree, + dmn); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unexpected server name '%s' during restart"), @@ -354,10 +380,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; @@ -374,7 +402,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; } @@ -485,29 +514,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; } @@ -878,8 +928,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; @@ -889,6 +941,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; @@ -1016,12 +1069,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) { @@ -1098,22 +1152,30 @@ 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"); + /* If exec-restarting from old virtlogd, we won't have an + * admin server present */ + if (virNetDaemonHasServer(logDaemon->dmn, "admin")) + adminSrv = virNetDaemonGetServer(logDaemon->dmn, "admin"); if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); @@ -1133,11 +1195,25 @@ 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 (adminSrv != NULL) { + 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; + } + } + /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virLogDaemonErrorHandler); @@ -1155,7 +1231,7 @@ int main(int argc, char **argv) { /* Start accepting new clients from network */ - virNetServerUpdateServices(srv, true); + virNetDaemonUpdateServices(logDaemon->dmn, true); virNetDaemonRun(logDaemon->dmn); if (execRestart && @@ -1168,7 +1244,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) { @@ -1184,6 +1262,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/23/2018 02:23 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 | 149 +++++++++++++++++++++++++++-------- 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, 149 insertions(+), 41 deletions(-) create mode 100644 src/logging/virtlogd-admin.socket.in
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" }
Improper alignment. Michal
On Wed, Jan 31, 2018 at 12:23:32PM +0100, Michal Privoznik wrote:
On 01/23/2018 02:23 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 | 149 +++++++++++++++++++++++++++-------- 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, 149 insertions(+), 41 deletions(-) create mode 100644 src/logging/virtlogd-admin.socket.in
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" }
Improper alignment.
Oh, some <tab> crept in there by mistake.... 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 | 145 ++++++++++++++++++++++++++-------- 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, 142 insertions(+), 34 deletions(-) create mode 100644 src/locking/virtlockd-admin.socket.in diff --git a/cfg.mk b/cfg.mk index 1a5de2b154..5d369aadb2 100644 --- a/cfg.mk +++ b/cfg.mk @@ -775,7 +775,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 b1f0665aaa..79ab90fc91 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, + remoteAdmClientNew, + remoteAdmClientPreExecRestart, + remoteAdmClientFree, + lockd->dmn))) goto error; + + if (virNetDaemonAddServer(lockd->dmn, srv) < 0) + goto error; virObjectUnref(srv); srv = NULL; @@ -206,6 +224,14 @@ virLockDaemonNewServerPostExecRestart(virNetDaemonPtr dmn ATTRIBUTE_UNUSED, virLockDaemonClientPreExecRestart, virLockDaemonClientFree, opaque); + } else if (STREQ(name, "admin")) { + return virNetServerNewPostExecRestart(object, + name, + remoteAdmClientNew, + remoteAdmClientNewPostExecRestart, + remoteAdmClientPreExecRestart, + remoteAdmClientFree, + dmn); } else { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unexpected server name '%s' during restart"), @@ -420,10 +446,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; @@ -440,7 +468,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; } @@ -557,29 +586,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; } @@ -1112,8 +1162,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; @@ -1123,6 +1175,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; @@ -1250,12 +1303,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) { @@ -1330,22 +1384,30 @@ 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"); + /* If exec-restarting from old virtlockd, we won't have an + * admin server present */ + if (virNetDaemonHasServer(lockDaemon->dmn, "admin")) + adminSrv = virNetDaemonGetServer(lockDaemon->dmn, "admin"); if (timeout != -1) { VIR_DEBUG("Registering shutdown timeout %d", timeout); @@ -1366,11 +1428,25 @@ 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 (adminSrv != NULL) { + 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; + } + } + /* Disable error func, now logging is setup */ virSetErrorFunc(NULL, virLockDaemonErrorHandler); @@ -1401,8 +1477,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) { @@ -1418,6 +1496,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/23/2018 02:23 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 | 145 ++++++++++++++++++++++++++-------- 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, 142 insertions(+), 34 deletions(-) create mode 100644 src/locking/virtlockd-admin.socket.in
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" }
Improper alignment. Michal
On 01/23/2018 02:23 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).
In v2:
- Fixed completely broken post-exec restart support for admin servers - Misc fixes from v1 review
Daniel P. Berrange (11): admin: move admins server impl/dispatch into src/admin directory libvirtd: rename virNetServerClient callback impls to match type names admin: add support for post-exec restart callbacks util: add virGetUNIXSocketPath helper rpc: clarify "void *" values passed to client callbacks rpc: pass virNetServer to post-exec restart callback in typesafe manner rpc: annotate various parameters as being required to be non-NULL rpc: add method for checking if a named server exists rpc: refactor virNetServer setup for post-exec restarts logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
.gitignore | 1 + cfg.mk | 10 +- daemon/Makefile.am | 33 +--- daemon/libvirtd.c | 10 +- daemon/libvirtd.h | 10 -- daemon/remote.c | 6 +- daemon/remote.h | 6 +- po/POTFILES.in | 6 +- 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 | 48 ++++-- .../admin.h => src/admin/admin_server_dispatch.h | 18 +- src/libvirt-admin.c | 23 ++- src/libvirt_private.syms | 1 + src/libvirt_remote.syms | 2 +- src/locking/lock_daemon.c | 186 +++++++++++++++----- 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 | 188 ++++++++++++++++----- 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/rpc/virnetdaemon.c | 164 ++++++++++-------- src/rpc/virnetdaemon.h | 20 ++- src/rpc/virnetserver.c | 6 +- src/rpc/virnetserver.h | 7 +- src/rpc/virnetserverclient.c | 14 +- src/rpc/virnetserverclient.h | 23 ++- src/rpc/virnetserverprogram.h | 3 - src/util/virutil.c | 45 +++++ src/util/virutil.h | 1 + tests/virnetdaemontest.c | 37 +++- 41 files changed, 670 insertions(+), 287 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 (92%) rename daemon/admin.h => src/admin/admin_server_dispatch.h (59%) create mode 100644 src/locking/virtlockd-admin.socket.in create mode 100644 src/logging/virtlogd-admin.socket.in
ACK if you fix those two alignments in 10/11 and 11/11. Michal
On Tue, Jan 23, 2018 at 13:23:36 +0000, Daniel 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).
In v2:
- Fixed completely broken post-exec restart support for admin servers - Misc fixes from v1 review
Daniel P. Berrange (11): admin: move admins server impl/dispatch into src/admin directory libvirtd: rename virNetServerClient callback impls to match type names admin: add support for post-exec restart callbacks util: add virGetUNIXSocketPath helper rpc: clarify "void *" values passed to client callbacks rpc: pass virNetServer to post-exec restart callback in typesafe manner rpc: annotate various parameters as being required to be non-NULL rpc: add method for checking if a named server exists rpc: refactor virNetServer setup for post-exec restarts logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
This series (probably) breaks build on my gentoo box: CC util/libvirt_setuid_rpc_client_la-viralloc.lo CC util/libvirt_setuid_rpc_client_la-virarch.lo In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from ./rpc/virnetdaemon.h:33, from admin/admin_server.h:27, from admin/admin_server.c:26: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9269: admin/libvirt_driver_admin_la-admin_server.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... In file included from admin/admin_protocol.c:7:0: admin/admin_protocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9262: admin/libvirt_driver_admin_la-admin_protocol.lo] Error 1 In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from admin/admin_server_dispatch.h:26, from admin/admin_server_dispatch.c:28: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9276: admin/libvirt_driver_admin_la-admin_server_dispatch.lo] Error 1
On Wed, Jan 31, 2018 at 06:01:10PM +0100, Peter Krempa wrote:
On Tue, Jan 23, 2018 at 13:23:36 +0000, Daniel 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).
In v2:
- Fixed completely broken post-exec restart support for admin servers - Misc fixes from v1 review
Daniel P. Berrange (11): admin: move admins server impl/dispatch into src/admin directory libvirtd: rename virNetServerClient callback impls to match type names admin: add support for post-exec restart callbacks util: add virGetUNIXSocketPath helper rpc: clarify "void *" values passed to client callbacks rpc: pass virNetServer to post-exec restart callback in typesafe manner rpc: annotate various parameters as being required to be non-NULL rpc: add method for checking if a named server exists rpc: refactor virNetServer setup for post-exec restarts logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
This series (probably) breaks build on my gentoo box:
CC util/libvirt_setuid_rpc_client_la-viralloc.lo CC util/libvirt_setuid_rpc_client_la-virarch.lo In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from ./rpc/virnetdaemon.h:33, from admin/admin_server.h:27, from admin/admin_server.c:26: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9269: admin/libvirt_driver_admin_la-admin_server.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... In file included from admin/admin_protocol.c:7:0: admin/admin_protocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9262: admin/libvirt_driver_admin_la-admin_protocol.lo] Error 1 In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from admin/admin_server_dispatch.h:26, from admin/admin_server_dispatch.c:28: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9276: admin/libvirt_driver_admin_la-admin_server_dispatch.lo] Error 1
Can you tell me if this fixes it: diff --git a/src/Makefile.am b/src/Makefile.am index a9182d29af..79adc9ba51 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2335,6 +2335,7 @@ libvirt_driver_admin_la_SOURCES = \ $(NULL) libvirt_driver_admin_la_CFLAGS = \ $(AM_CFLAGS) \ + $(XDR_CFLAGS) \ -I$(top_srcdir)/src/util \ -I$(top_srcdir)/src/admin \ $(NULL) 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 :|
On Wed, Jan 31, 2018 at 05:15:51PM +0000, Daniel P. Berrangé wrote:
On Wed, Jan 31, 2018 at 06:01:10PM +0100, Peter Krempa wrote:
On Tue, Jan 23, 2018 at 13:23:36 +0000, Daniel 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).
In v2:
- Fixed completely broken post-exec restart support for admin servers - Misc fixes from v1 review
Daniel P. Berrange (11): admin: move admins server impl/dispatch into src/admin directory libvirtd: rename virNetServerClient callback impls to match type names admin: add support for post-exec restart callbacks util: add virGetUNIXSocketPath helper rpc: clarify "void *" values passed to client callbacks rpc: pass virNetServer to post-exec restart callback in typesafe manner rpc: annotate various parameters as being required to be non-NULL rpc: add method for checking if a named server exists rpc: refactor virNetServer setup for post-exec restarts logd: add support for admin protocol in virtlogd lockd: add support for admin protocol in virtlockd
This series (probably) breaks build on my gentoo box:
CC util/libvirt_setuid_rpc_client_la-viralloc.lo CC util/libvirt_setuid_rpc_client_la-virarch.lo In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from ./rpc/virnetdaemon.h:33, from admin/admin_server.h:27, from admin/admin_server.c:26: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9269: admin/libvirt_driver_admin_la-admin_server.lo] Error 1 make[3]: *** Waiting for unfinished jobs.... In file included from admin/admin_protocol.c:7:0: admin/admin_protocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9262: admin/libvirt_driver_admin_la-admin_protocol.lo] Error 1 In file included from ./rpc/virnetmessage.h:24:0, from ./rpc/virnetserverprogram.h:27, from admin/admin_server_dispatch.h:26, from admin/admin_server_dispatch.c:28: ./rpc/virnetprotocol.h:9:21: fatal error: rpc/rpc.h: No such file or directory #include <rpc/rpc.h> ^ compilation terminated. make[3]: *** [Makefile:9276: admin/libvirt_driver_admin_la-admin_server_dispatch.lo] Error 1
Can you tell me if this fixes it:
diff --git a/src/Makefile.am b/src/Makefile.am index a9182d29af..79adc9ba51 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -2335,6 +2335,7 @@ libvirt_driver_admin_la_SOURCES = \ $(NULL) libvirt_driver_admin_la_CFLAGS = \ $(AM_CFLAGS) \ + $(XDR_CFLAGS) \ -I$(top_srcdir)/src/util \ -I$(top_srcdir)/src/admin \ $(NULL)
I found this reproducable on Fedora rawhide too, so was able to validate this fix, and pushed it under build breaker rule 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 (5)
-
Daniel P. Berrange -
Daniel P. Berrangé
-
John Ferlan -
Michal Privoznik -
Peter Krempa