[libvirt] [PATCH] storage: add findPoolSources to iscsi_direct pool backend
From: Clementine Hayat <clem@lse.epita.fr> Change the SetContext function to be able to take the session type in argument. Took the function findPoolSources of iscsi backend and wired it to my function since the formatting is essentially the same. Signed-off-by: Clementine Hayat <clem@lse.epita.fr> --- src/storage/storage_backend_iscsi_direct.c | 179 ++++++++++++++++++++- 1 file changed, 171 insertions(+), 8 deletions(-) diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index ab192730fb..fc30f2dfac 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -131,7 +131,8 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi, static int virISCSIDirectSetContext(struct iscsi_context *iscsi, - const char *target_name) + const char *target_name, + enum iscsi_session_type session) { if (iscsi_init_transport(iscsi, TCP_TRANSPORT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -139,13 +140,15 @@ virISCSIDirectSetContext(struct iscsi_context *iscsi, iscsi_get_error(iscsi)); return -1; } - if (iscsi_set_targetname(iscsi, target_name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to set target name: %s"), - iscsi_get_error(iscsi)); - return -1; + if (session == ISCSI_SESSION_NORMAL) { + if (iscsi_set_targetname(iscsi, target_name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to set target name: %s"), + iscsi_get_error(iscsi)); + return -1; + } } - if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) < 0) { + if (iscsi_set_session_type(iscsi, session) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set session type: %s"), iscsi_get_error(iscsi)); @@ -400,6 +403,92 @@ virISCSIDirectDisconnect(struct iscsi_context *iscsi) return 0; } +static void +virFreeTargets(char **targets, size_t ntargets, char *target) +{ + size_t i; + + VIR_FREE(target); + for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets); +} + +static int +virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, + size_t *ntargets, + char ***targets) +{ + int ret = -1; + struct iscsi_discovery_address *addr; + struct iscsi_discovery_address *tmp_addr; + size_t tmp_ntargets = 0; + char **tmp_targets = NULL; + + if (!(addr = iscsi_discovery_sync(iscsi))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to discover session: %s"), + iscsi_get_error(iscsi)); + return ret; + } + *ntargets = 0; + for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { + char *target = NULL; + if (VIR_STRDUP(target, tmp_addr->target_name) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, NULL); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate memory for: %s"), + tmp_addr->target_name); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, target); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to append to the list element: %s"), + tmp_addr->target_name); + goto cleanup; + } + + } + + if (tmp_ntargets) { + *targets = tmp_targets; + *ntargets = tmp_ntargets; + } + + ret = 0; + cleanup: + iscsi_free_discovery_data(iscsi, addr); + return ret; +} + +static int +virISCSIDirectScanTargets(char *initiator_iqn, + char *portal, + size_t *ntargets, + char ***targets) +{ + struct iscsi_context *iscsi = NULL; + int ret = -1; + + if (!(iscsi = virISCSIDirectCreateContext(initiator_iqn))) + goto cleanup; + if (virISCSIDirectSetContext(iscsi, NULL, ISCSI_SESSION_DISCOVERY) < 0) + goto cleanup; + if (virISCSIDirectConnect(iscsi, portal) < 0) + goto cleanup; + if (virISCSIDirectUpdateTargets(iscsi, ntargets, targets) < 0) + goto disconnect; + + ret = 0; + disconnect: + virISCSIDirectDisconnect(iscsi); + cleanup: + iscsi_destroy_context(iscsi); + return ret; +} + static int virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, bool *isActive) @@ -408,6 +497,79 @@ virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, return 0; } +static char * +virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, + unsigned int flags) +{ + virStoragePoolSourcePtr source = NULL; + size_t ntargets = 0; + char **targets = NULL; + char *ret = NULL; + size_t i; + virStoragePoolSourceList list = { + .type = VIR_STORAGE_POOL_ISCSI_DIRECT, + .nsources = 0, + .sources = NULL + }; + char *portal = NULL; + + virCheckFlags(0, NULL); + + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for iscsi sources")); + return NULL; + } + + if (!(source = virStoragePoolDefParseSourceString(srcSpec, list.type))) + return NULL; + + if (source->nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + } + + if (!(portal = virStorageBackendISCSIDirectPortal(source))) + goto cleanup; + + if (virISCSIDirectScanTargets(source->initiator.iqn, portal, &ntargets, &targets) < 0) + goto cleanup; + + if (VIR_ALLOC_N(list.sources, ntargets) < 0) + goto cleanup; + + for (i = 0; i < ntargets; i++) { + if (VIR_ALLOC_N(list.sources[i].devices, 1) < 0 || + VIR_ALLOC_N(list.sources[i].hosts, 1) < 0) + goto cleanup; + list.sources[i].nhost = 1; + list.sources[i].hosts[0] = source->hosts[0]; + list.sources[i].initiator = source->initiator; + list.sources[i].ndevice = 1; + list.sources[i].devices[0].path = targets[i]; + list.nsources++; + } + + if (!(ret = virStoragePoolSourceListFormat(&list))) + goto cleanup; + + cleanup: + if (list.sources) { + for (i = 0; i < ntargets; i++) { + VIR_FREE(list.sources[i].hosts); + VIR_FREE(list.sources[i].devices); + } + VIR_FREE(list.sources); + } + for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets); + VIR_FREE(portal); + virStoragePoolSourceFree(source); + return ret; +} + static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { @@ -422,7 +584,7 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) goto cleanup; if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0) goto cleanup; - if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path) < 0) + if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path, ISCSI_SESSION_NORMAL) < 0) goto cleanup; if (virISCSIDirectConnect(iscsi, portal) < 0) goto cleanup; @@ -442,6 +604,7 @@ virStorageBackend virStorageBackendISCSIDirect = { .type = VIR_STORAGE_POOL_ISCSI_DIRECT, .checkPool = virStorageBackendISCSIDirectCheckPool, + .findPoolSources = virStorageBackendISCSIDirectFindPoolSources, .refreshPool = virStorageBackendISCSIDirectRefreshPool, }; -- 2.18.0
On 08/02/2018 10:33 AM, clem@lse.epita.fr wrote:
From: Clementine Hayat <clem@lse.epita.fr>
Change the SetContext function to be able to take the session type in argument. Took the function findPoolSources of iscsi backend and wired it to my function since the formatting is essentially the same.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr> --- src/storage/storage_backend_iscsi_direct.c | 179 ++++++++++++++++++++- 1 file changed, 171 insertions(+), 8 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index ab192730fb..fc30f2dfac 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -131,7 +131,8 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi,
static int virISCSIDirectSetContext(struct iscsi_context *iscsi, - const char *target_name) + const char *target_name, + enum iscsi_session_type session) { if (iscsi_init_transport(iscsi, TCP_TRANSPORT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -139,13 +140,15 @@ virISCSIDirectSetContext(struct iscsi_context *iscsi, iscsi_get_error(iscsi)); return -1; } - if (iscsi_set_targetname(iscsi, target_name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to set target name: %s"), - iscsi_get_error(iscsi)); - return -1; + if (session == ISCSI_SESSION_NORMAL) { + if (iscsi_set_targetname(iscsi, target_name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to set target name: %s"), + iscsi_get_error(iscsi)); + return -1; + } } - if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) < 0) { + if (iscsi_set_session_type(iscsi, session) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set session type: %s"), iscsi_get_error(iscsi)); @@ -400,6 +403,92 @@ virISCSIDirectDisconnect(struct iscsi_context *iscsi) return 0; }
+static void +virFreeTargets(char **targets, size_t ntargets, char *target) +{ + size_t i; + + VIR_FREE(target);
+ for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets);
This is virStringListFreeCount().
+} + +static int +virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, + size_t *ntargets, + char ***targets) +{ + int ret = -1; + struct iscsi_discovery_address *addr; + struct iscsi_discovery_address *tmp_addr; + size_t tmp_ntargets = 0; + char **tmp_targets = NULL; + + if (!(addr = iscsi_discovery_sync(iscsi))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to discover session: %s"), + iscsi_get_error(iscsi)); + return ret; + } + *ntargets = 0;
1: ^^
+ for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { + char *target = NULL; + if (VIR_STRDUP(target, tmp_addr->target_name) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, NULL);
Instead of calling this function in every failure path, how about dissolving it under clean up label?
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate memory for: %s"), + tmp_addr->target_name);
No need to set error message. VIR_STRDUP() reports OOM.
+ goto cleanup; + } + + if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, target); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to append to the list element: %s"), + tmp_addr->target_name);
No need
+ goto cleanup; + } + + } + + if (tmp_ntargets) {
So I'd just drop this check and replace [1] with an empty line.
+ *targets = tmp_targets; + *ntargets = tmp_ntargets; + } + + ret = 0; + cleanup: + iscsi_free_discovery_data(iscsi, addr); + return ret; +} + +static int +virISCSIDirectScanTargets(char *initiator_iqn, + char *portal, + size_t *ntargets, + char ***targets) +{ + struct iscsi_context *iscsi = NULL; + int ret = -1; + + if (!(iscsi = virISCSIDirectCreateContext(initiator_iqn))) + goto cleanup; + if (virISCSIDirectSetContext(iscsi, NULL, ISCSI_SESSION_DISCOVERY) < 0) + goto cleanup; + if (virISCSIDirectConnect(iscsi, portal) < 0) + goto cleanup; + if (virISCSIDirectUpdateTargets(iscsi, ntargets, targets) < 0) + goto disconnect; + + ret = 0; + disconnect: + virISCSIDirectDisconnect(iscsi); + cleanup: + iscsi_destroy_context(iscsi); + return ret; +} + static int virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, bool *isActive) @@ -408,6 +497,79 @@ virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, return 0; }
+static char * +virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, + unsigned int flags) +{ + virStoragePoolSourcePtr source = NULL; + size_t ntargets = 0; + char **targets = NULL; + char *ret = NULL; + size_t i; + virStoragePoolSourceList list = { + .type = VIR_STORAGE_POOL_ISCSI_DIRECT, + .nsources = 0, + .sources = NULL + }; + char *portal = NULL; + + virCheckFlags(0, NULL); + + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for iscsi sources")); + return NULL; + } + + if (!(source = virStoragePoolDefParseSourceString(srcSpec, list.type))) + return NULL; + + if (source->nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + }
2: here ^^
+ + if (!(portal = virStorageBackendISCSIDirectPortal(source))) + goto cleanup; + + if (virISCSIDirectScanTargets(source->initiator.iqn, portal, &ntargets, &targets) < 0) + goto cleanup;
so if source->initiator.iqn is NULL (because it wasn't provided by user) then libiscsi crashes us instantly. You need to check for it somewhere around [2] (where you already do another checks). The same problem exists when starting new pool.
+ + if (VIR_ALLOC_N(list.sources, ntargets) < 0) + goto cleanup; + + for (i = 0; i < ntargets; i++) { + if (VIR_ALLOC_N(list.sources[i].devices, 1) < 0 || + VIR_ALLOC_N(list.sources[i].hosts, 1) < 0) + goto cleanup; + list.sources[i].nhost = 1; + list.sources[i].hosts[0] = source->hosts[0]; + list.sources[i].initiator = source->initiator; + list.sources[i].ndevice = 1; + list.sources[i].devices[0].path = targets[i]; + list.nsources++; + } + + if (!(ret = virStoragePoolSourceListFormat(&list))) + goto cleanup; + + cleanup: + if (list.sources) { + for (i = 0; i < ntargets; i++) { + VIR_FREE(list.sources[i].hosts); + VIR_FREE(list.sources[i].devices); + } + VIR_FREE(list.sources); + } + for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets); + VIR_FREE(portal); + virStoragePoolSourceFree(source); + return ret; +} + static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { @@ -422,7 +584,7 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) goto cleanup; if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0) goto cleanup; - if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path) < 0) + if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path, ISCSI_SESSION_NORMAL) < 0) goto cleanup; if (virISCSIDirectConnect(iscsi, portal) < 0) goto cleanup; @@ -442,6 +604,7 @@ virStorageBackend virStorageBackendISCSIDirect = { .type = VIR_STORAGE_POOL_ISCSI_DIRECT,
.checkPool = virStorageBackendISCSIDirectCheckPool, + .findPoolSources = virStorageBackendISCSIDirectFindPoolSources, .refreshPool = virStorageBackendISCSIDirectRefreshPool, };
Otherwise looking good. Fixing all the issues I've found locally and ACKing. Will push after the release. Michal
On Thu, Aug 02, 2018 at 10:33:27AM +0200, clem@lse.epita.fr wrote:
From: Clementine Hayat <clem@lse.epita.fr>
Change the SetContext function to be able to take the session type in argument. Took the function findPoolSources of iscsi backend and wired it to my function since the formatting is essentially the same.
Signed-off-by: Clementine Hayat <clem@lse.epita.fr> ---
There should be a note that this is a follow-up to the series sent separately.
src/storage/storage_backend_iscsi_direct.c | 179 ++++++++++++++++++++- 1 file changed, 171 insertions(+), 8 deletions(-)
diff --git a/src/storage/storage_backend_iscsi_direct.c b/src/storage/storage_backend_iscsi_direct.c index ab192730fb..fc30f2dfac 100644 --- a/src/storage/storage_backend_iscsi_direct.c +++ b/src/storage/storage_backend_iscsi_direct.c @@ -131,7 +131,8 @@ virStorageBackendISCSIDirectSetAuth(struct iscsi_context *iscsi,
static int virISCSIDirectSetContext(struct iscsi_context *iscsi, - const char *target_name) + const char *target_name, + enum iscsi_session_type session) { if (iscsi_init_transport(iscsi, TCP_TRANSPORT) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -139,13 +140,15 @@ virISCSIDirectSetContext(struct iscsi_context *iscsi, iscsi_get_error(iscsi)); return -1; } - if (iscsi_set_targetname(iscsi, target_name) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to set target name: %s"), - iscsi_get_error(iscsi)); - return -1; + if (session == ISCSI_SESSION_NORMAL) { + if (iscsi_set_targetname(iscsi, target_name) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to set target name: %s"), + iscsi_get_error(iscsi)); + return -1; + } } - if (iscsi_set_session_type(iscsi, ISCSI_SESSION_NORMAL) < 0) { + if (iscsi_set_session_type(iscsi, session) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Failed to set session type: %s"), iscsi_get_error(iscsi)); @@ -400,6 +403,92 @@ virISCSIDirectDisconnect(struct iscsi_context *iscsi) return 0; }
+static void +virFreeTargets(char **targets, size_t ntargets, char *target) +{ + size_t i; + + VIR_FREE(target); + for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets); +}
You don't really need ^this method, see below.
+ +static int +virISCSIDirectUpdateTargets(struct iscsi_context *iscsi, + size_t *ntargets, + char ***targets) +{ + int ret = -1; + struct iscsi_discovery_address *addr; + struct iscsi_discovery_address *tmp_addr; + size_t tmp_ntargets = 0; + char **tmp_targets = NULL; + + if (!(addr = iscsi_discovery_sync(iscsi))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to discover session: %s"), + iscsi_get_error(iscsi)); + return ret; + } + *ntargets = 0; + for (tmp_addr = addr; tmp_addr; tmp_addr = tmp_addr->next) { + char *target = NULL;
So, Pavel or Michal might have further suggestions regarding the design, I'm only going to comment on a few things related to our coding style that caught my eye. Anyhow, if ^this hunk is to stay, then target could be declared as VIR_AUTOFREE, thus not having to free it explicitly.
+ if (VIR_STRDUP(target, tmp_addr->target_name) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, NULL);
Since tmp_targets is char **, you can use the already existing virStringListFreeCount method. Also, if you use VIR_STEAL_PTR as I'm suggesting below, freeing can be moved to the cleanup section.
+ virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to allocate memory for: %s"), + tmp_addr->target_name); + goto cleanup; + } + + if (VIR_APPEND_ELEMENT(tmp_targets, tmp_ntargets, target) < 0) { + virFreeTargets(tmp_targets, tmp_ntargets, target); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to append to the list element: %s"), + tmp_addr->target_name); + goto cleanup; + } + + } + + if (tmp_ntargets) {
Do we have to make this conditional? I mean, if there were any targets, you'd rewrite the caller-provided pointer anyway, so rewriting it to NULL on success because there were no targets is IMHO fine. Besides, you already reset *ntargets to 0 above, so I think the conditional should be dropped.
+ *targets = tmp_targets;
^This should become VIR_STEAL_PTR(*targets, tmp_targets); instead.
+ *ntargets = tmp_ntargets; + } + + ret = 0; + cleanup: + iscsi_free_discovery_data(iscsi, addr); + return ret; +} + +static int +virISCSIDirectScanTargets(char *initiator_iqn, + char *portal, + size_t *ntargets, + char ***targets) +{ + struct iscsi_context *iscsi = NULL; + int ret = -1; + + if (!(iscsi = virISCSIDirectCreateContext(initiator_iqn))) + goto cleanup; + if (virISCSIDirectSetContext(iscsi, NULL, ISCSI_SESSION_DISCOVERY) < 0) + goto cleanup; + if (virISCSIDirectConnect(iscsi, portal) < 0) + goto cleanup; + if (virISCSIDirectUpdateTargets(iscsi, ntargets, targets) < 0) + goto disconnect; + + ret = 0; + disconnect: + virISCSIDirectDisconnect(iscsi); + cleanup: + iscsi_destroy_context(iscsi); + return ret; +} + static int virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, bool *isActive) @@ -408,6 +497,79 @@ virStorageBackendISCSIDirectCheckPool(virStoragePoolObjPtr pool, return 0; }
+static char * +virStorageBackendISCSIDirectFindPoolSources(const char *srcSpec, + unsigned int flags) +{ + virStoragePoolSourcePtr source = NULL; + size_t ntargets = 0; + char **targets = NULL; + char *ret = NULL; + size_t i; + virStoragePoolSourceList list = { + .type = VIR_STORAGE_POOL_ISCSI_DIRECT, + .nsources = 0, + .sources = NULL + }; + char *portal = NULL; + + virCheckFlags(0, NULL); + + if (!srcSpec) { + virReportError(VIR_ERR_INVALID_ARG, "%s", + _("hostname must be specified for iscsi sources")); + return NULL; + } + + if (!(source = virStoragePoolDefParseSourceString(srcSpec, list.type))) + return NULL; + + if (source->nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + } + + if (!(portal = virStorageBackendISCSIDirectPortal(source))) + goto cleanup; + + if (virISCSIDirectScanTargets(source->initiator.iqn, portal, &ntargets, &targets) < 0) + goto cleanup; + + if (VIR_ALLOC_N(list.sources, ntargets) < 0) + goto cleanup; + + for (i = 0; i < ntargets; i++) { + if (VIR_ALLOC_N(list.sources[i].devices, 1) < 0 || + VIR_ALLOC_N(list.sources[i].hosts, 1) < 0)
Plain VIR_ALLOC will do just fine ^here.
+ goto cleanup; + list.sources[i].nhost = 1; + list.sources[i].hosts[0] = source->hosts[0]; + list.sources[i].initiator = source->initiator; + list.sources[i].ndevice = 1; + list.sources[i].devices[0].path = targets[i]; + list.nsources++; + } + + if (!(ret = virStoragePoolSourceListFormat(&list))) + goto cleanup; + + cleanup: + if (list.sources) { + for (i = 0; i < ntargets; i++) { + VIR_FREE(list.sources[i].hosts); + VIR_FREE(list.sources[i].devices); + } + VIR_FREE(list.sources);
Hmm, I'm wondering if it weren't beneficial to have a virStoragePoolSourceListFree wrapper for ^this.
+ } + for (i = 0; i < ntargets; i++) + VIR_FREE(targets[i]); + VIR_FREE(targets);
virStringListFreeCount ^here as well...
+ VIR_FREE(portal); + virStoragePoolSourceFree(source); + return ret; +} + static int virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) { @@ -422,7 +584,7 @@ virStorageBackendISCSIDirectRefreshPool(virStoragePoolObjPtr pool) goto cleanup; if (virStorageBackendISCSIDirectSetAuth(iscsi, &def->source) < 0) goto cleanup; - if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path) < 0) + if (virISCSIDirectSetContext(iscsi, def->source.devices[0].path, ISCSI_SESSION_NORMAL) < 0) goto cleanup; if (virISCSIDirectConnect(iscsi, portal) < 0) goto cleanup; @@ -442,6 +604,7 @@ virStorageBackend virStorageBackendISCSIDirect = { .type = VIR_STORAGE_POOL_ISCSI_DIRECT,
.checkPool = virStorageBackendISCSIDirectCheckPool, + .findPoolSources = virStorageBackendISCSIDirectFindPoolSources, .refreshPool = virStorageBackendISCSIDirectRefreshPool, };
-- 2.18.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
clem＠lse.epita.fr -
Erik Skultety -
Michal Privoznik