[libvirt] [PATCH 0/7] Q35 support part 2

[Laine Stump]

This series adds everything needed to create and start a Q35 machine
from libvirt, with a couple of small exceptions:

1) Although the implicit pcie-root controller on q35 machinetypes is
referred to as "pcie.0" by qemu, the code as it stands now still puts
"pci.0" on the commandline, which means it's still unusable. I'm
working on a patch to fix that right now.

2) The q35 machinetype doesn't create a default usb controller as the
other machinetypes do. The reason for that is explained in Patch
7/7. I think we probably *should* have a default usb controller added
for q35 (just to make creating a new machine config easier), but it
probably should be a usb2 "trifecta" of controllers, rather than an
old-fashioned usb1 piix3 controller.

With (1) fixed, this should be enough functionality that people can
start testing q35 guests via libvirt.

Laine Stump (7):
conf: add default USB controller in qemu post-parse callback
qemu: rename some functions in qemu_command.c
qemu: eliminate almost-duplicate code in qemu_command.c
qemu: enable auto-allocate of all PCI addresses
qemu: add pcie-root controller
qemu: add dmi-to-pci-bridge controller
qemu: fix handling of default/implicit devices for q35

docs/formatdomain.html.in | 43 +-
docs/schemas/domaincommon.rng | 2 +
src/conf/domain_conf.c | 17 +-
src/conf/domain_conf.h | 2 +
src/qemu/qemu_capabilities.c | 2 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 619 +++++++++++++++------
src/qemu/qemu_command.h | 14 +-
src/qemu/qemu_domain.c | 48 +-
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 5 +
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml | 20 +
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 6 +
tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 25 +
tests/qemuxml2argvtest.c | 10 +
.../qemuxml2xmlout-pcie-root.xml | 23 +
tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 26 +
tests/qemuxml2xmltest.c | 2 +
17 files changed, 673 insertions(+), 192 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml

[Laine Stump]

The parser shouldn't be doing arch-specific things like adding in
implicit controllers to the config. This should instead be done in the
hypervisor's post-parse callback.

This patch removes the auto-add of a usb controller from the domain
parser, and puts it into the qemu driver's post-parse callback (just
as is already done with the auto-add of the pci-root controller). In
the future, any machine/arch that shouldn't have a default usb
controller added should just set addDefaultUSB = false in this
function.

We've recently seen that q35 and ARMV7L domains shouldn't get a default USB
controller, so I've set addDefaultUSB to false for both of those.
---
src/conf/domain_conf.c | 6 ------
src/qemu/qemu_domain.c | 14 +++++++++++++-
2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e3aec69..63350b6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11718,12 +11718,6 @@ virDomainDefParseXML(xmlDocPtr xml,
goto error;
}

- if (def->virtType == VIR_DOMAIN_VIRT_QEMU ||
- def->virtType == VIR_DOMAIN_VIRT_KQEMU ||
- def->virtType == VIR_DOMAIN_VIRT_KVM)
- if (virDomainDefMaybeAddController(def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0)
- goto error;
-
/* analysis of the resource leases */
if ((n = virXPathNodeSet("./devices/lease", ctxt, &nodes)) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 1ff802c..d3da666 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -699,6 +699,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
virCapsPtr caps,
void *opaque ATTRIBUTE_UNUSED)
{
+ bool addDefaultUSB = true;
bool addPCIRoot = false;

/* check for emulator and create a default one if needed */
@@ -714,8 +715,10 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
if (STRPREFIX(def->os.machine, "pc-q35") ||
STREQ(def->os.machine, "q35") ||
- STREQ(def->os.machine, "isapc"))
+ STREQ(def->os.machine, "isapc")) {
+ addDefaultUSB = false;
break;
+ }
if (!STRPREFIX(def->os.machine, "pc-0.") &&
!STRPREFIX(def->os.machine, "pc-1.") &&
!STRPREFIX(def->os.machine, "pc-i440") &&
@@ -725,6 +728,10 @@ qemuDomainDefPostParse(virDomainDefPtr def,
addPCIRoot = true;
break;

+ case VIR_ARCH_ARMV7L:
+ addDefaultUSB = false;
+ break;
+
case VIR_ARCH_ALPHA:
case VIR_ARCH_PPC:
case VIR_ARCH_PPC64:
@@ -737,6 +744,11 @@ qemuDomainDefPostParse(virDomainDefPtr def,
break;
}

+ if (addDefaultUSB &&
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0)
+ return -1;
+
if (addPCIRoot &&
virDomainDefMaybeAddController(
def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,

[Eric Blake]

Post by Laine Stump
The parser shouldn't be doing arch-specific things like adding in
implicit controllers to the config. This should instead be done in the
hypervisor's post-parse callback.
This patch removes the auto-add of a usb controller from the domain
parser, and puts it into the qemu driver's post-parse callback (just
as is already done with the auto-add of the pci-root controller). In
the future, any machine/arch that shouldn't have a default usb
controller added should just set addDefaultUSB = false in this
function.
We've recently seen that q35 and ARMV7L domains shouldn't get a default USB
controller, so I've set addDefaultUSB to false for both of those.
---
src/conf/domain_conf.c | 6 ------
src/qemu/qemu_domain.c | 14 +++++++++++++-
2 files changed, 13 insertions(+), 7 deletions(-)

ACK.

[Laine Stump]

Post by Laine Stump
The parser shouldn't be doing arch-specific things like adding in
implicit controllers to the config. This should instead be done in the
hypervisor's post-parse callback.
This patch removes the auto-add of a usb controller from the domain
parser, and puts it into the qemu driver's post-parse callback (just
as is already done with the auto-add of the pci-root controller). In
the future, any machine/arch that shouldn't have a default usb
controller added should just set addDefaultUSB = false in this
function.
We've recently seen that q35 and ARMV7L domains shouldn't get a default USB
controller, so I've set addDefaultUSB to false for both of those.
---
src/conf/domain_conf.c | 6 ------
src/qemu/qemu_domain.c | 14 +++++++++++++-
2 files changed, 13 insertions(+), 7 deletions(-)

ACK.

Thanks. I pushed this one (in part because Cole needs it for his ARM work).

[Laine Stump]

* qemuDomainPCIAddressSetNextAddr

The name of this function was confusing because 1) other functions in
the file that end in "Addr" are only operating on a single function of
one PCI slot, not the entire slot, while functions that do something
with the entire slot end in "Slot", and 2) it didn't contain a verb
describing what it is doing (the "Set" refers to the set that contains
all PCI buses in the system, used to keep track of which slots in
which buses are already reserved for use).

It is now renamed to qemuDomainPCIAddressReserveNextSlot, which more
clearly describes what it is doing. Arguably, it could have been
changed to qemuDomainPCIAddressSetReserveNextSlot, but 1) the word
"set" is confusing in this context because it could be intended as a
verb or as a noun, and 2) most other functions that operate on a
single slot or address within this set are also named
qemuDomainPCIAddress... rather than qemuDomainPCIAddressSet... Only
the Create, Free, and Grow functions for an address set (which modify the
entire set, not just one element) use "Set" in their name.

* qemuPCIAddressAsString, qemuPCIAddressValidate

All the other functions in this set are named
qemuDomainPCIAddressxxxxx, so I renamed these to be consistent.
---
src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++-------------------
src/qemu/qemu_command.h | 6 ++--
2 files changed, 51 insertions(+), 33 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index aa3a2fd..4345456 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1439,9 +1439,9 @@ struct _qemuDomainPCIAddressSet {
* with the specified PCI address set.
*/
static bool
-qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED,
- virDevicePCIAddressPtr addr,
- qemuDomainPCIConnectFlags flags)
+qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
+ virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags flags)
{
qemuDomainPCIAddressBusPtr bus;

@@ -1578,7 +1578,7 @@ qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs,


static char *
-qemuPCIAddressAsString(virDevicePCIAddressPtr addr)
+qemuDomainPCIAddressAsString(virDevicePCIAddressPtr addr)
{
char *str;

@@ -1648,10 +1648,10 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
* that the bus is of the correct type for the device (via
* comparing the flags).
*/
- if (!qemuPCIAddressValidate(addrs, addr, flags))
+ if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
return -1;

- if (!(str = qemuPCIAddressAsString(addr)))
+ if (!(str = qemuDomainPCIAddressAsString(addr)))
goto cleanup;

/* check if already in use */
@@ -1729,7 +1729,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0)
goto cleanup;
/* Reserve 1 extra slot for a (potential) bridge */
- if (qemuDomainPCIAddressSetNextAddr(addrs, &info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0)
goto cleanup;

for (i = 1; i < addrs->nbuses; i++) {
@@ -1740,7 +1740,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
i, bus->model)) < 0)
goto cleanup;
/* If we added a new bridge, we will need one more address */
- if (rv > 0 && qemuDomainPCIAddressSetNextAddr(addrs, &info, flags) < 0)
+ if (rv > 0 && qemuDomainPCIAddressReserveNextSlot(addrs, &info,
+ flags) < 0)
goto cleanup;
}
nbuses = addrs->nbuses;
@@ -1881,7 +1882,7 @@ qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
return -1;

- if (!(str = qemuPCIAddressAsString(addr)))
+ if (!(str = qemuDomainPCIAddressAsString(addr)))
return -1;

VIR_DEBUG("Reserving PCI addr %s", str);
@@ -1923,7 +1924,7 @@ qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
return -1;

- if (!(str = qemuPCIAddressAsString(addr)))
+ if (!(str = qemuDomainPCIAddressAsString(addr)))
return -1;

VIR_DEBUG("Reserving PCI slot %s", str);
@@ -1959,12 +1960,12 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
return -1;
}

- if (!qemuPCIAddressValidate(addrs, &dev->addr.pci, flags))
+ if (!qemuDomainPCIAddressValidate(addrs, &dev->addr.pci, flags))
return -1;

ret = qemuDomainPCIAddressReserveSlot(addrs, &dev->addr.pci, flags);
} else {
- ret = qemuDomainPCIAddressSetNextAddr(addrs, dev, flags);
+ ret = qemuDomainPCIAddressReserveNextSlot(addrs, dev, flags);
}
return ret;
}
@@ -1986,7 +1987,7 @@ qemuDomainPCIAddressReleaseSlot(qemuDomainPCIAddressSetPtr addrs,
*/
qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPES_MASK;

- if (!qemuPCIAddressValidate(addrs, addr, flags))
+ if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
return -1;

addrs->buses[addr->bus].slots[addr->slot] = 0;
@@ -2059,9 +2060,9 @@ success:
}

int
-qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
- virDomainDeviceInfoPtr dev,
- qemuDomainPCIConnectFlags flags)
+qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs,
+ virDomainDeviceInfoPtr dev,
+ qemuDomainPCIConnectFlags flags)
{
virDevicePCIAddress addr;
if (qemuDomainPCIAddressGetNextSlot(addrs, &addr, flags) < 0)
@@ -2188,14 +2189,16 @@ qemuValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
primaryVideo->info.addr.pci.function = 0;
addrptr = &primaryVideo->info.addr.pci;

- if (!qemuPCIAddressValidate(addrs, addrptr, flags))
+ if (!qemuDomainPCIAddressValidate(addrs, addrptr, flags))
goto error;

if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
if (qemuDeviceVideoUsable) {
virResetLastError();
- if (qemuDomainPCIAddressSetNextAddr(addrs, &primaryVideo->info, flags) < 0)
- goto error;;
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &primaryVideo->info,
+ flags) < 0)
+ goto error;
} else {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("PCI address 0:0:2.0 is in use, "
@@ -2296,7 +2299,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
continue;
if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &def->controllers[i]->info,
+ flags) < 0)
goto error;
}
}
@@ -2307,7 +2312,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,

/* Only support VirtIO-9p-pci so far. If that changes,
* we might need to skip devices here */
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->fss[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->fss[i]->info,
+ flags) < 0)
goto error;
}

@@ -2321,7 +2327,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
(def->nets[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)) {
continue;
}
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->nets[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->nets[i]->info,
+ flags) < 0)
goto error;
}

@@ -2334,7 +2341,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
def->sounds[i]->model == VIR_DOMAIN_SOUND_MODEL_PCSPK)
continue;

- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->sounds[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->sounds[i]->info,
+ flags) < 0)
goto error;
}

@@ -2409,7 +2417,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
def->controllers[i]->info.addr.pci = addr;
} else {
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->controllers[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &def->controllers[i]->info,
+ flags) < 0)
goto error;
}
}
@@ -2434,7 +2444,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
goto error;
}

- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->disks[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->disks[i]->info,
+ flags) < 0)
goto error;
}

@@ -2446,7 +2457,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
def->hostdevs[i]->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI)
continue;

- if (qemuDomainPCIAddressSetNextAddr(addrs, def->hostdevs[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ def->hostdevs[i]->info,
+ flags) < 0)
goto error;
}

@@ -2454,7 +2467,9 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->memballoon &&
def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO &&
def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->memballoon->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &def->memballoon->info,
+ flags) < 0)
goto error;
}

@@ -2462,7 +2477,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->rng &&
def->rng->model == VIR_DOMAIN_RNG_MODEL_VIRTIO &&
def->rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->rng->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &def->rng->info, flags) < 0)
goto error;
}

@@ -2470,7 +2486,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
if (def->watchdog &&
def->watchdog->model != VIR_DOMAIN_WATCHDOG_MODEL_IB700 &&
def->watchdog->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->watchdog->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->watchdog->info,
+ flags) < 0)
goto error;
}

@@ -2483,7 +2500,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
}
if (def->videos[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
- if (qemuDomainPCIAddressSetNextAddr(addrs, &def->videos[i]->info, flags) < 0)
+ if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info,
+ flags) < 0)
goto error;
}
for (i = 0; i < def->ninputs; i++) {
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index 2b02d6e..c9f1600 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -254,9 +254,9 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
qemuDomainPCIConnectFlags flags);
-int qemuDomainPCIAddressSetNextAddr(qemuDomainPCIAddressSetPtr addrs,
- virDomainDeviceInfoPtr dev,
- qemuDomainPCIConnectFlags flags);
+int qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs,
+ virDomainDeviceInfoPtr dev,
+ qemuDomainPCIConnectFlags flags);
int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev);
int qemuDomainPCIAddressReleaseAddr(qemuDomainPCIAddressSetPtr addrs,

[Eric Blake]

Post by Laine Stump
* qemuDomainPCIAddressSetNextAddr
The name of this function was confusing because 1) other functions in
the file that end in "Addr" are only operating on a single function of
one PCI slot, not the entire slot, while functions that do something
with the entire slot end in "Slot", and 2) it didn't contain a verb
describing what it is doing (the "Set" refers to the set that contains
all PCI buses in the system, used to keep track of which slots in
which buses are already reserved for use).
It is now renamed to qemuDomainPCIAddressReserveNextSlot, which more
clearly describes what it is doing. Arguably, it could have been
changed to qemuDomainPCIAddressSetReserveNextSlot, but 1) the word
"set" is confusing in this context because it could be intended as a
verb or as a noun, and 2) most other functions that operate on a
single slot or address within this set are also named
qemuDomainPCIAddress... rather than qemuDomainPCIAddressSet... Only
the Create, Free, and Grow functions for an address set (which modify the
entire set, not just one element) use "Set" in their name.
* qemuPCIAddressAsString, qemuPCIAddressValidate
All the other functions in this set are named
qemuDomainPCIAddressxxxxx, so I renamed these to be consistent.
---
src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++-------------------
src/qemu/qemu_command.h | 6 ++--
2 files changed, 51 insertions(+), 33 deletions(-)

Not a 1:1 line mapping; but I checked that it's all because of line
re-wrapping now that the names are longer.

ACK.

[Laine Stump]

Post by Eric Blake

Post by Laine Stump
* qemuDomainPCIAddressSetNextAddr
The name of this function was confusing because 1) other functions in
the file that end in "Addr" are only operating on a single function of
one PCI slot, not the entire slot, while functions that do something
with the entire slot end in "Slot", and 2) it didn't contain a verb
describing what it is doing (the "Set" refers to the set that contains
all PCI buses in the system, used to keep track of which slots in
which buses are already reserved for use).
It is now renamed to qemuDomainPCIAddressReserveNextSlot, which more
clearly describes what it is doing. Arguably, it could have been
changed to qemuDomainPCIAddressSetReserveNextSlot, but 1) the word
"set" is confusing in this context because it could be intended as a
verb or as a noun, and 2) most other functions that operate on a
single slot or address within this set are also named
qemuDomainPCIAddress... rather than qemuDomainPCIAddressSet... Only
the Create, Free, and Grow functions for an address set (which modify the
entire set, not just one element) use "Set" in their name.
* qemuPCIAddressAsString, qemuPCIAddressValidate
All the other functions in this set are named
qemuDomainPCIAddressxxxxx, so I renamed these to be consistent.
---
src/qemu/qemu_command.c | 78 ++++++++++++++++++++++++++++++-------------------
src/qemu/qemu_command.h | 6 ++--
2 files changed, 51 insertions(+), 33 deletions(-)

