On 04/13/2016 03:15 AM, Olaf Hering wrote: > This uses the API version of the proposed vscsi support in libxl (v12): > http://lists.xenproject.org/archives/html/xen-devel/2016-04/msg01772.html We'll need to wait for that to land in xen.git before committing libxl scsi support to libvirt.git, but thanks for putting the current work up for review. > Is there anything else that needs to be done in libvirt? We'll need code in src/xenconfig/xen_xl.* to convert xl vscis cfg <-> libvirt domXML, along with tests in tests/xlconfigtest. > Right now libvirt scsi > support is very simple minded, no support at all to describe host devices with > persistant names. Yes, I think you are correct, but I'm not aware of any reason that support couldn't be added. > Example used during testing: > > <hostdev mode='subsystem' type='scsi' managed='no' sgio='filtered' rawio='yes'> IIRC, the 'managed' attribute only applies to PCI hostdevs. > <source> > <adapter name='scsi_host5'/> > <address bus='0' target='1' unit='0'/> > </source> > <readonly/> > <address type='drive' controller='0' bus='0' target='0' unit='0'/> > </hostdev> > > Signed-off-by: Olaf Hering <olaf@xxxxxxxxx> > Cc: Jim Fehlig <jfehlig@xxxxxxxx> > --- > src/libxl/libxl_conf.c | 59 +++++++++++++++ > src/libxl/libxl_conf.h | 1 + > src/libxl/libxl_domain.c | 2 +- > src/libxl/libxl_driver.c | 187 +++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 248 insertions(+), 1 deletion(-) Needs rebased against current libvirt.git master. > > diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c > index f5ef50f..1e3615e 100644 > --- a/src/libxl/libxl_conf.c > +++ b/src/libxl/libxl_conf.c > @@ -1915,6 +1915,61 @@ libxlMakePCIList(virDomainDefPtr def, libxl_domain_config *d_config) > } > > static int > +libxlMakeVscsiList(libxl_ctx *ctx, > + XLU_Config *xlu, > + virDomainDefPtr def, > + libxl_domain_config *d_config) > +{ > + virDomainHostdevDefPtr *l_hostdevs = def->hostdevs; > + size_t i, nhostdevs = def->nhostdevs; > + virDomainHostdevDefPtr hostdev; > + virDomainHostdevSubsysSCSIPtr scsisrc; > + char *str; > + int rc = 0; > + > + if (nhostdevs == 0) > + return 0; > + > + for (i = 0; i < nhostdevs; i++) { > + hostdev = l_hostdevs[i]; > + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) > + continue; > + if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI) > + continue; > + scsisrc = &hostdev->source.subsys.u.scsi; > + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) > + continue; > +#if defined(LIBXL_HAVE_VSCSI) > + if (virAsprintf(&str, "%s:%u:%u:%u,%u:%u:%u:%llu%s", > + scsisrc->u.host.adapter + strlen("scsi_host"), > + scsisrc->u.host.bus, > + scsisrc->u.host.target, > + scsisrc->u.host.unit, > + hostdev->info->addr.drive.controller, > + hostdev->info->addr.drive.bus, > + hostdev->info->addr.drive.target, > + hostdev->info->addr.drive.unit, > + scsisrc->rawio == VIR_TRISTATE_BOOL_YES ? ",feature-host" : "") < 0) { > + goto error; > + }; > + rc = xlu_vscsi_config_add(xlu, ctx, str, &d_config->num_vscsictrls, &d_config->vscsictrls); > + VIR_FREE(str); > + if (rc) > + goto error; > +#else > + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", > + _("This version of libxenlight does not support vscsi")); > + goto error; > +#endif > + } > + > + return 0; > + > + error: > + return -1; > +} > + > +static int > libxlMakeVideo(virDomainDefPtr def, libxl_domain_config *d_config) > > { > @@ -2059,6 +2114,7 @@ int > libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > virDomainDefPtr def, > libxl_ctx *ctx, > + XLU_Config *xlu, > libxl_domain_config *d_config) > { > libxl_domain_config_init(d_config); > @@ -2084,6 +2140,9 @@ libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > if (libxlMakePCIList(def, d_config) < 0) > return -1; > > + if (libxlMakeVscsiList(ctx, xlu, def, d_config) < 0) > + return -1; > + > /* > * Now that any potential VFBs are defined, update the build info with > * the data of the primary display. Some day libxl might implicitely do > diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h > index b069e45..422c1d6 100644 > --- a/src/libxl/libxl_conf.h > +++ b/src/libxl/libxl_conf.h > @@ -218,6 +218,7 @@ int > libxlBuildDomainConfig(virPortAllocatorPtr graphicsports, > virDomainDefPtr def, > libxl_ctx *ctx, > + XLU_Config *xlu, > libxl_domain_config *d_config); > > static inline void > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c > index aed904b..02379c9 100644 > --- a/src/libxl/libxl_domain.c > +++ b/src/libxl/libxl_domain.c > @@ -1051,7 +1051,7 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm, > VIR_FREE(priv->lockState); > > if (libxlBuildDomainConfig(driver->reservedGraphicsPorts, vm->def, > - cfg->ctx, &d_config) < 0) > + cfg->ctx, cfg->xlu, &d_config) < 0) > goto cleanup_dom; > > if (cfg->autoballoon && libxlDomainFreeMem(cfg->ctx, &d_config) < 0) > diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c > index bf97c9c..186f9d4 100644 > --- a/src/libxl/libxl_driver.c > +++ b/src/libxl/libxl_driver.c > @@ -3024,6 +3024,117 @@ libxlDomainAttachHostPCIDevice(libxlDriverPrivatePtr driver, > } > > static int > +libxlDomainAttachHostSCSIDevice(libxlDriverPrivatePtr driver, > + virDomainObjPtr vm, > + virDomainHostdevDefPtr hostdev) > +{ > +#if defined(LIBXL_HAVE_VSCSI) > + libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); > + libxl_device_vscsictrl ctrl, existing; > + libxl_device_vscsidev dev; > + bool found_existing; > + virDomainHostdevDefPtr found; > + virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr; > + virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi; > + char *str = NULL; > + int ret = -1; > + > + if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) > + return -1; > + > + libxl_device_vscsictrl_init(&existing); > + libxl_device_vscsictrl_init(&ctrl); > + libxl_device_vscsidev_init(&dev); > + > + if (virDomainHostdevFind(vm->def, hostdev, &found) >= 0) { > + virReportError(VIR_ERR_OPERATION_FAILED, > + _("target scsi device %u:%u:%u:%llu already exists"), > + hostdev->info->addr.drive.controller, > + hostdev->info->addr.drive.bus, > + hostdev->info->addr.drive.target, > + hostdev->info->addr.drive.unit); > + goto cleanup; > + } > + > + if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) > + goto cleanup; > + > + if (virHostdevPrepareSCSIDevices(hostdev_mgr, LIBXL_DRIVER_NAME, > + vm->def->name, &hostdev, 1) < 0) > + goto cleanup; > + > + if (virAsprintf(&str, "%s:%u:%u:%u,%u:%u:%u:%llu%s", > + scsisrc->u.host.adapter + strlen("scsi_host"), > + scsisrc->u.host.bus, > + scsisrc->u.host.target, > + scsisrc->u.host.unit, > + hostdev->info->addr.drive.controller, > + hostdev->info->addr.drive.bus, > + hostdev->info->addr.drive.target, > + hostdev->info->addr.drive.unit, > + scsisrc->rawio == VIR_TRISTATE_BOOL_YES ? > + ",feature-host" : "") < 0) { It feels like the tedious creation of these strings should be done by a helper function (similar to virSCSIDeviceGetSgName) instead of repeated code. Otherwise looking good. Regards, Jim -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list