[libvirt] [PATCH v1 10/19] qemu: Add vhost-scsi string for -device parameter

[Michal Privoznik]

On 25.07.2016 22:48, Eric Farman wrote:
> Open /dev/vhost-scsi, and record the resulting file descriptor, so that
> the guest has access to the host device outside of the libvirt daemon.
> Pass this information, along with data parsed from the XML file, to build
> a device string for the qemu command line.  That device string will be
> for either a vhost-scsi-ccw device in the case of an s390 machine, or
> vhost-scsi-pci for any others.
> 
> Signed-off-by: Eric Farman <farman at linux.vnet.ibm.com>
> Reviewed-by: Boris Fiuczynski <fiuczy at linux.vnet.ibm.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/qemu/qemu_command.c  | 67 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/qemu/qemu_command.h  |  5 ++++
>  src/util/virscsi.c       | 26 +++++++++++++++++++
>  src/util/virscsi.h       |  1 +
>  5 files changed, 99 insertions(+), 1 deletion(-)
> 
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 9396c4e..b91c7a8 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -2253,6 +2253,7 @@ virSCSIDeviceListNew;
>  virSCSIDeviceListSteal;
>  virSCSIDeviceNew;
>  virSCSIDeviceSetUsedBy;
> +virSCSIOpenVhost;
>  
>  
>  # util/virseclabel.h
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index dd15ff8..31b30a4 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4533,6 +4533,67 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
>  }
>  
>  char *
> +qemuBuildSCSIVhostHostdevDevStr(const virDomainDef *def,
> +                                virDomainHostdevDefPtr dev,
> +                                virQEMUCapsPtr qemuCaps,
> +                                virCommandPtr cmd)
> +{
> +    size_t i;
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
> +    virDomainHostdevSubsysSCSIVhostPtr vhostsrc = &scsisrc->u.vhost;
> +    int *vhostfd = NULL;
> +    size_t vhostfdSize = 1;
> +
> +    if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VHOST_SCSI)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                       _("This QEMU doesn't support vhost-scsi devices"));
> +        goto cleanup;
> +    }
> +
> +    if (ARCH_IS_S390(def->os.arch))
> +        virBufferAddLit(&buf, "vhost-scsi-ccw");
> +    else
> +        virBufferAddLit(&buf, "vhost-scsi-pci");
> +
> +    virBufferAsprintf(&buf, ",wwpn=%s", vhostsrc->wwpn);
> +
> +    if (VIR_ALLOC_N(vhostfd, vhostfdSize) < 0)
> +        goto cleanup;
> +
> +    memset(vhostfd, -1, sizeof(*vhostfd) * vhostfdSize);
> +
> +    if (virSCSIOpenVhost(vhostfd, &vhostfdSize) < 0)
> +        goto cleanup;
> +
> +    for (i = 0; i < vhostfdSize; i++) {
> +        if (cmd) {

If cmd == NULL here we are in way bigger trouble. Just drop the check.

> +            virCommandPassFD(cmd, vhostfd[i],
> +                             VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +        }
> +    }
> +
> +    if (vhostfdSize == 1) {
> +        virBufferAsprintf(&buf, ",vhostfd=%d", vhostfd[0]);
> +    } else {
> +        /* FIXME if 'vhostfds' became a valid vhost-scsi property in QEMU */
> +        goto cleanup;

Instead of failing here silently, we should report an error. I'll leave
the error message up to you ;-)

> +    }
> +
> +    virBufferAsprintf(&buf, ",id=%s", dev->info->alias);

Here we should check if there has been an error while constructing @buf.
For instance an OOM might have occurred.

    if (virBufferCheckError(&buf) < 0)
        goto cleanup;

> +
> +    VIR_FREE(vhostfd);
> +    return virBufferContentAndReset(&buf);
> +
> + cleanup:
> +    for (i = 0; vhostfd && i < vhostfdSize; i++)
> +        VIR_FORCE_CLOSE(vhostfd[i]);
> +    VIR_FREE(vhostfd);
> +    virBufferFreeAndReset(&buf);
> +    return NULL;
> +}

The rest looks okay. Perhaps, if you want you can introduce
virSCSIOpenVhost() in a separate patch. But only if you want. I don't
care that much.

Michal