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 :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
