Re: [Libvir] Patch to attach/detach virtual devices on a running domain

[Daniel Veillard <veillard redhat com>]

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 bull net 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 redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/