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