Re: [libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type

2018-07-24 Thread Michal Privoznik 
On 07/24/2018 06:19 PM, Clementine Hayat wrote:
> 2018-07-24 14:24 GMT+02:00 Michal Privoznik :
>> On 07/23/2018 08:43 PM, c...@lse.epita.fr wrote:
>>> From: Clementine Hayat 
>>>
>>> Introducing the pool as a noop. Integration inside the build
>>> system. Implementation will be in the following commits.
>>>
>>> Signed-off-by: Clementine Hayat 
>>> ---
>>>  configure.ac   |  6 ++-
>>>  m4/virt-storage-iscsi-direct.m4| 41 +++
>>>  src/conf/domain_conf.c |  4 ++
>>>  src/conf/storage_conf.c| 33 ++--
>>>  src/conf/storage_conf.h|  1 +
>>>  src/conf/virstorageobj.c   |  2 +
>>>  src/storage/Makefile.inc.am| 22 
>>>  src/storage/storage_backend.c  |  6 +++
>>>  src/storage/storage_backend_iscsi_direct.c | 58 ++
>>>  src/storage/storage_backend_iscsi_direct.h |  6 +++
>>>  src/storage/storage_driver.c   |  1 +
>>>  tools/virsh-pool.c |  3 ++
>>>  12 files changed, 178 insertions(+), 5 deletions(-)
>>>  create mode 100644 m4/virt-storage-iscsi-direct.m4
>>>  create mode 100644 src/storage/storage_backend_iscsi_direct.c
>>>  create mode 100644 src/storage/storage_backend_iscsi_direct.h
>>


>>> @@ -803,6 +814,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>>>  goto error;
>>>  }
>>>
>>> +if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
>>> +if (!ret->source.initiator.iqn) {
>>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>>> +   _("missing storage pool initiator iqn"));
>>> +goto error;
>>> +}
>>> +if (!ret->source.ndevice) {
>>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>>> +   _("missing storage pool device path"));
>>> +goto error;
>>> +}
>>> +}
>>
>> So the wole idea of poolOptions and volOptions is to specify which parts
>> of pool/volume XML are required so that we don't have to base checks
>> like this on ret->type rather than flags.
>> On the other hand, comment just above VIR_STORAGE_POOL_SOURCE_* enum
>> says it declares mandatory components which it clearly doesn't. So after
>> all I think we are safe here.
> 
> That actually isn't the case for the initiator iqn flag.
> Is it on purpose or should I patch it in another thread?

I think saving that for a separate patch is okay.
Speaking of threads, I forgot to mention, feel free to send v3 as a
completely new thread. We don't really thread versions under v1.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type

