[libvirt] [PATCH libvirt v2 2/2] libxl: support vscsi
Jim Fehlig
jfehlig at suse.com
Fri Apr 15 22:20:51 UTC 2016
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 at aepfle.de>
> Cc: Jim Fehlig <jfehlig at suse.com>
> ---
> 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
More information about the libvir-list
mailing list