Re: [libvirt] [PATCH] Refactor ESX storage driver and add iSCSI support
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(a)googlemail.com
To: ata.husain(a)hotmail.com
CC: libvir-list(a)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 freefunction. 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
splittingthis massive diff into two parts). Sorry for late replies but lately not getting
much timeto finish up this work.
--
Matthias Bolte
http://photron.blogspot.com
Thanks!Ata
Attachments:
- attachment.html (text/html — 6.2 KB)