Re: [libvirt] [PATCH 4/5] virISCSIScanTargets: Honour iSCSI interface
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