[libvirt] [PATCH] reserve slot 1 on pci bus0

After supporting multi function pci device, we only reserve function 1 on slot 1. The user can use the other function on slot 1 in the xml config file. We should detect this wrong usage. --- src/qemu/qemu_command.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 287ad8d..4b5734c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1072,6 +1072,7 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) int i; bool reservedIDE = false; bool reservedVGA = false; + int function; /* Host bridge */ if (qemuDomainPCIAddressReserveSlot(addrs, 0) < 0) @@ -1107,9 +1108,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) * hardcoded slot=1, multifunction device */ - if (!reservedIDE && - qemuDomainPCIAddressReserveSlot(addrs, 1) < 0) - goto error; + for (function = 0; function <= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + if (function == 1 && reservedIDE) + /* we have reserved this pci address */ + continue; + + if (qemuDomainPCIAddressReserveFunction(addrs, 1, function) < 0) + goto error; + } /* First VGA is hardcoded slot=2 */ if (def->nvideos > 0) { -- 1.7.1

On 08/23/2011 08:47 PM, Wen Congyang wrote:
After supporting multi function pci device, we only reserve function 1 on slot 1. The user can use the other function on slot 1 in the xml config file. We should detect this wrong usage.
--- src/qemu/qemu_command.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
@@ -1107,9 +1108,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) * hardcoded slot=1, multifunction device */ - if (!reservedIDE&& - qemuDomainPCIAddressReserveSlot(addrs, 1)< 0) - goto error; + for (function = 0; function<= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
Isn't this an off-by-one? Compare commit 7ae740fc. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 08/31/2011 06:28 AM, Eric Blake Write:
On 08/23/2011 08:47 PM, Wen Congyang wrote:
After supporting multi function pci device, we only reserve function 1 on slot 1. The user can use the other function on slot 1 in the xml config file. We should detect this wrong usage.
--- src/qemu/qemu_command.c | 12 +++++++++--- 1 files changed, 9 insertions(+), 3 deletions(-)
@@ -1107,9 +1108,14 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, qemuDomainPCIAddressSetPtr addrs) /* PIIX3 (ISA bridge, IDE controller, something else unknown, USB controller) * hardcoded slot=1, multifunction device */ - if (!reservedIDE&& - qemuDomainPCIAddressReserveSlot(addrs, 1)< 0) - goto error; + for (function = 0; function<= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
Isn't this an off-by-one? Compare commit 7ae740fc.
Yes, it should be 'function < QEMU_PCI_ADDRESS_LAST_FUNCTION' Thanks Wen Congyang

On 08/30/2011 06:16 PM, Wen Congyang wrote:
At 08/31/2011 06:28 AM, Eric Blake Write:
On 08/23/2011 08:47 PM, Wen Congyang wrote:
After supporting multi function pci device, we only reserve function 1 on slot 1. The user can use the other function on slot 1 in the xml config file. We should detect this wrong usage.
+ for (function = 0; function<= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
Isn't this an off-by-one? Compare commit 7ae740fc.
Yes, it should be 'function< QEMU_PCI_ADDRESS_LAST_FUNCTION'
I've now had a closer look at your patch, and it looks reasonable. ACK with that change. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

At 08/31/2011 08:59 AM, Eric Blake Write:
On 08/30/2011 06:16 PM, Wen Congyang wrote:
At 08/31/2011 06:28 AM, Eric Blake Write:
On 08/23/2011 08:47 PM, Wen Congyang wrote:
After supporting multi function pci device, we only reserve function 1 on slot 1. The user can use the other function on slot 1 in the xml config file. We should detect this wrong usage.
+ for (function = 0; function<= QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) {
Isn't this an off-by-one? Compare commit 7ae740fc.
Yes, it should be 'function< QEMU_PCI_ADDRESS_LAST_FUNCTION'
I've now had a closer look at your patch, and it looks reasonable.
ACK with that change.
Pushed with that fixed Thanks
participants (2)
-
Eric Blake
-
Wen Congyang