2012/10/15 Ata Bohra <ata.husain@xxxxxxxxxxx>: > Sorry for late reply! > > Thanks Mathias for the review comments. > Please see inline. >> Date: Sat, 6 Oct 2012 18:58:21 +0200 > >> Subject: Re: [PATCH] Refactor ESX storage driver and add iSCSI >> support >> From: matthias.bolte@xxxxxxxxxxxxxx >> To: ata.husain@xxxxxxxxxxx >> CC: libvir-list@xxxxxxxxxx >> >> Finally, here's the second part of my review. >> >> Almost all functions call esxStorageGetBackendDriver, so this approach >> slows down the dirver usage. A better appraoch would be to store a >> pointer to the backend with the virStoragePool and virStorageVol >> objects, so the overhead of calling esxStorageGetBackendDriver for >> each operation can be avoided. >> >> Currently virStoragePool and virStorageVol objects don't allow to >> store a privateData pointer, so we'll need to discuss/implement this >> first: >> >> https://www.redhat.com/archives/libvir-list/2012-October/msg00196.html > [AB]: > As per Daniels response, it seems he is OK with the approach proposed by > you. > I would modify the code to do following: > Modify internal _virStoragePool/Vol to store "backend driver pointer" and a > free > function. > virGetStoragePool/Vol will assign the user passed value to these variables. > virStoragePoolDispose/VolDispose would use the free function to cleanup. > > I need to ask one question, I did not completly understood the role of > "freefunction" > in above proposal? As I understand backend driver in this case does not > perform > open/close operation, so does it means they can be NULL for this particular > case? The privateData pointer in _virStoragePool/Vol structs is meant for generic use. The free function is there to allow use cases were you need to do some cleanup on the privateData when a _virStoragePool/Vol struct is freed. This is a general concept that is used in multiple places in libvirt. You're correct, in case of the ESX storage driver the free function can be NULL because we don't need to cleanup the backend driver pointer stored there when a _virStoragePool/Vol struct is free. -- Matthias Bolte http://photron.blogspot.com -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list