[libvirt] [PATCH v4 3/4] virStorageFileDeinit: don't free metadata used for storage driver access

[Prasanna Kalever]

On Wed, Dec 7, 2016 at 7:39 PM, Peter Krempa <pkrempa at redhat.com> wrote:
> On Tue, Dec 06, 2016 at 22:52:00 +0530, Prasanna Kumar Kalever wrote:
>> Let the metadata for storage driver access to remote and local volumes
>> be cleaned by its respective driver *Deinit methods.
>>
>> This will be used in the next patch, which will implement a connection
>> cache for/in gluster protocol driver.
>>
>> Signed-off-by: Prasanna Kumar Kalever <prasanna.kalever at redhat.com>
>> ---
>>  src/storage/storage_backend_fs.c      | 2 ++
>>  src/storage/storage_backend_gluster.c | 3 ++-
>>  src/storage/storage_driver.c          | 5 +----
>>  3 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
>> index de0e8d5..0e03e06 100644
>> --- a/src/storage/storage_backend_fs.c
>> +++ b/src/storage/storage_backend_fs.c
>> @@ -1488,6 +1488,8 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
>>
>>      VIR_FREE(priv->canonpath);
>>      VIR_FREE(priv);
>> +
>> +    VIR_FREE(src->drv);
>
> Hmm, so this is a global (for the storage driver) data structure for the
> initialized file. It contains the function pointers and a pointer to
> private data.
>
> Even after this patch it's still initialized in the global function.
>
> Also the src->drv object describes the state of a given
> virStorageSource.
>
> I thought you wanted to get rid of this construct and replace it by
> something else, but looking at the following patches it's not the case.

Something like ?
Apologies, I don't understand your intention here, with some clues and
if it is needed I can make it better.

>
> As of such, this change does not make sense. virStorageFileInit(As)
> allocates this structure, so virStorageFileDeinit should free it.

Okay, will discard this patch

Thanks,
--
Prasanna

>
> NACK