Bug 1439991

Summary: Suggest to disable update "boot order" in interface by update-device
Product: Red Hat Enterprise Linux 7 Reporter: yalzhang <yalzhang>
Component: libvirtAssignee: Michal Privoznik <mprivozn>
Status: CLOSED ERRATA QA Contact: yalzhang <yalzhang>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 7.4CC: fjin, jsuchane, mprivozn, rbalakri, xuzhang
Target Milestone: rcKeywords: Upstream
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: libvirt-3.9.0-1.el7 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-04-10 10:42:33 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description yalzhang@redhat.com 2017-04-07 03:24:33 UTC
Description of problem:
use update-device to update boot order in interface will not check os/boot elements, which will cause invalid xml

Version-Release number of selected component (if applicable):
libvirt-3.1.0-2.el7.x86_64

How reproducible:
100%

Steps to Reproduce:
1. Update the live configuration of boot order make no changes
# virsh start rhel7.4 

# virsh dumpxml rhel7.4
...
  <os>
    <type arch='x86_64' machine='pc-i440fx-rhel7.4.0'>hvm</type>
    <boot dev='hd'/>
  </os>
....
        <interface type='network'>
      <mac address='00:16:3e:77:e2:ed'/>
      <source network='net7' bridge='virbr7'/>
      <target dev='vnet0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>

2. 
# cat net
   <interface type='network'>
 <source network='net7' bridge='virbr0'/>
 <mac address='00:16:3e:77:e2:ed'/>
  <boot order='1'/>
<model type='virtio'/>
    </interface>

3. live update the interface with <boot order='1'/>
# virsh update-device rhel7.4 net 
Device updated successfully

# echo $?
0

4. check the xml
# virsh dumpxml rhel7.4 | grep /interface -B7
    <interface type='network'>
      <mac address='00:16:3e:77:e2:ed'/>
      <source network='net7' bridge='virbr7'/>
      <target dev='vnet0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>

# virsh dumpxml rhel7.4 --inactive| grep /interface -B5
    <interface type='network'>
      <mac address='00:16:3e:77:e2:ed'/>
      <source network='net7'/>
      <model type='virtio'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>

5. update the persistent configuration
# virsh update-device rhel7.4 net  --config
Device updated successfully

# virsh dumpxml rhel7.4 --inactive| grep /interface -B6
      <interface type='network'>
      <mac address='00:16:3e:77:e2:ed'/>
      <source network='net7'/>
      <model type='virtio'/>
      <boot order='1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>

6. # virsh destroy rhel7.4 
Domain rhel7.4 destroyed

# virsh start rhel7.4
error: Failed to start domain rhel7.4
error: unsupported configuration: per-device boot elements cannot be used together with os/boot elements

Actual results:
step 3 and 4, update successfully but it makes no changes
step 5 and 6, update persistent config succeed with problematic xml

Expected results:
1) suggest to disable updating the "boot order" in interface as it is no use for a running vm, and for persistent config, need to check os/boot or other <boot order/> settings
or 2) do the check

Additional info:
N/A

Comment 2 Michal Privoznik 2017-09-05 14:38:18 UTC
Patch proposed upstream:

https://www.redhat.com/archives/libvir-list/2017-September/msg00092.html

Comment 3 Michal Privoznik 2017-09-06 09:38:00 UTC
I've just pushed the patch upstream:

commit c57f3fd2f8999d17e01272246b26d31179ab7b6e
Author:     Michal Privoznik <mprivozn>
AuthorDate: Tue Sep 5 16:24:14 2017 +0200
Commit:     Michal Privoznik <mprivozn>
CommitDate: Wed Sep 6 11:19:30 2017 +0200

    conf: Validate device on update-device
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1439991
    
    Whenever a device is being updated via
    virDomainUpdateDeviceFlags() API, we parse the device XML and
    ideally run some generic checks to validate the configuration
    (e.g. if device defines per-device boot order but the domain has
    os/boot element already). Well, that's the theory - due to a
    missing check we've jumped early from that check function.
    
    Signed-off-by: Michal Privoznik <mprivozn>
    Reviewed-by: Erik Skultety <eskultet>

v3.7.0-14-gc57f3fd2f

