On Wed, Aug 31, 2022 at 13:40:51 -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@xxxxxxxxxx>
> ---
> src/qemu/qemu_nbdkit.c | 253 ++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 251 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> index 3faa16674a..ac498b948c 100644
> --- a/src/qemu/qemu_nbdkit.c
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -339,11 +339,260 @@ virNbdkitCapsNewData(const char *binary,
> }
>
>
> +static int
> +qemuNbdkitCapsValidateBinary(qemuNbdkitCaps *nbdkitCaps,
> + xmlXPathContextPtr ctxt)
> +{
> + g_autofree char *str = NULL;
> +
> + if (!(str = virXPathString("string(./path)", ctxt))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing path in nbdkit capabilities cache"));
> + return -1;
> + }
> +
> + if (STRNEQ(str, nbdkitCaps->path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Expected caps for '%s' but saw '%s'"),
> + nbdkitCaps->path, str);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> +
> +static int
> +qemuNbdkitCapsParseFlags(qemuNbdkitCaps *nbdkitCaps,
> + xmlXPathContextPtr ctxt)
> +{
> + g_autofree xmlNodePtr *nodes = NULL;
> + size_t i;
> + int n;
> +
> + if ((n = virXPathNodeSet("./flag", ctxt, &nodes)) < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("failed to parse qemu capabilities flags"));
> + return -1;
> + }
> +
> + VIR_DEBUG("Got flags %d", n);
> + for (i = 0; i < n; i++) {
> + g_autofree char *str = NULL;
> + int flag;
> +
> + if (!(str = virXMLPropString(nodes[i], "name"))) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> + _("missing flag name in QEMU capabilities cache"));
> + return -1;
> + }
> +
> + flag = qemuNbdkitCapsTypeFromString(str);
> + if (flag < 0) {
> + virReportError(VIR_ERR_INTERNAL_ERROR,
> + _("Unknown nbdkit capabilities flag %s"), str);
> + return -1;
> + }
You can use virXMLPropEnum instead of having two clauses.
> +
> + qemuNbdkitCapsSet(nbdkitCaps, flag);
> + }
> +
> + return 0;
> +}
> +
> +
> +/*
> + * Parsing a doc that looks like
> + *
> + * <nbdkitCaps>
> + * <path>/some/path</path>
> + * <nbdkitctime>234235253</nbdkitctime>
> + * <plugindirmtime>234235253</plugindirmtime>
> + * <filterdirmtime>234235253</filterdirmtime>
> + * <selfctime>234235253</selfctime>
> + * <selfvers>1002016</selfvers>
> + * <flag name='foo'/>
> + * <flag name='bar'/>
> + * ...
> + * </nbdkitCaps>
> + *
> + * Returns 0 on success, 1 if outdated, -1 on error
> + */
> +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 = virXMLParseFile(filename)))
I'll be posting soon patches that make 'virXMLParse' usable here without
the need to ...
> + return -1;
> +
> + if (!(ctxt = virXMLXPathContextNew(doc)))
> + return -1;
... fetch the context ...
> +
> + ctxt->node = xmlDocGetRootElement(doc);
... get the root node...
> +
> + if (STRNEQ((const char*)ctxt->node->name, "nbdkitCaps")) {
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unexpected root element <%s>, "
> + "expecting <nbdkitCaps>"),
> + ctxt->node->name);
> + return -1;
> + }
... and also do this validation by doing all of that in the virXMLParse
function.
> +
> + 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;
> +
> + 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,
> + (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)
> + nbdkitCaps->pluginDirMtime = (time_t)l;
> +
> + if (virXPathLongLong("string(./filterdirmtime)", ctxt, &l) == 0)
> + nbdkitCaps->filterDirMtime = (time_t)l;
Any reason why these are optional but the ctime of nbdkit is not?
> +
> + 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;
> + }
> +
> + 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>%llu</nbdkitctime>\n",
> + (long long)nbdkitCaps->ctime);
> + if (nbdkitCaps->pluginDirMtime > 0) {
> + virBufferAsprintf(&buf, "<plugindirmtime>%llu</plugindirmtime>\n",
The format substitution is specifying an unsigned long long ...
> + (long long)nbdkitCaps->pluginDirMtime);
while you typecast to long long. Similarly above in the parser you parse
as long long instead of unsigned long long.
> + }
> + if (nbdkitCaps->filterDirMtime > 0) {
> + virBufferAsprintf(&buf, "<filterdirmtime>%llu</filterdirmtime>\n",
> + (long long)nbdkitCaps->filterDirMtime);
> + }
> + virBufferAsprintf(&buf, "<selfctime>%llu</selfctime>\n",
> + (long long)nbdkitCaps->libvirtCtime);
> + virBufferAsprintf(&buf, "<selfvers>%lu</selfvers>\n",
> + (unsigned long)nbdkitCaps->libvirtVersion);
> +
> + 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.1
>
