9:32 a.m.
On Sat, Jul 14, 2018 at 12:06:14AM +0200, clem(a)lse.epita.fr wrote:
From: Clementine Hayat <clem(a)lse.epita.fr>
I know that this is more like RFC patch series so before we get to the
actual patch which should be pushed into upstream there should be some
commit message.
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
configure.ac | 6 ++-
m4/virt-storage-iscsi-direct.m4 | 41 +++++++++++++++++++++
src/conf/domain_conf.c | 1 +
src/conf/storage_conf.c | 31 ++++++++++++++--
src/conf/storage_conf.h | 1 +
src/conf/virstorageobj.c | 2 +
src/storage/Makefile.inc.am | 21 +++++++++++
src/storage/storage_backend.c | 6 +++
src/storage/storage_backend_iscsi_direct.c | 43 ++++++++++++++++++++++
src/storage/storage_backend_iscsi_direct.h | 6 +++
src/storage/storage_driver.c | 1 +
tools/virsh-pool.c | 3 ++
12 files changed, 157 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/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7396616eda..0a9509de0b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30163,6 +30163,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
break;
+ case VIR_STORAGE_POOL_ISCSI_DIRECT:
case VIR_STORAGE_POOL_ISCSI:
if (def->startupPolicy) {
virReportError(VIR_ERR_XML_ERROR, "%s",
This will not be good enough. We need to set the default
def->src->srcpool->mode to VIR_STORAGE_SOURCE_POOL_MODE_DIRECT if the
storage pool is "iscsi-direct" and if the mode is already configured in
domain XML we need to check whether it's "direct" mode if the storage
pool is "iscsi-direct".
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 5036ab9ef8..7a4b00ad8c 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,16 @@ static virStoragePoolTypeInfo poolTypeInfo[] = {
.formatToString = virStoragePoolFormatDiskTypeToString,
}
},
+ {.poolType = VIR_STORAGE_POOL_ISCSI_DIRECT,
+ .poolOptions = {
+ .flags = (VIR_STORAGE_POOL_SOURCE_HOST |
+ VIR_STORAGE_POOL_SOURCE_NETWORK |
+ VIR_STORAGE_POOL_SOURCE_INITIATOR_IQN),
We need to use VIR_STORAGE_POOL_SOURCE_DEVICE as well, otherwise it
would not be formatted back.
+ },
+ .volOptions = {
+ .formatToString = virStoragePoolFormatDiskTypeToString,
+ }
+ },
{.poolType = VIR_STORAGE_POOL_SCSI,
.poolOptions = {
.flags = (VIR_STORAGE_POOL_SOURCE_ADAPTER),
@@ -802,6 +812,18 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
"./target/permissions") < 0)
goto error;
}
One empty line would be nice.
+ 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;
+ }
+ }
cleanup:
VIR_FREE(uuid);
@@ -1004,7 +1026,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, "<target>\n");
virBufferAdjustIndent(buf, 2);
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h
index 15dfd8becf..858623783d 100644
--- a/src/conf/storage_conf.h
+++ b/src/conf/storage_conf.h
@@ -85,6 +85,7 @@ typedef enum {
VIR_STORAGE_POOL_LOGICAL, /* Logical volume groups / volumes */
VIR_STORAGE_POOL_DISK, /* Disk partitions */
VIR_STORAGE_POOL_ISCSI, /* iSCSI targets */
+ VIR_STORAGE_POOL_ISCSI_DIRECT, /* iSCSI targets using libiscsi */
VIR_STORAGE_POOL_SCSI, /* SCSI HBA */
VIR_STORAGE_POOL_MPATH, /* Multipath devices */
VIR_STORAGE_POOL_RBD, /* RADOS Block Device */
[...]
diff --git a/src/storage/Makefile.inc.am
b/src/storage/Makefile.inc.am
index ea98c0ee52..d81864f5b9 100644
--- a/src/storage/Makefile.inc.am
+++ b/src/storage/Makefile.inc.am
@@ -31,6 +31,11 @@ STORAGE_DRIVER_ISCSI_SOURCES = \
storage/storage_backend_iscsi.c \
$(NULL)
+STORAGE_DRIVER_ISCSI_DIRECT_SOURCES = \
+ storage/storage_backend_iscsi_direct.h \
+ storage/storage_backend_iscsi_direct.c \
+ $(NULL)
+
STORAGE_DRIVER_SCSI_SOURCES = \
storage/storage_backend_scsi.h \
storage/storage_backend_scsi.c \
@@ -89,6 +94,7 @@ EXTRA_DIST += \
$(STORAGE_FILE_FS_SOURCES) \
$(STORAGE_DRIVER_LVM_SOURCES) \
$(STORAGE_DRIVER_ISCSI_SOURCES) \
+ $(STORAGE_DRIVER_ISCSI_DIRECT_SOURCES) \
$(STORAGE_DRIVER_SCSI_SOURCES) \
$(STORAGE_DRIVER_MPATH_SOURCES) \
$(STORAGE_DRIVER_DISK_SOURCES) \
@@ -193,6 +199,21 @@ libvirt_storage_backend_iscsi_la_LIBADD = \
$(NULL)
endif WITH_STORAGE_ISCSI
+if WITH_STORAGE_ISCSI_DIRECT
+libvirt_storage_backend_iscsi_direct_la_SOURCES =
$(STORAGE_DRIVER_ISCSI_DIRECT_SOURCES)
+libvirt_storage_backend_iscsi_direct_la_CFLAGS = \
So I remember point out that we don't need '-I$(srcdir)/conf \' here
right now but I was wrong. We need it because some files from that
directory are indirectly included.
+ $(LIBISCSI_CFLAGS) \
+ $(AM_CFLAGS) \
+ $(NULL)
+
+storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi-direct.la
+libvirt_storage_backend_iscsi_direct_la_LDFLAGS = $(AM_LDFLAGS_MOD)
+libvirt_storage_backend_iscsi_direct_la_LIBADD = \
+ libvirt.la \
+ ../gnulib/lib/libgnu.la \
+ $(NULL)
+endif WITH_STORAGE_ISCSI_DIRECT
+
if WITH_STORAGE_SCSI
libvirt_storage_backend_scsi_la_SOURCES = $(STORAGE_DRIVER_SCSI_SOURCES)
libvirt_storage_backend_scsi_la_CFLAGS = \
[...]
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
new file mode 100644
index 0000000000..e3c1f75b42
--- /dev/null
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -0,0 +1,43 @@
Missing copyright comment.
+#include <config.h>
config.h is not a system include so we usually separate it by empty line
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
All of these can be removed, they are not needed now.
+
+#include "datatypes.h"
+#include "driver.h"
+#include "storage_backend_iscsi_direct.h"
+#include "storage_util.h"
+#include "virlog.h"
+#include "virobject.h"
"datatypes.h", "driver.h" and "virobject.h" can be removed
from this
patche, they are not needed now.
+
+#define VIR_FROM_THIS VIR_FROM_STORAGE
+
+VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
+
+
+static int
+virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
+ bool *isActive ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
+
+static int
+virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool ATTRIBUTE_UNUSED)
+{
+ return 0;
+}
+
+virStorageBackend virStorageBackendISCSIDirect = {
+ .type = VIR_STORAGE_POOL_ISCSI_DIRECT,
+
+ .checkPool = virStorageBackendISCSIDirectCheckPool,
+ .refreshPool = virStorageBackendISCSIDirectRefreshPool,
+};
+
+int
+virStorageBackendISCSIDirectRegister(void)
+{
+ return virStorageBackendRegister(&virStorageBackendISCSIDirect);
+}
Some minor issues, otherwise looks good.
Pavel