Comment 5 yalzhang@redhat.com 2017-10-11 02:20:58 UTC
verify this bug on libvirt-3.8.0-1.el7.x86_64, it will do the check for os/boot, and any duplicate 'boot order', but it do not validate the value of the 'boot order'. Using negative number or string will cause libvirtd crash(see step 6).  

1. attach interface with boot order to the guest with os/boot:

# cat net.xml
<interface type='network'>
 <source network='default' bridge='virbr0'/>
 <mac address='00:16:3e:77:e2:ed'/>
  <boot order='1'/>
  <model type='virtio'/>
    </interface>

# virsh dumpxml rhel
...
 <os>
    <type arch='x86_64' machine='pc-i440fx-rhel7.4.0'>hvm</type>
    <boot dev='hd'/>
  </os>
...

# virsh attach-device rhel7.4 net.xml
error: Failed to attach device from net.xml
error: unsupported configuration: per-device boot elements cannot be used together with os/boot elements

2. attach interface with <boot order='1'/> to the guest with <boot order='1'/> in <disk/>:

# virsh attach-device rhel7.4 net.xml
error: Failed to attach device from net.xml
error: unsupported configuration: boot order 1 is already used by another device

3. attach interface with <boot order='2'/> to the guest with <boot order='1'/> in <disk/>:

# cat net1.xml
<interface type='network'>
 <source network='default' bridge='virbr0'/>
 <mac address='00:16:3e:77:e2:ed'/>
  <boot order='2'/>
  <model type='virtio'/>
    </interface>
# virsh attach-device rhel7.4 net1.xml
Device attached successfully

4. cold plug with guest have os/boot
# virsh attach-device rhel7.4 net.xml --persistent
error: Failed to attach device from net.xml
error: unsupported configuration: per-device boot elements cannot be used together with os/boot elements

5. cold plug with <boot order='1'/> while <boot order='1'/> already defined
# virsh attach-device rhel7.4 net.xml --persistent
error: Failed to attach device from net.xml
error: unsupported configuration: boot order 1 is already used by another device


6. use negative number or string:
# cat net2.xml
<interface type='network'>
 <source network='default' bridge='virbr0'/>
 <mac address='00:16:3e:77:e2:ed'/>
  <boot order='-1'/>
  <model type='virtio'/>
    </interface>

# pidof libvirtd
23939

# virsh attach-device rhel7.4 net2.xml 
error: Disconnected from qemu:///system due to end of file
error: Failed to attach device from net2.xml
error: End of file while reading data: Input/output error

# pidof libvirtd
24255

# cat net3.xml
<interface type='network'>
 <source network='default' bridge='virbr0'/>
 <mac address='00:16:3e:77:e2:ed'/>
  <boot order='s'/>
  <model type='virtio'/>
    </interface>

# virsh attach-device rhel7.4 net3.xml 
error: Disconnected from qemu:///system due to end of file
error: Failed to attach device from net3.xml
error: End of file while reading data: Input/output error

# pidof libvirtd
24480

Comment 6 Michal Privoznik 2017-10-23 05:58:21 UTC
Yes, that is a crasher. However, it's fixed upstream:

commit 921d61575d6ca7f6242195e7e316904b766da668
Author:     Nikolay Shirokovskiy <nshirokovskiy>
AuthorDate: Tue Oct 17 16:56:33 2017 +0300
Commit:     John Ferlan <jferlan>
CommitDate: Tue Oct 17 15:49:25 2017 -0400

    conf: fix use of uninitialized variable
    
    If same boot order is specified twice (or more) in domain xml
    we call free for uninitiaziled loadparm on cleanup in virDomainDeviceBootParseXML
    and SIGABRT (or similar) as a result.

v3.8.0-138-g921d61575

Moving back to post to pick up this commit.

Comment 7 yalzhang@redhat.com 2017-10-24 03:11:19 UTC
Test on upstream with the patch, it works well and the crash issue is fixed.

Comment 8 yalzhang@redhat.com 2017-11-03 07:17:15 UTC
Test on libvirt-3.9.0-1.el7.x86_64 with scenarios in comment 5, it works ok.

Comment 12 errata-xmlrpc 2018-04-10 10:42:33 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, 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/RHEA-2018:0704