[libvirt] [PATCHv2 7/7] qemu: add support for vhost-vsock-pci

[Peter Krempa]

On Thu, May 24, 2018 at 12:39:15 +0200, Ján Tomko wrote:
> Create a new vsock endpoint by opening /dev/vhost-vsock,
> set the requested CID via ioctl (or assign a free one if auto='yes'),
> pass the file descriptor to QEMU and build the command line.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1291851
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  src/qemu/qemu_alias.c                              | 16 ++++++++
>  src/qemu/qemu_command.c                            | 45 ++++++++++++++++++++++
>  src/qemu/qemu_domain.c                             |  5 +++
>  src/qemu/qemu_domain.h                             |  2 +-
>  src/qemu/qemu_process.c                            | 35 +++++++++++++++++
>  .../vhost-vsock-auto.x86_64-latest.args            | 32 +++++++++++++++
>  .../vhost-vsock.x86_64-latest.args                 | 32 +++++++++++++++
>  tests/qemuxml2argvtest.c                           | 14 +++++++
>  8 files changed, 180 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock.x86_64-latest.args

[...]

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9da2d609e8..ef0716d683 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9865,6 +9865,47 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd,
>  }
>  
>  
> +static int
> +qemuBuildVsockCommandLine(virCommandPtr cmd,
> +                          virDomainDefPtr def,
> +                          virDomainVsockDefPtr vsock,
> +                          const char *fdprefix,

fdprefix is always empty string, so why is it necessary?

[1]

> +                          virQEMUCapsPtr qemuCaps)
> +{
> +    qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData;
> +    const char *device = "vhost-vsock-pci";
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    char *devstr = NULL;
> +    int ret = -1;
> +
> +    virBufferAsprintf(&buf, "%s", device);
> +    virBufferAsprintf(&buf, ",id=%s", vsock->info.alias);
> +    virBufferAsprintf(&buf, ",guest-cid=%u", vsock->guest_cid);
> +    virBufferAsprintf(&buf, ",vhostfd=%s%u", fdprefix, priv->vhostfd);
> +    if (qemuBuildDeviceAddressStr(&buf, def, &vsock->info, qemuCaps) < 0)
> +        goto cleanup;
> +#if 0
> +    if (qemuBuildVirtioOptionsStr(&buf, net->virtio, qemuCaps) < 0)
> +        goto error;
> +#endif

Leftover stuff from previous version?

> +
> +    if (virBufferCheckError(&buf) < 0)
> +        goto cleanup;
> +
> +    devstr = virBufferContentAndReset(&buf);
> +
> +    virCommandPassFD(cmd, priv->vhostfd, VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +    priv->vhostfd = -1;
> +    virCommandAddArgList(cmd, "-device", devstr, NULL);
> +
> +    ret = 0;
> + cleanup:
> +    virBufferFreeAndReset(&buf);
> +    VIR_FREE(devstr);
> +    return ret;
> +}
> +
> +
>  /*
>   * Constructs a argv suitable for launching qemu with config defined
>   * for a given virtual machine.
> @@ -10111,6 +10152,10 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
>              goto error;
>      }
>  
> +    if (def->vsock &&
> +        qemuBuildVsockCommandLine(cmd, def, def->vsock, "", qemuCaps) < 0)

[1]

> +        goto error;
> +
>      /* In some situations, eg. VFIO passthrough, QEMU might need to lock a
>       * significant amount of memory, so we need to set the limit accordingly */
>      virCommandSetMaxMemLock(cmd, qemuDomainGetMemLockLimitBytes(def));

[...]

> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index 174d932ae7..e318639963 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c

[...]

> @@ -5975,6 +5976,36 @@ qemuProcessPrepareHostStorage(virQEMUDriverPtr driver,
>  }
>  
>  
> +static int
> +qemuProcessOpenVhostVsock(virDomainVsockDefPtr vsock)
> +{
> +    qemuDomainVsockPrivatePtr priv = (qemuDomainVsockPrivatePtr)vsock->privateData;
> +    const char *vsock_path = "/dev/vhost-vsock";
> +    int fd;
> +
> +    if ((fd = open(vsock_path, O_RDWR)) < 0) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       "%s", _("unable to open vhost-vsock device"));
> +        return -1;
> +    }
> +
> +    if (vsock->guest_cid) {
> +        if (virVsockSetGuestCid(fd, vsock->guest_cid) < 0)
> +            goto error;
> +    } else {
> +        if (virVsockAcquireGuestCid(fd, &vsock->guest_cid) < 0)
> +            goto error;

This logic is wrong. You should base the decision on auto_cid, rather
than presence of guest_cid. If automatic cid is requested you should
always honour it.

Specifically this might create problem with migration as the cid that
was used on the source was already taken, but the user is okay with
autogenerating it.

Or if the cid actually can't change, you should make it part of the ABI
stability check.

> +    }
> +
> +    priv->vhostfd = fd;
> +    return 0;
> +
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180529/252a80c1/attachment-0001.sig>