Re: [libvirt] [PATCH 1/5] Code to return interface name or pci_addr of the VF in actualDevice
On 06/11/2012 11:50 AM, Daniel P. Berrange wrote:
On Fri, Jun 08, 2012 at 04:29:14PM +0100, Shradha Shah wrote:
> @@ -925,6 +931,44 @@ error:
> return result;
> }
>
> +/* Function to compare strings with wildcard strings*/
> +/* When editing this function also edit the one in src/network/bridge_driver.c*/
> +static int
> +wildcmp(const char *wild, const char *string)
> +{
> +/* Written by Jack Handy - <A
href="mailto:jakkhandy@hotmail.com">jakkhandy@hotmail.com</A>*/
Isn't this just reimplementing the Glibc 'fnmatch' function that
we already use ?
Beyond that, this is being used to support another case of crowding
multiple data items into a single attribute, which we actively
discourage (see the current thread about dhcpsnoop :-).
Instead of re-purposing the singular char *dev, it would be better to
follow the example of, e.g., virDomainDeviceInfo, and have a union like
this:
int type;
union {
virDomainDevicePCIAddress pci;
char *dev;
} addr;
or just put both directly in the struct if you might want both to be
populated at the same time for some reason (maybe you want to derive all
the PCI addresses from the netdev name, or vice versa, and cache them
here. Or maybe you don't, I don't have an opinion).
The XML for this would then look something like this:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<interface type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
Or maybe, for consistency with source addresses in domain device
definitions, it should be:
<network>
<name>hostdev-network</name>
<forward mode="hostdev">
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x01'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x02'/>
<address type='pci' domain='0x0000' bus='0x04'
slot='0x00'
function='0x03'/>
</forward>
</network>
(anyone else have an opinion on this?)
As a preparation for this, you should probably have a prerequisite patch
that renames virDomainDevicePCIAddress to virDevicePCIAddress, moves it
to the new file device_conf.h, and also moves
virDomainDevicePCIAddressParseXML into device_conf.c (renaming it on the
way). Similarly, the part of virDomainHostdevSourceFormat that formats a
pci address could/should be moved into its own function in device_conf.c.
(I'm suggesting the name "device_conf.c" rather than
"pci_device_conf.c"
because I'm thinking it might be good to rename/move the other
virDomainDevice*Address structs and functions to the same place. Before
taking the time to make this reorganization, it might be good to get
feedback from some other people.)