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

[Martin Kletzander]

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

Thanks, I pushed my clumsy version as I had it prepared, but feel free
to change that.

Martin