There are other places it would be convenient to give the netdev name instead of needing to figure out the PCI address. For example, in the xml of the domain <interface type='hostdev'> definition, and in the pool of devices of a <forward mode='hostdev'> network. We should consider those (and maybe there are others?) before implementing this purely as a virsh extension.

At the level of virsh, I think this should be implemented with a new -type for attach-interface rather than making a new command. It would look something like this:

  virsh attach-interface --type hostdev --source 0000:06:02.4 --managed

or

  virsh attach-interface --type hostdev --source eth0 --managed

(perhaps managed should be the default, and we provide a way to turn it off, like "--managed no" ?)

This would produce the following XML:

  <interface type='hostdev' managed='yes'>
    <source>
      <address type='pci' domain='0x0000' bus='0x06' slot='0x02' function='0x4'/>
    </source>
  </interface>

or

  <interface type='hostdev' managed='yes'>
    <source dev='eth0'/>
  </interface>

This, in turn, requires supporting specification of the host device with its netdev name rather than a pci address.

In addition to this, network pools of devices should also be extended in the same way. As we already support this syntax for networks:

  <network>
    <name>test-net</name>
    <forward mode='bridge'>
      <interface dev='eth0'/>
      <interface dev='eth1'/>
      ..
    </forward>
  </network>

We would not need any new XML, but just be able to deal with being given a netdev rather than a PCI address from the pool when forward mode='hostdev'; the network driver would only need to be modified to *allow* this, not do anything special with it. If the qemu driver was given a netdev for type='hostdev', it would just call [whatever function, I don't have the name on my mind right now] to convert into a PCI address.

So there are really 3 tasks here. Here is the order to implement them:

1) support <source dev='$netdev'/> for <interface type='hostdev'/>

2) allow specifying <interface type='$netdev'/> for network hostdev pools, and returning an interface name rather than a pci address from networkAllocateActualDevice (network driver).