Not a 1:1 line mapping; but I checked that it's all because of line
re-wrapping now that the names are longer.
ACK.

Thanks. This one is pushed too (and also eliminates the need for one of
Cole's ARM support patch series)

[Laine Stump]

* The functions qemuDomainPCIAddressReserveAddr and
qemuDomainPCIAddressReserveSlot were very similar (and should have
been more similar) and were about to get more code added to them which
would create even more duplicated code, so this patch gives
qemuDomainPCIAddressReserveAddr a "reserveEntireSlot" arg, then
replaces the body of qemuDomainPCIAddressReserveSlot with a call to
qemuDomainPCIAddressReserveAddr.

You will notice that addrs->lastaddr was previously set in
qemuDomainPCIAddressReserveAddr (but *not* set in
qemuDomainPCIAddressReserveSlot). For consistency and cleanliness of
code, that bit was removed and put into the one caller of
qemuDomainPCIAddressReserveAddr (there is a similar place where the
caller of qemuDomainPCIAddressReserveSlot sets lastaddr). This does
guarantee identical functionality to pre-patch code, but in practice
isn't really critical, because lastaddr is just keeping track of where
to start when looking for a free slot - if it isn't updated, we will
just start looking on a slot that's already occupied, then skip up to
one that isn't.

* qemuCollectPCIAddress was essentially doing the same thing as
qemuDomainPCIAddressReserveAddr, but with some extra special case
checking at the beginning. The duplicate code has been replaced with
a call to qemuDomainPCIAddressReserveAddr. This required adding a
"fromConfig" boolean, which is only used to change the log error
code from VIR_ERR_INTERNAL_ERROR (when the address was
auto-generated by libvirt) to VIR_ERR_XML_ERROR (when the address is
coming from the config); without this differentiation, it would be
difficult to tell if an error was caused by something wrong in
libvirt's auto-allocate code or just bad config.

* the bit of code in qemuDomainPCIAddressValidate that checks the
connect type flags is going to be used in a couple more places where
we don't need to also check the slot limits (because we're generating
the slot number ourselves), so that has been pulled out into a
separate qemuDomainPCIAddressFlagsCompatible function.
---
src/qemu/qemu_command.c | 232 ++++++++++++++++++++++++------------------------
src/qemu/qemu_command.h | 4 +-
2 files changed, 119 insertions(+), 117 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4345456..abc973a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1434,9 +1434,53 @@ struct _qemuDomainPCIAddressSet {
};


-/*
- * Check that the PCI address is valid for use
- * with the specified PCI address set.
+static bool
+qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags busFlags,
+ qemuDomainPCIConnectFlags devFlags,
+ bool reportError)
+{
+ /* 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 (reportError) {
+ if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires a standard PCI slot, "
+ "which is not provided by this bus"),
+ addr->domain, addr->bus);
+ } else {
+ /* this should never happen. If it does, there is a
+ * bug in the code that sets the flag bits for devices.
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("The device information has no PCI "
+ "connection types listed"));
+ }
+ }
+ return false;
+ }
+ if ((devFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
+ !(busFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
+ if (reportError) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires hot-plug capability, "
+ "which is not provided by the bus"),
+ addr->domain, addr->bus);
+ }
+ return false;
+ }
+ return true;
+}
+
+
+/* Verify that the address is in bounds for the chosen bus, and
+ * that the bus is of the correct type for the device (via
+ * comparing the flags).
*/
static bool
qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
@@ -1466,24 +1510,9 @@ qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
/* assure that at least one of the requested connection types is
* provided by this bus
*/
- if (!(flags & bus->flags & QEMU_PCI_CONNECT_TYPES_MASK)) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid PCI address: The PCI controller "
- "providing bus %04x doesn't support "
- "connections appropriate for the device "
- "(%x required vs. %x provided by bus)"),
- addr->bus, flags & QEMU_PCI_CONNECT_TYPES_MASK,
- bus->flags & QEMU_PCI_CONNECT_TYPES_MASK);
- return false;
- }
- /* make sure this bus allows hot-plug if the caller demands it */
- if ((flags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
- !(bus->flags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid PCI address: hot-pluggable slot requested, "
- "but bus %04x doesn't support hot-plug"), addr->bus);
+ if (!qemuDomainPCIAddressFlagsCompatible(addr, bus->flags, flags, true))
return false;
- }
+
/* some "buses" are really just a single port */
if (bus->minSlot && addr->slot < bus->minSlot) {
virReportError(VIR_ERR_XML_ERROR,
@@ -1597,10 +1626,10 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainDeviceInfoPtr info,
void *opaque)
{
+ qemuDomainPCIAddressSetPtr addrs = opaque;
int ret = -1;
- char *str = NULL;
virDevicePCIAddressPtr addr = &info->addr.pci;
- qemuDomainPCIAddressSetPtr addrs = opaque;
+ bool entireSlot;
/* FIXME: flags should be set according to the requirements of @device */
qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
QEMU_PCI_CONNECT_TYPE_PCI);
@@ -1639,56 +1668,15 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
return 0;
}

- /* add an additional bus of the correct type if needed */
- if (addrs->dryRun &&
- qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
-
- /* verify that the address is in bounds for the chosen bus, and
- * that the bus is of the correct type for the device (via
- * comparing the flags).
- */
- if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
- return -1;
-
- if (!(str = qemuDomainPCIAddressAsString(addr)))
- goto cleanup;
+ entireSlot = (addr->function == 0 &&
+ addr->multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON);

- /* check if already in use */
- if (addrs->buses[addr->bus].slots[addr->slot] & (1 << addr->function)) {
- if (info->addr.pci.function != 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address '%s' "
- "(may need \"multifunction='on'\" for device on function 0)"),
- str);
- } else {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address '%s'"), str);
- }
+ if (qemuDomainPCIAddressReserveAddr(addrs, addr, flags,
+ entireSlot, true) < 0)
goto cleanup;
- }

- /* mark as in use */
- if ((info->addr.pci.function == 0) &&
- (info->addr.pci.multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON)) {
- /* a function 0 w/o multifunction=on must reserve the entire slot */
- if (addrs->buses[addr->bus].slots[addr->slot]) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address on slot '%s' "
- "(need \"multifunction='off'\" for device "
- "on function 0)"),
- str);
- goto cleanup;
- }
- addrs->buses[addr->bus].slots[addr->slot] = 0xFF;
- VIR_DEBUG("Remembering PCI slot: %s (multifunction=off)", str);
- } else {
- VIR_DEBUG("Remembering PCI addr: %s", str);
- addrs->buses[addr->bus].slots[addr->slot] |= 1 << addr->function;
- }
ret = 0;
cleanup:
- VIR_FREE(str);
return ret;
}

@@ -1871,46 +1859,73 @@ static bool qemuDomainPCIAddressSlotInUse(qemuDomainPCIAddressSetPtr addrs,
}


+/*
+ * Reserve a slot (or just one function) for a device. If
+ * reserveEntireSlot is true, all functions for the slot are reserved,
+ * otherwise only one. If fromConfig is true, the address being
+ * requested came directly from the config and errors should be worded
+ * appropriately. If fromConfig is false, the address was
+ * automatically created by libvirt, so it is an internal error (not
+ * XML).
+ */
int
qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
- qemuDomainPCIConnectFlags flags)
+ qemuDomainPCIConnectFlags flags,
+ bool reserveEntireSlot,
+ bool fromConfig)
{
- char *str;
+ int ret = -1;
+ char *str = NULL;
qemuDomainPCIAddressBusPtr bus;
-
- if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
+ virErrorNumber errType = (fromConfig
+ ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);

if (!(str = qemuDomainPCIAddressAsString(addr)))
- return -1;
+ goto cleanup;

- VIR_DEBUG("Reserving PCI addr %s", str);
+ /* Add an extra bus if necessary */
+ if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
+ goto cleanup;
+ /* Check that the requested bus exists, is the correct type, and we
+ * are asking for a valid slot
+ */
+ if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
+ goto cleanup;

bus = &addrs->buses[addr->bus];
- if ((bus->minSlot && addr->slot < bus->minSlot) ||
- addr->slot > bus->maxSlot) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to reserve PCI address %s. "
- "Slot %02x can't be used on bus %04x, "
- "only slots %02zx - %02zx are permitted on this bus."),
- str, addr->slot, addr->bus, bus->minSlot, bus->maxSlot);
- }

- if (bus->slots[addr->slot] & (1 << addr->function)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to reserve PCI address %s already in use"), str);
- VIR_FREE(str);
- return -1;
+ if (reserveEntireSlot) {
+ if (bus->slots[addr->slot]) {
+ virReportError(errType,
+ _("Attempted double use of PCI slot %s "
+ "(may need \"multifunction='on'\" for "
+ "device on function 0)"), str);
+ goto cleanup;
+ }
+ bus->slots[addr->slot] = 0xFF; /* reserve all functions of slot */
+ VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", str);
+ } else {
+ if (bus->slots[addr->slot] & (1 << addr->function)) {
+ if (addr->function == 0) {
+ virReportError(errType,
+ _("Attempted double use of PCI Address %s"), str);
+ } else {
+ virReportError(errType,
+ _("Attempted double use of PCI Address %s "
+ "(may need \"multifunction='on'\" "
+ "for device on function 0)"), str);
+ }
+ goto cleanup;
+ }
+ bus->slots[addr->slot] |= (1 << addr->function);
+ VIR_DEBUG("Reserving PCI address %s", str);
}

+ ret = 0;
+cleanup:
VIR_FREE(str);
-
- addrs->lastaddr = *addr;
- addrs->lastaddr.function = 0;
- addrs->lastaddr.multi = 0;
- bus->slots[addr->slot] |= 1 << addr->function;
- return 0;
+ return ret;
}


@@ -1919,26 +1934,7 @@ qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
qemuDomainPCIConnectFlags flags)
{
- char *str;
-
- if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
-
- if (!(str = qemuDomainPCIAddressAsString(addr)))
- return -1;
-
- VIR_DEBUG("Reserving PCI slot %s", str);
-
- if (addrs->buses[addr->bus].slots[addr->slot]) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to reserve PCI slot %s"), str);
- VIR_FREE(str);
- return -1;
- }
-
- VIR_FREE(str);
- addrs->buses[addr->bus].slots[addr->slot] = 0xFF;
- return 0;
+ return qemuDomainPCIAddressReserveAddr(addrs, addr, flags, true, false);
}

int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
@@ -2044,12 +2040,11 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use",
a.domain, a.bus, a.slot);
}
-
}
}

virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("No more available PCI addresses"));
+ "%s", _("No more available PCI slots"));
return -1;

success:
@@ -2409,9 +2404,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,

addr.bus = tmp_addr.bus;
addr.slot = tmp_addr.slot;
+
+ addrs->lastaddr = addr;
+ addrs->lastaddr.function = 0;
+ addrs->lastaddr.multi = 0;
}
/* Finally we can reserve the slot+function */
- if (qemuDomainPCIAddressReserveAddr(addrs, &addr, flags) < 0)
+ if (qemuDomainPCIAddressReserveAddr(addrs, &addr, flags,
+ false, false) < 0)
goto error;

def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index c9f1600..bf4953a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -253,7 +253,9 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
qemuDomainPCIConnectFlags flags);
int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
- qemuDomainPCIConnectFlags flags);
+ qemuDomainPCIConnectFlags flags,
+ bool reserveEntireSlot,
+ bool fromConfig);
int qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev,
qemuDomainPCIConnectFlags flags);

[Eric Blake]

Post by Laine Stump
* The functions qemuDomainPCIAddressReserveAddr and
qemuDomainPCIAddressReserveSlot were very similar (and should have
been more similar) and were about to get more code added to them which
would create even more duplicated code, so this patch gives
qemuDomainPCIAddressReserveAddr a "reserveEntireSlot" arg, then
replaces the body of qemuDomainPCIAddressReserveSlot with a call to
qemuDomainPCIAddressReserveAddr.
You will notice that addrs->lastaddr was previously set in
qemuDomainPCIAddressReserveAddr (but *not* set in
qemuDomainPCIAddressReserveSlot). For consistency and cleanliness of
code, that bit was removed and put into the one caller of
qemuDomainPCIAddressReserveAddr (there is a similar place where the
caller of qemuDomainPCIAddressReserveSlot sets lastaddr). This does
guarantee identical functionality to pre-patch code, but in practice
isn't really critical, because lastaddr is just keeping track of where
to start when looking for a free slot - if it isn't updated, we will
just start looking on a slot that's already occupied, then skip up to
one that isn't.
* qemuCollectPCIAddress was essentially doing the same thing as
qemuDomainPCIAddressReserveAddr, but with some extra special case
checking at the beginning. The duplicate code has been replaced with
a call to qemuDomainPCIAddressReserveAddr. This required adding a
"fromConfig" boolean, which is only used to change the log error
code from VIR_ERR_INTERNAL_ERROR (when the address was
auto-generated by libvirt) to VIR_ERR_XML_ERROR (when the address is
coming from the config); without this differentiation, it would be
difficult to tell if an error was caused by something wrong in
libvirt's auto-allocate code or just bad config.
* the bit of code in qemuDomainPCIAddressValidate that checks the
connect type flags is going to be used in a couple more places where
we don't need to also check the slot limits (because we're generating
the slot number ourselves), so that has been pulled out into a
separate qemuDomainPCIAddressFlagsCompatible function.
---
src/qemu/qemu_command.c | 232 ++++++++++++++++++++++++------------------------
src/qemu/qemu_command.h | 4 +-
2 files changed, 119 insertions(+), 117 deletions(-)

ACK.

[Laine Stump]

ACK.

Thanks. I pushed this one, too.

[Doug Goldstein]

