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

John Ferlan jferlan at redhat.com
Fri Nov 11 21:47:02 UTC 2016



On 11/08/2016 01:26 PM, 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>
> ---
>  src/qemu/qemu_cgroup.c         | 32 +++++++++++++++++
>  src/qemu/qemu_command.c        | 79 ++++++++++++++++++++++++++++++++++++++++++
>  src/qemu/qemu_command.h        |  5 +++
>  src/qemu/qemu_domain_address.c | 10 ++++++
>  src/qemu/qemu_hostdev.c        | 41 ++++++++++++++++++++++
>  src/qemu/qemu_hostdev.h        |  8 +++++
>  6 files changed, 175 insertions(+)
> 

Beyond the aforementioned "Host" to "SCSIHost" that will need to persist
into here...

> diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
> index ee31d14..a22a1bf 100644
> --- a/src/qemu/qemu_cgroup.c
> +++ b/src/qemu/qemu_cgroup.c
> @@ -277,6 +277,25 @@ qemuSetupHostSCSIDeviceCgroup(virSCSIDevicePtr dev ATTRIBUTE_UNUSED,
>      return ret;
>  }
>  
> +static int
> +qemuSetupHostHostDeviceCgroup(virHostDevicePtr dev ATTRIBUTE_UNUSED,
> +                              const char *path,
> +                              void *opaque)
> +{
> +    virDomainObjPtr vm = opaque;
> +    qemuDomainObjPrivatePtr priv = vm->privateData;
> +    int ret;
> +
> +    VIR_DEBUG("Process path '%s' for scsi_host device", path);
> +
> +    ret = virCgroupAllowDevicePath(priv->cgroup, path,
> +                                   VIR_CGROUP_DEVICE_RW, false);
> +
> +    virDomainAuditCgroupPath(vm, priv->cgroup, "allow", path, "rw", ret == 0);
> +
> +    return ret;
> +}
> +
>  int
>  qemuSetupHostdevCgroup(virDomainObjPtr vm,
>                         virDomainHostdevDefPtr dev)
> @@ -286,9 +305,11 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
>      virDomainHostdevSubsysUSBPtr usbsrc = &dev->source.subsys.u.usb;
>      virDomainHostdevSubsysPCIPtr pcisrc = &dev->source.subsys.u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &dev->source.subsys.u.scsi;
> +    virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
>      virPCIDevicePtr pci = NULL;
>      virUSBDevicePtr usb = NULL;
>      virSCSIDevicePtr scsi = NULL;
> +    virHostDevicePtr host = NULL;
>      char *path = NULL;
>  
>      /* currently this only does something for PCI devices using vfio
> @@ -377,6 +398,16 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
>          }
>  
>          case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST: {
> +            if (hostsrc->protocol ==
> +                VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST) {
> +                if ((host = virHostDeviceNew(hostsrc->wwpn)) == NULL)

if (!host = vir...()))

is the more commonly used approach although yes I see you copied SCSI

> +                    goto cleanup;
> +
> +                if (virHostDeviceFileIterate(host,
> +                                             qemuSetupHostHostDeviceCgroup,
> +                                             vm) < 0)
> +                    goto cleanup;
> +            }
>              break;
>          }
>  
> @@ -390,6 +421,7 @@ qemuSetupHostdevCgroup(virDomainObjPtr vm,
>      virPCIDeviceFree(pci);
>      virUSBDeviceFree(usb);
>      virSCSIDeviceFree(scsi);
> +    virHostDeviceFree(host);
>      VIR_FREE(path);
>      return ret;
>  }
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 9adf0fe..ecd3286 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4730,6 +4730,43 @@ qemuBuildSCSIiSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
>  }
>  
>  char *
> +qemuBuildHostHostdevDevStr(const virDomainDef *def,
> +                           virDomainHostdevDefPtr dev,
> +                           virQEMUCapsPtr qemuCaps,
> +                           char *vhostfdName)
> +{
> +    virBuffer buf = VIR_BUFFER_INITIALIZER;
> +    virDomainHostdevSubsysHostPtr hostsrc = &dev->source.subsys.u.host;
> +
> +    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", hostsrc->wwpn);
> +    virBufferAsprintf(&buf, ",vhostfd=%s", vhostfdName);
> +    virBufferAsprintf(&buf, ",id=%s", dev->info->alias);

These could be combined into one formatted print...
> +
> +    if (qemuBuildDeviceAddressStr(&buf, def, dev->info, qemuCaps) < 0)

and yet no AddressStr is added to the command line - so we're missing a
place to configure that.

> +        goto cleanup;
> +
> +    if (virBufferCheckError(&buf) < 0)
> +        goto cleanup;
> +
> +    return virBufferContentAndReset(&buf);
> +
> + cleanup:
> +    virBufferFreeAndReset(&buf);
> +    return NULL;
> +}
> +
> +char *
>  qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev)
>  {
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
> @@ -5210,6 +5247,48 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd,
>                  return -1;
>              }
>          }
> +
> +        /* SCSI_host */
> +        if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +            subsys->type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) {
> +            if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_SCSI_GENERIC)) {

Is this the right check? I guess it's unclear why scsi_generic is needed
- is that necessary for vhost-scsi?  I know it's used in order to pass
SCSI LUN's to the guest (eg the SCSI hostdev code).

> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                               _("SCSI passthrough is not supported by this "
> +                                 "version of qemu"));
> +                return -1;
> +            }
> +
> +            if (hostdev->source.subsys.u.host.protocol ==
> +                VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST) {
> +                char *vhostfdName = NULL;

Once I read ahead to patch 6 this made more sense - although it's a bit
strange to read...  This is essentially following the PCI 'configfd'
processing code.

> +                int vhostfd = -1;
> +
> +                if (virHostOpenVhostSCSI(&vhostfd) < 0)
> +                    return -1;
> +
> +                if (virAsprintf(&vhostfdName, "%d", vhostfd) < 0) {
> +                    VIR_FORCE_CLOSE(vhostfd);
> +                    return -1;
> +                }
> +
> +                virCommandPassFD(cmd, vhostfd,
> +                                 VIR_COMMAND_PASS_FD_CLOSE_PARENT);
> +
> +                virCommandAddArg(cmd, "-device");
> +                if (!(devstr = qemuBuildHostHostdevDevStr(def,
> +                                                          hostdev,
> +                                                          qemuCaps,
> +                                                          vhostfdName))) {
> +                    VIR_FREE(vhostfdName);
> +                    VIR_FORCE_CLOSE(vhostfd);
> +                    return -1;
> +                }
> +                virCommandAddArg(cmd, devstr);
> +
> +                VIR_FREE(vhostfdName);
> +                VIR_FREE(devstr);
> +            }
> +        }
>      }
>  
>      return 0;
> diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
> index 0ddaba4..0abf31e 100644
> --- a/src/qemu/qemu_command.h
> +++ b/src/qemu/qemu_command.h
> @@ -163,6 +163,11 @@ char *qemuBuildSCSIHostdevDrvStr(virDomainHostdevDefPtr dev);
>  char *qemuBuildSCSIHostdevDevStr(const virDomainDef *def,
>                                   virDomainHostdevDefPtr dev,
>                                   virQEMUCapsPtr qemuCaps);
> +char *
> +qemuBuildHostHostdevDevStr(const virDomainDef *def,
> +                           virDomainHostdevDefPtr dev,
> +                           virQEMUCapsPtr qemuCaps,
> +                           char *vhostfdName);
>  
>  char *qemuBuildRedirdevDevStr(const virDomainDef *def,
>                                virDomainRedirdevDefPtr dev,
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index b35a95f..90d3357 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -312,6 +312,16 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def,

Is this CCW addresses are typically assigned? It just seems odd that a
"PrimeVirtioDeviceAddresses" is being used to prime the vhost-scsi-ccw
address.  It just doesn't feel like the right place.  Certain the PCI
address isn't being set here as only TYPE_VIRTIO_CCW or _MMIO is passed.

Theoretically speaking it wouldn't be necessary if the address was
assigned during device post parse...

John

>          }
>      }
>  
> +    for (i = 0; i < def->nhostdevs; i++) {
> +        if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +            def->hostdevs[i]->source.subsys.type ==
> +            VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST &&
> +            def->hostdevs[i]->source.subsys.u.host.protocol ==
> +            VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST &&
> +            def->hostdevs[i]->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> +            def->hostdevs[i]->info->type = type;
> +    }
> +
>      if (def->memballoon &&
>          def->memballoon->model == VIR_DOMAIN_MEMBALLOON_MODEL_VIRTIO &&
>          def->memballoon->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
> diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
> index dd3a3cf..29aedeb 100644
> --- a/src/qemu/qemu_hostdev.c
> +++ b/src/qemu/qemu_hostdev.c
> @@ -292,6 +292,17 @@ qemuHostdevPrepareSCSIDevices(virQEMUDriverPtr driver,
>                                          name, hostdevs, nhostdevs);
>  }
>  
> +int
> +qemuHostdevPrepareHostDevices(virQEMUDriverPtr driver,
> +                              const char *name,
> +                              virDomainHostdevDefPtr *hostdevs,
> +                              int nhostdevs)
> +{
> +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +    return virHostdevPrepareHostDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> +                                        name, hostdevs, nhostdevs);
> +}
>  
>  int
>  qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
> @@ -315,6 +326,10 @@ qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                        def->hostdevs, def->nhostdevs) < 0)
>          return -1;
>  
> +    if (qemuHostdevPrepareHostDevices(driver, def->name,
> +                                      def->hostdevs, def->nhostdevs) < 0)
> +        return -1;
> +
>      return 0;
>  }
>  
> @@ -370,6 +385,29 @@ qemuHostdevReAttachSCSIDevices(virQEMUDriverPtr driver,
>  }
>  
>  void
> +qemuHostdevReAttachHostDevices(virQEMUDriverPtr driver,
> +                               const char *name,
> +                               virDomainHostdevDefPtr *hostdevs,
> +                               int nhostdevs)
> +{
> +    size_t i;
> +    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> +
> +    for (i = 0; i < nhostdevs; i++) {
> +        virDomainHostdevDefPtr hostdev = hostdevs[i];
> +        virDomainDeviceDef dev;
> +
> +        dev.type = VIR_DOMAIN_DEVICE_HOSTDEV;
> +        dev.data.hostdev = hostdev;
> +
> +        ignore_value(qemuRemoveSharedDevice(driver, &dev, name));
> +    }
> +
> +    virHostdevReAttachHostDevices(hostdev_mgr, QEMU_DRIVER_NAME,
> +                                  name, hostdevs, nhostdevs);
> +}
> +
> +void
>  qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>                                   virDomainDefPtr def)
>  {
> @@ -384,4 +422,7 @@ qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>  
>      qemuHostdevReAttachSCSIDevices(driver, def->name, def->hostdevs,
>                                     def->nhostdevs);
> +
> +    qemuHostdevReAttachHostDevices(driver, def->name, def->hostdevs,
> +                                   def->nhostdevs);
>  }
> diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h
> index 0a3c715..1788d12 100644
> --- a/src/qemu/qemu_hostdev.h
> +++ b/src/qemu/qemu_hostdev.h
> @@ -55,6 +55,10 @@ int qemuHostdevPrepareSCSIDevices(virQEMUDriverPtr driver,
>                                    const char *name,
>                                    virDomainHostdevDefPtr *hostdevs,
>                                    int nhostdevs);
> +int qemuHostdevPrepareHostDevices(virQEMUDriverPtr driver,
> +                                  const char *name,
> +                                  virDomainHostdevDefPtr *hostdevs,
> +                                  int nhostdevs);
>  int qemuHostdevPrepareDomainDevices(virQEMUDriverPtr driver,
>                                      virDomainDefPtr def,
>                                      virQEMUCapsPtr qemuCaps,
> @@ -72,6 +76,10 @@ void qemuHostdevReAttachSCSIDevices(virQEMUDriverPtr driver,
>                                      const char *name,
>                                      virDomainHostdevDefPtr *hostdevs,
>                                      int nhostdevs);
> +void qemuHostdevReAttachHostDevices(virQEMUDriverPtr driver,
> +                                    const char *name,
> +                                    virDomainHostdevDefPtr *hostdevs,
> +                                    int nhostdevs);
>  void qemuHostdevReAttachDomainDevices(virQEMUDriverPtr driver,
>                                        virDomainDefPtr def);
>  
> 




More information about the libvir-list mailing list