[libvirt] [PATCH v2 6/9] admin: Add URI support and introduce virAdmGetDefaultURI
Martin Kletzander
mkletzan at redhat.com
Tue Nov 3 19:40:09 UTC 2015
On Fri, Oct 16, 2015 at 08:12:23PM +0200, Erik Skultety wrote:
>Since virt-admin should be able to connect to various admin servers
>on hosted different daemons, we need to provide URI support to
>libvirt-admin.
>---
> include/libvirt/libvirt-admin.h | 2 +
> src/datatypes.c | 2 +
> src/datatypes.h | 1 +
> src/libvirt-admin.c | 132 +++++++++++++++++++++++++++++++---------
> src/libvirt_admin_public.syms | 1 +
> tools/virt-admin.c | 39 ++++++++++++
> 6 files changed, 147 insertions(+), 30 deletions(-)
>
>diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
>index 72671c6..aae10b4 100644
>--- a/include/libvirt/libvirt-admin.h
>+++ b/include/libvirt/libvirt-admin.h
>@@ -54,6 +54,8 @@ int virAdmConnectClose(virAdmConnectPtr conn);
> int virAdmConnectRef(virAdmConnectPtr conn);
> int virAdmConnectIsAlive(virAdmConnectPtr conn);
>
>+char * virAdmConnectGetURI(virAdmConnectPtr conn);
extra space after asterisk
>+
> # ifdef __cplusplus
> }
> # endif
>diff --git a/src/datatypes.c b/src/datatypes.c
>index 12bcfc1..aeac301 100644
>--- a/src/datatypes.c
>+++ b/src/datatypes.c
>@@ -832,4 +832,6 @@ virAdmConnectDispose(void *obj)
>
> if (conn->privateDataFreeFunc)
> conn->privateDataFreeFunc(conn);
>+
>+ virURIFree(conn->uri);
> }
>diff --git a/src/datatypes.h b/src/datatypes.h
>index be108fe..95aa49e 100644
>--- a/src/datatypes.h
>+++ b/src/datatypes.h
>@@ -397,6 +397,7 @@ struct _virConnect {
> */
> struct _virAdmConnect {
> virObjectLockable object;
>+ virURIPtr uri;
extra whitespace
>
> void *privateData;
> virFreeCallback privateDataFreeFunc;
>diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
>index cdc712c..9fc8393 100644
>--- a/src/libvirt-admin.c
>+++ b/src/libvirt-admin.c
>@@ -27,6 +27,7 @@
> #include "configmake.h"
>
> #include "viralloc.h"
>+#include "virconf.h"
> #include "virlog.h"
> #include "virnetclient.h"
> #include "virobject.h"
>@@ -95,60 +96,54 @@ virAdmInitialize(void)
> }
>
> static char *
>-getSocketPath(const char *name)
>+getSocketPath(virURIPtr uri)
> {
> char *rundir = virGetUserRuntimeDirectory();
> char *sock_path = NULL;
> size_t i = 0;
>- virURIPtr uri = NULL;
>
>- if (name) {
>- if (!(uri = virURIParse(name)))
>- goto error;
>+ if (!uri)
>+ goto cleanup;
>
>- if (STRNEQ(uri->scheme, "admin") ||
>- uri->server || uri->user || uri->fragment) {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>- _("Invalid connection name '%s'"), name);
>- goto error;
>- }
>
>- for (i = 0; i < uri->paramsCount; i++) {
>- virURIParamPtr param = &uri->params[i];
>+ for (i = 0; i < uri->paramsCount; i++) {
>+ virURIParamPtr param = &uri->params[i];
>
>- if (STREQ(param->name, "socket")) {
>- VIR_FREE(sock_path);
>- if (VIR_STRDUP(sock_path, param->value) < 0)
>- goto error;
>- } else {
>- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>- _("Unknown URI parameter '%s'"), param->name);
>+ if (STREQ(param->name, "socket")) {
>+ VIR_FREE(sock_path);
>+ if (VIR_STRDUP(sock_path, param->value) < 0)
> goto error;
>- }
>+ } else {
>+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+ _("Unknown URI parameter '%s'"), param->name);
>+ goto error;
> }
> }
>
> if (!sock_path) {
>- if (!uri || !uri->path || STREQ(uri->path, "/system")) {
>+ if (STRNEQ(uri->scheme, "libvirtd")) {
>+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>+ _("Unsupported URI scheme '%s'"),
>+ uri->scheme);
>+ goto error;
>+ }
>+ if (STREQ_NULLABLE(uri->path, "/system")) {
> if (VIR_STRDUP(sock_path, LIBVIRTD_ADMIN_UNIX_SOCKET) < 0)
> goto error;
> } else if (STREQ_NULLABLE(uri->path, "/session")) {
>- if (!rundir)
>- goto error;
>-
>- if (virAsprintf(&sock_path,
>- "%s%s", rundir, LIBVIRTD_ADMIN_SOCK_NAME) < 0)
>+ if (!rundir && virAsprintf(&sock_path, "%s%s", rundir,
>+ LIBVIRTD_ADMIN_SOCK_NAME) < 0)
> goto error;
This is the reason why I kept the conditions separated. So it's clear
to read. You are changing the logic here, what you meant is to use ||
instead of &&. Either fix it to || or leave it unchanged.
> } else {
> virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>- _("Invalid URI path '%s'"), uri->path);
>+ _("Invalid URI path '%s', try '/system'"),
>+ uri->path ? uri->path : "");
> goto error;
> }
> }
>
> cleanup:
> VIR_FREE(rundir);
>- virURIFree(uri);
> return sock_path;
>
> error:
>@@ -156,6 +151,40 @@ getSocketPath(const char *name)
> goto cleanup;
> }
>
>+static int
>+virAdmGetDefaultURI(virConfPtr conf, virURIPtr *uri)
>+{
>+ virConfValuePtr value = NULL;
>+ const char *uristr = NULL;
>+
>+ uristr = virGetEnvBlockSUID("LIBVIRT_ADMIN_DEFAULT_URI");
I don't see the point for the 'BlockSUID()' version, did I miss
something?
>+ if (uristr && *uristr) {
>+ VIR_DEBUG("Using LIBVIRT_ADMIN_DEFAULT_URI '%s'", uristr);
>+ } else if ((value = virConfGetValue(conf, "uri_default_admin"))) {
I would use 'admin' as a prefix, so we can differentiate any future
config options. So either uri_admin_default or admin_uri_default.
>+ if (value->type != VIR_CONF_STRING) {
>+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+ _("Expected a string for 'uri_default_admin' config "
>+ "parameter"));
>+ return -1;
>+ }
>+
>+ VIR_DEBUG("Using config file uri '%s'", value->str);
>+ uristr = value->str;
>+ } else {
>+ /* Since we can't probe connecting via any hypervisor driver as libvirt
>+ * does, if no explicit URI was given and neither the environment
>+ * variable, nor the configuration parameter had previously been set,
>+ * we set the default admin server URI to 'libvirtd://system'.
>+ */
>+ uristr = "libvirtd:///system";
>+ }
>+
>+ if (!(*uri = virURIParse(uristr)))
>+ return -1;
>+
>+ return 0;
>+}
>+
> /**
> * virAdmConnectOpen:
> * @name: uri of the daemon to connect to, NULL for default
>@@ -170,6 +199,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> {
> char *sock_path = NULL;
> virAdmConnectPtr conn = NULL;
>+ virConfPtr conf = NULL;
>
> if (virAdmInitialize() < 0)
> goto error;
>@@ -180,7 +210,18 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> if (!(conn = virAdmConnectNew()))
> goto error;
>
>- if (!(sock_path = getSocketPath(name)))
>+ if (virGetLibvirtConfigFile(&conf) < 0)
>+ goto error;
>+
>+ if (!name) {
>+ if (virAdmGetDefaultURI(conf, &conn->uri) < 0)
Are we planning on using the @conf anywhere else? If not, I'd put the
config getting inside this function.
>+ goto error;
>+ } else {
>+ if (!(conn->uri = virURIParse(name)))
Change @name to @uri, so it's clear what it is.
>+ goto error;
>+ }
>+
>+ if (!(sock_path = getSocketPath(conn->uri)))
> goto error;
>
> if (!(conn->privateData = remoteAdminPrivNew(sock_path)))
>@@ -304,3 +345,34 @@ virAdmConnectIsAlive(virAdmConnectPtr conn)
> else
> return 0;
> }
>+
>+/**
>+ * virAdmConnectGetURI:
>+ * @conn: pointer to an admin connection
>+ *
>+ * String returned by this method is normally the same as the string passed
>+ * to the virAdmConnectOpen. Even if NULL was passed to virAdmConnectOpen,
>+ * this method returns a non-null URI string.
>+ *
>+ * Returns an URI string related to the connection or NULL in case of an error.
>+ * Caller is responsible for freeing the string.
>+ */
>+char *
>+virAdmConnectGetURI(virAdmConnectPtr conn)
>+{
>+ char *uri = NULL;
>+ VIR_DEBUG("conn=%p", conn);
>+
>+ virResetLastError();
>+
>+ virCheckAdmConnectReturn(conn, NULL);
>+
>+ if (!(uri = virURIFormat(conn->uri)))
>+ goto error;
>+
>+ return uri;
>+
>+ error:
>+ virDispatchError(NULL);
>+ return uri;
You could save some lines with:
if (!(uri = virURIFormat(conn->uri)))
virDispatchError(NULL);
return uri;
But whatever flows your boat.
>+}
>diff --git a/src/libvirt_admin_public.syms b/src/libvirt_admin_public.syms
>index 16cfd42..069508c 100644
>--- a/src/libvirt_admin_public.syms
>+++ b/src/libvirt_admin_public.syms
>@@ -16,4 +16,5 @@ LIBVIRT_ADMIN_1.3.0 {
> virAdmConnectClose;
> virAdmConnectRef;
> virAdmConnectIsAlive;
>+ virAdmConnectGetURI;
> };
>diff --git a/tools/virt-admin.c b/tools/virt-admin.c
>index 1bc10b0..c00a3b3 100644
>--- a/tools/virt-admin.c
>+++ b/tools/virt-admin.c
>@@ -120,6 +120,39 @@ vshAdmReconnect(vshControl *ctl)
> disconnected = false;
> }
>
>+/*
>+ * 'uri' command
>+ */
>+
>+static const vshCmdInfo info_uri[] = {
>+ {.name = "help",
>+ .data = N_("print the admin server URI")
>+ },
>+ {.name = "desc",
>+ .data = ""
>+ },
>+ {.name = NULL}
>+};
>+
>+static bool
>+cmdURI(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED)
>+{
>+ char *uri;
>+ vshAdmControlPtr priv = ctl->privData;
>+
>+ uri = virAdmConnectGetURI(priv->conn);
>+ if (!uri) {
>+ vshError(ctl, "%s", _("failed to get URI"));
>+ return false;
>+ }
>+
>+ vshPrint(ctl, "%s\n", uri);
>+ VIR_FREE(uri);
>+
>+ return true;
>+}
>+
>+
> /* ---------------
> * Command Connect
> * ---------------
>@@ -454,6 +487,12 @@ static const vshCmdDef vshAdmCmds[] = {
> VSH_CMD_HELP,
> VSH_CMD_PWD,
> VSH_CMD_QUIT,
>+ {.name = "uri",
>+ .handler = cmdURI,
>+ .opts = NULL,
>+ .info = info_uri,
>+ .flags = 0
>+ },
> {.name = "connect",
> .handler = cmdConnect,
> .opts = opts_connect,
>--
>2.4.3
>
>--
>libvir-list mailing list
>libvir-list at redhat.com
>https://www.redhat.com/mailman/listinfo/libvir-list
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151103/5d5e70b4/attachment-0001.sig>
More information about the libvir-list
mailing list