2018-07-24 Thread Pavel Hrdina 
On Mon, Jul 23, 2018 at 08:43:00PM +0200, c...@lse.epita.fr wrote:
> From: Clementine Hayat 
> 
> Introducing the pool as a noop. Integration inside the build
> system. Implementation will be in the following commits.
> 
> Signed-off-by: Clementine Hayat 
> ---
>  configure.ac   |  6 ++-
>  m4/virt-storage-iscsi-direct.m4| 41 +++
>  src/conf/domain_conf.c |  4 ++
>  src/conf/storage_conf.c| 33 ++--
>  src/conf/storage_conf.h|  1 +
>  src/conf/virstorageobj.c   |  2 +
>  src/storage/Makefile.inc.am| 22 
>  src/storage/storage_backend.c  |  6 +++
>  src/storage/storage_backend_iscsi_direct.c | 58 ++
>  src/storage/storage_backend_iscsi_direct.h |  6 +++
>  src/storage/storage_driver.c   |  1 +
>  tools/virsh-pool.c |  3 ++
>  12 files changed, 178 insertions(+), 5 deletions(-)
>  create mode 100644 m4/virt-storage-iscsi-direct.m4
>  create mode 100644 src/storage/storage_backend_iscsi_direct.c
>  create mode 100644 src/storage/storage_backend_iscsi_direct.h
> 
> diff --git a/configure.ac b/configure.ac
> index c668630a79..87ac4dc2c3 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -564,6 +564,7 @@ LIBVIRT_STORAGE_ARG_DIR
>  LIBVIRT_STORAGE_ARG_FS
>  LIBVIRT_STORAGE_ARG_LVM
>  LIBVIRT_STORAGE_ARG_ISCSI
> +LIBVIRT_STORAGE_ARG_ISCSI_DIRECT
>  LIBVIRT_STORAGE_ARG_SCSI
>  LIBVIRT_STORAGE_ARG_MPATH
>  LIBVIRT_STORAGE_ARG_DISK
> @@ -578,6 +579,7 @@ if test "$with_libvirtd" = "no"; then
>with_storage_fs=no
>with_storage_lvm=no
>with_storage_iscsi=no
> +  with_storage_iscsi_direct=no
>with_storage_scsi=no
>with_storage_mpath=no
>with_storage_disk=no
> @@ -598,6 +600,7 @@ LIBVIRT_STORAGE_CHECK_DIR
>  LIBVIRT_STORAGE_CHECK_FS
>  LIBVIRT_STORAGE_CHECK_LVM
>  LIBVIRT_STORAGE_CHECK_ISCSI
> +LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT
>  LIBVIRT_STORAGE_CHECK_SCSI
>  LIBVIRT_STORAGE_CHECK_MPATH
>  LIBVIRT_STORAGE_CHECK_DISK
> @@ -608,7 +611,7 @@ LIBVIRT_STORAGE_CHECK_ZFS
>  LIBVIRT_STORAGE_CHECK_VSTORAGE
>  
>  with_storage=no
> -for backend in dir fs lvm iscsi scsi mpath rbd disk; do
> +for backend in dir fs lvm iscsi iscsi_direct scsi mpath rbd disk; do
>  if eval test \$with_storage_$backend = yes; then
>  with_storage=yes
>  break
> @@ -936,6 +939,7 @@ LIBVIRT_STORAGE_RESULT_DIR
>  LIBVIRT_STORAGE_RESULT_FS
>  LIBVIRT_STORAGE_RESULT_LVM
>  LIBVIRT_STORAGE_RESULT_ISCSI
> +LIBVIRT_STORAGE_RESULT_ISCSI_DIRECT
>  LIBVIRT_STORAGE_RESULT_SCSI
>  LIBVIRT_STORAGE_RESULT_MPATH
>  LIBVIRT_STORAGE_RESULT_DISK
> diff --git a/m4/virt-storage-iscsi-direct.m4 b/m4/virt-storage-iscsi-direct.m4
> new file mode 100644
> index 00..cc2d490352
> --- /dev/null
> +++ b/m4/virt-storage-iscsi-direct.m4
> @@ -0,0 +1,41 @@
> +dnl Iscsi-direct storage
> +dnl
> +dnl Copyright (C) 2018 Clementine Hayat.
> +dnl
> +dnl This library is free software; you can redistribute it and/or
> +dnl modify it under the terms of the GNU Lesser General Public
> +dnl License as published by the Free Software Foundation; either
> +dnl version 2.1 of the License, or (at your option) any later version.
> +dnl
> +dnl This library is distributed in the hope that it will be useful,
> +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
> +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +dnl Lesser General Public License for more details.
> +dnl
> +dnl You should have received a copy of the GNU Lesser General Public
> +dnl License along with this library.  If not, see
> +dnl .
> +dnl
> +
> +AC_DEFUN([LIBVIRT_STORAGE_ARG_ISCSI_DIRECT], [
> +  LIBVIRT_ARG_WITH_FEATURE([STORAGE_ISCSI_DIRECT],
> +   [iscsi-direct backend for the storage driver],
> +   [check])
> +])
> +
> +AC_DEFUN([LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT], [
> +  AC_REQUIRE([LIBVIRT_CHECK_LIBISCSI])
> +  if test "$with_storage_iscsi_direct" = "check"; then
> +with_storage_iscsi_direct=$with_libiscsi
> +  fi
> +  if test "$with_storage_iscsi_direct" = "yes"; then
> +AC_DEFINE_UNQUOTED([WITH_STORAGE_ISCSI_DIRECT], [1],
> +   [whether iSCSI backend for storage driver is enabled])
> +  fi
> +  AM_CONDITIONAL([WITH_STORAGE_ISCSI_DIRECT],
> + [test "$with_storage_iscsi_direct" = "yes"])
> +])
> +
> +AC_DEFUN([LIBVIRT_STORAGE_RESULT_ISCSI_DIRECT], [
> +  LIBVIRT_RESULT([iscsi-direct], [$with_storage_iscsi_direct])
> +])
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7396616eda..5af27a6ad2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30163,6 +30163,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr 
> def)
>  
>  break;
>  
> +case VIR_STORAGE_POOL_ISCSI_DIRECT:
> +def->src->srcpool->mode =

