[libvirt] [PATCH v2 2/3] qemu: Add support for gic-version machine option

[Martin Kletzander]

On Thu, Oct 01, 2015 at 09:53:00AM +0300, Pavel Fedin wrote:
> Hello!
>
> > Indentation's off here.
>
> Damn, sorry, overlooked...
>
>> Also before this patch we would allow def->gic_version == 2 for any
>> machine type.  I don't have a problem with this since GIC doesn't make
>> sense anywhere else then on ARM machines,
>
> I'm OK with this. I used 0 for 'no version supplied' just because libvirt originally does this.
>
>> but shouldn't we check for
>> the fact that the request is for ARM (in case, for example, if ppc64
>> gains some 'virt' machine type)?  Because we have no guarantee that
>> it's ARM just based on the machine type.
>
> Yes, i guess we should.
>
>> I'd change this to:
>>
>> if (gic != 2) {
>>     if (!caps)
>>         error;
>>     append_cmd();
>> }
>
> You know, if we are talking about making changes in parser code, we could do more. Actually, as i
>said in my cover letter, qemu supports more than just 2 or 3. We can also specify 'host' for 'best
>possible'. Could we accommodate this somehow too? I believe in order to do this, we should change
>parameter type from numeric to string.
>

Yes, we can certainly do that.

> And also we could add some another boolean, which would allow to disable in-kernel GIC emulation
>(kernel_irqchip=off). This works with any machine type, BTW, not only with ARM. Something like <gic
>kvm='off'/>.
>

I don't know what that is.  Is that only GIC related?  Where could I
find more details?  If that makes sense for anything, we can sure do
that, the only reason why I would be against this, which I can come up
now, is if there is nobody using it.

> I believe these changes could go as a separate patch, after we discuss details.
>

Yeah, sure.

>> If you're ok with that, just let me know and I'll push it with the
>> following diff squashed in, right after the release:
>
> Yes, ACK.
>
>> +                    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>>                                     _("gic-version option is available "
>>                                       "only for virt machine"));
>
> Then "...only for ARM virt machine".
>

Oh, good catch, I'll fix that, thanks.

>Kind regards,
>Pavel Fedin
>Expert Engineer
>Samsung Electronics Research center Russia
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151001/de1953d0/attachment-0001.sig>