Re: [libvirt] [PATCH 2/9] Extend <disk> element with controller information
Daniel P. Berrange wrote:
On Fri, Sep 18, 2009 at 05:26:09PM +0200, Wolfgang Mauerer wrote:
> This allows us to connect a disk with a specific controller,
> which is required for disk hotadd/remove. A new XML child
> element is added to the <disk> container:
>
> <disk>
> ...
> <controller pci_addr="<addr>" id="<identifier>"
bus="<number>" unit="<number>"/>
> </disk>
>
> Either id _or_ pci_addr can be specified. When the controller
> has been brought into the system via tghe hotplug mechanism also
> included in this patch series, it will typically have an ID.
> In other cases, the controller can also be specified with
> the PCI address.
I think it would be desriable to make 'id' the mandatory
argument and not use 'pci_addr' in this area of the XML.
<controller id="<identifier>" bus="<number>"
unit="<number>"/>
If fact I think it'd be good to rename 'id' to 'name'
since in libvirt context we typically use 'id' to be
be positive integer identifier. 'name' reflects the
fact that this device idenifier is really a string.
I'm find with s/id/name/g.
I completely agree with you that using a name instead of
a PCI address would suit the generic spirit of libvirt much
better. However, do all hypervisors allow us to attach
individual controllers, or are there cases where
controller are implicitely added to the system? In this
case, assigning names to controllers might be impractical
or maybe impossible without further ado. So for the time
being, I would suggest to keep the pci_addr parameter,
but clearly state that name is much preferred.
Outside the scope of your patches, I think it would be
worth adding a 'name' attribute to all devices in the
libvirt XML as a standardized unique identifier. We
already have to keep track of a 'name' internally for
NIC hotplug with QEMU, and with QEMU's qdev work we're
going to end up having to track a 'name' for pretty much
all devices. Thus we might as well expose it in the XML
There's a few whitespace issues here - I guess some tabs crept
into the patch
Whops - I'll eliminate them in the next series.
Thanks, Wolfgang