[libvirt] [PATCH 0/2] qemu: make PCI multifunction support more manual

Problems have been encountered/realized with the practice of unconditionally setting the multifunction bit for all functions of all devices. PATCH 2/2 remedies that (details in its own commit comment). PATCH 1/2 is a one-liner I fixed in the meantime which will cause a simple conflict if it's not applied to other branches at the same time as PATCH 2/2, so I'm sending them together.

While adding a new enum, I noticed a VIR_ENUM_DECL for a type that doesn't exist. There is also of course no matching VIR_ENUM_IMPL for it. --- src/conf/domain_conf.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0bc0042..4a99ea4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1830,7 +1830,6 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) -VIR_ENUM_DECL(virDomainDeviceAddressMode) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) -- 1.7.3.4

On Fri, Sep 30, 2011 at 01:40:45AM -0400, Laine Stump wrote:
While adding a new enum, I noticed a VIR_ENUM_DECL for a type that doesn't exist. There is also of course no matching VIR_ENUM_IMPL for it. --- src/conf/domain_conf.h | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 0bc0042..4a99ea4 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1830,7 +1830,6 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) -VIR_ENUM_DECL(virDomainDeviceAddressMode) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus)
ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

When support for was added for PCI multifunction cards (in commit 9f8baf, first included in libvirt 0.9.3), it was done by always turning on the multifunction bit for all PCI devices. Since that time it has been realized that this is not an ideal solution, and that the multifunction bit must be selectively turned on. For example, see https://bugzilla.redhat.com/show_bug.cgi?id=728174 and the discussion before and after https://www.redhat.com/archives/libvir-list/2011-September/msg01036.html This patch modifies multifunction support so that the multifunction=on option is only added to the qemu commandline for a device if its PCI <address> definition has the attribute "multifunction='on'", e.g.: <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> In practice, the multifunction bit should only be turned on if function='0' AND other functions will be used in the same slot - it usually isn't needed for functions 1-7 (although there are apparently some exceptions, e.g. the Intel X53 according to the QEMU source code), and should never be set if only function 0 will be used in the slot. The test cases have been changed accordingly to illustrate. With this patch in place, it is possible for a user to assign multiple functions in a slot without setting the multifunction bit for function 0; in this case, qemu will issue an error at startup time. In the future, we may decide to detect this situation and automatically add multifunction=on to avoid the error; even then it will still be useful to have a manual method of turning on multifunction since, as stated above, there are some devices that excpect it to be turned on for all functions in a slot. --- docs/formatdomain.html.in | 29 +++++++++++++------ docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 22 ++++++++++++++- src/conf/domain_conf.h | 10 +++++++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 18 ++++++++---- .../qemuxml2argv-multifunction-pci-device.args | 18 ++++++------ .../qemuxml2argv-multifunction-pci-device.xml | 6 ++-- .../qemuxml2argv-usb-ich9-companion.args | 10 +++--- .../qemuxml2argv-usb-ich9-companion.xml | 2 +- .../qemuxml2argv-usb-ich9-ehci-addr.args | 2 +- .../qemuxml2argv-usb-piix3-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 10 +++--- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 20 +++++++------- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 4 +- 16 files changed, 111 insertions(+), 54 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 49a2c09..6308ebc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1118,10 +1118,14 @@ The <code>type</code> attribute is mandatory, and is typically "pci" or "drive". For a "pci" controller, additional attributes for <code>bus</code>, <code>slot</code>, - and <code>function</code> must be present, as well as an - optional <code>domain</code>. For a "drive" controller, - additional attributes <code>controller</code>, <code>bus</code>, + and <code>function</code> must be present, as well as + optional <code>domain</code> + and <code>multifunction</code><span class="since">since + 0.9.7, requires QEMU 0.13</span> (defaults to 'off'). For a + "drive" controller, additional attributes + <code>controller</code>, <code>bus</code>, and <code>unit</code> are available, each defaulting to 0. + </dd> </dl> @@ -1298,7 +1302,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> ... </devices> @@ -1431,10 +1435,16 @@ with <code>virsh nodedev-list</code>. The <code>bus</code> attribute allows the hexadecimal values 0 to ff, the <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and - the <code>function</code> attribute allows the hexadecimal values 0 to - 7. There is also an optional <code>domain</code> attribute for the - PCI domain, with hexadecimal values 0 to ffff, but it is currently - not used by qemu.</dd> + the <code>function</code> attribute allows the hexadecimal values 0 to 7. + The <code>multifunction</code> attribute controls turning on the + multifunction bit for a particular slot/function in the PCI + control register<span class="since">since 0.9.7, requires QEMU + 0.13</span>. <code>multifunction</code> defaults to 'off', but + should be set to 'on' for function 0 of a slot that will have + multiple functions used. + There is also an optional <code>domain</code> attribute for + the PCI domain, with hexadecimal values 0 to ffff, but it is + currently not used by qemu.</dd> </dl> <h4><a name="elementsRedir">Redirected devices</a></h4> @@ -1602,7 +1612,8 @@ the interface to a particular pci slot, with attribute <code>type='pci'</code> and additional attributes <code>domain</code>, <code>bus</code>, <code>slot</code>, - and <code>function</code> as appropriate. + <code>function</code>, and <code>multifunction</code> + <span class="since">since 0.9.7, requires QEMU 0.13</span> as appropriate. </p> <h5><a name="elementsNICSVirtual">Virtual network</a></h5> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4972fac..cffaac2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2118,6 +2118,14 @@ <attribute name="function"> <ref name="pciFunc"/> </attribute> + <optional> + <attribute name="multifunction"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </define> <define name="driveaddress"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c141982..48e0985 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -138,6 +138,12 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "ccid", "usb") +VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", @@ -1652,6 +1658,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.pci.bus, info->addr.pci.slot, info->addr.pci.function); + if (info->addr.pci.multi) { + virBufferAsprintf(buf, " multifunction='%s'", + virDomainDeviceAddressPciMultiTypeToString(info->addr.pci.multi)); + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: @@ -1696,7 +1706,7 @@ static int virDomainDevicePCIAddressParseXML(xmlNodePtr node, virDomainDevicePCIAddressPtr addr) { - char *domain, *slot, *bus, *function; + char *domain, *slot, *bus, *function, *multi; int ret = -1; memset(addr, 0, sizeof(*addr)); @@ -1705,6 +1715,7 @@ virDomainDevicePCIAddressParseXML(xmlNodePtr node, bus = virXMLPropString(node, "bus"); slot = virXMLPropString(node, "slot"); function = virXMLPropString(node, "function"); + multi = virXMLPropString(node, "multifunction"); if (domain && virStrToLong_ui(domain, NULL, 0, &addr->domain) < 0) { @@ -1734,6 +1745,14 @@ virDomainDevicePCIAddressParseXML(xmlNodePtr node, goto cleanup; } + if (multi && + ((addr->multi = virDomainDeviceAddressPciMultiTypeFromString(multi)) < 0)) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown value '%s' for <address> 'multifunction' attribute"), + multi); + goto cleanup; + + } if (!virDomainDevicePCIAddressIsValid(addr)) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Insufficient specification for PCI address")); @@ -1747,6 +1766,7 @@ cleanup: VIR_FREE(bus); VIR_FREE(slot); VIR_FREE(function); + VIR_FREE(multi); return ret; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4a99ea4..bc41d34 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -74,6 +74,14 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; +enum virDomainDeviceAddressPciMulti { + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_DEFAULT = 0, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF, + + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST +}; + typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress; typedef virDomainDevicePCIAddress *virDomainDevicePCIAddressPtr; struct _virDomainDevicePCIAddress { @@ -81,6 +89,7 @@ struct _virDomainDevicePCIAddress { unsigned int bus; unsigned int slot; unsigned int function; + int multi; /* enum virDomainDeviceAddressPciMulti */ }; typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress; @@ -1830,6 +1839,7 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) +VIR_ENUM_DECL(virDomainDeviceAddressPciMulti) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2a3fab..1ac486f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -269,6 +269,8 @@ virDomainDefParseNode; virDomainDefParseString; virDomainDeleteConfig; virDomainDeviceAddressIsValid; +virDomainDeviceAddressPciMultiTypeFromString; +virDomainDeviceAddressPciMultiTypeToString; virDomainDeviceAddressTypeToString; virDomainDeviceDefFree; virDomainDeviceDefParse; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a13ba71..1704cf6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1376,7 +1376,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, if (info->addr.pci.function != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Only PCI device addresses with function=0 " - "are supported")); + "are supported with this QEMU binary")); + return -1; + } + if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'multifunction=on' is not supported with " + "this QEMU binary")); return -1; } } @@ -1391,11 +1397,11 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, virBufferAsprintf(buf, ",bus=pci.0"); else virBufferAsprintf(buf, ",bus=pci"); - if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) - virBufferAsprintf(buf, ",multifunction=on,addr=0x%x.0x%x", - info->addr.pci.slot, info->addr.pci.function); - else - virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) + virBufferAddLit(buf, ",multifunction=on"); + virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.function != 0) + virBufferAsprintf(buf, ".0x%x", info->addr.pci.function); } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virBufferAsprintf(buf, ",bus="); qemuUsbId(buf, info->addr.usb.bus); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args index ff229f2..ee85cda 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args @@ -1,15 +1,15 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ --device lsi,id=scsi0,bus=pci.0,multifunction=on,addr=0x3.0x0 \ --device lsi,id=scsi1,bus=pci.0,multifunction=on,addr=0x4.0x0 \ +-device lsi,id=scsi0,bus=pci.0,addr=0x3 \ +-device lsi,id=scsi1,bus=pci.0,multifunction=on,addr=0x4 \ -device lsi,id=scsi2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device lsi,id=scsi3,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device lsi,id=scsi4,bus=pci.0,multifunction=on,addr=0x4.0x3 \ --device lsi,id=scsi5,bus=pci.0,multifunction=on,addr=0x4.0x4 \ --device lsi,id=scsi6,bus=pci.0,multifunction=on,addr=0x4.0x5 \ --device lsi,id=scsi7,bus=pci.0,multifunction=on,addr=0x4.0x6 \ --device lsi,id=scsi8,bus=pci.0,multifunction=on,addr=0x4.0x7 \ +-device lsi,id=scsi3,bus=pci.0,addr=0x4.0x2 \ +-device lsi,id=scsi4,bus=pci.0,addr=0x4.0x3 \ +-device lsi,id=scsi5,bus=pci.0,addr=0x4.0x4 \ +-device lsi,id=scsi6,bus=pci.0,addr=0x4.0x5 \ +-device lsi,id=scsi7,bus=pci.0,addr=0x4.0x6 \ +-device lsi,id=scsi8,bus=pci.0,addr=0x4.0x7 \ -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0 \ -device scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 \ --usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x5.0x0 +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml index 672fb61..24b95b8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml @@ -20,13 +20,13 @@ <address type='drive' controller='0' bus='0' unit='0'/> </disk> <controller type='scsi' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0' multifunction='off'/> </controller> <controller type='scsi' index='1'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> </controller> <controller type='scsi' index='2'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1' multifunction='on'/> </controller> <controller type='scsi' index='3'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args index 1007544..fea6c35 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args @@ -1,6 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml index 05a6adf..5a43638 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml @@ -15,7 +15,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> <controller type='usb' index='0' model='ich9-uhci2'> <master startport='2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args index 0059ab5..94a4602 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args index 06863bb..546d119 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device piix3-usb-uhci,id=usb,bus=pci.0,multifunction=on,addr=0x1.0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args index f6270d5..7d34c2a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args @@ -1,10 +1,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ -chardev socket,id=charredir0,host=localhost,port=4000 \ -device usb-redir,chardev=charredir0,id=redir0 \ -chardev spicevmc,id=charredir1,name=usbredir \ -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml index 1dac3fb..a359a3d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml @@ -19,7 +19,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> </controller> <controller type='usb' index='0' model='ich9-uhci2'> <master startport='2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args index be4a78e..580cace 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args @@ -1,15 +1,15 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device piix3-usb-uhci,id=usb,bus=pci.0,multifunction=on,addr=0x1.0x2 \ --device ich9-usb-ehci1,id=usb1,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb1.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb1.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device ich9-usb-ehci1,id=usb2,bus=pci.0,multifunction=on,addr=0x5.0x7 \ --device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5.0x0 \ --device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.0,multifunction=on,addr=0x5.0x1 \ --device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.0,multifunction=on,addr=0x5.0x2 \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-device ich9-usb-ehci1,id=usb1,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb1.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb1.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-device ich9-usb-ehci1,id=usb2,bus=pci.0,addr=0x5.0x7 \ +-device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 \ +-device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.0,addr=0x5.0x1 \ +-device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.0,addr=0x5.0x2 \ -device usb-hub,id=hub0,bus=usb1.0,port=1 \ -device usb-tablet,id=input0,bus=usb.0,port=2 \ -device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=usb2.0,port=1 \ -device usb-host,hostbus=14,hostaddr=7,id=hostdev1,bus=usb2.0,port=2 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml index e8ada4d..b12b841 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml @@ -21,7 +21,7 @@ </controller> <controller type='usb' index='1' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> <controller type='usb' index='1' model='ich9-uhci2'> <master startport='2'/> @@ -37,7 +37,7 @@ </controller> <controller type='usb' index='2' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='5' function='0'/> + <address type='pci' domain='0' bus='0' slot='5' function='0' multifunction='on'/> </controller> <controller type='usb' index='2' model='ich9-uhci2'> <master startport='2'/> -- 1.7.3.4

