From 4aa36a68a6bdb9d085db16fd6ddeb7f4458ca8b0 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Fri, 23 Apr 2010 11:57:16 -0400 Subject: [PATCH] Fix a memory leak in the snapshot code in libvirtd. While running libvirtd under valgrind and doing some snapshot testing I noticed that we would always leak a connection reference. The problem was actually that we were leaking a domain reference in the libvirtd remote snapshot code, which was in turn causing a leaked connection reference. Fix the situation by explicitly taking and dropping a domain reference where we need it. Signed-off-by: Chris Lalancette --- daemon/remote.c | 107 +++++++++++++++++++++++++++++------------------- 1 file changed, 65 insertions(+), 42 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 738799cb78..98abd33f51 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -68,7 +68,7 @@ static virStoragePoolPtr get_nonnull_storage_pool (virConnectPtr conn, remote_no static virStorageVolPtr get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol); static virSecretPtr get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret); static virNWFilterPtr get_nonnull_nwfilter (virConnectPtr conn, remote_nonnull_nwfilter nwfilter); -static virDomainSnapshotPtr get_nonnull_domain_snapshot (virConnectPtr conn, remote_nonnull_domain_snapshot snapshot); +static virDomainSnapshotPtr get_nonnull_domain_snapshot (virDomainPtr domain, remote_nonnull_domain_snapshot snapshot); static void make_nonnull_domain (remote_nonnull_domain *dom_dst, virDomainPtr dom_src); static void make_nonnull_network (remote_nonnull_network *net_dst, virNetworkPtr net_src); static void make_nonnull_interface (remote_nonnull_interface *interface_dst, virInterfacePtr interface_src); @@ -5905,25 +5905,34 @@ remoteDispatchDomainSnapshotDumpXml (struct qemud_server *server ATTRIBUTE_UNUSE remote_domain_snapshot_dump_xml_args *args, remote_domain_snapshot_dump_xml_ret *ret) { - virDomainSnapshotPtr snapshot; + virDomainPtr domain = NULL; + virDomainSnapshotPtr snapshot = NULL; + int rc = -1; - snapshot = get_nonnull_domain_snapshot(conn, args->snap); - if (snapshot == NULL) { - remoteDispatchConnError(rerr, conn); - return -1; - } + domain = get_nonnull_domain(conn, args->snap.domain); + if (domain == NULL) + goto cleanup; + + snapshot = get_nonnull_domain_snapshot(domain, args->snap); + if (snapshot == NULL) + goto cleanup; /* remoteDispatchClientRequest will free this. */ ret->xml = virDomainSnapshotGetXMLDesc(snapshot, args->flags); - if (!ret->xml) { + if (!ret->xml) + goto cleanup; + + rc = 0; + +cleanup: + if (snapshot) virDomainSnapshotFree(snapshot); + if (domain) + virDomainFree(domain); + if (rc < 0) remoteDispatchConnError(rerr, conn); - return -1; - } - virDomainSnapshotFree(snapshot); - - return 0; + return rc; } static int @@ -6108,23 +6117,32 @@ remoteDispatchDomainRevertToSnapshot (struct qemud_server *server ATTRIBUTE_UNUS remote_domain_revert_to_snapshot_args *args, void *ret ATTRIBUTE_UNUSED) { - virDomainSnapshotPtr snapshot; + virDomainPtr domain = NULL; + virDomainSnapshotPtr snapshot = NULL; + int rc = -1; - snapshot = get_nonnull_domain_snapshot(conn, args->snap); - if (snapshot == NULL) { - remoteDispatchConnError(rerr, conn); - return -1; - } + domain = get_nonnull_domain(conn, args->snap.domain); + if (domain == NULL) + goto cleanup; - if (virDomainRevertToSnapshot(snapshot, args->flags) == -1) { + snapshot = get_nonnull_domain_snapshot(domain, args->snap); + if (snapshot == NULL) + goto cleanup; + + if (virDomainRevertToSnapshot(snapshot, args->flags) == -1) + goto cleanup; + + rc = 0; + +cleanup: + if (snapshot) virDomainSnapshotFree(snapshot); + if (domain) + virDomainFree(domain); + if (rc < 0) remoteDispatchConnError(rerr, conn); - return -1; - } - virDomainSnapshotFree(snapshot); - - return 0; + return rc; } static int @@ -6136,23 +6154,32 @@ remoteDispatchDomainSnapshotDelete (struct qemud_server *server ATTRIBUTE_UNUSED remote_domain_snapshot_delete_args *args, void *ret ATTRIBUTE_UNUSED) { - virDomainSnapshotPtr snapshot; + virDomainPtr domain = NULL; + virDomainSnapshotPtr snapshot = NULL; + int rc = -1; - snapshot = get_nonnull_domain_snapshot(conn, args->snap); - if (snapshot == NULL) { - remoteDispatchConnError(rerr, conn); - return -1; - } + domain = get_nonnull_domain(conn, args->snap.domain); + if (domain == NULL) + goto cleanup; - if (virDomainSnapshotDelete(snapshot, args->flags) == -1) { + snapshot = get_nonnull_domain_snapshot(domain, args->snap); + if (snapshot == NULL) + goto cleanup; + + if (virDomainSnapshotDelete(snapshot, args->flags) == -1) + goto cleanup; + + rc = 0; + +cleanup: + if (snapshot) virDomainSnapshotFree(snapshot); + if (domain) + virDomainFree(domain); + if (rc < 0) remoteDispatchConnError(rerr, conn); - return -1; - } - virDomainSnapshotFree(snapshot); - - return 0; + return rc; } @@ -6466,12 +6493,8 @@ get_nonnull_nwfilter (virConnectPtr conn, remote_nonnull_nwfilter nwfilter) } static virDomainSnapshotPtr -get_nonnull_domain_snapshot (virConnectPtr conn, remote_nonnull_domain_snapshot snapshot) +get_nonnull_domain_snapshot (virDomainPtr domain, remote_nonnull_domain_snapshot snapshot) { - virDomainPtr domain; - domain = get_nonnull_domain(conn, snapshot.domain); - if (domain == NULL) - return NULL; return virGetDomainSnapshot(domain, snapshot.name); }