[libvirt] [PATCH v3 06/11] admin: Add URI support and introduce virAdmGetDefaultURI
Martin Kletzander
mkletzan at redhat.com
Mon Nov 16 16:24:32 UTC 2015
On Fri, Nov 06, 2015 at 12:46:21PM +0100, 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 | 129 ++++++++++++++++++++++++++++++----------
> src/libvirt.conf | 7 ++-
> src/libvirt_admin_public.syms | 1 +
> tools/virt-admin.c | 39 ++++++++++++
> 7 files changed, 146 insertions(+), 35 deletions(-)
>
>diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
>index 72671c6..145720d 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);
>+
> # 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..b16bdb1 100644
>--- a/src/datatypes.h
>+++ b/src/datatypes.h
>@@ -397,6 +397,7 @@ struct _virConnect {
> */
> struct _virAdmConnect {
> virObjectLockable object;
>+ virURIPtr uri;
>
> void *privateData;
> virFreeCallback privateDataFreeFunc;
>diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
>index 1367164..599c30a 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)
Oh, you will hate me for this... I know I made you change the "name"
to "uri" or the other way around or whatever, it doesn't matter. I
just noticed that we're not consistent. I don't care what word we
use, as long as it's consistent. I just see that virt-admin uses
connname (as virsh does) and maybe it looks better considering the
consistency between libvirt and libvirt-admin.
> {
> 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;
> } 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,9 +151,40 @@ getSocketPath(const char *name)
> goto cleanup;
> }
>
>+static const char *
>+virAdmGetDefaultURI(virConfPtr conf)
>+{
>+ virConfValuePtr value = NULL;
>+ const char *uristr = NULL;
>+
>+ uristr = virGetEnvAllowSUID("LIBVIRT_ADMIN_DEFAULT_URI");
>+ if (uristr && *uristr) {
>+ VIR_DEBUG("Using LIBVIRT_ADMIN_DEFAULT_URI '%s'", uristr);
>+ } else if ((value = virConfGetValue(conf, "admin_uri_default"))) {
>+ if (value->type != VIR_CONF_STRING) {
>+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>+ _("Expected a string for 'admin_uri_default' config "
>+ "parameter"));
>+ return NULL;
>+ }
>+
>+ 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";
>+ }
>+
>+ return uristr;
>+}
>+
> /**
> * virAdmConnectOpen:
>- * @name: uri of the daemon to connect to, NULL for default
>+ * @uri: uri of the daemon to connect to, NULL for default
> * @flags: unused, must be 0
> *
> * Opens connection to admin interface of the daemon.
>@@ -166,10 +192,12 @@ getSocketPath(const char *name)
> * Returns @virAdmConnectPtr object or NULL on error
> */
> virAdmConnectPtr
>-virAdmConnectOpen(const char *name, unsigned int flags)
>+virAdmConnectOpen(const char *uri, unsigned int flags)
> {
> char *sock_path = NULL;
>+ const char *default_uri = NULL;
You don't need this...
> virAdmConnectPtr conn = NULL;
>+ virConfPtr conf = NULL;
>
> if (virAdmInitialize() < 0)
> goto error;
>@@ -180,7 +208,16 @@ virAdmConnectOpen(const char *name, unsigned int flags)
> if (!(conn = virAdmConnectNew()))
> goto error;
>
>- if (!(sock_path = getSocketPath(name)))
>+ if (virConfLoadDefault(&conf) < 0)
>+ goto error;
>+
>+ if (!uri && !(default_uri = virAdmGetDefaultURI(conf)))
...you can save the virAdmGetDefaultURI() to @uri...
>+ goto error;
>+
>+ if (!(conn->uri = virURIParse(uri ? uri : default_uri)))
...and get rid of this ternary operator.
ACK with that fixed (and consistency of uri<->name kept ;) ).
also ACK to patches 4 and 5 (mentioning here just to save your inbox)
-------------- 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/20151116/a5dd368b/attachment-0001.sig>
More information about the libvir-list
mailing list