Hi
On Tue, Aug 27, 2019 at 12:30 AM Cole Robinson <crobinso(a)redhat.com> wrote:
On 8/23/19 12:21 PM, Cole Robinson wrote:
> From: Marc-André Lureau <marcandre.lureau(a)redhat.com>
>
> Add qemuVhostUserFetchConfigs() to discover vhost-user helpers.
>
> qemuVhostUserFillDomainGPU() will find the first matching GPU helper
> with the required capabilities and set the associated
> vhost_user_binary.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau(a)redhat.com>
> Signed-off-by: Cole Robinson <crobinso(a)redhat.com>
> ---
> src/conf/device_conf.c | 1 +
> src/conf/device_conf.h | 1 +
> src/qemu/Makefile.inc.am | 2 +
> src/qemu/qemu_vhost_user.c | 394 ++++++++++++++++++
> src/qemu/qemu_vhost_user.h | 48 +++
> tests/Makefile.am | 9 +
> .../etc/qemu/vhost-user/40-gpu.json | 1 +
> .../etc/qemu/vhost-user/50-gpu.json | 0
> .../qemu/vhost-user/test-vhost-user-gpu | 11 +
> .../usr/share/qemu/vhost-user/30-gpu.json | 1 +
> .../usr/share/qemu/vhost-user/50-gpu.json | 8 +
> .../usr/share/qemu/vhost-user/60-gpu.json | 1 +
> tests/qemuvhostusertest.c | 132 ++++++
> 13 files changed, 609 insertions(+)
> create mode 100644 src/qemu/qemu_vhost_user.c
> create mode 100644 src/qemu/qemu_vhost_user.h
> create mode 120000 tests/qemuvhostuserdata/etc/qemu/vhost-user/40-gpu.json
> create mode 100644 tests/qemuvhostuserdata/etc/qemu/vhost-user/50-gpu.json
> create mode 100755
tests/qemuvhostuserdata/usr/libexec/qemu/vhost-user/test-vhost-user-gpu
> create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/30-gpu.json
> create mode 100644 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/50-gpu.json
> create mode 120000 tests/qemuvhostuserdata/usr/share/qemu/vhost-user/60-gpu.json
> create mode 100644 tests/qemuvhostusertest.c
>
> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
> index 4c57f0995f..2f7077ddc4 100644
> --- a/src/conf/device_conf.c
> +++ b/src/conf/device_conf.c
> @@ -96,6 +96,7 @@ virDomainDeviceInfoClear(virDomainDeviceInfoPtr info)
> VIR_FREE(info->loadparm);
> info->isolationGroup = 0;
> info->isolationGroupLocked = false;
> + VIR_FREE(info->vhost_user_binary);
> }
>
> void
> diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h
> index d0854925e3..0b0c525ad8 100644
> --- a/src/conf/device_conf.h
> +++ b/src/conf/device_conf.h
> @@ -179,6 +179,7 @@ struct _virDomainDeviceInfo {
> * cases we might want to prevent that from happening by
> * locking the isolation group */
> bool isolationGroupLocked;
> + char *vhost_user_binary;
> };
>
> void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info);
> diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am
> index f7a0fa4a84..18a9220d01 100644
> --- a/src/qemu/Makefile.inc.am
> +++ b/src/qemu/Makefile.inc.am
> @@ -60,6 +60,8 @@ QEMU_DRIVER_SOURCES = \
> qemu/qemu_qapi.h \
> qemu/qemu_tpm.c \
> qemu/qemu_tpm.h \
> + qemu/qemu_vhost_user.c \
> + qemu/qemu_vhost_user.h \
> $(NULL)
>
>
> diff --git a/src/qemu/qemu_vhost_user.c b/src/qemu/qemu_vhost_user.c
> new file mode 100644
> index 0000000000..7a97052ab9
> --- /dev/null
> +++ b/src/qemu/qemu_vhost_user.c
> @@ -0,0 +1,394 @@
> +/*
> + * qemu_vhost_user.c: QEMU vhost-user
> + *
> + * Copyright (C) 2019 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 "qemu_vhost_user.h"
> +#include "qemu_configs.h"
> +#include "virjson.h"
> +#include "virlog.h"
> +#include "virstring.h"
> +#include "viralloc.h"
> +#include "virenum.h"
> +
> +#define VIR_FROM_THIS VIR_FROM_QEMU
> +
> +VIR_LOG_INIT("qemu.qemu_vhost_user");
> +
> +typedef enum {
> + QEMU_VHOST_USER_TYPE_NONE = 0,
> + QEMU_VHOST_USER_TYPE_9P,
> + QEMU_VHOST_USER_TYPE_BALLOON,
> + QEMU_VHOST_USER_TYPE_BLOCK,
> + QEMU_VHOST_USER_TYPE_CAIF,
> + QEMU_VHOST_USER_TYPE_CONSOLE,
> + QEMU_VHOST_USER_TYPE_CRYPTO,
> + QEMU_VHOST_USER_TYPE_GPU,
> + QEMU_VHOST_USER_TYPE_INPUT,
> + QEMU_VHOST_USER_TYPE_NET,
> + QEMU_VHOST_USER_TYPE_RNG,
> + QEMU_VHOST_USER_TYPE_RPMSG,
> + QEMU_VHOST_USER_TYPE_RPROC_SERIAL,
> + QEMU_VHOST_USER_TYPE_SCSI,
> + QEMU_VHOST_USER_TYPE_VSOCK,
> +
> + QEMU_VHOST_USER_TYPE_LAST
> +} qemuVhostUserType;
> +
> +VIR_ENUM_DECL(qemuVhostUserType);
> +VIR_ENUM_IMPL(qemuVhostUserType,
> + QEMU_VHOST_USER_TYPE_LAST,
> + "",
> + "9p",
> + "balloon",
> + "block",
> + "caif",
> + "console",
> + "crypto",
> + "gpu",
> + "input",
> + "net",
> + "rng",
> + "rpmsg",
> + "rproc-serial",
> + "scsi",
> + "vsock",
> +);
> +
> +typedef enum {
> + QEMU_VHOST_USER_GPU_FEATURE_NONE = 0,
> + QEMU_VHOST_USER_GPU_FEATURE_VIRGL,
> + QEMU_VHOST_USER_GPU_FEATURE_RENDER_NODE,
> +
> + QEMU_VHOST_USER_GPU_FEATURE_LAST
> +} qemuVhostUserGPUFeature;
> +
> +VIR_ENUM_DECL(qemuVhostUserGPUFeature);
> +VIR_ENUM_IMPL(qemuVhostUserGPUFeature,
> + QEMU_VHOST_USER_GPU_FEATURE_LAST,
> + "",
> + "virgl",
> + "render-node",
> +);
> +
> +typedef struct _qemuVhostUserGPU qemuVhostUserGPU;
> +typedef qemuVhostUserGPU *qemuVhostUserGPUPtr;
> +struct _qemuVhostUserGPU {
> + size_t nfeatures;
> + qemuVhostUserGPUFeature *features;
> +};
> +
> +struct _qemuVhostUser {
> + /* Description intentionally not parsed. */
> + qemuVhostUserType type;
> + char *binary;
> + /* Tags intentionally not parsed. */
> +
> + union {
> + qemuVhostUserGPU gpu;
> + } capabilities;
> +};
> +
> +static void
> +qemuVhostUserGPUFeatureFree(qemuVhostUserGPUFeature *features)
> +{
> + VIR_FREE(features);
> +}
> +
> +
Inconsistent double newline spacing here. But double newline seems to be
more common for new code so I suggest standardizing on that.
> +VIR_DEFINE_AUTOPTR_FUNC(qemuVhostUserGPUFeature, qemuVhostUserGPUFeatureFree);
> +
> +void
> +qemuVhostUserFree(qemuVhostUserPtr vu)
> +{
> + if (!vu)
> + return;
> +
> + if (vu->type == QEMU_VHOST_USER_TYPE_GPU)
> + VIR_FREE(vu->capabilities.gpu.features);
> +
> + VIR_FREE(vu->binary);
> +
> + VIR_FREE(vu);
> +}
> +
> +/* 1MiB should be enough for everybody (TM) */
> +#define DOCUMENT_SIZE (1024 * 1024)
> +
Since this is identical to qemu_firmware.c, maybe give it a proper name
and move it to the shared file?
QEMU_INTEROP_DOCUMENT_SIZE ? (not sure it's really worth it, I am not
changing it yet)
Otherwise this and the preceding uncommented patches look okay to me,
the behavior here largely mirrors qemu_firmware.c