Re: [libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type

2018-07-24 Thread Clementine Hayat 
2018-07-24 14:24 GMT+02:00 Michal Privoznik :
> On 07/23/2018 08:43 PM, c...@lse.epita.fr wrote:
>> From: Clementine Hayat 
>>
>> Introducing the pool as a noop. Integration inside the build
>> system. Implementation will be in the following commits.
>>
>> Signed-off-by: Clementine Hayat 
>> ---
>>  configure.ac   |  6 ++-
>>  m4/virt-storage-iscsi-direct.m4| 41 +++
>>  src/conf/domain_conf.c |  4 ++
>>  src/conf/storage_conf.c| 33 ++--
>>  src/conf/storage_conf.h|  1 +
>>  src/conf/virstorageobj.c   |  2 +
>>  src/storage/Makefile.inc.am| 22 
>>  src/storage/storage_backend.c  |  6 +++
>>  src/storage/storage_backend_iscsi_direct.c | 58 ++
>>  src/storage/storage_backend_iscsi_direct.h |  6 +++
>>  src/storage/storage_driver.c   |  1 +
>>  tools/virsh-pool.c |  3 ++
>>  12 files changed, 178 insertions(+), 5 deletions(-)
>>  create mode 100644 m4/virt-storage-iscsi-direct.m4
>>  create mode 100644 src/storage/storage_backend_iscsi_direct.c
>>  create mode 100644 src/storage/storage_backend_iscsi_direct.h
>
> Missing documentation. I can not push these without any documentation.
> You need to document what the new type is doing, what the XML should
> look like. Also, might be worth putting some test cases into
> storagepoolxml2xmltest.
> Since you will be sending v3, can you add docs/news.xml entry (in a
> separate patch) too please?

Yes sure.

>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 7396616eda..5af27a6ad2 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -30163,6 +30163,10 @@ 
>> virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
>>
>>  break;
>>
>> +case VIR_STORAGE_POOL_ISCSI_DIRECT:
>> +def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
>> +break;
>> +
>>  case VIR_STORAGE_POOL_ISCSI:
>>  if (def->startupPolicy) {
>>  virReportError(VIR_ERR_XML_ERROR, "%s",
>> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
>> index 5036ab9ef8..ee1586410b 100644
>> --- a/src/conf/storage_conf.c
>> +++ b/src/conf/storage_conf.c
>> @@ -62,9 +62,9 @@ VIR_ENUM_IMPL(virStoragePool,
>>VIR_STORAGE_POOL_LAST,
>>"dir", "fs", "netfs",
>>"logical", "disk", "iscsi",
>> -  "scsi", "mpath", "rbd",
>> -  "sheepdog", "gluster", "zfs",
>> -  "vstorage")
>> +  "iscsi-direct", "scsi", "mpath",
>> +  "rbd", "sheepdog", "gluster",
>> +  "zfs", "vstorage")
>>
>>  VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
>>VIR_STORAGE_POOL_FS_LAST,
>> @@ -207,6 +207,17 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
>>   .formatToString = virStoragePoolFormatDiskTypeToString,
>>}
>>  },
>> +{.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
>> + .poolOptions = {
>> + .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
>> +   VIR_STORAGE_POOL_SOURCE_DEVICE |
>> +   VIR_STORAGE_POOL_SOURCE_NETWORK |
>> +   VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
>> +  },
>> +  .volOptions = {
>> + .formatToString = virStoragePoolFormatDiskTypeToString,
>> +  }
>> +},
>>  {.poolType = VIR_STORAGE_POOL_SCSI,
>>   .poolOptions = {
>>   .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
>> @@ -803,6 +814,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>>  goto error;
>>  }
>>
>> +if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
>> +if (!ret->source.initiator.iqn) {
>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>> +   _("missing storage pool initiator iqn"));
>> +goto error;
>> +}
>> +if (!ret->source.ndevice) {
>> +virReportError(VIR_ERR_XML_ERROR, "%s",
>> +   _("missing storage pool device path"));
>> +goto error;
>> +}
>> +}
>
> So the wole idea of poolOptions and volOptions is to specify which parts
> of pool/volume XML are required so that we don't have to base checks
> like this on ret->type rather than flags.
> On the other hand, comment just above VIR_STORAGE_POOL_SOURCE_* enum
> says it declares mandatory components which it clearly doesn't. So after
> all I think we are safe here.