On Fri, Sep 30, 2011 at 01:40:46AM -0400, Laine Stump wrote:
When support for was added for PCI multifunction cards (in commit 9f8baf, first included in libvirt 0.9.3), it was done by always turning on the multifunction bit for all PCI devices. Since that time it has been realized that this is not an ideal solution, and that the multifunction bit must be selectively turned on. For example, see
https://bugzilla.redhat.com/show_bug.cgi?id=728174
and the discussion before and after
https://www.redhat.com/archives/libvir-list/2011-September/msg01036.html
This patch modifies multifunction support so that the multifunction=on option is only added to the qemu commandline for a device if its PCI <address> definition has the attribute "multifunction='on'", e.g.:
<address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> [...] diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4972fac..cffaac2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2118,6 +2118,14 @@ <attribute name="function"> <ref name="pciFunc"/> </attribute> + <optional> + <attribute name="multifunction"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </define> <define name="driveaddress"> <optional>
okay [...]
+VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", @@ -1652,6 +1658,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.pci.bus, info->addr.pci.slot, info->addr.pci.function); [...]
+ if (multi && + ((addr->multi = virDomainDeviceAddressPciMultiTypeFromString(multi)) < 0)) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown value '%s' for <address> 'multifunction' attribute"), + multi); + goto cleanup; + + }
This code allows mutifunction="default" input if you make the test <= 0 then it should reject "default" as expected...
if (!virDomainDevicePCIAddressIsValid(addr)) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Insufficient specification for PCI address"));
[...]
@@ -1391,11 +1397,11 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, virBufferAsprintf(buf, ",bus=pci.0"); else virBufferAsprintf(buf, ",bus=pci"); - if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) - virBufferAsprintf(buf, ",multifunction=on,addr=0x%x.0x%x", - info->addr.pci.slot, info->addr.pci.function); - else - virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) + virBufferAddLit(buf, ",multifunction=on");
Hum seems to me that if the users explicitely specified mutifunction="off" then that ought to be saved, and hence else if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF) virBufferAddLit(buf, ",multifunction=off"); need to be added (since it's not the default which is 0 that won't pollute XML where it's not specified).
+ virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.function != 0) + virBufferAsprintf(buf, ".0x%x", info->addr.pci.function); } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virBufferAsprintf(buf, ",bus="); qemuUsbId(buf, info->addr.usb.bus);
ACK with those 2 problems fixed Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

