Re: [PATCH v5 0/8] virtio-net: add support for SR-IOV emulation

[Cédric Le Goater]

On 7/30/24 19:56, Michael S. Tsirkin wrote:
> On Tue, Jul 30, 2024 at 09:26:20PM +0900, Akihiko Odaki wrote:
> > On 2024/07/30 20:37, Michael S. Tsirkin wrote:
> > > On Mon, Jul 15, 2024 at 02:19:06PM +0900, Akihiko Odaki wrote:
> > > > Based-on: <20240714-rombar-v2-0-af1504ef55de@daynix.com>
> > > > ("[PATCH v2 0/4] hw/pci: Convert rom_bar into OnOffAuto")
> > >
> > > OK I will revert this for now. We'll try again after the release,
> > > there will be time to address s390.
> >
> > I'd like to know if anybody wants to use igb on a s390x machine configured
> > with libvirt. Such a configuration is already kind of broken, and it is
> > likely to require significant effort on both side of libvirt and QEMU to fix
> > it.
>
>
> I assume Cédric wouldn't report it if was unused.
>
>
> > As an alternative, I'm also introducing SR-IOV support to virtio-net-pci. It
> > does not suffer the same problem with igb thanks to its flexible
> > configuration mechanism.
> >
> > Regards,
> > Akihiko Odaki
>
> Sounds like this needs more review time, anyway.

Using an emulated IGB device in a s390x VM is not a common scenario.
The IGB device is not supported downstream (RHEL on Z). However, the
change broke the s390x machines I use for upstream QEMU development,
I removed the IGB device as a fix for now.

The Z PCI device model has 'uid' and 'fid' properties which are set
by the VMM, and in this case, they are auto-generated, hence the
conflicting ids with libvirt. This is Z specific but, possibly there
are subtle use cases on other platforms which could have similar
consequences. Something to be aware of.


Also, and this is why I thought it was important to report. Creating
PCI devices at machine init time (with -nodefaults) in the back of the
management layer is a no-no. libvirt requests to have full control
on the machine topology and this change seems like a violation of
this rule, even if VFs are kind of special.

Daniel, did I understand correctly the above constraint on the machine
definition ?


I can't say if we should revert for 9.1. Again, this Z is specific.
The changes were introduced to catch errors early when the PF device
is realized. There is no doubt they are useful. Some rework is needed
to avoid the conflicting id issue though.


Thanks,

C.