|
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@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? > > > 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
|
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list