[libvirt] [PATCH] openvz: fixed two memory leaks on migration code

--- src/openvz/openvz_driver.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 57b3c22..3147311 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2364,7 +2364,10 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, } done: - virURIFree(uri); + if (!uri_in) + VIR_FREE(hostname); + else + virURIFree(uri); if (vm) virObjectUnlock(vm); return ret; @@ -2385,7 +2388,7 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, virDomainObjPtr vm = NULL; const char *uri_str = NULL; virURIPtr uri = NULL; - virCommandPtr cmd = virCommandNew(VZMIGRATE); + virCommandPtr cmd = NULL; int ret = -1; virCheckFlags(OPENVZ_MIGRATION_FLAGS, -1); @@ -2412,6 +2415,7 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, if (uri == NULL || uri->server == NULL) goto cleanup; + cmd = virCommandNew(VZMIGRATE); if (flags & VIR_MIGRATE_LIVE) virCommandAddArg(cmd, "--live"); virCommandAddArg(cmd, uri->server); -- 1.7.1

Rather sparse commit message. On 16.09.2014 04:22, Hongbin Lu wrote:
--- src/openvz/openvz_driver.c | 8 ++++++-- 1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 57b3c22..3147311 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2364,7 +2364,10 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, }
done: - virURIFree(uri); + if (!uri_in) + VIR_FREE(hostname); + else + virURIFree(uri); if (vm) virObjectUnlock(vm); return ret;
While this is technically correct, I find it hard to see at first glance why hostname can't be freed in case of uri_in != NULL. Moreover - and I should raised that in the previous review I did - hostname is once used as const char * while it may be used as char *. This is inconsistency that I don't like so I'm squashing this in: diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 3147311..6c73eaf 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2286,7 +2286,8 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, const char *uri_in = NULL; virDomainDefPtr def = NULL; virDomainObjPtr vm = NULL; - char *hostname = NULL; + char *my_hostname = NULL; + const char *hostname = NULL; virURIPtr uri = NULL; int ret = -1; @@ -2321,10 +2322,10 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, def = NULL; if (!uri_in) { - if ((hostname = virGetHostname()) == NULL) + if ((my_hostname = virGetHostname()) == NULL) goto error; - if (STRPREFIX(hostname, "localhost")) { + if (STRPREFIX(my_hostname, "localhost")) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("hostname on destination resolved to localhost," " but migration requires an FQDN")); @@ -2364,10 +2365,8 @@ openvzDomainMigratePrepare3Params(virConnectPtr dconn, } done: - if (!uri_in) - VIR_FREE(hostname); - else - virURIFree(uri); + VIR_FREE(my_hostname); + virURIFree(uri); if (vm) virObjectUnlock(vm); return ret;
@@ -2385,7 +2388,7 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, virDomainObjPtr vm = NULL; const char *uri_str = NULL; virURIPtr uri = NULL; - virCommandPtr cmd = virCommandNew(VZMIGRATE); + virCommandPtr cmd = NULL; int ret = -1;
virCheckFlags(OPENVZ_MIGRATION_FLAGS, -1); @@ -2412,6 +2415,7 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, if (uri == NULL || uri->server == NULL) goto cleanup;
+ cmd = virCommandNew(VZMIGRATE); if (flags & VIR_MIGRATE_LIVE) virCommandAddArg(cmd, "--live"); virCommandAddArg(cmd, uri->server);
ACKed, fixed & pushed. Thanks for nailing this down. Michal
participants (2)
-
Hongbin Lu
-
Michal Privoznik