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)
signature.asc
Description: PGP signature
-- libvir-list mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvir-list
