Re: [libvirt] [PATCH 03/11] qemu: Move GIC checks to qemuDomainDefValidateFeatures()
On Mon, 2018-02-12 at 14:59 -0500, John Ferlan wrote:
> @@ -3252,6 +3252,16 @@ qemuDomainDefValidateFeatures(const
virDomainDef *def)
> }
> break;
>
> + case VIR_DOMAIN_FEATURE_GIC:
> + if (def->features[i] == VIR_TRISTATE_SWITCH_ON &&
> + !qemuDomainIsVirt(def)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("The '%s' feature is only supported
for %s guests"),
s/for %s guests/for '%s' guests/ ??
Sure, why not :)
> + featureName,
"mach-virt");
Not that I think it matters greatly, the error message changes from ARM
specifically to mach-virt... I guess I'm just used to seeing 'ARM' or
'aarch64' and not 'mach-virt' (although the XML would be AIUI
"<type
arch='aarch64' machine='virt'>"). Suffice to say it caused me to
wonder
and I have to imagine it would do the same for anyone getting that message.
I don't have a strong opinion one way or other and it's not really
easily "decode-able" based on someone just reading the "<os... <type
...
machine=''..." in the docs.
Yeah, we're being quite inconsistent throughout both the code and
the test suite.
The root of the problem is that, while other architectures use very
recognizable or at least distinct names for the machine types we
care about, such as pseries or q35, on aarch64 the machine type is
just... virt. Which is not really a great name. But we can't just
use the architecture name either, because there are a lot of arm
and aarch64 machine types out there.
Using mach-virt is an attempt to sidestep the problem, and it's
used (at least internally) in QEMU. But I agree it can be confusing.
Maybe the solution in this case is to use the approach originally
used for the <ioapic/> feature and just say
The '%s' feature is not supported for architecture '%s' or
machine type '%s'
though that's slightly less helpful for users...
--
Andrea Bolognani / Red Hat / Virtualization