4:22 a.m.
Fix a memleak and make the code a bit neater.
Peter Krempa (4):
util: typedparam: Introduce virTypedParamListFromParams
virGetConnectGeneric: Fix memleak of 'identparams' when connecting
between split daemons
virIdentityGetParameters: Return 'virTypedParamList'
doRemoteOpen: Refactor control flow
src/driver.c | 8 +++----
src/libvirt_private.syms | 1 +
src/remote/remote_daemon_dispatch.c | 36 ++++++++++++-----------------
src/util/viridentity.c | 15 ++++--------
src/util/viridentity.h | 5 ++--
src/util/virtypedparam.c | 13 +++++++++++
src/util/virtypedparam.h | 4 ++++
7 files changed, 44 insertions(+), 38 deletions(-)
--
2.35.3
4:22 a.m.
New subject: [PATCH 1/4] util: typedparam: Introduce virTypedParamListFromParams
The helper constructs a virTypedParamList from loose params.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/libvirt_private.syms | 1 +
src/util/virtypedparam.c | 13 +++++++++++++
src/util/virtypedparam.h | 4 ++++
3 files changed, 18 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d95c181793..3711e33cf5 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3493,6 +3493,7 @@ virTypedParamListAddString;
virTypedParamListAddUInt;
virTypedParamListAddULLong;
virTypedParamListFree;
+virTypedParamListFromParams;
virTypedParamListStealParams;
virTypedParamsCheck;
virTypedParamsCopy;
diff --git a/src/util/virtypedparam.c b/src/util/virtypedparam.c
index aa6a871049..2d7e4ab354 100644
--- a/src/util/virtypedparam.c
+++ b/src/util/virtypedparam.c
@@ -738,6 +738,19 @@ virTypedParamListStealParams(virTypedParamList *list,
return ret;
}
+virTypedParamList *
+virTypedParamListFromParams(virTypedParameterPtr *params,
+ size_t nparams)
+{
+ virTypedParamList *l = g_new0(virTypedParamList, 1);
+
+ l->par = g_steal_pointer(params);
+ l->npar = nparams;
+ l->par_alloc = nparams;
+
+ return l;
+}
+
static int G_GNUC_PRINTF(2, 0)
virTypedParamSetNameVPrintf(virTypedParameterPtr par,
diff --git a/src/util/virtypedparam.h b/src/util/virtypedparam.h
index f4b3921c38..c4bc58ee8f 100644
--- a/src/util/virtypedparam.h
+++ b/src/util/virtypedparam.h
@@ -138,6 +138,10 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(virTypedParamList,
virTypedParamListFree);
size_t virTypedParamListStealParams(virTypedParamList *list,
virTypedParameterPtr *params);
+virTypedParamList *
+virTypedParamListFromParams(virTypedParameterPtr *params,
+ size_t nparams);
+
int virTypedParamListAddInt(virTypedParamList *list,
int value,
const char *namefmt,
--
2.35.3
4:22 a.m.
New subject: [PATCH 2/4] virGetConnectGeneric: Fix memleak of 'identparams' when connecting between split daemons
The 'identparams' typed parameter list obtained from
virIdentityGetParameters is leaked when called from
'virGetConnectGeneric'.
Use 'virTypedParamListFromParams' to absorb it into a virTypedParamList
which can be autofreed.
Note that the memleak is observable only when running in split-daemon
mode.
Closes: https://gitlab.com/libvirt/libvirt/-/issues/314
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/driver.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/driver.c b/src/driver.c
index 41b4f0055b..d4136c438f 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -36,6 +36,7 @@
#include "viridentity.h"
#include "datatypes.h"
#include "configmake.h"
+#include "virtypedparam.h"
VIR_LOG_INIT("driver");
@@ -159,6 +160,7 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
if (conn->driver->connectSetIdentity != NULL) {
g_autoptr(virIdentity) ident = NULL;
+ g_autoptr(virTypedParamList) paramlist = NULL;
virTypedParameterPtr identparams = NULL;
int nidentparams = 0;
@@ -169,7 +171,9 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
if (virIdentityGetParameters(ident, &identparams, &nidentparams) <
0)
goto error;
- if (virConnectSetIdentity(conn, identparams, nidentparams, 0) < 0)
+ paramlist = virTypedParamListFromParams(&identparams, nidentparams);
+
+ if (virConnectSetIdentity(conn, paramlist->par, paramlist->npar, 0)
< 0)
goto error;
}
}
--
2.35.3
4:22 a.m.
New subject: [PATCH 3/4] virIdentityGetParameters: Return 'virTypedParamList'
Refactor the code to use virTypedParamList which simplifies cleanup.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/driver.c | 10 +++-------
src/remote/remote_daemon_dispatch.c | 8 +++-----
src/util/viridentity.c | 15 +++++----------
src/util/viridentity.h | 5 ++---
4 files changed, 13 insertions(+), 25 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index d4136c438f..cea74bdf95 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -160,20 +160,16 @@ virGetConnectGeneric(virThreadLocal *threadPtr, const char *name)
if (conn->driver->connectSetIdentity != NULL) {
g_autoptr(virIdentity) ident = NULL;
- g_autoptr(virTypedParamList) paramlist = NULL;
- virTypedParameterPtr identparams = NULL;
- int nidentparams = 0;
+ g_autoptr(virTypedParamList) identparams = NULL;
VIR_DEBUG("Attempting to delegate current identity");
if (!(ident = virIdentityGetCurrent()))
goto error;
- if (virIdentityGetParameters(ident, &identparams, &nidentparams) <
0)
+ if (!(identparams = virIdentityGetParameters(ident)))
goto error;
- paramlist = virTypedParamListFromParams(&identparams, nidentparams);
-
- if (virConnectSetIdentity(conn, paramlist->par, paramlist->npar, 0)
< 0)
+ if (virConnectSetIdentity(conn, identparams->par, identparams->npar, 0)
< 0)
goto error;
}
}
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 2463386e39..39953f46cf 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1793,8 +1793,7 @@ remoteOpenConn(const char *uri,
bool preserveIdentity,
virConnectPtr *conn)
{
- virTypedParameterPtr params = NULL;
- int nparams = 0;
+ g_autoptr(virTypedParamList) identparams = NULL;
int ret = -1;
VIR_DEBUG("Getting secondary uri=%s readonly=%d preserveIdent=%d conn=%p",
@@ -1814,7 +1813,7 @@ remoteOpenConn(const char *uri,
if (!(ident = virIdentityGetCurrent()))
return -1;
- if (virIdentityGetParameters(ident, ¶ms, &nparams) < 0)
+ if (!(identparams = virIdentityGetParameters(ident)))
goto error;
}
@@ -1828,7 +1827,7 @@ remoteOpenConn(const char *uri,
VIR_DEBUG("Opened driver %p", *conn);
if (preserveIdentity) {
- if (virConnectSetIdentity(*conn, params, nparams, 0) < 0)
+ if (virConnectSetIdentity(*conn, identparams->par, identparams->npar, 0)
< 0)
goto error;
VIR_DEBUG("Forwarded current identity to secondary driver");
@@ -1836,7 +1835,6 @@ remoteOpenConn(const char *uri,
ret = 0;
cleanup:
- virTypedParamsFree(params, nparams);
return ret;
error:
diff --git a/src/util/viridentity.c b/src/util/viridentity.c
index 70843ecf9f..e3a9cbb661 100644
--- a/src/util/viridentity.c
+++ b/src/util/viridentity.c
@@ -838,17 +838,12 @@ int virIdentitySetParameters(virIdentity *ident,
}
-int virIdentityGetParameters(virIdentity *ident,
- virTypedParameterPtr *params,
- int *nparams)
+virTypedParamList *virIdentityGetParameters(virIdentity *ident)
{
- *params = NULL;
- *nparams = 0;
+ virTypedParameter *tmp = NULL;
- if (virTypedParamsCopy(params, ident->params, ident->nparams) < 0)
- return -1;
-
- *nparams = ident->nparams;
+ if (virTypedParamsCopy(&tmp, ident->params, ident->nparams) < 0)
+ return NULL;
- return 0;
+ return virTypedParamListFromParams(&tmp, ident->nparams);
}
diff --git a/src/util/viridentity.h b/src/util/viridentity.h
index 6da6d0c557..5f87d7268b 100644
--- a/src/util/viridentity.h
+++ b/src/util/viridentity.h
@@ -23,6 +23,7 @@
#include "internal.h"
#include <glib-object.h>
+#include "virtypedparam.h"
#define VIR_TYPE_IDENTITY vir_identity_get_type()
G_DECLARE_FINAL_TYPE(virIdentity, vir_identity, VIR, IDENTITY, GObject);
@@ -88,6 +89,4 @@ int virIdentitySetParameters(virIdentity *ident,
virTypedParameterPtr params,
int nparams);
-int virIdentityGetParameters(virIdentity *ident,
- virTypedParameterPtr *params,
- int *nparams);
+virTypedParamList *virIdentityGetParameters(virIdentity *ident);
--
2.35.3
4:22 a.m.
New subject: [PATCH 4/4] doRemoteOpen: Refactor control flow
Use a temporary variable 'newconn' to hold the newly opened connection
until we are ready to pass it back instead of the original connection.
This way we can avoid complicated 'error'/'cleanup' sections.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 28 ++++++++++++----------------
1 file changed, 12 insertions(+), 16 deletions(-)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 39953f46cf..c1f85925a3 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -1794,7 +1794,7 @@ remoteOpenConn(const char *uri,
virConnectPtr *conn)
{
g_autoptr(virTypedParamList) identparams = NULL;
- int ret = -1;
+ g_autoptr(virConnect) newconn = NULL;
VIR_DEBUG("Getting secondary uri=%s readonly=%d preserveIdent=%d conn=%p",
NULLSTR(uri), readonly, preserveIdentity, conn);
@@ -1814,34 +1814,30 @@ remoteOpenConn(const char *uri,
return -1;
if (!(identparams = virIdentityGetParameters(ident)))
- goto error;
+ return -1;
}
VIR_DEBUG("Opening driver %s", uri);
if (readonly)
- *conn = virConnectOpenReadOnly(uri);
+ newconn = virConnectOpenReadOnly(uri);
else
- *conn = virConnectOpen(uri);
- if (!*conn)
- goto error;
- VIR_DEBUG("Opened driver %p", *conn);
+ newconn = virConnectOpen(uri);
+
+ if (!newconn)
+ return -1;
+
+ VIR_DEBUG("Opened driver %p", newconn);
if (preserveIdentity) {
if (virConnectSetIdentity(*conn, identparams->par, identparams->npar, 0)
< 0)
- goto error;
+ return -1;
VIR_DEBUG("Forwarded current identity to secondary driver");
}
- ret = 0;
- cleanup:
- return ret;
+ *conn = g_steal_pointer(&newconn);
- error:
- if (*conn) {
- g_clear_pointer(conn, virConnectClose);
- }
- goto cleanup;
+ return 0;
}
--
2.35.3
4:29 a.m.
On Wed, May 18, 2022 at 11:22:09AM +0200, Peter Krempa wrote:
Fix a memleak and make the code a bit neater.
Peter Krempa (4):
util: typedparam: Introduce virTypedParamListFromParams
virGetConnectGeneric: Fix memleak of 'identparams' when connecting
between split daemons
virIdentityGetParameters: Return 'virTypedParamList'
doRemoteOpen: Refactor control flow
Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com>
920
days inactive
920
days old
5 comments
2 participants
participants (2)
-
Pavel Hrdina -
Peter Krempa