[libvirt] [PATCH 3/9] Add new domain device: "controller"

[Daniel P. Berrange]

On Fri, Sep 25, 2009 at 03:03:27PM +0200, Wolfgang Mauerer wrote:
> Daniel P. Berrange wrote:
> > On Fri, Sep 18, 2009 at 05:26:10PM +0200, Wolfgang Mauerer wrote:
> >> This augments virDomainDevice with a <controller> element
> >> that is used to represent disk controllers (e.g., scsi
> >> controllers). The XML format is given by
> >>
> >> <controller type="scsi" id="<my_id>">
> >>    <bus addr="<Domain>:<Bus>:<Slot>">
> >> </controller>
> >>
> >> where type denotes the disk interface (scsi, ide,...), id
> >> is an arbitrary string that identifies the controller for
> >> disk hotadd/remove, and bus addr denotes the controller address
> >> on the PCI bus.
> >>
> >> The bus element can be omitted; in this case,
> >> an address will be assigned automatically. Currently,
> >> only hotplugging scsi controllers on a PCI bus
> >> is supported, and this only for qemu guests
> > 
> > As mentioned in the previous patch, I reckon 'id' is better
> > called 'name'. 
> > 
> > For PCI addresses, it is desirable to fully normalize the XML
> > format, by which I mean have separate attributes for domain,
> > bus and slot. We already have a syntax for PCI addresses used
> > for host device passthrough, so it'd make sense to use the
> > same syntax here for controllers. More broadly, we're probably
> > going to have to add a PCI address element to all our devices.
> > While it is unlikely we'll need non-PCI addresses, it doesn't
> > hurt to make it explicit by adding a type='pci' attribute
> > 
> > Thus I'd suggest
> > 
> >     <address type='pci' domain='0x0000' bus='0x06' slot='0x12'/>
> > 
> > Instead of
> > 
> >     <bus addr="<Domain>:<Bus>:<Slot>">
> > 
> > In the domain_conf.c/.h parser, we could have a datatype like
> > 
> >    enum {
> >       VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
> > 
> >       VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST
> >    };
> > 
> >    typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress;
> >    struct _virDomainDevicePCIAddress {
> >         unsigned domain;
> >         unsigned bus;
> >         unsigned slot;
> >         unsigned function;
> >    };
> >    typedef struct _virDomainDeviceAddress virDomainDeviceAddress;
> >    struct _virDomainDeviceAddress {
> >       int type;
> >       union {
> >          virDomainDevicePCIAddress pci;
> >       } data;
> >    };
> > 
> > 
> > Which we then use in all the places in domain_conf.h wanting
> > address information. Obviously we'd not use the 'function'
> > field in most places, but doesn't hurt to have it.
> > 
> > And a pair of methods for parsing/formatting this address info
> > we can call from all the appropriate locations.
> 
> using a generic format for PCi addresses surely makes sense,
> especially wrt. the common data structure and parsing routines -
> I'll add these into the next series. However, I'm not sure
> if using <address type='pci' domain='0x0000' bus='0x06' slot='0x12'/>
> instead of a simple string provides much of an advantage
> and does not just increase the typing strain (though there are
> maybe some small advantages for computerised parsing of libvirt
> configuration files by other apps).

I think its valuable to have the consistent representation across the
different areas of the XML, and although its obviously more verbose,
it is beneficial for applications to only need an XML parser and not
have to then write further parsers for attribute values.

> There's already a generic definition of PCI addresses in
> docs/schemas/domain.rng:\approx 1140, I suppose it's your
> intention to extend if by a type field?

Yes, prettty much

Regards,
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 :|