[PATCH] qemu: do not add model when actual iface type is hostdev

[Paulo de Rezende Pinatti]

On 17/06/20 06:01, Laine Stump wrote:
> On 6/16/20 10:32 AM, Paulo de Rezende Pinatti wrote:
>> No default model should be added to the interface
>> entry at post parse when its actual network type is hostdev
>> as doing so might cause a mismatch between the interface
>> definition and its actual device type.
> 
> 
> Have you encountered a real problem from this? (I have, and have been 
> thinking about the issue for awhile, but only late at night when I'm not 
> near my keyboard to do something about it. I'm just wondering what 
> problem you've had :-))

Yes I have. I first defined a passthrough network out of a PF and then 
created the domain with an interface definition '<source 
network=passthrough/>' and no model (basically the process outlined in 
https://wiki.libvirt.org/page/Networking#Assignment_from_a_pool_of_SRIOV_VFs_in_a_libvirt_%3Cnetwork%3E_definition) 
, but after saving it libvirt automatically added <model type='virtio'> 
(that's the default model on S390) which led to a qemu error when trying 
to start the vm (see below).

> 
> 
> The reason it's been on my mind is this: The default model is rtl8139. 
> When libvirt is auto-assigning PCI addresses to devices at parse time, 
> it decides whether to assign a network device to a conventional PCI or 
> PCI Express slot based on the model, and rtl8139 is conventional PCI. So 
> if you have <interface type='network'> where the network is a pool of 
> hostdevs, and if you don't assign a "fake" model like "virtio" or 
> "e1000e", then the hostdev device (which is 100% certainly a PCIe 
> device) will be assigned to a conventional PCI slot. That works, but 
> is.... "sub-optimal" :-)
> 

For me it didn't work at all because model virtio on S390 leads to a 
'devno' property being specified which of course is wrong for this type 
of device (i.e. '-device 
vfio-pci,host=0100:00:08.2,id=hostdev0,devno=fe.0.0006').

> 
> I think we will still need to add a bit to 
> qemuDomainDeviceCalculatePCIConnectFlags() in order to get the right 
> type of slot set, but this is a good start.
> 
> 
> Reviewed-by: Laine Stump <laine at redhat.com>
> 
> 
> Thanks, and congratulations on your first libvirt patch!
> 

Thanks! :)


> 
> 
> ---
>>   src/qemu/qemu_domain.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>> index 2dad823a86..33ce0ad992 100644
>> --- a/src/qemu/qemu_domain.c
>> +++ b/src/qemu/qemu_domain.c
>> @@ -5831,6 +5831,7 @@ 
>> qemuDomainDeviceNetDefPostParse(virDomainNetDefPtr net,
>>                                   virQEMUCapsPtr qemuCaps)
>>   {
>>       if (net->type != VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>> +        virDomainNetResolveActualType(net) != 
>> VIR_DOMAIN_NET_TYPE_HOSTDEV &&
>>           !virDomainNetGetModelString(net))
>>           net->model = qemuDomainDefaultNetModel(def, qemuCaps);
> 
> 

-- 
Best regards,

Paulo de Rezende Pinatti