Re: [PATCHv2 05/17] conf: add new <model> subelement with type attribute to <controller>

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 17, 2015 at 02:43:32PM -0400, Laine Stump wrote:
> This new subelement is used in PCI controllers: the toplevel
> *attribute* "model" of a controller denotes what kind of PCI
> controller is being described, e.g. a "dmi-to-pci-bridge",
> "pci-bridge", or "pci-root". But in the future there will be different
> implementations of some of those types of PCI controllers, which
> behave similarly from libvirt's point of view (and so should have the
> same model), but use a different device in qemu (and present
> themselves as a different piece of hardware in the guest). In an ideal
> world we (i.e. "I") would have thought of that back when the pci
> controllers were added, and used some sort of type/class/model
> notation (where class was used in the way we are now using model, and
> model was used for the actual manufacturer's model number of a
> particular family of PCI controller), but that opportunity is long
> past, so as an alternative, this patch allows selecting a particular
> implementation of a pci controller with the "type" attribute of the
> <model> subelement, e.g.:
> 
>   <controller type='pci' model='dmi-to-pci-bridge' index='1'>
>     <model type='i82801b11-bridge'/>
>   </controller>
> 

I'd say 'type' would be more fitting for the generic class of the device
(dmi-to-pci-bridge), not the exact device model (i82801b11-bridge) so
<model name='i82801b11-bridge'/> looks nicer to me, but I'm not going to
suggest it again.

> In this case, "dmi-to-pci-bridge" is the kind of controller (one that
> has a single PCIe port upstream, and 32 standard PCI ports downstream,
> which are not hotpluggable), and the qemu device to be used to
> implement this kind of controller is named "i82801b11-bridge".
> 
> Implementing the above now will allow us in the future to add a new
> kind of dmi-to-pci-bridge that doesn't use qemu's i82801b11-bridge
> device, but instead uses something else (which doesn't yet exist, but
> qemu people have been discussing it), all without breaking existing
> configs.
> 
> (note that for the existing "pci-bridge" type of PCI controller, both
> the model attribute and <model> type are 'pci-bridge'. This is just a
> coincidence, since it turns out that in this case the device name in
> qemu really is a generic 'pci-bridge' rather than being the name of
> some real-world chip)
> ---
> new in V2 (previously was a part of the patch to add pcie-root-port)
> 
>  docs/formatdomain.html.in                       | 12 ++++++++++++
>  docs/schemas/domaincommon.rng                   | 13 +++++++++++++
>  src/conf/domain_conf.c                          | 23 +++++++++++++++++++++--
>  src/conf/domain_conf.h                          |  8 ++++++++
>  tests/qemuxml2argvdata/qemuxml2argv-q35.xml     |  8 ++++++--
>  tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml |  8 ++++++--
>  6 files changed, 66 insertions(+), 6 deletions(-)

> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 50750c1..09fe3c0 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -797,6 +797,14 @@ typedef virDomainPCIControllerOpts *virDomainPCIControllerOptsPtr;
>  struct _virDomainPCIControllerOpts {
>      bool pcihole64;
>      unsigned long pcihole64size;
> +
> +    /* the type is in the "model" subelement, e.g.:
> +     * <controller type='pci' model='pcie-root-port'>
> +     *   <model type='ioh3420''/>
> +     *   ...
> +     * similar to the model of <interface> devices.
> +     */
> +    char *type; /* the exact name of the device in hypervisor */

This would be nicer as an enum - we don't allow arbitrary strings there
anyway.

Jan

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]