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.
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 believe these changes could go as a separate patch, after we discuss details.
> 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".
Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list