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@xxxxxxxxxxxxxxxxxx> > --- > 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); > > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list