[libvirt] [PATCH] qemu: caps: Use unique key for domCaps caching
Jonathon Jongsma
jjongsma at redhat.com
Wed Oct 16 16:22:55 UTC 2019
On Wed, 2019-10-16 at 09:12 -0300, Daniel Henrique Barboza wrote:
>
> 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>
>
Looks good to me as well. But in keeping with the theme, we could also
use g_strdup_printf() below instead of virAsprintf(). That is one of
the suggestions made in the recent updates to our hacking documentation
from Daniel's glib patch series.
Reviewed-by: Jonathon Jongsma <jjongsma at redhat.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;
> > }
> >
More information about the libvir-list
mailing list