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

Daniel P. Berrangé posted 48 patches 6 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Daniel P. Berrangé 6 years, 6 months ago
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@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@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 \
 	$(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 */
                       const char *sock_path,
                       const char *sock_path_ro,
-                      const char *sock_path_adm,
-                      bool ipsock,
-                      bool privileged)
+                      const char *sock_path_adm)
 {
     gid_t unix_sock_gid = 0;
     int unix_sock_ro_mask = 0;
@@ -397,15 +399,19 @@ daemonSetupNetworking(virNetServerPtr srv,
         { .name = DAEMON_NAME ".socket", .family = AF_UNIX, .path = sock_path },
         { .name = DAEMON_NAME "-ro.socket", .family = AF_UNIX, .path = sock_path_ro },
         { .name = DAEMON_NAME "-admin.socket", .family = AF_UNIX, .path = sock_path_adm },
+#ifdef ENABLE_IP
         { .name = DAEMON_NAME "-tcp.socket", .family = AF_INET },
         { .name = DAEMON_NAME "-tls.socket", .family = AF_INET },
+#endif /* ! ENABLE_IP */
     };
 
+#ifdef ENABLE_IP
     if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0)
         return -1;
 
     if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0)
         return -1;
+#endif /* ! ENABLE_IP */
 
     if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0)
         return -1;
@@ -470,6 +476,7 @@ daemonSetupNetworking(virNetServerPtr srv,
                                    config->admin_max_client_requests) < 0)
         goto cleanup;
 
+#ifdef ENABLE_IP
     if (((ipsock && config->listen_tcp) || act) &&
         virNetServerAddServiceTCP(srv,
                                   act,
@@ -544,6 +551,7 @@ daemonSetupNetworking(virNetServerPtr srv,
         }
         virObjectUnref(ctxt);
     }
+#endif /* ! ENABLE_IP */
 
     if (act &&
         virSystemdActivationComplete(act) < 0)
@@ -892,7 +900,9 @@ daemonUsage(const char *argv0, bool privileged)
         { "-h | --help", N_("Display program help") },
         { "-v | --verbose", N_("Verbose messages") },
         { "-d | --daemon", N_("Run as a daemon & write PID file") },
+#ifdef ENABLE_IP
         { "-l | --listen", N_("Listen for TCP/IP connections") },
+#endif /* ENABLE_IP */
         { "-t | --timeout <secs>", N_("Exit after timeout period") },
         { "-f | --config <file>", N_("Configuration file") },
         { "-V | --version", N_("Display version information") },
@@ -929,6 +939,7 @@ daemonUsage(const char *argv0, bool privileged)
                 LOCALSTATEDIR, SOCK_PREFIX);
     fprintf(stderr, "\n");
 
+#ifdef ENABLE_IP
     fprintf(stderr, "    %s:\n", _("TLS"));
     fprintf(stderr, "      %s: %s\n",
             _("CA certificate"),
@@ -940,6 +951,7 @@ daemonUsage(const char *argv0, bool privileged)
             _("Server private key"),
             privileged ? LIBVIRT_SERVERKEY : "$HOME/.pki/libvirt/serverkey.pem");
     fprintf(stderr, "\n");
+#endif /* ENABLE_IP */
 
     fprintf(stderr, "    %s:\n",
             _("PID file (unless overridden by -p)"));
