[libvirt] [PATCH 1/2] Assign PCI address to primary video card in pseries guests

[Cole Robinson]

On 11/07/2013 05:40 AM, Laine Stump wrote:
> On 11/06/2013 06:13 PM, Vitor de Lima wrote:
>> From: Vitor de Lima <vitor.lima at eldorado.org.br>
> 
> I would remove reference to "pseries" from the subject, as it's
> assigning a pci address on any arch that hasn't already assigned one
> (and the only ones that have are x86 Q35 and i440fx/PIIX3
>>
>> This patch assings
> 
> s/assings/assigns/
> 
>>  a PCI address to the primary video card if it
>> does not have any kind of address. This fixes issues with pseries
>> guests.
> 
> It's probably worth adding in a statement that this was broken when
> adding support for Q35, which moved the assignment of a PCI address for
> the primary video card out to arch-specific functions for Q35 and
> i440fx, but forgot to do anything for all the other arches.
> 
> 
> I'm now nervous about assigning PCI addresses to devices due to the lack
> of PCI bus on many arches. Cole - does this particular change cause a
> problem for ARM? (I don't see anywhere that the address type is set to
> != NONE on video devices other than where it's set to PCI specifically
> for Q35 or i440fx, so if ARM arches support video, this patch will try
> to allocate a PCI slot, which will fail).
> 
>> ---
>>  src/qemu/qemu_command.c | 9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index e48c9c2..159d920 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -2959,6 +2959,15 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
>>              goto error;
>>      }
>>  
>> +    /* Assign a PCI slot to the primary video card if there is not an
>> +     * assigned address. */
>> +    if (def->nvideos >=1 &&
> 
> You missed a space, and other comparisons of nvideos use "> 0" instead
> of ">= 1".
> 
>> +        def->videos[0]->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
>> +        if (qemuDomainPCIAddressReserveNextSlot(addrs, &def->videos[0]->info,
>> +                                                flags) < 0)
>> +            goto error;
>> +    }
>> +
>>      /* Further non-primary video cards which have to be qxl type */
>>      for (i = 1; i < def->nvideos; i++) {
>>          if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
> 
> ACK to this patch if Cole says that it's okay for ARM. I'll make the
> small changes and push it if so.
> 

If the 'make check' doesn't regress then it's fine for known ARM needs.

- Cole