[libvirt] [PATCH v3 33/48] admin: add ability to connect to the per-driver daemon sockets

Daniel P. Berrangé posted 48 patches 6 years, 6 months ago
There is a newer version of this series
[libvirt] [PATCH v3 33/48] admin: add ability to connect to the per-driver daemon sockets
Posted by Daniel P. Berrangé 6 years, 6 months ago
The admin client now supports addressing the per-driver daemons using
the obvious URI schemes for each daemon. eg virtqemud:///system
virtqemud:///session, etc.

Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libvirt-admin.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
index 74dedf64d8..fa077d5a46 100644
--- a/src/libvirt-admin.c
+++ b/src/libvirt-admin.c
@@ -36,10 +36,6 @@
 
 #define VIR_FROM_THIS VIR_FROM_ADMIN
 
-#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");
 
@@ -127,27 +123,29 @@ getSocketPath(virURIPtr uri)
     }
 
     if (!sock_path) {
-        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 if (STREQ_NULLABLE(uri->scheme, "virtlockd")) {
-            sockbase = VIRTLOCKD_ADMIN_SOCK_NAME;
-        } else {
+        bool legacy = false;
+        if (!uri->scheme) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           "%s", _("No URI scheme specified"));
+            goto error;
+        }
+        if (STREQ(uri->scheme, "libvirtd")) {
+            legacy = true;
+        } else if (!STRPREFIX(uri->scheme, "virt")) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("Unsupported URI scheme '%s'"),
-                           NULLSTR(uri->scheme));
+                           uri->scheme);
             goto error;
         }
 
         if (STREQ_NULLABLE(uri->path, "/system")) {
-            if (virAsprintf(&sock_path, LOCALSTATEDIR "/run/libvirt/%s",
-                            sockbase) < 0)
+            if (virAsprintf(&sock_path, "%s/run/libvirt/%s-admin-sock",
+                            LOCALSTATEDIR,
+                            legacy ? "libvirt" : uri->scheme) < 0)
                 goto error;
         } else if (STREQ_NULLABLE(uri->path, "/session")) {
-            if (!rundir || virAsprintf(&sock_path, "%s/%s", rundir,
-                                       sockbase) < 0)
+            if (!rundir || virAsprintf(&sock_path, "%s/%s-admin-sock", rundir,
+                                       legacy ? "libvirt" : uri->scheme) < 0)
                 goto error;
         } else {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 33/48] admin: add ability to connect to the per-driver daemon sockets
Posted by Andrea Bolognani 6 years, 6 months ago
On Mon, 2019-07-29 at 18:11 +0100, Daniel P. Berrangé wrote:
> +++ b/src/libvirt-admin.c
> @@ -127,27 +123,29 @@ getSocketPath(virURIPtr uri)
>          if (STREQ_NULLABLE(uri->path, "/system")) {
> -            if (virAsprintf(&sock_path, LOCALSTATEDIR "/run/libvirt/%s",
> -                            sockbase) < 0)
> +            if (virAsprintf(&sock_path, "%s/run/libvirt/%s-admin-sock",
> +                            LOCALSTATEDIR,
> +                            legacy ? "libvirt" : uri->scheme) < 0)
>                  goto error;
>          } else if (STREQ_NULLABLE(uri->path, "/session")) {
> -            if (!rundir || virAsprintf(&sock_path, "%s/%s", rundir,
> -                                       sockbase) < 0)
> +            if (!rundir || virAsprintf(&sock_path, "%s/%s-admin-sock", rundir,
> +                                       legacy ? "libvirt" : uri->scheme) < 0)
>                  goto error;

I'm not sure why I didn't suggest this during the previous review
round, but you could also do something like

  VIR_AUTOFREE(char *) sockbase = NULL;

  /* ... */

  if (legacy) {
      if (VIR_STRDUP(sockbase, "libvirt-admin-sock") < 0)
          goto error;
  } else {
      if (virAsprintf(&sockbase, "%s-admin-sock", uri->scheme) < 0)
          goto error;
  }

or even

  VIR_AUTOFREE(char *) sockbase = NULL;

  /* ... */

  if (virAsprintf(&sockbase, "%s-admin-sock",
                  legacy ? "libvirt" : uri->scheme) < 0) {
      goto error;
  }

and then keep using sockbase when building sock_path, getting rid of
the duplicated Elvis operator in the process.

But either version works, really.

-- 
Andrea Bolognani / Red Hat / Virtualization

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