On Fri, Jan 08, 2010 at 05:22:58PM +0000, Daniel P. Berrange wrote:
All guest devices now use a common device address structure summarized by:
enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI, };
struct _virDomainDevicePCIAddress { unsigned int domain; unsigned int bus; unsigned int slot; unsigned int function; };
struct _virDomainDeviceInfo { int type; union { virDomainDevicePCIAddress pci; } addr; };
This replaces the anonymous structs in Disk/Net/Hostdev data structures. Where available, the address is *always* printed in the XML file, instead of being hidden in the internal state file.
<address type='pci' domain='0x0000' bus='0x1e' slot='0x07' function='0x0'/>
The structure definition is based on Wolfgang Mauerer's disk controller patch series.
* docs/schemas/domain.rng: Define the <address> syntax and associate it with disk/net/hostdev devices * src/conf/domain_conf.h, src/conf/domain_conf.c, src/libvirt_private.syms: APIs for parsing/formatting address information. Also remove the QEMU specific 'pci_addr' attributes * src/qemu/qemu_driver.c: Replace use of 'pci_addr' attrs with new standardized format. --- docs/schemas/domain.rng | 55 +++++-- src/conf/domain_conf.c | 420 +++++++++++++++++++++++++++++++++------------- src/conf/domain_conf.h | 84 +++++----- src/libvirt_private.syms | 6 + src/qemu/qemu_driver.c | 64 ++++--- 5 files changed, 428 insertions(+), 201 deletions(-)
[...]
+int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, + int type) +{ + if (info->type != type) + return 0; + + switch (info->type) { + case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI: + return virDomainDevicePCIAddressIsValid(&info->addr.pci); + }
Hum, a switch without default and not handling all cases may generate a warning with some compiler options (I find this useful when extending the union) maybe we should explicitely return 0 with _NONE
+static int +virDomainDevicePCIAddressParseXML(virConnectPtr conn, + xmlNodePtr node, + virDomainDevicePCIAddressPtr addr) +{ + char *domain, *slot, *bus, *function; + int ret = -1; + + memset(addr, 0, sizeof(*addr)); + + domain = virXMLPropString(node, "domain"); + bus = virXMLPropString(node, "bus"); + slot = virXMLPropString(node, "slot"); + function = virXMLPropString(node, "function"); + + if (domain && + virStrToLong_ui(domain, NULL, 16, &addr->domain) < 0) { + virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'domain' attribute")); + goto cleanup; + }
Hum, there is a small mismatch between the parsing function and the validation, virStrToLong_ uses strtol like function decoding 0 leading numbers as octal, but we don't match octal in the Relax-NG associated functions: <define name="pciDomain"> <data type="string"> <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> </data> </define> Do we really intent to allow 0 started octal values ? I guess in octal we would need more than 4 digit to cover the address range. But it's a minor point, we could fix the RNG later ACK, the new routines are nice and the refactoring is a good cleanup ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/