From c70fe441c47030833541ec9401c9b6729bdae2fe Mon Sep 17 00:00:00 2001

Message-Id: <c70fe441c47030833541ec9401c9b6729bdae2fe@dist-git>

From: "Daniel P. Berrange" <berrange@redhat.com>

Date: Tue, 16 Aug 2016 13:06:02 +0200

Subject: [PATCH] libvirt: convert to typesafe virConf accessors

Signed-off-by: Daniel P. Berrange <berrange@redhat.com>

(cherry picked from commit f5da0d18053376cc2a80e4648c68840b801cba81)

Although the upstream commit applied to downstream cleanly, it had to be

further modified in the way how it retrieves data from our config structure.

The original commit relied on some new accessors (wrappers) added by 6381c89f

which however was not backported. Therefore the old way of retrieving data from

the config structure by virConfGetValue had to be used.

https://bugzilla.redhat.com/show_bug.cgi?id=1367269

Signed-off-by: Erik Skultety <eskultet@redhat.com>

---

 src/libvirt-admin.c | 74 +++++++++++++++++++++++++++++++-------------------

 src/libvirt.c       | 78 +++++++++++++++++++++++++++++++++--------------------

 2 files changed, 96 insertions(+), 56 deletions(-)

diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c

index f07cb10..c485752 100644

--- a/src/libvirt-admin.c

+++ b/src/libvirt-admin.c

@@ -158,35 +158,44 @@ getSocketPath(virURIPtr uri)

     goto cleanup;

 }

-static const char *

-virAdmGetDefaultURI(virConfPtr conf)

+static int

+virAdmGetDefaultURI(virConfPtr conf, char **uristr)

 {

     virConfValuePtr value = NULL;

-    const char *uristr = NULL;

+    const char *defname = virGetEnvAllowSUID("LIBVIRT_ADMIN_DEFAULT_URI");

-    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;

+    if (defname && *defname) {

+        if (VIR_STRDUP(*uristr, defname) < 0)

+            return -1;

+        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 -1;

+            }

+

+            VIR_FREE(*uristr);

+            if (VIR_STRDUP(*uristr, value->str) < 0)

+                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 (*uristr) {

+            VIR_DEBUG("Using config file uri '%s'", *uristr);

+        } 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'.

+             */

+            if (VIR_STRDUP(*uristr, "libvirtd:///system") < 0)

+                return -1;

+        }

     }

-    return uristr;

+    return 0;

 }

 /**

@@ -206,6 +215,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)

     char *alias = NULL;

     virAdmConnectPtr conn = NULL;

     virConfPtr conf = NULL;

+    char *uristr = NULL;

     if (virAdmInitialize() < 0)

         goto error;

@@ -219,14 +229,24 @@ virAdmConnectOpen(const char *name, unsigned int flags)

     if (virConfLoadConfig(&conf, "libvirt-admin.conf") < 0)

         goto error;

-    if (!name && !(name = virAdmGetDefaultURI(conf)))

-        goto error;

+    if (name) {

+        if (VIR_STRDUP(uristr, name) < 0)

+            goto error;

+    } else {

+        if (virAdmGetDefaultURI(conf, &uristr) < 0)

+            goto error;

+    }

     if ((!(flags & VIR_CONNECT_NO_ALIASES) &&

-         virURIResolveAlias(conf, name, &alias) < 0))

+         virURIResolveAlias(conf, uristr, &alias) < 0))

         goto error;

-    if (!(conn->uri = virURIParse(alias ? alias : name)))

+    if (alias) {

+        VIR_FREE(uristr);

+        uristr = alias;

+    }

+

+    if (!(conn->uri = virURIParse(uristr)))

         goto error;

     if (!(sock_path = getSocketPath(conn->uri)))

@@ -242,7 +262,7 @@ virAdmConnectOpen(const char *name, unsigned int flags)

  cleanup:

     VIR_FREE(sock_path);

-    VIR_FREE(alias);

+    VIR_FREE(uristr);

     virConfFree(conf);

     return conn;

diff --git a/src/libvirt.c b/src/libvirt.c

index f26eec4..215701f 100644

--- a/src/libvirt.c

+++ b/src/libvirt.c

@@ -903,22 +903,32 @@ virGetVersion(unsigned long *libVer, const char *type ATTRIBUTE_UNUSED,

 static int

 virConnectGetDefaultURI(virConfPtr conf,

-                        const char **name)

+                        char **name)

 {

     int ret = -1;

     virConfValuePtr value = NULL;

     const char *defname = virGetEnvBlockSUID("LIBVIRT_DEFAULT_URI");

+

     if (defname && *defname) {

         VIR_DEBUG("Using LIBVIRT_DEFAULT_URI '%s'", defname);

-        *name = defname;

-    } else if ((value = virConfGetValue(conf, "uri_default"))) {

-        if (value->type != VIR_CONF_STRING) {

-            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

-                           _("Expected a string for 'uri_default' config parameter"));

+        if (VIR_STRDUP(*name, defname) < 0)

             goto cleanup;

+    } else {

+        if ((value = virConfGetValue(conf, "uri_default"))) {

+            if (value->type != VIR_CONF_STRING) {

+                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",

+                               _("Expected a string for 'uri_default' config "

+                                 "parameter"));

+                goto cleanup;

+            }

+

+            VIR_FREE(*name);

+            if (VIR_STRDUP(*name, value->str) < 0)

+                goto cleanup;

         }

-        VIR_DEBUG("Using config file uri '%s'", value->str);

-        *name = value->str;

+

+        if (*name)

+            VIR_DEBUG("Using config file uri '%s'", *name);

     }

     ret = 0;

@@ -965,6 +975,7 @@ virConnectOpenInternal(const char *name,

     int res;

     virConnectPtr ret;

     virConfPtr conf = NULL;

+    char *uristr = NULL;

     ret = virGetConnect();

     if (ret == NULL)

@@ -982,54 +993,61 @@ virConnectOpenInternal(const char *name,

         goto failed;

     }

+    /* Convert xen -> xen:/// for back compat */

