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