[libvirt] [PATCH] domcaps: Check for architecture more wisely
Cole Robinson
crobinso at redhat.com
Wed Apr 8 15:25:44 UTC 2015
On 04/08/2015 11:12 AM, Michal Privoznik wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1209948
>
> So we have this bug. The virConnectGetDomainCapabilities() API
> performs a couple of checks before it produces any result. One of
> the checks is if the architecture requested by user can be run by
> the binary (again user provided). However, the check is pretty
> dumb. It merely compares if the default binary architecture
> matches the one provided by user. However, a qemu binary can run
> multiple architectures. For instance: qemu-system-ppc64 can run:
> ppc, ppcle, ppc64, ppc64le and ppcemb. The default is ppc64, so
> if user requested something else, like ppc64le, the check would
> have failed without obvious reason.
>
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
> src/qemu/qemu_driver.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 921417c..d217481 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -18788,7 +18788,10 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
> arch_from_caps = virQEMUCapsGetArch(qemuCaps);
>
> if (arch_from_caps != arch &&
> - (arch_from_caps != VIR_ARCH_X86_64 || arch != VIR_ARCH_I686)) {
> + !((ARCH_IS_X86(arch) && ARCH_IS_X86(arch_from_caps)) ||
> + (ARCH_IS_PPC(arch) && ARCH_IS_PPC(arch_from_caps)) ||
> + (ARCH_IS_ARM(arch) && ARCH_IS_ARM(arch_from_caps)) ||
> + (ARCH_IS_S390(arch) && ARCH_IS_S390(arch_from_caps)))) {
> virReportError(VIR_ERR_INVALID_ARG,
> _("architecture from emulator '%s' doesn't "
> "match given architecture '%s'"),
>
As I mentioned in the bug, really I think this is more a problem of the
virQEMUCapsCacheLookup* interface. Nowadays we can have the same emulatorbin
map to multiple architectures, and the same architecture provided by multiple
emulators, so CacheLookup* functions that search for only the specified
emulator or only the specified arch are just going to breed more bugs like this.
So rather than extend this specific site with a whitelist that could grow out
of date, I'd rather see a CacheLookup function added that takes at least arch
+ emulator, but probably also machine type and virt type (like domcapabilities
takes) and finds the actually requested caps.
And separately we should audit all other uses of CacheLookup* to make sure
other bugs aren't lingering, or just file a bz for it
Thanks,
Cole
More information about the libvir-list
mailing list