[libvirt] [PATCH 5/6] iscsi: Inhibit autologin for only libvirt managed targets
John Ferlan
jferlan at redhat.com
Mon May 16 12:49:17 UTC 2016
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 at 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 at 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;
>
More information about the libvir-list
mailing list