Bug 872028

Summary: virsh detach-device does not check the interface type
Product: Red Hat Enterprise Linux 7 Reporter: yanbing du <ydu>
Component: libvirtAssignee: Ján Tomko <jtomko>
Status: CLOSED ERRATA QA Contact: Virtualization Bugs <virt-bugs>
Severity: medium Docs Contact:
Priority: medium    
Version: 7.0CC: cwei, dyuan, laine, lcheng, mzhan, rbalakri
Target Milestone: rc   
Target Release: ---   
Hardware: Unspecified   
OS: Linux   
Whiteboard:
Fixed In Version: libvirt-1.2.13-1.el7 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-11-19 05:35:58 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 yanbing du 2012-11-01 03:18:21 UTC
Description of problem:
When detach an interface device from an XML file, both 'network' and 'bridge' type interfaces can be detached with reverse type in XML file.
And when detach an interface with correct MAC address and wrong PCI address, the error message should be more clear.


Version-Release number of selected component (if applicable):
libvirt-0.10.2-6.el6


How reproducible:
100%


Steps to Reproduce:
1. Attach a 'bridge' type interface 
# virsh attach-interface guest bridge virbr0  --mac 52:54:00:5a:a0:8d
2. Prepare a XML file to detach the newly attach interface, but change the interface type to 'network'
# cat interface.xml 
    <interface type='network'>
      <mac address='52:54:00:5a:a0:8d'/>
      <source network='default'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </interface>
3. Detach the interface by virsh detach-device
# virsh detach-device guest interface.xml
Device detached successfully
4. Repeat step1~3 to attach a 'network' type interface and detach it with the reverse interface type in XML file.
5. Repeat step1~3 to attach a 'network' type interface and detach it with the wrong PCI address in XML file
# virsh detach-device guest interface-bridge.xml 
error: Failed to detach device from interface-bridge.xml
error: operation failed: network device 52:54:00:5A:A0:8D not found

in fact, the device with MAC address 52:54:00:5A:A0:8D do exist, and the error message should point out the specified PCI address, not only the MAC address.

# virsh domiflist guest
Interface  Type       Source     Model       MAC
-------------------------------------------------------
vnet0      bridge     virbr0     -           52:54:00:5a:a0:8b
vnet2      network    default    -           52:54:00:5a:a0:8d

Actual results:
As reproduce steps show.

Expect result:
virsh detach-device should check the interface type when detach the interface.
And when detach device with wrong PCI address, the error message should be more clear.


Additional info:

Comment 3 Laine Stump 2012-11-05 15:53:05 UTC
In my opinion, making "type" one of the match attributes for virsh detach-interface (and a *require* parameter at that!) was a mistake - it is a very generic attribute, and the chances of it helping to uniquely identify the device are very low. PCI (newly added as a key - see Bug 862515) and MAC (always been there) address are much better. It is *only* virsh that does anything with matching of the interface type.

Internal to libvirt, we historically only checked for a matching MAC address. A check for match of PCI address (if specified in the input XML) was recently added, because MAC address by itself isn't always unique, but PCI address is guaranteed to be unique.

You could possibly consider checking for a match of type if it's specified, however type must *always* be specified, so we would then always check for it. The problem then becomes that existing functional code that fills in just the MAC address (along with some generic placeholder value for the type) would suddenly stop working.

If we're concerned about that, a better solution may be to simply document what attributes are checked for a match during detach (MAC address and PCI address), and point out that the other attributes are unnecessary to guarantee unique specification and are therefore ignored.

Comment 7 Ján Tomko 2014-04-01 15:15:48 UTC
Patches cleaning up the error message:
https://www.redhat.com/archives/libvir-list/2014-April/msg00021.html

Comment 8 Ján Tomko 2014-04-03 07:08:58 UTC
Error messages have been cleaned up upstream:
commit 246317d3d98cf87b2e24932a118ebf2669462242
Author:     Ján Tomko <jtomko>
AuthorDate: 2014-04-01 16:26:24 +0200
Commit:     Ján Tomko <jtomko>
CommitDate: 2014-04-03 08:59:36 +0200

    Include PCI address in the error in virDomainNetFindIdx
    
    When looking up a net device by a MAC and PCI address, it is possible
    that we've got a match on the MAC address but failed to match the
    PCI address.
    
    In that case, outputting just the MAC address can be confusing.
    
    Partially resolves:
    https://bugzilla.redhat.com/show_bug.cgi?id=872028

