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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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

NACK

Attachment: signature.asc
Description: PGP signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]
  Powered by Linux