On Wed, Dec 7, 2016 at 7:39 PM, Peter Krempa <pkrempa@xxxxxxxxxx> 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@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. 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list