On 05/13/2016 05:29 PM, John Ferlan wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1331552
Based on code originally posted by Fritz Elfert <fritz(a)fritz-elfert.de>
to remove the Autologin code entirely from libvirt, but reworked to only
set Autologin for libvirt managed targets.
Commit id '3c12b654' took a "large hammer" approach to inhibiting
logins
which causes issues if there are iSCSI targets not being managed by libvirt.
Now that the previous commit ensures that the iscsi initiator doesn't update
the /var/lib/iscsi tree with the results for a 'sendtargets' by using the
"--op nonpersistent" option, let's remove the code from
virISCSIScanTargets
that disables autologin for every target, but add that same setting into
the start pool code for each managed/started target to ensure that nothing
else goes and tries to autologin.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/storage/storage_backend_iscsi.c | 11 +++++++++++
src/util/viriscsi.c | 15 ++-------------
2 files changed, 13 insertions(+), 13 deletions(-)
Over the weekend I got more "data" on this from Fritz...
Seems with targets implemented by external SAN hardware, the host has a
single iSCSI initiator with multiple targets. The claim is libvirt
doesn't need the targets in manual mode. I'm still trying to process
that so patches 3-5 could need more adjustment. The adjustment being the
removal of the autologin code. Since patch 4 would add the "--op
nonpersistent" to the listing command line, the adjustment made to the
/var/lib/iscsi doesn't happen. Still trying to process that feedback. I
did find that even with this patch applied, if something runs the
"--mode discovery" without the "--op nonpersistent" (something other
than libvirt), then the mode in the /var/lib/iscsi goes *back to*
automatic. So in a way, it doesn't matter what we set this to, it can
be overridden. So what's the point of setting it then <sigh>. Was
really trying to follow the spirit of the original change...
John
diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
index 9e2d01e..5fbf390 100644
--- a/src/storage/storage_backend_iscsi.c
+++ b/src/storage/storage_backend_iscsi.c
@@ -404,6 +404,17 @@ virStorageBackendISCSIStartPool(virConnectPtr conn,
NULL, NULL) < 0)
goto cleanup;
+ /* Inhibit our autologin for our managed source device */
+ if (virISCSITargetAutologin(portal,
+ pool->def->source.initiator.iqn,
+ pool->def->source.devices[0].path,
+ false) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("failed to inhibit autologin for target
'%s'"),
+ pool->def->source.devices[0].path);
+ goto cleanup;
+ }
+
if (virStorageBackendISCSISetAuth(portal, conn, &pool->def->source)
< 0)
goto cleanup;
diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
index 36612c5..3133d88 100644
--- a/src/util/viriscsi.c
+++ b/src/util/viriscsi.c
@@ -415,13 +415,14 @@ virISCSITargetAutologin(const char *portal,
"--value", enable ? "automatic" :
"manual",
NULL };
+ VIR_DEBUG("set autologin for '%s' to '%d'", target,
enable);
return virISCSIConnection(portal, initiatoriqn, target, extraargv);
}
int
virISCSIScanTargets(const char *portal,
- const char *initiatoriqn,
+ const char *initiatoriqn ATTRIBUTE_UNUSED,
size_t *ntargetsret,
char ***targetsret)
{
@@ -459,18 +460,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;