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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list