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

Re: [libvirt] [PATCH] Refactor ESX storage driver and add iSCSI support



2012/10/15 Ata Bohra <ata husain hotmail com>:
> Sorry for late reply!
>
> Thanks Mathias for the review comments.
> Please see inline.
>> Date: Sat, 6 Oct 2012 18:58:21 +0200
>
>> Subject: Re: [libvirt] [PATCH] Refactor ESX storage driver and add iSCSI
>> support
>> From: matthias bolte googlemail com
>> To: ata husain hotmail com
>> CC: libvir-list redhat com
>>
>> 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


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