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

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(a)redhat.com&gt; >>> --- >>> 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.