5:26 p.m.
From: Clementine Hayat <clem(a)lse.epita.fr>
Hello,
This series of patch is the follow up of:
v1:https://www.redhat.com/archives/libvir-list/2018-August/msg00557.html
v2:https://www.redhat.com/archives/libvir-list/2018-August/msg00570.html
Best Regards,
--
Clementine Hayat
Clementine Hayat (3):
storage: refactor volume capacity in iscsi-direct backend
storage: add SetConnection to iscsi-direct backend
storage: add wipeVol to iscsi-direct storage backend
src/storage/storage_backend_iscsi_direct.c | 178 ++++++++++++++++++---
1 file changed, 157 insertions(+), 21 deletions(-)
--
2.18.0
5:26 p.m.
New subject: [libvirt] [PATCH v3 1/3] storage: refactor volume capacity in iscsi-direct backend
From: Clementine Hayat <clem(a)lse.epita.fr>
The size of blocks inside a volume and the number of blocks are needed
later. Having a function that return thoses information will avoid code
duplication.
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
src/storage/storage_backend_iscsi_direct.c | 21 ++++++++++++---------
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
index 0764356b62..62b7e0d8bc 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -237,13 +237,13 @@ virISCSIDirectSetVolumeAttributes(virStoragePoolObjPtr pool,
}
static int
-virISCSIDirectSetVolumeCapacity(struct iscsi_context *iscsi,
- virStorageVolDefPtr vol,
- int lun)
+virISCSIDirectGetVolumeCapacity(struct iscsi_context *iscsi,
+ int lun,
+ uint32_t *block_size,
+ uint32_t *nb_block)
{
struct scsi_task *task = NULL;
struct scsi_inquiry_standard *inq = NULL;
- long long size = 0;
int ret = -1;
if (!(task = iscsi_inquiry_sync(iscsi, lun, 0, 0, 64)) ||
@@ -282,10 +282,8 @@ virISCSIDirectSetVolumeCapacity(struct iscsi_context *iscsi,
goto cleanup;
}
- size = rc10->block_size;
- size *= rc10->lba;
- vol->target.capacity = size;
- vol->target.allocation = size;
+ *block_size = rc10->block_size;
+ *nb_block = rc10->lba;
}
@@ -303,6 +301,8 @@ virISCSIDirectRefreshVol(virStoragePoolObjPtr pool,
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
virStorageVolDefPtr vol = NULL;
+ uint32_t block_size;
+ uint32_t nb_block;
int ret = -1;
virStoragePoolObjClearVols(pool);
@@ -314,9 +314,12 @@ virISCSIDirectRefreshVol(virStoragePoolObjPtr pool,
vol->type = VIR_STORAGE_VOL_NETWORK;
- if (virISCSIDirectSetVolumeCapacity(iscsi, vol, lun) < 0)
+ if (virISCSIDirectGetVolumeCapacity(iscsi, lun,
+ &block_size, &nb_block) < 0)
goto cleanup;
+ vol->target.capacity = block_size * nb_block;
+ vol->target.allocation = block_size * nb_block;
def->capacity += vol->target.capacity;
def->allocation += vol->target.allocation;
--
2.18.0
5:26 p.m.
New subject: [libvirt] [PATCH v3 2/3] storage: add SetConnection to iscsi-direct backend
From: Clementine Hayat <clem(a)lse.epita.fr>
The code to set the connection and connect using libiscsi will always be
the same. Add function to avoid code duplication.
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
src/storage/storage_backend_iscsi_direct.c | 38 +++++++++++++++-------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
index 62b7e0d8bc..7bdd39582b 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -557,23 +557,37 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec,
return ret;
}
+struct iscsi_context *
+virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool,
+ char **portal)
+{
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ struct iscsi_context *iscsi = NULL;
+
+ if (!(*iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
+ return iscsi;
+ if (!(*portal = virStorageBackendISCSIDirectPortal(&def->source)))
+ goto error ;
+ if (virStorageBackendISCSIDirectSetAuth(*iscsi, &def->source) < 0)
+ goto error ;
+ if (virISCSIDirectSetContext(*iscsi, def->source.devices[0].path,
ISCSI_SESSION_NORMAL) < 0)
+ goto error ;
+ if (virISCSIDirectConnect(*iscsi, *portal) < 0)
+ goto error ;
+ return iscsi;
+
+ error:
+ iscsi_destroy_context(iscsi);
+ return iscsi;
+}
+
static int
virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
{
- virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
struct iscsi_context *iscsi = NULL;
char *portal = NULL;
int ret = -1;
-
- if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
- goto cleanup;
- if (!(portal = virStorageBackendISCSIDirectPortal(&def->source)))
- goto cleanup;
- if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0)
- goto cleanup;
- if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path,
ISCSI_SESSION_NORMAL) < 0)
- goto cleanup;
- if (virISCSIDirectConnect(iscsi, portal) < 0)
+ if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
goto cleanup;
if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0)
goto disconect;
@@ -581,9 +595,9 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
ret = 0;
disconect:
virISCSIDirectDisconnect(iscsi);
+ iscsi_destroy_context(iscsi);
cleanup:
VIR_FREE(portal);
- iscsi_destroy_context(iscsi);
return ret;
}
--
2.18.0
3:10 a.m.
New subject: [libvirt] [PATCH v3 2/3] storage: add SetConnection to iscsi-direct backend
On 08/13/2018 12:26 AM, clem(a)lse.epita.fr wrote:
From: Clementine Hayat <clem(a)lse.epita.fr>
The code to set the connection and connect using libiscsi will always be
the same. Add function to avoid code duplication.
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
src/storage/storage_backend_iscsi_direct.c | 38 +++++++++++++++-------
1 file changed, 26 insertions(+), 12 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
index 62b7e0d8bc..7bdd39582b 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -557,23 +557,37 @@ virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec,
return ret;
}
+struct iscsi_context *
+virStorageBackendISCSIDirectSetConnection(virStoragePoolObjPtr pool,
+ char **portal)
The function needs to be static. You're not exporting it, only using it
within this file.
Also, right now the only caller of the function needs to know the
portal, but that will not always be the case. So I think we can have the
function accepting NULL for *portal meaning caller doesn't care what the
portal is.
+{
+ virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
+ struct iscsi_context *iscsi = NULL;
+
+ if (!(*iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
+ return iscsi;
This doesn't look right. We don't know what iscsi_context look like, we
shouldn't dereference it. Have you tried to compile this patch? ;-)
+ if (!(*portal =
virStorageBackendISCSIDirectPortal(&def->source)))
+ goto error ;
+ if (virStorageBackendISCSIDirectSetAuth(*iscsi, &def->source) < 0)
+ goto error ;
+ if (virISCSIDirectSetContext(*iscsi, def->source.devices[0].path,
ISCSI_SESSION_NORMAL) < 0)
+ goto error ;
+ if (virISCSIDirectConnect(*iscsi, *portal) < 0)
+ goto error ;
+ return iscsi;
+
+ error:
+ iscsi_destroy_context(iscsi);
+ return iscsi;
+}
+
static int
virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
{
- virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
struct iscsi_context *iscsi = NULL;
char *portal = NULL;
int ret = -1;
-
- if (!(iscsi = virISCSIDirectCreateContext(def->source.initiator.iqn)))
- goto cleanup;
- if (!(portal = virStorageBackendISCSIDirectPortal(&def->source)))
- goto cleanup;
- if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0)
- goto cleanup;
- if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path,
ISCSI_SESSION_NORMAL) < 0)
- goto cleanup;
- if (virISCSIDirectConnect(iscsi, portal) < 0)
+ if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
goto cleanup;
if (virISCSIDirectReportLuns(pool, iscsi, portal) < 0)
goto disconect;
@@ -581,9 +595,9 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
ret = 0;
disconect:
virISCSIDirectDisconnect(iscsi);
+ iscsi_destroy_context(iscsi);
cleanup:
VIR_FREE(portal);
- iscsi_destroy_context(iscsi);
return ret;
}
Michal
5:26 p.m.
New subject: [libvirt] [PATCH v3 3/3] storage: add wipeVol to iscsi-direct storage backend
From: Clementine Hayat <clem(a)lse.epita.fr>
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
src/storage/storage_backend_iscsi_direct.c | 121 ++++++++++++++++++++-
1 file changed, 120 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
index 7bdd39582b..5a7d70f24d 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -41,6 +41,8 @@
#define ISCSI_DEFAULT_TARGET_PORT 3260
#define VIR_ISCSI_TEST_UNIT_TIMEOUT 30 * 1000
+#define BLOCK_PER_PACKET 128
+#define VOL_NAME_PREFIX "unit:0:0:"
VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
@@ -225,7 +227,7 @@ virISCSIDirectSetVolumeAttributes(virStoragePoolObjPtr pool,
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- if (virAsprintf(&vol->name, "unit:0:0:%u", lun) < 0)
+ if (virAsprintf(&vol->name, "%s%u", VOL_NAME_PREFIX, lun) < 0)
return -1;
if (virAsprintf(&vol->key, "ip-%s-iscsi-%s-lun-%u", portal,
def->source.devices[0].path, lun) < 0)
@@ -601,12 +603,129 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool)
return ret;
}
+static int
+virStorageBackendISCSIDirectGetLun(virStorageVolDefPtr vol,
+ int *lun)
+{
+ const char *name = vol->name;
+ int ret = -1;
+
+ if (!STRPREFIX(name, VOL_NAME_PREFIX)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid volume name %s"), name);
+ goto cleanup;
+ }
+
+ name += strlen(VOL_NAME_PREFIX);
+ if (virStrToLong_i(name, NULL, 10, lun) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+static int
+virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol,
+ struct iscsi_context *iscsi)
+{
+ uint32_t lba = 0;
+ uint32_t block_size;
+ uint32_t nb_block;
+ struct scsi_task *task = NULL;
+ int lun = 0;
+ int ret = -1;
+ unsigned char *data;
+
+ if (virStorageBackendISCSIDirectGetLun(vol, &lun) < 0)
+ return ret;
+ if (virISCSIDirectTestUnitReady(iscsi, lun) < 0)
+ return ret;
+ if (virISCSIDirectGetVolumeCapacity(iscsi, lun, &block_size, &nb_block))
+ return ret;
+ if (VIR_ALLOC_N(data, block_size * BLOCK_PER_PACKET))
+ return ret;
+
+ while (lba < nb_block) {
+ if (nb_block - lba > block_size * BLOCK_PER_PACKET) {
+
+ if (!(task = iscsi_write10_sync(iscsi, lun, lba, data,
+ block_size * BLOCK_PER_PACKET,
+ block_size, 0, 0, 0, 0, 0)))
+ goto cleanup;
+ scsi_free_scsi_task(task);
+ lba += BLOCK_PER_PACKET;
+ } else {
+ if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, block_size,
+ block_size, 0, 0, 0, 0, 0)))
+ goto cleanup;
+ scsi_free_scsi_task(task);
+ lba++;
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(data);
+ return ret;
+}
+
+static int
+virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ unsigned int algorithm,
+ unsigned int flags)
+{
+ struct iscsi_context *iscsi = NULL;
+ char *portal = NULL;
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
+ goto cleanup;
+
+ switch ((virStorageVolWipeAlgorithm) algorithm) {
+ case VIR_STORAGE_VOL_WIPE_ALG_ZERO:
+ if (virStorageBackendISCSIDirectVolWipeZero(vol, iscsi) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to wipe volume %s"),
+ vol->name);
+ goto disconnect;
+ }
+ break;
+ case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
+ case VIR_STORAGE_VOL_WIPE_ALG_NNSA:
+ case VIR_STORAGE_VOL_WIPE_ALG_DOD:
+ case VIR_STORAGE_VOL_WIPE_ALG_BSI:
+ case VIR_STORAGE_VOL_WIPE_ALG_GUTMANN:
+ case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER:
+ case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7:
+ case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
+ case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
+ case VIR_STORAGE_VOL_WIPE_ALG_LAST:
+ virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"),
+ algorithm);
+ goto disconnect;
+ }
+
+ ret = 0;
+ disconnect:
+ virISCSIDirectDisconnect(iscsi);
+ iscsi_destroy_context(iscsi);
+ cleanup:
+ VIR_FREE(portal);
+ return ret;
+}
+
+
virStorageBackend virStorageBackendISCSIDirect = {
.type = VIR_STORAGE_POOL_ISCSI_DIRECT,
.checkPool = virStorageBackendISCSIDirectCheckPool,
.findPoolSources = virStorageBackendISCSIDirectFindPoolSources,
.refreshPool = virStorageBackendISCSIDirectRefreshPool,
+ .wipeVol = virStorageBackenISCSIDirectWipeVol,
};
int
--
2.18.0
3:10 a.m.
New subject: [libvirt] [PATCH v3 3/3] storage: add wipeVol to iscsi-direct storage backend
On 08/13/2018 12:26 AM, clem(a)lse.epita.fr wrote:
From: Clementine Hayat <clem(a)lse.epita.fr>
Signed-off-by: Clementine Hayat <clem(a)lse.epita.fr>
---
src/storage/storage_backend_iscsi_direct.c | 121 ++++++++++++++++++++-
1 file changed, 120 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c
b/src/storage/storage_backend_iscsi_direct.c
index 7bdd39582b..5a7d70f24d 100644
--- a/src/storage/storage_backend_iscsi_direct.c
+++ b/src/storage/storage_backend_iscsi_direct.c
@@ -41,6 +41,8 @@
#define ISCSI_DEFAULT_TARGET_PORT 3260
#define VIR_ISCSI_TEST_UNIT_TIMEOUT 30 * 1000
+#define BLOCK_PER_PACKET 128
+#define VOL_NAME_PREFIX "unit:0:0:"
VIR_LOG_INIT("storage.storage_backend_iscsi_direct");
@@ -225,7 +227,7 @@ virISCSIDirectSetVolumeAttributes(virStoragePoolObjPtr pool,
{
virStoragePoolDefPtr def = virStoragePoolObjGetDef(pool);
- if (virAsprintf(&vol->name, "unit:0:0:%u", lun) < 0)
+ if (virAsprintf(&vol->name, "%s%u", VOL_NAME_PREFIX, lun) < 0)
return -1;
if (virAsprintf(&vol->key, "ip-%s-iscsi-%s-lun-%u", portal,
def->source.devices[0].path, lun) < 0)
@@ -601,12 +603,129 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr
pool)
return ret;
}
+static int
+virStorageBackendISCSIDirectGetLun(virStorageVolDefPtr vol,
+ int *lun)
+{
+ const char *name = vol->name;
+ int ret = -1;
+
+ if (!STRPREFIX(name, VOL_NAME_PREFIX)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Invalid volume name %s"), name);
+ goto cleanup;
+ }
+
+ name += strlen(VOL_NAME_PREFIX);
+ if (virStrToLong_i(name, NULL, 10, lun) < 0)
+ goto cleanup;
+
+ ret = 0;
+ cleanup:
+ return ret;
+}
+
+static int
+virStorageBackendISCSIDirectVolWipeZero(virStorageVolDefPtr vol,
+ struct iscsi_context *iscsi)
+{
+ uint32_t lba = 0;
+ uint32_t block_size;
+ uint32_t nb_block;
+ struct scsi_task *task = NULL;
+ int lun = 0;
+ int ret = -1;
+ unsigned char *data;
+
+ if (virStorageBackendISCSIDirectGetLun(vol, &lun) < 0)
+ return ret;
+ if (virISCSIDirectTestUnitReady(iscsi, lun) < 0)
+ return ret;
+ if (virISCSIDirectGetVolumeCapacity(iscsi, lun, &block_size, &nb_block))
+ return ret;
+ if (VIR_ALLOC_N(data, block_size * BLOCK_PER_PACKET))
+ return ret;
+
+ while (lba < nb_block) {
+ if (nb_block - lba > block_size * BLOCK_PER_PACKET) {
+
+ if (!(task = iscsi_write10_sync(iscsi, lun, lba, data,
+ block_size * BLOCK_PER_PACKET,
+ block_size, 0, 0, 0, 0, 0)))
+ goto cleanup;
+ scsi_free_scsi_task(task);
+ lba += BLOCK_PER_PACKET;
+ } else {
+ if (!(task = iscsi_write10_sync(iscsi, lun, lba, data, block_size,
+ block_size, 0, 0, 0, 0, 0)))
+ goto cleanup;
+ scsi_free_scsi_task(task);
+ lba++;
+ }
+ }
+
+ ret = 0;
+ cleanup:
+ VIR_FREE(data);
+ return ret;
+}
+
+static int
+virStorageBackenISCSIDirectWipeVol(virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
+ unsigned int algorithm,
+ unsigned int flags)
+{
+ struct iscsi_context *iscsi = NULL;
+ char *portal = NULL;
After my changes in 2/3 this variable is not needed.
+ int ret = -1;
+
+ virCheckFlags(0, -1);
+
+ if (!(iscsi = virStorageBackendISCSIDirectSetConnection(pool, &portal)))
+ goto cleanup;
+
+ switch ((virStorageVolWipeAlgorithm) algorithm) {
+ case VIR_STORAGE_VOL_WIPE_ALG_ZERO:
+ if (virStorageBackendISCSIDirectVolWipeZero(vol, iscsi) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to wipe volume %s"),
+ vol->name);
+ goto disconnect;
+ }
+ break;
+ case VIR_STORAGE_VOL_WIPE_ALG_TRIM:
+ case VIR_STORAGE_VOL_WIPE_ALG_NNSA:
+ case VIR_STORAGE_VOL_WIPE_ALG_DOD:
+ case VIR_STORAGE_VOL_WIPE_ALG_BSI:
+ case VIR_STORAGE_VOL_WIPE_ALG_GUTMANN:
+ case VIR_STORAGE_VOL_WIPE_ALG_SCHNEIER:
+ case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER7:
+ case VIR_STORAGE_VOL_WIPE_ALG_PFITZNER33:
+ case VIR_STORAGE_VOL_WIPE_ALG_RANDOM:
+ case VIR_STORAGE_VOL_WIPE_ALG_LAST:
+ virReportError(VIR_ERR_INVALID_ARG, _("unsupported algorithm %d"),
+ algorithm);
+ goto disconnect;
It's a pity we have to connect even though we know whether requested
algorithm is supported or not. However, I'm unable to come up with
generic enough code that would work for more than this single case (e.g.
when we introduce ALG_RANDOM).
+ }
+
+ ret = 0;
+ disconnect:
+ virISCSIDirectDisconnect(iscsi);
+ iscsi_destroy_context(iscsi);
+ cleanup:
+ VIR_FREE(portal);
+ return ret;
+}
+
+
virStorageBackend virStorageBackendISCSIDirect = {
.type = VIR_STORAGE_POOL_ISCSI_DIRECT,
.checkPool = virStorageBackendISCSIDirectCheckPool,
.findPoolSources = virStorageBackendISCSIDirectFindPoolSources,
.refreshPool = virStorageBackendISCSIDirectRefreshPool,
+ .wipeVol = virStorageBackenISCSIDirectWipeVol,
};
int
3:10 a.m.
On 08/13/2018 12:26 AM, clem(a)lse.epita.fr wrote:
From: Clementine Hayat <clem(a)lse.epita.fr>
Hello,
This series of patch is the follow up of:
v1:https://www.redhat.com/archives/libvir-list/2018-August/msg00557.html
v2:https://www.redhat.com/archives/libvir-list/2018-August/msg00570.html
Best Regards,
I'm fixing all the issues I've found, ACKing and pushing.
Michal
2293
days inactive
2294
days old
6 comments
2 participants
participants (2)
-
clem＠lse.epita.fr -
Michal Prívozník