(V2: address Daniel Veillard's two review points (don't allow "multifunction='default'", and add "multifunction=off" to the qemu commandline when that's what the XML says), and modify the checks for duplicate PCI address usage attempts to account for multifunction=off on a device's function 0, per IRC discussion with Dan Berrange (see new qemuCollectPCIAddress()). As a side effect, attempts to re-use the same PCI address are now logged rather than silently generating an error.) When support for was added for PCI multifunction cards (in commit 9f8baf, first included in libvirt 0.9.3), it was done by always turning on the multifunction bit for all PCI devices. Since that time it has been realized that this is not an ideal solution, and that the multifunction bit must be selectively turned on. For example, see https://bugzilla.redhat.com/show_bug.cgi?id=728174 and the discussion before and after https://www.redhat.com/archives/libvir-list/2011-September/msg01036.html This patch modifies multifunction support so that the multifunction=on option is only added to the qemu commandline for a device if its PCI <address> definition has the attribute "multifunction='on'", e.g.: <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> In practice, the multifunction bit should only be turned on if function='0' AND other functions will be used in the same slot - it usually isn't needed for functions 1-7 (although there are apparently some exceptions, e.g. the Intel X53 according to the QEMU source code), and should never be set if only function 0 will be used in the slot. The test cases have been changed accordingly to illustrate. With this patch in place, if a user attempts to assign multiple functions in a slot without setting the multifunction bit for function 0, libvirt will issue an error when the domain is defined, and the define operation will fail. In the future, we may decide to detect this situation and automatically add multifunction=on to avoid the error; even then it will still be useful to have a manual method of turning on multifunction since, as stated above, there are some devices that excpect it to be turned on for all functions in a slot. A side effect of this patch is that attempts to use the same PCI address for two different devices will now log an error (previously this would cause the domain define operation to fail, but there would be no log message generated). Because the function doing this log was almost completely rewritten, I didn't think it worthwhile to make a separate patch for that fix (the entire patch would immediately be obsoleted). --- docs/formatdomain.html.in | 29 +++++-- docs/schemas/domaincommon.rng | 8 ++ src/conf/domain_conf.c | 22 +++++- src/conf/domain_conf.h | 10 +++ src/libvirt_private.syms | 2 + src/qemu/qemu_command.c | 81 ++++++++++++++++---- .../qemuxml2argv-multifunction-pci-device.args | 18 ++-- .../qemuxml2argv-multifunction-pci-device.xml | 6 +- .../qemuxml2argv-usb-ich9-companion.args | 10 +- .../qemuxml2argv-usb-ich9-companion.xml | 2 +- .../qemuxml2argv-usb-ich9-ehci-addr.args | 2 +- .../qemuxml2argv-usb-piix3-controller.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args | 10 +- tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml | 2 +- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args | 20 +++--- tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml | 4 +- 16 files changed, 165 insertions(+), 63 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 49a2c09..6308ebc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1118,10 +1118,14 @@ The <code>type</code> attribute is mandatory, and is typically "pci" or "drive". For a "pci" controller, additional attributes for <code>bus</code>, <code>slot</code>, - and <code>function</code> must be present, as well as an - optional <code>domain</code>. For a "drive" controller, - additional attributes <code>controller</code>, <code>bus</code>, + and <code>function</code> must be present, as well as + optional <code>domain</code> + and <code>multifunction</code><span class="since">since + 0.9.7, requires QEMU 0.13</span> (defaults to 'off'). For a + "drive" controller, additional attributes + <code>controller</code>, <code>bus</code>, and <code>unit</code> are available, each defaulting to 0. + </dd> </dl> @@ -1298,7 +1302,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> ... </devices> @@ -1431,10 +1435,16 @@ with <code>virsh nodedev-list</code>. The <code>bus</code> attribute allows the hexadecimal values 0 to ff, the <code>slot</code> attribute allows the hexadecimal values 0 to 1f, and - the <code>function</code> attribute allows the hexadecimal values 0 to - 7. There is also an optional <code>domain</code> attribute for the - PCI domain, with hexadecimal values 0 to ffff, but it is currently - not used by qemu.</dd> + the <code>function</code> attribute allows the hexadecimal values 0 to 7. + The <code>multifunction</code> attribute controls turning on the + multifunction bit for a particular slot/function in the PCI + control register<span class="since">since 0.9.7, requires QEMU + 0.13</span>. <code>multifunction</code> defaults to 'off', but + should be set to 'on' for function 0 of a slot that will have + multiple functions used. + There is also an optional <code>domain</code> attribute for + the PCI domain, with hexadecimal values 0 to ffff, but it is + currently not used by qemu.</dd> </dl> <h4><a name="elementsRedir">Redirected devices</a></h4> @@ -1602,7 +1612,8 @@ the interface to a particular pci slot, with attribute <code>type='pci'</code> and additional attributes <code>domain</code>, <code>bus</code>, <code>slot</code>, - and <code>function</code> as appropriate. + <code>function</code>, and <code>multifunction</code> + <span class="since">since 0.9.7, requires QEMU 0.13</span> as appropriate. </p> <h5><a name="elementsNICSVirtual">Virtual network</a></h5> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 4972fac..cffaac2 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2118,6 +2118,14 @@ <attribute name="function"> <ref name="pciFunc"/> </attribute> + <optional> + <attribute name="multifunction"> + <choice> + <value>on</value> + <value>off</value> + </choice> + </attribute> + </optional> </define> <define name="driveaddress"> <optional> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c141982..6fb1888 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -138,6 +138,12 @@ VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST, "ccid", "usb") +VIR_ENUM_IMPL(virDomainDeviceAddressPciMulti, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST, + "default", + "on", + "off") + VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST, "block", "file", @@ -1652,6 +1658,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.pci.bus, info->addr.pci.slot, info->addr.pci.function); + if (info->addr.pci.multi) { + virBufferAsprintf(buf, " multifunction='%s'", + virDomainDeviceAddressPciMultiTypeToString(info->addr.pci.multi)); + } break; case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE: @@ -1696,7 +1706,7 @@ static int virDomainDevicePCIAddressParseXML(xmlNodePtr node, virDomainDevicePCIAddressPtr addr) { - char *domain, *slot, *bus, *function; + char *domain, *slot, *bus, *function, *multi; int ret = -1; memset(addr, 0, sizeof(*addr)); @@ -1705,6 +1715,7 @@ virDomainDevicePCIAddressParseXML(xmlNodePtr node, bus = virXMLPropString(node, "bus"); slot = virXMLPropString(node, "slot"); function = virXMLPropString(node, "function"); + multi = virXMLPropString(node, "multifunction"); if (domain && virStrToLong_ui(domain, NULL, 0, &addr->domain) < 0) { @@ -1734,6 +1745,14 @@ virDomainDevicePCIAddressParseXML(xmlNodePtr node, goto cleanup; } + if (multi && + ((addr->multi = virDomainDeviceAddressPciMultiTypeFromString(multi)) <= 0)) { + virDomainReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unknown value '%s' for <address> 'multifunction' attribute"), + multi); + goto cleanup; + + } if (!virDomainDevicePCIAddressIsValid(addr)) { virDomainReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Insufficient specification for PCI address")); @@ -1747,6 +1766,7 @@ cleanup: VIR_FREE(bus); VIR_FREE(slot); VIR_FREE(function); + VIR_FREE(multi); return ret; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 4a99ea4..bc41d34 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -74,6 +74,14 @@ enum virDomainDeviceAddressType { VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST }; +enum virDomainDeviceAddressPciMulti { + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_DEFAULT = 0, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON, + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF, + + VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_LAST +}; + typedef struct _virDomainDevicePCIAddress virDomainDevicePCIAddress; typedef virDomainDevicePCIAddress *virDomainDevicePCIAddressPtr; struct _virDomainDevicePCIAddress { @@ -81,6 +89,7 @@ struct _virDomainDevicePCIAddress { unsigned int bus; unsigned int slot; unsigned int function; + int multi; /* enum virDomainDeviceAddressPciMulti */ }; typedef struct _virDomainDeviceDriveAddress virDomainDeviceDriveAddress; @@ -1830,6 +1839,7 @@ VIR_ENUM_DECL(virDomainLifecycle) VIR_ENUM_DECL(virDomainLifecycleCrash) VIR_ENUM_DECL(virDomainDevice) VIR_ENUM_DECL(virDomainDeviceAddress) +VIR_ENUM_DECL(virDomainDeviceAddressPciMulti) VIR_ENUM_DECL(virDomainDisk) VIR_ENUM_DECL(virDomainDiskDevice) VIR_ENUM_DECL(virDomainDiskBus) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c2a3fab..1ac486f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -269,6 +269,8 @@ virDomainDefParseNode; virDomainDefParseString; virDomainDeleteConfig; virDomainDeviceAddressIsValid; +virDomainDeviceAddressPciMultiTypeFromString; +virDomainDeviceAddressPciMultiTypeToString; virDomainDeviceAddressTypeToString; virDomainDeviceDefFree; virDomainDeviceDefParse; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index a13ba71..ff83e2d 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -774,22 +774,65 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainDeviceInfoPtr dev, void *opaque) { + int ret = -1; + char *addr = NULL; qemuDomainPCIAddressSetPtr addrs = opaque; - if (dev->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - char *addr = qemuPCIAddressAsString(dev); - if (!addr) - return -1; + if (dev->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + return 0; - VIR_DEBUG("Remembering PCI addr %s", addr); + addr = qemuPCIAddressAsString(dev); + if (!addr) + goto cleanup; - if (virHashAddEntry(addrs->used, addr, addr) < 0) { - VIR_FREE(addr); - return -1; + if (virHashLookup(addrs->used, addr)) { + if (dev->addr.pci.function != 0) { + qemuReportError(VIR_ERR_XML_ERROR, + _("Attempted double use of PCI Address '%s' " + "(may need \"multifunction='on'\" for device on function 0"), + addr); + } else { + qemuReportError(VIR_ERR_XML_ERROR, + _("Attempted double use of PCI Address '%s'"), addr); } + goto cleanup; } - return 0; + VIR_DEBUG("Remembering PCI addr %s", addr); + if (virHashAddEntry(addrs->used, addr, addr) < 0) + goto cleanup; + addr = NULL; + + if ((dev->addr.pci.function == 0) && + (dev->addr.pci.multi != VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON)) { + /* a function 0 w/o multifunction=on must reserve the entire slot */ + int function; + virDomainDeviceInfo temp_dev = *dev; + + for (function = 1; function < QEMU_PCI_ADDRESS_LAST_FUNCTION; function++) { + temp_dev.addr.pci.function = function; + addr = qemuPCIAddressAsString(&temp_dev); + if (!addr) + goto cleanup; + + if (virHashLookup(addrs->used, addr)) { + qemuReportError(VIR_ERR_XML_ERROR, + _("Attempted double use of PCI Address '%s'" + "(need \"multifunction='off'\" for device on function 0)"), + addr); + goto cleanup; + } + + VIR_DEBUG("Remembering PCI addr %s (multifunction=off for function 0)", addr); + if (virHashAddEntry(addrs->used, addr, addr)) + goto cleanup; + addr = NULL; + } + } + ret = 0; +cleanup: + VIR_FREE(addr); + return ret; } @@ -1376,7 +1419,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, if (info->addr.pci.function != 0) { qemuReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Only PCI device addresses with function=0 " - "are supported")); + "are supported with this QEMU binary")); + return -1; + } + if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) { + qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("'multifunction=on' is not supported with " + "this QEMU binary")); return -1; } } @@ -1391,11 +1440,13 @@ qemuBuildDeviceAddressStr(virBufferPtr buf, virBufferAsprintf(buf, ",bus=pci.0"); else virBufferAsprintf(buf, ",bus=pci"); - if (qemuCapsGet(qemuCaps, QEMU_CAPS_PCI_MULTIFUNCTION)) - virBufferAsprintf(buf, ",multifunction=on,addr=0x%x.0x%x", - info->addr.pci.slot, info->addr.pci.function); - else - virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_ON) + virBufferAddLit(buf, ",multifunction=on"); + else if (info->addr.pci.multi == VIR_DOMAIN_DEVICE_ADDRESS_PCI_MULTI_OFF) + virBufferAddLit(buf, ",multifunction=off"); + virBufferAsprintf(buf, ",addr=0x%x", info->addr.pci.slot); + if (info->addr.pci.function != 0) + virBufferAsprintf(buf, ".0x%x", info->addr.pci.function); } else if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB) { virBufferAsprintf(buf, ",bus="); qemuUsbId(buf, info->addr.usb.bus); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args index ff229f2..8a2150e 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.args @@ -1,15 +1,15 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M \ pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults \ -monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ --device lsi,id=scsi0,bus=pci.0,multifunction=on,addr=0x3.0x0 \ --device lsi,id=scsi1,bus=pci.0,multifunction=on,addr=0x4.0x0 \ +-device lsi,id=scsi0,bus=pci.0,multifunction=off,addr=0x3 \ +-device lsi,id=scsi1,bus=pci.0,multifunction=on,addr=0x4 \ -device lsi,id=scsi2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device lsi,id=scsi3,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device lsi,id=scsi4,bus=pci.0,multifunction=on,addr=0x4.0x3 \ --device lsi,id=scsi5,bus=pci.0,multifunction=on,addr=0x4.0x4 \ --device lsi,id=scsi6,bus=pci.0,multifunction=on,addr=0x4.0x5 \ --device lsi,id=scsi7,bus=pci.0,multifunction=on,addr=0x4.0x6 \ --device lsi,id=scsi8,bus=pci.0,multifunction=on,addr=0x4.0x7 \ +-device lsi,id=scsi3,bus=pci.0,addr=0x4.0x2 \ +-device lsi,id=scsi4,bus=pci.0,addr=0x4.0x3 \ +-device lsi,id=scsi5,bus=pci.0,addr=0x4.0x4 \ +-device lsi,id=scsi6,bus=pci.0,addr=0x4.0x5 \ +-device lsi,id=scsi7,bus=pci.0,addr=0x4.0x6 \ +-device lsi,id=scsi8,bus=pci.0,addr=0x4.0x7 \ -drive file=/tmp/scsidisk.img,if=none,id=drive-scsi0-0-0 \ -device scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 \ --usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x5.0x0 +-usb -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x5 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml index 672fb61..24b95b8 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-multifunction-pci-device.xml @@ -20,13 +20,13 @@ <address type='drive' controller='0' bus='0' unit='0'/> </disk> <controller type='scsi' index='0'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0' multifunction='off'/> </controller> <controller type='scsi' index='1'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> </controller> <controller type='scsi' index='2'> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x1' multifunction='on'/> </controller> <controller type='scsi' index='3'> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args index 1007544..fea6c35 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.args @@ -1,6 +1,6 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml index 05a6adf..5a43638 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-companion.xml @@ -15,7 +15,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> <controller type='usb' index='0' model='ich9-uhci2'> <master startport='2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args index 0059ab5..94a4602 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args index 06863bb..546d119 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-piix3-controller.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device piix3-usb-uhci,id=usb,bus=pci.0,multifunction=on,addr=0x1.0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args index f6270d5..7d34c2a 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.args @@ -1,10 +1,10 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ +-device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ -chardev socket,id=charredir0,host=localhost,port=4000 \ -device usb-redir,chardev=charredir0,id=redir0 \ -chardev spicevmc,id=charredir1,name=usbredir \ -device usb-redir,chardev=charredir1,id=redir1,bus=usb.0,port=4 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml index 1dac3fb..a359a3d 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml @@ -19,7 +19,7 @@ </controller> <controller type='usb' index='0' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0' multifunction='on'/> </controller> <controller type='usb' index='0' model='ich9-uhci2'> <master startport='2'/> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args index be4a78e..580cace 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.args @@ -1,15 +1,15 @@ LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ --device piix3-usb-uhci,id=usb,bus=pci.0,multifunction=on,addr=0x1.0x2 \ --device ich9-usb-ehci1,id=usb1,bus=pci.0,multifunction=on,addr=0x4.0x7 \ --device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4.0x0 \ --device ich9-usb-uhci2,masterbus=usb1.0,firstport=2,bus=pci.0,multifunction=on,addr=0x4.0x1 \ --device ich9-usb-uhci3,masterbus=usb1.0,firstport=4,bus=pci.0,multifunction=on,addr=0x4.0x2 \ --device ich9-usb-ehci1,id=usb2,bus=pci.0,multifunction=on,addr=0x5.0x7 \ --device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5.0x0 \ --device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.0,multifunction=on,addr=0x5.0x1 \ --device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.0,multifunction=on,addr=0x5.0x2 \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-device ich9-usb-ehci1,id=usb1,bus=pci.0,addr=0x4.0x7 \ +-device ich9-usb-uhci1,masterbus=usb1.0,firstport=0,bus=pci.0,multifunction=on,addr=0x4 \ +-device ich9-usb-uhci2,masterbus=usb1.0,firstport=2,bus=pci.0,addr=0x4.0x1 \ +-device ich9-usb-uhci3,masterbus=usb1.0,firstport=4,bus=pci.0,addr=0x4.0x2 \ +-device ich9-usb-ehci1,id=usb2,bus=pci.0,addr=0x5.0x7 \ +-device ich9-usb-uhci1,masterbus=usb2.0,firstport=0,bus=pci.0,multifunction=on,addr=0x5 \ +-device ich9-usb-uhci2,masterbus=usb2.0,firstport=2,bus=pci.0,addr=0x5.0x1 \ +-device ich9-usb-uhci3,masterbus=usb2.0,firstport=4,bus=pci.0,addr=0x5.0x2 \ -device usb-hub,id=hub0,bus=usb1.0,port=1 \ -device usb-tablet,id=input0,bus=usb.0,port=2 \ -device usb-host,hostbus=14,hostaddr=6,id=hostdev0,bus=usb2.0,port=1 \ -device usb-host,hostbus=14,hostaddr=7,id=hostdev1,bus=usb2.0,port=2 \ --device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml index e8ada4d..b12b841 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb1-usb2.xml @@ -21,7 +21,7 @@ </controller> <controller type='usb' index='1' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='4' function='0'/> + <address type='pci' domain='0' bus='0' slot='4' function='0' multifunction='on'/> </controller> <controller type='usb' index='1' model='ich9-uhci2'> <master startport='2'/> @@ -37,7 +37,7 @@ </controller> <controller type='usb' index='2' model='ich9-uhci1'> <master startport='0'/> - <address type='pci' domain='0' bus='0' slot='5' function='0'/> + <address type='pci' domain='0' bus='0' slot='5' function='0' multifunction='on'/> </controller> <controller type='usb' index='2' model='ich9-uhci2'> <master startport='2'/> -- 1.7.3.4

