<br><font size=2 face="Courier New">About your comments on virDomainXMLDevID() added in xml.c :</font>
<br><font size=2 face="Courier New"> 4/ For disks, it gets the target dev attribute, attached to a source either physical (if <disk type='block'>, <source dev=...>) or file (if <disk type='file'>, <source file=...>). Do you mean that it does not work in second case?</font>
<br><font size=2 face="Courier New"> 5/ In current xml.c (Libvirt-0.1.8), xs_directory() is directly accessed from virDomainGetXMLDevices() and virDomainGetXMLInterfaces(), xs_read() is directly accessed from virDomainGetXMLDeviceInfo(). Is this to be also changed, and "#include <xs.h>" removed from xml.c?</font>
<br>
<br>
<table width=100%>
<tr valign=top>
<td>
<td><font size=1 face="sans-serif"><b>Daniel Veillard <veillard@redhat.com></b></font>
<p><font size=1 face="sans-serif">14/11/2006 16:05</font>
<br><font size=1 face="sans-serif">Veuillez répondre à veillard</font>
<br>
<td><font size=1 face="Arial"> </font>
<br><font size=1 face="sans-serif"> Pour : michel.ponceau@bull.net</font>
<br><font size=1 face="sans-serif"> cc : libvir-list@redhat.com</font>
<br><font size=1 face="sans-serif"> Objet : Re: [Libvir] Patch to attach/detach virtual devices on a running domain</font></table>
<br>
<br><font size=2 face="Courier New">On Tue, Nov 14, 2006 at 03:34:05PM +0100, michel.ponceau@bull.net wrote:<br>
> This patch adds to Libvirt-0.1.8 the two functions described in my <br>
> preceding mail. It has been tested in our Bull environment.<br>
<br>
There have been no negative comments to the interfaces proposed, so<br>
I guess we can assume that is okay, I'm still a bit vary of using XML<br>
for the Detach operation, but this avoid having to add the concept of<br>
device object at the API level right now, and keep things a bit simpler.<br>
<br>
Okay, here is a few remarks on the content of the patch:<br>
<br>
1/ DOS end of lines in the patch, in general it's safer to<br>
do the diffs and code changes on a Unix box to avoid troubles :-)<br>
2/ The public functions call directly the xenDaemon... implementation<br>
routines, that's not proper, everything need to go though the driver<br>
indirection, i.e. the driver structures need to be extended and<br>
the new functions need to be registered that way<br>
3/ // type of code comments are banned in my code, sorry :-)<br>
4/ virDomainXMLDevID() seems a bit weak, for example it seems to work<br>
only on disk registered though a physical device, though I think<br>
there is a need for (un)registering file based domU devices. For<br>
vif I wonder if the test on the mac address is really the only<br>
way to select the proper device, that's probably sufficient.<br>
5/ virDomainXMLDevID does direct acces to xs_directory() and xs_read(),<br>
that's not proper it should not call xenStore directly that need to<br>
be cleaned up, define one high level function exported from <br>
xs_internal.[ch] and call that but no direct access should be done<br>
that leads to unmaintainable code.<br>
<br>
and a couple of tiny things left and right, which I can take care of.<br>
Basically 1/ 2/ 3/ are not blockers, I can fix those, for 4/ the limitations<br>
need at least to be documented, better fixed. 5/ really need to be fixed.<br>
I'm not against starting to work now to integrate that patch but I need<br>
5 (and preferably 4/) to be fixed, if you agree I will work on the other<br>
points in parallel,<br>
</font>