6:10 a.m.
On Thu, Oct 20, 2022 at 16:58:57 -0500, Jonathon Jongsma wrote:
Implement the loadFile and saveFile virFileCacheHandlers callbacks
so
that nbdkit capabilities are cached perstistently across daemon
restarts. The format and implementation is modeled on the qemu
capabilities, but simplified slightly.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
src/qemu/qemu_nbdkit.c | 234 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 232 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index 9ab048e9e1..a47e169a0f 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
[...]
+static int
+qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps,
+ const char *filename)
+{
+ g_autoptr(xmlDoc) doc = NULL;
+ g_autoptr(xmlXPathContext) ctxt = NULL;
+ long long int l;
+ unsigned long lu;
+
+ if (!(doc = virXMLParse(filename, NULL, NULL, "nbdkitCaps", &ctxt,
NULL, false)))
+ return -1;
+
+ if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing selfctime in nbdkit capabilities XML"));
+ return -1;
+ }
+ nbdkitCaps->libvirtCtime = (time_t)l;
+
+ nbdkitCaps->libvirtVersion = 0;
+ if (virXPathULong("string(./selfvers)", ctxt, &lu) == 0)
+ nbdkitCaps->libvirtVersion = lu;
This no longer compiles because I've removed the 'Long' versions of the
XPath helpers apply the following diff:
diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
index a47e169a0f..82f80d3524 100644
--- a/src/qemu/qemu_nbdkit.c
+++ b/src/qemu/qemu_nbdkit.c
@@ -393,7 +393,6 @@ qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps,
g_autoptr(xmlDoc) doc = NULL;
g_autoptr(xmlXPathContext) ctxt = NULL;
long long int l;
- unsigned long lu;
if (!(doc = virXMLParse(filename, NULL, NULL, "nbdkitCaps", &ctxt,
NULL, false)))
return -1;
@@ -406,8 +405,7 @@ qemuNbdkitCapsLoadCache(qemuNbdkitCaps *nbdkitCaps,
nbdkitCaps->libvirtCtime = (time_t)l;
nbdkitCaps->libvirtVersion = 0;
- if (virXPathULong("string(./selfvers)", ctxt, &lu) == 0)
- nbdkitCaps->libvirtVersion = lu;
+ virXPathUInt("string(./selfvers)", ctxt,
&nbdkitCaps->libvirtVersion);
if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() ||
nbdkitCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
+
+ if (nbdkitCaps->libvirtCtime != virGetSelfLastChanged() ||
+ nbdkitCaps->libvirtVersion != LIBVIR_VERSION_NUMBER) {
+ VIR_DEBUG("Outdated capabilities in %s: libvirt changed "
+ "(%lld vs %lld, %lu vs %lu), stopping load",
+ nbdkitCaps->path,
Preferrably no linebreak in this diagnostic message.
+ (long long)nbdkitCaps->libvirtCtime,
+ (long long)virGetSelfLastChanged(),
+ (unsigned long)nbdkitCaps->libvirtVersion,
+ (unsigned long)LIBVIR_VERSION_NUMBER);
+ return 1;
+ }
+
+ if (qemuNbdkitCapsValidateBinary(nbdkitCaps, ctxt) < 0)
+ return -1;
+
+ if (virXPathLongLong("string(./nbdkitctime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing nbdkitctime in nbdkit capabilities XML"));
+ return -1;
+ }
+ nbdkitCaps->ctime = (time_t)l;
+
+ if (virXPathLongLong("string(./plugindirmtime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing plugindirmtime in nbdkit capabilities
XML"));
+ return -1;
+ }
+ nbdkitCaps->pluginDirMtime = (time_t)l;
+
+ if (virXPathLongLong("string(./filterdirmtime)", ctxt, &l) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing filterdirmtime in nbdkit capabilities
XML"));
+ return -1;
+ }
+ nbdkitCaps->filterDirMtime = (time_t)l;
+
+ if (qemuNbdkitCapsParseFlags(nbdkitCaps, ctxt) < 0)
+ return -1;
+
+ if ((nbdkitCaps->version = virXPathString("string(./version)", ctxt))
== NULL) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("missing version in nbdkit capabilities cache"));
+ return -1;
+ }
I presume it's an artifact from the domain caps caching code, but
reporting error for any of these missing fields make much sense to me.
If the cache is invalid for any reason it will be dropped and re-fetched
again so the 'error' level just puts that info into the logs but no
actual error will happen. I think that the above should be a debug
level.
+
+ return 0;
+}
+
+
+static void*
+virNbdkitCapsLoadFile(const char *filename,
+ const char *binary,
+ void *privData G_GNUC_UNUSED,
+ bool *outdated)
+{
+ g_autoptr(qemuNbdkitCaps) nbdkitCaps = qemuNbdkitCapsNew(binary);
+ int ret;
+
+ if (!nbdkitCaps)
+ return NULL;
+
+ ret = qemuNbdkitCapsLoadCache(nbdkitCaps, filename);
+ if (ret < 0)
+ return NULL;
+ if (ret == 1) {
+ *outdated = true;
+ return NULL;
+ }
+
+ return g_steal_pointer(&nbdkitCaps);
+}
+
+
+static char*
+qemuNbdkitCapsFormatCache(qemuNbdkitCaps *nbdkitCaps)
+{
+ g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+ size_t i;
+
+ virBufferAddLit(&buf, "<nbdkitCaps>\n");
+ virBufferAdjustIndent(&buf, 2);
+
+ virBufferEscapeString(&buf, "<path>%s</path>\n",
+ nbdkitCaps->path);
+ virBufferAsprintf(&buf,
"<nbdkitctime>%lld</nbdkitctime>\n",
+ (long long)nbdkitCaps->ctime);
+ virBufferAsprintf(&buf,
"<plugindirmtime>%lld</plugindirmtime>\n",
+ (long long)nbdkitCaps->pluginDirMtime);
+ virBufferAsprintf(&buf,
"<filterdirmtime>%lld</filterdirmtime>\n",
+ (long long)nbdkitCaps->filterDirMtime);
+ virBufferAsprintf(&buf, "<selfctime>%lld</selfctime>\n",
+ (long long)nbdkitCaps->libvirtCtime);
+ virBufferAsprintf(&buf, "<selfvers>%lu</selfvers>\n",
+ (unsigned long)nbdkitCaps->libvirtVersion);
Do not use 'long' type. The variable is unsigned int anyways so use the
proper conversion and drop the typecast.
+
+ for (i = 0; i < QEMU_NBDKIT_CAPS_LAST; i++) {
+ if (qemuNbdkitCapsGet(nbdkitCaps, i)) {
+ virBufferAsprintf(&buf, "<flag name='%s'/>\n",
+ qemuNbdkitCapsTypeToString(i));
+ }
+ }
+
+ virBufferAsprintf(&buf, "<version>%s</version>\n",
+ nbdkitCaps->version);
+
+ virBufferAdjustIndent(&buf, -2);
+ virBufferAddLit(&buf, "</nbdkitCaps>\n");
+
+ return virBufferContentAndReset(&buf);
+}
+
+
+static int
+virNbdkitCapsSaveFile(void *data,
+ const char *filename,
+ void *privData G_GNUC_UNUSED)
+{
+ qemuNbdkitCaps *nbdkitCaps = data;
+ g_autofree char *xml = NULL;
+
+ xml = qemuNbdkitCapsFormatCache(nbdkitCaps);
+
+ if (virFileWriteStr(filename, xml, 0600) < 0) {
+ virReportSystemError(errno,
+ _("Failed to save '%s' for
'%s'"),
+ filename, nbdkitCaps->path);
+ return -1;
+ }
+
+ VIR_DEBUG("Saved caps '%s' for '%s' with (%lld, %lld)",
+ filename, nbdkitCaps->path,
+ (long long)nbdkitCaps->ctime,
+ (long long)nbdkitCaps->libvirtCtime);
+
+ return 0;
+}
+
+
virFileCacheHandlers nbdkitCapsCacheHandlers = {
.isValid = virNbdkitCapsIsValid,
.newData = virNbdkitCapsNewData,
- .loadFile = NULL,
- .saveFile = NULL,
+ .loadFile = virNbdkitCapsLoadFile,
+ .saveFile = virNbdkitCapsSaveFile,
.privFree = NULL,
};
--
2.37.3