Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
On Tue, Jul 30, 2019 at 12:46:31PM +0200, Christophe de Dinechin wrote:
Daniel P. Berrangé writes:
> Prepare for reusing libvirtd source to create other daemons by making
> the use of IP sockets conditionally defined by the make rules.
>
> The main libvirtd daemon will retain IP listen ability, but all the
> driver specific daemons will be local UNIX sockets only. Apps needing
> IP connectivity will connect via the libvirtd daemon which will proxy
> to the driver specfic daemon.
>
> Reviewed-by: Andrea Bolognani <abologna(a)redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
> ---
> src/remote/Makefile.inc.am | 1 +
> src/remote/remote_daemon.c | 39 ++++++++++++++++++++++++++-----
> src/remote/remote_daemon_config.c | 36 ++++++++++++++++++++--------
> src/remote/remote_daemon_config.h | 10 +++++++-
> 4 files changed, 69 insertions(+), 17 deletions(-)
>
> diff --git a/src/remote/Makefile.inc.am b/src/remote/Makefile.inc.am
> index b72186109a..2277bf49d2 100644
> --- a/src/remote/Makefile.inc.am
> +++ b/src/remote/Makefile.inc.am
> @@ -148,6 +148,7 @@ libvirtd_CFLAGS = \
> -I$(srcdir)/rpc \
> -DSOCK_PREFIX="\"libvirt\"" \
> -DDAEMON_NAME="\"libvirtd\"" \
> + -DENABLE_IP \
What about using "WITH_IP" to stay consistent with the other enabler macros?
Perhaps. I'll see how much hell that creates with merge conflicts in the
later part of the series.
> $(NULL)
>
> libvirtd_LDFLAGS = \
> diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c
> index 97621884b0..fadfc7c016 100644
> --- a/src/remote/remote_daemon.c
> +++ b/src/remote/remote_daemon.c
> @@ -381,11 +381,13 @@ static int ATTRIBUTE_NONNULL(3)
> daemonSetupNetworking(virNetServerPtr srv,
> virNetServerPtr srvAdm,
> struct daemonConfig *config,
> +#ifdef ENABLE_IP
> + bool ipsock,
> + bool privileged,
> +#endif /* ! ENABLE_IP */
Absolute nit, but I would move the two bool last to avoid arch-dependent
and config-dependent padding in the middle of the struct.
I moved them here, because if you have #ifdef conditional around
the last parameter in the function, the formatting gets messy
wrt to the closing ')', and need to trim the trailing ',' on the
previous parameter.
> const char *sock_path,
> const char *sock_path_ro,
> - const char *sock_path_adm,
> - bool ipsock,
> - bool privileged)
> + const char *sock_path_adm)
> {
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 :|