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

On 01/23/2012 09:08 AM, Paolo Bonzini wrote: 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.