[libvirt PATCH v2 02/16] qemu: Add qemuNbdkitCaps to qemu driver
Peter Krempa
pkrempa at redhat.com
Mon Sep 19 14:09:03 UTC 2022
On Wed, Aug 31, 2022 at 13:40:47 -0500, Jonathon Jongsma wrote:
> In future commits, we will optionally use nbdkit to serve some remote
> disk sources. This patch queries to see whether nbdkit is installed on
> the host and queries it for capabilities. These capabilities are stored
> in the qemu driver.
>
> Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
> ---
> po/POTFILES | 1 +
> src/qemu/meson.build | 1 +
> src/qemu/qemu_conf.h | 3 +
> src/qemu/qemu_driver.c | 3 +
> src/qemu/qemu_nbdkit.c | 226 +++++++++++++++++++++++++++++++++++++++++
> src/qemu/qemu_nbdkit.h | 51 ++++++++++
> 6 files changed, 285 insertions(+)
> create mode 100644 src/qemu/qemu_nbdkit.c
> create mode 100644 src/qemu/qemu_nbdkit.h
[...]
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 707f4cc1bb..943fa8621d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -832,6 +832,9 @@ qemuStateInitialize(bool privileged,
> defsecmodel)))
> goto error;
>
> + /* find whether nbdkit is available and query its capabilities */
> + qemu_driver->nbdkitCaps = qemuQueryNbdkitCaps();
This is leaked when qemu driver will be destroyed in qemuStateCleanup.
Additionally patch 7/16 basically reverts all of this and it is unused
inbetween of those commits. So I strongly suggest you simply add the new
file cache when it's ready.
> +
> /* If hugetlbfs is present, then we need to create a sub-directory within
> * it, since we can't assume the root mount point has permissions that
> * will let our spawned QEMU instances use it. */
> diff --git a/src/qemu/qemu_nbdkit.c b/src/qemu/qemu_nbdkit.c
> new file mode 100644
> index 0000000000..f55f68299f
> --- /dev/null
> +++ b/src/qemu/qemu_nbdkit.c
> @@ -0,0 +1,226 @@
> +/*
> + * qemu_nbdkit.c: helpers for using nbdkit
> + *
> + * Copyright (C) 2021 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#include <config.h>
> +#include <glib.h>
> +
> +#include "vircommand.h"
> +#include "virerror.h"
> +#include "virlog.h"
> +#include "virpidfile.h"
> +#include "virsecureerase.h"
> +#include "virutil.h"
> +#include "qemu_block.h"
> +#include "qemu_conf.h"
> +#include "qemu_domain.h"
> +#include "qemu_driver.h"
> +#include "qemu_extdevice.h"
> +#include "qemu_nbdkit.h"
> +#include "qemu_security.h"
> +
> +#include <fcntl.h>
> +
> +#define VIR_FROM_THIS VIR_FROM_QEMU
> +
> +VIR_LOG_INIT("qemu.nbdkit");
> +
> +VIR_ENUM_IMPL(qemuNbdkitCaps,
> + QEMU_NBDKIT_CAPS_LAST,
> + /* 0 */
> + "plugin-curl", /* QEMU_NBDKIT_CAPS_PLUGIN_CURL */
> + "plugin-ssh", /* QEMU_NBDKIT_CAPS_PLUGIN_SSH */
> + "filter-readahead", /* QEMU_NBDKIT_CAPS_FILTER_READAHEAD */
> +);
> +
> +struct _qemuNbdkitCaps {
> + GObject parent;
> +
> + char *path;
> + char *version;
> +
> + virBitmap *flags;
> +};
> +G_DEFINE_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, G_TYPE_OBJECT);
> +
> +
> +static void
> +qemuNbdkitCheckCommandCap(qemuNbdkitCaps *nbdkit,
> + virCommand *cmd,
> + qemuNbdkitCapsFlags cap)
> +{
> + if (virCommandRun(cmd, NULL) != 0)
> + return;
> +
> + VIR_DEBUG("Setting nbdkit capability %i", cap);
> + ignore_value(virBitmapSetBit(nbdkit->flags, cap));
> +}
> +
> +
> +static void
> +qemuNbdkitQueryFilter(qemuNbdkitCaps *nbdkit,
> + const char *filter,
> + qemuNbdkitCapsFlags cap)
> +{
> + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
> + "--version",
> + NULL);
> +
> + virCommandAddArgPair(cmd, "--filter", filter);
> +
> + /* use null plugin to check for filter */
> + virCommandAddArg(cmd, "null");
> +
> + qemuNbdkitCheckCommandCap(nbdkit, cmd, cap);
> +}
> +
> +
> +static void
> +qemuNbdkitQueryPlugin(qemuNbdkitCaps *nbdkit,
> + const char *plugin,
> + qemuNbdkitCapsFlags cap)
> +{
> + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
> + plugin,
> + "--version",
> + NULL);
> +
> + qemuNbdkitCheckCommandCap(nbdkit, cmd, cap);
> +}
> +
> +
> +static void
> +qemuNbdkitCapsQueryPlugins(qemuNbdkitCaps *nbdkit)
> +{
> + qemuNbdkitQueryPlugin(nbdkit, "curl", QEMU_NBDKIT_CAPS_PLUGIN_CURL);
> + qemuNbdkitQueryPlugin(nbdkit, "ssh", QEMU_NBDKIT_CAPS_PLUGIN_SSH);
> +}
> +
> +
> +static void
> +qemuNbdkitCapsQueryFilters(qemuNbdkitCaps *nbdkit)
> +{
> + qemuNbdkitQueryFilter(nbdkit, "readahead",
> + QEMU_NBDKIT_CAPS_FILTER_READAHEAD);
> +}
> +
> +
> +static int
> +qemuNbdkitCapsQueryVersion(qemuNbdkitCaps *nbdkit)
> +{
> + g_autoptr(virCommand) cmd = virCommandNewArgList(nbdkit->path,
> + "--version",
> + NULL);
> +
> + virCommandSetOutputBuffer(cmd, &nbdkit->version);
> +
> + if (virCommandRun(cmd, NULL) != 0)
> + return -1;
> +
> + VIR_DEBUG("Got nbdkit version %s", nbdkit->version);
> + return 0;
> +}
> +
> +
> +static void qemuNbdkitCapsFinalize(GObject *object)
Inconsistent header formatting with others in this file.
> +{
> + qemuNbdkitCaps *nbdkit = QEMU_NBDKIT_CAPS(object);
> +
> + g_clear_pointer(&nbdkit->path, g_free);
> + g_clear_pointer(&nbdkit->version, g_free);
> + g_clear_pointer(&nbdkit->flags, virBitmapFree);
> +
> + G_OBJECT_CLASS(qemu_nbdkit_caps_parent_class)->finalize(object);
> +}
> +
> +
> +void qemu_nbdkit_caps_init(qemuNbdkitCaps *caps)
Same here and also inconsistent capitalization format.
> +{
> + caps->flags = virBitmapNew(QEMU_NBDKIT_CAPS_LAST);
> + caps->version = NULL;
> +}
> +
> +
> +static void
> +qemu_nbdkit_caps_class_init(qemuNbdkitCapsClass *klass)
> +{
> + GObjectClass *obj = G_OBJECT_CLASS(klass);
> +
> + obj->finalize = qemuNbdkitCapsFinalize;
> +}
> +
> +
> +qemuNbdkitCaps *
> +qemuNbdkitCapsNew(const char *path)
> +{
> + qemuNbdkitCaps *caps = g_object_new(QEMU_TYPE_NBDKIT_CAPS, NULL);
> + caps->path = g_strdup(path);
> +
> + return caps;
> +}
> +
> +
> +static void
> +qemuNbdkitCapsQuery(qemuNbdkitCaps *caps)
> +{
> + qemuNbdkitCapsQueryPlugins(caps);
> + qemuNbdkitCapsQueryFilters(caps);
> + qemuNbdkitCapsQueryVersion(caps);
> +}
> +
> +
> +qemuNbdkitCaps *
> +qemuQueryNbdkitCaps(void)
> +{
> + qemuNbdkitCaps *caps = NULL;
> + g_autofree char *path = virFindFileInPath("nbdkit");
> +
> + if (!path)
> + return NULL;
> +
> + if (!virFileIsExecutable(path)) {
> + virReportError(VIR_ERR_INTERNAL_ERROR, _("nbdkit '%s' is not executable"),
> + path);
> + return NULL;
> + }
> +
> + VIR_DEBUG("found nbdkit executable '%s'", path);
> +
> + caps = qemuNbdkitCapsNew(path);
> + qemuNbdkitCapsQuery(caps);
> +
> + return caps;
> +}
> +
> +
> +bool
> +qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps,
> + qemuNbdkitCapsFlags flag)
> +{
> + return virBitmapIsBitSet(nbdkitCaps->flags, flag);
> +}
> +
> +
> +void
> +qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps,
> + qemuNbdkitCapsFlags flag)
> +{
> + ignore_value(virBitmapSetBit(nbdkitCaps->flags, flag));
> +}
> diff --git a/src/qemu/qemu_nbdkit.h b/src/qemu/qemu_nbdkit.h
> new file mode 100644
> index 0000000000..4baf4d4d31
> --- /dev/null
> +++ b/src/qemu/qemu_nbdkit.h
> @@ -0,0 +1,51 @@
> +/*
> + * qemu_nbdkit.h: helpers for using nbdkit
> + *
> + * Copyright (C) 2021 Red Hat, Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see
> + * <http://www.gnu.org/licenses/>.
> + *
> + */
> +
> +#pragma once
> +
> +#include "internal.h"
> +#include "virbitmap.h"
> +#include "vircgroup.h"
> +#include "vircommand.h"
> +#include "virstorageobj.h"
> +#include "viruri.h"
Nothing from these headers is used in the header file. Please don't
polute the headers with unnecessary includes. Stuff required for the
code should be included in the .c
> +
> +typedef struct _qemuNbdkitCaps qemuNbdkitCaps;
> +
> +typedef enum {
You should go for the same markers you have when implementing the enum.
> + QEMU_NBDKIT_CAPS_PLUGIN_CURL,
> + QEMU_NBDKIT_CAPS_PLUGIN_SSH,
> + QEMU_NBDKIT_CAPS_FILTER_READAHEAD,
> + QEMU_NBDKIT_CAPS_LAST,
> +} qemuNbdkitCapsFlags;
> +
> +VIR_ENUM_DECL(qemuNbdkitCaps);
> +
> +qemuNbdkitCaps* qemuQueryNbdkitCaps(void);
> +
> +qemuNbdkitCaps* qemuNbdkitCapsNew(const char *path);
> +
> +bool qemuNbdkitCapsGet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag);
> +
> +void qemuNbdkitCapsSet(qemuNbdkitCaps *nbdkitCaps, qemuNbdkitCapsFlags flag);
Please use the modern header formatting style (same as the function
headers in the .c file).
> +
> +#define QEMU_TYPE_NBDKIT_CAPS qemu_nbdkit_caps_get_type()
> +G_DECLARE_FINAL_TYPE(qemuNbdkitCaps, qemu_nbdkit_caps, QEMU, NBDKIT_CAPS, GObject);
> --
> 2.37.1
>
More information about the libvir-list
mailing list