That actually isn't the case for the initiator iqn flag.
Is it on purpose or should I patch it in another thread?

>> +
>>   cleanup:
>>  VIR_FREE(uuid);
>>  VIR_FREE(type);
>> @@ -1004,7 +1028,8 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
>>   * files, so they don't have a target */
>>  if (def->type != VIR_STORAGE_POOL_RBD &&
>>  def->type != VIR_STORAGE_POOL_SHEEPDOG &&
>> -def->type !=

Re: [libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type

2018-07-24 Thread Michal Privoznik 
On 07/23/2018 08:43 PM, c...@lse.epita.fr wrote:
> From: Clementine Hayat 
> 
> Introducing the pool as a noop. Integration inside the build
> system. Implementation will be in the following commits.
> 
> Signed-off-by: Clementine Hayat 
> ---
>  configure.ac   |  6 ++-
>  m4/virt-storage-iscsi-direct.m4| 41 +++
>  src/conf/domain_conf.c |  4 ++
>  src/conf/storage_conf.c| 33 ++--
>  src/conf/storage_conf.h|  1 +
>  src/conf/virstorageobj.c   |  2 +
>  src/storage/Makefile.inc.am| 22 
>  src/storage/storage_backend.c  |  6 +++
>  src/storage/storage_backend_iscsi_direct.c | 58 ++
>  src/storage/storage_backend_iscsi_direct.h |  6 +++
>  src/storage/storage_driver.c   |  1 +
>  tools/virsh-pool.c |  3 ++
>  12 files changed, 178 insertions(+), 5 deletions(-)
>  create mode 100644 m4/virt-storage-iscsi-direct.m4
>  create mode 100644 src/storage/storage_backend_iscsi_direct.c
>  create mode 100644 src/storage/storage_backend_iscsi_direct.h

Missing documentation. I can not push these without any documentation.
You need to document what the new type is doing, what the XML should
look like. Also, might be worth putting some test cases into
storagepoolxml2xmltest.
Since you will be sending v3, can you add docs/news.xml entry (in a
separate patch) too please?

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 7396616eda..5af27a6ad2 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -30163,6 +30163,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr 
> def)
>  
>  break;
>  
> +case VIR_STORAGE_POOL_ISCSI_DIRECT:
> +def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
> +break;
> +
>  case VIR_STORAGE_POOL_ISCSI:
>  if (def->startupPolicy) {
>  virReportError(VIR_ERR_XML_ERROR, "%s",
> diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
> index 5036ab9ef8..ee1586410b 100644
> --- a/src/conf/storage_conf.c
> +++ b/src/conf/storage_conf.c
> @@ -62,9 +62,9 @@ VIR_ENUM_IMPL(virStoragePool,
>VIR_STORAGE_POOL_LAST,
>"dir", "fs", "netfs",
>"logical", "disk", "iscsi",
> -  "scsi", "mpath", "rbd",
> -  "sheepdog", "gluster", "zfs",
> -  "vstorage")
> +  "iscsi-direct", "scsi", "mpath",
> +  "rbd", "sheepdog", "gluster",
> +  "zfs", "vstorage")
>  
>  VIR_ENUM_IMPL(virStoragePoolFormatFileSystem,
>VIR_STORAGE_POOL_FS_LAST,
> @@ -207,6 +207,17 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
>   .formatToString = virStoragePoolFormatDiskTypeToString,
>}
>  },
> +{.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
> + .poolOptions = {
> + .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
> +   VIR_STORAGE_POOL_SOURCE_DEVICE |
> +   VIR_STORAGE_POOL_SOURCE_NETWORK |
> +   VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
> +  },
> +  .volOptions = {
> + .formatToString = virStoragePoolFormatDiskTypeToString,
> +  }
> +},
>  {.poolType = VIR_STORAGE_POOL_SCSI,
>   .poolOptions = {
>   .flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
> @@ -803,6 +814,19 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
>  goto error;
>  }
>  
> +if (ret->type == VIR_STORAGE_POOL_ISCSI_DIRECT) {
> +if (!ret->source.initiator.iqn) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("missing storage pool initiator iqn"));
> +goto error;
> +}
> +if (!ret->source.ndevice) {
> +virReportError(VIR_ERR_XML_ERROR, "%s",
> +   _("missing storage pool device path"));
> +goto error;
> +}
> +}

