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

[Daniel Veillard]

On Fri, Jan 15, 2010 at 04:33:01PM +0000, Daniel P. Berrange wrote:
> 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.

 okay, I though I had looked if we included the base in the call, weird.

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/