On 04/03/2013 11:50 AM, Ján Tomko wrote:
> From: liguang <lig.fnst@xxxxxxxxxxxxxx>
>
> add a new controller type, then one can
> define a pci-bridge controller like this:
> <controller type='pci-bridge' index='0'/>
In the next patch we're prohibiting exactly this config (index='0')
because the pre-existing pci bus on the "pc-*" machinetypes is already
named pci.0. If we don't allow it, we shouldn't include it as an example
in the commit log :-)
More on this - one of the things this points out is that there is no
representation in the config of the pci.0 bus, it's just assumed to
always be there. That is the case for pc-* machinetypes (and probably
several others with PCI buses), but for q35, there is no pci.0 bus in
the basic machine, only a pcie.0; if you want a pci.0 on q35 (which
*will* be necessary in order to attach any pci devices, so I imagine we
will always want one), you have to attach a pcie->pci bridge, which is
the device "i82801b11-bridge", to pcie.0.
The reason I bring this up here, is I'm wondering:
1) should we have some representation of the default pci.0 bus in the
config, even though it is just "always there" for the pc machinetypes
and there is no way to disable it, and nothing on the commandline that
specifies its existence?
2) For the q35 machinetype, should we just always add an
i82801b11-bridge device and name it pci.0? Or should that need to be
present in the xml?
3) Most important - depending on the answers to (1) and (2), should we
maybe name this device "pci", and use a different backend depending on
index and machinetype? (or alternately explicitly specifiable with a
<driver> subelement). To be specific, we would have:
<controller type='pci' index='0'/>
which on pc machinetypes would just be a placeholder in the config (and
always inserted if it wasn't there, for machinetypes that have a pci
bus). On the q35 machinetype, that same line would equate to adding an
i82801b11-bridge device (with source defaulting to
bus=pcie.0,addr=1e.0). This would serve several purposes:
a) on pc machinetypes, it would be a visual aid indicating that pci.0
exists, and that index='0' isn't available for a new pci controller.
b) it would make switching a domain config from pc to q35 simpler, since
pci.0 would always already be in place for attaching pci devices
(including pci.1, pci.2, etc)
c) it would make the config a true complete description of the machine
being created.
(I've suggested naming the controller "pci" rather than "pci-bridge"
because in the case of a "root" bus like pci.0 it seems to not be a
"bridge", but maybe the name "pci-bridge" is always appropriate, even
when it's a root bus. Maybe someone with better pci/pcie knowledge can
provide an opinion on this)
> <controller type='pci-bridge' index='1'>
> <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
> </controller>
> actually, it works as a pci-bus, so as to support
> multi-pci-bus via pci-to-pci bridge
>
> Signed-off-by: liguang <lig.fnst@xxxxxxxxxxxxxx>
> ---
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c | 3 ++-
> src/conf/domain_conf.h | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 8d7e6db..b6dc013 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1357,6 +1357,7 @@
> <value>sata</value>
> <value>ccid</value>
> <value>usb</value>
> + <value>pci-bridge</value>
> </choice>
> </attribute>
> </optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cc26f21..6a990bb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -295,7 +295,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
> "sata",
> "virtio-serial",
> "ccid",
> - "usb")
> + "usb",
> + "pci-bridge")
>
> VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
> "auto",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index edddf25..1ec8564 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -682,6 +682,7 @@ enum virDomainControllerType {
> VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL,
> VIR_DOMAIN_CONTROLLER_TYPE_CCID,
> VIR_DOMAIN_CONTROLLER_TYPE_USB,
> + VIR_DOMAIN_CONTROLLER_TYPE_PCI_BRIDGE,
>
> VIR_DOMAIN_CONTROLLER_TYPE_LAST
> };
If nobody thinks making the name of the controller "pci" instead of
"pci-bridge" makes sense, then ACK once the commit log has the bad
example removed. (We'll need to make the default initial index be 1 for
these instead of 0, but I think that should go in the next patch anyway,
because as I said above, there may be a use for a pci controller with
index='0').
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list