Re: [PATCH v3 0/6] PCI hostdev partial assignment support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/26/19 8:33 AM, Daniel Henrique Barboza wrote:
> changes from previous version [1]:
> - do not overload address type='none'. A new address type called
> 'unassigned' is introduced;
> - there is no unassigned' flag being created in virDomainHostdevDef.
> The logic added by new address type is enough;
> - do not allow function zero of multifunction devices to be
> unassigned.
> 
> Nothing too special to discuss in this cover letter. More details
> can be found at the discussions of the previous version [1]. Commit
> messages of the patches have more background info as well.
> 
> [1] https://www.redhat.com/archives/libvir-list/2019-November/msg01099.html
> 

For 1-4:

Reviewed-by: Cole Robinson <crobinso@xxxxxxxxxx>

You asked in the last posting whether to use ADDRESS_TYPE_NONE and add a
new hostdev unassigned= attribute, or this approach. I think this is the
correct approach. Address type='unassigned' captures the description
well, and since address type='none' isn't printed in the XML, it would
make XML output a bit more confusing IMO if no address was printed.

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

>   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.

Maybe laine knows better if there's precedent for similar checks at
Validate time

Thanks,
Cole

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux