Bug 2021437

commit a453ebcd2bfb63ba3fc85e9c39bcdcf47c3d9f1c 
Author: Peter Krempa <pkrempa>
Date:   Thu Nov 25 16:17:16 2021 +0100

    qemu: Fix validation of PCI option rom settings on hotplug
    
    Commit 24be92b8e moved the option rom settings validation code to the
    validation callbacks, but that doesn't work properly with device hotplug
    as we assign addresses only after parsing the whole XML. The check is
    too strict for that and caused failures when hotplugging devices such
    as:
    
     <interface type='network'>
       <source network='default'/>
       <model type='virtio'/>
       <rom enabled='no'/>
     </interface>
    
    This patch relaxes the check in the validation callback to accept also
    _NONE and _UNASSIGNED address types and returns the check to
    'qemuBuildRomProps' so that we preserve the full validation as we've
    used to.
    
    Fixes: 24be92b8e38762e9ba13e
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2021437
    Signed-off-by: Peter Krempa <pkrempa>
    Reviewed-by: Ján Tomko <jtomko>

v7.10.0-rc1-3-ga453ebcd2b

Auto job run with libvirt-7.10.0-1.el9.x86_64 and there is no such issue.

Reproduced on:
liibvirt-7.9.0-1.el9.x86_64
qemu-kvm-6.2.0-3.el9.x86_64

Steps:
1. none address
(1)interface
# virsh attach-device r8 iface-romno-none.xml 
error: Failed to attach device from iface-romno-none.xml
error: unsupported configuration: ROM tuning is only supported for PCI devices

(2)hostdev
# cat hostdev-romno-none.xml
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
    </hostdev>

# virsh attach-device r8 hostdev-romno-none.xml 
error: Failed to attach device from hostdev-romno-none.xml
error: unsupported configuration: ROM tuning is only supported for PCI devices


2. address type='unassigned'
Since it’s only valid for PCI hostdevs:
# cat hostdev-romno-unassigned.xml 
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='unassigned'/>
    </hostdev>

# virsh attach-device r8 hostdev-romno-unassigned.xml 
error: Failed to attach device from hostdev-romno-unassigned.xml
error: unsupported configuration: ROM tuning is only supported for PCI devices



Verified on:
# rpm -q libvirt qemu-kvm
libvirt-8.0.0-0rc1.1.el9.x86_64
Qemu-kvm-6.2.0-3.el9.x86_64

1. # virsh attach-device r8 iface-romno-none.xml 
Device attached successfully

# virsh dumpxml r8|grep /interface -B8
    <interface type='network'>
      <mac address='52:54:00:96:4d:35'/>
      <source network='default' portid='bdf43f4f-67e9-41d3-aef3-018965a40369' bridge='virbr0'/>
      <target dev='vnet7'/>
      <model type='virtio'/>
      <alias name='net3'/>
      <rom enabled='no'/>
      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
    </interface>

(2)# virsh attach-device r8 hostdev-romno-none.xml
Device attached successfully

# virsh dumpxml r8|grep /hostdev -B8
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/>
    </hostdev>

2.
# virsh attach-device r8 hostdev-romno-unassigned.xml 
Device attached successfully

# virsh dumpxml r8|grep /hostdev -B8
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
    </hostdev>

3. Regression test for address=pci:
(1)<interface type='network'>
  <source network='default'/>
  <model type='virtio'/>
  <rom enabled='no'/>
  <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
</interface>

# virsh attach-device r8 iface-romno-pci.xml 
Device attached successfully

(2)# cat hostdev-romno-pci.xml
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
            <address type='pci' domain='0x0000' bus='0x09' slot='0x00' function='0x0'/>
    </hostdev>
# virsh attach-device r8 hostdev-romno-pci.xml 
Device attached successfully

Hi Peter,

I found an issue when test 'unassigned' address hostdev. Could you help check?

If start guest with hostdev, <address type='unassigned'/> will not change in live xml. But if hotplug a hostdev with 'unassigned' address, it will changed to normal pci address.

1.start with
# virsh dumpxml r8|grep hostdev -B8
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <rom enabled='no'/>
      <address type='unassigned'/>
    </hostdev>

# virsh start r8
Domain 'r8' started

# virsh dumpxml r8|grep hostdev -B8
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='unassigned'/>
    </hostdev>

2.hotplug
# virsh dumpxml r8|grep /hostdev -B8
(no output)
# cat hostdev-romno-unassigned.xml 
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='unassigned'/>
    </hostdev>

# virsh attach-device r8 hostdev-romno-unassigned.xml
Device attached successfully

# virsh dumpxml r8|grep /hostdev -B8
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <driver name='vfio'/>
      <source>
        <address domain='0x0000' bus='0x3b' slot='0x0a' function='0x0'/>
      </source>
      <alias name='hostdev0'/>
      <rom enabled='no'/>
      <address type='pci' domain='0x0000' bus='0x05' slot='0x00' function='0x0'/>
    </hostdev>

Is this designed as it. Or should I open a new bug to track it?

Thank you.

I'm sorry but that is not really related to this bug and I don't know how that is supposed to work.

Mark this bug as verified per comment 5.

Hi Cole,

I found address type='unassigned' was involved by your reviewed commits. https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=96999404cb3bfcdb0f169857d9a0ea3d602cbc43
I'm not familiar with it. So could you help check the issue in comment 6 please?

Thank you!

(In reply to yanqzhan from comment #9)
> Hi Cole,
> 
> I found address type='unassigned' was involved by your reviewed commits.
> https://libvirt.org/git/?p=libvirt.git;a=commitdiff;
> h=96999404cb3bfcdb0f169857d9a0ea3d602cbc43
> I'm not familiar with it. So could you help check the issue in comment 6
> please?
> 
> Thank you!

Track in bz2040548.

The failure is arguable correct, the rom setting has no effect for address type='unassigned', since that device is not passed to qemu in any way. Maybe the error message could be more specific but IMO this is realllly obscure error checking condition, we should just ignore it for RHEL, it's not worth the effort to track and fix and backport IMO

(In reply to Cole Robinson from comment #11)
> The failure is arguable correct, the rom setting has no effect for address
> type='unassigned', since that device is not passed to qemu in any way. Maybe
> the error message could be more specific but IMO this is realllly obscure
> error checking condition, we should just ignore it for RHEL, it's not worth
> the effort to track and fix and backport IMO

Hi. Thanks Cole,
But my concern is in comment6, about "starting and hotplugging 'unassigned' hostdev have different behaviors", nothing about rom. I'll cc you in another tracking bug.
But with your reply, I guess rom setting should not be allowed for 'unassigned' address type device, right? This bug's fix permits hotplug for it. Do you think it should be prohibited back or sth? (comment5's error is just my reproduce, now hotplug succeeds.)

I commented on the other bug.

rom setting does not have any effect for type='unassigned' but it's very minor impact, I don't think it's worth tracking and fixing for RHEL

(In reply to Cole Robinson from comment #13)

Ok. Thank you so much!

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (new packages: libvirt), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2022:2390