On Tue, Nov 14, 2006 at 03:34:05PM +0100, michel.ponceau@xxxxxxxx wrote:
> This patch adds to Libvirt-0.1.8 the two functions described in my
> preceding mail. It has been tested in our Bull environment.
There have been no negative comments to the interfaces proposed, so
I guess we can assume that is okay, I'm still a bit vary of using XML
for the Detach operation, but this avoid having to add the concept of
device object at the API level right now, and keep things a bit simpler.
Okay, here is a few remarks on the content of the patch:
1/ DOS end of lines in the patch, in general it's safer to
do the diffs and code changes on a Unix box to avoid troubles :-)
2/ The public functions call directly the xenDaemon... implementation
routines, that's not proper, everything need to go though the driver
indirection, i.e. the driver structures need to be extended and
the new functions need to be registered that way
3/ // type of code comments are banned in my code, sorry :-)
4/ virDomainXMLDevID() seems a bit weak, for example it seems to work
only on disk registered though a physical device, though I think
there is a need for (un)registering file based domU devices. For
vif I wonder if the test on the mac address is really the only
way to select the proper device, that's probably sufficient.
5/ virDomainXMLDevID does direct acces to xs_directory() and xs_read(),
that's not proper it should not call xenStore directly that need to
be cleaned up, define one high level function exported from
xs_internal.[ch] and call that but no direct access should be done
that leads to unmaintainable code.
and a couple of tiny things left and right, which I can take care of.
Basically 1/ 2/ 3/ are not blockers, I can fix those, for 4/ the limitations
need at least to be documented, better fixed. 5/ really need to be fixed.
I'm not against starting to work now to integrate that patch but I need
5 (and preferably 4/) to be fixed, if you agree I will work on the other
points in parallel,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard@xxxxxxxxxx | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/