Somewhere around commit 9ff9d9f reserving entire PCI slots was
eliminated, as demonstrated by commit 6cc2014.
Reserve the functions required by the implicit devices:
00:01.0 ISA Bridge
00:01.1 IDE Controller
00:01.2 USB Controller (unless USB is disabled)
00:01.3 Bridge
https://bugzilla.redhat.com/show_bug.cgi?id=1460143
---
src/qemu/qemu_domain_address.c | 15 ++++++++++---
.../qemuxml2argv-440fx-ide-address-conflict.xml | 25 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 38 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-440fx-ide-address-conflict.xml
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index 69c0c8bf2..f35e01e5a 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1429,15 +1429,24 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
cont->info.addr.pci.slot = 1;
cont->info.addr.pci.function = 2;
}
+ } else {
+ /* this controller is not skipped in qemuDomainCollectPCIAddress */
+ continue;
}
+ if (addrs->nbuses &&
+ virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
+ goto cleanup;
}
- /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller)
- * hardcoded slot=1, multifunction device
- */
+ /* Implicit PIIX3 devices living on slot 1 not handled above */
if (addrs->nbuses) {
memset(&tmp_addr, 0, sizeof(tmp_addr));
tmp_addr.slot = 1;
+ /* ISA Bridge at 00:01.0 */
+ if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
+ goto cleanup;
+ /* Bridge at 00:01.3 */
+ tmp_addr.function = 3;
if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, 0) < 0)
goto cleanup;
}
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-440fx-ide-address-conflict.xml b/tests/qemuxml2argvdata/qemuxml2argv-440fx-ide-address-conflict.xml
new file mode 100644
index 000000000..4195880aa
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-440fx-ide-address-conflict.xml
@@ -0,0 +1,25 @@
+<domain type='qemu'>
+ <name>QEMUGuest1</name>
+ <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+ <memory unit='KiB'>219100</memory>
+ <currentMemory unit='KiB'>219100</currentMemory>
+ <os>
+ <type arch='i686' machine='pc'>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/bin/qemu-system-i686</emulator>
+ <controller type='usb' index='0'/>
+ <controller type='ide' index='0'/>
+ <controller type='pci' index='0' model='pci-root'/>
+ <input type='mouse' bus='ps2'/>
+ <input type='keyboard' bus='ps2'/>
+ <memballoon model='virtio'>
+ <address type='pci' slot='1' function='1'/>
+ </memballoon>
+ </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 70be0c32d..5a16760cc 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2249,6 +2249,7 @@ mymain(void)
QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
QEMU_CAPS_DEVICE_QXL);
DO_TEST_PARSE_ERROR("440fx-wrong-root", NONE);
+ DO_TEST_PARSE_ERROR("440fx-ide-address-conflict", NONE);
DO_TEST_PARSE_ERROR("pcie-root-port-too-many",
QEMU_CAPS_DEVICE_IOH3420,
--
2.13.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/26/2017 07:05 AM, Ján Tomko wrote: > Somewhere around commit 9ff9d9f reserving entire PCI slots was > eliminated, as demonstrated by commit 6cc2014. > > Reserve the functions required by the implicit devices: > 00:01.0 ISA Bridge > 00:01.1 IDE Controller > 00:01.2 USB Controller (unless USB is disabled) > 00:01.3 Bridge > > https://bugzilla.redhat.com/show_bug.cgi?id=1460143 > --- > src/qemu/qemu_domain_address.c | 15 ++++++++++--- > .../qemuxml2argv-440fx-ide-address-conflict.xml | 25 ++++++++++++++++++++++ > tests/qemuxml2argvtest.c | 1 + > 3 files changed, 38 insertions(+), 3 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-440fx-ide-address-conflict.xml > Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2017-09-26 at 13:05 +0200, Ján Tomko wrote:
> @@ -1429,15 +1429,24 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> cont->info.addr.pci.slot = 1;
> cont->info.addr.pci.function = 2;
> }
> + } else {
> + /* this controller is not skipped in qemuDomainCollectPCIAddress */
> + continue;
> }
> + if (addrs->nbuses &&
> + virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
> + goto cleanup;
I think it would make more sense to reserve the address for these
devices in qemuDomainCollectPCIAddress() directly, along with the
other ones, eg. squash in the following diff:
---------- 8< ---------- 8< ---------- 8< ---------- 8< ----------
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index f35e01e5a..ecb91c565 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1265,13 +1265,12 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
_("Bus 0 must be PCI for integrated PIIX3 "
"USB or IDE controllers"));
return -1;
- } else {
- return 0;
}
}
}
- if (virDomainPCIAddressReserveAddr(addrs, addr,
+ if (addrs->nbuses &&
+ virDomainPCIAddressReserveAddr(addrs, addr,
info->pciConnectFlags,
info->isolationGroup) < 0) {
goto cleanup;
@@ -1429,13 +1428,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
cont->info.addr.pci.slot = 1;
cont->info.addr.pci.function = 2;
}
- } else {
- /* this controller is not skipped in qemuDomainCollectPCIAddress */
- continue;
}
- if (addrs->nbuses &&
- virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
- goto cleanup;
}
/* Implicit PIIX3 devices living on slot 1 not handled above */
---------- >8 ---------- >8 ---------- >8 ---------- >8 ----------
What do you think about it?
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Oct 11, 2017 at 01:14:58PM +0200, Andrea Bolognani wrote:
>On Tue, 2017-09-26 at 13:05 +0200, Ján Tomko wrote:
>> @@ -1429,15 +1429,24 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
>> cont->info.addr.pci.slot = 1;
>> cont->info.addr.pci.function = 2;
>> }
>> + } else {
>> + /* this controller is not skipped in qemuDomainCollectPCIAddress */
>> + continue;
>> }
>> + if (addrs->nbuses &&
>> + virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
>> + goto cleanup;
>
>I think it would make more sense to reserve the address for these
>devices in qemuDomainCollectPCIAddress() directly, along with the
>other ones, eg. squash in the following diff:
>
>---------- 8< ---------- 8< ---------- 8< ---------- 8< ----------
>diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
>index f35e01e5a..ecb91c565 100644
>--- a/src/qemu/qemu_domain_address.c
>+++ b/src/qemu/qemu_domain_address.c
>@@ -1265,13 +1265,12 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> _("Bus 0 must be PCI for integrated PIIX3 "
> "USB or IDE controllers"));
> return -1;
>- } else {
>- return 0;
> }
> }
> }
>
>- if (virDomainPCIAddressReserveAddr(addrs, addr,
>+ if (addrs->nbuses &&
>+ virDomainPCIAddressReserveAddr(addrs, addr,
> info->pciConnectFlags,
> info->isolationGroup) < 0) {
> goto cleanup;
>@@ -1429,13 +1428,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> cont->info.addr.pci.slot = 1;
> cont->info.addr.pci.function = 2;
> }
>- } else {
>- /* this controller is not skipped in qemuDomainCollectPCIAddress */
>- continue;
> }
>- if (addrs->nbuses &&
>- virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
>- goto cleanup;
> }
>
> /* Implicit PIIX3 devices living on slot 1 not handled above */
>---------- >8 ---------- >8 ---------- >8 ---------- >8 ----------
>
>What do you think about it?
>
I think I cannot possibly reserve the address before setting it.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, 2017-10-11 at 14:45 +0200, Ján Tomko wrote:
> > I think it would make more sense to reserve the address for these
> > devices in qemuDomainCollectPCIAddress() directly, along with the
> > other ones, eg. squash in the following diff:
> >
> > ---------- 8< ---------- 8< ---------- 8< ---------- 8< ----------
> > diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> > index f35e01e5a..ecb91c565 100644
> > --- a/src/qemu/qemu_domain_address.c
> > +++ b/src/qemu/qemu_domain_address.c
> > @@ -1265,13 +1265,12 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED,
> > _("Bus 0 must be PCI for integrated PIIX3 "
> > "USB or IDE controllers"));
> > return -1;
> > - } else {
> > - return 0;
> > }
> > }
> > }
> >
> > - if (virDomainPCIAddressReserveAddr(addrs, addr,
> > + if (addrs->nbuses &&
> > + virDomainPCIAddressReserveAddr(addrs, addr,
> > info->pciConnectFlags,
> > info->isolationGroup) < 0) {
> > goto cleanup;
> > @@ -1429,13 +1428,7 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def,
> > cont->info.addr.pci.slot = 1;
> > cont->info.addr.pci.function = 2;
> > }
> > - } else {
> > - /* this controller is not skipped in qemuDomainCollectPCIAddress */
> > - continue;
> > }
> > - if (addrs->nbuses &&
> > - virDomainPCIAddressReserveAddr(addrs, &cont->info.addr.pci, flags, 0) < 0)
> > - goto cleanup;
> > }
> >
> > /* Implicit PIIX3 devices living on slot 1 not handled above */
> > ---------- >8 ---------- >8 ---------- >8 ---------- >8 ----------
> >
> > What do you think about it?
>
> I think I cannot possibly reserve the address before setting it.
qemuDomainCollectPCIAddress() is called by
qemuDomainPCIAddressSetCreate(), which is called both before and
after calling qemuDomainValidateDevicePCISlotsChipsets() - which
in turn calls qemuDomainValidateDevicePCISlotsPIIX3().
So if the controllers already had been assigned a PCI address, then
it was stored in the domain XML and it will be picked up during the
first loop, reserved in qemuDomainCollectPCIAddress() and validated
by qemuDomainValidateDevicePCISlotsPIIX3(); if it has never been
assigned a PCI address, then it will be assigned the correct one by
qemuDomainValidateDevicePCISlotsPIIX3() and that address will be
reserved the second time qemuDomainCollectPCIAddress() is called.
So it looks to me like it would work. Or did I miss something?
--
Andrea Bolognani / Red Hat / Virtualization
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.