Re: [libvirt] [PATCH] storage: Avoid double virCommandFree in virStorageBackendLogicalDeletePool

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(a)redhat.com&gt; > --- > 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.