Post by Laine Stump
* The functions qemuDomainPCIAddressReserveAddr and
qemuDomainPCIAddressReserveSlot were very similar (and should have
been more similar) and were about to get more code added to them which
would create even more duplicated code, so this patch gives
qemuDomainPCIAddressReserveAddr a "reserveEntireSlot" arg, then
replaces the body of qemuDomainPCIAddressReserveSlot with a call to
qemuDomainPCIAddressReserveAddr.
You will notice that addrs->lastaddr was previously set in
qemuDomainPCIAddressReserveAddr (but *not* set in
qemuDomainPCIAddressReserveSlot). For consistency and cleanliness of
code, that bit was removed and put into the one caller of
qemuDomainPCIAddressReserveAddr (there is a similar place where the
caller of qemuDomainPCIAddressReserveSlot sets lastaddr). This does
guarantee identical functionality to pre-patch code, but in practice
isn't really critical, because lastaddr is just keeping track of where
to start when looking for a free slot - if it isn't updated, we will
just start looking on a slot that's already occupied, then skip up to
one that isn't.
* qemuCollectPCIAddress was essentially doing the same thing as
qemuDomainPCIAddressReserveAddr, but with some extra special case
checking at the beginning. The duplicate code has been replaced with
a call to qemuDomainPCIAddressReserveAddr. This required adding a
"fromConfig" boolean, which is only used to change the log error
code from VIR_ERR_INTERNAL_ERROR (when the address was
auto-generated by libvirt) to VIR_ERR_XML_ERROR (when the address is
coming from the config); without this differentiation, it would be
difficult to tell if an error was caused by something wrong in
libvirt's auto-allocate code or just bad config.
* the bit of code in qemuDomainPCIAddressValidate that checks the
connect type flags is going to be used in a couple more places where
we don't need to also check the slot limits (because we're generating
the slot number ourselves), so that has been pulled out into a
separate qemuDomainPCIAddressFlagsCompatible function.
---
src/qemu/qemu_command.c | 232 ++++++++++++++++++++++++------------------------
src/qemu/qemu_command.h | 4 +-
2 files changed, 119 insertions(+), 117 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4345456..abc973a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1434,9 +1434,53 @@ struct _qemuDomainPCIAddressSet {
};
-/*
- * Check that the PCI address is valid for use
- * with the specified PCI address set.
+static bool
+qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags busFlags,
+ qemuDomainPCIConnectFlags devFlags,
+ bool reportError)
+{
+ /* 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 (reportError) {
+ if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires a standard PCI slot, "
+ "which is not provided by this bus"),
+ addr->domain, addr->bus);

So even though this patch has been ACK'd and committed. I really would
have liked to see some info about the device printed in the error
message because when you're defining a domain its really not clear
which device is the problem.

Post by Laine Stump
+ } else {
+ /* this should never happen. If it does, there is a
+ * bug in the code that sets the flag bits for devices.
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("The device information has no PCI "
+ "connection types listed"));
+ }
+ }
+ return false;
+ }
+ if ((devFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
+ !(busFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
+ if (reportError) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires hot-plug capability, "
+ "which is not provided by the bus"),
+ addr->domain, addr->bus);

Same as above, when you're defining the whole domain its really not
clear which device is the problem so provide some details on which one
it is.

Post by Laine Stump
+ }
+ return false;
+ }
+ return true;
+}
+
+
+/* Verify that the address is in bounds for the chosen bus, and
+ * that the bus is of the correct type for the device (via
+ * comparing the flags).
*/
static bool
qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
@@ -1466,24 +1510,9 @@ qemuDomainPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs,
/* assure that at least one of the requested connection types is
* provided by this bus
*/
- if (!(flags & bus->flags & QEMU_PCI_CONNECT_TYPES_MASK)) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid PCI address: The PCI controller "
- "providing bus %04x doesn't support "
- "connections appropriate for the device "
- "(%x required vs. %x provided by bus)"),
- addr->bus, flags & QEMU_PCI_CONNECT_TYPES_MASK,
- bus->flags & QEMU_PCI_CONNECT_TYPES_MASK);
- return false;
- }
- /* make sure this bus allows hot-plug if the caller demands it */
- if ((flags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
- !(bus->flags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid PCI address: hot-pluggable slot requested, "
- "but bus %04x doesn't support hot-plug"), addr->bus);
+ if (!qemuDomainPCIAddressFlagsCompatible(addr, bus->flags, flags, true))
return false;
- }
+
/* some "buses" are really just a single port */
if (bus->minSlot && addr->slot < bus->minSlot) {
virReportError(VIR_ERR_XML_ERROR,
@@ -1597,10 +1626,10 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
virDomainDeviceInfoPtr info,
void *opaque)
{
+ qemuDomainPCIAddressSetPtr addrs = opaque;
int ret = -1;
- char *str = NULL;
virDevicePCIAddressPtr addr = &info->addr.pci;
- qemuDomainPCIAddressSetPtr addrs = opaque;
+ bool entireSlot;
qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
QEMU_PCI_CONNECT_TYPE_PCI);
@@ -1639,56 +1668,15 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
return 0;
}
- /* add an additional bus of the correct type if needed */
- if (addrs->dryRun &&
- qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
-
- /* verify that the address is in bounds for the chosen bus, and
- * that the bus is of the correct type for the device (via
- * comparing the flags).
- */
- if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
- return -1;
-
- if (!(str = qemuDomainPCIAddressAsString(addr)))
- goto cleanup;
+ entireSlot = (addr->function == 0 &&
+ addr->multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON);
- /* check if already in use */
- if (addrs->buses[addr->bus].slots[addr->slot] & (1 << addr->function)) {
- if (info->addr.pci.function != 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address '%s' "
- "(may need \"multifunction='on'\" for device on function 0)"),
- str);
- } else {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address '%s'"), str);
- }
+ if (qemuDomainPCIAddressReserveAddr(addrs, addr, flags,
+ entireSlot, true) < 0)
goto cleanup;
- }
- /* mark as in use */
- if ((info->addr.pci.function == 0) &&
- (info->addr.pci.multi != VIR_DEVICE_ADDRESS_PCI_MULTI_ON)) {
- /* a function 0 w/o multifunction=on must reserve the entire slot */
- if (addrs->buses[addr->bus].slots[addr->slot]) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Attempted double use of PCI Address on slot '%s' "
- "(need \"multifunction='off'\" for device "
- "on function 0)"),
- str);
- goto cleanup;
- }
- addrs->buses[addr->bus].slots[addr->slot] = 0xFF;
- VIR_DEBUG("Remembering PCI slot: %s (multifunction=off)", str);
- } else {
- VIR_DEBUG("Remembering PCI addr: %s", str);
- addrs->buses[addr->bus].slots[addr->slot] |= 1 << addr->function;
- }
ret = 0;
- VIR_FREE(str);
return ret;
}
@@ -1871,46 +1859,73 @@ static bool qemuDomainPCIAddressSlotInUse(qemuDomainPCIAddressSetPtr addrs,
}
+/*
+ * Reserve a slot (or just one function) for a device. If
+ * reserveEntireSlot is true, all functions for the slot are reserved,
+ * otherwise only one. If fromConfig is true, the address being
+ * requested came directly from the config and errors should be worded
+ * appropriately. If fromConfig is false, the address was
+ * automatically created by libvirt, so it is an internal error (not
+ * XML).
+ */
int
qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
- qemuDomainPCIConnectFlags flags)
+ qemuDomainPCIConnectFlags flags,
+ bool reserveEntireSlot,
+ bool fromConfig)
{
- char *str;
+ int ret = -1;
+ char *str = NULL;
qemuDomainPCIAddressBusPtr bus;
-
- if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
+ virErrorNumber errType = (fromConfig
+ ? VIR_ERR_XML_ERROR : VIR_ERR_INTERNAL_ERROR);
if (!(str = qemuDomainPCIAddressAsString(addr)))
- return -1;
+ goto cleanup;
- VIR_DEBUG("Reserving PCI addr %s", str);
+ /* Add an extra bus if necessary */
+ if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
+ goto cleanup;
+ /* Check that the requested bus exists, is the correct type, and we
+ * are asking for a valid slot
+ */
+ if (!qemuDomainPCIAddressValidate(addrs, addr, flags))
+ goto cleanup;
bus = &addrs->buses[addr->bus];
- if ((bus->minSlot && addr->slot < bus->minSlot) ||
- addr->slot > bus->maxSlot) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to reserve PCI address %s. "
- "Slot %02x can't be used on bus %04x, "
- "only slots %02zx - %02zx are permitted on this bus."),
- str, addr->slot, addr->bus, bus->minSlot, bus->maxSlot);
- }
- if (bus->slots[addr->slot] & (1 << addr->function)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to reserve PCI address %s already in use"), str);
- VIR_FREE(str);
- return -1;
+ if (reserveEntireSlot) {
+ if (bus->slots[addr->slot]) {
+ virReportError(errType,
+ _("Attempted double use of PCI slot %s "
+ "(may need \"multifunction='on'\" for "
+ "device on function 0)"), str);
+ goto cleanup;
+ }
+ bus->slots[addr->slot] = 0xFF; /* reserve all functions of slot */
+ VIR_DEBUG("Reserving PCI slot %s (multifunction='off')", str);
+ } else {
+ if (bus->slots[addr->slot] & (1 << addr->function)) {
+ if (addr->function == 0) {
+ virReportError(errType,
+ _("Attempted double use of PCI Address %s"), str);
+ } else {
+ virReportError(errType,
+ _("Attempted double use of PCI Address %s "
+ "(may need \"multifunction='on'\" "
+ "for device on function 0)"), str);
+ }
+ goto cleanup;
+ }
+ bus->slots[addr->slot] |= (1 << addr->function);
+ VIR_DEBUG("Reserving PCI address %s", str);
}
+ ret = 0;
VIR_FREE(str);
-
- addrs->lastaddr = *addr;
- addrs->lastaddr.function = 0;
- addrs->lastaddr.multi = 0;
- bus->slots[addr->slot] |= 1 << addr->function;
- return 0;
+ return ret;
}
@@ -1919,26 +1934,7 @@ qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
qemuDomainPCIConnectFlags flags)
{
- char *str;
-
- if (addrs->dryRun && qemuDomainPCIAddressSetGrow(addrs, addr, flags) < 0)
- return -1;
-
- if (!(str = qemuDomainPCIAddressAsString(addr)))
- return -1;
-
- VIR_DEBUG("Reserving PCI slot %s", str);
-
- if (addrs->buses[addr->bus].slots[addr->slot]) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unable to reserve PCI slot %s"), str);
- VIR_FREE(str);
- return -1;
- }
-
- VIR_FREE(str);
- addrs->buses[addr->bus].slots[addr->slot] = 0xFF;
- return 0;
+ return qemuDomainPCIAddressReserveAddr(addrs, addr, flags, true, false);
}
int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
@@ -2044,12 +2040,11 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
VIR_DEBUG("PCI slot %.4x:%.2x:%.2x already in use",
a.domain, a.bus, a.slot);
}
-
}
}
virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("No more available PCI addresses"));
+ "%s", _("No more available PCI slots"));
return -1;
@@ -2409,9 +2404,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
addr.bus = tmp_addr.bus;
addr.slot = tmp_addr.slot;
+
+ addrs->lastaddr = addr;
+ addrs->lastaddr.function = 0;
+ addrs->lastaddr.multi = 0;
}
/* Finally we can reserve the slot+function */
- if (qemuDomainPCIAddressReserveAddr(addrs, &addr, flags) < 0)
+ if (qemuDomainPCIAddressReserveAddr(addrs, &addr, flags,
+ false, false) < 0)
goto error;
def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index c9f1600..bf4953a 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -253,7 +253,9 @@ int qemuDomainPCIAddressReserveSlot(qemuDomainPCIAddressSetPtr addrs,
qemuDomainPCIConnectFlags flags);
int qemuDomainPCIAddressReserveAddr(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr addr,
- qemuDomainPCIConnectFlags flags);
+ qemuDomainPCIConnectFlags flags,
+ bool reserveEntireSlot,
+ bool fromConfig);
int qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev,
qemuDomainPCIConnectFlags flags);
--
1.7.11.7
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

[Laine Stump]

Post by Doug Goldstein

Post by Laine Stump
- * Check that the PCI address is valid for use
- * with the specified PCI address set.
+static bool
+qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
+ qemuDomainPCIConnectFlags busFlags,
+ qemuDomainPCIConnectFlags devFlags,
+ bool reportError)
+{
+ /* 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 (reportError) {
+ if (devFlags & QEMU_PCI_CONNECT_TYPE_PCI) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires a standard PCI slot, "
+ "which is not provided by this bus"),
+ addr->domain, addr->bus);

So even though this patch has been ACK'd and committed. I really would
have liked to see some info about the device printed in the error
message because when you're defining a domain its really not clear
which device is the problem.

Yes, that would require some extra plumbing and adding in extra code
because this is a common function called for many different types of
device, so we would need to pass in a device-type enum. If, on the other
hand, we just send an error code back up the return stack to the caller,
then we'll have a dozen or so callers with nearly (but not quite)
identical error messages.

Post by Doug Goldstein

Post by Laine Stump
+ } else {
+ /* this should never happen. If it does, there is a
+ * bug in the code that sets the flag bits for devices.
+ */
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("The device information has no PCI "
+ "connection types listed"));
+ }
+ }
+ return false;
+ }
+ if ((devFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE) &&
+ !(busFlags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) {
+ if (reportError) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires hot-plug capability, "
+ "which is not provided by the bus"),
+ addr->domain, addr->bus);

Same as above, when you're defining the whole domain its really not
clear which device is the problem so provide some details on which one
it is.

Same issue makes it cumbersome - the more detailed info about the type
of device (and which device of that type) is several layers up the call
chain, so it's going to take extra setup at the higher level, and
passing a device-type enum down to do this.

I agree that it needs to be done before 1.1.2, but I think it's
important for this code to get wider testing even sooner.

[Laine Stump]

Previous refactoring of the guest PCI address reservation/allocation
code allowed for slot types other than basic PCI (e.g. PCI express,
non-hotpluggable slots, etc) but would not auto-allocate a slot for a
device that required any type other than a basic hot-pluggable
PCI slot.

This patch refactors the code to be aware of different slot types
during auto-allocation of addresses as well - as long as there is an
empty slot of the required type, it will be found and used.

The piece that *wasn't* added is that we don't auto-create a new PCI
bus when needed for anything except basic PCI devices. This is because
there are multiple different types of controllers that can provide,
for example, a PCI express slot (in addition to the pcie-root
controller, these can also be found on a "root-port" or on a
"downstream-switch-port"). Since we currently don't support any PCIe
devices (except pending support for dmi-to-pci-bridge), we can defer
any decision on what to do about this.
---
src/qemu/qemu_command.c | 112 ++++++++++++++++++++++++++++++++++++++----------
1 file changed, 90 insertions(+), 22 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index abc973a..cafc4bf 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1429,6 +1429,7 @@ struct _qemuDomainPCIAddressSet {
qemuDomainPCIAddressBus *buses;
size_t nbuses;
virDevicePCIAddress lastaddr;
+ qemuDomainPCIConnectFlags lastFlags;
bool dryRun; /* on a dry run, new buses are auto-added
and addresses aren't saved in device infos */
};
@@ -1630,7 +1631,7 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
int ret = -1;
virDevicePCIAddressPtr addr = &info->addr.pci;
bool entireSlot;
- /* FIXME: flags should be set according to the requirements of @device */
+ /* flags may be changed from default below */
qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
QEMU_PCI_CONNECT_TYPE_PCI);

@@ -1644,28 +1645,60 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
return 0;
}

+ /* 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;
+ break;
+ default:
+ break;
+ }
+ }
+
/* Ignore implicit controllers on slot 0:0:1.0:
* implicit IDE controller on 0:0:1.1 (no qemu command line)
* implicit USB controller on 0:0:1.2 (-usb)
*
* If the machine does have a PCI bus, they will get reserved
* in qemuAssignDevicePCISlots().
- *
- * FIXME: When we have support for a pcie-root controller at bus
- * 0, we will no longer be able to skip checking of these devices,
- * as they are PCI, and thus can't be connected to bus 0 if it is
- * PCIe rather than PCI.
+ */
+
+ /* These are the IDE and USB controllers in the PIIX3, hardcoded
+ * to bus 0 slot 1. They cannot be attached to a PCIe slot, only
+ * PCI.
*/
if (device->type == VIR_DOMAIN_DEVICE_CONTROLLER && addr->domain == 0 &&
addr->bus == 0 && addr->slot == 1) {
virDomainControllerDefPtr cont = device->data.controller;
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && cont->idx == 0 &&
- addr->function == 1)
- return 0;
- if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 &&
- (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
- cont->model == -1) && addr->function == 2)
- return 0;
+
+ if ((cont->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE && cont->idx == 0 &&
+ addr->function == 1) ||
+ (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB && cont->idx == 0 &&
+ (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_USB_PIIX3_UHCI ||
+ cont->model == -1) && addr->function == 2)) {
+ /* Note the check for nbuses > 0 - if there are no PCI
+ * buses, we skip this check. This is a quirk required for
+ * some machinetypes such as s390, which pretend to have a
+ * PCI bus for long enough to generate the "-usb" on the
+ * commandline, but that don't really care if a PCI bus
+ * actually exists. */
+ if (addrs->nbuses > 0 &&
+ !(addrs->buses[0].flags & QEMU_PCI_CONNECT_TYPE_PCI)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Bus 0 must be PCI for integrated PIIX3 "
+ "USB or IDE controllers"));
+ return -1;
+ } else {
+ return 0;
+ }
+ }
}

entireSlot = (addr->function == 0 &&
@@ -1695,8 +1728,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
int nbuses = 0;
size_t i;
int rv;
- qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
- QEMU_PCI_CONNECT_TYPE_PCI);
+ qemuDomainPCIConnectFlags flags = QEMU_PCI_CONNECT_TYPE_PCI;

