8:44 p.m.
The first 4 patches are bugfixes/reorganizations that have no controversy.
The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI
controller:
5-7 - <controller type='pci' model='pcie-root-port'/>
This is based on qemu's ioh3420.
8-10 - <controller type='pci' model='pcie-switch-upstream-port'/>
Based on qemu's x3130-upstream
11-13 - <controller type='pci'
model='pcie-switch-downstream-port'/>
(xio3130-downstream)
The first patch of each set adds a capability bit for qemu (again
non-controversial), the 2nd adds the new pci controller model, and the
3rd implements that model in qemu (by checking for the capability and
adding a commandline arg or failing).
The "controversial"/RFC bit is this - talking to Alex Williamson
(after I'd rwritten these patches, which is why I'm presenting them in
a form that I *don't* want to push) about the possibility of qemu
adding generic root-port, switch-upstream-port, and
switch-downstream-port controllers, and possibly also a generic
dmi-to-pci-bridge (i.e. controllers not tied to any particular
hardware implementation as with those currently available), I'm
realizing that, while it was a correct decision to make all of the
existing pci controllers "type='pci'" (since they share an address
space), using the "model" attribute to set the kind of controller was
probably a mistake. The problem - if:
<controller type='pci' model='dmi-to-pci-bridge'/>
currently means to add an i82801b11-bridge controller to the domain,
once qemu implements a generic dmi-to-pci-bridge, how will *that* be
denoted, and how will we avoid replacing the existing i81801b11-bridge
in a particular domain with the generic version when a guest is
restarted following a qemu/libvirt upgrade?
In hindsight, it probably would have been better to do something like
this with the four existing pci controllers:
<controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
<controller type='pci' subType='pci-bridge'
model='pci-bridge'/> (or maybe blank?)
<controller type='pci' subType='pci-root'/> (again maybe model is
blank)
<controller type='pci' subType='pcie-root'/>(and again)
(instead, what is shown above as "subType" is currently placed in the
"model" attribute).
Then we could add the 3 new types like this:
<controller type='pci' subType='pcie-root-port'
model='ioh3420'/>
<controller type='pci' subType='pcie-switch-upstream-port'
model='x3130-upstream/>
<controller type='pci' subType='pcie-switch-downstream-port'
model='xio3130-downstream/>
and we would easily be able to add support for new generic controllers
that behaved identically, by just adding a new model. But we haven't
done that, and anything we do in the future must be backwards
compatible with what's already there (right?). I'm trying to think of
how to satisfy backward compatibility while making things work better
in the future.
Some ideas, in no particular order:
===
Idea 1: multiplex the meaning of the "model" attribute, so we currently have:
<controller type='pci' model='dmi-to-pci-bridge'/>
which means "add an i82801b11-bridge device" and when we add a generic
version of this type of controller, we would do it with something like:
<controller type='pci' model='generic-dmi-to-pci-bridge'/>
and for another vendor's mythical controller:
<controller type='pci' model='xyz-dmi-to-pci-bridge'/>
Cons: This will make for ugliness in switch statements where a new
case will have to be added whenever a different controller with
similar behavior/usage is supported. And it's generally not a good idea to
have a single attribute be used for two different functions.
===
Idea 2: implement new controllers as suggested in "20/20 hindsight"
above. For controllers in existing domains (dmi-to-pci-bridge,
pic-bridge, pci-root, and pcie-root) imply it into the controller
definition of an existing domain when only model has been given (but
don't write it out that way, to preserve the ability to downgrade). So
this:
[1] <controller type='pci' model='dmi-to-pci-bridge'/>
would internally mean this:
[2] <controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
(but would remain as [1] when config is rewritten/migrated) while
this:
[3] <controller type='pci' subType='dmi-to-pci-bridge'
model='anything whatsoever/>
would mean exactly what it says.
Cons: Keeping this straight would mean having some sort of
"oldStyleCompat" flag in the controller object, to be sure that [1]
wasn't sent in migration status as [2] (since the destination might
not recognize it). It would also mean keeping the code in the parser
and formatter to deal with this flag. Forever.
===
Idea 3: interpret controllers with missing subType as above, but
actually write it out to the config/migration/etc in the new modified
format.
Cons: This would prevent downgrading libvirt or migrating from a host
with newer libvirt to one with older libvirt. (Although preserving
compatibility at some level when downgrading may be a stated
requirement of some downstream distros' builds of libvirt, I think for
upstream it is only a "best effort"; I'm just not certain how much
"best" is considered to be :-)
===
Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
continue to use "model" to denote the subtype/class/whatever of
controller, and create a new attribute to denote the different
specific implementations of each one. So for example:
[4] <controller type='pci' model='dmi-to-pci-bridge'/>
would become:
[5] <controller type='pci' model='dmi-to-pci-bridge'
implementation='i82801b11-bridge'/>
(or some other name in place of "implementation" - ideas? I'm horrible
at thinkin up names)
Pros: wouldn't create compatibility problems when downgrading or
migrating cross version.
Cons: Is inconsistent with every other use of "model" attribute in
libvirt, and each new addition of a PCI controller further propagates
this misuse.
====
I currently like either option 2 or 3 (depending on how good we want
to be about supporting downgrade/intra-version migration), but as is
obvious by the fact that it was me that suggested putting the type of
pci controller into "model", I am very good at making the wrong
decision on matters like this.
Whatever everyone thinks is best, patches 5-13 of this series would be
changed accordingly, and possibly a couple new patches would be made
to adjust the 4 existing controller types.
Note that this will also effect the libvirt support for the upcoming
qemu "pxb" controller, which is a PCI root bus that can be placed in
its own numa node (my description may be incorrect, but I think it
gets the idea across). Anyway, with idea 2 or 3 it would be:
<controller type='pci' subType='pci-root' model='pxb'/>
(along with some options to setup numa info).
So, along with any comments on the individual patches (including
whether the specific args added to the qemu commandline are correct -
I'm looking at you, qemu people!), I would appreciate opinions on how
I can save us from this misuse of "model" that I've created.
Laine Stump (13):
qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate
code
qemu: always permit PCI devices to be manually assigned to a PCIe bus
qemu: ignore assumptions about hotplug requirement when address is
from config
docs: document when pcie-root/dmi-to-pci-bridge support was added
qemu: add capabilities bit for device ioh3420
conf: new pci controller model "pcie-root-port"
qemu: support new pci controller model "pcie-root-port"
qemu: add capabilities bit for device x3130-upstream
conf: new pci controller model "pcie-switch-upstream-port"
qemu: support new pci controller model "pcie-switch-upstream-port"
qemu: add capabilities bit for device xio3130-downstream
conf: new pcie-controller model "pcie-switch-downstream-port"
qemu: support new pci controller model "pcie-switch-downstream-port"
docs/formatdomain.html.in | 48 +++++++++---
docs/schemas/domaincommon.rng | 3 +
src/conf/domain_addr.c | 47 ++++++++++--
src/conf/domain_addr.h | 23 ++++--
src/conf/domain_conf.c | 5 +-
src/conf/domain_conf.h | 3 +
src/qemu/qemu_capabilities.c | 8 +-
src/qemu/qemu_capabilities.h | 5 +-
src/qemu/qemu_command.c | 86 +++++++++++++++++-----
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 3 +
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 3 +
tests/qemuhelptest.c | 10 ++-
.../qemuxml2argv-pcie-root-port.args | 10 +++
.../qemuxml2argv-pcie-root-port.xml | 33 +++++++++
.../qemuxml2argv-pcie-switch-downstream-port.args | 13 ++++
.../qemuxml2argv-pcie-switch-downstream-port.xml | 36 +++++++++
.../qemuxml2argv-pcie-switch-upstream-port.args | 9 +++
.../qemuxml2argv-pcie-switch-upstream-port.xml | 32 ++++++++
tests/qemuxml2argvtest.c | 25 +++++++
tests/qemuxml2xmltest.c | 3 +
25 files changed, 375 insertions(+), 45 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 01/13] qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate code
The PCI case of the switch statement in this function contains another
switch statement with a case for each model. Currently every model
except pci-root and pcie-root have a check for index > 0 (since only
those two can have index==0), and the function should never be called
for those two anyway. If we move the check for !pci[e]-root to the top
of the pci case, then we can move the check for index > 0 out of the
individual model cases. This will save repeating that check for the
three new controller models about to be added.
---
src/qemu/qemu_command.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3886b4f..5e5cfe6 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4581,13 +4581,20 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
break;
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
+ def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("wrong function called for pci-root/pcie-root"));
+ return NULL;
+ }
+ if (def->idx == 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("index for pci controllers of model '%s' must
be > 0"),
+ virDomainControllerModelPCITypeToString(def->model));
+ goto error;
+ }
switch (def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- if (def->idx == 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("PCI bridge index should be > 0"));
- goto error;
- }
virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=%s",
def->idx, def->info.alias);
break;
@@ -4598,18 +4605,8 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
"controller is not supported in this QEMU
binary"));
goto error;
}
- if (def->idx == 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("dmi-to-pci-bridge index should be > 0"));
- goto error;
- }
virBufferAsprintf(&buf, "i82801b11-bridge,id=%s",
def->info.alias);
break;
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("wrong function called for pci-root/pcie-root"));
- return NULL;
}
break;
--
2.1.0
10:52 a.m.
New subject: [libvirt] [PATCH 01/13] qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate code
On Mon, Jun 22, 2015 at 02:44:06PM -0400, Laine Stump wrote:
The PCI case of the switch statement in this function contains
another
switch statement with a case for each model. Currently every model
except pci-root and pcie-root have a check for index > 0 (since only
every model has
those two can have index==0), and the function should never be
called
for those two anyway. If we move the check for !pci[e]-root to the top
of the pci case, then we can move the check for index > 0 out of the
individual model cases. This will save repeating that check for the
three new controller models about to be added.
---
src/qemu/qemu_command.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
ACK
Jan
8:44 p.m.
New subject: [libvirt] [PATCH 02/13] qemu: always permit PCI devices to be manually assigned to a PCIe bus
When support for the pcie-root and dmi-to-pci-bridge buses on a Q35
machinetype was added, I was concerned that even though qemu at the
time allowed plugging a PCI device into a PCIe port, that it might not
be supported in the future. To prevent painful backtracking in the
possible future where this happened, I disallowed such connections
except in a few specific cases requested by qemu developers (indicated
in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG).
Now that a couple years have passed, there is a clear message from
qemu that there is no danger in allowing PCI devices to be plugged
into PCIe ports. This patch eliminates
VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always
allow PCI->PCIe or PCIe->PCI connection *when the PCI address is
specified in the config. (For newly added devices that haven't yet
been given a PCI address, the auto-placement still prefers using the
correct type of bus).
---
docs/formatdomain.html.in | 18 +++++++++++-------
src/conf/domain_addr.c | 18 +++++++++++-------
src/conf/domain_addr.h | 16 ++++++++++------
src/qemu/qemu_command.c | 6 ++----
4 files changed, 34 insertions(+), 24 deletions(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 4e85b51..1dca2ac 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -3034,17 +3034,21 @@
bus (for example, the machine types based on the Q35 chipset),
the pcie-root controller with index=0 is auto-added to the
domain's configuration. pcie-root has also no address, provides
- 31 slots (numbered 1-31) and can only be used to attach PCIe
- devices. In order to connect standard PCI devices on a system
- which has a pcie-root controller, a pci controller
+ 31 slots (numbered 1-31) that can be used to attach PCIe or PCI
+ devices (although libvirt will never auto-assign a PCI device to
+ a PCIe slot, it will allow manual specification of such an
+ assignment). Devices connected to pcie-root cannot be
+ hotplugged. In order to make standard PCI slots available on a
+ system which has a pcie-root controller, a pci controller
with <code>model='dmi-to-pci-bridge'</code> is automatically
- added. A dmi-to-pci-bridge controller plugs into a PCIe slot (as
- provided by pcie-root), and itself provides 31 standard PCI
- slots (which are not hot-pluggable). In order to have
+ added, usually at the defacto standard location of slot=0x1e. A
+ dmi-to-pci-bridge controller plugs into a PCIe slot (as provided
+ by pcie-root), and itself provides 31 standard PCI slots (which
+ also do not support device hotplug). In order to have
hot-pluggable PCI slots in the guest system, a pci-bridge
controller will also be automatically created and connected to
one of the slots of the auto-created dmi-to-pci-bridge
- controller; all guest devices with PCI addresses that are
+ controller; all guest PCI devices with addresses that are
auto-determined by libvirt will be placed on this pci-bridge
device. (<span class="since">since 1.1.2</span>).
</p>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index d657c7f..93c6043 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -1,7 +1,7 @@
/*
* domain_addr.c: helper APIs for managing domain device addresses
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -42,16 +42,21 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
{
virErrorNumber errType = (fromConfig
? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
- virDomainPCIConnectFlags flagsMatchMask = VIR_PCI_CONNECT_TYPES_MASK;
- if (fromConfig)
- flagsMatchMask |= VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG;
+ if (fromConfig) {
+ /* If the requested connection was manually specified in
+ * config, allow a PCI device to connect to a PCIe slot, or
+ * vice versa.
+ */
+ if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT)
+ busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT;
+ }
/* If this bus doesn't allow the type of connection (PCI
* vs. PCIe) required by the device, or if the device requires
* hot-plug and this bus doesn't have it, return false.
*/
- if (!(devFlags & busFlags & flagsMatchMask)) {
+ if (!(devFlags & busFlags & VIR_PCI_CONNECT_TYPES_MASK)) {
if (reportError) {
if (devFlags & VIR_PCI_CONNECT_TYPE_PCI) {
virReportError(errType,
@@ -174,8 +179,7 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
* specified in user config *and* the particular device being
* attached also allows it
*/
- bus->flags = (VIR_PCI_CONNECT_TYPE_PCIE |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE;
bus->minSlot = 1;
bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
break;
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index c18e130..dcfd2e5 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -1,7 +1,7 @@
/*
* domain_addr.h: helper APIs for managing domain device addresses
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -39,10 +39,6 @@ typedef enum {
/* PCI devices can connect to this bus */
VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3,
/* PCI Express devices can connect to this bus */
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4,
- /* PCI *and* PCIe devices allowed, if the address
- * was specified in the config by the user
- */
} virDomainPCIConnectFlags;
typedef struct {
@@ -70,12 +66,20 @@ struct _virDomainPCIAddressSet {
typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet;
typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
-/* a combination of all bit that describe the type of connections
+/* a combination of all bits that describe the type of connections
* allowed, e.g. PCI, PCIe, switch
*/
# define VIR_PCI_CONNECT_TYPES_MASK \
(VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
+/* combination of all bits that could be used to connect a normal
+ * endpoint device (i.e. excluding the connection possible between an
+ * upstream and downstream switch port, or a PCIe root port and a PCIe
+ * port)
+ */
+# define VIR_PCI_CONNECT_TYPES_ENDPOINT \
+ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
+
char *virDomainPCIAddressAsString(virDevicePCIAddressPtr addr)
ATTRIBUTE_NONNULL(1);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 5e5cfe6..4952797 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1621,8 +1621,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI2:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI3:
case VIR_DOMAIN_CONTROLLER_MODEL_USB_VT82C686B_UHCI:
- flags = (VIR_PCI_CONNECT_TYPE_PCI |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ flags = VIR_PCI_CONNECT_TYPE_PCI;
break;
case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
/* should this be PCIE-only? Or do we need to allow PCI
@@ -1643,8 +1642,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
switch (device->data.sound->model) {
case VIR_DOMAIN_SOUND_MODEL_ICH6:
case VIR_DOMAIN_SOUND_MODEL_ICH9:
- flags = (VIR_PCI_CONNECT_TYPE_PCI |
- VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ flags = VIR_PCI_CONNECT_TYPE_PCI;
break;
}
break;
--
2.1.0
10:54 a.m.
New subject: [libvirt] [PATCH 02/13] qemu: always permit PCI devices to be manually assigned to a PCIe bus
On Mon, Jun 22, 2015 at 02:44:07PM -0400, Laine Stump wrote:
When support for the pcie-root and dmi-to-pci-bridge buses on a Q35
machinetype was added, I was concerned that even though qemu at the
time allowed plugging a PCI device into a PCIe port, that it might not
be supported in the future. To prevent painful backtracking in the
possible future where this happened, I disallowed such connections
except in a few specific cases requested by qemu developers (indicated
in the code with the flag VIR_PCI_CONNEcT_TYPE_EITHER_IF_CONFIG).
s/CONNEcT/CONNECT/g
Now that a couple years have passed, there is a clear message from
qemu that there is no danger in allowing PCI devices to be plugged
into PCIe ports. This patch eliminates
VIR_PCI_CONNECT_TYPE_EITHER_IF_CONFIG and changes the code to always
allow PCI->PCIe or PCIe->PCI connection *when the PCI address is
specified in the config. (For newly added devices that haven't yet
been given a PCI address, the auto-placement still prefers using the
correct type of bus).
---
docs/formatdomain.html.in | 18 +++++++++++-------
src/conf/domain_addr.c | 18 +++++++++++-------
src/conf/domain_addr.h | 16 ++++++++++------
src/qemu/qemu_command.c | 6 ++----
4 files changed, 34 insertions(+), 24 deletions(-)
ACK
Jan
8:44 p.m.
New subject: [libvirt] [PATCH 03/13] qemu: ignore assumptions about hotplug requirement when address is from config
Certain PCI buses don't support hotplug, and when automatically
assigning PCI addresses for devices, libvirt is very concervative in
its assumptions about whether or not a device will need to be
hotplugged/unplugged in the future. But if the user manually assigns
an address, they likely are aware of any hotplug requirements of the
device (or at least they should be).
In short, after this patch, automatically PCI address assignment will
assume that the device must be pluggedin ot a hot-pluggable slot, but
manually assignment can place the device in any bus that is
compatible, regardless of whether or not it supports hotplug. If the
user makes a mistake and plugs the device into a bus that doesn't
support hotplug, then later tries to do a hot-unplug, qemu will give
an appropriate error.
(in the future we may want to add a "hotpluggable" attribute to all
devices, with default being "yes" for autoassign, and "no" for manual
assign).
---
src/conf/domain_addr.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 93c6043..2be98c5 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -50,6 +50,12 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
*/
if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT)
busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT;
+ /* Also allow manual specification of bus to override
+ * libvirt's assumptions about whether or not hotplug
+ * capability will be required.
+ */
+ if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
+ busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
}
/* If this bus doesn't allow the type of connection (PCI
--
2.1.0
11 a.m.
New subject: [libvirt] [PATCH 03/13] qemu: ignore assumptions about hotplug requirement when address is from config
On Mon, Jun 22, 2015 at 02:44:08PM -0400, Laine Stump wrote:
Certain PCI buses don't support hotplug, and when automatically
assigning PCI addresses for devices, libvirt is very concervative in
conservative
its assumptions about whether or not a device will need to be
hotplugged/unplugged in the future. But if the user manually assigns
an address, they likely are aware of any hotplug requirements of the
device (or at least they should be).
In short, after this patch, automatically PCI address assignment will
assume that the device must be pluggedin ot a hot-pluggable slot, but
plugged in to
manually assignment can place the device in any bus that is
compatible, regardless of whether or not it supports hotplug. If the
user makes a mistake and plugs the device into a bus that doesn't
support hotplug, then later tries to do a hot-unplug, qemu will give
an appropriate error.
(in the future we may want to add a "hotpluggable" attribute to all
devices, with default being "yes" for autoassign, and "no" for
manual
assign).
---
src/conf/domain_addr.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 93c6043..2be98c5 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -50,6 +50,12 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
*/
if (busFlags & VIR_PCI_CONNECT_TYPES_ENDPOINT)
busFlags |= VIR_PCI_CONNECT_TYPES_ENDPOINT;
Here, it's the bus that allows plugging both PCI and PCIe devices in.
+ /* Also allow manual specification of bus to override
+ * libvirt's assumptions about whether or not hotplug
+ * capability will be required.
+ */
+ if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
+ busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
But the bus might not be hotpluggable here. We just don't care
about being able to hotplug the device.
How about:
devFlags ^= VIR_PCI_CONNECT_HOTPLUGGABLE;
ACK either way
Jan
8:44 p.m.
New subject: [libvirt] [PATCH 04/13] docs: document when pcie-root/dmi-to-pci-bridge support was added
Also move the mention of version numbers for the various PCI
controller models up to the end of the sentence where they are first
given, to avoid confusion.
---
docs/formatdomain.html.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 1dca2ac..fb3e2fb 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2998,6 +2998,9 @@
PCI controllers have an optional <code>model</code> attribute with
possible values <code>pci-root</code>,
<code>pcie-root</code>,
<code>pci-bridge</code>, or
<code>dmi-to-pci-bridge</code>.
+ (pci-root and pci-bridge <span class="since">since
1.0.5</span>,
+ pcie-root and dmi-to-pci-bridge <span class="since">since
+ 1.1.2</span>)
The root controllers (<code>pci-root</code> and
<code>pcie-root</code>)
have an optional <code>pcihole64</code> element specifying how big
(in kilobytes, or in the unit specified by
<code>pcihole64</code>'s
@@ -3017,7 +3020,6 @@
only refer to PCI buses provided by already specified PCI controllers.
Leaving gaps in the PCI controller indexes might lead to an invalid
configuration.
- (pci-root and pci-bridge <span class="since">since
1.0.5</span>)
</p>
<pre>
...
--
2.1.0
11 a.m.
New subject: [libvirt] [PATCH 04/13] docs: document when pcie-root/dmi-to-pci-bridge support was added
On Mon, Jun 22, 2015 at 02:44:09PM -0400, Laine Stump wrote:
Also move the mention of version numbers for the various PCI
controller models up to the end of the sentence where they are first
given, to avoid confusion.
---
docs/formatdomain.html.in | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
ACK
Jan
8:44 p.m.
New subject: [libvirt] [PATCH 05/13] qemu: add capabilities bit for device ioh3420
This is a PCIE "root port". It connects only to a port of the
integrated pcie.0 bus of a Q35 machine (can't be hotplugged), and
provides a single PCIe port that can have PCI or PCIe devices
hotplugged into it.
This device will be used to implement the "pcie-root-port" model of
pci controller.
---
src/qemu/qemu_capabilities.c | 4 +++-
src/qemu/qemu_capabilities.h | 3 ++-
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemuhelptest.c | 6 ++++--
10 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 27a632a..a9a19e1 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -1,7 +1,7 @@
/*
* qemu_capabilities.c: QEMU capabilities generation
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"aarch64-off",
"vhost-user-multiqueue", /* 190 */
+ "ioh3420",
);
@@ -1566,6 +1567,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "ivshmem", QEMU_CAPS_DEVICE_IVSHMEM },
{ "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM },
{ "pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL },
+ { "ioh3420", QEMU_CAPS_DEVICE_IOH3420 },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 30aa504..d38505e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -1,7 +1,7 @@
/*
* qemu_capabilities.h: QEMU capabilities generation
*
- * Copyright (C) 2006-2014 Red Hat, Inc.
+ * Copyright (C) 2006-2015 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -230,6 +230,7 @@ typedef enum {
QEMU_CAPS_DEVICE_PCI_SERIAL = 188, /* -device pci-serial */
QEMU_CAPS_CPU_AARCH64_OFF = 189, /* -cpu ...,aarch64=off */
QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
+ QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
index 30239df..a1fafa6 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
@@ -120,4 +120,5 @@
<flag name='vmware-svga.vgamem_mb'/>
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
index ea3d850..824ef02 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
@@ -135,4 +135,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
index 2c19ddc..7ef5199 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
@@ -136,4 +136,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
index aadccd5..8c3d48e 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
@@ -145,4 +145,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index 3e81cbf..72f7625 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -151,4 +151,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 84c357f..d81c80c 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -151,4 +151,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
index b1ee8df..1a39dce 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
@@ -167,4 +167,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pc-dimm'/>
<flag name='pci-serial'/>
+ <flag name='ioh3420'/>
</qemuCaps>
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 507831c..6211640 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -753,7 +753,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_KBD,
QEMU_CAPS_DEVICE_USB_STORAGE,
QEMU_CAPS_SPLASH_TIMEOUT,
- QEMU_CAPS_DEVICE_IVSHMEM);
+ QEMU_CAPS_DEVICE_IVSHMEM,
+ QEMU_CAPS_DEVICE_IOH3420);
DO_TEST("qemu-1.1.0", 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -853,7 +854,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_STORAGE,
QEMU_CAPS_OBJECT_USB_AUDIO,
QEMU_CAPS_SPLASH_TIMEOUT,
- QEMU_CAPS_DEVICE_IVSHMEM);
+ QEMU_CAPS_DEVICE_IVSHMEM,
+ QEMU_CAPS_DEVICE_IOH3420);
DO_TEST_FULL("qemu-1.2.0", 1002000, 0, 0, VIR_ERR_CONFIG_UNSUPPORTED,
QEMU_CAPS_LAST);
DO_TEST_FULL("qemu-kvm-1.2.0", 1002000, 1, 0, VIR_ERR_CONFIG_UNSUPPORTED,
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 06/13] conf: new pci controller model "pcie-root-port"
This controller can be connected (at domain startup time only - not
hotpluggable) only to a port on the pcie root complex ("pcie-root" in
libvirt config), hence the new connect type
VIR_PCI_CONNECT_TYPE_PCIE_ROOT. It provides a hotpluggable port that
will accept any PCI or PCIe device.
---
docs/formatdomain.html.in | 16 +++++++++--
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_addr.c | 10 +++++--
src/conf/domain_addr.h | 5 +++-
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-pcie-root-port.xml | 33 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
8 files changed, 64 insertions(+), 6 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index fb3e2fb..e6c140a 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2997,10 +2997,11 @@
<p>
PCI controllers have an optional <code>model</code> attribute with
possible values <code>pci-root</code>,
<code>pcie-root</code>,
- <code>pci-bridge</code>, or
<code>dmi-to-pci-bridge</code>.
+ <code>pcie-root-port</code>, <code>pci-bridge</code>,
+ or <code>dmi-to-pci-bridge</code>.
(pci-root and pci-bridge <span class="since">since
1.0.5</span>,
pcie-root and dmi-to-pci-bridge <span class="since">since
- 1.1.2</span>)
+ 1.1.2</span>, pcie-root-port <span class="since">since
1.2.17</span>)
The root controllers (<code>pci-root</code> and
<code>pcie-root</code>)
have an optional <code>pcihole64</code> element specifying how big
(in kilobytes, or in the unit specified by
<code>pcihole64</code>'s
@@ -3054,6 +3055,17 @@
auto-determined by libvirt will be placed on this pci-bridge
device. (<span class="since">since 1.1.2</span>).
</p>
+ <p>
+ Domains with an implicit pcie-root can also add controllers
+ with <code>model='pcie-root-port'</code>. This is a simple type
of
+ bridge device that can connect only to one of the 31 slots on
+ the pcie-root bus on its upstream side, and makes a single
+ (PCIe, hotpluggable) port (at slot='0') available on the
+ downstream side. This controller can be used to provide a single
+ slot to later hotplug a PCIe device (but is not itself
+ hotpluggable - it must be in the configuration when the domain
+ is started). (<span class="since">since 1.2.17</span>)
+ </p>
<pre>
...
<devices>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f0f7400..6139dfb 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1751,6 +1751,7 @@
<choice>
<value>pci-bridge</value>
<value>dmi-to-pci-bridge</value>
+ <value>pcie-root-port</value>
</choice>
</attribute>
</group>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 2be98c5..4b5e81e 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -183,9 +183,9 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
/* slots 1 - 31, no hotplug, PCIe only unless the address was
* specified in user config *and* the particular device being
- * attached also allows it
+ * attached also allows it.
*/
- bus->flags = VIR_PCI_CONNECT_TYPE_PCIE;
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
bus->minSlot = 1;
bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
break;
@@ -196,6 +196,12 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
bus->minSlot = 1;
bus->maxSlot = VIR_PCI_ADDRESS_SLOT_LAST;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ /* provides one slot which is pcie and hotpluggable */
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE;
+ bus->minSlot = 1;
+ bus->maxSlot = 1;
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PCI controller model %d"), model);
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index dcfd2e5..2a0ff96 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -39,6 +39,8 @@ typedef enum {
/* PCI devices can connect to this bus */
VIR_PCI_CONNECT_TYPE_PCIE = 1 << 3,
/* PCI Express devices can connect to this bus */
+ VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 << 4,
+ /* for devices that can only connect to pcie-root (i.e. root-port) */
} virDomainPCIConnectFlags;
typedef struct {
@@ -70,7 +72,8 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
* allowed, e.g. PCI, PCIe, switch
*/
# define VIR_PCI_CONNECT_TYPES_MASK \
- (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE)
+ (VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \
+ VIR_PCI_CONNECT_TYPE_PCIE_ROOT)
/* combination of all bits that could be used to connect a normal
* endpoint device (i.e. excluding the connection possible between an
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca55981..0bdd51e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -323,7 +323,8 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pci-root",
"pcie-root",
"pci-bridge",
- "dmi-to-pci-bridge")
+ "dmi-to-pci-bridge",
+ "pcie-root-port")
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 ba17a8d..946210e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -752,6 +752,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
} virDomainControllerModelPCI;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
new file mode 100644
index 0000000..a1fc354
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
@@ -0,0 +1,33 @@
+<domain type='qemu'>
+ <name>q35-test</name>
+ <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>2097152</currentMemory>
+ <vcpu placement='static' cpuset='0-1'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'/>
+ <controller type='pci' index='2' model='pci-bridge'/>
+ <controller type='pci' index='3'
model='pcie-root-port'/>
+ <controller type='pci' index='4'
model='pcie-root-port'/>
+ <controller type='sata' index='0'/>
+ <video>
+ <model type='qxl' ram='65536' vram='32768'
vgamem='8192' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 44b388c..2acbae1 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -566,6 +566,7 @@ mymain(void)
DO_TEST_DIFFERENT("pci-autoadd-idx");
DO_TEST_DIFFERENT("pcie-root");
DO_TEST_DIFFERENT("q35");
+ DO_TEST("pcie-root-port");
DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 07/13] qemu: support new pci controller model "pcie-root-port"
This is backed by the qemu device ioh3420.
---
src/qemu/qemu_command.c | 20 ++++++++++++++++++++
.../qemuxml2argv-pcie-root-port.args | 10 ++++++++++
tests/qemuxml2argvtest.c | 7 +++++++
3 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4952797..01ac690 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1598,6 +1598,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ /* pcie-root-port can only connect to pcie-root, isn't
+ * hot-pluggable
+ */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+ break;
default:
break;
}
@@ -2345,6 +2351,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ /* pcie-root-port can only plug into pcie-root */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+ break;
default:
flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
break;
@@ -4605,6 +4615,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
}
virBufferAsprintf(&buf, "i82801b11-bridge,id=%s",
def->info.alias);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-root-port (ioh3420) controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf, "ioh3420,port=%d,chassis=%d,id=%s",
+ def->idx, def->idx, def->info.alias);
+ break;
}
break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
new file mode 100644
index 0000000..30bec85
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
@@ -0,0 +1,10 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-device ioh3420,port=3,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
+-device ioh3420,port=4,chassis=4,id=pci.4,bus=pcie.0,addr=0x3 \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
+-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a90f9a6..999edff 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1461,6 +1461,13 @@ mymain(void)
QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("pcie-root-port",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
--
2.1.0
5:31 p.m.
New subject: [libvirt] [PATCH 07/13] qemu: support new pci controller model "pcie-root-port"
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
This is backed by the qemu device ioh3420.
---
src/qemu/qemu_command.c | 20 ++++++++++++++++++++
.../qemuxml2argv-pcie-root-port.args | 10 ++++++++++
tests/qemuxml2argvtest.c | 7 +++++++
3 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4952797..01ac690 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1598,6 +1598,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ /* pcie-root-port can only connect to pcie-root, isn't
+ * hot-pluggable
+ */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+ break;
default:
break;
}
@@ -2345,6 +2351,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ /* pcie-root-port can only plug into pcie-root */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
+ break;
default:
flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
break;
@@ -4605,6 +4615,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
}
virBufferAsprintf(&buf, "i82801b11-bridge,id=%s",
def->info.alias);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-root-port (ioh3420) controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf, "ioh3420,port=%d,chassis=%d,id=%s",
+ def->idx, def->idx, def->info.alias);
So you didn't like the idea of using the flat devfn as the port number
for root ports? I'm not a fan of QEMU's use of chassis_nr for bridges
and with an 8bit limit on port numbers, using idx we'll run out of bits
at some point. Thanks,
Alex
+ break;
}
break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
new file mode 100644
index 0000000..30bec85
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
@@ -0,0 +1,10 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-device ioh3420,port=3,chassis=3,id=pci.3,bus=pcie.0,addr=0x2 \
+-device ioh3420,port=4,chassis=4,id=pci.4,bus=pcie.0,addr=0x3 \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
+-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index a90f9a6..999edff 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1461,6 +1461,13 @@ mymain(void)
QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("pcie-root-port",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
7:51 p.m.
New subject: [libvirt] [PATCH 07/13] qemu: support new pci controller model "pcie-root-port"
On 06/23/2015 11:31 AM, Alex Williamson wrote:
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
> This is backed by the qemu device ioh3420.
> ---
> src/qemu/qemu_command.c | 20 ++++++++++++++++++++
> .../qemuxml2argv-pcie-root-port.args | 10 ++++++++++
> tests/qemuxml2argvtest.c | 7 +++++++
> 3 files changed, 37 insertions(+)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 4952797..01ac690 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1598,6 +1598,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> */
> flags = VIR_PCI_CONNECT_TYPE_PCIE;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
> + /* pcie-root-port can only connect to pcie-root, isn't
> + * hot-pluggable
> + */
> + flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
> + break;
> default:
> break;
> }
> @@ -2345,6 +2351,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
> */
> flags = VIR_PCI_CONNECT_TYPE_PCIE;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
> + /* pcie-root-port can only plug into pcie-root */
> + flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
> + break;
> default:
> flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
> break;
> @@ -4605,6 +4615,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> }
> virBufferAsprintf(&buf, "i82801b11-bridge,id=%s",
def->info.alias);
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("The pcie-root-port (ioh3420) controller
"
> + "is not supported in this QEMU
binary"));
> + goto error;
> + }
> + virBufferAsprintf(&buf,
"ioh3420,port=%d,chassis=%d,id=%s",
> + def->idx, def->idx, def->info.alias);
So you didn't like the idea of using the flat devfn as the port number
for root ports? I'm not a fan of QEMU's use of chassis_nr for bridges
and with an 8bit limit on port numbers, using idx we'll run out of bits
at some point.
No, I just didn't understand your point until you re-explained on IRC a
little while ago. I'm changing it as you suggest.
In the meantime, I'm thinking about the other point that you made on IRC
- that we're putting logic into libvirt to derive some of these
parameters (port, chassis, chassis_nr) but in the future may decide that
we need to derive them a different way, and if we do that to an existing
machine during migration, things will get "seriously messed up". Even if
this change takes place while the guest isn't running, the guest OS
could get upset about such a change. All this implies that we really
need to be saving the derived values in the config to avoid such surprises.
8:44 p.m.
New subject: [libvirt] [PATCH 08/13] qemu: add capabilities bit for device x3130-upstream
This is the upstream part of a PCIe switch. It connects to a PCIe port
(but not PCI) on the upstream side, and can have up to 31
xio3130-downstream controllers (but no other types of devices)
connected to its downstream side.
This device will be used to implement the "pcie-switch" model of pci
controller.
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemuhelptest.c | 6 ++++--
10 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index a9a19e1..5a33efb 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -288,6 +288,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"vhost-user-multiqueue", /* 190 */
"ioh3420",
+ "x3130-upstream",
);
@@ -1568,6 +1569,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "pc-dimm", QEMU_CAPS_DEVICE_PC_DIMM },
{ "pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL },
{ "ioh3420", QEMU_CAPS_DEVICE_IOH3420 },
+ { "x3130-upstream", QEMU_CAPS_DEVICE_X3130_UPSTREAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d38505e..3d28941 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -231,6 +231,7 @@ typedef enum {
QEMU_CAPS_CPU_AARCH64_OFF = 189, /* -cpu ...,aarch64=off */
QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM = 192, /* -device x3130-upstream */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
index a1fafa6..78d7b82 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
@@ -121,4 +121,5 @@
<flag name='qxl.vgamem_mb'/>
<flag name='qxl-vga.vgamem_mb'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
index 824ef02..7cec7f9 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
@@ -136,4 +136,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
index 7ef5199..f5f0034 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
@@ -137,4 +137,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
index 8c3d48e..9f0461a 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
@@ -146,4 +146,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index 72f7625..1b23b82 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -152,4 +152,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index d81c80c..ff0427f 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -152,4 +152,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
index 1a39dce..56b27e5 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
@@ -168,4 +168,5 @@
<flag name='pc-dimm'/>
<flag name='pci-serial'/>
<flag name='ioh3420'/>
+ <flag name='x3130-upstream'/>
</qemuCaps>
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 6211640..62b9a0c 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -754,7 +754,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_STORAGE,
QEMU_CAPS_SPLASH_TIMEOUT,
QEMU_CAPS_DEVICE_IVSHMEM,
- QEMU_CAPS_DEVICE_IOH3420);
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM);
DO_TEST("qemu-1.1.0", 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -855,7 +856,8 @@ mymain(void)
QEMU_CAPS_OBJECT_USB_AUDIO,
QEMU_CAPS_SPLASH_TIMEOUT,
QEMU_CAPS_DEVICE_IVSHMEM,
- QEMU_CAPS_DEVICE_IOH3420);
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM);
DO_TEST_FULL("qemu-1.2.0", 1002000, 0, 0, VIR_ERR_CONFIG_UNSUPPORTED,
QEMU_CAPS_LAST);
DO_TEST_FULL("qemu-kvm-1.2.0", 1002000, 1, 0, VIR_ERR_CONFIG_UNSUPPORTED,
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 09/13] conf: new pci controller model "pcie-switch-upstream-port"
This controller can be connected to any PCIe port, but not to a
standard PCI port, which is the reason for the new connect type
VIR_PCI_CONNECT_TYPE_PCIE_ONLY. pcie-switch provides 31 ports (slot=1
to slot=31), which can only have pci controllers of model
"pcie-switch-downstream-port" plugged into them, which is the reason for the
other new connect type VIR_PCI_CONNECT_TYPE_PCIE_SWITCH.
---
docs/formatdomain.html.in | 5 ++--
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_addr.c | 11 +++++++-
src/conf/domain_addr.h | 6 +++-
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-pcie-switch-upstream-port.xml | 32 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
8 files changed, 55 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e6c140a..646c9ee 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2998,10 +2998,11 @@
PCI controllers have an optional <code>model</code> attribute with
possible values <code>pci-root</code>,
<code>pcie-root</code>,
<code>pcie-root-port</code>, <code>pci-bridge</code>,
- or <code>dmi-to-pci-bridge</code>.
+ <code>dmi-to-pci-bridge</code>, or
<code>pcie-switch-upstream-port</code>.
(pci-root and pci-bridge <span class="since">since
1.0.5</span>,
pcie-root and dmi-to-pci-bridge <span class="since">since
- 1.1.2</span>, pcie-root-port <span class="since">since
1.2.17</span>)
+ 1.1.2</span>, pcie-root-port and
+ pcie-switch-upstream-port <span class="since">since
1.2.17</span>)
The root controllers (<code>pci-root</code> and
<code>pcie-root</code>)
have an optional <code>pcihole64</code> element specifying how big
(in kilobytes, or in the unit specified by
<code>pcihole64</code>'s
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6139dfb..3f74cfc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1752,6 +1752,7 @@
<value>pci-bridge</value>
<value>dmi-to-pci-bridge</value>
<value>pcie-root-port</value>
+ <value>pcie-switch-upstream-port</value>
</choice>
</attribute>
</group>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4b5e81e..59da745 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -57,6 +57,9 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
}
+ /* devices with PCIE_ONLY can't connect to PCI, even if fromConfig */
+ if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_ONLY)
+ devFlags |= VIR_PCI_CONNECT_TYPE_PCIE;
/* If this bus doesn't allow the type of connection (PCI
* vs. PCIe) required by the device, or if the device requires
@@ -199,8 +202,14 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
/* provides one slot which is pcie and hotpluggable */
bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE;
+ bus->minSlot = 0;
+ bus->maxSlot = 0;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ /* 31 slots, can only accept pcie-switch-port, no hotplug */
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
bus->minSlot = 1;
- bus->maxSlot = 1;
+ bus->maxSlot = 31;
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2a0ff96..8e9ca32 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -41,6 +41,10 @@ typedef enum {
/* PCI Express devices can connect to this bus */
VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 << 4,
/* for devices that can only connect to pcie-root (i.e. root-port) */
+ VIR_PCI_CONNECT_TYPE_PCIE_ONLY = 1 << 5,
+ /* devices that can't connect to pci even if manual config */
+ VIR_PCI_CONNECT_TYPE_PCIE_SWITCH = 1 << 6,
+ /* devices that can only connect to a pcie-switch */
} virDomainPCIConnectFlags;
typedef struct {
@@ -73,7 +77,7 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
*/
# define VIR_PCI_CONNECT_TYPES_MASK \
(VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \
- VIR_PCI_CONNECT_TYPE_PCIE_ROOT)
+ VIR_PCI_CONNECT_TYPE_PCIE_ROOT | VIR_PCI_CONNECT_TYPE_PCIE_SWITCH)
/* combination of all bits that could be used to connect a normal
* endpoint device (i.e. excluding the connection possible between an
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0bdd51e..c0b8c92 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -324,7 +324,8 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pcie-root",
"pci-bridge",
"dmi-to-pci-bridge",
- "pcie-root-port")
+ "pcie-root-port",
+ "pcie-switch-upstream-port")
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 946210e..b7ce0ce 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -753,6 +753,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
} virDomainControllerModelPCI;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
new file mode 100644
index 0000000..428a4ea
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>q35-test</name>
+ <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>2097152</currentMemory>
+ <vcpu placement='static' cpuset='0-1'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'/>
+ <controller type='pci' index='2' model='pci-bridge'/>
+ <controller type='pci' index='3'
model='pcie-switch-upstream-port'/>
+ <controller type='sata' index='0'/>
+ <video>
+ <model type='qxl' ram='65536' vram='32768'
vgamem='8192' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2acbae1..e5fcc5d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -567,6 +567,7 @@ mymain(void)
DO_TEST_DIFFERENT("pcie-root");
DO_TEST_DIFFERENT("q35");
DO_TEST("pcie-root-port");
+ DO_TEST("pcie-switch-upstream-port");
DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");
--
2.1.0
5:23 p.m.
New subject: [libvirt] [PATCH 09/13] conf: new pci controller model "pcie-switch-upstream-port"
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
This controller can be connected to any PCIe port, but not to a
standard PCI port, which is the reason for the new connect type
VIR_PCI_CONNECT_TYPE_PCIE_ONLY. pcie-switch provides 31 ports (slot=1
to slot=31), which can only have pci controllers of model
"pcie-switch-downstream-port" plugged into them, which is the reason for the
other new connect type VIR_PCI_CONNECT_TYPE_PCIE_SWITCH.
---
docs/formatdomain.html.in | 5 ++--
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_addr.c | 11 +++++++-
src/conf/domain_addr.h | 6 +++-
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-pcie-switch-upstream-port.xml | 32 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
8 files changed, 55 insertions(+), 5 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index e6c140a..646c9ee 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2998,10 +2998,11 @@
PCI controllers have an optional <code>model</code> attribute with
possible values <code>pci-root</code>,
<code>pcie-root</code>,
<code>pcie-root-port</code>, <code>pci-bridge</code>,
- or <code>dmi-to-pci-bridge</code>.
+ <code>dmi-to-pci-bridge</code>, or
<code>pcie-switch-upstream-port</code>.
(pci-root and pci-bridge <span class="since">since
1.0.5</span>,
pcie-root and dmi-to-pci-bridge <span class="since">since
- 1.1.2</span>, pcie-root-port <span class="since">since
1.2.17</span>)
+ 1.1.2</span>, pcie-root-port and
+ pcie-switch-upstream-port <span class="since">since
1.2.17</span>)
The root controllers (<code>pci-root</code> and
<code>pcie-root</code>)
have an optional <code>pcihole64</code> element specifying how big
(in kilobytes, or in the unit specified by
<code>pcihole64</code>'s
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 6139dfb..3f74cfc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1752,6 +1752,7 @@
<value>pci-bridge</value>
<value>dmi-to-pci-bridge</value>
<value>pcie-root-port</value>
+ <value>pcie-switch-upstream-port</value>
</choice>
</attribute>
</group>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 4b5e81e..59da745 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -57,6 +57,9 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
}
+ /* devices with PCIE_ONLY can't connect to PCI, even if fromConfig */
+ if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_ONLY)
+ devFlags |= VIR_PCI_CONNECT_TYPE_PCIE;
/* If this bus doesn't allow the type of connection (PCI
* vs. PCIe) required by the device, or if the device requires
@@ -199,8 +202,14 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
/* provides one slot which is pcie and hotpluggable */
bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE;
+ bus->minSlot = 0;
+ bus->maxSlot = 0;
Shouldn't this have belonged to a previous patch?
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ /* 31 slots, can only accept pcie-switch-port, no hotplug */
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
bus->minSlot = 1;
- bus->maxSlot = 1;
+ bus->maxSlot = 31;
Why exactly are we reserving slot 0? I know we discussed on IRC that
slot 0 is reserved for the host bridge on root buses, but there are no
reserved slots downstream of an upstream switch port. If QEMU is
limiting this, it's a bug, if libvirt wants to impose this, it should be
documented as such. Thanks,
Alex
break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h
index 2a0ff96..8e9ca32 100644
--- a/src/conf/domain_addr.h
+++ b/src/conf/domain_addr.h
@@ -41,6 +41,10 @@ typedef enum {
/* PCI Express devices can connect to this bus */
VIR_PCI_CONNECT_TYPE_PCIE_ROOT = 1 << 4,
/* for devices that can only connect to pcie-root (i.e. root-port) */
+ VIR_PCI_CONNECT_TYPE_PCIE_ONLY = 1 << 5,
+ /* devices that can't connect to pci even if manual config */
+ VIR_PCI_CONNECT_TYPE_PCIE_SWITCH = 1 << 6,
+ /* devices that can only connect to a pcie-switch */
} virDomainPCIConnectFlags;
typedef struct {
@@ -73,7 +77,7 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr;
*/
# define VIR_PCI_CONNECT_TYPES_MASK \
(VIR_PCI_CONNECT_TYPE_PCI | VIR_PCI_CONNECT_TYPE_PCIE | \
- VIR_PCI_CONNECT_TYPE_PCIE_ROOT)
+ VIR_PCI_CONNECT_TYPE_PCIE_ROOT | VIR_PCI_CONNECT_TYPE_PCIE_SWITCH)
/* combination of all bits that could be used to connect a normal
* endpoint device (i.e. excluding the connection possible between an
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0bdd51e..c0b8c92 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -324,7 +324,8 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pcie-root",
"pci-bridge",
"dmi-to-pci-bridge",
- "pcie-root-port")
+ "pcie-root-port",
+ "pcie-switch-upstream-port")
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 946210e..b7ce0ce 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -753,6 +753,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
} virDomainControllerModelPCI;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
new file mode 100644
index 0000000..428a4ea
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
@@ -0,0 +1,32 @@
+<domain type='qemu'>
+ <name>q35-test</name>
+ <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>2097152</currentMemory>
+ <vcpu placement='static' cpuset='0-1'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'/>
+ <controller type='pci' index='2' model='pci-bridge'/>
+ <controller type='pci' index='3'
model='pcie-switch-upstream-port'/>
+ <controller type='sata' index='0'/>
+ <video>
+ <model type='qxl' ram='65536' vram='32768'
vgamem='8192' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 2acbae1..e5fcc5d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -567,6 +567,7 @@ mymain(void)
DO_TEST_DIFFERENT("pcie-root");
DO_TEST_DIFFERENT("q35");
DO_TEST("pcie-root-port");
+ DO_TEST("pcie-switch-upstream-port");
DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");
7:47 p.m.
New subject: [libvirt] [PATCH 09/13] conf: new pci controller model "pcie-switch-upstream-port"
On 06/23/2015 11:23 AM, Alex Williamson wrote:
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
> index 4b5e81e..59da745 100644
> --- a/src/conf/domain_addr.c
> +++ b/src/conf/domain_addr.c
> @@ -57,6 +57,9 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
> if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
> busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
> }
> + /* devices with PCIE_ONLY can't connect to PCI, even if fromConfig */
> + if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_ONLY)
> + devFlags |= VIR_PCI_CONNECT_TYPE_PCIE;
>
> /* If this bus doesn't allow the type of connection (PCI
> * vs. PCIe) required by the device, or if the device requires
> @@ -199,8 +202,14 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
> /* provides one slot which is pcie and hotpluggable */
> bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE;
> + bus->minSlot = 0;
> + bus->maxSlot = 0;
Shouldn't this have belonged to a previous patch?
Yes, and I'm pretty certain that it originally was. I must have messed
it up when resolving merge conflicts during my myriads of rebases to
change names.
> + break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
> + /* 31 slots, can only accept pcie-switch-port, no hotplug */
> + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
> bus->minSlot = 1;
> - bus->maxSlot = 1;
> + bus->maxSlot = 31;
Why exactly are we reserving slot 0?
Uh, because I forgot to change it after talking to you? Thanks for
catching it!
I know we discussed on IRC that
slot 0 is reserved for the host bridge on root buses, but there are no
reserved slots downstream of an upstream switch port.
(Btw, I've forgotten - is slot 0 also reserved for pci-bridge and
82801b11-bridge? Or can I send a patch to allow slot 0 on those as well?)
9:07 p.m.
New subject: [libvirt] [PATCH 09/13] conf: new pci controller model "pcie-switch-upstream-port"
On Tue, 2015-06-23 at 13:47 -0400, Laine Stump wrote:
On 06/23/2015 11:23 AM, Alex Williamson wrote:
> On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
>> diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
>> index 4b5e81e..59da745 100644
>> --- a/src/conf/domain_addr.c
>> +++ b/src/conf/domain_addr.c
>> @@ -57,6 +57,9 @@ virDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr
addr,
>> if (devFlags & VIR_PCI_CONNECT_HOTPLUGGABLE)
>> busFlags |= VIR_PCI_CONNECT_HOTPLUGGABLE;
>> }
>> + /* devices with PCIE_ONLY can't connect to PCI, even if fromConfig */
>> + if (devFlags & VIR_PCI_CONNECT_TYPE_PCIE_ONLY)
>> + devFlags |= VIR_PCI_CONNECT_TYPE_PCIE;
>>
>> /* If this bus doesn't allow the type of connection (PCI
>> * vs. PCIe) required by the device, or if the device requires
>> @@ -199,8 +202,14 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr
bus,
>> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT:
>> /* provides one slot which is pcie and hotpluggable */
>> bus->flags = VIR_PCI_CONNECT_TYPE_PCIE |
VIR_PCI_CONNECT_HOTPLUGGABLE;
>> + bus->minSlot = 0;
>> + bus->maxSlot = 0;
> Shouldn't this have belonged to a previous patch?
Yes, and I'm pretty certain that it originally was. I must have messed
it up when resolving merge conflicts during my myriads of rebases to
change names.
>
>> + break;
>> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
>> + /* 31 slots, can only accept pcie-switch-port, no hotplug */
>> + bus->flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
>> bus->minSlot = 1;
>> - bus->maxSlot = 1;
>> + bus->maxSlot = 31;
> Why exactly are we reserving slot 0?
Uh, because I forgot to change it after talking to you? Thanks for
catching it!
> I know we discussed on IRC that
> slot 0 is reserved for the host bridge on root buses, but there are no
> reserved slots downstream of an upstream switch port.
(Btw, I've forgotten - is slot 0 also reserved for pci-bridge and
82801b11-bridge? Or can I send a patch to allow slot 0 on those as well?)
I don't think there's anything special about slot 0 for either of those
bridges, a quick test would verify. However, we're immediately into
your case of changing the auto-assignment parameters. Have we always
stored the resulting auto-assigned bus addresses for devices so we don't
suddenly change the location of devices for existing domains? Thanks,
Alex
9:40 p.m.
New subject: [libvirt] [PATCH 09/13] conf: new pci controller model "pcie-switch-upstream-port"
On 06/23/2015 03:07 PM, Alex Williamson wrote:
I don't think there's anything special about slot 0 for
either of those
bridges, a quick test would verify. However, we're immediately into
your case of changing the auto-assignment parameters. Have we always
stored the resulting auto-assigned bus addresses for devices so we don't
suddenly change the location of devices for existing domains?
Yes. libvirt always stores the bus/slot (and of course function and
domain, which are currently both always 0 in the case of auto-assign)
back in the XML. It has been this way for "a very long time".
8:44 p.m.
New subject: [libvirt] [PATCH 10/13] qemu: support new pci controller model "pcie-switch-upstream-port"
this is backed by the qemu device x3130-upstream.
---
src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
.../qemuxml2argv-pcie-switch-upstream-port.args | 9 +++++++++
tests/qemuxml2argvtest.c | 9 +++++++++
3 files changed, 40 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 01ac690..b4df65a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1604,6 +1604,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ /* pcie-switch can only connect to a true
+ * pcie bus, and can't be hot-plugged.
+ */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY;
+ break;
default:
break;
}
@@ -2355,6 +2361,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
/* pcie-root-port can only plug into pcie-root */
flags = VIR_PCI_CONNECT_TYPE_PCIE_ROOT;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ /* pcie-switch really does need a real PCIe
+ * port, but it doesn't need to be pcie-root
+ */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY;
+ break;
default:
flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
break;
@@ -4625,6 +4637,16 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
virBufferAsprintf(&buf, "ioh3420,port=%d,chassis=%d,id=%s",
def->idx, def->idx, def->info.alias);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_X3130_UPSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-switch-upstream-port "
+ "(x3130-upstream) controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf, "x3130-upstream,id=%s",
def->info.alias);
+ break;
}
break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
new file mode 100644
index 0000000..d737b6b
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
@@ -0,0 +1,9 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-device x3130-upstream,id=pci.3,bus=pcie.0,addr=0x2 \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
+-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 999edff..b15352e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1469,6 +1469,15 @@ mymain(void)
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("pcie-switch-upstream-port",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+
DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_SCSI_LSI,
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 11/13] qemu: add capabilities bit for device xio3130-downstream
The downstream ports of an x3130-upstream switch can each have one of
these plugged into them (and that is the only place they can be
connected). Each xio3130-downstream provides a single PCIe port that
can have PCI or PCIe devices hotplugged into it. Apparently an entire
set of x3130-upstream + several xio3130-downstreams can be hotplugged
as a unit, but it's not clear to me yet how that would be done, since
qemu only allows attaching a single device at a time.
This device will be used to implement the "pcie-switch-port" model of
pci controller.
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 1 +
tests/qemuhelptest.c | 6 ++++--
10 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 5a33efb..4103b6e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -289,6 +289,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"vhost-user-multiqueue", /* 190 */
"ioh3420",
"x3130-upstream",
+ "xio3130-downstream",
);
@@ -1570,6 +1571,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "pci-serial", QEMU_CAPS_DEVICE_PCI_SERIAL },
{ "ioh3420", QEMU_CAPS_DEVICE_IOH3420 },
{ "x3130-upstream", QEMU_CAPS_DEVICE_X3130_UPSTREAM },
+ { "xio3130-downstream", QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 3d28941..121e06b 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -232,6 +232,7 @@ typedef enum {
QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
QEMU_CAPS_DEVICE_IOH3420 = 191, /* -device ioh3420 */
QEMU_CAPS_DEVICE_X3130_UPSTREAM = 192, /* -device x3130-upstream */
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM = 193, /* -device xio3130-downstream */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
index 78d7b82..ba16635 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
@@ -122,4 +122,5 @@
<flag name='qxl-vga.vgamem_mb'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
index 7cec7f9..51cd6d9 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
@@ -137,4 +137,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
index f5f0034..03d0a3e 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
@@ -138,4 +138,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
index 9f0461a..e2f22e4 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
@@ -147,4 +147,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index 1b23b82..874a050 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -153,4 +153,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index ff0427f..dd3bcda 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -153,4 +153,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
index 56b27e5..3ee2d6f 100644
--- a/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_2.1.1-1.caps
@@ -169,4 +169,5 @@
<flag name='pci-serial'/>
<flag name='ioh3420'/>
<flag name='x3130-upstream'/>
+ <flag name='xio3130-downstream'/>
</qemuCaps>
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 62b9a0c..8f317d4 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -755,7 +755,8 @@ mymain(void)
QEMU_CAPS_SPLASH_TIMEOUT,
QEMU_CAPS_DEVICE_IVSHMEM,
QEMU_CAPS_DEVICE_IOH3420,
- QEMU_CAPS_DEVICE_X3130_UPSTREAM);
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
DO_TEST("qemu-1.1.0", 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -857,7 +858,8 @@ mymain(void)
QEMU_CAPS_SPLASH_TIMEOUT,
QEMU_CAPS_DEVICE_IVSHMEM,
QEMU_CAPS_DEVICE_IOH3420,
- QEMU_CAPS_DEVICE_X3130_UPSTREAM);
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM);
DO_TEST_FULL("qemu-1.2.0", 1002000, 0, 0, VIR_ERR_CONFIG_UNSUPPORTED,
QEMU_CAPS_LAST);
DO_TEST_FULL("qemu-kvm-1.2.0", 1002000, 1, 0, VIR_ERR_CONFIG_UNSUPPORTED,
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 12/13] conf: new pcie-controller model "pcie-switch-downstream-port"
This controller can be connected only to a port on a
"pcie-switch-upstream-port". It provides a single hotpluggable port
that will accept any PCI or PCIe device.
---
docs/formatdomain.html.in | 31 +++++++++++++------
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_addr.c | 6 ++++
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 1 +
.../qemuxml2argv-pcie-switch-downstream-port.xml | 36 ++++++++++++++++++++++
tests/qemuxml2xmltest.c | 1 +
7 files changed, 68 insertions(+), 11 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 646c9ee..7e16dc4 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2998,11 +2998,12 @@
PCI controllers have an optional <code>model</code> attribute with
possible values <code>pci-root</code>,
<code>pcie-root</code>,
<code>pcie-root-port</code>, <code>pci-bridge</code>,
- <code>dmi-to-pci-bridge</code>, or
<code>pcie-switch-upstream-port</code>.
+ <code>dmi-to-pci-bridge</code>,
<code>pcie-switch-upstream-port</code>, or
+ <code>pcie-switch-downstream-port</code>.
(pci-root and pci-bridge <span class="since">since
1.0.5</span>,
pcie-root and dmi-to-pci-bridge <span class="since">since
- 1.1.2</span>, pcie-root-port and
- pcie-switch-upstream-port <span class="since">since
1.2.17</span>)
+ 1.1.2</span>, pcie-root-port, pcie-switch-upstream-port, and
+ pcie-switch-downstream-port <span class="since">since
1.2.30</span>)
The root controllers (<code>pci-root</code> and
<code>pcie-root</code>)
have an optional <code>pcihole64</code> element specifying how big
(in kilobytes, or in the unit specified by
<code>pcihole64</code>'s
@@ -3058,14 +3059,24 @@
</p>
<p>
Domains with an implicit pcie-root can also add controllers
- with <code>model='pcie-root-port'</code>. This is a simple type
of
- bridge device that can connect only to one of the 31 slots on
- the pcie-root bus on its upstream side, and makes a single
- (PCIe, hotpluggable) port (at slot='0') available on the
- downstream side. This controller can be used to provide a single
- slot to later hotplug a PCIe device (but is not itself
+ with <code>model='pcie-root-port'</code>,
+ <code>model='pcie-switch-upstream-port'</code>,
+ and <code>model='pcie-switch-downstream-port'</code>.
pcie-root-port
+ is a simple type of bridge device that can connect only to one
+ of the 31 slots on the pcie-root bus on its upstream side, and
+ makes a single (PCIe, hotpluggable) port (at slot='0') available
+ on the downstream side. This controller can be used to provide a
+ single slot to later hotplug a PCIe device (but is not itself
hotpluggable - it must be in the configuration when the domain
- is started). (<span class="since">since 1.2.17</span>)
+ is started). pcie-switch-upstream-port is a more flexible (but
+ also more complex) device that can plug into any PCIe port on
+ the upstream side, and provides 31 ports on the downstream side
+ that accept only pcie-switch-downstream-port devices; each
+ pcie-switch-downstream-port device can only plug into a
+ pcie-switch-upstream-port on its upstream side, and on its
+ downstream side provides a single hotpluggable pcie port that
+ can accept any standard pci or pcie device.
+ (<span class="since">since 1.2.30</span>)
</p>
<pre>
...
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3f74cfc..490c0e7 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1753,6 +1753,7 @@
<value>dmi-to-pci-bridge</value>
<value>pcie-root-port</value>
<value>pcie-switch-upstream-port</value>
+ <value>pcie-switch-downstream-port</value>
</choice>
</attribute>
</group>
diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c
index 59da745..b97a62c 100644
--- a/src/conf/domain_addr.c
+++ b/src/conf/domain_addr.c
@@ -211,6 +211,12 @@ virDomainPCIAddressBusSetModel(virDomainPCIAddressBusPtr bus,
bus->minSlot = 1;
bus->maxSlot = 31;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* provides one slot which is pcie and hotpluggable */
+ bus->flags = VIR_PCI_CONNECT_TYPE_PCIE | VIR_PCI_CONNECT_HOTPLUGGABLE;
+ bus->minSlot = 0;
+ bus->maxSlot = 0;
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PCI controller model %d"), model);
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c0b8c92..8987e81 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -325,7 +325,8 @@ VIR_ENUM_IMPL(virDomainControllerModelPCI,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pci-bridge",
"dmi-to-pci-bridge",
"pcie-root-port",
- "pcie-switch-upstream-port")
+ "pcie-switch-upstream-port",
+ "pcie-switch-downstream-port")
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 b7ce0ce..1351eef 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -754,6 +754,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
} virDomainControllerModelPCI;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
new file mode 100644
index 0000000..7d30bd1
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+ <name>q35-test</name>
+ <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
+ <memory unit='KiB'>2097152</memory>
+ <currentMemory unit='KiB'>2097152</currentMemory>
+ <vcpu placement='static' cpuset='0-1'>2</vcpu>
+ <os>
+ <type arch='x86_64' machine='q35'>hvm</type>
+ <boot dev='hd'/>
+ </os>
+ <clock offset='utc'/>
+ <on_poweroff>destroy</on_poweroff>
+ <on_reboot>restart</on_reboot>
+ <on_crash>destroy</on_crash>
+ <devices>
+ <emulator>/usr/libexec/qemu-kvm</emulator>
+ <disk type='block' device='disk'>
+ <source dev='/dev/HostVG/QEMUGuest1'/>
+ <target dev='sda' bus='sata'/>
+ <address type='drive' controller='0' bus='0'
target='0' unit='0'/>
+ </disk>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='pci' index='1'
model='dmi-to-pci-bridge'/>
+ <controller type='pci' index='2' model='pci-bridge'/>
+ <controller type='pci' index='3'
model='pcie-switch-upstream-port'/>
+ <controller type='pci' index='4'
model='pcie-switch-downstream-port'/>
+ <controller type='pci' index='5'
model='pcie-switch-downstream-port'/>
+ <controller type='pci' index='6'
model='pcie-switch-downstream-port'/>
+ <controller type='pci' index='7'
model='pcie-switch-downstream-port'/>
+ <controller type='sata' index='0'/>
+ <video>
+ <model type='qxl' ram='65536' vram='32768'
vgamem='8192' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index e5fcc5d..d121fa0 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -568,6 +568,7 @@ mymain(void)
DO_TEST_DIFFERENT("q35");
DO_TEST("pcie-root-port");
DO_TEST("pcie-switch-upstream-port");
+ DO_TEST("pcie-switch-downstream-port");
DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");
--
2.1.0
8:44 p.m.
New subject: [libvirt] [PATCH 13/13] qemu: support new pci controller model "pcie-switch-downstream-port"
This is backed by the qemu device xio3130-downstream. It can only be
connected to a pcie-switch-upstream-port (x3130-upstream) on the
upstream side.
---
src/qemu/qemu_command.c | 15 +++++++++++++++
.../qemuxml2argv-pcie-switch-downstream-port.args | 13 +++++++++++++
tests/qemuxml2argvtest.c | 9 +++++++++
3 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b4df65a..064adcb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2367,6 +2367,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* pcie-switch-port can only plug into pcie-switch */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
+ break;
default:
flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
break;
@@ -4647,6 +4651,17 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
}
virBufferAsprintf(&buf, "x3130-upstream,id=%s",
def->info.alias);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-switch-downstream-port "
+ "(xio3130-downstream) controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf,
"xio3130-downstream,port=1,chassis=%d,id=%s",
+ def->idx, def->info.alias);
+ break;
}
break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
new file mode 100644
index 0000000..04b760c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
@@ -0,0 +1,13 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-device x3130-upstream,id=pci.3,bus=pcie.0,addr=0x2 \
+-device xio3130-downstream,port=1,chassis=4,id=pci.4,bus=pci.3,addr=0x1 \
+-device xio3130-downstream,port=1,chassis=5,id=pci.5,bus=pci.3,addr=0x2 \
+-device xio3130-downstream,port=1,chassis=6,id=pci.6,bus=pci.3,addr=0x3 \
+-device xio3130-downstream,port=1,chassis=7,id=pci.7,bus=pci.3,addr=0x4 \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
+-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b15352e..fc16152 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1477,6 +1477,15 @@ mymain(void)
QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("pcie-switch-downstream-port",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
--
2.1.0
5:35 p.m.
New subject: [libvirt] [PATCH 13/13] qemu: support new pci controller model "pcie-switch-downstream-port"
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
This is backed by the qemu device xio3130-downstream. It can only be
connected to a pcie-switch-upstream-port (x3130-upstream) on the
upstream side.
---
src/qemu/qemu_command.c | 15 +++++++++++++++
.../qemuxml2argv-pcie-switch-downstream-port.args | 13 +++++++++++++
tests/qemuxml2argvtest.c | 9 +++++++++
3 files changed, 37 insertions(+)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index b4df65a..064adcb 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2367,6 +2367,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
*/
flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ /* pcie-switch-port can only plug into pcie-switch */
+ flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
+ break;
default:
flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
break;
@@ -4647,6 +4651,17 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
}
virBufferAsprintf(&buf, "x3130-upstream,id=%s",
def->info.alias);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The pcie-switch-downstream-port "
+ "(xio3130-downstream) controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ virBufferAsprintf(&buf,
"xio3130-downstream,port=1,chassis=%d,id=%s",
+ def->idx, def->info.alias);
And why put all the downstream switch ports on port=1? I had suggested
the slot address would match what I see on physical hardware. Is the
slot address not available at this point? Thanks,
Alex
+ break;
}
break;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
new file mode 100644
index 0000000..04b760c
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
@@ -0,0 +1,13 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/libexec/qemu-kvm -S -M q35 -m 2048 -smp 2 -nographic -nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
+-device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x1e \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-device x3130-upstream,id=pci.3,bus=pcie.0,addr=0x2 \
+-device xio3130-downstream,port=1,chassis=4,id=pci.4,bus=pci.3,addr=0x1 \
+-device xio3130-downstream,port=1,chassis=5,id=pci.5,bus=pci.3,addr=0x2 \
+-device xio3130-downstream,port=1,chassis=6,id=pci.6,bus=pci.3,addr=0x3 \
+-device xio3130-downstream,port=1,chassis=7,id=pci.7,bus=pci.3,addr=0x4 \
+-drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-sata0-0-0 \
+-device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \
+-vga qxl -global qxl-vga.ram_size=67108864 -global qxl-vga.vram_size=33554432
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b15352e..fc16152 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1477,6 +1477,15 @@ mymain(void)
QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
+ DO_TEST("pcie-switch-downstream-port",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_IOH3420,
+ QEMU_CAPS_DEVICE_X3130_UPSTREAM,
+ QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM,
+ QEMU_CAPS_DRIVE, QEMU_CAPS_ICH9_AHCI,
+ QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
+ QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
7:56 p.m.
New subject: [libvirt] [PATCH 13/13] qemu: support new pci controller model "pcie-switch-downstream-port"
On 06/23/2015 11:35 AM, Alex Williamson wrote:
On Mon, 2015-06-22 at 14:44 -0400, Laine Stump wrote:
> This is backed by the qemu device xio3130-downstream. It can only be
> connected to a pcie-switch-upstream-port (x3130-upstream) on the
> upstream side.
> ---
> src/qemu/qemu_command.c | 15 +++++++++++++++
> .../qemuxml2argv-pcie-switch-downstream-port.args | 13 +++++++++++++
> tests/qemuxml2argvtest.c | 9 +++++++++
> 3 files changed, 37 insertions(+)
> create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b4df65a..064adcb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -2367,6 +2367,10 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
> */
> flags = VIR_PCI_CONNECT_TYPE_PCIE_ONLY;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
> + /* pcie-switch-port can only plug into pcie-switch */
> + flags = VIR_PCI_CONNECT_TYPE_PCIE_SWITCH;
> + break;
> default:
> flags = VIR_PCI_CONNECT_HOTPLUGGABLE | VIR_PCI_CONNECT_TYPE_PCI;
> break;
> @@ -4647,6 +4651,17 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> }
> virBufferAsprintf(&buf, "x3130-upstream,id=%s",
def->info.alias);
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT:
> + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_XIO3130_DOWNSTREAM)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("The pcie-switch-downstream-port "
> + "(xio3130-downstream) controller "
> + "is not supported in this QEMU
binary"));
> + goto error;
> + }
> + virBufferAsprintf(&buf,
"xio3130-downstream,port=1,chassis=%d,id=%s",
> + def->idx, def->info.alias);
And why put all the downstream switch ports on port=1?
Just because that's what was shown as an example in docs/q35-chipset.cfg
in the qemu source.
I had suggested
the slot address would match what I see on physical hardware. Is the
slot address not available at this point?
Yes, the slot is available - def->info.addr.pci.slot. I was just in "new
information overload" and missed that part. I'll do it that way the next
round.
Thanks again for the comments.
10:40 a.m.
On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote:
The first 4 patches are bugfixes/reorganizations that have no
controversy.
The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI
controller:
5-7 - <controller type='pci' model='pcie-root-port'/>
This is based on qemu's ioh3420.
8-10 - <controller type='pci' model='pcie-switch-upstream-port'/>
Based on qemu's x3130-upstream
11-13 - <controller type='pci'
model='pcie-switch-downstream-port'/>
(xio3130-downstream)
The first patch of each set adds a capability bit for qemu (again
non-controversial), the 2nd adds the new pci controller model, and the
3rd implements that model in qemu (by checking for the capability and
adding a commandline arg or failing).
The "controversial"/RFC bit is this - talking to Alex Williamson
(after I'd rwritten these patches, which is why I'm presenting them in
a form that I *don't* want to push) about the possibility of qemu
adding generic root-port, switch-upstream-port, and
switch-downstream-port controllers, and possibly also a generic
dmi-to-pci-bridge (i.e. controllers not tied to any particular
hardware implementation as with those currently available), I'm
realizing that, while it was a correct decision to make all of the
existing pci controllers "type='pci'" (since they share an address
space), using the "model" attribute to set the kind of controller was
probably a mistake. The problem - if:
<controller type='pci' model='dmi-to-pci-bridge'/>
currently means to add an i82801b11-bridge controller to the domain,
once qemu implements a generic dmi-to-pci-bridge, how will *that* be
denoted, and how will we avoid replacing the existing i81801b11-bridge
in a particular domain with the generic version when a guest is
restarted following a qemu/libvirt upgrade?
In hindsight, it probably would have been better to do something like
this with the four existing pci controllers:
<controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
<controller type='pci' subType='pci-bridge'
model='pci-bridge'/> (or maybe blank?)
<controller type='pci' subType='pci-root'/> (again maybe model is
blank)
<controller type='pci' subType='pcie-root'/>(and again)
(instead, what is shown above as "subType" is currently placed in the
"model" attribute).
Then we could add the 3 new types like this:
<controller type='pci' subType='pcie-root-port'
model='ioh3420'/>
<controller type='pci' subType='pcie-switch-upstream-port'
model='x3130-upstream/>
<controller type='pci' subType='pcie-switch-downstream-port'
model='xio3130-downstream/>
and we would easily be able to add support for new generic controllers
that behaved identically, by just adding a new model. But we haven't
done that, and anything we do in the future must be backwards
compatible with what's already there (right?). I'm trying to think of
how to satisfy backward compatibility while making things work better
in the future.
Some ideas, in no particular order:
===
Idea 1: multiplex the meaning of the "model" attribute, so we currently have:
<controller type='pci' model='dmi-to-pci-bridge'/>
which means "add an i82801b11-bridge device" and when we add a generic
version of this type of controller, we would do it with something like:
<controller type='pci' model='generic-dmi-to-pci-bridge'/>
and for another vendor's mythical controller:
<controller type='pci' model='xyz-dmi-to-pci-bridge'/>
Cons: This will make for ugliness in switch statements where a new
case will have to be added whenever a different controller with
similar behavior/usage is supported. And it's generally not a good idea to
have a single attribute be used for two different functions.
===
Idea 2: implement new controllers as suggested in "20/20 hindsight"
above. For controllers in existing domains (dmi-to-pci-bridge,
pic-bridge, pci-root, and pcie-root) imply it into the controller
definition of an existing domain when only model has been given (but
don't write it out that way, to preserve the ability to downgrade). So
this:
[1] <controller type='pci' model='dmi-to-pci-bridge'/>
would internally mean this:
[2] <controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
(but would remain as [1] when config is rewritten/migrated) while
this:
[3] <controller type='pci' subType='dmi-to-pci-bridge'
model='anything whatsoever/>
would mean exactly what it says.
Cons: Keeping this straight would mean having some sort of
"oldStyleCompat" flag in the controller object, to be sure that [1]
wasn't sent in migration status as [2] (since the destination might
not recognize it). It would also mean keeping the code in the parser
and formatter to deal with this flag. Forever.
===
Idea 3: interpret controllers with missing subType as above, but
actually write it out to the config/migration/etc in the new modified
format.
Cons: This would prevent downgrading libvirt or migrating from a host
with newer libvirt to one with older libvirt. (Although preserving
compatibility at some level when downgrading may be a stated
requirement of some downstream distros' builds of libvirt, I think for
upstream it is only a "best effort"; I'm just not certain how much
"best" is considered to be :-)
===
Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
continue to use "model" to denote the subtype/class/whatever of
controller, and create a new attribute to denote the different
specific implementations of each one. So for example:
[4] <controller type='pci' model='dmi-to-pci-bridge'/>
would become:
[5] <controller type='pci' model='dmi-to-pci-bridge'
implementation='i82801b11-bridge'/>
(or some other name in place of "implementation" - ideas? I'm horrible
at thinkin up names)
Pros: wouldn't create compatibility problems when downgrading or
migrating cross version.
Cons: Is inconsistent with every other use of "model" attribute in
libvirt, and each new addition of a PCI controller further propagates
this misuse.
I must say that I thought of this ^^ exactly when reading first three
ideas. I wouldn't necessarily say it's a "misuse" of the
'model'
naming. We can say it's 'model' and 'subModel' or 'driver'
or
whatever. One thing to add to pros is that if you don't care about
the implementation (submodel/driver), then you don't need to increase
the size of the XML. Pus the code complexity added is not greater
than the benefit gained.
Having said that, I don't insist on that, it's merely my two cents
that I like the last idea the best.
====
I currently like either option 2 or 3 (depending on how good we want
to be about supporting downgrade/intra-version migration), but as is
obvious by the fact that it was me that suggested putting the type of
pci controller into "model", I am very good at making the wrong
decision on matters like this.
Whatever everyone thinks is best, patches 5-13 of this series would be
changed accordingly, and possibly a couple new patches would be made
to adjust the 4 existing controller types.
Note that this will also effect the libvirt support for the upcoming
qemu "pxb" controller, which is a PCI root bus that can be placed in
its own numa node (my description may be incorrect, but I think it
gets the idea across). Anyway, with idea 2 or 3 it would be:
<controller type='pci' subType='pci-root' model='pxb'/>
(along with some options to setup numa info).
So, along with any comments on the individual patches (including
whether the specific args added to the qemu commandline are correct -
I'm looking at you, qemu people!), I would appreciate opinions on how
I can save us from this misuse of "model" that I've created.
Laine Stump (13):
qemu: refactor qemuBuildControllerDevStr to eliminate future duplicate
code
qemu: always permit PCI devices to be manually assigned to a PCIe bus
qemu: ignore assumptions about hotplug requirement when address is
from config
docs: document when pcie-root/dmi-to-pci-bridge support was added
qemu: add capabilities bit for device ioh3420
conf: new pci controller model "pcie-root-port"
qemu: support new pci controller model "pcie-root-port"
qemu: add capabilities bit for device x3130-upstream
conf: new pci controller model "pcie-switch-upstream-port"
qemu: support new pci controller model "pcie-switch-upstream-port"
qemu: add capabilities bit for device xio3130-downstream
conf: new pcie-controller model "pcie-switch-downstream-port"
qemu: support new pci controller model "pcie-switch-downstream-port"
docs/formatdomain.html.in | 48 +++++++++---
docs/schemas/domaincommon.rng | 3 +
src/conf/domain_addr.c | 47 ++++++++++--
src/conf/domain_addr.h | 23 ++++--
src/conf/domain_conf.c | 5 +-
src/conf/domain_conf.h | 3 +
src/qemu/qemu_capabilities.c | 8 +-
src/qemu/qemu_capabilities.h | 5 +-
src/qemu/qemu_command.c | 86 +++++++++++++++++-----
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 3 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 3 +
tests/qemucapabilitiesdata/caps_2.1.1-1.caps | 3 +
tests/qemuhelptest.c | 10 ++-
.../qemuxml2argv-pcie-root-port.args | 10 +++
.../qemuxml2argv-pcie-root-port.xml | 33 +++++++++
.../qemuxml2argv-pcie-switch-downstream-port.args | 13 ++++
.../qemuxml2argv-pcie-switch-downstream-port.xml | 36 +++++++++
.../qemuxml2argv-pcie-switch-upstream-port.args | 9 +++
.../qemuxml2argv-pcie-switch-upstream-port.xml | 32 ++++++++
tests/qemuxml2argvtest.c | 25 +++++++
tests/qemuxml2xmltest.c | 3 +
25 files changed, 375 insertions(+), 45 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-downstream-port.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-switch-upstream-port.xml
--
2.1.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
5:35 p.m.
On 06/23/2015 04:40 AM, Martin Kletzander wrote:
On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote:
> The first 4 patches are bugfixes/reorganizations that have no
> controversy.
>
> The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI
> controller:
>
> 5-7 - <controller type='pci' model='pcie-root-port'/>
> This is based on qemu's ioh3420.
>
> 8-10 - <controller type='pci'
model='pcie-switch-upstream-port'/>
> Based on qemu's x3130-upstream
>
> 11-13 - <controller type='pci'
model='pcie-switch-downstream-port'/>
> (xio3130-downstream)
>
> The first patch of each set adds a capability bit for qemu (again
> non-controversial), the 2nd adds the new pci controller model, and the
> 3rd implements that model in qemu (by checking for the capability and
> adding a commandline arg or failing).
>
> The "controversial"/RFC bit is this - talking to Alex Williamson
> (after I'd rwritten these patches, which is why I'm presenting them in
> a form that I *don't* want to push) about the possibility of qemu
> adding generic root-port, switch-upstream-port, and
> switch-downstream-port controllers, and possibly also a generic
> dmi-to-pci-bridge (i.e. controllers not tied to any particular
> hardware implementation as with those currently available), I'm
> realizing that, while it was a correct decision to make all of the
> existing pci controllers "type='pci'" (since they share an address
> space), using the "model" attribute to set the kind of controller was
> probably a mistake. The problem - if:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> currently means to add an i82801b11-bridge controller to the domain,
> once qemu implements a generic dmi-to-pci-bridge, how will *that* be
> denoted, and how will we avoid replacing the existing i81801b11-bridge
> in a particular domain with the generic version when a guest is
> restarted following a qemu/libvirt upgrade?
>
> In hindsight, it probably would have been better to do something like
> this with the four existing pci controllers:
>
> <controller type='pci' subType='dmi-to-pci-bridge'
> model='i82801b11-bridge'/>
> <controller type='pci' subType='pci-bridge'
> model='pci-bridge'/> (or maybe blank?)
> <controller type='pci' subType='pci-root'/> (again maybe model
is
> blank)
> <controller type='pci' subType='pcie-root'/>(and again)
>
> (instead, what is shown above as "subType" is currently placed in the
> "model" attribute).
>
> Then we could add the 3 new types like this:
>
> <controller type='pci' subType='pcie-root-port'
model='ioh3420'/>
> <controller type='pci' subType='pcie-switch-upstream-port'
> model='x3130-upstream/>
> <controller type='pci' subType='pcie-switch-downstream-port'
> model='xio3130-downstream/>
>
> and we would easily be able to add support for new generic controllers
> that behaved identically, by just adding a new model. But we haven't
> done that, and anything we do in the future must be backwards
> compatible with what's already there (right?). I'm trying to think of
> how to satisfy backward compatibility while making things work better
> in the future.
>
> Some ideas, in no particular order:
>
> ===
> Idea 1: multiplex the meaning of the "model" attribute, so we
> currently have:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> which means "add an i82801b11-bridge device" and when we add a generic
> version of this type of controller, we would do it with something like:
>
> <controller type='pci' model='generic-dmi-to-pci-bridge'/>
>
> and for another vendor's mythical controller:
>
> <controller type='pci' model='xyz-dmi-to-pci-bridge'/>
>
> Cons: This will make for ugliness in switch statements where a new
> case will have to be added whenever a different controller with
> similar behavior/usage is supported. And it's generally not a good
> idea to
> have a single attribute be used for two different functions.
>
> ===
>
> Idea 2: implement new controllers as suggested in "20/20 hindsight"
> above. For controllers in existing domains (dmi-to-pci-bridge,
> pic-bridge, pci-root, and pcie-root) imply it into the controller
> definition of an existing domain when only model has been given (but
> don't write it out that way, to preserve the ability to downgrade). So
> this:
>
> [1] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would internally mean this:
>
> [2] <controller type='pci' subType='dmi-to-pci-bridge'
> model='i82801b11-bridge'/>
>
> (but would remain as [1] when config is rewritten/migrated) while
> this:
>
> [3] <controller type='pci' subType='dmi-to-pci-bridge'
> model='anything whatsoever/>
>
> would mean exactly what it says.
>
> Cons: Keeping this straight would mean having some sort of
> "oldStyleCompat" flag in the controller object, to be sure that [1]
> wasn't sent in migration status as [2] (since the destination might
> not recognize it). It would also mean keeping the code in the parser
> and formatter to deal with this flag. Forever.
>
> ===
> Idea 3: interpret controllers with missing subType as above, but
> actually write it out to the config/migration/etc in the new modified
> format.
>
> Cons: This would prevent downgrading libvirt or migrating from a host
> with newer libvirt to one with older libvirt. (Although preserving
> compatibility at some level when downgrading may be a stated
> requirement of some downstream distros' builds of libvirt, I think for
> upstream it is only a "best effort"; I'm just not certain how much
> "best" is considered to be :-)
>
> ===
> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
> continue to use "model" to denote the subtype/class/whatever of
> controller, and create a new attribute to denote the different
> specific implementations of each one. So for example:
>
> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would become:
>
> [5] <controller type='pci' model='dmi-to-pci-bridge'
> implementation='i82801b11-bridge'/>
>
> (or some other name in place of "implementation" - ideas? I'm horrible
> at thinkin up names)
>
> Pros: wouldn't create compatibility problems when downgrading or
> migrating cross version.
>
> Cons: Is inconsistent with every other use of "model" attribute in
> libvirt, and each new addition of a PCI controller further propagates
> this misuse.
>
I must say that I thought of this ^^ exactly when reading first three
ideas. I wouldn't necessarily say it's a "misuse" of the
'model'
naming. We can say it's 'model' and 'subModel' or 'driver'
or
whatever. One thing to add to pros is that if you don't care about
the implementation (submodel/driver), then you don't need to increase
the size of the XML. Pus the code complexity added is not greater
than the benefit gained.
Yes, it definitely is the simplest, provides full functionality, and
gives no downgrade/version compatibility problems. I'm just a pedant
about consistency, and this would bother me *forever*, so I'm either
looking for a better way, or a fact that I can use to placate my
feelings of regret at having not done it right in the first place :-)
In libvirt, "model" is usually used to describe the exact device
provided to the guest, in many/most cases by spelling out the exact
model number of the real hardware that is being emulated, while "driver"
is usually an element that provides details about the internal
implementation of that model. So if I did it this way, I don't think
"driver" is the right name (and neither is my hastily suggested
"implementation"). It might be okay as "subModel" though (but
starting
from scratch I would still prefer type, subType, and model)
5:57 p.m.
On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote:
The first 4 patches are bugfixes/reorganizations that have no
controversy.
The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI
controller:
5-7 - <controller type='pci' model='pcie-root-port'/>
This is based on qemu's ioh3420.
8-10 - <controller type='pci' model='pcie-switch-upstream-port'/>
Based on qemu's x3130-upstream
11-13 - <controller type='pci'
model='pcie-switch-downstream-port'/>
(xio3130-downstream)
The first patch of each set adds a capability bit for qemu (again
non-controversial), the 2nd adds the new pci controller model, and the
3rd implements that model in qemu (by checking for the capability and
adding a commandline arg or failing).
The "controversial"/RFC bit is this - talking to Alex Williamson
(after I'd rwritten these patches, which is why I'm presenting them in
a form that I *don't* want to push) about the possibility of qemu
adding generic root-port, switch-upstream-port, and
switch-downstream-port controllers, and possibly also a generic
dmi-to-pci-bridge (i.e. controllers not tied to any particular
hardware implementation as with those currently available), I'm
realizing that, while it was a correct decision to make all of the
existing pci controllers "type='pci'" (since they share an address
space), using the "model" attribute to set the kind of controller was
probably a mistake. The problem - if:
<controller type='pci' model='dmi-to-pci-bridge'/>
currently means to add an i82801b11-bridge controller to the domain,
once qemu implements a generic dmi-to-pci-bridge, how will *that* be
denoted, and how will we avoid replacing the existing i81801b11-bridge
in a particular domain with the generic version when a guest is
restarted following a qemu/libvirt upgrade?
In hindsight, it probably would have been better to do something like
this with the four existing pci controllers:
<controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
<controller type='pci' subType='pci-bridge'
model='pci-bridge'/> (or maybe blank?)
<controller type='pci' subType='pci-root'/> (again maybe model is
blank)
<controller type='pci' subType='pcie-root'/>(and again)
(instead, what is shown above as "subType" is currently placed in the
"model" attribute).
So far, just the dmi-to-pci-bridge does not match the actual device
model used by qemu -- pci-bridge is generic and we don't format the
-roots.
Then we could add the 3 new types like this:
<controller type='pci' subType='pcie-root-port'
model='ioh3420'/>
<controller type='pci' subType='pcie-switch-upstream-port'
model='x3130-upstream/>
<controller type='pci' subType='pcie-switch-downstream-port'
model='xio3130-downstream/>
and we would easily be able to add support for new generic controllers
that behaved identically, by just adding a new model. But we haven't
done that, and anything we do in the future must be backwards
compatible with what's already there (right?). I'm trying to think of
how to satisfy backward compatibility while making things work better
in the future.
Some ideas, in no particular order:
===
Idea 1: multiplex the meaning of the "model" attribute, so we currently have:
<controller type='pci' model='dmi-to-pci-bridge'/>
which means "add an i82801b11-bridge device" and when we add a generic
version of this type of controller, we would do it with something like:
<controller type='pci' model='generic-dmi-to-pci-bridge'/>
and for another vendor's mythical controller:
<controller type='pci' model='xyz-dmi-to-pci-bridge'/>
This could be just 'xyz'. We do not have <model
type='rtl8193-pci-network-card'/>
or <memballoon model='virtio-memballoon'/>
For the new ones, it would be model='ioh3420', 'x3130-upstream', etc.
Cons: This will make for ugliness in switch statements where a new
case will have to be added whenever a different controller with
similar behavior/usage is supported. And it's generally not a good idea to
have a single attribute be used for two different functions.
===
Idea 2: implement new controllers as suggested in "20/20 hindsight"
above. For controllers in existing domains (dmi-to-pci-bridge,
pic-bridge, pci-root, and pcie-root) imply it into the controller
definition of an existing domain when only model has been given (but
don't write it out that way, to preserve the ability to downgrade). So
this:
[1] <controller type='pci' model='dmi-to-pci-bridge'/>
would internally mean this:
[2] <controller type='pci' subType='dmi-to-pci-bridge'
model='i82801b11-bridge'/>
If we want to allow forward migration, we need to treat a missing model
as 'i82801b11-bridge' anyway.
(but would remain as [1] when config is rewritten/migrated) while
this:
[3] <controller type='pci' subType='dmi-to-pci-bridge'
model='anything whatsoever/>
would mean exactly what it says.
Cons: Keeping this straight would mean having some sort of
"oldStyleCompat" flag in the controller object, to be sure that [1]
wasn't sent in migration status as [2] (since the destination might
not recognize it). It would also mean keeping the code in the parser
and formatter to deal with this flag. Forever.
So when would the 'subType' be formatted? If it's just internal, this
seems to be identical with my suggestion to Idea #1.
===
Idea 3: interpret controllers with missing subType as above, but
actually write it out to the config/migration/etc in the new modified
format.
Cons: This would prevent downgrading libvirt or migrating from a host
with newer libvirt to one with older libvirt. (Although preserving
compatibility at some level when downgrading may be a stated
requirement of some downstream distros' builds of libvirt, I think for
upstream it is only a "best effort"; I'm just not certain how much
"best" is considered to be :-)
I do not know of any effort done to make downgrading libvirt work.
Any machine configs that use new values for old attributes will
disappear (and so will running machines, because of new qemu
capabilities).
Migration is somewhat supported, so the compatible format could be used
only when the model is default and VIR_DOMAIN_DEF_FORMAT_MIGRATABLE was
specified?
===
Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
continue to use "model" to denote the subtype/class/whatever of
controller, and create a new attribute to denote the different
specific implementations of each one. So for example:
[4] <controller type='pci' model='dmi-to-pci-bridge'/>
would become:
[5] <controller type='pci' model='dmi-to-pci-bridge'
implementation='i82801b11-bridge'/>
(or some other name in place of "implementation" - ideas? I'm horrible
at thinkin up names)
device? actualModel? :)
Pros: wouldn't create compatibility problems when downgrading or
migrating cross version.
If you tried to migrate to older libvirt with:
model='dmi-to-pci-bridge' impl='generic', older libvirt would not parse
the impl flag and create a machine with i8201b11-bridge. (Assuming the
QEMU would know the machine type).
Cons: Is inconsistent with every other use of "model"
attribute in
libvirt, and each new addition of a PCI controller further propagates
this misuse.
It is consistent with all the other inconsistencies in libvirt :)
====
I currently like either option 2 or 3 (depending on how good we want
to be about supporting downgrade/intra-version migration), but as is
obvious by the fact that it was me that suggested putting the type of
pci controller into "model", I am very good at making the wrong
decision on matters like this.
My favorite would be #2 without the subType (AKA #1 without the
controller type in new model names) - it is implied from the
model anyway. A more structured XML might be user-frendlier, but this
way even old libvirt will recognize that it cannot start the machine
because of an incompatible model.
This way only the dmi-to-pci-bridge -> i82801b11-bridge mapping would be
odd.
Jan
3:06 a.m.
On 06/23/2015 11:57 AM, Ján Tomko wrote:
On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote:
> The first 4 patches are bugfixes/reorganizations that have no controversy.
>
> The sets of 5-7, 8-10, and 11-13 each implement a new model of PCI
> controller:
>
> 5-7 - <controller type='pci' model='pcie-root-port'/>
> This is based on qemu's ioh3420.
>
> 8-10 - <controller type='pci'
model='pcie-switch-upstream-port'/>
> Based on qemu's x3130-upstream
>
> 11-13 - <controller type='pci'
model='pcie-switch-downstream-port'/>
> (xio3130-downstream)
>
> The first patch of each set adds a capability bit for qemu (again
> non-controversial), the 2nd adds the new pci controller model, and the
> 3rd implements that model in qemu (by checking for the capability and
> adding a commandline arg or failing).
>
> The "controversial"/RFC bit is this - talking to Alex Williamson
> (after I'd rwritten these patches, which is why I'm presenting them in
> a form that I *don't* want to push) about the possibility of qemu
> adding generic root-port, switch-upstream-port, and
> switch-downstream-port controllers, and possibly also a generic
> dmi-to-pci-bridge (i.e. controllers not tied to any particular
> hardware implementation as with those currently available), I'm
> realizing that, while it was a correct decision to make all of the
> existing pci controllers "type='pci'" (since they share an address
> space), using the "model" attribute to set the kind of controller was
> probably a mistake. The problem - if:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> currently means to add an i82801b11-bridge controller to the domain,
> once qemu implements a generic dmi-to-pci-bridge, how will *that* be
> denoted, and how will we avoid replacing the existing i81801b11-bridge
> in a particular domain with the generic version when a guest is
> restarted following a qemu/libvirt upgrade?
>
> In hindsight, it probably would have been better to do something like
> this with the four existing pci controllers:
>
> <controller type='pci' subType='dmi-to-pci-bridge'
> model='i82801b11-bridge'/>
> <controller type='pci' subType='pci-bridge'
> model='pci-bridge'/> (or maybe blank?)
> <controller type='pci' subType='pci-root'/> (again maybe
model is blank)
> <controller type='pci' subType='pcie-root'/>(and again)
>
> (instead, what is shown above as "subType" is currently placed in the
"model" attribute).
So far, just the dmi-to-pci-bridge does not match the actual device
model used by qemu -- pci-bridge is generic and we don't format the
-roots.
Not yet, but as far as I understand, the pxb is another kind of pci-root.
> Then we could add the 3 new types like this:
>
> <controller type='pci' subType='pcie-root-port'
model='ioh3420'/>
> <controller type='pci' subType='pcie-switch-upstream-port'
> model='x3130-upstream/>
> <controller type='pci' subType='pcie-switch-downstream-port'
> model='xio3130-downstream/>
>
> and we would easily be able to add support for new generic controllers
> that behaved identically, by just adding a new model. But we haven't
> done that, and anything we do in the future must be backwards
> compatible with what's already there (right?). I'm trying to think of
> how to satisfy backward compatibility while making things work better
> in the future.
>
> Some ideas, in no particular order:
>
> ===
> Idea 1: multiplex the meaning of the "model" attribute, so we currently
have:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> which means "add an i82801b11-bridge device" and when we add a generic
> version of this type of controller, we would do it with something like:
>
> <controller type='pci' model='generic-dmi-to-pci-bridge'/>
>
> and for another vendor's mythical controller:
>
> <controller type='pci' model='xyz-dmi-to-pci-bridge'/>
>
This could be just 'xyz'. We do not have <model
type='rtl8193-pci-network-card'/>
or <memballoon model='virtio-memballoon'/>
because in those cases the type of hardware is already well known and
the different models have a similar function on the guest (in form, if
not exactly in implementation)
For the new ones, it would be model='ioh3420', 'x3130-upstream', etc.
I like having the XML give a clue to the exact function of the device
without needing to look up exactly what each specific device from some
random chipset does.
> Cons: This will make for ugliness in switch statements where a new
> case will have to be added whenever a different controller with
> similar behavior/usage is supported. And it's generally not a good idea to
> have a single attribute be used for two different functions.
>
> ===
>
> Idea 2: implement new controllers as suggested in "20/20 hindsight"
> above. For controllers in existing domains (dmi-to-pci-bridge,
> pic-bridge, pci-root, and pcie-root) imply it into the controller
> definition of an existing domain when only model has been given (but
> don't write it out that way, to preserve the ability to downgrade). So
> this:
>
> [1] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would internally mean this:
>
> [2] <controller type='pci' subType='dmi-to-pci-bridge'
> model='i82801b11-bridge'/>
>
If we want to allow forward migration, we need to treat a missing model
as 'i82801b11-bridge' anyway.
Correct. Just as we set a network device model of rtl8139 for any
<interface> that has no model (because rtl8139 has always been the
default up until now, but may not be in the future), we would treat a
missing [whatever it is] as "i82801b11-bridge", and also encode it into
the XML whenever this domain is re-saved to disk.
> (but would remain as [1] when config is rewritten/migrated) while
> this:
>
> [3] <controller type='pci' subType='dmi-to-pci-bridge'
> model='anything whatsoever/>
>
> would mean exactly what it says.
>
> Cons: Keeping this straight would mean having some sort of
> "oldStyleCompat" flag in the controller object, to be sure that [1]
> wasn't sent in migration status as [2] (since the destination might
> not recognize it). It would also mean keeping the code in the parser
> and formatter to deal with this flag. Forever.
So when would the 'subType' be formatted? If it's just internal, this
seems to be identical with my suggestion to Idea #1.
The idea would be that when it is specified in the XML of define/create,
it will also be formatted. But I think I've soured on this idea anyway.
> ===
> Idea 3: interpret controllers with missing subType as above, but
> actually write it out to the config/migration/etc in the new modified
> format.
>
> Cons: This would prevent downgrading libvirt or migrating from a host
> with newer libvirt to one with older libvirt. (Although preserving
> compatibility at some level when downgrading may be a stated
> requirement of some downstream distros' builds of libvirt, I think for
> upstream it is only a "best effort"; I'm just not certain how much
"best" is considered to be :-)
>
I do not know of any effort done to make downgrading libvirt work.
You haven't talked to enough people deploying RHEV/oVirt in production -
they want the ability to upgrade some nodes, migrate guests over to
them, then migrate the guests back if the upgrade needs to be undone.
Any machine configs that use new values for old attributes will
disappear (and so will running machines, because of new qemu
capabilities).
Migration is somewhat supported, so the compatible format could be used
only when the model is default and VIR_DOMAIN_DEF_FORMAT_MIGRATABLE was
specified?
Isn't VIR_DOMAIN_DEF_FORMAT_MIGRATABLE there in part so that migrations
from newer libvirt to older libvirt might work? (it doesn't guarantee
it, but it does make it work in some situations where it otherwise
wouldn't).
> ===
> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
> continue to use "model" to denote the subtype/class/whatever of
> controller, and create a new attribute to denote the different
> specific implementations of each one. So for example:
>
> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would become:
>
> [5] <controller type='pci' model='dmi-to-pci-bridge'
> implementation='i82801b11-bridge'/>
>
> (or some other name in place of "implementation" - ideas? I'm horrible
> at thinkin up names)
>
device? actualModel? :)
hey, I think I might like "device"!
> Pros: wouldn't create compatibility problems when downgrading or
> migrating cross version.
>
If you tried to migrate to older libvirt with:
model='dmi-to-pci-bridge' impl='generic', older libvirt would not parse
the impl flag and create a machine with i8201b11-bridge. (Assuming the
QEMU would know the machine type).
Well, yeah, this does point out a flaw in my thinking :-/
> Cons: Is inconsistent with every other use of "model" attribute in
> libvirt, and each new addition of a PCI controller further propagates
> this misuse.
>
It is consistent with all the other inconsistencies in libvirt :)
> ====
>
> I currently like either option 2 or 3 (depending on how good we want
> to be about supporting downgrade/intra-version migration), but as is
> obvious by the fact that it was me that suggested putting the type of
> pci controller into "model", I am very good at making the wrong
> decision on matters like this.
My favorite would be #2 without the subType (AKA #1 without the
controller type in new model names) - it is implied from the
model anyway. A more structured XML might be user-frendlier, but this
way even old libvirt will recognize that it cannot start the machine
because of an incompatible model.
This way only the dmi-to-pci-bridge -> i82801b11-bridge mapping would be
odd.
I have to think about this some more...
10:32 a.m.
On Tue, Jun 23, 2015 at 09:06:24PM -0400, Laine Stump wrote:
On 06/23/2015 11:57 AM, Ján Tomko wrote:
> On Mon, Jun 22, 2015 at 02:44:05PM -0400, Laine Stump wrote:
[snip]
>> ===
>> Idea 3: interpret controllers with missing subType as above, but
>> actually write it out to the config/migration/etc in the new modified
>> format.
>>
>> Cons: This would prevent downgrading libvirt or migrating from a host
>> with newer libvirt to one with older libvirt. (Although preserving
>> compatibility at some level when downgrading may be a stated
>> requirement of some downstream distros' builds of libvirt, I think for
>> upstream it is only a "best effort"; I'm just not certain how much
"best" is considered to be :-)
>>
> I do not know of any effort done to make downgrading libvirt work.
You haven't talked to enough people deploying RHEV/oVirt in production -
they want the ability to upgrade some nodes, migrate guests over to
them, then migrate the guests back if the upgrade needs to be undone.
That is migration, where we pass the "migratable" XML.
The domain configs and live status XMLs can contain things that won't
be parsable by older libvirt. Under 'downgrade' I imagined simply
installing older libvirt packages - this can possibly fail, even if
the domains only use features present in the old libvirt too.
> Any machine configs that use new values for old attributes will
> disappear (and so will running machines, because of new qemu
> capabilities).
>
> Migration is somewhat supported, so the compatible format could be used
> only when the model is default and VIR_DOMAIN_DEF_FORMAT_MIGRATABLE was
> specified?
Isn't VIR_DOMAIN_DEF_FORMAT_MIGRATABLE there in part so that migrations
from newer libvirt to older libvirt might work? (it doesn't guarantee
it, but it does make it work in some situations where it otherwise
wouldn't).
Yes, if the domain worked with the old libvirt, we should be able to
migrate to the new libvirt and back, thanks to this flag.
>
>> ===
>> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
>> continue to use "model" to denote the subtype/class/whatever of
>> controller, and create a new attribute to denote the different
>> specific implementations of each one. So for example:
>>
>> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>>
>> would become:
>>
>> [5] <controller type='pci' model='dmi-to-pci-bridge'
>> implementation='i82801b11-bridge'/>
>>
>> (or some other name in place of "implementation" - ideas? I'm
horrible
>> at thinkin up names)
>>
> device? actualModel? :)
hey, I think I might like "device"!
>
>> Pros: wouldn't create compatibility problems when downgrading or
>> migrating cross version.
>>
> If you tried to migrate to older libvirt with:
> model='dmi-to-pci-bridge' impl='generic', older libvirt would not
parse
> the impl flag and create a machine with i8201b11-bridge. (Assuming the
> QEMU would know the machine type).
Well, yeah, this does point out a flaw in my thinking :-/
This happens with all new XML elements - libvirt's parser usually does
an XPath query for the elements it knows and ignores the unknown ones.
IIRC this is on purpose.
Jan
6:04 p.m.
I think the only votes were for option 1 and 4 (interesting how the only
ones that were chosen were those that I *didn't* pick personally :-).
See comments below. In the meantime the other issue Alex pointed out may
cause this to take a slightly different direction.
On 06/22/2015 02:44 PM, Laine Stump wrote:
===
Idea 1: multiplex the meaning of the "model" attribute, so we currently have:
<controller type='pci' model='dmi-to-pci-bridge'/>
which means "add an i82801b11-bridge device" and when we add a generic
version of this type of controller, we would do it with something like:
<controller type='pci' model='generic-dmi-to-pci-bridge'/>
and for another vendor's mythical controller:
<controller type='pci' model='xyz-dmi-to-pci-bridge'/>
Cons: This will make for ugliness in switch statements where a new
case will have to be added whenever a different controller with
similar behavior/usage is supported. And it's generally not a good idea to
have a single attribute be used for two different functions.
jtomko advocated this one because it would make it easier for an older
libvirt to notice an unsupported class+model of controller during a
migration attempt from a newer libvirt. An example would be if a newer
libvirt had a guest with
<controller type='pci' model='xyz'/>
(where xyz is some qemu device that implements a dmi-to-pci-bridge)
That would fail on an attempt to migrate to older libvirt, but
<controller type='pci' model='dmi-to-pci-bridge'/>
would succeed. On the other hand, if we did this:
<controller type='pci' model='dmi-to-pci-bridge
submodel='xyz'/>
that would succeed (and create the wrong device) because submodel would
be ignored.
Since there is currently no alternate implementation of a
dmi-to-pci-bridge available in qemu, and it will likely be awhile until
that happens, I think if we start filling out the XML now as:
<controller type='pci' model='dmi-to-pci-bridge'
submodel='i82801b11-bridge'/>
by the time we get to the point that we do have an alternate 'xyz'
controller, any version of libvirt that doesn't understand "submodel"
will be so far in the past that we wouldn't want to be able to migrate
back to it anyway.
===
Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
continue to use "model" to denote the subtype/class/whatever of
controller, and create a new attribute to denote the different
specific implementations of each one. So for example:
[4] <controller type='pci' model='dmi-to-pci-bridge'/>
would become:
[5] <controller type='pci' model='dmi-to-pci-bridge'
implementation='i82801b11-bridge'/>
(or some other name in place of "implementation" - ideas? I'm horrible
at thinkin up names)
Pros: wouldn't create compatibility problems when downgrading or
migrating cross version.
Cons: Is inconsistent with every other use of "model" attribute in
libvirt, and each new addition of a PCI controller further propagates
this misuse.
mkletzan preferred this one, and danpb was amenable to it in IRC. I
think I now agree, but Alex's comments about needing to keep track of
what we put in the qemu commandline for port and chassis have me
thinking this isn't enough.
The problem is that the "ioh3420" device needs its "port" and
"chassis"
options set; Alex recommends setting port to:
(slot << 3)+function
Likewise, he recommends setting "port" for the xio3130-downstream device
to the same value as slot. So, for the following:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
</controller>
we would end up with the following commandline:
-device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0',
addr=0x4.0x1
(chassis is the same as "index" in the xml (again per Alex's
recommendation), and port is (4 << 3) + 1 = 0x21)
*However* he also says that we may change our mind on these in the
future, so chassis and port may end up being something different. Since
those values are visible to the guest, we can't allow them to change on
an existing machine, as it breaks the guest ABI. So we need to store
them in the XML. They aren't part of the PCI address though, they are
options. So I need to figure out the best way to represent that in the
XML, and fill it in when it is auto-generated when the controller is
defined. A few possible ways to solve both problems at once:
[1] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<target type='ioh3420' chassis='3' port='0x21'/>
</controller>
Precedent: <target> is used to store the port number for serial/console
devices, specify guest-side bus and device name for disks, specify
guest-side mount location for filesystems, specify the *host*-side name
of tap devices for network interfaces, memory size for <memory>. So it
seems kind of proper. (slight variation - <target model='ioh3420' .../>
:-/ )
[2] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420' chassis='3' port='0x21'/>
</controller>
(How's *that* for confusing?!? Both a model attribute *and* a model
subelement.)
Precedent: This is exactly patterned after the use of <model> in <video>
devices though, where the specific card being emulated along with any
memory/etc options are put in <model>, so at least there is precedence)
[3] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<device type='ioh3420' chassis='3' port='0x21'/>
</controller>
(This one is completely new, just for a fresh start in case people think
neither of the others are exactly right)
At this point you can see that it all comes down to what name we want to
give the subelement; within that, we give the exact name of the qemu
device, and the exact name/value of any qemu options that we set to
non-default values.
Somebody *please* have an opinion on the name for this, because none of
these strikes me as better or worse (well, I think I dislike <driver>
(P.S. There are other "automagic" qemu options being specified by
libvirt that maybe could use the same treatment as these. Two that come
to mind are chassis_nr for pci-bridge controllers (set to the
controller's index), and "vectors" for virtio-net multiqueue support
(set to (queues*2)+2).
6:17 p.m.
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote:
I think the only votes were for option 1 and 4 (interesting how the
only
ones that were chosen were those that I *didn't* pick personally :-).
See comments below. In the meantime the other issue Alex pointed out may
cause this to take a slightly different direction.
On 06/22/2015 02:44 PM, Laine Stump wrote:
> ===
> Idea 1: multiplex the meaning of the "model" attribute, so we currently
have:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> which means "add an i82801b11-bridge device" and when we add a generic
> version of this type of controller, we would do it with something like:
>
> <controller type='pci' model='generic-dmi-to-pci-bridge'/>
>
> and for another vendor's mythical controller:
>
> <controller type='pci' model='xyz-dmi-to-pci-bridge'/>
>
> Cons: This will make for ugliness in switch statements where a new
> case will have to be added whenever a different controller with
> similar behavior/usage is supported. And it's generally not a good idea to
> have a single attribute be used for two different functions.
jtomko advocated this one because it would make it easier for an older
libvirt to notice an unsupported class+model of controller during a
migration attempt from a newer libvirt. An example would be if a newer
libvirt had a guest with
<controller type='pci' model='xyz'/>
(where xyz is some qemu device that implements a dmi-to-pci-bridge)
That would fail on an attempt to migrate to older libvirt, but
<controller type='pci' model='dmi-to-pci-bridge'/>
would succeed. On the other hand, if we did this:
<controller type='pci' model='dmi-to-pci-bridge
submodel='xyz'/>
that would succeed (and create the wrong device) because submodel would
be ignored.
Since there is currently no alternate implementation of a
dmi-to-pci-bridge available in qemu, and it will likely be awhile until
that happens, I think if we start filling out the XML now as:
<controller type='pci' model='dmi-to-pci-bridge'
submodel='i82801b11-bridge'/>
by the time we get to the point that we do have an alternate 'xyz'
controller, any version of libvirt that doesn't understand "submodel"
will be so far in the past that we wouldn't want to be able to migrate
back to it anyway.
> ===
> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
> continue to use "model" to denote the subtype/class/whatever of
> controller, and create a new attribute to denote the different
> specific implementations of each one. So for example:
>
> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would become:
>
> [5] <controller type='pci' model='dmi-to-pci-bridge'
> implementation='i82801b11-bridge'/>
>
> (or some other name in place of "implementation" - ideas? I'm
horrible
> at thinkin up names)
>
> Pros: wouldn't create compatibility problems when downgrading or
> migrating cross version.
>
> Cons: Is inconsistent with every other use of "model" attribute in
> libvirt, and each new addition of a PCI controller further propagates
> this misuse.
>
mkletzan preferred this one, and danpb was amenable to it in IRC. I
think I now agree, but Alex's comments about needing to keep track of
what we put in the qemu commandline for port and chassis have me
thinking this isn't enough.
The problem is that the "ioh3420" device needs its "port" and
"chassis"
options set; Alex recommends setting port to:
(slot << 3)+function
Likewise, he recommends setting "port" for the xio3130-downstream device
to the same value as slot. So, for the following:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
</controller>
we would end up with the following commandline:
-device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0',
addr=0x4.0x1
(chassis is the same as "index" in the xml (again per Alex's
recommendation), and port is (4 << 3) + 1 = 0x21)
*However* he also says that we may change our mind on these in the
future, so chassis and port may end up being something different. Since
those values are visible to the guest, we can't allow them to change on
an existing machine, as it breaks the guest ABI. So we need to store
them in the XML. They aren't part of the PCI address though, they are
options. So I need to figure out the best way to represent that in the
XML, and fill it in when it is auto-generated when the controller is
defined. A few possible ways to solve both problems at once:
[1] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<target type='ioh3420' chassis='3' port='0x21'/>
</controller>
Precedent: <target> is used to store the port number for serial/console
devices, specify guest-side bus and device name for disks, specify
guest-side mount location for filesystems, specify the *host*-side name
of tap devices for network interfaces, memory size for <memory>. So it
seems kind of proper. (slight variation - <target model='ioh3420' .../>
:-/ )
[2] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420' chassis='3' port='0x21'/>
</controller>
(How's *that* for confusing?!? Both a model attribute *and* a model
subelement.)
That's no worse than model & submodel really. If anything it is better
as having a child element gives more room for expansion than an
attribute, and <model> is still a fairly standard approach we've
used inpast.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
8:44 a.m.
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote:
I think the only votes were for option 1 and 4 (interesting how the
only
ones that were chosen were those that I *didn't* pick personally :-).
See comments below. In the meantime the other issue Alex pointed out may
cause this to take a slightly different direction.
On 06/22/2015 02:44 PM, Laine Stump wrote:
> ===
> Idea 1: multiplex the meaning of the "model" attribute, so we currently
have:
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> which means "add an i82801b11-bridge device" and when we add a generic
> version of this type of controller, we would do it with something like:
>
> <controller type='pci' model='generic-dmi-to-pci-bridge'/>
>
> and for another vendor's mythical controller:
>
> <controller type='pci' model='xyz-dmi-to-pci-bridge'/>
>
> Cons: This will make for ugliness in switch statements where a new
> case will have to be added whenever a different controller with
> similar behavior/usage is supported. And it's generally not a good idea to
> have a single attribute be used for two different functions.
jtomko advocated this one because it would make it easier for an older
libvirt to notice an unsupported class+model of controller during a
migration attempt from a newer libvirt. An example would be if a newer
libvirt had a guest with
<controller type='pci' model='xyz'/>
(where xyz is some qemu device that implements a dmi-to-pci-bridge)
That would fail on an attempt to migrate to older libvirt, but
<controller type='pci' model='dmi-to-pci-bridge'/>
would succeed. On the other hand, if we did this:
<controller type='pci' model='dmi-to-pci-bridge
submodel='xyz'/>
that would succeed (and create the wrong device) because submodel would
be ignored.
Since there is currently no alternate implementation of a
dmi-to-pci-bridge available in qemu, and it will likely be awhile until
that happens, I think if we start filling out the XML now as:
<controller type='pci' model='dmi-to-pci-bridge'
submodel='i82801b11-bridge'/>
by the time we get to the point that we do have an alternate 'xyz'
controller, any version of libvirt that doesn't understand "submodel"
will be so far in the past that we wouldn't want to be able to migrate
back to it anyway.
> ===
> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
> continue to use "model" to denote the subtype/class/whatever of
> controller, and create a new attribute to denote the different
> specific implementations of each one. So for example:
>
> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would become:
>
> [5] <controller type='pci' model='dmi-to-pci-bridge'
> implementation='i82801b11-bridge'/>
>
> (or some other name in place of "implementation" - ideas? I'm horrible
> at thinkin up names)
>
> Pros: wouldn't create compatibility problems when downgrading or
> migrating cross version.
>
> Cons: Is inconsistent with every other use of "model" attribute in
> libvirt, and each new addition of a PCI controller further propagates
> this misuse.
>
mkletzan preferred this one, and danpb was amenable to it in IRC. I
think I now agree, but Alex's comments about needing to keep track of
what we put in the qemu commandline for port and chassis have me
thinking this isn't enough.
The problem is that the "ioh3420" device needs its "port" and
"chassis"
options set; Alex recommends setting port to:
(slot << 3)+function
Likewise, he recommends setting "port" for the xio3130-downstream device
to the same value as slot. So, for the following:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
</controller>
we would end up with the following commandline:
-device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0',
addr=0x4.0x1
(chassis is the same as "index" in the xml (again per Alex's
recommendation), and port is (4 << 3) + 1 = 0x21)
*However* he also says that we may change our mind on these in the
future, so chassis and port may end up being something different. Since
those values are visible to the guest, we can't allow them to change on
an existing machine, as it breaks the guest ABI. So we need to store
them in the XML. They aren't part of the PCI address though, they are
options. So I need to figure out the best way to represent that in the
XML, and fill it in when it is auto-generated when the controller is
defined. A few possible ways to solve both problems at once:
[1] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<target type='ioh3420' chassis='3' port='0x21'/>
</controller>
Precedent: <target> is used to store the port number for serial/console
devices, specify guest-side bus and device name for disks, specify
guest-side mount location for filesystems, specify the *host*-side name
of tap devices for network interfaces, memory size for <memory>. So it
seems kind of proper. (slight variation - <target model='ioh3420' .../>
:-/ )
[2] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420' chassis='3' port='0x21'/>
</controller>
(How's *that* for confusing?!? Both a model attribute *and* a model
subelement.)
Precedent: This is exactly patterned after the use of <model> in <video>
devices though, where the specific card being emulated along with any
memory/etc options are put in <model>, so at least there is precedence)
[3] <controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<device type='ioh3420' chassis='3' port='0x21'/>
</controller>
(This one is completely new, just for a fresh start in case people think
neither of the others are exactly right)
At this point you can see that it all comes down to what name we want to
give the subelement; within that, we give the exact name of the qemu
device, and the exact name/value of any qemu options that we set to
non-default values.
Somebody *please* have an opinion on the name for this, because none of
these strikes me as better or worse (well, I think I dislike <driver>
To be honest, I kinds dislike all of them. Not that they would be
chosen poorly, no, it's simply because the good sensible choice is
unavailable due to another poor decision in the past (this may be
another point for Michal's talk on KVM Forum). Thinking about it I
must say I don't like how target (which is supposed to match a place
where the device appears for the guest) is used for the model
specification, on the other hand (ab)using 'model' element for the
specification of an "address" in guest (that's what I understand
chassis and port are) doesn't feel any better. What if we go with two
of those elements? Would that be too much pain? E.g.:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420'/>
<target chassis='3' port='0x21'/>
</controller>
I understand this might look like an overkill, but I think it's better
safe then sorry, I guess I just see us not so far in the future
regretting any decision made now.
Another idea I briefly thought about, which would also deal with the
migration to older libvirt versions was inventing a new address
type ... and then I realized how stupid^Wugly was that.
Anyway, I must say that I don't fully get the idea of how some of
these controllers are supposed to be specified and what particular HW
they should describe, so I might not had the best idea in town.
<rant> Mainly I don't understand why there's no pci switch. Or is
<controller type='pci'/> actually a pci switch (I usderstand it like
that)? Then why is there a downstream and upstream device when
{down,up}stream should just indicate whether the port aims towards the
root complex or the endpoints, respectively. </rant>
(P.S. There are other "automagic" qemu options being specified by
libvirt that maybe could use the same treatment as these. Two that come
to mind are chassis_nr for pci-bridge controllers (set to the
controller's index), and "vectors" for virtio-net multiqueue support
(set to (queues*2)+2).
I guess 'chassis_nr' is something exposed to guest, similarly to
'chassis' in the pci-root-port case, so that might be worth saving in
the XML. Maybe we can use 'chassis' in the target/model specification
for storing the chassis_nr as a level of abstraction on libvirt level.
But the number of vectors (to my knowledge) is not something exposed
to guest and is only advised to being set according to the formula you
wrote, above.
10:59 a.m.
On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote:
To be honest, I kinds dislike all of them. Not that they would be
chosen poorly, no, it's simply because the good sensible choice is
unavailable due to another poor decision in the past (this may be
another point for Michal's talk on KVM Forum). Thinking about it I
must say I don't like how target (which is supposed to match a place
where the device appears for the guest) is used for the model
specification, on the other hand (ab)using 'model' element for the
specification of an "address" in guest (that's what I understand
chassis and port are) doesn't feel any better. What if we go with two
of those elements? Would that be too much pain? E.g.:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420'/>
<target chassis='3' port='0x21'/>
</controller>
I understand this might look like an overkill, but I think it's better
safe then sorry, I guess I just see us not so far in the future
regretting any decision made now.
I'd be fine with this proposal too.
Regards,
Daniel
--
|: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
11:46 a.m.
On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote:
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote:
>At this point you can see that it all comes down to what name we want to
>give the subelement; within that, we give the exact name of the qemu
>device, and the exact name/value of any qemu options that we set to
>non-default values.
>
>Somebody *please* have an opinion on the name for this, because none of
>these strikes me as better or worse (well, I think I dislike <driver>
>
To be honest, I kinds dislike all of them. Not that they would be
chosen poorly, no, it's simply because the good sensible choice is
unavailable due to another poor decision in the past (this may be
another point for Michal's talk on KVM Forum). Thinking about it I
must say I don't like how target (which is supposed to match a place
where the device appears for the guest) is used for the model
specification, on the other hand (ab)using 'model' element for the
specification of an "address" in guest (that's what I understand
chassis and port are) doesn't feel any better. What if we go with two
of those elements? Would that be too much pain? E.g.:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420'/>
<target chassis='3' port='0x21'/>
</controller>
I like this, essentially a subModel without the camelCase.
One small nit: <model name='ioh4320'/> would look nicer to me,
but we're using <model type=/> in at least one other place,
so I'm fine with both.
Jan
I understand this might look like an overkill, but I think it's better
safe then sorry, I guess I just see us not so far in the future
regretting any decision made now.
12:51 p.m.
On Thu, Jun 25, 2015 at 11:46:41AM +0200, Ján Tomko wrote:
On Thu, Jun 25, 2015 at 08:44:17AM +0200, Martin Kletzander wrote:
> On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote:
> >At this point you can see that it all comes down to what name we want to
> >give the subelement; within that, we give the exact name of the qemu
> >device, and the exact name/value of any qemu options that we set to
> >non-default values.
> >
> >Somebody *please* have an opinion on the name for this, because none of
> >these strikes me as better or worse (well, I think I dislike <driver>
> >
>
> To be honest, I kinds dislike all of them. Not that they would be
> chosen poorly, no, it's simply because the good sensible choice is
> unavailable due to another poor decision in the past (this may be
> another point for Michal's talk on KVM Forum). Thinking about it I
> must say I don't like how target (which is supposed to match a place
> where the device appears for the guest) is used for the model
> specification, on the other hand (ab)using 'model' element for the
> specification of an "address" in guest (that's what I understand
> chassis and port are) doesn't feel any better. What if we go with two
> of those elements? Would that be too much pain? E.g.:
>
> <controller type='pci model='pci-root-port' index='3'>
> <address type='pci' bus='0' slot='4'
function='1'>
> <model type='ioh3420'/>
> <target chassis='3' port='0x21'/>
> </controller>
I like this, essentially a subModel without the camelCase.
One small nit: <model name='ioh4320'/> would look nicer to me,
but we're using <model type=/> in at least one other place,
so I'm fine with both.
I didn't think about the 'type', I probably just typed it in there
because each line had a 'type' already in :) I like 'name' better
too, but diving deeply into it, I have no strong opinions on that.
Jan
>
> I understand this might look like an overkill, but I think it's better
> safe then sorry, I guess I just see us not so far in the future
> regretting any decision made now.
>
5:54 p.m.
On 06/25/2015 02:44 AM, Martin Kletzander wrote:
On Wed, Jun 24, 2015 at 12:04:33PM -0400, Laine Stump wrote:
> I think the only votes were for option 1 and 4 (interesting how the only
> ones that were chosen were those that I *didn't* pick personally :-).
> See comments below. In the meantime the other issue Alex pointed out may
> cause this to take a slightly different direction.
>
> On 06/22/2015 02:44 PM, Laine Stump wrote:
>> ===
>> Idea 1: multiplex the meaning of the "model" attribute, so we
>> currently have:
>>
>> <controller type='pci' model='dmi-to-pci-bridge'/>
>>
>> which means "add an i82801b11-bridge device" and when we add a generic
>> version of this type of controller, we would do it with something like:
>>
>> <controller type='pci'
model='generic-dmi-to-pci-bridge'/>
>>
>> and for another vendor's mythical controller:
>>
>> <controller type='pci' model='xyz-dmi-to-pci-bridge'/>
>>
>> Cons: This will make for ugliness in switch statements where a new
>> case will have to be added whenever a different controller with
>> similar behavior/usage is supported. And it's generally not a good
>> idea to
>> have a single attribute be used for two different functions.
>
> jtomko advocated this one because it would make it easier for an older
> libvirt to notice an unsupported class+model of controller during a
> migration attempt from a newer libvirt. An example would be if a newer
> libvirt had a guest with
>
> <controller type='pci' model='xyz'/>
>
> (where xyz is some qemu device that implements a dmi-to-pci-bridge)
>
> That would fail on an attempt to migrate to older libvirt, but
>
> <controller type='pci' model='dmi-to-pci-bridge'/>
>
> would succeed. On the other hand, if we did this:
>
> <controller type='pci' model='dmi-to-pci-bridge
submodel='xyz'/>
>
> that would succeed (and create the wrong device) because submodel would
> be ignored.
>
> Since there is currently no alternate implementation of a
> dmi-to-pci-bridge available in qemu, and it will likely be awhile until
> that happens, I think if we start filling out the XML now as:
>
> <controller type='pci' model='dmi-to-pci-bridge'
> submodel='i82801b11-bridge'/>
>
> by the time we get to the point that we do have an alternate 'xyz'
> controller, any version of libvirt that doesn't understand "submodel"
> will be so far in the past that we wouldn't want to be able to migrate
> back to it anyway.
>
>
>> ===
>> Idea 4: Unlike other uses of "model" in libvirt, for pci controllers,
>> continue to use "model" to denote the subtype/class/whatever of
>> controller, and create a new attribute to denote the different
>> specific implementations of each one. So for example:
>>
>> [4] <controller type='pci' model='dmi-to-pci-bridge'/>
>>
>> would become:
>>
>> [5] <controller type='pci' model='dmi-to-pci-bridge'
>> implementation='i82801b11-bridge'/>
>>
>> (or some other name in place of "implementation" - ideas? I'm
horrible
>> at thinkin up names)
>>
>> Pros: wouldn't create compatibility problems when downgrading or
>> migrating cross version.
>>
>> Cons: Is inconsistent with every other use of "model" attribute in
>> libvirt, and each new addition of a PCI controller further propagates
>> this misuse.
>>
>
> mkletzan preferred this one, and danpb was amenable to it in IRC. I
> think I now agree, but Alex's comments about needing to keep track of
> what we put in the qemu commandline for port and chassis have me
> thinking this isn't enough.
>
> The problem is that the "ioh3420" device needs its "port" and
"chassis"
> options set; Alex recommends setting port to:
>
> (slot << 3)+function
>
> Likewise, he recommends setting "port" for the xio3130-downstream device
> to the same value as slot. So, for the following:
>
> <controller type='pci model='pci-root-port' index='3'>
> <address type='pci' bus='0' slot='4'
function='1'>
> </controller>
>
> we would end up with the following commandline:
>
> -device ioh3420,chassis=3,port=0x21,id='pcie.3',bus='pcie.0',
> addr=0x4.0x1
>
> (chassis is the same as "index" in the xml (again per Alex's
> recommendation), and port is (4 << 3) + 1 = 0x21)
>
> *However* he also says that we may change our mind on these in the
> future, so chassis and port may end up being something different. Since
> those values are visible to the guest, we can't allow them to change on
> an existing machine, as it breaks the guest ABI. So we need to store
> them in the XML. They aren't part of the PCI address though, they are
> options. So I need to figure out the best way to represent that in the
> XML, and fill it in when it is auto-generated when the controller is
> defined. A few possible ways to solve both problems at once:
>
> [1] <controller type='pci model='pci-root-port' index='3'>
> <address type='pci' bus='0' slot='4'
function='1'>
> <target type='ioh3420' chassis='3' port='0x21'/>
> </controller>
>
> Precedent: <target> is used to store the port number for serial/console
> devices, specify guest-side bus and device name for disks, specify
> guest-side mount location for filesystems, specify the *host*-side name
> of tap devices for network interfaces, memory size for <memory>. So it
> seems kind of proper. (slight variation - <target model='ioh3420'
.../>
> :-/ )
>
> [2] <controller type='pci model='pci-root-port' index='3'>
> <address type='pci' bus='0' slot='4'
function='1'>
> <model type='ioh3420' chassis='3' port='0x21'/>
> </controller>
>
> (How's *that* for confusing?!? Both a model attribute *and* a model
> subelement.)
>
> Precedent: This is exactly patterned after the use of <model> in <video>
> devices though, where the specific card being emulated along with any
> memory/etc options are put in <model>, so at least there is precedence)
>
> [3] <controller type='pci model='pci-root-port' index='3'>
> <address type='pci' bus='0' slot='4'
function='1'>
> <device type='ioh3420' chassis='3' port='0x21'/>
> </controller>
>
> (This one is completely new, just for a fresh start in case people think
> neither of the others are exactly right)
>
> At this point you can see that it all comes down to what name we want to
> give the subelement; within that, we give the exact name of the qemu
> device, and the exact name/value of any qemu options that we set to
> non-default values.
>
> Somebody *please* have an opinion on the name for this, because none of
> these strikes me as better or worse (well, I think I dislike <driver>
>
To be honest, I kinds dislike all of them. Not that they would be
chosen poorly, no, it's simply because the good sensible choice is
unavailable due to another poor decision in the past (this may be
another point for Michal's talk on KVM Forum). Thinking about it I
must say I don't like how target (which is supposed to match a place
where the device appears for the guest) is used for the model
specification, on the other hand (ab)using 'model' element for the
specification of an "address" in guest (that's what I understand
chassis and port are)
Actually they aren't an address. The address where the controller is
connected is completely specified in its <address> element, and the
address where other devices can connect to this controller is specified
by 1) the index of the controller and 2) implied (from the class of
controller) information about how many "slots" are available on this
controller and the starting slot#.
As it was described to me (by Alex in IRC and email) chassis,
chassis_nr, and port are just numbers that are stored in a register that
the guest can examine, and on real hardware they are used to learn, e.g.
the physical location of a connector. From libvirt's point of view, you
can end the previous sentence after the word "register". This is part of
the reason the proper solution isn't obvious/clear.
doesn't feel any better. What if we go with two
of those elements? Would that be too much pain? E.g.:
<controller type='pci model='pci-root-port' index='3'>
<address type='pci' bus='0' slot='4'
function='1'>
<model type='ioh3420'/>
<target chassis='3' port='0x21'/>
</controller>
I understand this might look like an overkill, but I think it's better
safe then sorry, I guess I just see us not so far in the future
regretting any decision made now.
Well, another item that we will want to specify in the future is the
numa node of a pxb bridge (which I now understand as another kind of
"pci-bridge", *not* another kind of "pci-root" as I previously
believed). I just looked for where numa node info is set for other
devices, and see that for the <memory> device, it is set with the numa
*subelement* of the target subelement of <memory>. So:
<memory ....>
<target>
<numa>1</numa>
</target>
<memory>
On the other hand, guest-side nume node for CPUs is set like this:
<cpu>
<numa>
<cell id='0' cpus='0-3' memory='512000'
unit='KiB'/>
<cell id='1' cpus='4-7' memory='512000'
unit='KiB'
memAccess='shared'/>
</numa>
</cpu>
so no use of <target> at all in that case.
I've tried to find a "smoking gun" item that would force me to use your
suggestion, but haven't yet.
So it comes down to this: Do we want to be more consistent with:
<video>
<model type='vga' vram='16384' heads='1'>
<acceleration accel3d='yes' accel2d='yes'/>
</model>
</video>
(i.e. everything in <model>) or:
<memory ...>
<target>
<size>blah</size>
<node>1</node>
</target>
...
</memory>
and
<disk ...>
<target dev='hdb' bus='ide'/>
</disk
???
Another idea I briefly thought about, which would also deal with the
migration to older libvirt versions was inventing a new address
type ... and then I realized how stupid^Wugly was that.
Anyway, I must say that I don't fully get the idea of how some of
these controllers are supposed to be specified and what particular HW
they should describe, so I might not had the best idea in town.
It's like a hybrid set of legos and duplos (and also some "leglos" or
"dupos" or something) mixed together - you can't just plug
"anything"
into "anything". Each controller has limitations on what it can be
plugged into, what (and how many) can be plugged into it, and whether or
not each of the connections (both up and down) supports hotplug/unplug.
It's taken me quite awhile to get a decent model in my mind of how they
all fit together, and I still learn something almost daily that
invalidates some belief I've had.
<rant> Mainly I don't understand why there's no pci switch. Or is
<controller type='pci'/> actually a pci switch (I usderstand it like
that)? Then why is there a downstream and upstream device when
{down,up}stream should just indicate whether the port aims towards the
root complex or the endpoints, respectively. </rant>
Yeah, that one bothers me too. The picture I have in my mind is that the
x3130-upstream is a switch with one upstream (pcie) port and 32
downstream (switch) ports, and that the xio3130-downstream device is a
"converter dongle" that has one upstream (switch) port and one
downstream (pcie) port. So the upstream port gives you a switch and a
bunch of downstream ports, but you can only plug one kind of thing into
them. I wanted to call the upstream one "pcie-switch" and the downstream
one "pcie-switch-port", but Alex (who is my go-to authority on all
things PCIe) said that seemed wrong to him, the "switch" is the
collection of one upstream and one or more downstream ports, and that he
really wanted the "upstream" and "downstream" in the names. That's
why I
now have pcie-switch-upstream-port and pcie-switch-downstream-port. So
the way I think about it now is that the "switch" is this magical
nonexistent thing that sits between an upstream and a bunch of
downstreams. I still like the other way better, but am going with what
seems right to Alex because he's been dealing with these devices for
much longer than me so he almost certainly knows some nuances that I'm
unaware of.
>
>
> (P.S. There are other "automagic" qemu options being specified by
> libvirt that maybe could use the same treatment as these. Two that come
> to mind are chassis_nr for pci-bridge controllers (set to the
> controller's index), and "vectors" for virtio-net multiqueue support
> (set to (queues*2)+2).
I guess 'chassis_nr' is something exposed to guest, similarly to
'chassis' in the pci-root-port case, so that might be worth saving in
the XML. Maybe we can use 'chassis' in the target/model specification
for storing the chassis_nr as a level of abstraction on libvirt level.
That thought crossed my mind, but then I thought of the possibility that
there may some day be a device that uses both chassis and chassis_nr at
the same time. I even have a vague memory of something like this
happening in the past (although it may have been somewhere totally
unrelated to libvirt). For that reason,
But the number of vectors (to my knowledge) is not something exposed
to guest and is only advised to being set according to the formula you
wrote, above.
Ah, good point. We're safe on that one then.
3373
days inactive
3377
days old
37 comments
5 participants
participants (5)
-
Alex Williamson -
Daniel P. Berrange -
Ján Tomko -
Laine Stump -
Martin Kletzander