On 09/30/2011 12:02 PM, Laine Stump wrote:
(V2: address Daniel Veillard's two review points (don't allow "multifunction='default'", and add "multifunction=off" to the qemu commandline when that's what the XML says), and modify the checks for duplicate PCI address usage attempts to account for multifunction=off on a device's function 0, per IRC discussion with Dan Berrange (see new qemuCollectPCIAddress()). As a side effect, attempts to re-use the same PCI address are now logged rather than silently generating an error.)
Yep, I can see those enhancements over v1.
A side effect of this patch is that attempts to use the same PCI address for two different devices will now log an error (previously this would cause the domain define operation to fail, but there would be no log message generated). Because the function doing this log was almost completely rewritten, I didn't think it worthwhile to make a separate patch for that fix (the entire patch would immediately be obsoleted).
Agree - no easy way to split it.
@@ -1118,10 +1118,14 @@ The<code>type</code> attribute is mandatory, and is typically "pci" or "drive". For a "pci" controller, additional attributes for<code>bus</code>,<code>slot</code>, - and<code>function</code> must be present, as well as an - optional<code>domain</code>. For a "drive" controller, - additional attributes<code>controller</code>,<code>bus</code>, + and<code>function</code> must be present, as well as + optional<code>domain</code> + and<code>multifunction</code><span class="since">since + 0.9.7, requires QEMU 0.13</span> (defaults to 'off'). For a
This wording makes it sound like both domain and multifunction were first introduced in 0.9.7, rather than the intended fact that 'multifunction' is the only newcomer attribute. Maybe reword along the lines of: ...as well as optional 'domain' and 'multifunction'. Multifunction defaults to 'off', any other value requires QEMU 0.13 and <span class="since">libvirt 0.9.7</span>.
+++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-ich9-ehci-addr.args @@ -1 +1 @@ -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,multifunction=on,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,multifunction=on,addr=0x3.0x0 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -nodefconfig -nodefaults -chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c -device ich9-usb-ehci1,id=usb,bus=pci.0,addr=0x4.0x7 -device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
These are some rather long lines. While you are touching them, it would be worth splitting them with \-newline pairs to fit in more reasonable limits. ACK with those nits fixed. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org

Okay, I made Eric's suggested changes and pushed. This means that multifunction will no longer be automatically turned on for function 0. This necessitates a change in, e.g. USB 2.0 configs, where multiple functions on a slot are used - you will need to add "multifunction='on'" to the <address> element of the device on function 0.
participants (3)
-
Daniel Veillard
-
Eric Blake
-
Laine Stump