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

[Michal Privoznik]

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 at 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