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?
>
>
> You should have reported this problem. There was a bug in the dynamic
> dispatch that resulted in dynamic dispatch errors when two types were
> not direct ancestors.
HostDevice and
HostScsiDisk are not directly
> related, because
ScsiLun sits in between.
>
> This patch should fix this problem. Could you verify this?
>
https://www.redhat.com/archives/libvir-list/2012-October/msg00197.html
[AB] Certainly I would incorporate the changes. Thanks for the fix!
> Finally, make sure to run make syntax-check and ensure that it passes.
>
I am working on getting the next version with all proposed changes (including splitting
this massive diff into two parts). Sorry for late replies but lately not getting much time
to finish up this work.
Thanks!
Ata