Re: [libvirt] [PATCH 4/5] virISCSIScanTargets: Honour iSCSI interface
On 07/03/2018 01:38 AM, John Ferlan wrote:
On 06/29/2018 11:01 AM, Michal Privoznik wrote:
> When scanning for targets, iSCSI might give different results
> depending on the interface used. This is basically just name of
assuming this means whether the initiatoriqn interface was used
Yes.
> config file under /etc/iscsi/ifaces to use. The file contains
> initiator IQN thus different results claim.
>
Strange to have activity here - was there a bz? Or something found by
the (I assume) GSoC project:
https://www.redhat.com/archives/libvir-list/2018-June/msg00249.html
Touching something that's been avoided for 8 years is always scary, but
if it's broken, then sure it ought to be fixed.
There is no BZ. And yes, the GSoC project got me experimenting (because
for libiscsi backend we will have to make IQN required since libiscsi
does not parse anything from /etc/iscsi nor initiatorname.iscsi). And
while experimenting I've tired to put my own IQN into iscsi pool we
already have and found this bug.
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/storage/storage_backend_iscsi.c | 4 +-
> src/util/viriscsi.c | 78 ++++++++++++++++++++++++++++++++++---
> src/util/viriscsi.h | 1 +
> tests/viriscsitest.c | 2 +-
> 4 files changed, 77 insertions(+), 8 deletions(-)
>
This patch fails to compile for me:
In file included from util/viriscsi.c:32:0:
util/viriscsi.c: In function 'virISCSIScanTargets':
util/virerror.h:187:5: error: this statement may fall through
[-Werror=implicit-fallthrough=]
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
__FUNCTION__, __LINE__, __VA_ARGS__)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
util/viriscsi.c:479:13: note: in expansion of macro 'virReportError'
virReportError(VIR_ERR_OPERATION_FAILED,
^~~~~~~~~~~~~~
util/viriscsi.c:482:9: note: here
case IQN_ERROR:
Ah, okay. My gcc doesn't warn me about this. Wonder if I have something
misconfigured.
^~~~
> diff --git a/src/storage/storage_backend_iscsi.c
b/src/storage/storage_backend_iscsi.c
> index 7871d1915b..3b9dddb4fd 100644
> --- a/src/storage/storage_backend_iscsi.c
> +++ b/src/storage/storage_backend_iscsi.c
> @@ -194,7 +194,9 @@ virStorageBackendISCSIFindPoolSources(const char *srcSpec,
> if (!(portal = virStorageBackendISCSIPortal(source)))
> goto cleanup;
>
> - if (virISCSIScanTargets(portal, &ntargets, &targets) < 0)
> + if (virISCSIScanTargets(portal,
> + source->initiator.iqn,
NIT: This could go on the previous line
> + &ntargets, &targets) < 0)
> goto cleanup;
>
> if (VIR_ALLOC_N(list.sources, ntargets) < 0)
> diff --git a/src/util/viriscsi.c b/src/util/viriscsi.c
> index 44788056fd..365669aac2 100644
> --- a/src/util/viriscsi.c
> +++ b/src/util/viriscsi.c
> @@ -40,6 +40,13 @@
> VIR_LOG_INIT("util.iscsi");
>
>
> +static int
> +virISCSIScanTargetsInternal(const char *portal,
> + const char *ifacename,
> + size_t *ntargetsret,
> + char ***targetsret);
> +
> +
> struct virISCSISessionData {
> char *session;
> const char *devpath;
> @@ -286,9 +293,10 @@ virISCSIConnection(const char *portal,
> * iscsiadm doesn't let you send commands to the Interface IQN,
> * unless you've first issued a 'sendtargets' command to
the
> * portal. Without the sendtargets all that is received is a
> - * "iscsiadm: No records found"
> + * "iscsiadm: No records found". However, we must ensure that
> + * the command is issued over interface name we invented above.
"invented" - again is this the make sure it's issued over the
initiatoriqn interface?
Yes. And we construct the interface name at the beginning of this function.
Michal