[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [libvirt] [PATCH 3/9] Add new domain device: "controlle
From: |
Daniel P. Berrange |
Subject: |
[Qemu-devel] Re: [libvirt] [PATCH 3/9] Add new domain device: "controller" |
Date: |
Thu, 24 Sep 2009 11:52:14 +0100 |
User-agent: |
Mutt/1.4.1i |
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 :|
- [Qemu-devel] [PATCH 0/9] Support disk-hotremove and controller hotplugging, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 7/9] Remove surprises in the semantics of disk-hotadd, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 4/9] Add disk-based hotplugging for the qemu backend, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 3/9] Add new domain device: "controller", Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] Re: [libvirt] [PATCH 3/9] Add new domain device: "controller",
Daniel P. Berrange <=
- [Qemu-devel] [PATCH 9/9] Implement disk- and controller hotremove, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 5/9] Implement controller hotplugging, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 2/9] Extend <disk> element with controller information, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 6/9] Allow controller selection by ID, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 1/9] Clarify documentation for private symbols, Wolfgang Mauerer, 2009/09/19
- [Qemu-devel] [PATCH 8/9] Factor out the method to get the PCI address of a controller for a given disk, Wolfgang Mauerer, 2009/09/19