for (i = 0; i < def->ncontrollers; i++) {
if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
@@ -1941,7 +1973,11 @@ int qemuDomainPCIAddressEnsureAddr(qemuDomainPCIAddressSetPtr addrs,
virDomainDeviceInfoPtr dev)
{
int ret = 0;
- /* FIXME: flags should be set according to the particular device */
+ /* Flags should be set according to the particular device,
+ * but only the caller knows the type of device. Currently this
+ * function is only used for hot-plug, though, and hot-plug is
+ * only supported for standard PCI devices, so we can safely use
+ * the setting below */
qemuDomainPCIConnectFlags flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE |
QEMU_PCI_CONNECT_TYPE_PCI);

@@ -2005,7 +2041,13 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
virDevicePCIAddressPtr next_addr,
qemuDomainPCIConnectFlags flags)
{
- virDevicePCIAddress a = addrs->lastaddr;
+ virDevicePCIAddress a = { 0, 0, 0, 0, false };
+
+ /* Avoid re-scanning from start if we're searching for exactly the
+ * same type of slot as last time.
+ */
+ if (flags == addrs->lastFlags)
+ a = addrs->lastaddr;

if (addrs->nbuses == 0) {
virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available"));
@@ -2014,6 +2056,12 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,

/* Start the search at the last used bus and slot */
for (a.slot++; a.bus < addrs->nbuses; a.bus++) {
+ if (!qemuDomainPCIAddressFlagsCompatible(&a, addrs->buses[a.bus].flags,
+ flags, false)) {
+ VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device",
+ a.domain, a.bus);
+ continue;
+ }
for (; a.slot <= QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
if (!qemuDomainPCIAddressSlotInUse(addrs, &a))
goto success;
@@ -2030,9 +2078,15 @@ qemuDomainPCIAddressGetNextSlot(qemuDomainPCIAddressSetPtr addrs,
if (qemuDomainPCIAddressSetGrow(addrs, &a, flags) < 0)
return -1;
goto success;
- } else {
+ } else if (flags == addrs->lastFlags) {
/* Check the buses from 0 up to the last used one */
for (a.bus = 0; a.bus <= addrs->lastaddr.bus; a.bus++) {
+ if (!qemuDomainPCIAddressFlagsCompatible(&a, addrs->buses[a.bus].flags,
+ flags, false)) {
+ VIR_DEBUG("PCI bus %.4x:%.2x is not compatible with the device",
+ a.domain, a.bus);
+ continue;
+ }
for (a.slot = 1; a.slot <= QEMU_PCI_ADDRESS_SLOT_LAST; a.slot++) {
if (!qemuDomainPCIAddressSlotInUse(addrs, &a))
goto success;
@@ -2072,6 +2126,7 @@ qemuDomainPCIAddressReserveNextSlot(qemuDomainPCIAddressSetPtr addrs,
}

addrs->lastaddr = addr;
+ addrs->lastFlags = flags;
return 0;
}

@@ -2285,15 +2340,26 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
goto error;
}

- flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
-
/* PCI controllers */
for (i = 0; i < def->ncontrollers; i++) {
if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
- if (def->controllers[i]->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT)
- continue;
if (def->controllers[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;
+ switch (def->controllers[i]->model) {
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ /* pci-root is implicit in the machine,
+ * and needs no address */
+ continue;
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
+ /* pci-bridge doesn't require hot-plug
+ * (although it does provide hot-plug in its slots)
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCI;
+ break;
+ default:
+ flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
+ break;
+ }
if (qemuDomainPCIAddressReserveNextSlot(addrs,
&def->controllers[i]->info,
flags) < 0)
@@ -2301,6 +2367,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
}
}

+ flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
+
for (i = 0; i < def->nfss; i++) {
if (def->fss[i]->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
continue;

[Laine Stump]

This controller is implicit on q35 machinetypes. It provides 31 PCIe
(*not* PCI) slots as controller 0.

Currently there are no devices that can connect to pcie-root, and no
implicit pci controller on a q35 machine, so q35 is still
unusable. For a usable q35 system, we need to add a
"dmi-to-pci-bridge" pci controller, which can connect to pcie-root,
and provides standard pci slots that can be used to connect other
devices.
---
docs/formatdomain.html.in | 27 ++++++++++++++++------
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 8 ++++---
src/conf/domain_conf.h | 1 +
src/qemu/qemu_command.c | 23 ++++++++++++++----
src/qemu/qemu_command.h | 4 +++-
src/qemu/qemu_domain.c | 19 +++++++++++----
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 4 ++++
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml | 21 +++++++++++++++++
tests/qemuxml2argvtest.c | 2 ++
tests/qemuxml2xmltest.c | 1 +
11 files changed, 92 insertions(+), 19 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 49c7c8d..330dca2 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2338,13 +2338,11 @@

<p>
PCI controllers have an optional <code>model</code> attribute with
- possible values <code>pci-root</code> or <code>pci-bridge</code>.
- For machine types which provide an implicit pci bus, the pci-root
+ possible values <code>pci-root</code>, <code>pcie-root</code>
+ or <code>pci-bridge</code>.
+ For machine types which provide an implicit PCI bus, the pci-root
controller with index=0 is auto-added and required to use PCI devices.
- PCI root has no address.
- PCI bridges are auto-added if there are too many devices to fit on
- the one bus provided by pci-root, or a PCI bus number greater than zero
- was specified.
+ pci-root has no address.
PCI bridges can also be specified manually, but their addresses should
only refer to PCI buses provided by already specified PCI controllers.
Leaving gaps in the PCI controller indexes might lead to an invalid
@@ -2356,11 +2354,26 @@
&lt;devices&gt;
&lt;controller type='pci' index='0' model='pci-root'/&gt;
&lt;controller type='pci' index='1' model='pci-bridge'&gt;
- &lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction=off'/&gt;
+ &lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction='off'/&gt;
&lt;/controller&gt;
&lt;/devices&gt;
...</pre>

+ <p>
+ For machine types which provide an implicit PCI Express (PCIe)
+ 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. (<span class="since">since 1.1.2</span>).
+ </p>
+<pre>
+ ...
+ &lt;devices&gt;
+ &lt;controller type='pci' index='0' model='pcie-root'/&gt;
+ &lt;/devices&gt;
+ ...</pre>
+
<h4><a name="elementsLease">Device leases</a></h4>

<p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 745b959..e04be12 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1538,6 +1538,7 @@
<attribute name="model">
<choice>
<value>pci-root</value>
+ <value>pcie-root</value>
<value>pci-bridge</value>
</choice>
</attribute>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 63350b6..59a96f2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -310,6 +310,7 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,

VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pci-root",
+ "pcie-root",
"pci-bridge")

VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
@@ -5714,16 +5715,17 @@ virDomainControllerDefParseXML(xmlNodePtr node,
case VIR_DOMAIN_CONTROLLER_TYPE_PCI:
switch (def->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("pci-root controller should not "
+ _("pci-root and pcie-root controllers should not "
"have an address"));
goto error;
}
if (def->idx != 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
- _("pci-root controller should have "
- "index 0"));
+ _("pci-root and pcie-root controllers "
+ "should have index 0"));
goto error;
}

diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index de3b59c..63a1444 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -768,6 +768,7 @@ enum virDomainControllerType {

typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,

VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index cafc4bf..b6912ce 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1453,6 +1453,12 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr,
"device. Device requires a standard PCI slot, "
"which is not provided by this bus"),
addr->domain, addr->bus);
+ } else if (devFlags & QEMU_PCI_CONNECT_TYPE_PCIE) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("PCI bus %.4x:%.2x is not compatible with the "
+ "device. Device requires a PCI Express slot, "
+ "which is not provided by this bus"),
+ addr->domain, addr->bus);
} else {
/* this should never happen. If it does, there is a
* bug in the code that sets the flag bits for devices.
@@ -1549,6 +1555,12 @@ qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
bus->minSlot = 1;
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;
+ bus->minSlot = 1;
+ bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PCI controller model %d"), model);
@@ -2347,7 +2359,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
continue;
switch (def->controllers[i]->model) {
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
- /* pci-root is implicit in the machine,
+ case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
+ /* pci-root and pcie-root are implicit in the machine,
* and needs no address */
continue;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE:
@@ -4336,8 +4349,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
def->idx, def->idx);
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"));
+ _("wrong function called for pci-root/pcie-root"));
return NULL;
}
break;
@@ -7615,9 +7629,10 @@ qemuBuildCommandLine(virConnectPtr conn,
continue;
}

- /* Skip pci-root */
+ /* Skip pci-root/pcie-root */
if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
- cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
+ (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
+ cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) {
continue;
}

diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index bf4953a..e5111d2 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -233,13 +233,15 @@ typedef enum {

QEMU_PCI_CONNECT_TYPE_PCI = 1 << 2,
/* PCI devices can connect to this bus */
+ QEMU_PCI_CONNECT_TYPE_PCIE = 1 << 3,
+ /* PCI Express devices can connect to this bus */
} qemuDomainPCIConnectFlags;

/* a combination of all bit that describe the type of connections
* allowed, e.g. PCI, PCIe, switch
*/
# define QEMU_PCI_CONNECT_TYPES_MASK \
- QEMU_PCI_CONNECT_TYPE_PCI
+ (QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE)


int qemuDomainAssignPCIAddresses(virDomainDefPtr def,
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3da666..4d04609 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -701,6 +701,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
{
bool addDefaultUSB = true;
bool addPCIRoot = false;
+ bool addPCIeRoot = false;

/* check for emulator and create a default one if needed */
if (!def->emulator &&
@@ -713,12 +714,16 @@ qemuDomainDefPostParse(virDomainDefPtr def,
case VIR_ARCH_X86_64:
if (!def->os.machine)
break;
- if (STRPREFIX(def->os.machine, "pc-q35") ||
- STREQ(def->os.machine, "q35") ||
- STREQ(def->os.machine, "isapc")) {
+ if (STREQ(def->os.machine, "isapc")) {
addDefaultUSB = false;
break;
}
+ if (STRPREFIX(def->os.machine, "pc-q35") ||
+ STREQ(def->os.machine, "q35")) {
+ addPCIeRoot = true;
+ addDefaultUSB = false;
+ break;
+ }
if (!STRPREFIX(def->os.machine, "pc-0.") &&
!STRPREFIX(def->os.machine, "pc-1.") &&
!STRPREFIX(def->os.machine, "pc-i440") &&
@@ -755,6 +760,12 @@ qemuDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
return -1;

+ if (addPCIeRoot &&
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0)
+ return -1;
+
return 0;
}

@@ -1421,7 +1432,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver,

if (pci && pci->idx == 0 &&
pci->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
- VIR_DEBUG("Removing default 'pci-root' from domain '%s'"
+ VIR_DEBUG("Removing default pci-root from domain '%s'"
" for migration compatibility", def->name);
toremove++;
} else {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
new file mode 100644
index 0000000..e937189
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -0,0 +1,4 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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 pci-bridge,chassis_nr=1,id=pci.1,bus=pci.1,addr=0x1 -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
new file mode 100644
index 0000000..1aa5455
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
@@ -0,0 +1,21 @@
+<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>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='usb' index='0'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index b7485fc..57c6989 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -994,6 +994,8 @@ mymain(void)
DO_TEST("pci-autoadd-idx", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
DO_TEST("pci-bridge-many-disks",
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
+ DO_TEST("pcie-root",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE);

DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 66be40e..ea511b8 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -294,6 +294,7 @@ mymain(void)
DO_TEST_DIFFERENT("pci-bridge-many-disks");
DO_TEST_DIFFERENT("pci-autoadd-addr");
DO_TEST_DIFFERENT("pci-autoadd-idx");
+ DO_TEST("pcie-root");

DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");

[Laine Stump]

This PCI controller connects to a PCI Express slot (e.g. one of the
slots provided by the pcie-root controller, aka "pcie.0" on the qemu
commandline), and provides 31 *non-hot-pluggable* PCI (*not* PCIe)
slots, numbered 1-31.

Any time a machine is defined which has a pcie-root controller
(i.e. any q35-based machinetype), libvirt will automatically add a
dmi-to-pci-bridge controller if one doesn't exist, and also add a
pci-bridge controller. The reasoning here is that any useful domain
will have either an immediate (startup time) or eventual (subsequent
hot-plug) need for a standard PCI slot; since the pcie-root controller
only provides PCIe slots, we need to connect a dmi-to-pci-bridge
controller to it in order to get a non-hot-plug PCI slot that we can
then use to connect a pci-bridge - the slots provided by the
pci-bridge will be both standard PCI and hot-pluggable.

Since pci-bridge devices themselves are not hot-pluggable, any new
pci-bridge controller that is added can (and will) be plugged into the
dmi-to-pci-bridge as long as it has empty slots available.
---
docs/formatdomain.html.in | 28 +++++++++++++++---
docs/schemas/domaincommon.rng | 1 +
src/conf/domain_conf.c | 3 +-
src/conf/domain_conf.h | 1 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 34 ++++++++++++++++++++++
src/qemu/qemu_domain.c | 14 +++++++--
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 3 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 7 +++++
tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 25 ++++++++++++++++
tests/qemuxml2argvtest.c | 8 ++++-
.../qemuxml2xmlout-pcie-root.xml | 23 +++++++++++++++
tests/qemuxml2xmltest.c | 3 +-
14 files changed, 143 insertions(+), 10 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.args
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.xml
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 330dca2..8fa4c0e 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2338,16 +2338,19 @@

<p>
PCI controllers have an optional <code>model</code> attribute with
- possible values <code>pci-root</code>, <code>pcie-root</code>
- or <code>pci-bridge</code>.
+ possible values <code>pci-root</code>, <code>pcie-root</code>,
+ <code>pci-bridge</code>, or <code>dmi-to-pci-bridge</code>.
For machine types which provide an implicit PCI bus, the pci-root
controller with index=0 is auto-added and required to use PCI devices.
pci-root has no address.
+ PCI bridges are auto-added if there are too many devices to fit on
+ the one bus provided by pci-root, or a PCI bus number greater than zero
+ was specified.
PCI bridges can also be specified manually, but their addresses should
only refer to PCI buses provided by already specified PCI controllers.
Leaving gaps in the PCI controller indexes might lead to an invalid
configuration.
- (<span class="since">since 1.0.5</span>)
+ (pci-root and pci-bridge <span class="since">since 1.0.5</span>).
</p>
<pre>
...
@@ -2365,12 +2368,29 @@
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. (<span class="since">since 1.1.2</span>).
+ devices. In order to connect standard PCI devices 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
+ 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
+ auto-determined by libvirt will be placed on this pci-bridge
+ device. (<span class="since">since 1.1.2</span>).
</p>
<pre>
...
&lt;devices&gt;
&lt;controller type='pci' index='0' model='pcie-root'/&gt;
+ &lt;controller type='pci' index='1' model='dmi-to-pci-bridge'&gt;
+ &lt;address type='pci' domain='0' bus='0' slot='0xe' function='0'/&gt;
+ &lt;/controller&gt;
+ &lt;controller type='pci' index='2' model='pci-bridge'&gt;
+ &lt;address type='pci' domain='0' bus='1' slot='1' function='0'/&gt;
+ &lt;/controller&gt;
&lt;/devices&gt;
...</pre>

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index e04be12..173359c 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1540,6 +1540,7 @@
<value>pci-root</value>
<value>pcie-root</value>
<value>pci-bridge</value>
+ <value>dmi-to-pci-bridge</value>
</choice>
</attribute>
</group>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 59a96f2..d17008f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -311,7 +311,8 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST,
"pci-root",
"pcie-root",
- "pci-bridge")
+ "pci-bridge",
+ "dmi-to-pci-bridge")

VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
"auto",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 63a1444..3e118d6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -770,6 +770,7 @@ typedef enum {
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
+ VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,

VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
} virDomainControllerModelPCI;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08406b8..47cc07a 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -234,6 +234,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,

"vnc-share-policy", /* 150 */
"device-del-event",
+ "dmi-to-pci-bridge",
);

struct _virQEMUCaps {
@@ -1381,6 +1382,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE },
{ "vfio-pci", QEMU_CAPS_DEVICE_VFIO_PCI },
{ "scsi-generic", QEMU_CAPS_DEVICE_SCSI_GENERIC },
+ { "i82801b11-bridge", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
};

static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index f5f685d..074e55d 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -190,6 +190,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_MLOCK = 149, /* -realtime mlock=on|off */
QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */

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 b6912ce..9caf036 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1561,6 +1561,13 @@ qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
bus->minSlot = 1;
bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ /* slots 1 - 31, standard PCI slots,
+ * but *not* hot-pluggable */
+ bus->flags = QEMU_PCI_CONNECT_TYPE_PCI;
+ bus->minSlot = 1;
+ bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
+ break;
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Invalid PCI controller model %d"), model);
@@ -1669,6 +1676,12 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
*/
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;
}
@@ -2369,6 +2382,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
*/
flags = QEMU_PCI_CONNECT_TYPE_PCI;
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ /* dmi-to-pci-bridge requires a non-hotplug PCIe
+ * slot
+ */
+ flags = QEMU_PCI_CONNECT_TYPE_PCIE;
+ break;
default:
flags = QEMU_PCI_CONNECT_HOTPLUGGABLE | QEMU_PCI_CONNECT_TYPE_PCI;
break;
@@ -4348,6 +4367,21 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d",
def->idx, def->idx);
break;
+ case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
+ if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("The dmi-to-pci-bridge (i82801b11-pci-bridge) "
+ "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-pci-bridge,chassis_nr=%d,id=pci.%d",
+ def->idx, def->idx);
+ break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 4d04609..f5030cd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -760,10 +760,20 @@ qemuDomainDefPostParse(virDomainDefPtr def,
VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
return -1;

+ /* When a machine has a pcie-root, make sure that there is always
+ * a dmi-to-pci-bridge controller added as bus 1, and a pci-bridge
+ * as bus 2, so that standard PCI devices can be connected
+ */
if (addPCIeRoot &&
- virDomainDefMaybeAddController(
+ (virDomainDefMaybeAddController(
def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
- VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0)
+ VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0 ||
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
+ VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
+ VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0))
return -1;

return 0;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index e937189..75a9844 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -1,4 +1,5 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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 pci-bridge,chassis_nr=1,id=pci.1,bus=pci.1,addr=0x1 -usb
+-device i82801b11-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 -usb
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
new file mode 100644
index 0000000..2c67739
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -0,0 +1,7 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
+/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-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
+-usb \
+-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
new file mode 100644
index 0000000..3541b14
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
@@ -0,0 +1,25 @@
+<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>
+ <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'/>
+ <video>
+ <model type='qxl' ram='65536' vram='18432' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 57c6989..aba0f88 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -995,7 +995,13 @@ mymain(void)
DO_TEST("pci-bridge-many-disks",
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
DO_TEST("pcie-root",
- QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
+ DO_TEST("q35",
+ QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
+ QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ 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,
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
new file mode 100644
index 0000000..25c77f1
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
@@ -0,0 +1,23 @@
+<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>
+ <controller type='pci' index='0' model='pcie-root'/>
+ <controller type='usb' index='0'/>
+ <controller type='pci' index='1' model='dmi-to-pci-bridge'/>
+ <controller type='pci' index='2' model='pci-bridge'/>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index ea511b8..8b4590a 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -294,7 +294,8 @@ mymain(void)
DO_TEST_DIFFERENT("pci-bridge-many-disks");
DO_TEST_DIFFERENT("pci-autoadd-addr");
DO_TEST_DIFFERENT("pci-autoadd-idx");
- DO_TEST("pcie-root");
+ DO_TEST_DIFFERENT("pcie-root");
+ DO_TEST("q35");

DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");

[Laine Stump]

Post by Laine Stump
This PCI controller connects to a PCI Express slot (e.g. one of the
slots provided by the pcie-root controller, aka "pcie.0" on the qemu
commandline), and provides 31 *non-hot-pluggable* PCI (*not* PCIe)
slots, numbered 1-31.
Any time a machine is defined which has a pcie-root controller
(i.e. any q35-based machinetype), libvirt will automatically add a
dmi-to-pci-bridge controller if one doesn't exist, and also add a
pci-bridge controller. The reasoning here is that any useful domain
will have either an immediate (startup time) or eventual (subsequent
hot-plug) need for a standard PCI slot; since the pcie-root controller
only provides PCIe slots, we need to connect a dmi-to-pci-bridge
controller to it in order to get a non-hot-plug PCI slot that we can
then use to connect a pci-bridge - the slots provided by the
pci-bridge will be both standard PCI and hot-pluggable.
Since pci-bridge devices themselves are not hot-pluggable, any new
pci-bridge controller that is added can (and will) be plugged into the
dmi-to-pci-bridge as long as it has empty slots available.

Sigh. (yeah, I know I've been saying that a lot lately :-)

I've just now figured out that seabios doesn't recognize any disk device
that is connected anywhere behind the i82801b11-bridge device. So if you
have a virtio-blk-pci device that is attached in this manner (the way
these patches are setup to work):

pcie.0->i82801b11-bridge->pci-bridge->virtio-blk-pci

or even like this (precluding later hot-plug, but at least still not
violating the hardware specs):

pcie.0->i82801b11-bridge->virtio-blk-pci

seabios won't see the disk, and it won't boot.

However, if you ignore the fact that the pci-bridge device requires a
standard PCI slot (*not* PCIe) and shouldn't be directly plugged into
pcie.0 and do this (which is allowed by qemu even though it strictly
speaking *shouldn't* be):

pcie.0->pci-bridge->virtio-blk-pci

then seabios recognizes the disk and is able to boot from it.

So I see three options for taking care of of this problem

1) leave this libvirt code as it is, and petition seabios to add support
for recognizing i82801b11-bridge devices and booting from disks that are
connected behind them

drawback: we will remain unable to create working Q35 machines with
libvirt until seabios is fixed.


2) modify these patches to add the pci-bridge device directly to pcie.0
and not add any dmi-to-pci-bridge in the middle (this will end up
encoded in the domain's config)

drawback: when seabios is fixed and libvirt switches to the "proper"
model of the hardware, there will be many configurations already out in
the field using the old model which will either need to be modified, or
libvirt will need to continue allowing/supporting that setup.


3) modify these patches to substitute a pci-bridge for the
i82801b11-bridge when someone requests a dmi-to-pci-bridge.

advantage: when seabios is fixed, configs already in the field will
continue to work

drawback: live migration of a domain from old to new will fail in some
strange as-yet-to-be-determined fashion. Also, until seabios is fixed
and we remove the patch to replace all i82801b11-bridge devices with
pci-bridge at runtime, there will be no testing of the i82801b11-bridge
code in qemu (at least not via libvirt-managed guests).


Presumably if I did option (2) or (3), this would be changed back at a
later date when seabios is fixed to recognize i82801b11-bridge properly.
Of course we will want libvirt to work properly no matter what is the
version of seabios/qemu, so we'll have to figure out a way to determine
if the seabios bugfix is in or not.


Does anyone have an opinion on which of these is preferred (or a
different idea)? I'm tending towards option (3), since that will cause
the least headaches later (there won't be a bunch of stale configs that
need to be updated). Whatever I do, it will be done in a separate patch
from this, because this patch does implement things in the "correct"
manner, and should stand; any hack to workaround the problem in seabios
should be a separate object in the git history.

[Gerd Hoffmann]

Hi,

Post by Laine Stump

Post by Laine Stump
Since pci-bridge devices themselves are not hot-pluggable, any new
pci-bridge controller that is added can (and will) be plugged into the
dmi-to-pci-bridge as long as it has empty slots available.

Sigh. (yeah, I know I've been saying that a lot lately :-)
I've just now figured out that seabios doesn't recognize any disk device
that is connected anywhere behind the i82801b11-bridge device.

Huh?

Post by Laine Stump
1) leave this libvirt code as it is, and petition seabios to add support
for recognizing i82801b11-bridge devices and booting from disks that are
connected behind them

This one. It's not that seabios has problems with PCI bridges. It
finds then, initializes them and maps the devices behind it just fine:

PCI: map device bdf=00:1f.3 bar 4, addr 0000d000, size 00000040 [io]
PCI: map device bdf=00:02.0 bar 1, addr 0000d040, size 00000040 [io]
PCI: map device bdf=00:1f.2 bar 4, addr 0000d080, size 00000020 [io]
PCI: map device bdf=00:02.0 bar 6, addr feb00000, size 00040000 [mem]
PCI: map device bdf=00:02.0 bar 0, addr feb40000, size 00020000 [mem]
PCI: map device bdf=00:01.0 bar 6, addr feb60000, size 00010000 [mem]
PCI: map device bdf=00:1f.2 bar 5, addr feb70000, size 00001000 [mem]
PCI: map device bdf=00:01.0 bar 1, addr feb71000, size 00001000 [mem]
PCI: map device bdf=00:01.0 bar 0, addr fc000000, size 02000000 [prefmem]
PCI: map device bdf=01:00.0 bar 0, addr 0000c000, size 00000040 [io]
PCI: map device bdf=01:00.0 bar 1, addr fea00000, size 00001000 [mem]

It looks more like a simple bug in the virtio code:

found virtio-scsi at 1:0
ERROR: queue size 65535 > 128
fail to find vq for virtio-scsi 1:0

Probably easy to fix, I'll have a look later today.

cheers,
Gerd

[Gerd Hoffmann]

Hi,

Post by Gerd Hoffmann
found virtio-scsi at 1:0
ERROR: queue size 65535 > 128
fail to find vq for virtio-scsi 1:0
Probably easy to fix, I'll have a look later today.

Ok, isn't that easy. Funny thing is that seabios boots just fine when
using "pci-bridge" but doesn't when using "i82801b11-bridge".

Looks like the I/O access is either blocked or misrouted somewhere
(65535 == 0xffff).

cheers,
Gerd

[Gerd Hoffmann]

Hi,

Post by Gerd Hoffmann
Ok, isn't that easy. Funny thing is that seabios boots just fine when
using "pci-bridge" but doesn't when using "i82801b11-bridge".

Turned out to not be seabios fault at all. Bug is in qemu,
i82801b11-bridge completely ignores any io window setup done by the
guest. Oops.

cheers,
Gerd

[Doug Goldstein]

Post by Gerd Hoffmann
Hi,

Post by Gerd Hoffmann
Ok, isn't that easy. Funny thing is that seabios boots just fine when
using "pci-bridge" but doesn't when using "i82801b11-bridge".

Turned out to not be seabios fault at all. Bug is in qemu,
i82801b11-bridge completely ignores any io window setup done by the
guest. Oops.
cheers,
Gerd

Thanks a bunch for tracking that down Gerd. I can confirm that fixes
the issue and the libvirt patch to work around this is unnecessary.

[Laine Stump]

Post by Doug Goldstein

Post by Gerd Hoffmann
Hi,

Post by Gerd Hoffmann
Ok, isn't that easy. Funny thing is that seabios boots just fine when
using "pci-bridge" but doesn't when using "i82801b11-bridge".

Turned out to not be seabios fault at all. Bug is in qemu,
i82801b11-bridge completely ignores any io window setup done by the
guest. Oops.
cheers,
Gerd

Thanks a bunch for tracking that down Gerd. I can confirm that fixes
the issue and the libvirt patch to work around this is unnecessary.

Yep. If we can count on that going into qemu for 1.6 (and being
backported to any appropriate maintenance and downstream releases) then
I'll gladly drop this patch like the steaming pile that it is :-)

Thanks Gerd for finding and fixing the problem, and thanks Doug for
taking the time to patch/rebuild both packages and test it!

[Laine Stump]

This patch adds in special handling for a few devices that need to be
treated differently for q35 domains:

usb - there is no implicit/default usb controller for the q35
machinetype. This is done because normally the default usb controller
is added to a domain by just adding "-usb" to the qemu commandline,
and it's assumed that this will add a single piix3 usb1 controller at
slot 1 function 2. That's not what happens when the machinetype is
q35, though. Instead, adding -usb to the commandline adds 3 usb
(version 2) controllers to the domain at slot 0x1D.{1,2,7}. Rather
than having

<controller type='usb' index='0'/>

translate into 3 separate devices on the PCI bus, it's cleaner to not
automatically add a default usb device; one can always be added
explicitly if desired. Or we may decide that on q35 machines, 3 usb
controllers will be automatically added when none is given. But for
this initial commit, at least we aren't locking ourselves into
something we later won't want.

video - for pc machine types, the primary video device is always in
slot 2, and that slot is reserved even when no video device has been
specified. On q35, when you specify "-vga qxl" on the qemu
commandline, the vga device is put in slot 1, not 2. Assuming that
this was done for a reason, this patch always puts the primary video
for q35 machines in slot 1, and reserves slot 1 even if there is no
video.

sata - a q35 machine always has a sata controller implicitly added at
slot 0x1F, function 2. There is no way to avoid this controller, so we
always add it. Note that the xml2xml tests for the pcie-root and q35
cases were changed to use DO_TEST_DIFFERENT() so that we can check for
the sata controller being automatically added. This is especially
important because we can't check for it in the xml2argv output (it has
no effect on that output since it's an implicit device).

ide - q35 has no ide controllers.

isa and smbus controllers - these two are always present in a q35 (at
slot 0x1F functions 0 and 3) but we have no way of modelling them in
our config. We do need to reserve those functions so that the user
doesn't attempt to put anything else there though.
---
src/qemu/qemu_command.c | 150 ++++++++++++++++++++-
src/qemu/qemu_domain.c | 7 +
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 4 +-
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml | 1 -
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 3 +-
tests/qemuxml2argvtest.c | 2 +
.../qemuxml2xmlout-pcie-root.xml | 2 +-
tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 26 ++++
tests/qemuxml2xmltest.c | 2 +-
9 files changed, 189 insertions(+), 8 deletions(-)
create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 9caf036..337a637 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1686,6 +1686,18 @@ qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
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;
+
+ /* video cards aren't hot-plugged, and can be put in either a PCI
+ * or PCIe slot
+ */
+ if (device->type == VIR_DOMAIN_DEVICE_VIDEO)
+ flags = QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE;

/* Ignore implicit controllers on slot 0:0:1.0:
* implicit IDE controller on 0:0:1.1 (no qemu command line)
@@ -2315,6 +2327,133 @@ error:
}


+static bool
+qemuDomainMachineIsQ35(virDomainDefPtr def)
+{
+ return (STRPREFIX(def->os.machine, "pc-q35") ||
+ STREQ(def->os.machine, "q35"));
+}
+
+
+static int
+qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def,
+ virQEMUCapsPtr qemuCaps,
+ qemuDomainPCIAddressSetPtr addrs)
+{
+ size_t i;
+ virDevicePCIAddress tmp_addr;
+ bool qemuDeviceVideoUsable = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+ virDevicePCIAddressPtr addrptr;
+ 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 error;
+ }
+ } 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;
+ }
+ }
+ }
+
+ /* Reserve slot 0x1F function 0 (ISA bridge, not in config model)
+ * and function 3 (SMBus, also not (yet) in config model). As with
+ * the SATA controller, these devices are always present in a q35
+ * machine; there is no way to not have them.
+ */
+ if (addrs->nbuses) {
+ memset(&tmp_addr, 0, sizeof(tmp_addr));
+ tmp_addr.slot = 0x1F;
+ tmp_addr.function = 0;
+ tmp_addr.multi = 1;
+ if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags,
+ false, false) < 0)
+ goto error;
+ tmp_addr.function = 3;
+ tmp_addr.multi = 0;
+ if (qemuDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags,
+ false, false) < 0)
+ goto error;
+ }
+
+ if (def->nvideos > 0) {
+ /* NB: unlike the pc machinetypes, q35 machinetypes put the primary VGA
+ * at slot 1 for some reason.
+ */
+ virDomainVideoDefPtr primaryVideo = def->videos[0];
+ if (primaryVideo->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ primaryVideo->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+ primaryVideo->info.addr.pci.domain = 0;
+ primaryVideo->info.addr.pci.bus = 0;
+ primaryVideo->info.addr.pci.slot = 1;
+ primaryVideo->info.addr.pci.function = 0;
+ addrptr = &primaryVideo->info.addr.pci;
+
+ if (!qemuDomainPCIAddressValidate(addrs, addrptr, flags))
+ goto error;
+
+ if (qemuDomainPCIAddressSlotInUse(addrs, addrptr)) {
+ if (qemuDeviceVideoUsable) {
+ virResetLastError();
+ if (qemuDomainPCIAddressReserveNextSlot(addrs,
+ &primaryVideo->info,
+ flags) < 0)
+ goto error;
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("PCI address 0:0:1.0 is in use, "
+ "QEMU needs it for primary video"));
+ goto error;
+ }
+ } else if (qemuDomainPCIAddressReserveSlot(addrs, addrptr, flags) < 0) {
+ goto error;
+ }
+ } else if (!qemuDeviceVideoUsable) {
+ if (primaryVideo->info.addr.pci.domain != 0 ||
+ primaryVideo->info.addr.pci.bus != 0 ||
+ primaryVideo->info.addr.pci.slot != 1 ||
+ primaryVideo->info.addr.pci.function != 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Primary video card must have PCI address 0:0:1.0"));
+ goto error;
+ }
+ /* If TYPE==PCI, then qemuCollectPCIAddress() function
+ * has already reserved the address, so we must skip */
+ }
+ } else if (addrs->nbuses && !qemuDeviceVideoUsable) {
+ memset(&tmp_addr, 0, sizeof(tmp_addr));
+ tmp_addr.slot = 1;
+
+ if (qemuDomainPCIAddressSlotInUse(addrs, &tmp_addr)) {
+ VIR_DEBUG("PCI address 0:0:1.0 in use, future addition of a video"
+ " device will not be possible without manual"
+ " intervention");
+ virResetLastError();
+ } else if (qemuDomainPCIAddressReserveSlot(addrs, &tmp_addr, flags) < 0) {
+ goto error;
+ }
+ }
+ return 0;
+
+error:
+ return -1;
+}
+
+
/*
* This assigns static PCI slots to all configured devices.
* The ordering here is chosen to match the ordering used
@@ -2365,6 +2504,11 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
goto error;
}

+ if (qemuDomainMachineIsQ35(def) &&
+ qemuDomainValidateDevicePCISlotsQ35(def, qemuCaps, addrs) < 0) {
+ goto error;
+ }
+
/* PCI controllers */
for (i = 0; i < def->ncontrollers; i++) {
if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
@@ -7677,6 +7821,9 @@ qemuBuildCommandLine(virConnectPtr conn,
_("SATA is not supported with this "
"QEMU binary"));
goto error;
+ } else if (cont->idx == 0 && qemuDomainMachineIsQ35(def)) {
+ /* first SATA controller on Q35 machines is implicit */
+ continue;
} else {
char *devstr;

@@ -7690,6 +7837,7 @@ qemuBuildCommandLine(virConnectPtr conn,
}
} else if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_USB &&
cont->model == -1 &&
+ !qemuDomainMachineIsQ35(def) &&
(!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PIIX3_USB_UHCI) ||
def->os.arch == VIR_ARCH_PPC64)) {
if (usblegacy) {
@@ -7714,7 +7862,7 @@ qemuBuildCommandLine(virConnectPtr conn,
}
}

- if (usbcontroller == 0)
+ if (usbcontroller == 0 && !qemuDomainMachineIsQ35(def))
virCommandAddArg(cmd, "-usb");

for (i = 0; i < def->nhubs; i++) {
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f5030cd..75be591 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -700,6 +700,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
void *opaque ATTRIBUTE_UNUSED)
{
bool addDefaultUSB = true;
+ bool addImplicitSATA = false;
bool addPCIRoot = false;
bool addPCIeRoot = false;

@@ -722,6 +723,7 @@ qemuDomainDefPostParse(virDomainDefPtr def,
STREQ(def->os.machine, "q35")) {
addPCIeRoot = true;
addDefaultUSB = false;
+ addImplicitSATA = true;
break;
}
if (!STRPREFIX(def->os.machine, "pc-0.") &&
@@ -754,6 +756,11 @@ qemuDomainDefPostParse(virDomainDefPtr def,
def, VIR_DOMAIN_CONTROLLER_TYPE_USB, 0, -1) < 0)
return -1;

+ if (addImplicitSATA &&
+ virDomainDefMaybeAddController(
+ def, VIR_DOMAIN_CONTROLLER_TYPE_SATA, 0, -1) < 0)
+ return -1;
+
if (addPCIRoot &&
virDomainDefMaybeAddController(
def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index 75a9844..5f6323f 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -1,5 +1,5 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \
--device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 -usb
+-device i82801b11-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x2 \
+-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
index 1aa5455..d7fb90c 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml
@@ -15,7 +15,6 @@
<devices>
<emulator>/usr/libexec/qemu-kvm</emulator>
<controller type='pci' index='0' model='pcie-root'/>
- <controller type='usb' index='0'/>
<memballoon model='none'/>
</devices>
</domain>
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index 2c67739..2295e56 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -1,7 +1,6 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
/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-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \
+-device i82801b11-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x2 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
--usb \
-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index aba0f88..0068d27 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -995,11 +995,13 @@ mymain(void)
DO_TEST("pci-bridge-many-disks",
QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_PCI_BRIDGE);
DO_TEST("pcie-root",
+ QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
DO_TEST("q35",
QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
+ QEMU_CAPS_ICH9_AHCI,
QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
index 25c77f1..f10e85b 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
@@ -15,7 +15,7 @@
<devices>
<emulator>/usr/libexec/qemu-kvm</emulator>
<controller type='pci' index='0' model='pcie-root'/>
- <controller type='usb' index='0'/>
+ <controller type='sata' index='0'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'/>
<controller type='pci' index='2' model='pci-bridge'/>
<memballoon model='none'/>
diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
new file mode 100644
index 0000000..2a86e61
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
@@ -0,0 +1,26 @@
+<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>
+ <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='sata' index='0'/>
+ <video>
+ <model type='qxl' ram='65536' vram='18432' heads='1'/>
+ </video>
+ <memballoon model='none'/>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 8b4590a..5c6730d 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -295,7 +295,7 @@ mymain(void)
DO_TEST_DIFFERENT("pci-autoadd-addr");
DO_TEST_DIFFERENT("pci-autoadd-idx");
DO_TEST_DIFFERENT("pcie-root");
- DO_TEST("q35");
+ DO_TEST_DIFFERENT("q35");

DO_TEST("hostdev-scsi-lsi");
DO_TEST("hostdev-scsi-virtio-scsi");

[Laine Stump]

We had been setting the device alias in the devinceinfo for pci
controllers to "pci%u", but then hardcoding "pci.%u" when creating the
device address for other devices using that pci bus. This all worked
just fine until we encountered the built-in "pcie.0" bus (the PCIe
root complex) in Q35 machines.

In order to create the correct commandline for this one case, this
patch:

1) sets the alias for PCI controllers correctly, to "pci.%u" (or
"pcie.%u" for the pcie-root controller)

2) eliminates the hardcoded "pci.%u" for pci controllers when
generatuing device address strings, and instead uses the controller's
alias.

3) plumbs a pointer to the virDomainDef all the way down to
qemuBuildDeviceAddressStr. This was necessary in order to make the
aliase of the controller *used by a device* available (previously
qemuBuildDeviceAddressStr only had the deviceinfo of the device
itself, *not* of the controller it was connecting to). This made for a
larger than desired diff, but at least in the future we won't have to
do it again, since all the information we could possibly ever need for
future enhancements is in the virDomainDef. (right?)

This should be done for *all* controllers, but for now we just do it
in the case of PCI controllers, to reduce the likelyhood of
regression.
---
src/qemu/qemu_command.c | 164 ++++++++++++++-------
src/qemu/qemu_command.h | 28 ++--
src/qemu/qemu_hotplug.c | 6 +-
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
5 files changed, 136 insertions(+), 66 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 337a637..08544be 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -860,10 +860,18 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller)
{
const char *prefix = virDomainControllerTypeToString(controller->type);

- if (virAsprintf(&controller->info.alias, "%s%d", prefix,
- controller->idx) < 0)
- return -1;
- return 0;
+ if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
+ /* only pcie-root uses a different naming convention
+ * ("pcie.0"), because it is hardcoded that way in qemu. All
+ * other buses use the consistent "pci.%u".
+ */
+ if (controller->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)
+ return virAsprintf(&controller->info.alias, "pcie.%d", controller->idx);
+ else
+ return virAsprintf(&controller->info.alias, "pci.%d", controller->idx);
+ }
+
+ return virAsprintf(&controller->info.alias, "%s%d", prefix, controller->idx);
}

static ssize_t
@@ -2781,22 +2789,57 @@ qemuUsbId(virBufferPtr buf, int idx)

static int
qemuBuildDeviceAddressStr(virBufferPtr buf,
+ virDomainDefPtr domainDef,
virDomainDeviceInfoPtr info,
virQEMUCapsPtr qemuCaps)
{
+ int ret = -1;
+ char *devStr = NULL;
+
if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+ const char *contAlias = NULL;
+ size_t i;
+
+ if (!(devStr = qemuDomainPCIAddressAsString(&info->addr.pci)))
+ goto cleanup;
+ for (i = 0; i < domainDef->ncontrollers; i++) {
+ virDomainControllerDefPtr cont = domainDef->controllers[i];
+
+ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
+ cont->idx == info->addr.pci.bus) {
+ contAlias = cont->info.alias;
+ if (!contAlias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Device alias was not set for PCI "
+ "controller with index %u required "
+ "for device at address %s"),
+ info->addr.pci.bus, devStr);
+ goto cleanup;
+ }
+ break;
+ }
+ }
+ if (!contAlias) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Could not find PCI "
+ "controller with index %u required "
+ "for device at address %s"),
+ info->addr.pci.bus, devStr);
+ goto cleanup;
+ }
+
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) {
if (info->addr.pci.function != 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Only PCI device addresses with function=0 "
"are supported with this QEMU binary"));
- return -1;
+ goto cleanup;
}
if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'multifunction=on' is not supported with "
"this QEMU binary"));
- return -1;
+ goto cleanup;
}
}

