From nobody Wed May 8 22:26:30 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) client-ip=205.139.110.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1592849666; cv=none; d=zohomail.com; s=zohoarc; b=YQ+5EPQ9RlLNEyS+6FfBydyoI4ohTuNjQGiUyFwCpoEz2fGPEz/4SMwctD56lTdj2TtYfrJCFuGT9OA3WEqcgpqiV5DD8h4Uaf4VeSl7o/SrQEzDsH8rHiukDT7XcbVFF2Lk5ugj3X1OfMLhHtYrHhYcX1McZ3RZIZbq4nauXJ4= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1592849666; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=Ikc7TSoS32lkQpTt3JTDEs1TYKnqYI1jlagwcOv5nn0=; b=Fk2c69024ZUfRSBFCNSA/2q5/Ben6BqWQ0TyGBSODoB6I4RD7VAjLEzHm6+c9a6RQMshs8y6Y4LIztOQNem+7BukzFSqoDVZT8ACvc+DEobJ5VgODGMqBIy9NP7xmwkcoOQrb9zgux9HH+CiFN6u5zwHlE7lBVnXTsIUb4Fj/4s= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by mx.zohomail.com with SMTPS id 1592849666623794.0668552421307; Mon, 22 Jun 2020 11:14:26 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-60-UMNlEACtNYOey7e1T0yn1g-1; Mon, 22 Jun 2020 14:14:22 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 75081464; Mon, 22 Jun 2020 18:14:17 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 419315C1BD; Mon, 22 Jun 2020 18:14:16 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 23FD41809547; Mon, 22 Jun 2020 18:14:11 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 05MIE9rd007121 for ; Mon, 22 Jun 2020 14:14:09 -0400 Received: by smtp.corp.redhat.com (Postfix) id 5B3A010013D9; Mon, 22 Jun 2020 18:14:09 +0000 (UTC) Received: from catbus.gsslab.fab.redhat.com (mustard.gsslab.fab.redhat.com [10.33.8.112]) by smtp.corp.redhat.com (Postfix) with ESMTP id B207010013D7; Mon, 22 Jun 2020 18:14:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1592849665; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=Ikc7TSoS32lkQpTt3JTDEs1TYKnqYI1jlagwcOv5nn0=; b=LSdC0gIpZMFiJf3awUQZ3UTTSVaDAmyeVF2g3uZlfhF8ZmEzPuuAfeufBMjRquRKW/uPTA fUbpkZTpJ9GmAUDxDjTzWj/q4kkJ8RydTXAyT2ZdIJ1VkTehtNdxUNRWKRV73l9VhV/wnH e+I8JGsC1XGtcjHtiGP03fEfh5HpP7o= X-MC-Unique: UMNlEACtNYOey7e1T0yn1g-1 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Subject: [libvirt PATCH v2] qemu: don't continue loading caps if outdated Date: Mon, 22 Jun 2020 19:07:01 +0100 Message-Id: <20200622180701.2769430-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) The XML format used for QEMU capabilities is not required to be stable across releases, as we invalidate the cache whenever the libvirt binary changes. We none the less always try to parse te entire XML file before we do any validity checks. Thus if we change the format of any part of the data, or change permitted values for enums, then libvirtd logs will be spammed with errors. These are not in fact errors, but an expected scenario. This change makes the loading code validate the cache timestamp against the libvirtd timestamp immediately. If they don't match then we stop loading the rest of the XML file. Signed-off-by: Daniel P. Berrang=C3=A9 Reviewed-by: Michal Privoznik --- src/qemu/qemu_capabilities.c | 63 +++++++++++++++++++++++------------- src/qemu/qemu_capspriv.h | 6 ++-- src/util/virfilecache.c | 11 ++++--- src/util/virfilecache.h | 11 +++++-- tests/testutilsqemu.c | 3 +- tests/virfilecachetest.c | 3 +- 6 files changed, 61 insertions(+), 36 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 484fff99e5..3053aa31f9 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1807,14 +1807,6 @@ virQEMUCapsNewBinary(const char *binary) } =20 =20 -void -virQEMUCapsSetInvalidation(virQEMUCapsPtr qemuCaps, - bool enabled) -{ - qemuCaps->invalidation =3D enabled; -} - - static int virQEMUCapsHostCPUDataCopy(virQEMUCapsHostCPUDataPtr dst, virQEMUCapsHostCPUDataPtr src) @@ -4205,11 +4197,14 @@ virQEMUCapsParseSEVInfo(virQEMUCapsPtr qemuCaps, xm= lXPathContextPtr ctxt) * * ... * + * + * Returns 0 on success, 1 if outdated, -1 on error */ int virQEMUCapsLoadCache(virArch hostArch, virQEMUCapsPtr qemuCaps, - const char *filename) + const char *filename, + bool skipInvalidation) { xmlDocPtr doc =3D NULL; int ret =3D -1; @@ -4237,6 +4232,31 @@ virQEMUCapsLoadCache(virArch hostArch, goto cleanup; } =20 + if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing selfctime in QEMU capabilities XML")); + goto cleanup; + } + qemuCaps->libvirtCtime =3D (time_t)l; + + qemuCaps->libvirtVersion =3D 0; + if (virXPathULong("string(./selfvers)", ctxt, &lu) =3D=3D 0) + qemuCaps->libvirtVersion =3D lu; + + if (!skipInvalidation && + (qemuCaps->libvirtCtime !=3D virGetSelfLastChanged() || + qemuCaps->libvirtVersion !=3D LIBVIR_VERSION_NUMBER)) { + VIR_DEBUG("Outdated capabilities in %s: libvirt changed " + "(%lld vs %lld, %lu vs %lu), stopping load", + qemuCaps->binary, + (long long)qemuCaps->libvirtCtime, + (long long)virGetSelfLastChanged(), + (unsigned long)qemuCaps->libvirtVersion, + (unsigned long)LIBVIR_VERSION_NUMBER); + ret =3D 1; + goto cleanup; + } + if (!(str =3D virXPathString("string(./emulator)", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing emulator in QEMU capabilities cache")); @@ -4256,17 +4276,6 @@ virQEMUCapsLoadCache(virArch hostArch, } qemuCaps->ctime =3D (time_t)l; =20 - if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("missing selfctime in QEMU capabilities XML")); - goto cleanup; - } - qemuCaps->libvirtCtime =3D (time_t)l; - - qemuCaps->libvirtVersion =3D 0; - if (virXPathULong("string(./selfvers)", ctxt, &lu) =3D=3D 0) - qemuCaps->libvirtVersion =3D lu; - if ((n =3D virXPathNodeSet("./flag", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("failed to parse qemu capabilities flags")); @@ -4418,6 +4427,9 @@ virQEMUCapsLoadCache(virArch hostArch, if (virXPathBoolean("boolean(./kvmSupportsSecureGuest)", ctxt) > 0) qemuCaps->kvmSupportsSecureGuest =3D true; =20 + if (skipInvalidation) + qemuCaps->invalidation =3D false; + ret =3D 0; cleanup: VIR_FREE(str); @@ -5489,16 +5501,23 @@ virQEMUCapsNewData(const char *binary, static void * virQEMUCapsLoadFile(const char *filename, const char *binary, - void *privData) + void *privData, + bool *outdated) { virQEMUCapsPtr qemuCaps =3D virQEMUCapsNewBinary(binary); virQEMUCapsCachePrivPtr priv =3D privData; + int ret; =20 if (!qemuCaps) return NULL; =20 - if (virQEMUCapsLoadCache(priv->hostArch, qemuCaps, filename) < 0) + ret =3D virQEMUCapsLoadCache(priv->hostArch, qemuCaps, filename, false= ); + if (ret < 0) goto error; + if (ret =3D=3D 1) { + *outdated =3D true; + goto error; + } =20 return qemuCaps; =20 diff --git a/src/qemu/qemu_capspriv.h b/src/qemu/qemu_capspriv.h index 5d2f448e41..f6c06ea008 100644 --- a/src/qemu/qemu_capspriv.h +++ b/src/qemu/qemu_capspriv.h @@ -37,12 +37,10 @@ virQEMUCapsNewForBinaryInternal(virArch hostArch, unsigned int microcodeVersion, const char *kernelVersion); =20 -void virQEMUCapsSetInvalidation(virQEMUCapsPtr qemuCaps, - bool enabled); - int virQEMUCapsLoadCache(virArch hostArch, virQEMUCapsPtr qemuCaps, - const char *filename); + const char *filename, + bool skipInvalidation); char *virQEMUCapsFormatCache(virQEMUCapsPtr qemuCaps); =20 int diff --git a/src/util/virfilecache.c b/src/util/virfilecache.c index aecabf173d..2162917b11 100644 --- a/src/util/virfilecache.c +++ b/src/util/virfilecache.c @@ -130,6 +130,7 @@ virFileCacheLoad(virFileCachePtr cache, g_autofree char *file =3D NULL; int ret =3D -1; void *loadData =3D NULL; + bool outdated =3D false; =20 *data =3D NULL; =20 @@ -148,10 +149,12 @@ virFileCacheLoad(virFileCachePtr cache, goto cleanup; } =20 - if (!(loadData =3D cache->handlers.loadFile(file, name, cache->priv)))= { - VIR_WARN("Failed to load cached data from '%s' for '%s': %s", - file, name, virGetLastErrorMessage()); - virResetLastError(); + if (!(loadData =3D cache->handlers.loadFile(file, name, cache->priv, &= outdated))) { + if (!outdated) { + VIR_WARN("Failed to load cached data from '%s' for '%s': %s", + file, name, virGetLastErrorMessage()); + virResetLastError(); + } ret =3D 0; goto cleanup; } diff --git a/src/util/virfilecache.h b/src/util/virfilecache.h index 006a9717cb..9a7edf07e6 100644 --- a/src/util/virfilecache.h +++ b/src/util/virfilecache.h @@ -62,15 +62,20 @@ typedef void * * @filename: name of a file with cached data * @name: name of the cached data * @priv: private data created together with cache + * @outdated: set to true if data was outdated * - * Loads the cached data from a file @filename. + * Loads the cached data from a file @filename. If + * NULL is returned, then @oudated indicates whether + * this was due to the data being outdated, or an + * error loading the cache. * - * Returns cached data object or NULL on error. + * Returns cached data object or NULL on outdated data or error. */ typedef void * (*virFileCacheLoadFilePtr)(const char *filename, const char *name, - void *priv); + void *priv, + bool *outdated); =20 /** * virFileCacheSaveFilePtr: diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 4dcc3089dd..e3b1e2813b 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -293,10 +293,9 @@ qemuTestParseCapabilitiesArch(virArch arch, virArchToString(arch)); =20 if (!(qemuCaps =3D virQEMUCapsNewBinary(binary)) || - virQEMUCapsLoadCache(arch, qemuCaps, capsFile) < 0) + virQEMUCapsLoadCache(arch, qemuCaps, capsFile, true) < 0) goto error; =20 - virQEMUCapsSetInvalidation(qemuCaps, false); return qemuCaps; =20 error: diff --git a/tests/virfilecachetest.c b/tests/virfilecachetest.c index 6d280b3bec..34e0d0ab2f 100644 --- a/tests/virfilecachetest.c +++ b/tests/virfilecachetest.c @@ -110,7 +110,8 @@ testFileCacheNewData(const char *name G_GNUC_UNUSED, static void * testFileCacheLoadFile(const char *filename, const char *name G_GNUC_UNUSED, - void *priv G_GNUC_UNUSED) + void *priv G_GNUC_UNUSED, + bool *outdated G_GNUC_UNUSED) { testFileCacheObjPtr obj; char *data; --=20 2.24.1