[libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon

[Daniel P. Berrangé]

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 at redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange at 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 :|