Re: [libvirt] [PATCH 2/2] qemu: make PCI multifunction support more manual
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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/