[libvirt] [PATCH 02/34] Introduce a standardized data structure for device addresses

[Daniel Veillard]

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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/