[libvirt] RFC: setting mac address on network devices being assigned to a guest via PCI passthrough (<hostdev>)

[Laine Stump]

On 01/23/2012 09:08 AM, Paolo Bonzini wrote:
>
> <devices>
> <interface type='hostdev'>
> <source>
> <address type='pci' bus='0x06' slot='0x02' function='0x0'/>
> </source>
> <mac address='00:16:3e:5d:c7:9e'/>
> <address type='pci' .../>
> </interface>
> </devices>
>

This is the model that I'm now following.

Looking further into it, I've found that there are lots of places in the 
libvirt code that scan through all the <hostdev> entries, and call 
functions that expect a virDomainHostdevDef as an argument. Of course 
all those same places will need to be visited with devices that are 
assigned via <interface> (virDomainNetDef) as well (and this will also 
apparently be needed for <controller> devices in the near future).

What I'm thinking of doing now, is changing virDomainHostdevDef in the 
following way:

typedef virDomainDeviceSourceInfo *virDomainDeviceSourceInfoPtr;
struct _virDomainDeviceSourceInfo {
     int mode; /* enum virDomainHostdevMode */
     bool managed;
     union {
         virDomainDeviceSubsysAddress subsys; /* USB or PCI */
         struct {
             int dummy;
         } caps;
     };
     virDomainHostdevOrigStates origstates;
};

typedef struct _virDomainHostdevDef virDomainHostdevDef;
typedef virDomainHostdevDef *virDomainHostdevDefPtr;
struct _virDomainHostdevDef {
     virDomainDeviceDefPtr     parent; /* specific device containing 
this def */
     virDomainDeviceSourceInfo source; /* host address info */
     virDomainDeviceInfoPtr    info;   /* guest address info */
};


(note that "info" is now a separate object, rather than simply being 
contained in the HostdevDef!)

This new HostdevDef can then be included directly in higher level device 
types, e.g:

struct _virDomainNetDef {
     enum virDomainNetType type;
     unsigned char mac[VIR_MAC_BUFLEN];
      ...
     union {
          ...
         struct {
             char *linkdev;
             int mode; /* enum virMacvtapMode from util/macvtap.h */
             virNetDevVPortProfilePtr virtPortProfile;
         } direct;
**      struct {
**          virDomainHostdevDef def;
         } hostdev;
     } data;
     struct {
         bool sndbuf_specified;
         unsigned long sndbuf;
     } tune;
      ...
     char *ifname;
     virDomainDeviceInfo info;
      ...
};


for <interface type='hostdev'>, the hostdev would be populated like this:

      (interface->data.hostdev.def.source will already be filled in from 
Parse)

      interface->data.hostdev.def.info = &interface->info;
      interface->data.hostdev.def.parent.type = VIR_DOMAIN_DEVICE_NET;
      interface->data.hostdev.def.parent.data.net = interface;

At this point, &interface->data.hostdev.def can be sent as a parameter 
to any function that's expecting a virDomainHostdevDef. Beyond that, I'm 
thinking it can *even be added to the hostdevs list in the DomainDef*. 
This would work in the following way:

0) If a parent device (in our example a virDomainNetDef) is 
type='hostdev', in addition to be included in its normal higher level 
device list (e.g. domain->nets), parent->data.hostdev will be filled in 
as indicated above, and &parent->data.hostdev will be added to the 
domain's hostdevs list.

1) When a function is scanning through all the hostdevs to do device 
management, it will act on this higher level device just as any generic 
device, except that there may be callouts to setup functions based on 
the value of hostdevs[n]->parent.type (e.g. to setup a MAC address or 
virtual port profile).

2) When an entry from the hostdevs list is being freed, any hostdev that 
has a non-NULL parent will simply be removed from the list (and a 
callout made to the equivalent function to remove the hostdev's parent 
from its own list).

3) When an entry from the higher level device list is being freed, it 
will also be removed from the hostdev list.

4) when one of these "intelligent hostdevs" is attached/detached, 
depending on hostdev->parent.type, it may callout to a device-specific 
function,

By doing things this way, we assure that these new higher level devices 
will always be included in any scans of hostdevs, while avoiding the 
necessity to add a new loop to every one of the functions that scans 
them each time we add support for PCI passthrough of another 
higher-level device type.


Does this sound reasonable? (I'm making a proof-of-concept now, but 
figured I'd solicit input in the meantime).

-------------------

The next problem: We will need to be able to configure everything that's 
in a <hostdev> from within an <interface>, but there are a few things we 
haven't discussed yet:

1) "type='pci'" vs. "type='usb'"

<hostdev> has one of these directly as an attribute of the toplevel 
element, so it isn't given in the source <address> element. In the case 
of <interface>, type is already used for something else in the toplevel 
element, but it can instead be given as part of the <address>. So which 
do you think is better:

<interface type='hostdev'>
<source>
<address type='pci' domain='0' ... />
</source>
      ...

or:

<interface type='pci'>
<source>
<address domain='0' .... />

?? In either case, "type='pci'" could be replaced with "type='usb'". 
Note that if we use the first option, it will be possible to do 
something like:

<interface type='hostdev'>
<source dev='eth22'/>

and have libvirt determine at attachtime whether eth22 is a usb or pci 
device (I'm sure 99 44/100% of all uses of this will be with pci 
devices, but still...).

2) "managed='yes'"

This obviously needs to go *somewhere*. Does this look okay?

<interface type='hostdev' managed='yes'>
     ...


3) "mode='subsystem'"

Since the other mode "capability" has never been implemented, and 
apparently won't be, I don't see any need to give this a place in the 
<interface> XML. For now it will always be subsystem, and if we ever 
need to add a mode attribute, "subsystem" will just be the default.


So what I end up with is this:

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

and

<interface type='hostdev' managed='yes'>
<source>
<address type='pci' domain='0' .... />
</source>
    ...

Note that when "dev='eth22'" is given, an <address> element will be 
added at attach time (I haven't decided yet if it's best for this 
element to persist (with appropriate checks to make sure it continues to 
match the named network device), or should be erased and re-learned each 
time.