<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">2014-03-04 0:57 GMT+08:00 Cedric Bosdonnat <span dir="ltr"><<a href="mailto:cbosdonnat@suse.com" target="_blank">cbosdonnat@suse.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hello ChunYan,<br>
<br>
I saw a few minor problems in some patches that made me rebase quite a<br>
lot of other patches in your serie, but otherwise it really looks good<br>
to me.<br>
<br>
Here is a summary of the changes I made or questions I have:<br>
    * Patch 2: Fixed a few remaining changes that broke the build.<br>
      Just remember that having the tree building after each commit<br>
      helps a lot when one later needs to bisect.<br>
    * Patch 4: when is the manager freed?<br></blockquote><div>It's hard to find a specified code point to free the manager, since we<br></div><div>don't know is there anyone else still using it.<br></div><div> </div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
    * Patch 5 & 6: remove the added ATTRIBUTE_UNUSED<br>
    * Patch 9: added { } around if block to match else style.<br>
    * Patch 10: remove added ATTRIBUTE_UNUSED<br>
    * Patch 11: fixed indent function rename<br>
    * Patch 42: Fix comment about qemuPrepareHostdevPCIDevices in<br>
earlier patch<br>
    * Patch 47: remove added ATTRIBUTE_UNUSED<br>
    * Patch 49: Fixed the version in libxl_driver.c as 1.2.2 is out<br>
