diff mbox

[libvirt,v2,2/2] libxl: support vscsi

Message ID 1460538959-23584-3-git-send-email-olaf@aepfle.de (mailing list archive)
State New, archived
Headers show

Commit Message

Olaf Hering April 13, 2016, 9:15 a.m. UTC 
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

Is there anything else that needs to be done in libvirt? Right now libvirt scsi
support is very simple minded, no support at all to describe host devices with
persistant names. Example used during testing:

 <hostdev mode='subsystem' type='scsi' managed='no' sgio='filtered' rawio='yes'>
  <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@aepfle.de>
Cc: Jim Fehlig <jfehlig@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(-)

Comments

Olaf Hering April 13, 2016, 9:20 a.m. UTC | #1 
On Wed, Apr 13, Olaf Hering wrote:

>  <hostdev mode='subsystem' type='scsi' managed='no' sgio='filtered' rawio='yes'>
>   <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>

What is the virsh equivalent of the following command?

xl scsi-attach domU [5:0:1:0|/dev/sdd|naa.23fd7b9cc41c4934.0] 0:0:0:0


Olaf
Jim Fehlig April 15, 2016, 10:20 p.m. UTC | #2 
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@aepfle.de>
> Cc: Jim Fehlig <jfehlig@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
diff mbox

Patch

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) {
+        goto error;
+    };
+
+    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", str);
+
+    if (xlu_vscsi_get_ctrl(cfg->xlu, cfg->ctx, vm->def->id, str, &ctrl, &dev, &existing, &found_existing) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("libxutil failed to parse scsi device %u:%u:%u:%llu"),
+                       hostdev->info->addr.drive.controller,
+                       hostdev->info->addr.drive.bus,
+                       hostdev->info->addr.drive.target,
+                       hostdev->info->addr.drive.unit);
+    }
+
+    /* Finally add the device */
+    if (found_existing) {
+        if (libxl_device_vscsidev_add(cfg->ctx, vm->def->id, &dev, NULL)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("libxenlight failed to attach scsi device %u:%u:%u:%llu"),
+                           hostdev->info->addr.drive.controller,
+                           hostdev->info->addr.drive.bus,
+                           hostdev->info->addr.drive.target,
+                           hostdev->info->addr.drive.unit);
+            goto error;
+        }
+    } else {
+        libxl_device_vscsictrl_append_vscsidev(cfg->ctx, &ctrl, &dev);
+        if (libxl_device_vscsictrl_add(cfg->ctx, vm->def->id, &ctrl, NULL)) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("libxenlight failed to attach scsi device %u:%u:%u:%llu"),
+                           hostdev->info->addr.drive.controller,
+                           hostdev->info->addr.drive.bus,
+                           hostdev->info->addr.drive.target,
+                           hostdev->info->addr.drive.unit);
+            goto error;
+        }
+    }
+
+    vm->def->hostdevs[vm->def->nhostdevs++] = hostdev;
+    ret = 0;
+    goto cleanup;
+
+ error:
+    virHostdevReAttachSCSIDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+                                  vm->def->name, &hostdev, 1);
+
+ cleanup:
+    VIR_FREE(str);
+    virObjectUnref(cfg);
+    libxl_device_vscsictrl_dispose(&existing);
+    libxl_device_vscsictrl_dispose(&ctrl);
+    libxl_device_vscsidev_dispose(&dev);
+    return ret;
+#else
+    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                   _("This version of libxenlight does not support vscsi"));
+    return -1;
+#endif
+}
+
+static int
 libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
                             virDomainObjPtr vm,
                             virDomainHostdevDefPtr hostdev)
@@ -3041,6 +3152,11 @@  libxlDomainAttachHostDevice(libxlDriverPrivatePtr driver,
             return -1;
         break;
 
+    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+        if (libxlDomainAttachHostSCSIDevice(driver, vm, hostdev) < 0)
+            return -1;
+        break;
+
     default:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("hostdev subsys type '%s' not supported"),
@@ -3385,6 +3501,74 @@  libxlDomainDetachHostPCIDevice(libxlDriverPrivatePtr driver,
 }
 
 static int
+libxlDomainDetachHostSCSIDevice(libxlDriverPrivatePtr driver,
+                               virDomainObjPtr vm,
+                               virDomainHostdevDefPtr hostdev)
+{
+#if defined(LIBXL_HAVE_VSCSI)
+    libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver);
+    virDomainHostdevDefPtr detach;
+    int idx;
+    virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
+    virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
+    int ret = -1;
+    char *str = NULL;
+
+    if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI)
+        return -1;
+
+    idx = virDomainHostdevFind(vm->def, hostdev, &detach);
+    if (idx < 0) {
+        virReportError(VIR_ERR_OPERATION_FAILED,
+                       _("target scsi device %u:%u:%u:%u not found"),
+                       hostdev->info->addr.drive.controller,
+                       hostdev->info->addr.drive.bus,
+                       hostdev->info->addr.drive.target,
+                       hostdev->info->addr.drive.unit);
+        goto cleanup;
+    }
+
+    if (virAsprintf(&str, "%u:%u:%u:%u",
+                    hostdev->info->addr.drive.controller,
+                    hostdev->info->addr.drive.bus,
+                    hostdev->info->addr.drive.target,
+                    hostdev->info->addr.drive.unit) < 0) {
+        goto error;
+    };
+
+    if (xlu_vscsi_detach(cfg->xlu, cfg->ctx, vm->def->id, str) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("libxenlight failed to detach pci device %u:%u:%u:%u"),
+                       hostdev->info->addr.drive.controller,
+                       hostdev->info->addr.drive.bus,
+                       hostdev->info->addr.drive.target,
+                       hostdev->info->addr.drive.unit);
+        goto error;
+    }
+
+
+    virDomainHostdevRemove(vm->def, idx);
+
+    virHostdevReAttachPCIDevices(hostdev_mgr, LIBXL_DRIVER_NAME,
+                                 vm->def->name, &hostdev, 1, NULL);
+
+    ret = 0;
+
+ error:
+    VIR_FREE(str);
+    virDomainHostdevDefFree(detach);
+
+ cleanup:
+    virObjectUnref(cfg);
+    return ret;
+#else
+    virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                   _("This version of libxenlight does not support vscsi"));
+    return -1;
+#endif
+}
+
+static int
 libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver,
                             virDomainObjPtr vm,
                             virDomainHostdevDefPtr hostdev)
@@ -3402,6 +3586,9 @@  libxlDomainDetachHostDevice(libxlDriverPrivatePtr driver,
         case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
             return libxlDomainDetachHostPCIDevice(driver, vm, hostdev);
 
+        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI:
+            return libxlDomainDetachHostSCSIDevice(driver, vm, hostdev);
+
         default:
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("unexpected hostdev type %d"), subsys->type);