Bug 1847259 - Should fail to save the xml if edit 'ramfb' video with pci address
Summary: Should fail to save the xml if edit 'ramfb' video with pci address
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux Advanced Virtualization
Classification: Red Hat
Component: libvirt
Version: 8.2
Hardware: Unspecified
OS: Unspecified
medium
low
Target Milestone: rc
: 8.3
Assignee: Jonathon Jongsma
QA Contact: Lili Zhu
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2020-06-16 05:27 UTC by yafu
Modified: 2020-11-17 17:49 UTC (History)
8 users (show)

Fixed In Version: libvirt-6.5.0-1.el8
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2020-11-17 17:49:16 UTC
Type: Bug
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description yafu 2020-06-16 05:27:15 UTC
Description of problem:
Should fail to save the xml if edit 'ramfb' video with pci address

Version-Release number of selected component (if applicable):
libvirt-6.0.0-24.module+el8.2.1+6997+c666f621.x86_64

How reproducible:
100%

Steps to Reproduce:
1.Edit guest xml to add 'ramfb' video with pci address:
#virsh edit test1
...
 <video>
      <model type='ramfb' heads='1' primary='yes'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
    </video>
...

2.Check the guest xml
#virsh dumpxml test1
...
 <video>
      <model type='ramfb' heads='1' primary='yes'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/>
    </video>
...

3.Start the guest:
# virsh start test1
error: Failed to start domain test1
error: internal error: qemu unexpectedly closed the monitor: 2020-06-16T05:23:02.759221Z qemu-kvm: -device ramfb,id=video0,bus=pcie.0,addr=0x1: Device 'ramfb' can't go on PCIE bus

Actual results:
Failed to start guest if 'ramfb' video having pci address

Expected results:
As 'ramfb' can't go on PCIE bus, it's better to fail to save xml if edit 'ramfb' video with pci address

Additional info:

Comment 2 Jonathon Jongsma 2020-06-24 22:07:21 UTC
patch sent upstream. 

https://www.redhat.com/archives/libvir-list/2020-June/msg01147.html

Comment 3 Jonathon Jongsma 2020-06-26 14:46:45 UTC
Pushed upstream.

6c560b2d3aa2b2e4ecec44270c814ff9faf5c484 qemu: ramfb video device doesn't support PCI address

Comment 6 Lili Zhu 2020-08-04 03:40:32 UTC
Verify this bug with:
libvirt-6.5.0-1.module+el8.3.0+7323+d54bb644.x86_64

1. edit the guest xml
# virsh edit rhel8.3
...
 <video>
      <model type='ramfb' heads='1' primary='yes'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
  </video>
...
error: unsupported configuration: 'address' is not supported for 'ramfb' video devices
Failed. Try again? [y,n,i,f,?]: 

2. prepare a video device xml snippet
# cat video.xml 
  <video>
      <model type='ramfb' heads='1' primary='yes'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
  </video>

3. coldplug the above device to guest
# virsh attach-device rhel8.3 video.xml --config 
error: Failed to attach device from video.xml
error: unsupported configuration: 'address' is not supported for 'ramfb' video devices

4. start the guest and hotplug the above device to guest
# virsh start rhel8.3 
Domain rhel8.3 started

# virsh attach-device rhel8.3 video.xml 
error: Failed to attach device from video.xml
error: unsupported configuration: 'address' is not supported for 'ramfb' video devices

Comment 7 Lili Zhu 2020-08-04 03:50:42 UTC
Hi, Jonathon

1. prepare the guest xml
# cat rhel8.3.xml |grep '<video' -A3
  <video>
      <model type='ramfb' heads='1' primary='yes'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
  </video>

2. try to validate the xml
# virt-xml-validate rhel8.3.xml 
rhel8.3.xml validates

If 'address' is unsupported configuration, should we also modify the schema?
Please help to check. Thanks

Comment 8 Jonathon Jongsma 2020-09-25 20:12:25 UTC
I'm inclined to say that we shouldn't modify the schema for this, because it's impossible to capture all logic of device type configuration within the XML schema. For example, the schema will also say that an xml file that specifies <address type='dimm'> for a qxl video device is valid, even though that is obviously nonsensical. Also, more practically, I'm not sure how you would capture these restrictions in a relax-ng schema.

Of course, one major difference between these two examples is that when I try to configure a qxl device with a 'dimm' address, it doesn't give a validation error. It parses it successfully, but then essentially just ignores it when it comes to formatting a qemu commandline for the device. 

On the other hand, we *do* report a validation error when you specify an address for the ramfb device. So maybe we should be more consistent between cases like this.  Either the ramfb should silently ignore the address like the previously-mentioned 'dimm' address for qxl, or both should produce an error. But I'm not sure which of those is preferable.

Comment 9 Lili Zhu 2020-09-28 04:11:32 UTC
(In reply to Jonathon Jongsma from comment #8)
> I'm inclined to say that we shouldn't modify the schema for this, because
> it's impossible to capture all logic of device type configuration within the
> XML schema. For example, the schema will also say that an xml file that
> specifies <address type='dimm'> for a qxl video device is valid, even though
> that is obviously nonsensical. Also, more practically, I'm not sure how you
> would capture these restrictions in a relax-ng schema.
> 
> Of course, one major difference between these two examples is that when I
> try to configure a qxl device with a 'dimm' address, it doesn't give a
> validation error. It parses it successfully, but then essentially just
> ignores it when it comes to formatting a qemu commandline for the device. 
> 
> On the other hand, we *do* report a validation error when you specify an
> address for the ramfb device. So maybe we should be more consistent between
> cases like this.  Either the ramfb should silently ignore the address like
> the previously-mentioned 'dimm' address for qxl, or both should produce an
> error. But I'm not sure which of those is preferable.

Hi, Jonathon,

I think it is better to report an error for both these two cases.
It is better to make the schema more accurate. 
I am not sure about the schedule of the bug now.
Should I wait for the fixing, or just file another bug to track it?
Thanks very much.

Comment 10 Lili Zhu 2020-10-26 08:15:59 UTC
Filed bug 1891416 to track the issue in comment 7, move the bug to verified

Comment 13 errata-xmlrpc 2020-11-17 17:49:16 UTC
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 (virt:8.3 bug fix and enhancement update), 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-2020:5137


Note You need to log in before you can comment on or make changes to this bug.