[libvirt] [PATCH] qemu: caps: Use unique key for domCaps caching

[Daniel Henrique Barboza]

On 10/15/19 1:01 PM, Cole Robinson wrote:
> When searching qemuCaps->domCapsCache for existing domCaps data,
> we check for a matching pair of arch+virttype+machine+emulator. However
> for the hash table key we only use the machine string. So if the
> cache already contains:
>
>    x86_64 + kvm + pc + /usr/bin/qemu-kvm
>
> But a new VM is defined with
>
>    x86_64 + qemu + pc + /usr/bin/qemu-kvm
>
> We correctly fail to find matching cached domCaps, but then attempt
> to use a colliding key with virHashAddEntry
>
> Fix this by building a hash key from the 4 values, not just machine
>
> Signed-off-by: Cole Robinson <crobinso at redhat.com>
> ---
> qemu_domain.c validation should be affected, but it is covered up
> by another bug, fixed here:
> https://www.redhat.com/archives/libvir-list/2019-October/msg00708.html
>
>   src/qemu/qemu_conf.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index 08cd784054..64ac8cbdd3 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1396,6 +1396,8 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriverPtr driver,
>       domCaps = virHashSearch(domCapsCache,
>                               virQEMUDriverSearchDomcaps, &data, NULL);
>       if (!domCaps) {
> +        VIR_AUTOFREE(char *) key = NULL;
> +

"g_autofree char *key = NULL;" instead of VIR_AUTOFREE for extra karma
points. Everything else LGTM.



Reviewed-by: Daniel Henrique Barboza <danielhb413 at gmail.com>




>           /* hash miss, build new domcaps */
>           if (!(domCaps = virDomainCapsNew(data.path, data.machine,
>                                            data.arch, data.virttype)))
> @@ -1406,7 +1408,14 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriverPtr driver,
>                                         cfg->firmwares, cfg->nfirmwares) < 0)
>               return NULL;
>   
> -        if (virHashAddEntry(domCapsCache, machine, domCaps) < 0)
> +        if (virAsprintf(&key, "%d:%d:%s:%s",
> +                        data.arch,
> +                        data.virttype,
> +                        NULLSTR(data.machine),
> +                        NULLSTR(data.path)) < 0)
> +            return NULL;
> +
> +        if (virHashAddEntry(domCapsCache, key, domCaps) < 0)
>               return NULL;
>       }
>