@@ -966,7 +978,9 @@ int main(int argc, char **argv) {
     int timeout = -1;        /* -t: Shutdown timeout */
     int verbose = 0;
     int godaemon = 0;
+#ifdef ENABLE_IP
     int ipsock = 0;
+#endif /* ! ENABLE_IP */
     struct daemonConfig *config;
     bool privileged = geteuid() == 0 ? true : false;
     bool implicit_conf = false;
@@ -976,7 +990,9 @@ int main(int argc, char **argv) {
     struct option opts[] = {
         { "verbose", no_argument, &verbose, 'v'},
         { "daemon", no_argument, &godaemon, 'd'},
+#ifdef ENABLE_IP
         { "listen", no_argument, &ipsock, 'l'},
+#endif /* ! ENABLE_IP */
         { "config", required_argument, NULL, 'f'},
         { "timeout", required_argument, NULL, 't'},
         { "pid-file", required_argument, NULL, 'p'},
@@ -999,8 +1015,13 @@ int main(int argc, char **argv) {
         int optidx = 0;
         int c;
         char *tmp;
+#ifdef ENABLE_IP
+        const char *optstr = "ldf:p:t:vVh";
+#else /* ! ENABLE_IP */
+        const char *optstr = "df:p:t:vVh";
+#endif /* ! ENABLE_IP */
 
-        c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
+        c = getopt_long(argc, argv, optstr, opts, &optidx);
 
         if (c == -1)
             break;
@@ -1015,9 +1036,12 @@ int main(int argc, char **argv) {
         case 'd':
             godaemon = 1;
             break;
+
+#ifdef ENABLE_IP
         case 'l':
             ipsock = 1;
             break;
+#endif /* ! ENABLE_IP */
 
         case 't':
             if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0
@@ -1331,10 +1355,13 @@ int main(int argc, char **argv) {
 
     if (daemonSetupNetworking(srv, srvAdm,
                               config,
+#ifdef ENABLE_IP
+                              ipsock,
+                              privileged,
+#endif /* !ENABLE_IP */
                               sock_file,
                               sock_file_ro,
-                              sock_file_adm,
-                              ipsock, privileged) < 0) {
+                              sock_file_adm) < 0) {
         ret = VIR_DAEMON_ERR_NETWORK;
         goto cleanup;
     }
diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c
index 3e62b4203f..3c5ccd5ba8 100644
--- a/src/remote/remote_daemon_config.c
+++ b/src/remote/remote_daemon_config.c
@@ -107,12 +107,14 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
     if (VIR_ALLOC(data) < 0)
         return NULL;
 
+#ifdef ENABLE_IP
     data->listen_tls = 1;
     data->listen_tcp = 0;
 
     if (VIR_STRDUP(data->tls_port, LIBVIRTD_TLS_PORT) < 0 ||
         VIR_STRDUP(data->tcp_port, LIBVIRTD_TCP_PORT) < 0)
         goto error;
+#endif /* !ENABLE_IP */
 
     /* Only default to PolicyKit if running as root */
 #if WITH_POLKIT
@@ -133,12 +135,14 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
         VIR_STRDUP(data->unix_sock_admin_perms, "0700") < 0)
         goto error;
 
-#if WITH_SASL
+#ifdef ENABLE_IP
+# if WITH_SASL
     data->auth_tcp = REMOTE_AUTH_SASL;
-#else
+# else
     data->auth_tcp = REMOTE_AUTH_NONE;
-#endif
+# endif
     data->auth_tls = REMOTE_AUTH_NONE;
+#endif /* ! ENABLE_IP */
 
     data->min_workers = 5;
     data->max_workers = 20;
@@ -182,9 +186,12 @@ daemonConfigFree(struct daemonConfig *data)
     if (!data)
         return;
 
+#ifdef ENABLE_IP
     VIR_FREE(data->listen_addr);
     VIR_FREE(data->tls_port);
     VIR_FREE(data->tcp_port);
+#endif /* ! ENABLE_IP */
+
     tmp = data->access_drivers;
     while (tmp && *tmp) {
         VIR_FREE(*tmp);
@@ -198,25 +205,28 @@ daemonConfigFree(struct daemonConfig *data)
     VIR_FREE(data->unix_sock_group);
     VIR_FREE(data->unix_sock_dir);
 
-    tmp = data->tls_allowed_dn_list;
+    tmp = data->sasl_allowed_username_list;
     while (tmp && *tmp) {
         VIR_FREE(*tmp);
         tmp++;
     }
-    VIR_FREE(data->tls_allowed_dn_list);
+    VIR_FREE(data->sasl_allowed_username_list);
 
-    tmp = data->sasl_allowed_username_list;
+#ifdef ENABLE_IP
+    tmp = data->tls_allowed_dn_list;
     while (tmp && *tmp) {
         VIR_FREE(*tmp);
         tmp++;
     }
-    VIR_FREE(data->sasl_allowed_username_list);
+    VIR_FREE(data->tls_allowed_dn_list);
+
     VIR_FREE(data->tls_priority);
 
     VIR_FREE(data->key_file);
     VIR_FREE(data->ca_file);
     VIR_FREE(data->cert_file);
     VIR_FREE(data->crl_file);
+#endif /* ! ENABLE_IP */
 
     VIR_FREE(data->host_uuid);
     VIR_FREE(data->host_uuid_source);
@@ -231,6 +241,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
                         const char *filename,
                         virConfPtr conf)
 {
+#ifdef ENABLE_IP
     if (virConfGetValueBool(conf, "listen_tcp", &data->listen_tcp) < 0)
         goto error;
     if (virConfGetValueBool(conf, "listen_tls", &data->listen_tls) < 0)
@@ -241,6 +252,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
         goto error;
     if (virConfGetValueString(conf, "listen_addr", &data->listen_addr) < 0)
         goto error;
+#endif /* !ENABLE_IP */
 
     if (remoteConfigGetAuth(conf, filename, "auth_unix_rw", &data->auth_unix_rw) < 0)
         goto error;
@@ -256,10 +268,13 @@ daemonConfigLoadOptions(struct daemonConfig *data,
 #endif
     if (remoteConfigGetAuth(conf, filename, "auth_unix_ro", &data->auth_unix_ro) < 0)
         goto error;
+
+#ifdef ENABLE_IP
     if (remoteConfigGetAuth(conf, filename, "auth_tcp", &data->auth_tcp) < 0)
         goto error;
     if (remoteConfigGetAuth(conf, filename, "auth_tls", &data->auth_tls) < 0)
         goto error;
+#endif /* ! ENABLE_IP */
 
     if (virConfGetValueStringList(conf, "access_drivers", false,
                                   &data->access_drivers) < 0)
@@ -277,6 +292,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
     if (virConfGetValueString(conf, "unix_sock_dir", &data->unix_sock_dir) < 0)
         goto error;
 
+#ifdef ENABLE_IP
     if (virConfGetValueBool(conf, "tls_no_sanity_certificate", &data->tls_no_sanity_certificate) < 0)
         goto error;
     if (virConfGetValueBool(conf, "tls_no_verify_certificate", &data->tls_no_verify_certificate) < 0)
@@ -295,14 +311,14 @@ daemonConfigLoadOptions(struct daemonConfig *data,
                                   &data->tls_allowed_dn_list) < 0)
         goto error;
 
+    if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0)
+        goto error;
+#endif /* ! ENABLE_IP */
 
     if (virConfGetValueStringList(conf, "sasl_allowed_username_list", false,
                                   &data->sasl_allowed_username_list) < 0)
         goto error;
 
-    if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0)
-        goto error;
-
     if (virConfGetValueUInt(conf, "min_workers", &data->min_workers) < 0)
         goto error;
     if (virConfGetValueUInt(conf, "max_workers", &data->max_workers) < 0)
diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h
index d580e7d49c..5a54abed85 100644
--- a/src/remote/remote_daemon_config.h
+++ b/src/remote/remote_daemon_config.h
@@ -27,11 +27,13 @@ struct daemonConfig {
     char *host_uuid;
     char *host_uuid_source;
 
+#ifdef ENABLE_IP
     bool listen_tls;
     bool listen_tcp;
     char *listen_addr;
     char *tls_port;
     char *tcp_port;
+#endif /* ! ENABLE_IP */
 
     char *unix_sock_admin_perms;
     char *unix_sock_ro_perms;
@@ -41,21 +43,27 @@ struct daemonConfig {
 
     int auth_unix_rw;
     int auth_unix_ro;
+
+#ifdef ENABLE_IP
     int auth_tcp;
     int auth_tls;
+#endif /* ! ENABLE_IP */
 
     char **access_drivers;
 
+#ifdef ENABLE_IP
     bool tls_no_verify_certificate;
     bool tls_no_sanity_certificate;
     char **tls_allowed_dn_list;
-    char **sasl_allowed_username_list;
     char *tls_priority;
 
     char *key_file;
     char *cert_file;
     char *ca_file;
     char *crl_file;
+#endif /* ! ENABLE_IP */
+
+    char **sasl_allowed_username_list;
 
     unsigned int min_workers;
     unsigned int max_workers;
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Christophe de Dinechin 6 years, 6 months ago
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@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@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?

>  	$(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.

>                        const char *sock_path,
>                        const char *sock_path_ro,
> -                      const char *sock_path_adm,
> -                      bool ipsock,
> -                      bool privileged)
> +                      const char *sock_path_adm)
>  {
>      gid_t unix_sock_gid = 0;
>      int unix_sock_ro_mask = 0;
> @@ -397,15 +399,19 @@ daemonSetupNetworking(virNetServerPtr srv,
>          { .name = DAEMON_NAME ".socket", .family = AF_UNIX, .path = sock_path },
>          { .name = DAEMON_NAME "-ro.socket", .family = AF_UNIX, .path = sock_path_ro },
>          { .name = DAEMON_NAME "-admin.socket", .family = AF_UNIX, .path = sock_path_adm },
> +#ifdef ENABLE_IP
>          { .name = DAEMON_NAME "-tcp.socket", .family = AF_INET },
>          { .name = DAEMON_NAME "-tls.socket", .family = AF_INET },
> +#endif /* ! ENABLE_IP */
>      };
>
> +#ifdef ENABLE_IP
>      if ((actmap[3].port = virSocketAddrResolveService(config->tcp_port)) < 0)
>          return -1;
>
>      if ((actmap[4].port = virSocketAddrResolveService(config->tls_port)) < 0)
>          return -1;
> +#endif /* ! ENABLE_IP */
>
>      if (virSystemdGetActivation(actmap, ARRAY_CARDINALITY(actmap), &act) < 0)
>          return -1;
> @@ -470,6 +476,7 @@ daemonSetupNetworking(virNetServerPtr srv,
>                                     config->admin_max_client_requests) < 0)
>          goto cleanup;
>
> +#ifdef ENABLE_IP
>      if (((ipsock && config->listen_tcp) || act) &&
>          virNetServerAddServiceTCP(srv,
>                                    act,
> @@ -544,6 +551,7 @@ daemonSetupNetworking(virNetServerPtr srv,
>          }
>          virObjectUnref(ctxt);
>      }
> +#endif /* ! ENABLE_IP */
>
>      if (act &&
>          virSystemdActivationComplete(act) < 0)
> @@ -892,7 +900,9 @@ daemonUsage(const char *argv0, bool privileged)
>          { "-h | --help", N_("Display program help") },
>          { "-v | --verbose", N_("Verbose messages") },
>          { "-d | --daemon", N_("Run as a daemon & write PID file") },
> +#ifdef ENABLE_IP
>          { "-l | --listen", N_("Listen for TCP/IP connections") },
> +#endif /* ENABLE_IP */
>          { "-t | --timeout <secs>", N_("Exit after timeout period") },
>          { "-f | --config <file>", N_("Configuration file") },
>          { "-V | --version", N_("Display version information") },
> @@ -929,6 +939,7 @@ daemonUsage(const char *argv0, bool privileged)
>                  LOCALSTATEDIR, SOCK_PREFIX);
>      fprintf(stderr, "\n");
>
> +#ifdef ENABLE_IP
>      fprintf(stderr, "    %s:\n", _("TLS"));
>      fprintf(stderr, "      %s: %s\n",
>              _("CA certificate"),
> @@ -940,6 +951,7 @@ daemonUsage(const char *argv0, bool privileged)
>              _("Server private key"),
>              privileged ? LIBVIRT_SERVERKEY : "$HOME/.pki/libvirt/serverkey.pem");
>      fprintf(stderr, "\n");
> +#endif /* ENABLE_IP */
>
>      fprintf(stderr, "    %s:\n",
>              _("PID file (unless overridden by -p)"));
> @@ -966,7 +978,9 @@ int main(int argc, char **argv) {
>      int timeout = -1;        /* -t: Shutdown timeout */
>      int verbose = 0;
>      int godaemon = 0;
> +#ifdef ENABLE_IP
>      int ipsock = 0;
> +#endif /* ! ENABLE_IP */
>      struct daemonConfig *config;
>      bool privileged = geteuid() == 0 ? true : false;
>      bool implicit_conf = false;
> @@ -976,7 +990,9 @@ int main(int argc, char **argv) {
>      struct option opts[] = {
>          { "verbose", no_argument, &verbose, 'v'},
>          { "daemon", no_argument, &godaemon, 'd'},
> +#ifdef ENABLE_IP
>          { "listen", no_argument, &ipsock, 'l'},
> +#endif /* ! ENABLE_IP */
>          { "config", required_argument, NULL, 'f'},
>          { "timeout", required_argument, NULL, 't'},
>          { "pid-file", required_argument, NULL, 'p'},
> @@ -999,8 +1015,13 @@ int main(int argc, char **argv) {
>          int optidx = 0;
>          int c;
>          char *tmp;
> +#ifdef ENABLE_IP
> +        const char *optstr = "ldf:p:t:vVh";
> +#else /* ! ENABLE_IP */
> +        const char *optstr = "df:p:t:vVh";
> +#endif /* ! ENABLE_IP */
>
> -        c = getopt_long(argc, argv, "ldf:p:t:vVh", opts, &optidx);
> +        c = getopt_long(argc, argv, optstr, opts, &optidx);
>
>          if (c == -1)
>              break;
> @@ -1015,9 +1036,12 @@ int main(int argc, char **argv) {
>          case 'd':
>              godaemon = 1;
>              break;
> +
> +#ifdef ENABLE_IP
>          case 'l':
>              ipsock = 1;
>              break;
> +#endif /* ! ENABLE_IP */
>
>          case 't':
>              if (virStrToLong_i(optarg, &tmp, 10, &timeout) != 0
> @@ -1331,10 +1355,13 @@ int main(int argc, char **argv) {
>
>      if (daemonSetupNetworking(srv, srvAdm,
>                                config,
> +#ifdef ENABLE_IP
> +                              ipsock,
> +                              privileged,
> +#endif /* !ENABLE_IP */
>                                sock_file,
>                                sock_file_ro,
> -                              sock_file_adm,
> -                              ipsock, privileged) < 0) {
> +                              sock_file_adm) < 0) {
>          ret = VIR_DAEMON_ERR_NETWORK;
>          goto cleanup;
>      }
> diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c
> index 3e62b4203f..3c5ccd5ba8 100644
> --- a/src/remote/remote_daemon_config.c
> +++ b/src/remote/remote_daemon_config.c
> @@ -107,12 +107,14 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>      if (VIR_ALLOC(data) < 0)
>          return NULL;
>
> +#ifdef ENABLE_IP
>      data->listen_tls = 1;
>      data->listen_tcp = 0;
>
>      if (VIR_STRDUP(data->tls_port, LIBVIRTD_TLS_PORT) < 0 ||
>          VIR_STRDUP(data->tcp_port, LIBVIRTD_TCP_PORT) < 0)
>          goto error;
> +#endif /* !ENABLE_IP */
>
>      /* Only default to PolicyKit if running as root */
>  #if WITH_POLKIT
> @@ -133,12 +135,14 @@ daemonConfigNew(bool privileged ATTRIBUTE_UNUSED)
>          VIR_STRDUP(data->unix_sock_admin_perms, "0700") < 0)
>          goto error;
>
> -#if WITH_SASL
> +#ifdef ENABLE_IP
> +# if WITH_SASL
>      data->auth_tcp = REMOTE_AUTH_SASL;
> -#else
> +# else
>      data->auth_tcp = REMOTE_AUTH_NONE;
> -#endif
> +# endif
>      data->auth_tls = REMOTE_AUTH_NONE;
> +#endif /* ! ENABLE_IP */
>
>      data->min_workers = 5;
>      data->max_workers = 20;
> @@ -182,9 +186,12 @@ daemonConfigFree(struct daemonConfig *data)
>      if (!data)
>          return;
>
> +#ifdef ENABLE_IP
>      VIR_FREE(data->listen_addr);
>      VIR_FREE(data->tls_port);
>      VIR_FREE(data->tcp_port);
> +#endif /* ! ENABLE_IP */
> +
>      tmp = data->access_drivers;
>      while (tmp && *tmp) {
>          VIR_FREE(*tmp);
> @@ -198,25 +205,28 @@ daemonConfigFree(struct daemonConfig *data)
>      VIR_FREE(data->unix_sock_group);
>      VIR_FREE(data->unix_sock_dir);
>
> -    tmp = data->tls_allowed_dn_list;
> +    tmp = data->sasl_allowed_username_list;
>      while (tmp && *tmp) {
>          VIR_FREE(*tmp);
>          tmp++;
>      }
> -    VIR_FREE(data->tls_allowed_dn_list);
> +    VIR_FREE(data->sasl_allowed_username_list);
>
> -    tmp = data->sasl_allowed_username_list;
> +#ifdef ENABLE_IP
> +    tmp = data->tls_allowed_dn_list;
>      while (tmp && *tmp) {
>          VIR_FREE(*tmp);
>          tmp++;
>      }
> -    VIR_FREE(data->sasl_allowed_username_list);
> +    VIR_FREE(data->tls_allowed_dn_list);
> +
>      VIR_FREE(data->tls_priority);
>
>      VIR_FREE(data->key_file);
>      VIR_FREE(data->ca_file);
>      VIR_FREE(data->cert_file);
>      VIR_FREE(data->crl_file);
> +#endif /* ! ENABLE_IP */
>
>      VIR_FREE(data->host_uuid);
>      VIR_FREE(data->host_uuid_source);
> @@ -231,6 +241,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>                          const char *filename,
>                          virConfPtr conf)
>  {
> +#ifdef ENABLE_IP
>      if (virConfGetValueBool(conf, "listen_tcp", &data->listen_tcp) < 0)
>          goto error;
>      if (virConfGetValueBool(conf, "listen_tls", &data->listen_tls) < 0)
> @@ -241,6 +252,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>          goto error;
>      if (virConfGetValueString(conf, "listen_addr", &data->listen_addr) < 0)
>          goto error;
> +#endif /* !ENABLE_IP */
>
>      if (remoteConfigGetAuth(conf, filename, "auth_unix_rw", &data->auth_unix_rw) < 0)
>          goto error;
> @@ -256,10 +268,13 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>  #endif
>      if (remoteConfigGetAuth(conf, filename, "auth_unix_ro", &data->auth_unix_ro) < 0)
>          goto error;
> +
> +#ifdef ENABLE_IP
>      if (remoteConfigGetAuth(conf, filename, "auth_tcp", &data->auth_tcp) < 0)
>          goto error;
>      if (remoteConfigGetAuth(conf, filename, "auth_tls", &data->auth_tls) < 0)
>          goto error;
> +#endif /* ! ENABLE_IP */
>
>      if (virConfGetValueStringList(conf, "access_drivers", false,
>                                    &data->access_drivers) < 0)
> @@ -277,6 +292,7 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>      if (virConfGetValueString(conf, "unix_sock_dir", &data->unix_sock_dir) < 0)
>          goto error;
>
> +#ifdef ENABLE_IP
>      if (virConfGetValueBool(conf, "tls_no_sanity_certificate", &data->tls_no_sanity_certificate) < 0)
>          goto error;
>      if (virConfGetValueBool(conf, "tls_no_verify_certificate", &data->tls_no_verify_certificate) < 0)
> @@ -295,14 +311,14 @@ daemonConfigLoadOptions(struct daemonConfig *data,
>                                    &data->tls_allowed_dn_list) < 0)
>          goto error;
>
> +    if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0)
> +        goto error;
> +#endif /* ! ENABLE_IP */
>
>      if (virConfGetValueStringList(conf, "sasl_allowed_username_list", false,
>                                    &data->sasl_allowed_username_list) < 0)
>          goto error;
>
> -    if (virConfGetValueString(conf, "tls_priority", &data->tls_priority) < 0)
> -        goto error;
> -
>      if (virConfGetValueUInt(conf, "min_workers", &data->min_workers) < 0)
>          goto error;
>      if (virConfGetValueUInt(conf, "max_workers", &data->max_workers) < 0)
> diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h
> index d580e7d49c..5a54abed85 100644
> --- a/src/remote/remote_daemon_config.h
> +++ b/src/remote/remote_daemon_config.h
> @@ -27,11 +27,13 @@ struct daemonConfig {
>      char *host_uuid;
>      char *host_uuid_source;
>
> +#ifdef ENABLE_IP
>      bool listen_tls;
>      bool listen_tcp;
>      char *listen_addr;
>      char *tls_port;
>      char *tcp_port;
> +#endif /* ! ENABLE_IP */
>
>      char *unix_sock_admin_perms;
>      char *unix_sock_ro_perms;
> @@ -41,21 +43,27 @@ struct daemonConfig {
>
>      int auth_unix_rw;
>      int auth_unix_ro;
> +
> +#ifdef ENABLE_IP
>      int auth_tcp;
>      int auth_tls;
> +#endif /* ! ENABLE_IP */
>
>      char **access_drivers;
>
> +#ifdef ENABLE_IP
>      bool tls_no_verify_certificate;
>      bool tls_no_sanity_certificate;
>      char **tls_allowed_dn_list;
> -    char **sasl_allowed_username_list;
>      char *tls_priority;
>
>      char *key_file;
>      char *cert_file;
>      char *ca_file;
>      char *crl_file;
> +#endif /* ! ENABLE_IP */
> +
> +    char **sasl_allowed_username_list;
>
>      unsigned int min_workers;
>      unsigned int max_workers;
> --
> 2.21.0

Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
--
Cheers,
Christophe de Dinechin (IRC c3d)

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Daniel P. Berrangé 6 years, 6 months ago
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@redhat.com>
> > Signed-off-by: Daniel P. Berrangé <berrange@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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Andrea Bolognani 6 years, 6 months ago
On Tue, 2019-07-30 at 12:00 +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 30, 2019 at 12:46:31PM +0200, Christophe de Dinechin wrote:
> > Daniel P. Berrangé writes:
> > > @@ -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.

I think Christophe, despite the fact that he quoted the function
signature, was actually referring to struct daemonConfig and the
members within.

I'm not sure whether we care about the padding and relative alignment
in this case, though.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Christophe de Dinechin 6 years, 6 months ago

> On 30 Jul 2019, at 13:15, Andrea Bolognani <abologna@redhat.com> wrote:
> 
> On Tue, 2019-07-30 at 12:00 +0100, Daniel P. Berrangé wrote:
>> On Tue, Jul 30, 2019 at 12:46:31PM +0200, Christophe de Dinechin wrote:
>>> Daniel P. Berrangé writes:
>>>> @@ -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.
> 
> I think Christophe, despite the fact that he quoted the function
> signature, was actually referring to struct daemonConfig and the
> members within.

Indeed.

> 
> I'm not sure whether we care about the padding and relative alignment
> in this case, though.

“Absolute nit” :-)

> 
> -- 
> Andrea Bolognani / Red Hat / Virtualization
> 


--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 10/48] remote: conditionalize IP socket usage in libvirtd daemon
Posted by Ján Tomko 6 years, 6 months ago
On Mon, Jul 29, 2019 at 06:10:52PM +0100, Daniel P. Berrangé wrote:
>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@redhat.com>
>Signed-off-by: Daniel P. Berrangé <berrange@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 \
> 	$(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
>@@ -892,7 +900,9 @@ daemonUsage(const char *argv0, bool privileged)
>         { "-h | --help", N_("Display program help") },
>         { "-v | --verbose", N_("Verbose messages") },
>         { "-d | --daemon", N_("Run as a daemon & write PID file") },
>+#ifdef ENABLE_IP
>         { "-l | --listen", N_("Listen for TCP/IP connections") },
>+#endif /* ENABLE_IP */
>         { "-t | --timeout <secs>", N_("Exit after timeout period") },
>         { "-f | --config <file>", N_("Configuration file") },
>         { "-V | --version", N_("Display version information") },
>@@ -929,6 +939,7 @@ daemonUsage(const char *argv0, bool privileged)
>                 LOCALSTATEDIR, SOCK_PREFIX);
>     fprintf(stderr, "\n");
>
>+#ifdef ENABLE_IP
>     fprintf(stderr, "    %s:\n", _("TLS"));
>     fprintf(stderr, "      %s: %s\n",
>             _("CA certificate"),
>@@ -940,6 +951,7 @@ daemonUsage(const char *argv0, bool privileged)
>             _("Server private key"),
>             privileged ? LIBVIRT_SERVERKEY : "$HOME/.pki/libvirt/serverkey.pem");
>     fprintf(stderr, "\n");
>+#endif /* ENABLE_IP */
>

These two use /* ENABLE_IP */ instead of /* ! ENABLE_IP */

Jano
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list