Re: [libvirt] [PATCH v2 2/3] qemu: Add support for gic-version machine option
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