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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
