On 07/03/2018 02:42 PM, John Ferlan wrote: > > > 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@xxxxxxxxxx> >>>> --- >>>> 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. Maybe it did. Maybe iscsiadm used to use all interfaces (which okay, sounds unlikely) for sendtargets. Or it remembered that we added a special interface for $PORTAL and preferred that. I have no idea. But it is clearly not working now. > 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. Well, I can merge them. It's just that I wanted these virISCSI* helpers to be generic enough so that when one day somebody wants to persists targets for default connection they can do that. I view @ifacename and @persist arguments as orthogonal. It's only the usage of APIs that makes the arguments mutually exclusive (or tied together or what). > 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. They shouldn't. That is why libvirt creates its own interface. > > 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. > I'll look into it. Michal -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