commit 2fbae1b2a91793646efc8ad11cf529470d4bc68b
Author:     Ján Tomko <jtomko>
AuthorDate: 2014-04-01 15:56:55 +0200
Commit:     Ján Tomko <jtomko>
CommitDate: 2014-04-03 08:59:36 +0200

    Move error reporting into virDomainNetFindIdx
    
    Every caller checked the return value and logged an error
    - one if no device with the specified MAC was found,
    other if there were multiple devices matching the MAC address
    (except for qemuDomainUpdateDeviceConfig which logged the same
     message in both cases).
    
    Move the error reporting into virDomainNetFindIdx, since in both cases,
    we couldn't find one single match - it's just the error messages that
    differ.

git describe: v1.2.3-34-g246317d

Comment 10 Jiri Denemark 2014-04-04 21:36:47 UTC
This bug was not selected to be addressed in Red Hat Enterprise Linux 6. We will look at it again within the Red Hat Enterprise Linux 7 product.

Comment 12 Ján Tomko 2015-02-20 11:40:41 UTC
Upstream patch documenting the behavior:
https://www.redhat.com/archives/libvir-list/2015-February/msg00822.html

Comment 14 Ján Tomko 2015-02-25 09:18:10 UTC
Pushed upstream:
commit c6807b507afce9c3483681bf24cc4a5260d29469
Author:     Ján Tomko <jtomko>
CommitDate: 2015-02-25 10:06:41 +0100

    Clarify behavior or virDomainDetachDevice
    
    Document that a complete device definition should be used
    and a partial match can lead to the device being detached.
    
    https://bugzilla.redhat.com/show_bug.cgi?id=872028

git describe: v1.2.13-rc1-9-gc6807b5

Comment 16 lcheng 2015-04-14 10:38:01 UTC
Verify it as follows. 

[root@localhost ~]# rpm -q libvirt
libvirt-1.2.14-1.el7.x86_64

1. Check document.
[root@localhost ~]# man virsh
...
       detach-device domain FILE [[[--live] [--config] | [--current]] | [--persistent]]
...
           Note: The supplied XML description of the device should be as specific as its definition in the domain
           XML. The set of attributes used to match the device are internal to the drivers. Using a partial
           definition, or attempting to detach a device that is not present in the domain XML, but shares some
           specific attributes with one that is present, may lead to unexpected results.

...


2. Check error messages.

[root@localhost ~]# virsh start 7
Domain 7 started

[root@localhost ~]# virsh attach-interface 7 network default --mac 52:54:00:5a:a0:8d
Interface attached successfully

[root@localhost ~]# virsh dumpxml 7 | grep interface -A10
    <interface type='network'>
      <mac address='00:16:3e:3e:a9:11'/>
      <source network='default' bridge='virbr0'/>
      <target dev='vnet1'/>
      <model type='rtl8139'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
    </interface>
    <interface type='network'>
      <mac address='52:54:00:5a:a0:8d'/>
      <source network='default' bridge='virbr0'/>
      <target dev='vnet2'/>
      <model type='rtl8139'/>
      <alias name='net1'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/>
    </interface>
...

[root@localhost ~]# cat br.xml 
    <interface type='network'>
      <mac address='52:54:00:5a:a0:8d'/>
      <source network='default'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>

[root@localhost ~]# virsh detach-device 7 br.xml 
error: Failed to detach device from br.xml
error: operation failed: no device matching mac address 52:54:00:5a:a0:8d found on 0000:00:03.0


[root@localhost ~]# vim br.xml
[root@localhost ~]# cat br.xml 
    <interface type='network'>
      <mac address='52:54:00:5a:a0:89'/>
      <source network='default'/>
    </interface>

[root@localhost ~]# virsh detach-device 7 br.xml 
error: Failed to detach device from br.xml
error: operation failed: no device matching mac address 52:54:00:5a:a0:89 found

Comment 17 lcheng 2015-04-16 03:20:39 UTC
According to comment 3 and comment 16, move to VERIFIED.

Comment 19 errata-xmlrpc 2015-11-19 05:35:58 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://rhn.redhat.com/errata/RHBA-2015-2202.html