On Tue, Nov 14, 2006 at 10:05:47AM -0500, Daniel Veillard wrote: > 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 :-) Okay, I applied the patches selectively and did the changes 1/ to 3/, as a result it changes more files since the driver structure grew. > 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. That still need to be isolated, I added a TODO in src/xml.c to this effect. thanks ! 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/