On 12/12/19 4:41 PM, Daniel Henrique Barboza wrote:
On 12/11/19 8:49 PM, Cole Robinson wrote:
On 11/26/19 8:33 AM, Daniel Henrique Barboza wrote:
changes from previous version [1]: [...]
One comment
Daniel Henrique Barboza (6): Introducing new address type='unassigned' for PCI hostdevs qemu: handle unassigned PCI hostdevs in command line and alias
Is there a specific reason to skip alias assign, beside it not being needed for the command line? If it doesn't hurt, maybe we just keep it. I don't have a strong argument for it though. If no one says anything I'll leave it as is
The reason why I was skipping aliases was cosmetic due to QEMU command line. I find it a bit odd that, in a scenario with 4 functions where function 1 is unassigned, QEMU cmd line would have hostdev0, hostdev2 and hostdev3, because 'hostdev1' alias got assigned to the unassigned function '1'
I don't have any strong feelings about this though. We can keep giving aliases or all functions, regardless of whether they are being assigned to the guest or not. Unit tests will need to be adjusted though.
virhostdev.c: check all IOMMU devs in virHostdevPreparePCIDevices formatdomain.html.in: document <address type='unassigned'/>
For the first 4 I'll give it a few days and push on Friday if no one complains.
For the last two:
utils: PCI multifunction detection helpers domain_conf.c: don't allow function zero to be unassigned
The domain_conf.c additions should go into the virDomainHostdevDefValidate. But this validation check seems to actually read from host PCI space. I'm not sure if that's a good idea to do for every XML parse? What's the failure scenario without this error message? Does it fail in an obvious way? If so, maybe it's better to side step the issue and just let it fail if it's a valid configuration.
This is a strange scenario TBH. I discussed it a bit with Alex Williamson in the v2 of this series. At first there is nothing wrong with this, in fact QEMU allows it. Problem is that Power guests handle this case in a "better" way than x86 and others, making the non-zero function being visible and usable by the guest without any extra kernel option (x86 needs pci_scan_all). It's also hardware dependent - the BCM5719 network card I am using for testing deals with this scenario, but there's no guarantee that other cards will.
To answer your question directly: this might not fail in an obvious way in the guest, but it's not like this feature is a default PCI assignment mode - the user have to deliberately unassigned the function 0 in the XML. Granted, I am being conservative with this extra check - we can simply let the user play with the configuration and, in case it blows up, the user can simply add function 0 back to the guest. If this turns out to be a "toxic" setup then I can go to QEMU and deal with it there - I mean, in the end it's QEMU that allows this, so makes sense to handle the case down there.
Okay I think we should drop the last two patches then. Small change of plan: Please respin this series with the alias changes dropped. Also in the docs patch, the version needs to be updated to 6.0.0, I missed that the first time. After that I will push Thanks, Cole