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(a)redhat.com&gt; --- .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) \ -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) \ +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(a)redhat.com&gt; */ -#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(a)redhat.com&gt; */ -#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

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(a)redhat.com&gt; --- 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

Signed-off-by: Daniel P. Berrange <berrange(a)redhat.com&gt; --- 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 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(a)redhat.com&gt; --- 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 02/01/2018 06:57 AM, John Ferlan wrote: Also wouldn't the NONNULL's need to change based on "#ifdef WITH_GNUTLS" for arguments after #5? And virnetdaemontest.c... John

On Thu, Feb 01, 2018 at 06:57:49AM -0500, John Ferlan wrote: 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.... ...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 :|

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(a)redhat.com&gt; --- 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

On 01/23/2018 02:23 PM, Daniel P. Berrange wrote: Improper alignment. Michal

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(a)redhat.com&gt; --- 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 Wed, Jan 31, 2018 at 06:01:10PM +0100, Peter Krempa wrote: 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 :|