[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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




About your comments on virDomainXMLDevID() added in xml.c :
  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?
  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?

Daniel Veillard <veillard redhat com>

14/11/2006 16:05
Veuillez répondre à veillard

       
        Pour :        michel ponceau bull net
        cc :        libvir-list redhat com
        Objet :        Re: [Libvir] Patch to attach/detach virtual devices on a running domain


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 :-)
  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,

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]