<br>
The whole updated patch series is sitting here for those wanting to see<br>
the changes applied:<br>
<a href="https://github.com/cbosdo/libvirt/commits/hostdev-passthrough" target="_blank">https://github.com/cbosdo/libvirt/commits/hostdev-passthrough</a><br>
<br>
Of course, I'ld love another pair of sharper eyes to look at the patch<br>
series. I'm not an expert on the hostdev topic ;)<br>
<br>
Kind regards<br>
--<br>
Cedric<br>
<div class="HOEnZb"><div class="h5"><br>
On Sat, 2014-03-01 at 14:28 +0800, Chunyan Liu wrote:<br>
> These patches implements a separate module for hostdev passthrough so that it<br>
> could be shared by different drivers and can maintain a global state of a host<br>
> device.<br>
><br>
> Patches 1~6 are to switch existing qemu and lxc driver to use common library<br>
> lists, so that to maintain a global state of every host device.<br>
><br>
> Patches 7~45 are to extract general code from qemu_hostdev.c piece by piece,<br>
> make them reusable common APIs.<br>
><br>
> Patches 46: unit test for the virhostdev common library<br>
> Patches 47: add a hostdev backend type for xen<br>
> Patches 48: add pci passthrough to libxl driver based on the common library<br>
> Patches 49: change lxc to use common library APIs<br>
><br>
> ---<br>
> changes to v12:<br>
>   * split "add hostdev passthrough common library" patch into small patches<br>
>     for easier review.<br>
>   * fix v12 comments<br>
>   * rebase to libxl changes<br>
><br>
><br>
> Chunyan Liu (49):<br>
>   add 'driver' info to used_by<br>
>   qemu: reuse hostdev interfaces to avoid duplicate<br>
>   qemu: remove functions now used internally only from qemu_hostdev.h<br>
>   add virhostdev files to maintain global state of host devices<br>
>   qemu: use general virhostdev lists instead of its own<br>
>   lxc: use general virhostdev lists instead of its own<br>
>   qemu_hostdev: move cfg->relaxedACS as a flag<br>
>   qemu_hostdev: move ColdBoot as a flag<br>
>   qemu_hostdev: move netconfig file location to virhostdev stateDir<br>
>   extract general code from qemuPrepareHostdevPCIDevices<br>
>   rename qemu*NetConfigRestore/Replace to<br>
>     virHostdevNetConfigRestore/Replace<br>
>   rename qemuGet*PciHostDeviceList to virHostdevGet*PciHostDeviceList<br>
>   pass driver name as a parameter to virHostdevPrepareHostdevPCIDevices<br>
>   extract general code from qemuDomainReAttachHostdevDevices<br>
>   pass driver name as a parameter to virHostdevReAttachPCIDevices<br>
>   rename qemuReAttachPciDevice to virHostdevReAttachPciDevice<br>
>   move virHostdevPrepare(ReAttach)PCIDevices to virhostdev.c<br>
>   extract general code from qemuUpdateActivePciHostdevs<br>
>   extract general code from qemuUpdateActiveUsbHostdevs<br>
>   extract general code from qemuUpdateActiveScsiHostdevs<br>
>   pass driver_name as parameter of virHostdevUpdate*Hostdevs functions<br>
>   move virHostdevUpdate* functions to virhostdev.c<br>
>   qemuPrepareUSBDevices: code adjustment for extracting general code<br>
>   extract general code from qemuPrepareHostUSBDevices<br>
>   rename qemu*USBDevices to virHostdev*USBDevices<br>
>   pass driver name to virHostdevPrepareUSBDevices<br>
>   move virHostdevPrepareHostUSBDevices to virhostdev.c<br>
>   extract general code from qemuPrepareHostSCSIDevices<br>
>   pass driver name as parameter to virHostdevPrepareSCSIDevices<br>
>   move virHostdevPrepareHostSCSIDevices to virhostdev.c<br>
>   extract general code from qemuDomainReAttachHostUsbDevices<br>
>   pass driver name as paramter to virHostdevReAttachUsbHostdevs<br>
>   move virHostdevDomainReAttachHostUsbDevices to virhostdev.c<br>
>   extract general code from qemuDomainReAttachHostScsiDevices<br>
>   pass driver name as parameter to virHostdevReAttachScciHostdevs<br>
>   move virHostdevReAttachHostScsiDevices to virhostdev.c<br>
>   extract general code of NodeDeviceDetach<br>
>   extract general code of NodeDeviceReAttach<br>
>   extract general code of NodeDeviceReset<br>
>   move virHostdevNodeDevice* to virhostdev.c<br>
>   improve parameter name to let it more meaningful<br>
>   rename some function names to keep consistency<br>
>   improve virHostdevUpdate* parameters to make it more widely used<br>
>   add 3 wrapper functions for prepare/reattach/update domain hostdevs<br>
>   add parameter checks to common interfaces<br>
>   add unit test for new virhostdev common library<br>
>   change lxc_hostdev.c to use virhostdev common library APIs<br>
>   add hostdev pci backend type for xen<br>
>   add pci passthrough to libxl driver<br>
><br>
>  .gitignore                    |    1 +<br>
>  docs/schemas/domaincommon.rng |    1 +<br>
>  po/POTFILES.in                |    1 +<br>
>  src/Makefile.am               |    1 +<br>
>  src/conf/domain_conf.c        |    3 +-<br>
>  src/conf/domain_conf.h        |    1 +<br>
>  src/libvirt_private.syms      |   19 +<br>
>  src/libxl/libxl_conf.c        |   63 ++<br>
>  src/libxl/libxl_conf.h        |    4 +<br>
>  src/libxl/libxl_domain.c      |    9 +<br>
>  src/libxl/libxl_driver.c      |  447 +++++++++++-<br>
>  src/lxc/lxc_conf.h            |    4 -<br>
>  src/lxc/lxc_driver.c          |   17 +-<br>
>  src/lxc/lxc_hostdev.c         |  315 +--------<br>
>  src/qemu/qemu_command.c       |    3 +-<br>
>  src/qemu/qemu_conf.h          |   10 +-<br>
>  src/qemu/qemu_driver.c        |   88 +--<br>
>  src/qemu/qemu_hostdev.c       | 1220 ++-----------------------------<br>
>  src/qemu/qemu_hostdev.h       |   27 +-<br>
>  src/qemu/qemu_hotplug.c       |   77 +--<br>
>  src/qemu/qemu_process.c       |    8 +-<br>
>  src/util/virhostdev.c         | 1621 +++++++++++++++++++++++++++++++++++++++++<br>
>  src/util/virhostdev.h         |  140 ++++<br>
>  src/util/virpci.c             |   31 +-<br>
>  src/util/virpci.h             |    9 +-<br>
>  src/util/virscsi.c            |   32 +-<br>
>  src/util/virscsi.h            |    7 +-<br>
>  src/util/virusb.c             |   31 +-<br>
>  src/util/virusb.h             |    8 +-<br>
>  tests/Makefile.am             |    5 +<br>
>  tests/virhostdevtest.c        |  507 +++++++++++++<br>
>  tests/virscsitest.c           |    6 +-<br>
>  32 files changed, 3081 insertions(+), 1635 deletions(-)<br>
>  create mode 100644 src/util/virhostdev.c<br>
>  create mode 100644 src/util/virhostdev.h<br>
>  create mode 100644 tests/virhostdevtest.c<br>
><br>
</div></div><span class="HOEnZb"><font color="#888888">> --<br>
> libvir-list mailing list<br>
> <a href="mailto:libvir-list@redhat.com">libvir-list@redhat.com</a><br>
> <a href="https://www.redhat.com/mailman/listinfo/libvir-list" target="_blank">https://www.redhat.com/mailman/listinfo/libvir-list</a><br>
><br>
<br>
<br>
<br>
</font></span></blockquote></div><br></div></div>