[libvirt] [PATCH 4/5] virISCSIScanTargets: Honour iSCSI interface

[Michal Prívozník]

On 07/03/2018 02:38 PM, John Ferlan wrote:
> 
> 
> On 07/03/2018 01:08 AM, Michal Prívozník wrote:
>> 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.
>>
> 
> FWIW: To a degree this is restoring the initiatoriqn argument that
> commit 027986f5 removed...

Yes, but I did not think it is worth mentioning anywhere. We change code
all the time, and if we were even to blame which commits caused what ..
it's a endless spiral IMO.

> 
> [...]
> 
>>>> +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.
>>
> 
> Oh right, the virStorageBackendCreateIfaceIQN call which creates that
> libvirt-iface-* name. That's just one of those context things that you
> know when you've been working through the code ;-).
> 
> Still without the next patch, does this work?

Not really because even though targets are refreshed their configs are
not saved and thus login still fails.

Michal