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@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list