[fedora-virt] [libvirt] [PATCH] qemu: qxl devices don't support multifunction yet

[Laine Stump]

On 09/27/2011 05:13 AM, Daniel P. Berrange wrote:
> On Tue, Sep 27, 2011 at 01:19:20AM -0400, Laine Stump wrote:
>> On 09/19/2011 01:32 PM, Daniel P. Berrange wrote:
>>> On Mon, Sep 19, 2011 at 07:16:22PM +0200, Marc-André Lureau wrote:
>>>> Hi hi
>>>>
>>>> On Fri, Sep 16, 2011 at 1:38 PM, Marc-André Lureau<mlureau at redhat.com>   wrote:
>>>>>>> How do we allow other devices to share the slot? It seems to me that
>>>>>>> qemuDomainPCIAddressSetNextAddr() only allocate whole slot, while
>>>>>>> making sure there is no conflicts on the same slot.
>>>>>> So, if the user wants to use multi function pci device, he should
>>>>>> specify the
>>>>>> pci address.
>>>>> So adding a check such as:
>>>>>
>>>>> if (!multiFunc&&   info->addr.pci.function != 0)
>>>>>    return error("The %s device doesn't support multifunction address")
>>>>>
>>>> Wen, does that sound reasonable to you?
>>>>
>>>> Daniel, did you had time to verify that PCI allocation is per-slot?
>>>>
>>>> (It would be nice to get this "workaround" for the next release)
>>> IMHO this kind of hack doesn't belong in libvirt. It is fine for distro
>>> vendors to consider as a one off quick-hack for their packages of libvirt,
>>> if they don't have time to fix the real QXL bug, but not for libvirt
>>> upstream releases. QXL/QEMU should really be fixed since that's where the
>>> problem appears to lie.
>> As it stands, Fedora 16 (currently using unpatched libvirt-0.9.6)
>> will be going into beta with QXL video broken for Windows guests, so
>> we need some kind of Fedora-only patch very soon (see the schedule
>> here: https://fedoraproject.org/wiki/Releases/16/Schedule -
>> fortunately just delayed another week)
>>
>> The original patch in this thread:
>>
>>    https://www.redhat.com/archives/libvir-list/2011-September/msg00534.html
>>
>> of course doesn't include the above mentioned additional code, and
>> there isn't a followup patch. It would be very good to push a patch
>> to the F16 git for this so it would hopefully get into the beta, but
>> want to make sure what I push is the "right" thing, so a "final"
>> patch (and some testing by people with F16 hosts) would be very
>> helpful!
> When we originally enabled multifunction for all PCI devices, we did so
> in the belief that this was effectively a no-op for guest OSes. ie it
> should not have changed guest OS behaviour at all, unless multiple devices
> were actually inserted in the slot.
>
> Evidentally this turns out to be incorrect. In other words we introduced
> a guest ABI change. This is very bad, because our stated goal is to have
> a stable guest ABI across libvirt&  QEMU releases.
>
> Thus IMHO we need to disable *all* setting of the 'multifunction=on'
> parameter for all guests by default, to unbreak the ABI compatibility.

Of course this has been in since 0.9.3, meaning there are official 
releases with broken ABI compatibility :-(. Still, the sooner it's fixed 
the better.

> Then we should introduce a new parameter 'multifunction='on' in the
> <address type=pci>  element to allow it to be optionally enabled per
> device.

Will it be acceptable to put that in a patch on top of 0.9.6 for Fedora 
16? (I think the answer is "yes", since it's only an XML change, but 
just want to make sure).