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

[Daniel P. Berrange]

On Fri, Jan 15, 2010 at 01:39:18PM +0100, Daniel Veillard wrote:
> 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:

We pass an explicit '16' to the virStrToLong_ui() so that should prevent
it doing octal  IIUC 

> 
>   <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

My intent was for these attributes to always be interpreted as hex,
no matter what, even if 0x is left off.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|