12:04 a.m.
Valgrind reported a couple of memory leaks and jumps conditional on
uninitialized values.
Happy New Year!
Michael Chapman (3):
qemu: do not copy out non-existent block job info
qemu: do not leak NBD disk data in migration cookie
storage: do not leak storage pool XML filename
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_migration.c | 5 +++--
src/storage/storage_driver.c | 10 +++++-----
3 files changed, 10 insertions(+), 9 deletions(-)
--
2.4.3
12:04 a.m.
New subject: [libvirt] [PATCH 1/3] qemu: do not copy out non-existent block job info
Valgrind complained:
==23975== Conditional jump or move depends on uninitialised value(s)
==23975== at 0x22255FA6: qemuDomainGetBlockJobInfo (qemu_driver.c:16538)
==23975== by 0x538E97C: virDomainGetBlockJobInfo (libvirt-domain.c:9685)
==23975== by 0x12F740: remoteDispatchDomainGetBlockJobInfoHelper (remote.c:2834)
==23975== by 0x53FF287: virNetServerProgramDispatch (virnetserverprogram.c:437)
==23975== by 0x540523D: virNetServerProcessMsg (virnetserver.c:135)
==23975== by 0x54052C7: virNetServerHandleJob (virnetserver.c:156)
==23975== by 0x52F515B: virThreadPoolWorker (virthreadpool.c:145)
==23975== by 0x52F4668: virThreadHelper (virthread.c:206)
==23975== by 0x6E08A50: start_thread (in /lib64/libpthread-2.12.so)
==23975== by 0x82BE93C: clone (in /lib64/libc-2.12.so)
==23975==
==23975== Conditional jump or move depends on uninitialised value(s)
==23975== at 0x22255FB4: qemuDomainGetBlockJobInfo (qemu_driver.c:16542)
==23975== by 0x538E97C: virDomainGetBlockJobInfo (libvirt-domain.c:9685)
==23975== by 0x12F740: remoteDispatchDomainGetBlockJobInfoHelper (remote.c:2834)
==23975== by 0x53FF287: virNetServerProgramDispatch (virnetserverprogram.c:437)
==23975== by 0x540523D: virNetServerProcessMsg (virnetserver.c:135)
==23975== by 0x54052C7: virNetServerHandleJob (virnetserver.c:156)
==23975== by 0x52F515B: virThreadPoolWorker (virthreadpool.c:145)
==23975== by 0x52F4668: virThreadHelper (virthread.c:206)
==23975== by 0x6E08A50: start_thread (in /lib64/libpthread-2.12.so)
==23975== by 0x82BE93C: clone (in /lib64/libc-2.12.so)
If no matching block job is found, qemuMonitorGetBlockJobInfo returns 0
and we should not write anything to the caller-supplied
virDomainBlockJobInfo pointer.
Signed-off-by: Michael Chapman <mike(a)very.puzzling.org>
---
src/qemu/qemu_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e8ba3a6..304165c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16527,7 +16527,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
disk->info.alias, &rawInfo);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -1;
- if (ret < 0)
+ if (ret <= 0)
goto endjob;
info->cur = rawInfo.cur;
@@ -16554,7 +16554,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
* we can ignore failure because it is only an optimization. We
* hold the vm lock, so modifying the in-memory representation is
* safe, even if we are a query rather than a modify job. */
- if (ret == 1 && disk->mirror &&
+ if (disk->mirror &&
rawInfo.ready != 0 &&
info->cur == info->end && !disk->mirrorState) {
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
--
2.4.3
12:04 a.m.
New subject: [libvirt] [PATCH 2/3] qemu: do not leak NBD disk data in migration cookie
Valgrind complained:
==18990== 20 (16 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record
188 of 996
==18990== at 0x4A057BB: calloc (vg_replace_malloc.c:593)
==18990== by 0x5292E9B: virAllocN (viralloc.c:191)
==18990== by 0x2221E731: qemuMigrationCookieXMLParseStr (qemu_migration.c:1012)
==18990== by 0x2221F390: qemuMigrationEatCookie (qemu_migration.c:1413)
==18990== by 0x222228CE: qemuMigrationPrepareAny (qemu_migration.c:3463)
==18990== by 0x22224121: qemuMigrationPrepareDirect (qemu_migration.c:3865)
==18990== by 0x22251C25: qemuDomainMigratePrepare3Params (qemu_driver.c:12414)
==18990== by 0x5389EE0: virDomainMigratePrepare3Params (libvirt-domain.c:5107)
==18990== by 0x1278DB: remoteDispatchDomainMigratePrepare3ParamsHelper (remote.c:5425)
==18990== by 0x53FF287: virNetServerProgramDispatch (virnetserverprogram.c:437)
==18990== by 0x540523D: virNetServerProcessMsg (virnetserver.c:135)
==18990== by 0x54052C7: virNetServerHandleJob (virnetserver.c:156)
==18990==
==18990== 20 (16 direct, 4 indirect) bytes in 1 blocks are definitely lost in loss record
189 of 996
==18990== at 0x4A057BB: calloc (vg_replace_malloc.c:593)
==18990== by 0x5292E9B: virAllocN (viralloc.c:191)
==18990== by 0x2221E731: qemuMigrationCookieXMLParseStr (qemu_migration.c:1012)
==18990== by 0x2221F390: qemuMigrationEatCookie (qemu_migration.c:1413)
==18990== by 0x222249D2: qemuMigrationRun (qemu_migration.c:4395)
==18990== by 0x22226365: doNativeMigrate (qemu_migration.c:4693)
==18990== by 0x22228E45: qemuMigrationPerform (qemu_migration.c:5553)
==18990== by 0x2225144B: qemuDomainMigratePerform3Params (qemu_driver.c:12621)
==18990== by 0x539F5D8: virDomainMigratePerform3Params (libvirt-domain.c:5206)
==18990== by 0x127305: remoteDispatchDomainMigratePerform3ParamsHelper (remote.c:5557)
==18990== by 0x53FF287: virNetServerProgramDispatch (virnetserverprogram.c:437)
==18990== by 0x540523D: virNetServerProcessMsg (virnetserver.c:135)
If we're replacing the NBD data, it's simplest to free the old object
(including the disk list) and allocate a new one.
Signed-off-by: Michael Chapman <mike(a)very.puzzling.org>
---
src/qemu/qemu_migration.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 4519aef..bb708a3 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -571,8 +571,9 @@ qemuMigrationCookieAddNBD(qemuMigrationCookiePtr mig,
int ret = -1, rc;
/* It is not a bug if there already is a NBD data */
- if (!mig->nbd &&
- VIR_ALLOC(mig->nbd) < 0)
+ qemuMigrationCookieNBDFree(mig->nbd);
+
+ if (VIR_ALLOC(mig->nbd) < 0)
return -1;
if (vm->def->ndisks &&
--
2.4.3
12:04 a.m.
New subject: [libvirt] [PATCH 3/3] storage: do not leak storage pool XML filename
Valgrind complained:
==28277== 38 bytes in 1 blocks are definitely lost in loss record 298 of 957
==28277== at 0x4A06A2E: malloc (vg_replace_malloc.c:270)
==28277== by 0x82D7F57: __vasprintf_chk (in /lib64/libc-2.12.so)
==28277== by 0x52EF16A: virVasprintfInternal (stdio2.h:199)
==28277== by 0x52EF25C: virAsprintfInternal (virstring.c:514)
==28277== by 0x52B1FA9: virFileBuildPath (virfile.c:2831)
==28277== by 0x19B1947C: storageDriverAutostart (storage_driver.c:191)
==28277== by 0x19B196A7: storageStateAutoStart (storage_driver.c:307)
==28277== by 0x538527E: virStateInitialize (libvirt.c:793)
==28277== by 0x11D7CF: daemonRunStateInit (libvirtd.c:947)
==28277== by 0x52F4694: virThreadHelper (virthread.c:206)
==28277== by 0x6E08A50: start_thread (in /lib64/libpthread-2.12.so)
==28277== by 0x82BE93C: clone (in /lib64/libc-2.12.so)
Signed-off-by: Michael Chapman <mike(a)very.puzzling.org>
---
src/storage/storage_driver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 0bb577f..c8f259e 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -151,7 +151,6 @@ static void
storageDriverAutostart(void)
{
size_t i;
- char *stateFile = NULL;
virConnectPtr conn = NULL;
/* XXX Remove hardcoding of QEMU URI */
@@ -187,6 +186,8 @@ storageDriverAutostart(void)
}
if (started) {
+ char *stateFile;
+
virStoragePoolObjClearVols(pool);
stateFile = virFileBuildPath(driver->stateDir,
pool->def->name, ".xml");
@@ -201,11 +202,10 @@ storageDriverAutostart(void)
VIR_ERROR(_("Failed to autostart storage pool '%s':
%s"),
pool->def->name, err ? err->message :
_("no error message found"));
- VIR_FREE(stateFile);
- virStoragePoolObjUnlock(pool);
- continue;
+ } else {
+ pool->active = true;
}
- pool->active = true;
+ VIR_FREE(stateFile);
}
virStoragePoolObjUnlock(pool);
}
--
2.4.3
8:03 a.m.
On 31.12.2015 07:04, Michael Chapman wrote:
Valgrind reported a couple of memory leaks and jumps conditional on
uninitialized values.
Happy New Year!
To you too!
Michael Chapman (3):
qemu: do not copy out non-existent block job info
qemu: do not leak NBD disk data in migration cookie
storage: do not leak storage pool XML filename
src/qemu/qemu_driver.c | 4 ++--
src/qemu/qemu_migration.c | 5 +++--
src/storage/storage_driver.c | 10 +++++-----
3 files changed, 10 insertions(+), 9 deletions(-)
ACKed and pushed.
Michal
8:21 a.m.
On Mon, 2016-01-04 at 15:03 +0100, Michal Privoznik wrote:
> Michael Chapman (3):
> qemu: do not copy out non-existent block job info
> qemu: do not leak NBD disk data in migration cookie
> storage: do not leak storage pool XML filename
>
> src/qemu/qemu_driver.c | 4 ++--
> src/qemu/qemu_migration.c | 5 +++--
> src/storage/storage_driver.c | 10 +++++-----
> 3 files changed, 10 insertions(+), 9 deletions(-)
ACKed and pushed.
I just sent out a follow-up to patch 1/3:
https://www.redhat.com/archives/libvir-list/2016-January/msg00026.html
Cheers.
--
Andrea Bolognani
Software Engineer - Virtualization Team
3362
days inactive
3366
days old
5 comments
3 participants
participants (3)
-
Andrea Bolognani -
Michael Chapman -
Michal Privoznik