@@ -2810,18 +2853,19 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
*/
if (info->addr.pci.bus != 0) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
- virBufferAsprintf(buf, ",bus=pci.%u", info->addr.pci.bus);
+ virBufferAsprintf(buf, ",bus=%s", contAlias);
} else {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Multiple PCI buses are not supported "
"with this QEMU binary"));
- return -1;
+ goto cleanup;
}
} else {
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS))
- virBufferAddLit(buf, ",bus=pci.0");
- else
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIBUS)) {
+ virBufferAsprintf(buf, ",bus=%s", contAlias);
+ } else {
virBufferAddLit(buf, ",bus=pci");
+ }
}
if (info->addr.pci.multi == VIR_DEVICE_ADDRESS_PCI_MULTI_ON)
virBufferAddLit(buf, ",multifunction=on");
@@ -2845,7 +2889,10 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
info->addr.ccw.devno);
}

- return 0;
+ ret = 0;
+cleanup:
+ VIR_FREE(devStr);
+ return ret;
}

static int
@@ -4179,13 +4226,13 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
(disk->device == VIR_DOMAIN_DISK_DEVICE_LUN)
? "on" : "off");
}
- if (qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
goto error;
break;
case VIR_DOMAIN_DISK_BUS_USB:
virBufferAddLit(&opt, "usb-storage");

- if (qemuBuildDeviceAddressStr(&opt, &disk->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&opt, def, &disk->info, qemuCaps) < 0)
goto error;
break;
default:
@@ -4309,7 +4356,8 @@ error:


char *
-qemuBuildFSDevStr(virDomainFSDefPtr fs,
+qemuBuildFSDevStr(virDomainDefPtr def,
+ virDomainFSDefPtr fs,
virQEMUCapsPtr qemuCaps)
{
virBuffer opt = VIR_BUFFER_INITIALIZER;
@@ -4325,7 +4373,7 @@ qemuBuildFSDevStr(virDomainFSDefPtr fs,
virBufferAsprintf(&opt, ",fsdev=%s%s", QEMU_FSDEV_HOST_PREFIX, fs->info.alias);
virBufferAsprintf(&opt, ",mount_tag=%s", fs->dst);

- if (qemuBuildDeviceAddressStr(&opt, &fs->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&opt, def, &fs->info, qemuCaps) < 0)
goto error;

if (virBufferError(&opt)) {
@@ -4546,7 +4594,7 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
if (def->queues)
virBufferAsprintf(&buf, ",num_queues=%u", def->queues);

- if (qemuBuildDeviceAddressStr(&buf, &def->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, domainDef, &def->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -4584,7 +4632,8 @@ qemuBuildNicStr(virDomainNetDefPtr net,


char *
-qemuBuildNicDevStr(virDomainNetDefPtr net,
+qemuBuildNicDevStr(virDomainDefPtr def,
+ virDomainNetDefPtr net,
int vlan,
int bootindex,
virQEMUCapsPtr qemuCaps)
@@ -4646,7 +4695,7 @@ qemuBuildNicDevStr(virDomainNetDefPtr net,
virBufferAsprintf(&buf, ",id=%s", net->info.alias);
virBufferAsprintf(&buf, ",mac=%s",
virMacAddrFormat(&net->mac, macaddr));
- if (qemuBuildDeviceAddressStr(&buf, &net->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &net->info, qemuCaps) < 0)
goto error;
if (qemuBuildRomStr(&buf, &net->info, qemuCaps) < 0)
goto error;
@@ -4801,7 +4850,8 @@ qemuBuildHostNetStr(virDomainNetDefPtr net,


char *
-qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
+qemuBuildWatchdogDevStr(virDomainDefPtr def,
+ virDomainWatchdogDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4814,7 +4864,7 @@ qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
}

virBufferAsprintf(&buf, "%s,id=%s", model, dev->info.alias);
- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -4831,7 +4881,8 @@ error:


char *
-qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
+qemuBuildMemballoonDevStr(virDomainDefPtr def,
+ virDomainMemballoonDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4851,7 +4902,7 @@ qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
}

virBufferAsprintf(&buf, ",id=%s", dev->info.alias);
- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -4894,7 +4945,8 @@ error:
}

char *
-qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
+qemuBuildUSBInputDevStr(virDomainDefPtr def,
+ virDomainInputDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4903,7 +4955,7 @@ qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
dev->type == VIR_DOMAIN_INPUT_TYPE_MOUSE ?
"usb-mouse" : "usb-tablet", dev->info.alias);

- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -4920,7 +4972,8 @@ error:


char *
-qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
+qemuBuildSoundDevStr(virDomainDefPtr def,
+ virDomainSoundDefPtr sound,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -4941,7 +4994,7 @@ qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
model = "intel-hda";

virBufferAsprintf(&buf, "%s,id=%s", model, sound->info.alias);
- if (qemuBuildDeviceAddressStr(&buf, &sound->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &sound->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -5001,7 +5054,8 @@ error:
}

static char *
-qemuBuildDeviceVideoStr(virDomainVideoDefPtr video,
+qemuBuildDeviceVideoStr(virDomainDefPtr def,
+ virDomainVideoDefPtr video,
virQEMUCapsPtr qemuCaps,
bool primary)
{
@@ -5055,7 +5109,7 @@ qemuBuildDeviceVideoStr(virDomainVideoDefPtr video,
virBufferAsprintf(&buf, ",vram_size=%u", video->vram * 1024);
}

- if (qemuBuildDeviceAddressStr(&buf, &video->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &video->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -5095,7 +5149,9 @@ qemuOpenPCIConfig(virDomainHostdevDefPtr dev)
}

char *
-qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd,
+qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
+ virDomainHostdevDefPtr dev,
+ const char *configfd,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5115,7 +5171,7 @@ qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev, const char *configfd,
virBufferAsprintf(&buf, ",id=%s", dev->info->alias);
if (dev->info->bootIndex)
virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex);
- if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
goto error;
if (qemuBuildRomStr(&buf, dev->info, qemuCaps) < 0)
goto error;
@@ -5221,7 +5277,7 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def,
virBufferAsprintf(&buf, ",bootindex=%d", dev->info.bootIndex);
}

- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -5237,7 +5293,8 @@ error:
}

char *
-qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
+qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
+ virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5260,7 +5317,7 @@ qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
if (dev->info->bootIndex)
virBufferAsprintf(&buf, ",bootindex=%d", dev->info->bootIndex);

- if (qemuBuildDeviceAddressStr(&buf, dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -5277,7 +5334,8 @@ error:


char *
-qemuBuildHubDevStr(virDomainHubDefPtr dev,
+qemuBuildHubDevStr(virDomainDefPtr def,
+ virDomainHubDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
virBuffer buf = VIR_BUFFER_INITIALIZER;
@@ -5297,7 +5355,7 @@ qemuBuildHubDevStr(virDomainHubDefPtr dev,

virBufferAddLit(&buf, "usb-hub");
virBufferAsprintf(&buf, ",id=%s", dev->info.alias);
- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto error;

if (virBufferError(&buf)) {
@@ -5818,6 +5876,7 @@ cleanup:

static int
qemuBuildRNGDeviceArgs(virCommandPtr cmd,
+ virDomainDefPtr def,
virDomainRNGDefPtr dev,
virQEMUCapsPtr qemuCaps)
{
@@ -5847,7 +5906,7 @@ qemuBuildRNGDeviceArgs(virCommandPtr cmd,
virBufferAddLit(&buf, ",period=1000");
}

- if (qemuBuildDeviceAddressStr(&buf, &dev->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&buf, def, &dev->info, qemuCaps) < 0)
goto cleanup;

virCommandAddArg(cmd, "-device");
@@ -7122,7 +7181,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
virCommandAddArgList(cmd, "-netdev", host, NULL);
}
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
- if (!(nic = qemuBuildNicDevStr(net, vlan, bootindex, qemuCaps)))
+ if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex, qemuCaps)))
goto cleanup;
virCommandAddArgList(cmd, "-device", nic, NULL);
} else {
@@ -7870,7 +7929,7 @@ qemuBuildCommandLine(virConnectPtr conn,
char *optstr;

virCommandAddArg(cmd, "-device");
- if (!(optstr = qemuBuildHubDevStr(hub, qemuCaps)))
+ if (!(optstr = qemuBuildHubDevStr(def, hub, qemuCaps)))
goto error;
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
@@ -8091,7 +8150,7 @@ qemuBuildCommandLine(virConnectPtr conn,
VIR_FREE(optstr);

virCommandAddArg(cmd, "-device");
- if (!(optstr = qemuBuildFSDevStr(fs, qemuCaps)))
+ if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps)))
goto error;
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
@@ -8449,7 +8508,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
char *optstr;
virCommandAddArg(cmd, "-device");
- if (!(optstr = qemuBuildUSBInputDevStr(input, qemuCaps)))
+ if (!(optstr = qemuBuildUSBInputDevStr(def, input, qemuCaps)))
goto error;
virCommandAddArg(cmd, optstr);
VIR_FREE(optstr);
@@ -8506,7 +8565,7 @@ qemuBuildCommandLine(virConnectPtr conn,
for (i = 0; i < def->nvideos; i++) {
char *str;
virCommandAddArg(cmd, "-device");
- if (!(str = qemuBuildDeviceVideoStr(def->videos[i], qemuCaps, !i)))
+ if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, !i)))
goto error;

virCommandAddArg(cmd, str);
@@ -8580,7 +8639,7 @@ qemuBuildCommandLine(virConnectPtr conn,

virCommandAddArg(cmd, "-device");

- if (!(str = qemuBuildDeviceVideoStr(def->videos[i], qemuCaps, false)))
+ if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], qemuCaps, false)))
goto error;

virCommandAddArg(cmd, str);
@@ -8644,7 +8703,7 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL);
} else {
virCommandAddArg(cmd, "-device");
- if (!(str = qemuBuildSoundDevStr(sound, qemuCaps)))
+ if (!(str = qemuBuildSoundDevStr(def, sound, qemuCaps)))
goto error;

virCommandAddArg(cmd, str);
@@ -8720,7 +8779,7 @@ qemuBuildCommandLine(virConnectPtr conn,
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
virCommandAddArg(cmd, "-device");

- optstr = qemuBuildWatchdogDevStr(watchdog, qemuCaps);
+ optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps);
if (!optstr)
goto error;
} else {
@@ -8833,7 +8892,7 @@ qemuBuildCommandLine(virConnectPtr conn,

if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) {
virCommandAddArg(cmd, "-device");
- if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev, qemuCaps)))
+ if (!(devstr = qemuBuildUSBHostdevDevStr(def, hostdev, qemuCaps)))
goto error;
virCommandAddArg(cmd, devstr);
VIR_FREE(devstr);
@@ -8881,7 +8940,7 @@ qemuBuildCommandLine(virConnectPtr conn,
}
}
virCommandAddArg(cmd, "-device");
- devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name, qemuCaps);
+ devstr = qemuBuildPCIHostdevDevStr(def, hostdev, configfd_name, qemuCaps);
VIR_FREE(configfd_name);
if (!devstr)
goto error;
@@ -9010,7 +9069,7 @@ qemuBuildCommandLine(virConnectPtr conn,
char *optstr;
virCommandAddArg(cmd, "-device");

- optstr = qemuBuildMemballoonDevStr(def->memballoon, qemuCaps);
+ optstr = qemuBuildMemballoonDevStr(def, def->memballoon, qemuCaps);
if (!optstr)
goto error;
virCommandAddArg(cmd, optstr);
@@ -9026,7 +9085,7 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;

/* add the device */
- if (qemuBuildRNGDeviceArgs(cmd, def->rng, qemuCaps) < 0)
+ if (qemuBuildRNGDeviceArgs(cmd, def, def->rng, qemuCaps) < 0)
goto error;
}

@@ -9104,6 +9163,7 @@ error:
*/
static int
qemuBuildSerialChrDeviceStr(char **deviceStr,
+ virDomainDefPtr def,
virDomainChrDefPtr serial,
virQEMUCapsPtr qemuCaps,
virArch arch,
@@ -9116,7 +9176,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
serial->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO) {
virBufferAsprintf(&cmd, "spapr-vty,chardev=char%s",
serial->info.alias);
- if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
goto error;
}
} else {
@@ -9138,7 +9198,7 @@ qemuBuildSerialChrDeviceStr(char **deviceStr,
goto error;
}

- if (qemuBuildDeviceAddressStr(&cmd, &serial->info, qemuCaps) < 0)
+ if (qemuBuildDeviceAddressStr(&cmd, def, &serial->info, qemuCaps) < 0)
goto error;
}
}
@@ -9253,7 +9313,7 @@ qemuBuildChrDeviceStr(char **deviceStr,

switch ((enum virDomainChrDeviceType) chr->deviceType) {
case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
- ret = qemuBuildSerialChrDeviceStr(deviceStr, chr, qemuCaps,
+ ret = qemuBuildSerialChrDeviceStr(deviceStr, vmdef, chr, qemuCaps,
vmdef->os.arch,
vmdef->os.machine);
break;
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index e5111d2..5c5c025 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -98,7 +98,8 @@ char * qemuBuildNicStr(virDomainNetDefPtr net,
int vlan);

/* Current, best practice */
-char * qemuBuildNicDevStr(virDomainNetDefPtr net,
+char * qemuBuildNicDevStr(virDomainDefPtr def,
+ virDomainNetDefPtr net,
int vlan,
int bootindex,
virQEMUCapsPtr qemuCaps);
@@ -119,7 +120,8 @@ char * qemuBuildDriveDevStr(virDomainDefPtr def,
virDomainDiskDefPtr disk,
int bootindex,
virQEMUCapsPtr qemuCaps);
-char * qemuBuildFSDevStr(virDomainFSDefPtr fs,
+char * qemuBuildFSDevStr(virDomainDefPtr domainDef,
+ virDomainFSDefPtr fs,
virQEMUCapsPtr qemuCaps);
/* Current, best practice */
char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
@@ -127,22 +129,27 @@ char * qemuBuildControllerDevStr(virDomainDefPtr domainDef,
virQEMUCapsPtr qemuCaps,
int *nusbcontroller);

-char * qemuBuildWatchdogDevStr(virDomainWatchdogDefPtr dev,
+char * qemuBuildWatchdogDevStr(virDomainDefPtr domainDef,
+ virDomainWatchdogDefPtr dev,
virQEMUCapsPtr qemuCaps);

-char * qemuBuildMemballoonDevStr(virDomainMemballoonDefPtr dev,
+char * qemuBuildMemballoonDevStr(virDomainDefPtr domainDef,
+ virDomainMemballoonDefPtr dev,
virQEMUCapsPtr qemuCaps);

-char * qemuBuildUSBInputDevStr(virDomainInputDefPtr dev,
+char * qemuBuildUSBInputDevStr(virDomainDefPtr domainDef,
+ virDomainInputDefPtr dev,
virQEMUCapsPtr qemuCaps);

-char * qemuBuildSoundDevStr(virDomainSoundDefPtr sound,
+char * qemuBuildSoundDevStr(virDomainDefPtr domainDef,
+ virDomainSoundDefPtr sound,
virQEMUCapsPtr qemuCaps);

/* Legacy, pre device support */
char * qemuBuildPCIHostdevPCIDevStr(virDomainHostdevDefPtr dev);
/* Current, best practice */
-char * qemuBuildPCIHostdevDevStr(virDomainHostdevDefPtr dev,
+char * qemuBuildPCIHostdevDevStr(virDomainDefPtr def,
+ virDomainHostdevDefPtr dev,
const char *configfd,
virQEMUCapsPtr qemuCaps);

@@ -151,7 +158,8 @@ int qemuOpenPCIConfig(virDomainHostdevDefPtr dev);
/* Legacy, pre device support */
char * qemuBuildUSBHostdevUsbDevStr(virDomainHostdevDefPtr dev);
/* Current, best practice */
-char * qemuBuildUSBHostdevDevStr(virDomainHostdevDefPtr dev,
+char * qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
+ virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);

char * qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev,
@@ -162,7 +170,9 @@ char * qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,
virDomainHostdevDefPtr dev,
virQEMUCapsPtr qemuCaps);

-char * qemuBuildHubDevStr(virDomainHubDefPtr dev, virQEMUCapsPtr qemuCaps);
+char * qemuBuildHubDevStr(virDomainDefPtr def,
+ virDomainHubDefPtr dev,
+ virQEMUCapsPtr qemuCaps);
char * qemuBuildRedirdevDevStr(virDomainDefPtr def,
virDomainRedirdevDefPtr dev,
virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 032de69..7a6946e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -859,7 +859,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
}

if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
- if (!(nicstr = qemuBuildNicDevStr(net, vlan, 0, priv->qemuCaps)))
+ if (!(nicstr = qemuBuildNicDevStr(vm->def, net, vlan, 0, priv->qemuCaps)))
goto try_remove;
} else {
if (!(nicstr = qemuBuildNicStr(net, NULL, vlan)))
@@ -1057,7 +1057,7 @@ int qemuDomainAttachHostPciDevice(virQEMUDriverPtr driver,
goto error;
}

- if (!(devstr = qemuBuildPCIHostdevDevStr(hostdev, configfd_name,
+ if (!(devstr = qemuBuildPCIHostdevDevStr(vm->def, hostdev, configfd_name,
priv->qemuCaps)))
goto error;

@@ -1302,7 +1302,7 @@ int qemuDomainAttachHostUsbDevice(virQEMUDriverPtr driver,
if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE)) {
if (qemuAssignDeviceHostdevAlias(vm->def, hostdev, -1) < 0)
goto cleanup;
- if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev, priv->qemuCaps)))
+ if (!(devstr = qemuBuildUSBHostdevDevStr(vm->def, hostdev, priv->qemuCaps)))
goto cleanup;
}

diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index 5f6323f..34cb876 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -1,5 +1,5 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x2 \
+-device i82801b11-pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index 2295e56..c7574b1 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -1,6 +1,6 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
/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-pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x2 \
+-device i82801b11-pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368

[Laine Stump]

Current seabios doesn't recognize any disk device that is attached
anywhere behind an i82801b11-bridge, so it can't boot from them. (It's
possible/probable that seabios simply doesn't recognize that bridge at
all, so *no* devices behind it are available to the bios). In order to
allow booting from any disk no matter where it is attached, this patch
flouts the hardware specs (which say that only PCIe devices can be
connected directly to a PCIe bus such as the q35 machine's "pcie.0"),
taking advantage of the fact that qemu also ignores this rule, and
installs a stnadard pci-bridge device anywhere the config requests a
"dmi-to-pci-bridge (which had been setup to use i82801b11-bridge).

Because this isn't really the way things should be, the original code
is left in place, and the new code is put inside an #ifdef indicating
why it is there.
---
This is one of the solutions to the problem that I proposed here:

https://www.redhat.com/archives/libvir-list/2013-August/msg00165.html

Since this is the one I currently prefer, I implemented it (and it works!)

src/qemu/qemu_command.c | 16 ++++++++++++++++
tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 +-
3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 3353c61..460144f 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4571,7 +4571,23 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
_("dmi-to-pci-bridge index should be > 0"));
goto error;
}
+#define SEABIOS_NO_I82801B11_SUPPORT
+#if defined SEABIOS_NO_I82801B11_SUPPORT
+ /* since seabios currently doesn't recognize boot devices
+ * that are anywhere behind an i82801b11-bridge, we take
+ * advantage of the fact that qemu erroneously
+ * (generously) allows us to plug a pci-bridge into a PCIe
+ * slot, and create a pci-bridge to take the place of the
+ * i82801b11-bridge. When seabios is fixed, we will figure
+ * out the best way to determine whether or not the fix is
+ * present, and switch to the more proper device when
+ * appropriate.
+ */
+ virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d",
+ def->idx, def->idx);
+#else
virBufferAsprintf(&buf, "i82801b11-bridge,id=pci.%d", def->idx);
+#endif
break;
case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
index 84428f9..da970db 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
@@ -1,5 +1,5 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /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 pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index 5ff4bc7..a0ec66e 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -1,6 +1,6 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
/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 pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \
-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368

[Laine Stump]

q35 machines have an implicit ahci (sata) controller at 00:1F.2 which
has no "id" associated with it. For this reason, we can't refer to it
as "ahci0". Instead, we don't give an id on the commandline, which
qemu interprets as "use the first ahci controller". We then need to
specify the unit with "unit=%d" rather than adding it onto the bus
arg.
---
src/qemu/qemu_command.c | 23 ++++++++++++++++++++---
tests/qemuxml2argvdata/qemuxml2argv-q35.args | 2 ++
tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 5 +++++
tests/qemuxml2argvtest.c | 2 +-
tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml | 5 +++++
5 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 460144f..cb25659 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4199,9 +4199,26 @@ qemuBuildDriveDevStr(virDomainDefPtr def,
virBufferAddLit(&opt, "ide-drive");
}

- virBufferAsprintf(&opt, ",bus=ahci%d.%d",
- disk->info.addr.drive.controller,
- disk->info.addr.drive.unit);
+ if (qemuDomainMachineIsQ35(def) &&
+ disk->info.addr.drive.controller == 0) {
+ /* Q35 machines have an implicit ahci (sata) controller at
+ * 00:1F.2 which has no "id" associated with it. For this
+ * reason, we can't refer to it as "ahci0". Instead, we
+ * don't give an id, which qemu interprets as "use the
+ * first ahci controller". We then need to specify the
+ * unit with "unit=%d" rather than adding it onto the bus
+ * arg.
+ */
+ virBufferAsprintf(&opt, ",unit=%d", disk->info.addr.drive.unit);
+ } else {
+ /* All other ahci controllers have been created by
+ * libvirt, so they *do* have an id, and we can identify
+ * them that way.
+ */
+ virBufferAsprintf(&opt, ",bus=ahci%d.%d",
+ disk->info.addr.drive.controller,
+ disk->info.addr.drive.unit);
+ }
break;
case VIR_DOMAIN_DISK_BUS_VIRTIO:
if (disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) {
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
index a0ec66e..6b30b4d 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
@@ -3,4 +3,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
-device pci-bridge,chassis_nr=1,id=pci.1,bus=pcie.0,addr=0x2 \
-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,unit=0,drive=drive-sata0-0-0,id=sata0-0-0 \
-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
index 3541b14..edaf6cb 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
@@ -14,6 +14,11 @@
<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'/>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 0068d27..679124e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1001,7 +1001,7 @@ mymain(void)
DO_TEST("q35",
QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
- QEMU_CAPS_ICH9_AHCI,
+ 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);

diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
index 2a86e61..96f8eaf 100644
--- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
+++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-q35.xml
@@ -14,6 +14,11 @@
<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'/>

[Laine Stump]

...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
end of this message:

# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)

You'll notice that everything except the VGA, the implicit devices are

Yay!

Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)

=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>

[Doug Goldstein]

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

[Laine Stump]

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

[Doug Goldstein]

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that. I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

[Laine Stump]

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that.

That took me a while to figure out too (and I was getting really worried
until I did). I'm just glad that Alex Williamson had previously told me
that qemu wasn't strict about the "can't plug a PCI device into a PCIe
slot" rule.

Post by Doug Goldstein
I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

Cool! The more real world testing the better! Thanks for taking the time
to do that. (My testing has been with an existing Fedora 15 guest that
was hanging around. Not sure why I picked that one; maybe because it was
the most disposable item on the list :-)

[Doug Goldstein]

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller
(rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that.

That took me a while to figure out too (and I was getting really worried
until I did). I'm just glad that Alex Williamson had previously told me
that qemu wasn't strict about the "can't plug a PCI device into a PCIe
slot" rule.

Post by Doug Goldstein
I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

Cool! The more real world testing the better! Thanks for taking the time
to do that. (My testing has been with an existing Fedora 15 guest that
was hanging around. Not sure why I picked that one; maybe because it was
the most disposable item on the list :-)

So with v2 I've achieved success as well with my CentOS 6.4 VM. Your
domain XML is shorter than mine (I'm using dumpxml --inactive) but
I'll post mine.

<domain type='kvm'>
<name>altima</name>
<uuid>c2ca3b04-2ae9-0dd2-9855-109dcd90e38c</uuid>
<description>CentOS 6.2 i386</description>
<memory unit='KiB'>524288</memory>
<currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup' track='guest'>
<catchup threshold='123' slew='120' limit='10000'/>
</timer>
<timer name='pit' tickpolicy='delay'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw'/>
<source dev='/dev/vms/altima.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x04'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<controller type='sata' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x1f'
function='0x2'/>
</controller>
<controller type='sata' index='1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x02'
function='0x0'/>
</controller>
<controller type='virtio-serial' index='0'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x03'
function='0x0'/>
</controller>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<address type='pci' domain='0x0000' bus='0x01' slot='0x01'
function='0x0'/>
</controller>
<controller type='usb' index='0' model='ich9-ehci1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x07'
function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x2'/>
</controller>
<controller type='ide' index='0'/>
<interface type='bridge'>
<mac address='52:54:00:a6:23:4f'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x01'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<source mode='bind' path='/var/lib/libvirt/qemu/altima.agent'/>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
<address type='virtio-serial' controller='0' bus='0' port='2'/>
</channel>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='spice' autoport='yes'/>
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0'/>
</memballoon>
</devices>
</domain>

[***@altima ~]# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
02:03.0 Communication controller: Red Hat, Inc Virtio console
02:04.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:05.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:06.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #1 (rev 03)
02:06.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #2 (rev 03)
02:06.2 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #3 (rev 03)


I did notice that the EHCI USB controller is missing on bus 2 slot 7
function 7. Not sure if I did something wrong there in my config or
we're still missing some of that plumbing.

[Doug Goldstein]

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that.

That took me a while to figure out too (and I was getting really worried
until I did). I'm just glad that Alex Williamson had previously told me
that qemu wasn't strict about the "can't plug a PCI device into a PCIe
slot" rule.

Post by Doug Goldstein
I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

Cool! The more real world testing the better! Thanks for taking the time
to do that. (My testing has been with an existing Fedora 15 guest that
was hanging around. Not sure why I picked that one; maybe because it was
the most disposable item on the list :-)

So with v2 I've achieved success as well with my CentOS 6.4 VM. Your
domain XML is shorter than mine (I'm using dumpxml --inactive) but
I'll post mine.
<domain type='kvm'>
<name>altima</name>
<uuid>c2ca3b04-2ae9-0dd2-9855-109dcd90e38c</uuid>
<description>CentOS 6.2 i386</description>
<memory unit='KiB'>524288</memory>
<currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup' track='guest'>
<catchup threshold='123' slew='120' limit='10000'/>
</timer>
<timer name='pit' tickpolicy='delay'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw'/>
<source dev='/dev/vms/altima.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x04'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<controller type='sata' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x1f'
function='0x2'/>
</controller>
<controller type='sata' index='1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x02'
function='0x0'/>
</controller>
<controller type='virtio-serial' index='0'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x03'
function='0x0'/>
</controller>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<address type='pci' domain='0x0000' bus='0x01' slot='0x01'
function='0x0'/>
</controller>
<controller type='usb' index='0' model='ich9-ehci1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x07'
function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x2'/>
</controller>
<controller type='ide' index='0'/>
<interface type='bridge'>
<mac address='52:54:00:a6:23:4f'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x01'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<source mode='bind' path='/var/lib/libvirt/qemu/altima.agent'/>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
<address type='virtio-serial' controller='0' bus='0' port='2'/>
</channel>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='spice' autoport='yes'/>
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0'/>
</memballoon>
</devices>
</domain>
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
02:03.0 Communication controller: Red Hat, Inc Virtio console
02:04.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:05.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:06.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #1 (rev 03)
02:06.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #2 (rev 03)
02:06.2 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #3 (rev 03)
I did notice that the EHCI USB controller is missing on bus 2 slot 7
function 7. Not sure if I did something wrong there in my config or
we're still missing some of that plumbing.
--
Doug Goldstein

Now for the ugly side...

You can see below I clearly screwed up the config with regards to the
bus, but I just wanted to see how it would react.

domain type='kvm'>
<name>altima</name>
<uuid>c2ca3b04-2ae9-0dd2-9855-109dcd90e38c</uuid>
<description>CentOS 6.2 i386</description>
<memory unit='KiB'>524288</memory>
<currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup' track='guest'>
<catchup threshold='123' slew='120' limit='10000'/>
</timer>
<timer name='pit' tickpolicy='delay'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw'/>
<source dev='/dev/vms/altima.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x04'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='virtio-serial' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x06'
function='0x0'/>
</controller>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='bridge'>
<mac address='52:54:00:a6:23:4f'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x03'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<source mode='bind' path='/var/lib/libvirt/qemu/altima.agent'/>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
<address type='virtio-serial' controller='0' bus='0' port='2'/>
</channel>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='spice' autoport='yes'/>
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x05'
function='0x0'/>
</memballoon>
</devices>
</domain>

Results in the following:

error: internal error: PCI bus 0000:00 is not compatible with the
device. Device requires a standard PCI slot, which is not provided by
this bus

Two issues:
- Not sure I'd really call that a VIR_ERR_INTERNAL_ERROR.
- What device? I screwed them all up. Might be nice to provide some
info about the device in the error message.

That was in patch 3/7 from the original series, which is already pushed.

[Laine Stump]

Post by Doug Goldstein
I did notice that the EHCI USB controller is missing on bus 2 slot 7
function 7. Not sure if I did something wrong there in my config or
we're still missing some of that plumbing.

Yeah, that doesn't seem good.

Post by Doug Goldstein
error: internal error: PCI bus 0000:00 is not compatible with the
device. Device requires a standard PCI slot, which is not provided by
this bus
- Not sure I'd really call that a VIR_ERR_INTERNAL_ERROR.

It's supposed to switch that to VIR_ERR_XML_ERROR when the address was
explicit in the config...

Post by Doug Goldstein
- What device? I screwed them all up. Might be nice to provide some
info about the device in the error message.

see my response to your other message on the subject. More useful error
messages would definitely be good. I can't do that this week, though, as
I'll be traveling > 1/2 the time.

[Doug Goldstein]

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that.

That took me a while to figure out too (and I was getting really worried
until I did). I'm just glad that Alex Williamson had previously told me
that qemu wasn't strict about the "can't plug a PCI device into a PCIe
slot" rule.

Post by Doug Goldstein
I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

Cool! The more real world testing the better! Thanks for taking the time
to do that. (My testing has been with an existing Fedora 15 guest that
was hanging around. Not sure why I picked that one; maybe because it was
the most disposable item on the list :-)

So with v2 I've achieved success as well with my CentOS 6.4 VM. Your
domain XML is shorter than mine (I'm using dumpxml --inactive) but
I'll post mine.
<domain type='kvm'>
<name>altima</name>
<uuid>c2ca3b04-2ae9-0dd2-9855-109dcd90e38c</uuid>
<description>CentOS 6.2 i386</description>
<memory unit='KiB'>524288</memory>
<currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup' track='guest'>
<catchup threshold='123' slew='120' limit='10000'/>
</timer>
<timer name='pit' tickpolicy='delay'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw'/>
<source dev='/dev/vms/altima.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x04'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<controller type='sata' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x1f'
function='0x2'/>
</controller>
<controller type='sata' index='1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x02'
function='0x0'/>
</controller>
<controller type='virtio-serial' index='0'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x03'
function='0x0'/>
</controller>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<address type='pci' domain='0x0000' bus='0x01' slot='0x01'
function='0x0'/>
</controller>
<controller type='usb' index='0' model='ich9-ehci1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x07'
function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x2'/>
</controller>
<controller type='ide' index='0'/>
<interface type='bridge'>
<mac address='52:54:00:a6:23:4f'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x01'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<source mode='bind' path='/var/lib/libvirt/qemu/altima.agent'/>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
<address type='virtio-serial' controller='0' bus='0' port='2'/>
</channel>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='spice' autoport='yes'/>
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0'/>
</memballoon>
</devices>
</domain>
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
02:03.0 Communication controller: Red Hat, Inc Virtio console
02:04.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:05.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:06.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #1 (rev 03)
02:06.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #2 (rev 03)
02:06.2 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #3 (rev 03)
I did notice that the EHCI USB controller is missing on bus 2 slot 7
function 7. Not sure if I did something wrong there in my config or
we're still missing some of that plumbing.
--
Doug Goldstein

Promise I'm done spamming the list after this. Try to remove all the
USB controllers and libvirt will still let you define the domain,
which is wrong given the USB based tablet for mouse input. When you
try to start the domain it will fail with the following:

qemu-system-x86_64: -device usb-tablet,id=input0: No 'usb-bus' bus
found for device 'usb-tablet'

So we obviously need to ensure there is a USB controller defined and
stop relying on the implicit one but that's outside of the scope of
q35 work since technically this affects i440fx as well.

[Laine Stump]

Post by Doug Goldstein

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump

Post by Doug Goldstein

Post by Laine Stump
...and here is the result of applying all 10 patches of this 7 patch
series, and starting up a domain using the config file attached to the
# lspci
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM
Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 Ethernet controller: Red Hat, Inc Virtio network device
02:03.0 Multimedia audio controller: Intel Corporation 82801AA AC'97
Audio Controller (rev 01)
02:04.0 Communication controller: Red Hat, Inc Virtio console
02:05.0 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #1 (rev 03)
02:05.1 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #2 (rev 03)
02:05.2 USB Controller: Intel Corporation 82801I (ICH9 Family) USB UHCI
Controller #3 (rev 03)
02:05.7 USB Controller: Intel Corporation 82801I (ICH9 Family) USB2 EHCI
Controller #1 (rev 03)
02:06.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:07.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:08.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH) 6
port SATA AHCI Controller (rev 02)
You'll notice that everything except the VGA, the implicit devices are
Yay!
Now if virt-manager just provided a way to change the machinetype of
guests as they were being created... (does virt-manager explicitly
specify USB controllers? Currently the Q35 doesn't automatically create
a USB controller (see the patch comments)
=========
<domain type='kvm'>
<name>F15-q35</name>
<memory unit='KiB'>1048576</memory>
<currentMemory unit='KiB'>1048576</currentMemory>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
<bootmenu enable='yes'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'/>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='file' device='disk'>
<driver name='qemu' type='raw'/>
<source file='/var/lib/libvirt/images/F15.img'/>
<target dev='vda' bus='virtio'/>
</disk>
<disk type='file' device='cdrom'>
<driver name='qemu' type='raw'/>
<source file='/home/laine/example.iso'/>
<target dev='hdc' bus='sata'/>
<readonly/>
<address type='drive' controller='0' bus='0' target='0' unit='0'/>
</disk>
<controller type='virtio-serial' index='0'/>
<controller type='usb' index='0' model='ich9-ehci1'/>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x2'/>
</controller>
<controller type='sata' index='0'/>
<controller type='sata' index='1'/>
<controller type='pci' index='0' model='pcie-root'/>
<interface type='network'>
<source network='ipv6'/>
<model type='virtio'/>
</interface>
<interface type='network'>
<source network='isolated'/>
<model type='virtio'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<input type='mouse' bus='ps2'/>
<graphics type='vnc' port='-1' autoport='yes'/>
<sound model='ac97'/>
<video>
<model type='qxl' ram='65536' vram='9216' heads='1'/>
</video>
<memballoon model='virtio'/>
</devices>
</domain>
--
libvir-list mailing list
https://www.redhat.com/mailman/listinfo/libvir-list

Excellent thanks for providing a reference example. I've been trying
to review all your patches and work through testing bits and pieces
myself. I should have probably spoke up on the ML rather than
remaining silent. I'll finish up testing it tonight and ACK a handful
shortly.

I have made some small changes to some of the other patches in the
meantime. If you're serious to the point of actually testing them out, I
should repost the ones I haven't yet pushed (only the first three).
Coming up momentarily...

Yeah please repost. Until you mentioned the seabios stuff in your last
e-mail I had been debugging that.

That took me a while to figure out too (and I was getting really worried
until I did). I'm just glad that Alex Williamson had previously told me
that qemu wasn't strict about the "can't plug a PCI device into a PCIe
slot" rule.

Post by Doug Goldstein
I've got a CentOS 6.4 VM I'm trying
to get up under Q35 on qemu-1.5.2 + libvirt master + your patch set as
part of my review.

Cool! The more real world testing the better! Thanks for taking the time
to do that. (My testing has been with an existing Fedora 15 guest that
was hanging around. Not sure why I picked that one; maybe because it was
the most disposable item on the list :-)

So with v2 I've achieved success as well with my CentOS 6.4 VM. Your
domain XML is shorter than mine (I'm using dumpxml --inactive) but
I'll post mine.
<domain type='kvm'>
<name>altima</name>
<uuid>c2ca3b04-2ae9-0dd2-9855-109dcd90e38c</uuid>
<description>CentOS 6.2 i386</description>
<memory unit='KiB'>524288</memory>
<currentMemory unit='KiB'>524288</currentMemory>
<vcpu placement='static'>1</vcpu>
<os>
<type arch='x86_64' machine='pc-q35-1.5'>hvm</type>
<boot dev='hd'/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<clock offset='utc'>
<timer name='rtc' tickpolicy='catchup' track='guest'>
<catchup threshold='123' slew='120' limit='10000'/>
</timer>
<timer name='pit' tickpolicy='delay'/>
</clock>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>restart</on_crash>
<devices>
<emulator>/usr/bin/qemu-kvm</emulator>
<disk type='block' device='disk'>
<driver name='qemu' type='raw'/>
<source dev='/dev/vms/altima.img'/>
<target dev='vda' bus='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x04'
function='0x0'/>
</disk>
<disk type='block' device='cdrom'>
<driver name='qemu' type='raw'/>
<target dev='hdc' bus='ide'/>
<readonly/>
<address type='drive' controller='0' bus='1' target='0' unit='0'/>
</disk>
<controller type='sata' index='0'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x1f'
function='0x2'/>
</controller>
<controller type='sata' index='1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x02'
function='0x0'/>
</controller>
<controller type='virtio-serial' index='0'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x03'
function='0x0'/>
</controller>
<controller type='pci' index='0' model='pcie-root'/>
<controller type='pci' index='1' model='dmi-to-pci-bridge'>
<address type='pci' domain='0x0000' bus='0x00' slot='0x02'
function='0x0'/>
</controller>
<controller type='pci' index='2' model='pci-bridge'>
<address type='pci' domain='0x0000' bus='0x01' slot='0x01'
function='0x0'/>
</controller>
<controller type='usb' index='0' model='ich9-ehci1'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x07'
function='0x7'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci1'>
<master startport='0'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x0' multifunction='on'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci2'>
<master startport='2'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x1'/>
</controller>
<controller type='usb' index='0' model='ich9-uhci3'>
<master startport='4'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x06'
function='0x2'/>
</controller>
<controller type='ide' index='0'/>
<interface type='bridge'>
<mac address='52:54:00:a6:23:4f'/>
<source bridge='br0'/>
<model type='virtio'/>
<address type='pci' domain='0x0000' bus='0x02' slot='0x01'
function='0x0'/>
</interface>
<serial type='pty'>
<target port='0'/>
</serial>
<console type='pty'>
<target type='serial' port='0'/>
</console>
<channel type='unix'>
<source mode='bind' path='/var/lib/libvirt/qemu/altima.agent'/>
<target type='virtio' name='org.qemu.guest_agent.0'/>
<address type='virtio-serial' controller='0' bus='0' port='1'/>
</channel>
<channel type='spicevmc'>
<target type='virtio' name='com.redhat.spice.0'/>
<address type='virtio-serial' controller='0' bus='0' port='2'/>
</channel>
<input type='tablet' bus='usb'/>
<input type='mouse' bus='ps2'/>
<graphics type='spice' autoport='yes'/>
<video>
<model type='qxl' ram='65536' vram='65536' heads='1'/>
<address type='pci' domain='0x0000' bus='0x00' slot='0x01'
function='0x0'/>
</video>
<memballoon model='virtio'>
<address type='pci' domain='0x0000' bus='0x02' slot='0x05'
function='0x0'/>
</memballoon>
</devices>
</domain>
00:00.0 Host bridge: Intel Corporation 82G33/G31/P35/P31 Express DRAM Controller
00:01.0 VGA compatible controller: Red Hat, Inc. Device 0100 (rev 04)
00:02.0 PCI bridge: Red Hat, Inc. Device 0001
00:1f.0 ISA bridge: Intel Corporation 82801IB (ICH9) LPC Interface
Controller (rev 02)
00:1f.2 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
00:1f.3 SMBus: Intel Corporation 82801I (ICH9 Family) SMBus Controller (rev 02)
01:01.0 PCI bridge: Red Hat, Inc. Device 0001
02:01.0 Ethernet controller: Red Hat, Inc Virtio network device
02:02.0 SATA controller: Intel Corporation 82801IR/IO/IH (ICH9R/DO/DH)
6 port SATA Controller [AHCI mode] (rev 02)
02:03.0 Communication controller: Red Hat, Inc Virtio console
02:04.0 SCSI storage controller: Red Hat, Inc Virtio block device
02:05.0 Unclassified device [00ff]: Red Hat, Inc Virtio memory balloon
02:06.0 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #1 (rev 03)
02:06.1 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #2 (rev 03)
02:06.2 USB controller: Intel Corporation 82801I (ICH9 Family) USB
UHCI Controller #3 (rev 03)
I did notice that the EHCI USB controller is missing on bus 2 slot 7
function 7. Not sure if I did something wrong there in my config or
we're still missing some of that plumbing.
--
Doug Goldstein

Promise I'm done spamming the list after this. Try to remove all the
USB controllers and libvirt will still let you define the domain,
which is wrong given the USB based tablet for mouse input. When you
qemu-system-x86_64: -device usb-tablet,id=input0: No 'usb-bus' bus
found for device 'usb-tablet'
So we obviously need to ensure there is a USB controller defined and
stop relying on the implicit one but that's outside of the scope of
q35 work since technically this affects i440fx as well.

That's taken care of for i440fx - when no usb device is defined, the
post-parse will automatically add:

<controller type='usb' index='0'/>

which equates to the simple usb1 controller that's part of the PIIX3
chipset that's included in the i440fx emulation. This works out to
adding "-usb" to the qemu commandline.


For q35, you can also add a "default" usb controller with "-usb", but it
actually adds 3 controllers, at function 1, 2, and 7 on slot 0x1D of
pcie.0. I wasn't comfortable with the idea of a single "<controller
type='usb' index='0'> adding 3 different devices, and wasn't sure if we
should handle this by instead adding a usb1 controller (seems kind of
ugly for q35) or adding three controllers to the config, so I punted by
not adding anything, so that whatever we finally decide to do will be
compatible with domains that are created in the meantime.

Right now I'm thinking that what we should do is: if there are *0* usb
controllers present during the post-parse callback, we should add all
three of the usb controllers to the config (*not* just a single
<controller type='usb' index='0'/>"), possibly on pcie.0 at slot 0x1D
(nice, but not necessary, and it would take some hacking because 1)
currently we have no way to suggesting what bus/slot to use in the
MaybeAddController function, and 2) usb devices are all classified as
PCI, and currently can't be plugged into pcie.0).

Realistically speaking, I can't do any of that in the next week, and
what is here is functional without closing the door to having an
auto-added usb setup in the future.

[Gerd Hoffmann]

Hi,

Post by Laine Stump
1) Although the implicit pcie-root controller on q35 machinetypes is
referred to as "pcie.0" by qemu, the code as it stands now still puts
"pci.0" on the commandline, which means it's still unusable. I'm
working on a patch to fix that right now.

FYI: /me has this hack sprinkled into my xml files for q35 testing:

<qemu:arg value='-set'/>
<qemu:arg value='device.scsi0.bus=pcie.0'/>
<qemu:arg value='-set'/>
<qemu:arg value='device.net0.bus=pcie.0'/>
<qemu:arg value='-set'/>
<qemu:arg value='device.video0.bus=pcie.0'/>
<qemu:arg value='-set'/>
<qemu:arg value='device.sound0.bus=pcie.0'/>
<qemu:arg value='-set'/>
<qemu:arg value='device.balloon0.bus=pcie.0'/>

cheers,
Gerd