So the wole idea of poolOptions and volOptions is to specify which parts
of pool/volume XML are required so that we don't have to base checks
like this on ret->type rather than flags.
On the other hand, comment just above VIR_STORAGE_POOL_SOURCE_* enum
says it declares mandatory components which it clearly doesn't. So after
all I think we are safe here.

> +
>   cleanup:
>  VIR_FREE(uuid);
>  VIR_FREE(type);
> @@ -1004,7 +1028,8 @@ virStoragePoolDefFormatBuf(virBufferPtr buf,
>   * files, so they don't have a target */
>  if (def->type != VIR_STORAGE_POOL_RBD &&
>  def->type != VIR_STORAGE_POOL_SHEEPDOG &&
> -def->type != VIR_STORAGE_POOL_GLUSTER) {
> +def->type != VIR_STORAGE_POOL_GLUSTER &&
> +def->type != VIR_STORAGE_POOL_ISCSI_DIRECT) {
>  virBufferAddLit(buf, "\n");
>  virBufferAdjustIndent(buf, 2);

Might be worth updating the comment just above this block ;-)


Michal

[libvirt] [PATCH v2 2/3] storage: Introduce iscsi_direct pool type

2018-07-23 Thread clem 
From: Clementine Hayat 

Introducing the pool as a noop. Integration inside the build
system. Implementation will be in the following commits.

Signed-off-by: Clementine Hayat 
---
 configure.ac   |  6 ++-
 m4/virt-storage-iscsi-direct.m4| 41 +++
 src/conf/domain_conf.c |  4 ++
 src/conf/storage_conf.c| 33 ++--
 src/conf/storage_conf.h|  1 +
 src/conf/virstorageobj.c   |  2 +
 src/storage/Makefile.inc.am| 22 
 src/storage/storage_backend.c  |  6 +++
 src/storage/storage_backend_iscsi_direct.c | 58 ++
 src/storage/storage_backend_iscsi_direct.h |  6 +++
 src/storage/storage_driver.c   |  1 +
 tools/virsh-pool.c |  3 ++
 12 files changed, 178 insertions(+), 5 deletions(-)
 create mode 100644 m4/virt-storage-iscsi-direct.m4
 create mode 100644 src/storage/storage_backend_iscsi_direct.c
 create mode 100644 src/storage/storage_backend_iscsi_direct.h

diff --git a/configure.ac b/configure.ac
index c668630a79..87ac4dc2c3 100644
--- a/configure.ac
+++ b/configure.ac
@@ -564,6 +564,7 @@ LIBVIRT_STORAGE_ARG_DIR
 LIBVIRT_STORAGE_ARG_FS
 LIBVIRT_STORAGE_ARG_LVM
 LIBVIRT_STORAGE_ARG_ISCSI
+LIBVIRT_STORAGE_ARG_ISCSI_DIRECT
 LIBVIRT_STORAGE_ARG_SCSI
 LIBVIRT_STORAGE_ARG_MPATH
 LIBVIRT_STORAGE_ARG_DISK
@@ -578,6 +579,7 @@ if test "$with_libvirtd" = "no"; then
   with_storage_fs=no
   with_storage_lvm=no
   with_storage_iscsi=no
+  with_storage_iscsi_direct=no
   with_storage_scsi=no
   with_storage_mpath=no
   with_storage_disk=no
@@ -598,6 +600,7 @@ LIBVIRT_STORAGE_CHECK_DIR
 LIBVIRT_STORAGE_CHECK_FS
 LIBVIRT_STORAGE_CHECK_LVM
 LIBVIRT_STORAGE_CHECK_ISCSI
+LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT
 LIBVIRT_STORAGE_CHECK_SCSI
 LIBVIRT_STORAGE_CHECK_MPATH
 LIBVIRT_STORAGE_CHECK_DISK
@@ -608,7 +611,7 @@ LIBVIRT_STORAGE_CHECK_ZFS
 LIBVIRT_STORAGE_CHECK_VSTORAGE
 
 with_storage=no
-for backend in dir fs lvm iscsi scsi mpath rbd disk; do
+for backend in dir fs lvm iscsi iscsi_direct scsi mpath rbd disk; do
 if eval test \$with_storage_$backend = yes; then
 with_storage=yes
 break
@@ -936,6 +939,7 @@ LIBVIRT_STORAGE_RESULT_DIR
 LIBVIRT_STORAGE_RESULT_FS
 LIBVIRT_STORAGE_RESULT_LVM
 LIBVIRT_STORAGE_RESULT_ISCSI
+LIBVIRT_STORAGE_RESULT_ISCSI_DIRECT
 LIBVIRT_STORAGE_RESULT_SCSI
 LIBVIRT_STORAGE_RESULT_MPATH
 LIBVIRT_STORAGE_RESULT_DISK
diff --git a/m4/virt-storage-iscsi-direct.m4 b/m4/virt-storage-iscsi-direct.m4
new file mode 100644
index 00..cc2d490352
--- /dev/null
+++ b/m4/virt-storage-iscsi-direct.m4
@@ -0,0 +1,41 @@
+dnl Iscsi-direct storage
+dnl
+dnl Copyright (C) 2018 Clementine Hayat.
+dnl
+dnl This library is free software; you can redistribute it and/or
+dnl modify it under the terms of the GNU Lesser General Public
+dnl License as published by the Free Software Foundation; either
+dnl version 2.1 of the License, or (at your option) any later version.
+dnl
+dnl This library is distributed in the hope that it will be useful,
+dnl but WITHOUT ANY WARRANTY; without even the implied warranty of
+dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+dnl Lesser General Public License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with this library.  If not, see
+dnl .
+dnl
+
+AC_DEFUN([LIBVIRT_STORAGE_ARG_ISCSI_DIRECT], [
+  LIBVIRT_ARG_WITH_FEATURE([STORAGE_ISCSI_DIRECT],
+   [iscsi-direct backend for the storage driver],
+   [check])
+])
+
+AC_DEFUN([LIBVIRT_STORAGE_CHECK_ISCSI_DIRECT], [
+  AC_REQUIRE([LIBVIRT_CHECK_LIBISCSI])
+  if test "$with_storage_iscsi_direct" = "check"; then
+with_storage_iscsi_direct=$with_libiscsi
+  fi
+  if test "$with_storage_iscsi_direct" = "yes"; then
+AC_DEFINE_UNQUOTED([WITH_STORAGE_ISCSI_DIRECT], [1],
+   [whether iSCSI backend for storage driver is enabled])
+  fi
+  AM_CONDITIONAL([WITH_STORAGE_ISCSI_DIRECT],
+ [test "$with_storage_iscsi_direct" = "yes"])
+])
+
+AC_DEFUN([LIBVIRT_STORAGE_RESULT_ISCSI_DIRECT], [
+  LIBVIRT_RESULT([iscsi-direct], [$with_storage_iscsi_direct])
+])
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7396616eda..5af27a6ad2 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30163,6 +30163,10 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr 
def)
 
 break;
 
+case VIR_STORAGE_POOL_ISCSI_DIRECT:
+def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_DIRECT;
+break;
+
 case VIR_STORAGE_POOL_ISCSI:
 if (def->startupPolicy) {
 virReportError(VIR_ERR_XML_ERROR, "%s",
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5036ab9ef8..ee1586410b 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@