Re: [libvirt] [PATCHv6 1/5] Add a hostdev PCI backend type
Laine Stump wrote:
On 11/22/2013 11:08 PM, Jim Fehlig wrote:
> Chunyan Liu wrote:
>
>> Add VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN. For legacy xen, it will use
"pciback" as
>> stub driver.
>>
>>
> Sorry for the long delay in helping review these patches. I finally have
> some time to work on libvirt :).
>
> With the pending release, this series will have to wait for the next
> cycle, but hopefully we can get it in early for prolonged testing. It
> plugs a big hole in the libxl dirver, so thanks for your work and
> perseverance :).
>
> I've applied your patches to latest git master and testing looks good so
> far, after fixing a small issue in 2/5. See the individual patches for
> further comments.
>
The part that has made me nervous about this series from the beginning
is that it is moving the implementation of some pretty hairy code from
qemu-specific to a general library while that code has itself been
getting fairly frequently bugfix tweaks.
When Chunyan originally submitted the patch adding PCI passthrough
support in the libxl driver, it was noted that a lot of the code is not
qemu-specific, based on the amount of copy-and-paste, and hence the
request for common code :-). Do you think a lot of the bugs in this code
are specific to the qemu driver? I don't have a feel for the number of
common vs hypervisor-specific bugs.
Fortunately we now have several
more unit tests on this code, but before pushing we still should do a
bit of extra due diligence to make sure that no recent bugfixes to the
qemu-specific version of the code are missing in the new versions.
Agreed. I'd suggest unit tests for the virhostdev interface introduced
in 2/5.
Chunyan, tests/virpcitest.c should provide some ideas for virhostdev
unit tests.
(BTW,
for this reason I think it is a very good idea to switch qemu and lxc
over to this new library immediately, rather than waiting as was done in
an earlier version of the patch series.)
I think her original post was only an RFC to get consensus on a
virhostdev interface, one which hopefully prevents hypervisor-specific
code from trickling into the common code. But definitely agree that the
qemu and lxc drivers should switch immediately.
Regards,
Jim