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@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@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;