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

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.