+    if (name && STRCASEEQ(name, "xen"))

+        name = "xen:///";

+

+    /* Convert xen:// -> xen:/// because xmlParseURI cannot parse the

+     * former.  This allows URIs such as xen://localhost to work.

+     */

+    if (name && STREQ(name, "xen://"))

+        name = "xen:///";

+

     /*

      * If no URI is passed, then check for an environment string if not

      * available probe the compiled in drivers to find a default hypervisor

      * if detectable.

      */

-    if (!name &&

-        virConnectGetDefaultURI(conf, &name) < 0)

-        goto failed;

-

     if (name) {

-        char *alias = NULL;

-        /* Convert xen -> xen:/// for back compat */

-        if (STRCASEEQ(name, "xen"))

-            name = "xen:///";

+        if (VIR_STRDUP(uristr, name) < 0)

+            goto failed;

+    } else {

+        if (virConnectGetDefaultURI(conf, &uristr) < 0)

+            goto failed;

+    }

-        /* Convert xen:// -> xen:/// because xmlParseURI cannot parse the

-         * former.  This allows URIs such as xen://localhost to work.

-         */

-        if (STREQ(name, "xen://"))

-            name = "xen:///";

+    if (uristr) {

+        char *alias = NULL;

         if (!(flags & VIR_CONNECT_NO_ALIASES) &&

-            virURIResolveAlias(conf, name, &alias) < 0)

+            virURIResolveAlias(conf, uristr, &alias) < 0)

             goto failed;

-        if (!(ret->uri = virURIParse(alias ? alias : name))) {

+        if (alias) {

+            VIR_FREE(uristr);

+            uristr = alias;

+        }

+

+        if (!(ret->uri = virURIParse(uristr))) {

             VIR_FREE(alias);

             goto failed;

         }

-        VIR_DEBUG("name \"%s\" to URI components:\n"

+        VIR_DEBUG("Split \"%s\" to URI components:\n"

                   "  scheme %s\n"

                   "  server %s\n"

                   "  user %s\n"

                   "  port %d\n"

                   "  path %s",

-                  alias ? alias : name,

+                  uristr,

                   NULLSTR(ret->uri->scheme), NULLSTR(ret->uri->server),

                   NULLSTR(ret->uri->user), ret->uri->port,

                   NULLSTR(ret->uri->path));

-        if (virConnectCheckURIMissingSlash(alias ? alias : name,

+        if (virConnectCheckURIMissingSlash(uristr,

                                            ret->uri) < 0) {

-            VIR_FREE(alias);

             goto failed;

         }

-

-        VIR_FREE(alias);

     } else {

         VIR_DEBUG("no name, allowing driver auto-select");

     }

@@ -1114,10 +1132,12 @@ virConnectOpenInternal(const char *name,

     }

     virConfFree(conf);

+    VIR_FREE(uristr);

     return ret;

  failed:

+    VIR_FREE(uristr);

     virConfFree(conf);

     virObjectUnref(ret);

-- 

2.9.2