3) expand the virsh attach-interface command to allow '--type hostdev' (note that if you put your netdevs in a network pool, you could just use '--type network'

I don't agree that this should be implemented at the libvirt level.  Main reason is the fact, that if you detach the device from host, than you cannot find it anymore by it's netdev name.  It only exist as a pci device and such domain containing:

  <interface type='hostdev' managed='yes'>
    <source dev='eth0'/>
  </interface>

cannot start, if the netdev device doesn't exists.

We should implement it only in virsh and it will translate the netdev name to pci address.  This means, there is no change needed for libvirt, just extend virsh attach-interface to support --type=hostdev and it will always create an XML with pci address.

The other places like network definition you've mentioned are completely different cases, because for example creating a bridge is done only with the netdev names and you don't need to translate them to pci addresses and you don't need to detach them from the host.

(In reply to Pavel Hrdina from comment #6)
> I don't agree that this should be implemented at the libvirt level.  Main
> reason is the fact, that if you detach the device from host, than you cannot
> find it anymore by it's netdev name.  It only exist as a pci device and such
> domain containing:
> 
>   <interface type='hostdev' managed='yes'>
>     <source dev='eth0'/>
>   </interface>
> 
> cannot start, if the netdev device doesn't exists.

That's a good point - using the netdev name can't work for managed='no'.

But I don't think that's a reason to not support it in XML for managed='yes' - just give an VIR_ERR_CONFIG_UNSUPPORTED if someone tries to use the netdev name and managed='no' at the same time.

> We should implement it only in virsh and it will translate the netdev name
> to pci address.  This means, there is no change needed for libvirt, just
> extend virsh attach-interface to support --type=hostdev and it will always
> create an XML with pci address.

The advantage of this is that a new virsh with the feature would be able to use it when talking to an old libvirtd, but it removes the convenience / simplicity for people who are editing the XML themselves.


> 
> The other places like network definition you've mentioned are completely
> different cases, because for example creating a bridge is done only with the
> netdev names and you don't need to translate them to pci addresses and you
> don't need to detach them from the host.

I think you're only considering networks using a device pool for macvtap (where listing the netdev names is already supported). For a network that is <forward mode='hostdev'>, if the pool were to list the netdev names rather than pci addresses (currently only the latter is supported) then we would need to translate the netdev to a PCI address, and detach it from the host (this wouldn't be done as a part of the network driver though - when qemu requested a device from the pool, a netdev name could be returned, and if the interface initialization had been enhanced to support using netdev names for PCI device assignment, it would all just be handled automatically)

(In reply to Laine Stump from comment #7)
> (In reply to Pavel Hrdina from comment #6)
> > I don't agree that this should be implemented at the libvirt level.  Main
> > reason is the fact, that if you detach the device from host, than you cannot
> > find it anymore by it's netdev name.  It only exist as a pci device and such
> > domain containing:
> > 
> >   <interface type='hostdev' managed='yes'>
> >     <source dev='eth0'/>
> >   </interface>
> > 
> > cannot start, if the netdev device doesn't exists.
> 
> That's a good point - using the netdev name can't work for managed='no'.
> 
> But I don't think that's a reason to not support it in XML for managed='yes'
> - just give an VIR_ERR_CONFIG_UNSUPPORTED if someone tries to use the netdev
> name and managed='no' at the same time.

Sure, this could be a solution for this case.  But you can also create an interface for a domain with managed='yes' and the network device could be detached from the guest and the guest will start.  Using a netdev name would fail in this case and that's the disadvantage of the netdev name.

> 
> > We should implement it only in virsh and it will translate the netdev name
> > to pci address.  This means, there is no change needed for libvirt, just
> > extend virsh attach-interface to support --type=hostdev and it will always
> > create an XML with pci address.
> 
> The advantage of this is that a new virsh with the feature would be able to
> use it when talking to an old libvirtd, but it removes the convenience /
> simplicity for people who are editing the XML themselves.
> 
> 
> > 
> > The other places like network definition you've mentioned are completely
> > different cases, because for example creating a bridge is done only with the
> > netdev names and you don't need to translate them to pci addresses and you
> > don't need to detach them from the host.
> 
> I think you're only considering networks using a device pool for macvtap
> (where listing the netdev names is already supported). For a network that is
> <forward mode='hostdev'>, if the pool were to list the netdev names rather
> than pci addresses (currently only the latter is supported) then we would
> need to translate the netdev to a PCI address, and detach it from the host
> (this wouldn't be done as a part of the network driver though - when qemu
> requested a device from the pool, a netdev name could be returned, and if
> the interface initialization had been enhanced to support using netdev names
> for PCI device assignment, it would all just be handled automatically)

Yes, I know, that there is a <forward mode='hostdev'>, but I'm not still convinced that it should be implemented and that it will create some kind of benefit for users.  Right now you can use <pf dev='eth0'/> to tell the libvirt where to find the virtual functions and it will use all of them.  If you want to use only a part of the VF for that network, than it's a special case and it's better to specify them via PCI address.  And again, there could be the same issue where the device could be already detached from the host.  Using the PCI address it will work also for <managed='yes'>, but using the netdev name would fail.

Upstream commit:

commit a9a583d6df74a795f26f4ac4187eede842a7a2b6
Author: Pavel Hrdina <phrdina>
Date:   Wed Oct 21 12:59:41 2015 +0200

    virsh-domain: update attach-interface to support type=hostdev
    
    Adding this feature will allow users to easily attach a hostdev network
    interface using PCI passthrough.
    
    The interface can be attached using --type=hostdev and PCI address or
    as --source.  This command also allows you to tell, whether the interface
    should be managed.
    
    Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=997561
    
    Signed-off-by: Pavel Hrdina <phrdina>

For comment 10 and comment 11, it is tested on upstream.
#git describe
v1.3.1-262-gcda1cc1

Verified on libvirt-1.3.1-1.el7.x86_64, the result is as expected, modify the status to verified.
Attach an interface with type=hostdev:

[root@sriov Desktop]# virsh start t_R7.2
Domain t_R7.2 started

[root@sriov Desktop]# virsh dumpxml t_R7.2 | grep /interface -B8
    </controller>
    <interface type='network'>
      <mac address='52:54:00:e0:69:0a'/>
      <source network='default' bridge='virbr0'/>
      <target dev='vnet0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>
[root@sriov Desktop]# virsh attach-interface t_R7.2 --type  hostdev --source 0000:03:10:1   --managed
Interface attached successfully

[root@sriov Desktop]# virsh attach-interface t_R7.2 --type  hostdev --source 0000:03:10:2  
error: Failed to attach interface
error: Path '/dev/vfio/24' is not accessible: No such file or directory

[root@sriov Desktop]# virsh dumpxml t_R7.2 | grep /interface -B8    </controller>
    <interface type='network'>
      <mac address='52:54:00:e0:69:0a'/>
      <source network='default' bridge='virbr0'/>
      <target dev='vnet0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>
    <interface type='hostdev' managed='yes'>
      <mac address='52:54:00:33:aa:b6'/>
      <driver name='vfio'/>
      <source>
        <address type='pci' domain='0x0000' bus='0x03' slot='0x10' function='0x1'/>
      </source>
      <alias name='hostdev0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
    </interface>
[root@sriov Desktop]#

The detach-insterface also support type of hostdev.

[root@sriov Desktop]# virsh detach-interface t_R7.2 --type hostdev --mac 52:54:00:33:aa:b6
Interface detached successfully

[root@sriov Desktop]# virsh dumpxml t_R7.2 | grep /interface -B7
    <interface type='network'>
      <mac address='52:54:00:e0:69:0a'/>
      <source network='default' bridge='virbr0'/>
      <target dev='vnet0'/>
      <model type='virtio'/>
      <alias name='net0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>
[root@sriov Desktop]#

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/RHSA-2016-2577.html