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.
> diff --git a/src/domain_conf.h b/src/domain_conf.h
> index 898f6c9..6b3cb09 100644
> --- a/src/domain_conf.h
> +++ b/src/domain_conf.h
> @@ -111,6 +111,11 @@ struct _virDomainDiskDef {
> char *src;
> char *dst;
> char *controller_id;
> + struct {
> + unsigned domain;
> + unsigned bus;
> + unsigned slot;
> + } controller_pci_addr;
I think we should stick to just using the controller name
as the mandatory identifier for cross-referencing disks
to controllers.
> char *driverName;
> char *driverType;
> char *serial;
> @@ -125,6 +130,19 @@ struct _virDomainDiskDef {
> virStorageEncryptionPtr encryption;
> };
>
> +/* Stores the virtual disk controller configuration */
> +typedef struct _virDomainControllerDef virDomainControllerDef;
> +typedef virDomainControllerDef *virDomainControllerDefPtr;
> +struct _virDomainControllerDef {
> + int type;
> + char *id;
> + struct {
> + unsigned domain;
> + unsigned bus;
> + unsigned slot;
> + } pci_addr;
> +};
With the generic address data type and s/id/name/, this would be just
struct _virDomainControllerDef {
int type;
char *name;
virDomainDeviceAddress addr;
};
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 :|
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list