[libvirt] [PATCH] storage: Avoid double virCommandFree in virStorageBackendLogicalDeletePool
When logical pool has no PVs associated with itself (user-created), virCommandFree(cmd) is called twice with the same pointer and that causes a segfault in daemon. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Worth v1.0.4 IMHO. --- src/storage/storage_backend_logical.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index bce407f..4cf6647 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -1,7 +1,7 @@ /* * storage_backend_logical.c: storage backend for logical volume handling * - * Copyright (C) 2007-2009, 2011 Red Hat, Inc. + * Copyright (C) 2007-2009, 2011, 2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -667,6 +667,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; virCommandFree(cmd); + cmd = NULL; /* now remove the pv devices and clear them out */ ret = 0; -- 1.8.1.5
On 28.03.2013 17:06, Martin Kletzander wrote:
When logical pool has no PVs associated with itself (user-created), virCommandFree(cmd) is called twice with the same pointer and that causes a segfault in daemon.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Worth v1.0.4 IMHO. --- src/storage/storage_backend_logical.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index bce407f..4cf6647 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -1,7 +1,7 @@ /* * storage_backend_logical.c: storage backend for logical volume handling * - * Copyright (C) 2007-2009, 2011 Red Hat, Inc. + * Copyright (C) 2007-2009, 2011, 2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -667,6 +667,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; virCommandFree(cmd); + cmd = NULL;
/* now remove the pv devices and clear them out */ ret = 0;=
Alternatively, you can just remove a few lines: diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 8cce978..d911cf3 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -667,11 +667,11 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, NULL); if (virCommandRun(cmd, NULL) < 0) goto cleanup; - virCommandFree(cmd); /* now remove the pv devices and clear them out */ ret = 0; for (i = 0 ; i < pool->def->source.ndevice ; i++) { + virCommandFree(cmd); cmd = virCommandNewArgList(PVREMOVE, pool->def->source.devices[i].path, NULL); @@ -679,8 +679,6 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, ret = -1; break; } - virCommandFree(cmd); - cmd = NULL; } cleanup: However, your approach works as well. ACK. Michal
On 03/28/2013 05:13 PM, Michal Privoznik wrote:
On 28.03.2013 17:06, Martin Kletzander wrote:
When logical pool has no PVs associated with itself (user-created), virCommandFree(cmd) is called twice with the same pointer and that causes a segfault in daemon.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- Worth v1.0.4 IMHO. --- src/storage/storage_backend_logical.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index bce407f..4cf6647 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -1,7 +1,7 @@ /* * storage_backend_logical.c: storage backend for logical volume handling * - * Copyright (C) 2007-2009, 2011 Red Hat, Inc. + * Copyright (C) 2007-2009, 2011, 2013 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -667,6 +667,7 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, if (virCommandRun(cmd, NULL) < 0) goto cleanup; virCommandFree(cmd); + cmd = NULL;
/* now remove the pv devices and clear them out */ ret = 0;=
Alternatively, you can just remove a few lines:
diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 8cce978..d911cf3 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -667,11 +667,11 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, NULL); if (virCommandRun(cmd, NULL) < 0) goto cleanup; - virCommandFree(cmd);
/* now remove the pv devices and clear them out */ ret = 0; for (i = 0 ; i < pool->def->source.ndevice ; i++) { + virCommandFree(cmd); cmd = virCommandNewArgList(PVREMOVE, pool->def->source.devices[i].path, NULL); @@ -679,8 +679,6 @@ virStorageBackendLogicalDeletePool(virConnectPtr conn ATTRIBUTE_UNUSED, ret = -1; break; } - virCommandFree(cmd); - cmd = NULL; }
cleanup:
However, your approach works as well. ACK.
Thanks, I pushed my clumsy version as I had it prepared, but feel free to change that. Martin
participants (2)
-
Martin Kletzander -
Michal Privoznik