[libvirt] [PATCH v2 0/4] Don't disable auto_login for iSCSI targets
v1: http://www.redhat.com/archives/libvir-list/2016-May/msg01027.html Changes since v2: * Patch 2 * Remove the 'str' fetch/free - it was unnecessary * Initialize and free 'error' * Did not adjust reported error since with the usage of exitstatus the call to virCommandRunRegex, that means the virCommandWait code will return the error rather than reporting it. So the reporting is left to this code to handle. Kept the same error code (INTERNAL) as virCommandWait Patch 3-5 * Remove 3, combine 4 & 5, but remove the setting of the autologin for libvirt managed targets. This is essentially Fritz's initial patch: http://www.redhat.com/archives/libvir-list/2016-May/msg00002.html but with the adjustment to add the "ATTRIBUTE_UNUSED" for the initiatoriqn parameter (it'll be removed in the next patch as it is a bit more invasive). Fritz Elfert (1): util: Remove disabling of autologin for iscsi-targets John Ferlan (3): util: Add exitstatus parameter to virCommandRunRegex iscsi: Add exit status checking for virISCSIGetSession iscsi: Remove initiatoriqn from virISCSIScanTargets src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_iscsi.c | 8 ++--- src/storage/storage_backend_logical.c | 10 +++--- src/util/vircommand.c | 9 ++++-- src/util/vircommand.h | 3 +- src/util/viriscsi.c | 60 +++++++++++++++-------------------- src/util/viriscsi.h | 1 - tests/viriscsitest.c | 7 ++-- 8 files changed, 46 insertions(+), 54 deletions(-) -- 2.5.5
Rather than have virCommandRun just spit out the error, allow callers to decide to pass the exitstatus so the caller can make intelligent decisions based on the error. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_fs.c | 2 +- src/storage/storage_backend_logical.c | 10 ++++++---- src/util/vircommand.c | 9 ++++++--- src/util/vircommand.h | 3 ++- src/util/viriscsi.c | 4 ++-- 5 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 02a129e..45474cb 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -267,7 +267,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) if (virCommandRunRegex(cmd, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, - state, NULL) < 0) + state, NULL, NULL) < 0) goto cleanup; ret = 0; diff --git a/src/storage/storage_backend_logical.c b/src/storage/storage_backend_logical.c index 90a194e..ca05fe1 100644 --- a/src/storage/storage_backend_logical.c +++ b/src/storage/storage_backend_logical.c @@ -398,7 +398,8 @@ virStorageBackendLogicalFindLVs(virStoragePoolObjPtr pool, vars, virStorageBackendLogicalMakeVol, &cbdata, - "lvs") < 0) + "lvs", + NULL) < 0) goto cleanup; ret = 0; @@ -511,10 +512,10 @@ virStorageBackendLogicalGetPoolSources(virStoragePoolSourceListPtr sourceList) cmd = virCommandNewArgList(PVS, "--noheadings", "-o", "pv_name,vg_name", - NULL); + NULL, NULL); if (virCommandRunRegex(cmd, 1, regexes, vars, virStorageBackendLogicalFindPoolSourcesFunc, - sourceList, "pvs") < 0) + sourceList, "pvs", NULL) < 0) goto cleanup; ret = 0; @@ -799,7 +800,8 @@ virStorageBackendLogicalRefreshPool(virConnectPtr conn ATTRIBUTE_UNUSED, vars, virStorageBackendLogicalRefreshPoolFunc, pool, - "vgs") < 0) + "vgs", + NULL) < 0) goto cleanup; ret = 0; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 027cb64..f5bd7af 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -2900,6 +2900,7 @@ virCommandSetDryRun(virBufferPtr buf, * needs to return 0 on success * @data: additional data that will be passed to the callback function * @prefix: prefix that will be skipped at the beginning of each line + * @exitstatus: allows the caller to handle command run exit failures * * Run an external program. * @@ -2917,7 +2918,8 @@ virCommandRunRegex(virCommandPtr cmd, int *nvars, virCommandRunRegexFunc func, void *data, - const char *prefix) + const char *prefix, + int *exitstatus) { int err; regex_t *reg; @@ -2959,7 +2961,7 @@ virCommandRunRegex(virCommandPtr cmd, goto cleanup; virCommandSetOutputBuffer(cmd, &outbuf); - if (virCommandRun(cmd, NULL) < 0) + if (virCommandRun(cmd, exitstatus) < 0) goto cleanup; if (!outbuf) { @@ -3114,7 +3116,8 @@ virCommandRunRegex(virCommandPtr cmd ATTRIBUTE_UNUSED, int *nvars ATTRIBUTE_UNUSED, virCommandRunRegexFunc func ATTRIBUTE_UNUSED, void *data ATTRIBUTE_UNUSED, - const char *prefix ATTRIBUTE_UNUSED) + const char *prefix ATTRIBUTE_UNUSED, + int *exitstatus ATTRIBUTE_UNUSED) { virReportError(VIR_ERR_INTERNAL_ERROR, _("%s not implemented on Win32"), __FUNCTION__); diff --git a/src/util/vircommand.h b/src/util/vircommand.h index 198da2f..44818ef 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -205,7 +205,8 @@ int virCommandRunRegex(virCommandPtr cmd, int *nvars, virCommandRunRegexFunc func, void *data, - const char *cmd_to_ignore); + const char *cmd_to_ignore, + int *exitstatus); int virCommandRunNul(virCommandPtr cmd, size_t n_columns, diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index bd34fea..846ea68 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -87,7 +87,7 @@ virISCSIGetSession(const char *devpath, regexes, vars, virISCSIExtractSession, - &cbdata, NULL) < 0) + &cbdata, NULL, NULL) < 0) goto cleanup; if (cbdata.session == NULL && !probe) { @@ -437,7 +437,7 @@ virISCSIScanTargets(const char *portal, regexes, vars, virISCSIGetTargets, - &list, NULL) < 0) + &list, NULL, NULL) < 0) goto cleanup; for (i = 0; i < list.ntargets; i++) { -- 2.5.5
Utilize the exit status parameter for virCommandRunRegex in order to check the return error from the 'iscsiadm --mode session' command. Without this enabled, if there are no sessions running then virCommandRun would have displayed an error such as: 2016-05-13 15:17:15.165+0000: 10920: error : virCommandWait:2553 : internal error: Child process (iscsiadm --mode session) unexpected exit status 21: iscsiadm: No active sessions. It is possible that for certain paths (when probe is true) we only care whether it's running or not to make certain decisions. Spitting out the error for those paths is unnecessary. If we do have a situation where probe = false and there's an error, then display the error from iscsiadm if it's there; otherwise, default to the non descript error. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/viriscsi.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 846ea68..3627eed 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -32,6 +32,7 @@ #include "virerror.h" #include "virfile.h" #include "virlog.h" +#include "virprocess.h" #include "virrandom.h" #include "virstring.h" @@ -79,24 +80,40 @@ virISCSIGetSession(const char *devpath, .session = NULL, .devpath = devpath, }; + char *error = NULL; + int exitstatus = 0; - virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", "session", NULL); + virCommandPtr cmd = virCommandNewArgList(ISCSIADM, "--mode", + "session", NULL); + virCommandSetErrorBuffer(cmd, &error); if (virCommandRunRegex(cmd, 1, regexes, vars, virISCSIExtractSession, - &cbdata, NULL, NULL) < 0) + &cbdata, NULL, &exitstatus) < 0) goto cleanup; if (cbdata.session == NULL && !probe) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot find session")); - goto cleanup; + /* If the command failed, let's give some information as to why */ + if (exitstatus != 0) { + char *st = virProcessTranslateStatus(exitstatus); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s --mode session' unexpected %s%s%s"), + ISCSIADM, NULLSTR(st), + error ? ": " : "", + error ? error : ""); + VIR_FREE(st); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot find session")); + } } cleanup: + VIR_FREE(error); virCommandFree(cmd); return cbdata.session; } -- 2.5.5
On Mon, May 16, 2016 at 11:03:09 -0400, John Ferlan wrote:
Utilize the exit status parameter for virCommandRunRegex in order to check the return error from the 'iscsiadm --mode session' command. Without this enabled, if there are no sessions running then virCommandRun would have displayed an error such as:
2016-05-13 15:17:15.165+0000: 10920: error : virCommandWait:2553 : internal error: Child process (iscsiadm --mode session) unexpected exit status 21: iscsiadm: No active sessions.
It is possible that for certain paths (when probe is true) we only care whether it's running or not to make certain decisions. Spitting out the error for those paths is unnecessary.
If we do have a situation where probe = false and there's an error, then display the error from iscsiadm if it's there; otherwise, default to the non descript error.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/viriscsi.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 846ea68..3627eed 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c
[...]
@@ -79,24 +80,40 @@ virISCSIGetSession(const char *devpath,
[....]
if (cbdata.session == NULL && !probe) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot find session")); - goto cleanup; + /* If the command failed, let's give some information as to why */ + if (exitstatus != 0) { + char *st = virProcessTranslateStatus(exitstatus); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s --mode session' unexpected %s%s%s"),
I still don't think this message is okay. As the error message above suggests error code 21 is returned if no active session is found which is definitely not unexpected here ... [1] Also it's quite unlikely that iscsiadm would die of any "unnatural" reasons and thus virProcessTranslateStatus would provide any helpful data.
+ ISCSIADM, NULLSTR(st), + error ? ": " : "", + error ? error : ""); + VIR_FREE(st); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot find session"));
[1] ... as this suggests.
+ } }
The direction of this patch is okay, but the error could be improved. Do you have any strong reason for keeping the message? Peter
On 05/17/2016 11:04 AM, Peter Krempa wrote:
On Mon, May 16, 2016 at 11:03:09 -0400, John Ferlan wrote:
Utilize the exit status parameter for virCommandRunRegex in order to check the return error from the 'iscsiadm --mode session' command. Without this enabled, if there are no sessions running then virCommandRun would have displayed an error such as:
2016-05-13 15:17:15.165+0000: 10920: error : virCommandWait:2553 : internal error: Child process (iscsiadm --mode session) unexpected exit status 21: iscsiadm: No active sessions.
It is possible that for certain paths (when probe is true) we only care whether it's running or not to make certain decisions. Spitting out the error for those paths is unnecessary.
If we do have a situation where probe = false and there's an error, then display the error from iscsiadm if it's there; otherwise, default to the non descript error.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/viriscsi.c | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 846ea68..3627eed 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c
[...]
@@ -79,24 +80,40 @@ virISCSIGetSession(const char *devpath,
[....]
if (cbdata.session == NULL && !probe) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "%s", _("cannot find session")); - goto cleanup; + /* If the command failed, let's give some information as to why */ + if (exitstatus != 0) { + char *st = virProcessTranslateStatus(exitstatus); + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s --mode session' unexpected %s%s%s"),
I still don't think this message is okay. As the error message above suggests error code 21 is returned if no active session is found which is definitely not unexpected here ... [1]
Also it's quite unlikely that iscsiadm would die of any "unnatural" reasons and thus virProcessTranslateStatus would provide any helpful data.
+ ISCSIADM, NULLSTR(st), + error ? ": " : "", + error ? error : ""); + VIR_FREE(st); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot find session"));
[1] ... as this suggests.
+ } }
The direction of this patch is okay, but the error could be improved. Do you have any strong reason for keeping the message?
No - beyond keeping the output the same as previously except that this message would only be displayed for the Refresh path; whereas, currently it's displayed for check, startup, and shutdown as well. I suppose all error paths could use the less than perfect second message perhaps with a tweak to add the error code to at least indicate what the error from the executed iscsiadm was. Just figured it'd be nice to actually translate that error (which just doesn't work the same when exitstatus = 0). John
On Tue, May 17, 2016 at 11:31:07 -0400, John Ferlan wrote:
On 05/17/2016 11:04 AM, Peter Krempa wrote:
On Mon, May 16, 2016 at 11:03:09 -0400, John Ferlan wrote:
[...]
+ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s --mode session' unexpected %s%s%s"),
I still don't think this message is okay. As the error message above suggests error code 21 is returned if no active session is found which is definitely not unexpected here ... [1]
Also it's quite unlikely that iscsiadm would die of any "unnatural" reasons and thus virProcessTranslateStatus would provide any helpful data.
+ ISCSIADM, NULLSTR(st), + error ? ": " : "", + error ? error : ""); + VIR_FREE(st); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot find session"));
[1] ... as this suggests.
+ } }
The direction of this patch is okay, but the error could be improved. Do you have any strong reason for keeping the message?
No - beyond keeping the output the same as previously except that this message would only be displayed for the Refresh path; whereas, currently it's displayed for check, startup, and shutdown as well. I suppose all error paths could use the less than perfect second message perhaps with a tweak to add the error code to at least indicate what the error from the executed iscsiadm was.
IMO that would be better than the current message.
Just figured it'd be nice to actually translate that error (which just doesn't work the same when exitstatus = 0).
I don't object to add the error message from iscsiadm itself to the message. That might help debugging stuff. I object that the resulting message isn't any better in case when you want to report it to the user. As I've written above. Error 21 with the message is not unexpected in this case. In terms of transatability the error message string is extremely wrong. If you as a translator see just "'%s --mode session' unexpected %s%s%s" you definitely need to go check what's happening there. The original message in the command code is wrong in terms of translatability but it's justified by being a catch-all case for every possible command execution that doesn't want to handle errors. In addition as the error string isn't actually exactly the same as in the command code (by partially adding the arguments verbatim to the message) it would need to be re-translated. "cannot find session iscsiadm session" followed by the message and/or containing the code will be probably the best case here. The function you are modifying is meant to find the session so any error is basically that it couldn't find it. The message from iscsiadm then helps debugging. Peter
On 05/18/2016 01:55 AM, Peter Krempa wrote:
On Tue, May 17, 2016 at 11:31:07 -0400, John Ferlan wrote:
On 05/17/2016 11:04 AM, Peter Krempa wrote:
On Mon, May 16, 2016 at 11:03:09 -0400, John Ferlan wrote:
[...]
+ + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s --mode session' unexpected %s%s%s"),
I still don't think this message is okay. As the error message above suggests error code 21 is returned if no active session is found which is definitely not unexpected here ... [1]
Also it's quite unlikely that iscsiadm would die of any "unnatural" reasons and thus virProcessTranslateStatus would provide any helpful data.
+ ISCSIADM, NULLSTR(st), + error ? ": " : "", + error ? error : ""); + VIR_FREE(st); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("cannot find session"));
[1] ... as this suggests.
+ } }
The direction of this patch is okay, but the error could be improved. Do you have any strong reason for keeping the message?
No - beyond keeping the output the same as previously except that this message would only be displayed for the Refresh path; whereas, currently it's displayed for check, startup, and shutdown as well. I suppose all error paths could use the less than perfect second message perhaps with a tweak to add the error code to at least indicate what the error from the executed iscsiadm was.
IMO that would be better than the current message.
Just figured it'd be nice to actually translate that error (which just doesn't work the same when exitstatus = 0).
I don't object to add the error message from iscsiadm itself to the message. That might help debugging stuff. I object that the resulting message isn't any better in case when you want to report it to the user.
As I've written above. Error 21 with the message is not unexpected in this case.
In terms of transatability the error message string is extremely wrong. If you as a translator see just "'%s --mode session' unexpected %s%s%s" you definitely need to go check what's happening there.
The original message in the command code is wrong in terms of translatability but it's justified by being a catch-all case for every possible command execution that doesn't want to handle errors.
In addition as the error string isn't actually exactly the same as in the command code (by partially adding the arguments verbatim to the message) it would need to be re-translated.
"cannot find session iscsiadm session" followed by the message and/or containing the code will be probably the best case here. The function you are modifying is meant to find the session so any error is basically that it couldn't find it. The message from iscsiadm then helps debugging.
Fair enough... Primary thought was to be consistent with previous error, but generating/using a unique error is fine... So rather than the if then else based on exit status, how about: virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find iscsiadm session: %s"), NULLSTR(error)); John
On Wed, May 18, 2016 at 07:15:54 -0400, John Ferlan wrote:
On 05/18/2016 01:55 AM, Peter Krempa wrote:
[...]
"cannot find session iscsiadm session" followed by the message and/or containing the code will be probably the best case here. The function you are modifying is meant to find the session so any error is basically that it couldn't find it. The message from iscsiadm then helps debugging.
Fair enough... Primary thought was to be consistent with previous error, but generating/using a unique error is fine... So rather than the if then else based on exit status, how about:
virReportError(VIR_ERR_INTERNAL_ERROR, _("cannot find iscsiadm session: %s"), NULLSTR(error));
That is reasonable. ACK to that including 1/4 if I didn't ACK it before.
From: Fritz Elfert <fritz@fritz-elfert.de> https://bugzilla.redhat.com/show_bug.cgi?id=1331552 Instead of disabling auto-login of all scsi targets (even those that do not "belong" to libvirt), use iscsiadm's "--op nonpersistent" during discovery of iSCSI targets (e.g. "iscsiadm --mode discovery --type sendtargets") in order to avoid the node database being altered which led to the need for the "large hammer" approach taken by commit id '3c12b654'. This commit removes the virISCSITargetAutologin adjustment (eg. the setting of node.startup to "manual"). The iscsiadm command has supported this mode of operation as of commit id 'ad873767' to open-iscsi. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/viriscsi.c | 30 ++---------------------------- tests/viriscsitest.c | 4 +++- 2 files changed, 5 insertions(+), 29 deletions(-) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 3627eed..8a0f559 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -403,24 +403,9 @@ virISCSIGetTargets(char **const groups, } -static int -virISCSITargetAutologin(const char *portal, - const char *initiatoriqn, - const char *target, - bool enable) -{ - const char *extraargv[] = { "--op", "update", - "--name", "node.startup", - "--value", enable ? "automatic" : "manual", - NULL }; - - return virISCSIConnection(portal, initiatoriqn, target, extraargv); -} - - int virISCSIScanTargets(const char *portal, - const char *initiatoriqn, + const char *initiatoriqn ATTRIBUTE_UNUSED, size_t *ntargetsret, char ***targetsret) { @@ -445,6 +430,7 @@ virISCSIScanTargets(const char *portal, "--mode", "discovery", "--type", "sendtargets", "--portal", portal, + "--op", "nonpersistent", NULL); memset(&list, 0, sizeof(list)); @@ -457,18 +443,6 @@ virISCSIScanTargets(const char *portal, &list, NULL, NULL) < 0) goto cleanup; - for (i = 0; i < list.ntargets; i++) { - /* We have to ignore failure, because we can't undo - * the results of 'sendtargets', unless we go scrubbing - * around in the dirt in /var/lib/iscsi. - */ - if (virISCSITargetAutologin(portal, - initiatoriqn, - list.targets[i], false) < 0) - VIR_WARN("Unable to disable auto-login on iSCSI target %s: %s", - portal, list.targets[i]); - } - if (ntargetsret && targetsret) { *ntargetsret = list.ntargets; *targetsret = list.targets; diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c index c697a4a..b5b0e20 100644 --- a/tests/viriscsitest.c +++ b/tests/viriscsitest.c @@ -90,7 +90,9 @@ static void testIscsiadmCb(const char *const*args, args[4] && STREQ(args[4], "sendtargets") && args[5] && STREQ(args[5], "--portal") && args[6] && STREQ(args[6], "10.20.30.40:3260,1") && - args[7] == NULL) { + args[7] && STREQ(args[7], "--op") && + args[8] && STREQ(args[8], "nonpersistent") && + args[9] == NULL) { ignore_value(VIR_STRDUP(*output, iscsiadmSendtargetsOutput)); } else { *status = -1; -- 2.5.5
On Mon, May 16, 2016 at 11:03:10 -0400, John Ferlan wrote:
From: Fritz Elfert <fritz@fritz-elfert.de>
https://bugzilla.redhat.com/show_bug.cgi?id=1331552
Instead of disabling auto-login of all scsi targets (even those that do not "belong" to libvirt), use iscsiadm's "--op nonpersistent" during discovery of iSCSI targets (e.g. "iscsiadm --mode discovery --type sendtargets") in order to avoid the node database being altered which led to the need for the "large hammer" approach taken by commit id '3c12b654'.
commit 3c12b6542cfc994f6b9cf3aef623767bf441c3ea Author: Daniel P. Berrange <berrange@redhat.com> Date: Fri Nov 12 15:21:48 2010 +0000
This commit removes the virISCSITargetAutologin adjustment (eg. the setting of node.startup to "manual"). The iscsiadm command has supported this mode of operation as of commit id 'ad873767' to open-iscsi.
commit ad873767436f1cc242f0d4a522a2fce7133795c1 Author: Mike Christie <michaelc@cs.wisc.edu> Date: Mon Mar 22 17:28:41 2010 -0500 iscsiadm: add nonpersistent mode to discovery mode Looks like this is already around for a while so I'm okay with using the new approach now.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/util/viriscsi.c | 30 ++---------------------------- tests/viriscsitest.c | 4 +++- 2 files changed, 5 insertions(+), 29 deletions(-)
ACK
No longer necessary to have it, so remove it. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/storage/storage_backend_iscsi.c | 8 ++------ src/util/viriscsi.c | 3 +-- src/util/viriscsi.h | 1 - tests/viriscsitest.c | 3 +-- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/storage/storage_backend_iscsi.c b/src/storage/storage_backend_iscsi.c index 9e2d01e..bccfba3 100644 --- a/src/storage/storage_backend_iscsi.c +++ b/src/storage/storage_backend_iscsi.c @@ -197,9 +197,7 @@ virStorageBackendISCSIFindPoolSources(virConnectPtr conn ATTRIBUTE_UNUSED, if (!(portal = virStorageBackendISCSIPortal(source))) goto cleanup; - if (virISCSIScanTargets(portal, - source->initiator.iqn, - &ntargets, &targets) < 0) + if (virISCSIScanTargets(portal, &ntargets, &targets) < 0) goto cleanup; if (VIR_ALLOC_N(list.sources, ntargets) < 0) @@ -399,9 +397,7 @@ virStorageBackendISCSIStartPool(virConnectPtr conn, * iscsiadm doesn't let you login to a target, unless you've * first issued a 'sendtargets' command to the portal :-( */ - if (virISCSIScanTargets(portal, - pool->def->source.initiator.iqn, - NULL, NULL) < 0) + if (virISCSIScanTargets(portal, NULL, NULL) < 0) goto cleanup; if (virStorageBackendISCSISetAuth(portal, conn, &pool->def->source) < 0) diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c index 8a0f559..2b452c5 100644 --- a/src/util/viriscsi.c +++ b/src/util/viriscsi.c @@ -320,7 +320,7 @@ virISCSIConnection(const char *portal, * portal. Without the sendtargets all that is received is a * "iscsiadm: No records found" */ - if (virISCSIScanTargets(portal, initiatoriqn, NULL, NULL) < 0) + if (virISCSIScanTargets(portal, NULL, NULL) < 0) goto cleanup; break; @@ -405,7 +405,6 @@ virISCSIGetTargets(char **const groups, int virISCSIScanTargets(const char *portal, - const char *initiatoriqn ATTRIBUTE_UNUSED, size_t *ntargetsret, char ***targetsret) { diff --git a/src/util/viriscsi.h b/src/util/viriscsi.h index f4093f7..459249e 100644 --- a/src/util/viriscsi.h +++ b/src/util/viriscsi.h @@ -49,7 +49,6 @@ virISCSIRescanLUNs(const char *session) int virISCSIScanTargets(const char *portal, - const char *initiatoriqn, size_t *ntargetsret, char ***targetsret) ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; diff --git a/tests/viriscsitest.c b/tests/viriscsitest.c index b5b0e20..40e4d10 100644 --- a/tests/viriscsitest.c +++ b/tests/viriscsitest.c @@ -145,8 +145,7 @@ testISCSIScanTargets(const void *data) virCommandSetDryRun(NULL, testIscsiadmCb, NULL); - if (virISCSIScanTargets(info->portal, NULL, - &ntargets, &targets) < 0) + if (virISCSIScanTargets(info->portal, &ntargets, &targets) < 0) goto cleanup; if (info->nexpected != ntargets) { -- 2.5.5
participants (2)
-
John Ferlan -
Peter Krempa