7:30 a.m.
These patches are all in response to the issues that Gerd Hoffman
raised in
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Three of the six are simple refactoring of existing code, done
separately so that the patches that change functionality would be
simpler to review. None of them is very long.
Laine Stump (6):
qemu: eliminate redundant if clauses in qemuCollectPCIAddress
qemu: allow some PCI devices to be attached to PCIe slots
qemu: replace multiple strcmps with a switch on an enum
qemu: support ich9-intel-hda audio device
qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 6 +-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 197 +++++++++++++++------
src/qemu/qemu_command.h | 4 +
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
.../qemuxml2argv-pcihole64-q35.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
.../qemuxml2argv-sound-device.args | 7 +-
.../qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 +
tests/qemuxml2argvtest.c | 3 +-
13 files changed, 175 insertions(+), 58 deletions(-)
--
1.8.3.1
7:30 a.m.
New subject: [libvirt] [PATCH 1/6] qemu: eliminate redundant if clauses in qemuCollectPCIAddress
Replace them with switch cases. This will make it more efficient when
we add exceptions more controller types, and other device types.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 57 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 25 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0376611..2683a9c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1731,37 +1731,44 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
/* Change flags according to differing requirements of different
* devices.
*/
- if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
- device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
- switch (device->data.controller->model) {
- case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
- /* pci-bridge needs a PCI slot, but it isn't
- * hot-pluggable, so it doesn't need a hot-pluggable slot.
- */
- flags = QEMU_PCI_CONNECT_TYPE_PCI;
+ switch (device->type) {
+ case VIR_DOMAIN_DEVICE_CONTROLLER:
+ switch (device->data.controller->type) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ switch (device->data.controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ /* pci-bridge needs a PCI slot, but it isn't
+ * hot-pluggable, so it doesn't need a hot-pluggable slot.
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCI;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ /* pci-bridge needs a PCIe slot, but it isn't
+ * hot-pluggable, so it doesn't need a hot-pluggable slot.
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
+ default:
+ break;
+ }
break;
- case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
- /* pci-bridge needs a PCIe slot, but it isn't
- * hot-pluggable, so it doesn't need a hot-pluggable slot.
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+ /* SATA controllers aren't hot-plugged, and can be put in
+ * either a PCI or PCIe slot
*/
- flags = QEMU_PCI_CONNECT_TYPE_PCIE;
- break;
- default:
+ flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
break;
}
- }
- /* SATA controllers aren't hot-plugged, and can be put in either a
- * PCI or PCIe slot
- */
- if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER &&
- device->data.controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA)
- flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
- /* video cards aren't hot-plugged, and can be put in either a PCI
- * or PCIe slot
- */
- if (device->type == VIR_DOMAIN_DEVICE_VIDEO)
+ case VIR_DOMAIN_DEVICE_VIDEO:
+ /* video cards aren't hot-plugged, and can be put in either a
+ * PCI or PCIe slot
+ */
flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
+ }
/* Ignore implicit controllers on slot 0:0:1.0:
* implicit IDE controller on 0:0:1.1 (no qemu command line)
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 1/6] qemu: eliminate redundant if clauses in qemuCollectPCIAddress
On 09/25/2013 02:30 PM, Laine Stump wrote:
Replace them with switch cases. This will make it more efficient
when
we add exceptions more controller types, and other device types.
s/more/for more/
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 57 +++++++++++++++++++++++++++----------------------
1 file changed, 32 insertions(+), 25 deletions(-)
ACK
Jan
7:30 a.m.
New subject: [libvirt] [PATCH 2/6] qemu: allow some PCI devices to be attached to PCIe slots
Part of the resolution to:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Although most devices available in qemu area defined as PCI devices,
and strictly speaking should only be attached via a PCI slot, in
practice qemu allows them to be attached to a PCIe slot and sometimes
this makes sense.
For example, The UHCI and EHCI USB controllers are usually attached
directly to the PCIe "root complex" (i.e. PCIe slots) on real
hardware, so that should be possible for a Q35-based qemu virtual
machine as well.
We still want to prefer a standard PCi slot when auto-assigning
addresses, though, and in general to disallow attaching PCI devices
via PCIe slots.
This patch makes that possible by adding a new
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done
with this flag:
1) It is set for the "pcie-root" controller
2) qemuCollectPCIAddress() now has a set of nested switches that set
this "EITHER" flag for devices that we want to allow connecting to
pcie-root when specifically requested in the config.
3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the
"flagsMatchMask" if the address being checked came from config rather
than being newly auto-allocated by libvirt (this knowledge is
conveniently already available in the "fromConfig" arg).
Now any device having the EITHER flag set can be connected to
pcie-root if explicitly requested, but auto-allocated addresses for
those devices will still be standard PCI slots instead.
This patch only loosens the restrictions on devices that have been
specifically requested, but the setup is such that it should be fairly
easy to add new devices.
---
src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_command.h | 4 ++++
2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2683a9c..280d8d2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1496,12 +1496,16 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
{
virErrorNumber errType = (fromConfig
? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
+ qemuDomainPCIConnectFlags flagsMatchMask = QEMU_PCI_CONNECT_TYPES_MASK;
+
+ if (fromConfig)
+ flagsMatchMask |= QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG;
/* 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 & QEMU_PCI_CONNECT_TYPES_MASK)) {
+ if (!(devFlags & busFlags & flagsMatchMask)) {
if (reportError) {
if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) {
virReportError(errType,
@@ -1620,8 +1624,12 @@ qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
- /* slots 1 - 31, PCIe devices only, no hotplug */
- bus->flags = QEMU_PCI_CONNECT_TYPE_PCIE;
+ /* slots 1 - 31, no hotplug, PCIe only unless the address was
+ * specified in user config *and* the particular device being
+ * attached also allows it
+ */
+ bus->flags = (QEMU_PCI_CONNECT_TYPE_PCIE |
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
bus->minSlot = 1;
bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
@@ -1759,6 +1767,42 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_USB:
+ /* allow UHCI and EHCI controllers to be manually placed on
+ * the PCIe bus (but don't put them there automatically)
+ */
+ switch (device->data.controller->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_EHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_EHCI1:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_ICH9_UHCI1:
+ 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 = (QEMU_PCI_CONNECT_TYPE_PCI |
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_NEC_XHCI:
+ /* should this be PCIE-only? Or do we need to allow PCI
+ * for backward compatibility?
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PCI_OHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI:
+ case VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX4_UHCI:
+ /* Allow these for PCI only */
+ break;
+ }
+ }
+ break;
+
+ case VIR_DOMAIN_DEVICE_SOUND:
+ switch (device->data.sound->model) {
+ case VIR_DOMAIN_SOUND_MODEL_ICH6:
+ flags = (QEMU_PCI_CONNECT_TYPE_PCI |
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
+ break;
}
break;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 0e16a3d..2e2acfb 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -239,6 +239,10 @@ typedef enum {
/* PCI devices can connect to this bus */
QEMU_PCI_CONNECT_TYPE_PCIE = 1 << 3,
/* PCI Express devices can connect to this bus */
+ QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG = 1 << 4,
+ /* PCI *and* PCIe devices allowed, if the address
+ * was specified in the config by the user
+ */
} qemuDomainPCIConnectFlags;
/* a combination of all bit that describe the type of connections
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 2/6] qemu: allow some PCI devices to be attached to PCIe slots
On 09/25/2013 02:30 PM, Laine Stump wrote:
Part of the resolution to:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Although most devices available in qemu area defined as PCI devices,
and strictly speaking should only be attached via a PCI slot, in
practice qemu allows them to be attached to a PCIe slot and sometimes
this makes sense.
For example, The UHCI and EHCI USB controllers are usually attached
directly to the PCIe "root complex" (i.e. PCIe slots) on real
hardware, so that should be possible for a Q35-based qemu virtual
machine as well.
We still want to prefer a standard PCi slot when auto-assigning
*PCI
addresses, though, and in general to disallow attaching PCI devices
via PCIe slots.
This patch makes that possible by adding a new
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG flag. Three things are done
with this flag:
1) It is set for the "pcie-root" controller
2) qemuCollectPCIAddress() now has a set of nested switches that set
this "EITHER" flag for devices that we want to allow connecting to
pcie-root when specifically requested in the config.
3) qemuDomainPCIAddressFlagsCompatible() adds this new flag to the
"flagsMatchMask" if the address being checked came from config rather
than being newly auto-allocated by libvirt (this knowledge is
conveniently already available in the "fromConfig" arg).
Now any device having the EITHER flag set can be connected to
pcie-root if explicitly requested, but auto-allocated addresses for
those devices will still be standard PCI slots instead.
This patch only loosens the restrictions on devices that have been
specifically requested, but the setup is such that it should be fairly
easy to add new devices.
---
src/qemu/qemu_command.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++---
src/qemu/qemu_command.h | 4 ++++
2 files changed, 51 insertions(+), 3 deletions(-)
The code looks OK, and it seems to work with my limited testing.
ACK, but I don't know the answer to that USB_NEC_XHCI question.
Jan
7:30 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: replace multiple strcmps with a switch on an enum
I'm not sure why this code was written to compare the strings that it
had just retrieved from an enum->string conversion, rather than just
look at the original enum values, but this yields the same results,
and is much more efficient (especially as you add more devices).
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 280d8d2..3156cff 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5269,13 +5269,18 @@ qemuBuildSoundDevStr(virDomainDefPtr def,
goto error;
}
- /* Hack for weirdly unusual devices name in QEMU */
- if (STREQ(model, "es1370"))
+ /* Hack for devices with different names in QEMU and libvirt */
+ switch (sound->model) {
+ case VIR_DOMAIN_SOUND_MODEL_ES1370:
model = "ES1370";
- else if (STREQ(model, "ac97"))
+ break;
+ case VIR_DOMAIN_SOUND_MODEL_AC97:
model = "AC97";
- else if (STREQ(model, "ich6"))
+ break;
+ case VIR_DOMAIN_SOUND_MODEL_ICH6:
model = "intel-hda";
+ break;
+ }
virBufferAsprintf(&buf, "%s,id=%s", model, sound->info.alias);
if (qemuBuildDeviceAddressStr(&buf, def, &sound->info, qemuCaps) < 0)
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 3/6] qemu: replace multiple strcmps with a switch on an enum
On 09/25/2013 02:30 PM, Laine Stump wrote:
I'm not sure why this code was written to compare the strings
that it
had just retrieved from an enum->string conversion, rather than just
look at the original enum values, but this yields the same results,
and is much more efficient (especially as you add more devices).
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
ACK
Jan
7:30 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: support ich9-intel-hda audio device
This resolves one of the issues in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
This device is identical to qemu's "intel-hda" device (known as
"ich6"
in libvirt), but has a different PCI device ID (which matches the ID
of the hda audio built into the ich9 chipset, of course). It's not
supported int earlier versions of qemu, so it requires a capability
bit.
---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 6 ++++--
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 16 ++++++++++++++--
tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 ++++++-
tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 +++++
tests/qemuxml2argvtest.c | 3 ++-
9 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index fb6a26c..5c5301d 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2947,6 +2947,7 @@
<value>pcspk</value>
<value>ac97</value>
<value>ich6</value>
+ <value>ich9</value>
</choice>
</attribute>
<interleave>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 240f318..4110127 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -462,7 +462,8 @@ VIR_ENUM_IMPL(virDomainSoundModel, VIR_DOMAIN_SOUND_MODEL_LAST,
"es1370",
"pcspk",
"ac97",
- "ich6")
+ "ich6",
+ "ich9")
VIR_ENUM_IMPL(virDomainMemDump, VIR_DOMAIN_MEM_DUMP_LAST,
"default",
@@ -8451,7 +8452,8 @@ virDomainSoundDefParseXML(const xmlNodePtr node,
goto error;
}
- if (def->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+ if (def->model == VIR_DOMAIN_SOUND_MODEL_ICH6 ||
+ def->model == VIR_DOMAIN_SOUND_MODEL_ICH9) {
int ncodecs;
xmlNodePtr *codecNodes = NULL;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 5c33e08..f20a916 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1253,6 +1253,7 @@ enum virDomainSoundModel {
VIR_DOMAIN_SOUND_MODEL_PCSPK,
VIR_DOMAIN_SOUND_MODEL_AC97,
VIR_DOMAIN_SOUND_MODEL_ICH6,
+ VIR_DOMAIN_SOUND_MODEL_ICH9,
VIR_DOMAIN_SOUND_MODEL_LAST
};
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d830e2a..dc8f0be 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -241,6 +241,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
"usb-storage", /* 155 */
"usb-storage.removable",
"virtio-mmio",
+ "ich9-intel-hda",
);
struct _virQEMUCaps {
@@ -1391,6 +1392,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "i82801b11-bridge", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
{ "usb-storage", QEMU_CAPS_DEVICE_USB_STORAGE },
{ "virtio-mmio", QEMU_CAPS_DEVICE_VIRTIO_MMIO },
+ { "ich9-intel-hda", QEMU_CAPS_DEVICE_ICH9_INTEL_HDA },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f3c8fa8..f8dc728 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -196,6 +196,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_USB_STORAGE = 155, /* -device usb-storage */
QEMU_CAPS_USB_STORAGE_REMOVABLE = 156, /* usb-storage.removable */
QEMU_CAPS_DEVICE_VIRTIO_MMIO = 157, /* -device virtio-mmio */
+ QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3156cff..c8a5c8b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1800,6 +1800,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
case VIR_DOMAIN_DEVICE_SOUND:
switch (device->data.sound->model) {
case VIR_DOMAIN_SOUND_MODEL_ICH6:
+ case VIR_DOMAIN_SOUND_MODEL_ICH9:
flags = (QEMU_PCI_CONNECT_TYPE_PCI |
QEMU_PCI_CONNECT_TYPE_EITHER_IF_CONFIG);
break;
@@ -5280,6 +5281,15 @@ qemuBuildSoundDevStr(virDomainDefPtr def,
case VIR_DOMAIN_SOUND_MODEL_ICH6:
model = "intel-hda";
break;
+ case VIR_DOMAIN_SOUND_MODEL_ICH9:
+ model = "ich9-intel-hda";
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ICH9_INTEL_HDA)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The ich9-intel-hda audio controller "
+ "is not supported in this QEMU binary"));
+ goto error;
+ }
+ break;
}
virBufferAsprintf(&buf, "%s,id=%s", model, sound->info.alias);
@@ -9065,7 +9075,8 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, str);
- if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+ if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 ||
+ sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) {
char *codecstr = NULL;
for (j = 0; j < sound->ncodecs; j++) {
@@ -9111,7 +9122,8 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
- if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6) {
+ if (sound->model == VIR_DOMAIN_SOUND_MODEL_ICH6 ||
+ sound->model == VIR_DOMAIN_SOUND_MODEL_ICH9) {
VIR_FREE(modstr);
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("this QEMU binary lacks hda support"));
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
index d358453..1511389 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.args
@@ -9,4 +9,9 @@ id=sound4-codec0,bus=sound4.0,cad=0 \
-device intel-hda,id=sound5,bus=pci.0,addr=0x6 \
-device hda-micro,id=sound5-codec0,bus=sound5.0,cad=0 \
-device hda-duplex,id=sound5-codec1,bus=sound5.0,cad=1 \
--device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x7
+-device ich9-intel-hda,id=sound6,bus=pci.0,addr=0x7 -device hda-duplex,\
+id=sound6-codec0,bus=sound6.0,cad=0 \
+-device ich9-intel-hda,id=sound7,bus=pci.0,addr=0x8 \
+-device hda-micro,id=sound7-codec0,bus=sound7.0,cad=0 \
+-device hda-duplex,id=sound7-codec1,bus=sound7.0,cad=1 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x9
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
index 7bf9ff9..8ce718e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml
@@ -31,6 +31,11 @@
<codec type='micro'/>
<codec type='duplex'/>
</sound>
+ <sound model='ich9'/>
+ <sound model='ich9'>
+ <codec type='micro'/>
+ <codec type='duplex'/>
+ </sound>
<memballoon model='virtio'/>
</devices>
</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ec4a6e5..0b808a4 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -855,7 +855,8 @@ mymain(void)
DO_TEST("sound", NONE);
DO_TEST("sound-device",
QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG,
- QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_HDA_MICRO);
+ QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_HDA_MICRO,
+ QEMU_CAPS_DEVICE_ICH9_INTEL_HDA);
DO_TEST("fs9p",
QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_FSDEV,
QEMU_CAPS_FSDEV_WRITEOUT);
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 4/6] qemu: support ich9-intel-hda audio device
On 09/25/2013 02:30 PM, Laine Stump wrote:
This resolves one of the issues in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
This device is identical to qemu's "intel-hda" device (known as
"ich6"
in libvirt), but has a different PCI device ID (which matches the ID
of the hda audio built into the ich9 chipset, of course). It's not
supported int earlier versions of qemu, so it requires a capability
*in
bit.
---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 6 ++++--
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 16 ++++++++++++++--
tests/qemuxml2argvdata/qemuxml2argv-sound-device.args | 7 ++++++-
tests/qemuxml2argvdata/qemuxml2argv-sound-device.xml | 5 +++++
tests/qemuxml2argvtest.c | 3 ++-
9 files changed, 36 insertions(+), 6 deletions(-)
ACK
Jan
7:30 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
This will make it simpler to add checks for other types of
controllers.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c8a5c8b..9baed56 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2534,27 +2534,32 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
char *addrStr = NULL;
qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCIE;
- /* Verify that the first SATA controller is at 00:1F.2 */
- /* the q35 machine type *always* has a SATA controller at this address */
for (i = 0; i < def->ncontrollers; i++) {
- if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_SATA
&&
- def->controllers[i]->idx == 0) {
- if (def->controllers[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
- if (def->controllers[i]->info.addr.pci.domain != 0 ||
- def->controllers[i]->info.addr.pci.bus != 0 ||
- def->controllers[i]->info.addr.pci.slot != 0x1F ||
- def->controllers[i]->info.addr.pci.function != 2) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Primary SATA controller must have PCI address
0:0:1f.2"));
- goto cleanup;
+ switch (def->controllers[i]->type) {
+ case VIR_DOMAIN_CONTROLLER_TYPE_SATA:
+ /* Verify that the first SATA controller is at 00:1F.2 the
+ * q35 machine type *always* has a SATA controller at this
+ * address.
+ */
+ if (def->controllers[i]->idx == 0) {
+ if (def->controllers[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ if (def->controllers[i]->info.addr.pci.domain != 0 ||
+ def->controllers[i]->info.addr.pci.bus != 0 ||
+ def->controllers[i]->info.addr.pci.slot != 0x1F ||
+ def->controllers[i]->info.addr.pci.function != 2) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Primary SATA controller must have PCI
address 0:0:1f.2"));
+ goto cleanup;
+ }
+ } else {
+ def->controllers[i]->info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ def->controllers[i]->info.addr.pci.domain = 0;
+ def->controllers[i]->info.addr.pci.bus = 0;
+ def->controllers[i]->info.addr.pci.slot = 0x1F;
+ def->controllers[i]->info.addr.pci.function = 2;
}
- } else {
- def->controllers[i]->info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
- def->controllers[i]->info.addr.pci.domain = 0;
- def->controllers[i]->info.addr.pci.bus = 0;
- def->controllers[i]->info.addr.pci.slot = 0x1F;
- def->controllers[i]->info.addr.pci.function = 2;
}
+ break;
}
}
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
On 09/25/2013 02:30 PM, Laine Stump wrote:
This will make it simpler to add checks for other types of
controllers.
This is a prerequisite for patches to resolve:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
---
src/qemu/qemu_command.c | 41 +++++++++++++++++++++++------------------
1 file changed, 23 insertions(+), 18 deletions(-)
ACK
Jan
7:30 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
This resolves one of the issues listed in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
00:1E.0 is the location of this controller on at least some actual Q35
hardware, so we try to replicate the placement. The bridge should work
just as well in any other location though, so if 00:1E.0 isn't
available, just allow it to be auto-assigned anywhere appropriate.
---
src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
.../qemuxml2argv-pcihole64-q35.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
4 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9baed56..38dd451 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -2560,6 +2560,28 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
}
}
break;
+
+ case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
+ if (def->controllers[i]->model ==
VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE &&
+ def->controllers[i]->info.type ==
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+ /* Try to assign this bridge to 00:1E.0 (because that
+ * is its standard location on real hardware) unless
+ * it's already taken, but don't insist on it.
+ */
+ memset(&tmp_addr, 0, sizeof(tmp_addr));
+ tmp_addr.slot = 0x1E;
+ if (!qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
+ if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr,
+ flags, true, false) < 0)
+ goto cleanup;
+ def->controllers[i]->info.type =
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ def->controllers[i]->info.addr.pci.domain = 0;
+ def->controllers[i]->info.addr.pci.bus = 0;
+ def->controllers[i]->info.addr.pci.slot = 0x1E;
+ def->controllers[i]->info.addr.pci.function = 0;
+ }
+ }
+ break;
}
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index 010e089..48c21cd 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -2,5 +2,5 @@ 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=0x2 \
+-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
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
index 175c0dc..6855cd2 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcihole64-q35.args
@@ -2,7 +2,7 @@ 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 -global q35-pcihost.pci-hole64-size=1048576K \
--device i82801b11-bridge,id=pci.1,bus=pcie.0,addr=0x2 \
+-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 \
-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 \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index 6bd5f4c..8cc5874 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -1,7 +1,7 @@
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=0x2 \
+-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 \
-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 \
--
1.8.3.1
9:07 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
On 09/25/2013 02:30 PM, Laine Stump wrote:
This resolves one of the issues listed in:
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
00:1E.0 is the location of this controller on at least some actual Q35
hardware, so we try to replicate the placement. The bridge should work
just as well in any other location though, so if 00:1E.0 isn't
available, just allow it to be auto-assigned anywhere appropriate.
---
src/qemu/qemu_command.c | 22 ++++++++++++++++++++++
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
.../qemuxml2argv-pcihole64-q35.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
4 files changed, 25 insertions(+), 3 deletions(-)
ACK
Jan
9:41 a.m.
On 09/25/2013 08:30 AM, Laine Stump wrote:
These patches are all in response to the issues that Gerd Hoffman
raised in
https://bugzilla.redhat.com/show_bug.cgi?id=1003983
Three of the six are simple refactoring of existing code, done
separately so that the patches that change functionality would be
simpler to review. None of them is very long.
Laine Stump (6):
qemu: eliminate redundant if clauses in qemuCollectPCIAddress
qemu: allow some PCI devices to be attached to PCIe slots
qemu: replace multiple strcmps with a switch on an enum
qemu: support ich9-intel-hda audio device
qemu: turn if into switch in qemuDomainValidateDevicePCISlotsQ35
qemu: prefer to put a Q35 machine's dmi-to-pci-bridge at 00:1E.0
Thanks for the reviews. I've pushed the series.
4078
days inactive
4078
days old
13 comments
2 participants
participants (2)
-
Ján Tomko -
Laine Stump