[libvirt] [PATCH 5/5] virISCSIScanTargets: Allow making targets persistent

[John Ferlan]

On 07/03/2018 01:08 AM, Michal Prívozník wrote:
> On 07/03/2018 01:40 AM, John Ferlan wrote:
>>
>>
>> On 06/29/2018 11:01 AM, Michal Privoznik wrote:
>>> After new iSCSI interface is successfully set up, we issue
>>
>> s/new/a new/
>> s/issue/issue a/
>>
>>> sendtargets command. However, after 56057900dc53df490d we don't
>>> update the host config which in turn makes login fail because
>>> iscsiadm is unable to find any matching record for the interface.
>>>
>>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>>> ---
>>>  src/storage/storage_backend_iscsi.c |  1 +
>>>  src/util/viriscsi.c                 | 21 ++++++++++++++++++---
>>>  src/util/viriscsi.h                 |  1 +
>>>  tests/viriscsitest.c                |  3 ++-
>>>  4 files changed, 22 insertions(+), 4 deletions(-)
>>>
>>
>> Like the previous patch - is there a specific bug or something that led
>> you down this path?  Can you show an example of the existing code that's
>> creating a bad command line and generating an error and then how this
>> fixes things.  It's not like we have tests and for this stuff it's
>> really nice to have plenty of examples.
> 
> So here is the run without my patches:
> 
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode session
> iscsiadm: No active sessions.
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --targetname $TARGET --op new
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.authmethod --value CHAP
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.username --value $USER
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.password --value $PASS
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface --interface libvirt-iface-03316143 --op new
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface --interface libvirt-iface-03316143 --op update --name iface.initiatorname --value $INITIATOR
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode discovery --type sendtargets --portal $PORTAL:3260,1 --op nonpersistent
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --targetname $TARGET --login --interface libvirt-iface-03316143
> error : virCommandWait:2600 : internal error: Child process (iscsiadm --mode node --portal $PORTAL:3260,1 --targetname $TARGET --login --interface libvirt-iface-03316143) unexpected exit status 21:
> iscsiadm: No records found
> iscsiadm: No records found
> 
> 
> And with my patches:
> 
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode session
> iscsiadm: No active sessions.
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --targetname $TARGET --op new
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.authmethod --value CHAP
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.username --value $USER
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --target $TARGET --op update --name node.session.auth.password --value $PASS
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface --interface libvirt-iface-28727243 --op new
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface --interface libvirt-iface-28727243 --op update --name iface.initiatorname --value $INITIATOR
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode iface
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode discovery --type sendtargets --portal $PORTAL:3260,1 --interface libvirt-iface-28727243
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode node --portal $PORTAL:3260,1 --targetname $TARGET --login --interface libvirt-iface-28727243
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode session
> debug : virCommandRunAsync:2476 : About to run iscsiadm --mode session -r 1 -R
> 
> 

So the example helps me understand - thanks! The difference of discovery
being:

  fail: iscsiadm --mode discovery --type sendtargets \
                 --portal $PORTAL:3260,1 \
                 --op nonpersistent

  succ: iscsiadm --mode discovery --type sendtargets \
                 --portal $PORTAL:3260,1 \
                 --interface libvirt-iface-28727243

So, what commit 56057900d did to "help" or "fix" auto-login for sessions
that do not "belong to" libvirt is being called out as causing problems
for the initiatoriqn sessions. Prior to that commit the command was:

    iscsiadm --mode discovery --type sendtargets --portal $PORTAL:3260,1

So, I have to wonder "how" or "if" discovery using an initiatoriqn
really worked since there wasn't an --interface argument. Not sure I
have the patience/desire to go through the history of refactors and
adjustments since 6aabcb5 though ;-). Although it does appear that the
original code would make a "node" during it's connection period rather
than doing a sendtargets using the --interface option (and whether that
option existed at the time is a different search through a different set
of code).

Given all of what's been discovered here - I think patch 4 and 5 should
be combined. And rather than adding a new "bool persist" parameter that
only is true when initiatoriqn is passed, let's use the fact that
initiatoriqn was passed in order to:

if (ifacename)
    virCommandAddArgList(cmd, "--interface", ifacename, NULL);
else
    virCommandAddArgList(cmd, "--op", "nonpersistent", NULL);

Then as long as non initiatoriqn sessions still work, even those without
a libvirt pool associated - we should be able to declare victory. Not
sure "if" non libvirt pool initiatoriqn sessions would be affected by
all this.

John

BTW: Based on that commit, I wonder if you can modify viriscsitest.c a
bit in order to generate the initiatoriqn output as well via a change to
testIscsiadmCb.